qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] hw/arm/bcm283x: Initialize CPU objects in SoC common DeviceRealize()
@ 2025-10-29  7:52 Philippe Mathieu-Daudé
  2025-10-29  8:01 ` Thomas Huth
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-29  7:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Thomas Huth, Eduardo Habkost,
	Philippe Mathieu-Daudé, Cédric Le Goater, Paolo Bonzini,
	Markus Armbruster, qemu-arm, Pierrick Bouvier

QOM .instance_init() handler can not fail. QDev DeviceRealize
can.

The device-introspect QTest enumerates all QDev types and
instantiate each one, without realizing it, then introspects
the instance properties.

When switching to a single QEMU binary, all QDev types are
available in the binary, but only a filtered subset might be
available, depending on which previous target the binary is
trying to mimic.

In particular with the Raspi machines, the TYPE_RASPI4B_MACHINE
and ARM_CPU_TYPE_NAME("cortex-a72") will be built in the
qemu-system-arm binary, while not available (because filtered
as being 64-bit, for the qemu-system-aarch64 binary).

However the TYPE_BCM2838 SoC is not filtered out, and will
abort when being initialized, because the "cortex-a72" CPU type
is filtered out, leading to device-introspect failure:

  1/1 qemu:qtest+qtest-arm / qtest-arm/device-introspect-test        ERROR            2.46s   killed by signal 6 SIGABRT
  stderr:
  unknown type 'cortex-a72-arm-cpu'
  Broken pipe
  ../../tests/qtest/libqtest.c:199: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0)
  (test program exited with status code -6)
  TAP parsing error: Too few tests run (expected 167, got 5)

In order to avoid that, move the CPU *initialization* in the
SoC DeviceRealize handler, so the SoC initialization won't
fail, while realization still will.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/bcm2836.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index cd61ba15054..6e4066f137d 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -25,12 +25,7 @@ static void bcm283x_base_init(Object *obj)
 {
     BCM283XBaseState *s = BCM283X_BASE(obj);
     BCM283XBaseClass *bc = BCM283X_BASE_GET_CLASS(obj);
-    int n;
 
-    for (n = 0; n < bc->core_count; n++) {
-        object_initialize_child(obj, "cpu[*]", &s->cpu[n].core,
-                                bc->cpu_type);
-    }
     if (bc->core_count > 1) {
         qdev_property_add_static(DEVICE(obj), &bcm2836_enabled_cores_property);
         qdev_prop_set_uint32(DEVICE(obj), "enabled-cpus", bc->core_count);
@@ -65,6 +60,11 @@ bool bcm283x_common_realize(DeviceState *dev, BCMSocPeripheralBaseState *ps,
     BCM283XBaseClass *bc = BCM283X_BASE_GET_CLASS(dev);
     Object *obj;
 
+    for (int n = 0; n < bc->core_count; n++) {
+        object_initialize_child(OBJECT(dev), "cpu[*]", &s->cpu[n].core,
+                                bc->cpu_type);
+    }
+
     /* common peripherals from bcm2835 */
 
     obj = object_property_get_link(OBJECT(dev), "ram", &error_abort);
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] hw/arm/bcm283x: Initialize CPU objects in SoC common DeviceRealize()
  2025-10-29  7:52 [PATCH] hw/arm/bcm283x: Initialize CPU objects in SoC common DeviceRealize() Philippe Mathieu-Daudé
@ 2025-10-29  8:01 ` Thomas Huth
  2025-10-29  8:01 ` Manos Pitsidianakis
  2025-10-29 14:14 ` Peter Maydell
  2 siblings, 0 replies; 5+ messages in thread
From: Thomas Huth @ 2025-10-29  8:01 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Peter Maydell, Eduardo Habkost, Cédric Le Goater,
	Paolo Bonzini, Markus Armbruster, qemu-arm, Pierrick Bouvier

On 29/10/2025 08.52, Philippe Mathieu-Daudé wrote:
> QOM .instance_init() handler can not fail. QDev DeviceRealize
> can.
> 
> The device-introspect QTest enumerates all QDev types and
> instantiate each one, without realizing it, then introspects
> the instance properties.
> 
> When switching to a single QEMU binary, all QDev types are
> available in the binary, but only a filtered subset might be
> available, depending on which previous target the binary is
> trying to mimic.
> 
> In particular with the Raspi machines, the TYPE_RASPI4B_MACHINE
> and ARM_CPU_TYPE_NAME("cortex-a72") will be built in the
> qemu-system-arm binary, while not available (because filtered
> as being 64-bit, for the qemu-system-aarch64 binary).
> 
> However the TYPE_BCM2838 SoC is not filtered out, and will
> abort when being initialized, because the "cortex-a72" CPU type
> is filtered out, leading to device-introspect failure:
> 
>    1/1 qemu:qtest+qtest-arm / qtest-arm/device-introspect-test        ERROR            2.46s   killed by signal 6 SIGABRT
>    stderr:
>    unknown type 'cortex-a72-arm-cpu'
>    Broken pipe
>    ../../tests/qtest/libqtest.c:199: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0)
>    (test program exited with status code -6)
>    TAP parsing error: Too few tests run (expected 167, got 5)
> 
> In order to avoid that, move the CPU *initialization* in the
> SoC DeviceRealize handler, so the SoC initialization won't
> fail, while realization still will.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/bcm2836.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index cd61ba15054..6e4066f137d 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -25,12 +25,7 @@ static void bcm283x_base_init(Object *obj)
>   {
>       BCM283XBaseState *s = BCM283X_BASE(obj);
>       BCM283XBaseClass *bc = BCM283X_BASE_GET_CLASS(obj);
> -    int n;
>   
> -    for (n = 0; n < bc->core_count; n++) {
> -        object_initialize_child(obj, "cpu[*]", &s->cpu[n].core,
> -                                bc->cpu_type);
> -    }
>       if (bc->core_count > 1) {
>           qdev_property_add_static(DEVICE(obj), &bcm2836_enabled_cores_property);
>           qdev_prop_set_uint32(DEVICE(obj), "enabled-cpus", bc->core_count);
> @@ -65,6 +60,11 @@ bool bcm283x_common_realize(DeviceState *dev, BCMSocPeripheralBaseState *ps,
>       BCM283XBaseClass *bc = BCM283X_BASE_GET_CLASS(dev);
>       Object *obj;
>   
> +    for (int n = 0; n < bc->core_count; n++) {
> +        object_initialize_child(OBJECT(dev), "cpu[*]", &s->cpu[n].core,
> +                                bc->cpu_type);
> +    }
> +
>       /* common peripherals from bcm2835 */
>   
>       obj = object_property_get_link(OBJECT(dev), "ram", &error_abort);

Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] hw/arm/bcm283x: Initialize CPU objects in SoC common DeviceRealize()
  2025-10-29  7:52 [PATCH] hw/arm/bcm283x: Initialize CPU objects in SoC common DeviceRealize() Philippe Mathieu-Daudé
  2025-10-29  8:01 ` Thomas Huth
@ 2025-10-29  8:01 ` Manos Pitsidianakis
  2025-10-29 14:14 ` Peter Maydell
  2 siblings, 0 replies; 5+ messages in thread
From: Manos Pitsidianakis @ 2025-10-29  8:01 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Peter Maydell, Thomas Huth, Eduardo Habkost,
	Cédric Le Goater, Paolo Bonzini, Markus Armbruster, qemu-arm,
	Pierrick Bouvier

On Wed, Oct 29, 2025 at 9:54 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> QOM .instance_init() handler can not fail. QDev DeviceRealize
> can.
>
> The device-introspect QTest enumerates all QDev types and
> instantiate each one, without realizing it, then introspects
> the instance properties.
>
> When switching to a single QEMU binary, all QDev types are
> available in the binary, but only a filtered subset might be
> available, depending on which previous target the binary is
> trying to mimic.
>
> In particular with the Raspi machines, the TYPE_RASPI4B_MACHINE
> and ARM_CPU_TYPE_NAME("cortex-a72") will be built in the
> qemu-system-arm binary, while not available (because filtered
> as being 64-bit, for the qemu-system-aarch64 binary).
>
> However the TYPE_BCM2838 SoC is not filtered out, and will
> abort when being initialized, because the "cortex-a72" CPU type
> is filtered out, leading to device-introspect failure:
>
>   1/1 qemu:qtest+qtest-arm / qtest-arm/device-introspect-test        ERROR            2.46s   killed by signal 6 SIGABRT
>   stderr:
>   unknown type 'cortex-a72-arm-cpu'
>   Broken pipe
>   ../../tests/qtest/libqtest.c:199: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0)
>   (test program exited with status code -6)
>   TAP parsing error: Too few tests run (expected 167, got 5)
>
> In order to avoid that, move the CPU *initialization* in the
> SoC DeviceRealize handler, so the SoC initialization won't
> fail, while realization still will.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

>  hw/arm/bcm2836.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index cd61ba15054..6e4066f137d 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -25,12 +25,7 @@ static void bcm283x_base_init(Object *obj)
>  {
>      BCM283XBaseState *s = BCM283X_BASE(obj);
>      BCM283XBaseClass *bc = BCM283X_BASE_GET_CLASS(obj);
> -    int n;
>
> -    for (n = 0; n < bc->core_count; n++) {
> -        object_initialize_child(obj, "cpu[*]", &s->cpu[n].core,
> -                                bc->cpu_type);
> -    }
>      if (bc->core_count > 1) {
>          qdev_property_add_static(DEVICE(obj), &bcm2836_enabled_cores_property);
>          qdev_prop_set_uint32(DEVICE(obj), "enabled-cpus", bc->core_count);
> @@ -65,6 +60,11 @@ bool bcm283x_common_realize(DeviceState *dev, BCMSocPeripheralBaseState *ps,
>      BCM283XBaseClass *bc = BCM283X_BASE_GET_CLASS(dev);
>      Object *obj;
>
> +    for (int n = 0; n < bc->core_count; n++) {
> +        object_initialize_child(OBJECT(dev), "cpu[*]", &s->cpu[n].core,
> +                                bc->cpu_type);
> +    }
> +
>      /* common peripherals from bcm2835 */
>
>      obj = object_property_get_link(OBJECT(dev), "ram", &error_abort);
> --
> 2.51.0
>
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] hw/arm/bcm283x: Initialize CPU objects in SoC common DeviceRealize()
  2025-10-29  7:52 [PATCH] hw/arm/bcm283x: Initialize CPU objects in SoC common DeviceRealize() Philippe Mathieu-Daudé
  2025-10-29  8:01 ` Thomas Huth
  2025-10-29  8:01 ` Manos Pitsidianakis
@ 2025-10-29 14:14 ` Peter Maydell
  2025-10-30  9:54   ` Thomas Huth
  2 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2025-10-29 14:14 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Thomas Huth, Eduardo Habkost, Cédric Le Goater,
	Paolo Bonzini, Markus Armbruster, qemu-arm, Pierrick Bouvier

On Wed, 29 Oct 2025 at 07:53, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> QOM .instance_init() handler can not fail. QDev DeviceRealize
> can.
>
> The device-introspect QTest enumerates all QDev types and
> instantiate each one, without realizing it, then introspects
> the instance properties.
>
> When switching to a single QEMU binary, all QDev types are
> available in the binary, but only a filtered subset might be
> available, depending on which previous target the binary is
> trying to mimic.
>
> In particular with the Raspi machines, the TYPE_RASPI4B_MACHINE
> and ARM_CPU_TYPE_NAME("cortex-a72") will be built in the
> qemu-system-arm binary, while not available (because filtered
> as being 64-bit, for the qemu-system-aarch64 binary).
>
> However the TYPE_BCM2838 SoC is not filtered out, and will
> abort when being initialized, because the "cortex-a72" CPU type
> is filtered out, leading to device-introspect failure:
>
>   1/1 qemu:qtest+qtest-arm / qtest-arm/device-introspect-test        ERROR            2.46s   killed by signal 6 SIGABRT
>   stderr:
>   unknown type 'cortex-a72-arm-cpu'
>   Broken pipe
>   ../../tests/qtest/libqtest.c:199: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0)
>   (test program exited with status code -6)
>   TAP parsing error: Too few tests run (expected 167, got 5)
>
> In order to avoid that, move the CPU *initialization* in the
> SoC DeviceRealize handler, so the SoC initialization won't
> fail, while realization still will.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/arm/bcm2836.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index cd61ba15054..6e4066f137d 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -25,12 +25,7 @@ static void bcm283x_base_init(Object *obj)
>  {
>      BCM283XBaseState *s = BCM283X_BASE(obj);
>      BCM283XBaseClass *bc = BCM283X_BASE_GET_CLASS(obj);
> -    int n;
>
> -    for (n = 0; n < bc->core_count; n++) {
> -        object_initialize_child(obj, "cpu[*]", &s->cpu[n].core,
> -                                bc->cpu_type);
> -    }
>      if (bc->core_count > 1) {
>          qdev_property_add_static(DEVICE(obj), &bcm2836_enabled_cores_property);
>          qdev_prop_set_uint32(DEVICE(obj), "enabled-cpus", bc->core_count);
> @@ -65,6 +60,11 @@ bool bcm283x_common_realize(DeviceState *dev, BCMSocPeripheralBaseState *ps,
>      BCM283XBaseClass *bc = BCM283X_BASE_GET_CLASS(dev);
>      Object *obj;
>
> +    for (int n = 0; n < bc->core_count; n++) {
> +        object_initialize_child(OBJECT(dev), "cpu[*]", &s->cpu[n].core,
> +                                bc->cpu_type);
> +    }
> +

This seems a bit odd to me. Yes, object instance_init isn't
allowed to fail. But it's OK for one object to init another
in its own init method, exactly because of this. And even
if we do move this, the failure won't cause the realize
method to fail cleanly, because object_initialize_child()
doesn't return a failure message.

The problem as described in the commit message seems to be
fairly general: we have effectively blacklisted some types
as "not really creatable", but we haven't got a mechanism for
propagating that to other types that unconditionally use those.
Working around this problem by moving child init from
init to realize in parent classes is going to result in
a lot of weird parent classes that do work in realize that
ought to be in init.

I think we should either:
(1) find a way to propagate the "this type doesn't really
exist for this binary" downwards
(2) allow the "shouldn't really exist types" to be created
programmatically, but just don't advertise them to the user.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] hw/arm/bcm283x: Initialize CPU objects in SoC common DeviceRealize()
  2025-10-29 14:14 ` Peter Maydell
@ 2025-10-30  9:54   ` Thomas Huth
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Huth @ 2025-10-30  9:54 UTC (permalink / raw)
  To: Peter Maydell, Philippe Mathieu-Daudé
  Cc: qemu-devel, Eduardo Habkost, Cédric Le Goater, Paolo Bonzini,
	Markus Armbruster, qemu-arm, Pierrick Bouvier

On 29/10/2025 15.14, Peter Maydell wrote:
> On Wed, 29 Oct 2025 at 07:53, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> QOM .instance_init() handler can not fail. QDev DeviceRealize
>> can.
>>
>> The device-introspect QTest enumerates all QDev types and
>> instantiate each one, without realizing it, then introspects
>> the instance properties.
>>
>> When switching to a single QEMU binary, all QDev types are
>> available in the binary, but only a filtered subset might be
>> available, depending on which previous target the binary is
>> trying to mimic.
>>
>> In particular with the Raspi machines, the TYPE_RASPI4B_MACHINE
>> and ARM_CPU_TYPE_NAME("cortex-a72") will be built in the
>> qemu-system-arm binary, while not available (because filtered
>> as being 64-bit, for the qemu-system-aarch64 binary).
>>
>> However the TYPE_BCM2838 SoC is not filtered out, and will
>> abort when being initialized, because the "cortex-a72" CPU type
>> is filtered out, leading to device-introspect failure:
>>
>>    1/1 qemu:qtest+qtest-arm / qtest-arm/device-introspect-test        ERROR            2.46s   killed by signal 6 SIGABRT
>>    stderr:
>>    unknown type 'cortex-a72-arm-cpu'
>>    Broken pipe
>>    ../../tests/qtest/libqtest.c:199: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0)
>>    (test program exited with status code -6)
>>    TAP parsing error: Too few tests run (expected 167, got 5)
>>
>> In order to avoid that, move the CPU *initialization* in the
>> SoC DeviceRealize handler, so the SoC initialization won't
>> fail, while realization still will.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/arm/bcm2836.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
>> index cd61ba15054..6e4066f137d 100644
>> --- a/hw/arm/bcm2836.c
>> +++ b/hw/arm/bcm2836.c
>> @@ -25,12 +25,7 @@ static void bcm283x_base_init(Object *obj)
>>   {
>>       BCM283XBaseState *s = BCM283X_BASE(obj);
>>       BCM283XBaseClass *bc = BCM283X_BASE_GET_CLASS(obj);
>> -    int n;
>>
>> -    for (n = 0; n < bc->core_count; n++) {
>> -        object_initialize_child(obj, "cpu[*]", &s->cpu[n].core,
>> -                                bc->cpu_type);
>> -    }
>>       if (bc->core_count > 1) {
>>           qdev_property_add_static(DEVICE(obj), &bcm2836_enabled_cores_property);
>>           qdev_prop_set_uint32(DEVICE(obj), "enabled-cpus", bc->core_count);
>> @@ -65,6 +60,11 @@ bool bcm283x_common_realize(DeviceState *dev, BCMSocPeripheralBaseState *ps,
>>       BCM283XBaseClass *bc = BCM283X_BASE_GET_CLASS(dev);
>>       Object *obj;
>>
>> +    for (int n = 0; n < bc->core_count; n++) {
>> +        object_initialize_child(OBJECT(dev), "cpu[*]", &s->cpu[n].core,
>> +                                bc->cpu_type);
>> +    }
>> +
> 
> This seems a bit odd to me. Yes, object instance_init isn't
> allowed to fail. But it's OK for one object to init another
> in its own init method, exactly because of this. And even
> if we do move this, the failure won't cause the realize
> method to fail cleanly, because object_initialize_child()
> doesn't return a failure message.
> 
> The problem as described in the commit message seems to be
> fairly general: we have effectively blacklisted some types
> as "not really creatable", but we haven't got a mechanism for
> propagating that to other types that unconditionally use those.
> Working around this problem by moving child init from
> init to realize in parent classes is going to result in
> a lot of weird parent classes that do work in realize that
> ought to be in init.
> 
> I think we should either:
> (1) find a way to propagate the "this type doesn't really
> exist for this binary" downwards
> (2) allow the "shouldn't really exist types" to be created
> programmatically, but just don't advertise them to the user.

Maybe we could have a DEFINE_TYPES_IF_ARCH(arch, ...) macro that uses
"if (qemu_arch_available(arch)) { ... }" internally, and then replace the 
DEFINE_TYPES() in hw/arm/bcm2836.c with that macro?

  Thomas



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-10-30  9:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-29  7:52 [PATCH] hw/arm/bcm283x: Initialize CPU objects in SoC common DeviceRealize() Philippe Mathieu-Daudé
2025-10-29  8:01 ` Thomas Huth
2025-10-29  8:01 ` Manos Pitsidianakis
2025-10-29 14:14 ` Peter Maydell
2025-10-30  9:54   ` Thomas Huth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).