diff mbox

[Xen-devel] xen: arm: ignore CPUs which are not marked available in the DT

Message ID 1f69ed8ee6a21c2a0c4d548ae83260b14c54ab36.1406133950.git.ian.campbell@citrix.com
State New
Headers show

Commit Message

Ian Campbell July 23, 2014, 4:45 p.m. UTC
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/smpboot.c |    3 +++
 1 file changed, 3 insertions(+)

Comments

Julien Grall July 23, 2014, 5:15 p.m. UTC | #1
Hi Ian,

On 07/23/2014 05:45 PM, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/smpboot.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index cf149da..4b0a738 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -134,6 +134,9 @@ void __init smp_init_cpus(void)
>          if ( !dt_device_type_is_equal(cpu, "cpu") )
>              continue;
>  
> +        if ( !dt_device_is_available(cpu) )
> +            continue;
> +

I can't find a such things on the Linux device tree bindings. Do you
have a use case where CPU are marked disabled?

Regards,

>          if ( dt_n_size_cells(cpu) != 0 )
>              printk(XENLOG_WARNING "cpu node `%s`: #size-cells %d\n",
>                     dt_node_full_name(cpu), dt_n_size_cells(cpu));
>
Ian Campbell July 23, 2014, 6:36 p.m. UTC | #2
On Wed, 2014-07-23 at 18:15 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 07/23/2014 05:45 PM, Ian Campbell wrote:
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  xen/arch/arm/smpboot.c |    3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> > index cf149da..4b0a738 100644
> > --- a/xen/arch/arm/smpboot.c
> > +++ b/xen/arch/arm/smpboot.c
> > @@ -134,6 +134,9 @@ void __init smp_init_cpus(void)
> >          if ( !dt_device_type_is_equal(cpu, "cpu") )
> >              continue;
> >  
> > +        if ( !dt_device_is_available(cpu) )
> > +            continue;
> > +
> 
> I can't find a such things on the Linux device tree bindings.

status is a generic property which is common to all nodes, it comes from
ePAPR.

>  Do you
> have a use case where CPU are marked disabled?

I use it locally when booting with models -- it allows me to turn off
cpus in the base .dts file using a wrapper instead of having to edit the
original.

Ian.
Julien Grall July 24, 2014, 10:36 a.m. UTC | #3
Hi Ian,

On 23/07/14 19:36, Ian Campbell wrote:
> On Wed, 2014-07-23 at 18:15 +0100, Julien Grall wrote:
>> On 07/23/2014 05:45 PM, Ian Campbell wrote:
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>> ---
>>>   xen/arch/arm/smpboot.c |    3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
>>> index cf149da..4b0a738 100644
>>> --- a/xen/arch/arm/smpboot.c
>>> +++ b/xen/arch/arm/smpboot.c
>>> @@ -134,6 +134,9 @@ void __init smp_init_cpus(void)
>>>           if ( !dt_device_type_is_equal(cpu, "cpu") )
>>>               continue;
>>>
>>> +        if ( !dt_device_is_available(cpu) )
>>> +            continue;
>>> +
>>
>> I can't find a such things on the Linux device tree bindings.
>
> status is a generic property which is common to all nodes, it comes from
> ePAPR.
>
>>   Do you
>> have a use case where CPU are marked disabled?
>
> I use it locally when booting with models -- it allows me to turn off
> cpus in the base .dts file using a wrapper instead of having to edit the
> original.

I read the ePAPR and the property status on CPU node. AFAIU, the 
property doesn't have this meaning for a such node.

This property means the CPU is in quiescent state and property to bring 
up the CPU is provides in the device tree node.

Section 5.5.2.2:

Before starting a client program on the boot cpu, the boot program shall 
set certain properties in the
device tree passed to the client as follows:
• Each secondary CPU’s cpu node shall have a status property with a 
value of “disabled”.

Regards,
Ian Campbell July 24, 2014, 10:40 a.m. UTC | #4
On Thu, 2014-07-24 at 11:36 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 23/07/14 19:36, Ian Campbell wrote:
> > On Wed, 2014-07-23 at 18:15 +0100, Julien Grall wrote:
> >> On 07/23/2014 05:45 PM, Ian Campbell wrote:
> >>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> >>> ---
> >>>   xen/arch/arm/smpboot.c |    3 +++
> >>>   1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> >>> index cf149da..4b0a738 100644
> >>> --- a/xen/arch/arm/smpboot.c
> >>> +++ b/xen/arch/arm/smpboot.c
> >>> @@ -134,6 +134,9 @@ void __init smp_init_cpus(void)
> >>>           if ( !dt_device_type_is_equal(cpu, "cpu") )
> >>>               continue;
> >>>
> >>> +        if ( !dt_device_is_available(cpu) )
> >>> +            continue;
> >>> +
> >>
> >> I can't find a such things on the Linux device tree bindings.
> >
> > status is a generic property which is common to all nodes, it comes from
> > ePAPR.
> >
> >>   Do you
> >> have a use case where CPU are marked disabled?
> >
> > I use it locally when booting with models -- it allows me to turn off
> > cpus in the base .dts file using a wrapper instead of having to edit the
> > original.
> 
> I read the ePAPR and the property status on CPU node. AFAIU, the 
> property doesn't have this meaning for a such node.
> 
> This property means the CPU is in quiescent state and property to bring 
> up the CPU is provides in the device tree node.
> 
> Section 5.5.2.2:
> 
> Before starting a client program on the boot cpu, the boot program shall 
> set certain properties in the
> device tree passed to the client as follows:
> • Each secondary CPU’s cpu node shall have a status property with a 
> value of “disabled”.

Oh well, it was only a convenience for me anyway.
Ian

> 
> Regards,
>
Julien Grall July 24, 2014, 10:44 a.m. UTC | #5
On 24/07/14 11:40, Ian Campbell wrote:
> On Thu, 2014-07-24 at 11:36 +0100, Julien Grall wrote:
>> Hi Ian,
>>
>> On 23/07/14 19:36, Ian Campbell wrote:
>>> On Wed, 2014-07-23 at 18:15 +0100, Julien Grall wrote:
>>>> On 07/23/2014 05:45 PM, Ian Campbell wrote:
>>>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>>>> ---
>>>>>    xen/arch/arm/smpboot.c |    3 +++
>>>>>    1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
>>>>> index cf149da..4b0a738 100644
>>>>> --- a/xen/arch/arm/smpboot.c
>>>>> +++ b/xen/arch/arm/smpboot.c
>>>>> @@ -134,6 +134,9 @@ void __init smp_init_cpus(void)
>>>>>            if ( !dt_device_type_is_equal(cpu, "cpu") )
>>>>>                continue;
>>>>>
>>>>> +        if ( !dt_device_is_available(cpu) )
>>>>> +            continue;
>>>>> +
>>>>
>>>> I can't find a such things on the Linux device tree bindings.
>>>
>>> status is a generic property which is common to all nodes, it comes from
>>> ePAPR.
>>>
>>>>    Do you
>>>> have a use case where CPU are marked disabled?
>>>
>>> I use it locally when booting with models -- it allows me to turn off
>>> cpus in the base .dts file using a wrapper instead of having to edit the
>>> original.
>>
>> I read the ePAPR and the property status on CPU node. AFAIU, the
>> property doesn't have this meaning for a such node.
>>
>> This property means the CPU is in quiescent state and property to bring
>> up the CPU is provides in the device tree node.
>>
>> Section 5.5.2.2:
>>
>> Before starting a client program on the boot cpu, the boot program shall
>> set certain properties in the
>> device tree passed to the client as follows:
>> • Each secondary CPU’s cpu node shall have a status property with a
>> value of “disabled”.
>
> Oh well, it was only a convenience for me anyway.
> Ian

Could we add a property xen,status for debugging (i.e when Xen is 
compiled with debug=y)? So we don't collapse with the actual property if 
someone decide to implement it in the bootloader.

Regards,
diff mbox

Patch

diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index cf149da..4b0a738 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -134,6 +134,9 @@  void __init smp_init_cpus(void)
         if ( !dt_device_type_is_equal(cpu, "cpu") )
             continue;
 
+        if ( !dt_device_is_available(cpu) )
+            continue;
+
         if ( dt_n_size_cells(cpu) != 0 )
             printk(XENLOG_WARNING "cpu node `%s`: #size-cells %d\n",
                    dt_node_full_name(cpu), dt_n_size_cells(cpu));