[PATCH 05/24] aspeed: Don't create unwanted "cortex-a7-arm-cpu" devices

Markus Armbruster posted 24 patches 5 years, 6 months ago
Maintainers: Sagar Karandikar <sagark@eecs.berkeley.edu>, Fabien Chouteau <chouteau@adacore.com>, Peter Maydell <peter.maydell@linaro.org>, Alistair Francis <Alistair.Francis@wdc.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Alistair Francis <alistair@alistair23.me>, Paolo Bonzini <pbonzini@redhat.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Andrzej Zaborowski <balrogg@gmail.com>, KONRAD Frederic <frederic.konrad@adacore.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Eduardo Habkost <ehabkost@redhat.com>, Laurent Vivier <laurent@vivier.eu>, David Gibson <david@gibson.dropbear.id.au>, Palmer Dabbelt <palmer@dabbelt.com>
There is a newer version of this series
[PATCH 05/24] aspeed: Don't create unwanted "cortex-a7-arm-cpu" devices
Posted by Markus Armbruster 5 years, 6 months ago
The number of CPUs is controlled by property "num-cpus".
aspeed_soc_ast2600_init() creates the maximum supported number.
aspeed_soc_ast2600_realize() realizes only the wanted number.  Works,
although it leaves unrealized devices hanging around in the QOM
composition tree.  Affects machines ast2600-evb and tacoma-bmc.

Make the init functions create only the wanted ones.  Visible in "info
qom-tree"; here's the change for ast2600-evb:

     /machine (ast2600-evb-machine)
       [...]
       /soc (ast2600-a1)
         [...]
         /cpu[0] (cortex-a7-arm-cpu)
           /unnamed-gpio-in[0] (irq)
           /unnamed-gpio-in[1] (irq)
           /unnamed-gpio-in[2] (irq)
           /unnamed-gpio-in[3] (irq)
    -    /cpu[1] (cortex-a7-arm-cpu)
    -      /unnamed-gpio-in[0] (irq)
    -      /unnamed-gpio-in[1] (irq)
    -      /unnamed-gpio-in[2] (irq)
    -      /unnamed-gpio-in[3] (irq)
         /ehci[0] (platform-ehci-usb)

Cc: "Cédric Le Goater" <clg@kaod.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: Joel Stanley <joel@jms.id.au>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/aspeed_ast2600.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 0a6a77dd54..6ffa587a7f 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -287,6 +287,9 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
             return;
         }
     }
+    for (; i < sc->num_cpus; i++) {
+        object_unparent(OBJECT(&s->cpu[i]));
+    }
 
     /* A7MPCORE */
     object_property_set_int(OBJECT(&s->a7mpcore), s->num_cpus, "num-cpu",
-- 
2.21.1


Re: [PATCH 05/24] aspeed: Don't create unwanted "cortex-a7-arm-cpu" devices
Posted by Cédric Le Goater 5 years, 6 months ago
On 5/18/20 7:03 AM, Markus Armbruster wrote:
> The number of CPUs is controlled by property "num-cpus".
> aspeed_soc_ast2600_init() creates the maximum supported number.
> aspeed_soc_ast2600_realize() realizes only the wanted number.  Works,
> although it leaves unrealized devices hanging around in the QOM
> composition tree.  Affects machines ast2600-evb and tacoma-bmc.
> 
> Make the init functions create only the wanted ones.  Visible in "info
> qom-tree"; here's the change for ast2600-evb:
> 
>      /machine (ast2600-evb-machine)
>        [...]
>        /soc (ast2600-a1)
>          [...]
>          /cpu[0] (cortex-a7-arm-cpu)
>            /unnamed-gpio-in[0] (irq)
>            /unnamed-gpio-in[1] (irq)
>            /unnamed-gpio-in[2] (irq)
>            /unnamed-gpio-in[3] (irq)
>     -    /cpu[1] (cortex-a7-arm-cpu)
>     -      /unnamed-gpio-in[0] (irq)
>     -      /unnamed-gpio-in[1] (irq)
>     -      /unnamed-gpio-in[2] (irq)
>     -      /unnamed-gpio-in[3] (irq)
>          /ehci[0] (platform-ehci-usb)
> 
> Cc: "Cédric Le Goater" <clg@kaod.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: Joel Stanley <joel@jms.id.au>
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Joel, Andrew,

Shouldn't we enforce a default/min/max number of CPUs of 2 for the AST2600 ? 
That's the SoC definition. The fact it is configurable in the Aspeed model
was nice to have during bringup but we are now done.  

Thanks,

C.

> ---
>  hw/arm/aspeed_ast2600.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 0a6a77dd54..6ffa587a7f 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -287,6 +287,9 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>              return;
>          }
>      }
> +    for (; i < sc->num_cpus; i++) {
> +        object_unparent(OBJECT(&s->cpu[i]));
> +    }
>  
>      /* A7MPCORE */
>      object_property_set_int(OBJECT(&s->a7mpcore), s->num_cpus, "num-cpu",
> 


Re: [PATCH 05/24] aspeed: Don't create unwanted "cortex-a7-arm-cpu" devices
Posted by Joel Stanley 5 years, 5 months ago
On Mon, 18 May 2020 at 12:24, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 5/18/20 7:03 AM, Markus Armbruster wrote:
> > The number of CPUs is controlled by property "num-cpus".
> > aspeed_soc_ast2600_init() creates the maximum supported number.
> > aspeed_soc_ast2600_realize() realizes only the wanted number.  Works,
> > although it leaves unrealized devices hanging around in the QOM
> > composition tree.  Affects machines ast2600-evb and tacoma-bmc.
> >
> > Make the init functions create only the wanted ones.  Visible in "info
> > qom-tree"; here's the change for ast2600-evb:
> >
> >      /machine (ast2600-evb-machine)
> >        [...]
> >        /soc (ast2600-a1)
> >          [...]
> >          /cpu[0] (cortex-a7-arm-cpu)
> >            /unnamed-gpio-in[0] (irq)
> >            /unnamed-gpio-in[1] (irq)
> >            /unnamed-gpio-in[2] (irq)
> >            /unnamed-gpio-in[3] (irq)
> >     -    /cpu[1] (cortex-a7-arm-cpu)
> >     -      /unnamed-gpio-in[0] (irq)
> >     -      /unnamed-gpio-in[1] (irq)
> >     -      /unnamed-gpio-in[2] (irq)
> >     -      /unnamed-gpio-in[3] (irq)
> >          /ehci[0] (platform-ehci-usb)
> >
> > Cc: "Cédric Le Goater" <clg@kaod.org>
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Cc: Andrew Jeffery <andrew@aj.id.au>
> > Cc: Joel Stanley <joel@jms.id.au>
> > Cc: qemu-arm@nongnu.org
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>
> Joel, Andrew,
>
> Shouldn't we enforce a default/min/max number of CPUs of 2 for the AST2600 ?
> That's the SoC definition. The fact it is configurable in the Aspeed model
> was nice to have during bringup but we are now done.

Agreed, we want there to always be two CPUs for the 2600.

>
> Thanks,
>
> C.
>
> > ---
> >  hw/arm/aspeed_ast2600.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> > index 0a6a77dd54..6ffa587a7f 100644
> > --- a/hw/arm/aspeed_ast2600.c
> > +++ b/hw/arm/aspeed_ast2600.c
> > @@ -287,6 +287,9 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> >              return;
> >          }
> >      }
> > +    for (; i < sc->num_cpus; i++) {
> > +        object_unparent(OBJECT(&s->cpu[i]));
> > +    }
> >
> >      /* A7MPCORE */
> >      object_property_set_int(OBJECT(&s->a7mpcore), s->num_cpus, "num-cpu",
> >
>

Re: [PATCH 05/24] aspeed: Don't create unwanted "cortex-a7-arm-cpu" devices
Posted by Markus Armbruster 5 years, 5 months ago
Joel Stanley <joel@jms.id.au> writes:

> On Mon, 18 May 2020 at 12:24, Cédric Le Goater <clg@kaod.org> wrote:
>>
>> On 5/18/20 7:03 AM, Markus Armbruster wrote:
>> > The number of CPUs is controlled by property "num-cpus".
>> > aspeed_soc_ast2600_init() creates the maximum supported number.
>> > aspeed_soc_ast2600_realize() realizes only the wanted number.  Works,
>> > although it leaves unrealized devices hanging around in the QOM
>> > composition tree.  Affects machines ast2600-evb and tacoma-bmc.
>> >
>> > Make the init functions create only the wanted ones.  Visible in "info
>> > qom-tree"; here's the change for ast2600-evb:
>> >
>> >      /machine (ast2600-evb-machine)
>> >        [...]
>> >        /soc (ast2600-a1)
>> >          [...]
>> >          /cpu[0] (cortex-a7-arm-cpu)
>> >            /unnamed-gpio-in[0] (irq)
>> >            /unnamed-gpio-in[1] (irq)
>> >            /unnamed-gpio-in[2] (irq)
>> >            /unnamed-gpio-in[3] (irq)
>> >     -    /cpu[1] (cortex-a7-arm-cpu)
>> >     -      /unnamed-gpio-in[0] (irq)
>> >     -      /unnamed-gpio-in[1] (irq)
>> >     -      /unnamed-gpio-in[2] (irq)
>> >     -      /unnamed-gpio-in[3] (irq)
>> >          /ehci[0] (platform-ehci-usb)
>> >
>> > Cc: "Cédric Le Goater" <clg@kaod.org>
>> > Cc: Peter Maydell <peter.maydell@linaro.org>
>> > Cc: Andrew Jeffery <andrew@aj.id.au>
>> > Cc: Joel Stanley <joel@jms.id.au>
>> > Cc: qemu-arm@nongnu.org
>> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>
>> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>>
>> Joel, Andrew,
>>
>> Shouldn't we enforce a default/min/max number of CPUs of 2 for the AST2600 ?
>> That's the SoC definition. The fact it is configurable in the Aspeed model
>> was nice to have during bringup but we are now done.
>
> Agreed, we want there to always be two CPUs for the 2600.

Follow-up patch welcome!


Re: [PATCH 05/24] aspeed: Don't create unwanted "cortex-a7-arm-cpu" devices
Posted by Cédric Le Goater 5 years, 5 months ago
On 5/19/20 7:46 AM, Markus Armbruster wrote:
> Joel Stanley <joel@jms.id.au> writes:
> 
>> On Mon, 18 May 2020 at 12:24, Cédric Le Goater <clg@kaod.org> wrote:
>>>
>>> On 5/18/20 7:03 AM, Markus Armbruster wrote:
>>>> The number of CPUs is controlled by property "num-cpus".
>>>> aspeed_soc_ast2600_init() creates the maximum supported number.
>>>> aspeed_soc_ast2600_realize() realizes only the wanted number.  Works,
>>>> although it leaves unrealized devices hanging around in the QOM
>>>> composition tree.  Affects machines ast2600-evb and tacoma-bmc.
>>>>
>>>> Make the init functions create only the wanted ones.  Visible in "info
>>>> qom-tree"; here's the change for ast2600-evb:
>>>>
>>>>      /machine (ast2600-evb-machine)
>>>>        [...]
>>>>        /soc (ast2600-a1)
>>>>          [...]
>>>>          /cpu[0] (cortex-a7-arm-cpu)
>>>>            /unnamed-gpio-in[0] (irq)
>>>>            /unnamed-gpio-in[1] (irq)
>>>>            /unnamed-gpio-in[2] (irq)
>>>>            /unnamed-gpio-in[3] (irq)
>>>>     -    /cpu[1] (cortex-a7-arm-cpu)
>>>>     -      /unnamed-gpio-in[0] (irq)
>>>>     -      /unnamed-gpio-in[1] (irq)
>>>>     -      /unnamed-gpio-in[2] (irq)
>>>>     -      /unnamed-gpio-in[3] (irq)
>>>>          /ehci[0] (platform-ehci-usb)
>>>>
>>>> Cc: "Cédric Le Goater" <clg@kaod.org>
>>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>>> Cc: Andrew Jeffery <andrew@aj.id.au>
>>>> Cc: Joel Stanley <joel@jms.id.au>
>>>> Cc: qemu-arm@nongnu.org
>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>
>>> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>>>
>>> Joel, Andrew,
>>>
>>> Shouldn't we enforce a default/min/max number of CPUs of 2 for the AST2600 ?
>>> That's the SoC definition. The fact it is configurable in the Aspeed model
>>> was nice to have during bringup but we are now done.
>>
>> Agreed, we want there to always be two CPUs for the 2600.
> 
> Follow-up patch welcome!

I just sent a patch on this topic.

C.