[RFC PATCH 0/4] hw/arm: Register target-specific QOM types at runtime

[RFC PATCH 0/4] hw/arm: Register target-specific QOM types at runtime

[Philippe Mathieu-Daudé]

In preparation of having a single binary for both ARM and
Aarch64 targets, unify raspi & aspeed by replacing the
compile-time TARGET_AARCH64 check by a QOM runtime call
on legacy_binary_is_64bit().

No behavior change with current binaries:

  $ ./qemu-system-arm -M help | fgrep raspi
  raspi0               Raspberry Pi Zero (revision 1.2)
  raspi1ap             Raspberry Pi A+ (revision 1.1)
  raspi2b              Raspberry Pi 2B (revision 1.1)

  $ ./qemu-system-aarch64 -M help | fgrep raspi
  raspi0               Raspberry Pi Zero (revision 1.2)
  raspi1ap             Raspberry Pi A+ (revision 1.1)
  raspi2b              Raspberry Pi 2B (revision 1.1)
  raspi3ap             Raspberry Pi 3A+ (revision 1.0)
  raspi3b              Raspberry Pi 3B (revision 1.2)
  raspi4b              Raspberry Pi 4B (revision 1.5)

Based-on: <20250305153929.43687-1-philmd@linaro.org>

Philippe Mathieu-Daudé (4):
  qom: Introduce TypeInfo::registerable() callback
  hw/arm/raspi: Replace TARGET_AARCH64 by legacy_binary_is_64bit()
  hw/arm/aspeed: Replace TARGET_AARCH64 by legacy_binary_is_64bit()
  hw/ppc: Remove TARGET_PPC64 use in ppc_create_page_sizes_prop()

 include/qom/object.h | 1 +
 hw/arm/aspeed.c      | 8 ++------
 hw/arm/bcm2836.c     | 6 ++----
 hw/arm/raspi.c       | 7 +++----
 hw/ppc/fdt.c         | 5 +++--
 qom/object.c         | 4 ++++
 qom/trace-events     | 1 +
 7 files changed, 16 insertions(+), 16 deletions(-)

-- 
2.47.1

[RFC PATCH 1/4] qom: Introduce TypeInfo::registerable() callback

[Philippe Mathieu-Daudé]

Introduce the TypeInfo::registerable() callback to allow
runtime decision on whether register a QOM type or not.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qom/object.h | 1 +
 qom/object.c         | 4 ++++
 qom/trace-events     | 1 +
 3 files changed, 6 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index 9192265db76..f046791f60c 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -476,6 +476,7 @@ struct TypeInfo
 {
     const char *name;
     const char *parent;
+    bool (*registerable)(void);
 
     size_t instance_size;
     size_t instance_align;
diff --git a/qom/object.c b/qom/object.c
index 01618d06bd8..c62b7fd1695 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -168,6 +168,10 @@ static TypeImpl *type_register_internal(const TypeInfo *info)
         fprintf(stderr, "Registering '%s' with illegal type name\n", info->name);
         abort();
     }
+    if (info->registerable && !info->registerable()) {
+        trace_object_register_skipped(info->name);
+        return NULL;
+    }
 
     ti = type_new(info);
 
diff --git a/qom/trace-events b/qom/trace-events
index b2e9f4a7127..29af95d8507 100644
--- a/qom/trace-events
+++ b/qom/trace-events
@@ -3,3 +3,4 @@
 # object.c
 object_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
 object_class_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
+object_register_skipped(const char *type) "Not registering '%s' type"
-- 
2.47.1

[RFC PATCH 2/4] hw/arm/raspi: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[Philippe Mathieu-Daudé]

For legacy ARM binaries, legacy_binary_is_64bit() is
equivalent of the compile time TARGET_AARCH64 definition.

Use it as TypeInfo::registerable() callback to dynamically
add Aarch64 specific types in qemu-system-aarch64 binary,
removing the need of TARGET_AARCH64 #ifdef'ry.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/bcm2836.c | 6 ++----
 hw/arm/raspi.c   | 7 +++----
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 95e16806fa1..88a32e5fc20 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -12,6 +12,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
+#include "qemu/legacy_binary_info.h"
 #include "hw/arm/bcm2836.h"
 #include "hw/arm/raspi_platform.h"
 #include "hw/sysbus.h"
@@ -195,7 +196,6 @@ static void bcm2836_class_init(ObjectClass *oc, void *data)
     dc->realize = bcm2836_realize;
 };
 
-#ifdef TARGET_AARCH64
 static void bcm2837_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -208,7 +208,6 @@ static void bcm2837_class_init(ObjectClass *oc, void *data)
     bc->clusterid = 0x0;
     dc->realize = bcm2836_realize;
 };
-#endif
 
 static const TypeInfo bcm283x_types[] = {
     {
@@ -219,12 +218,11 @@ static const TypeInfo bcm283x_types[] = {
         .name           = TYPE_BCM2836,
         .parent         = TYPE_BCM283X,
         .class_init     = bcm2836_class_init,
-#ifdef TARGET_AARCH64
     }, {
         .name           = TYPE_BCM2837,
         .parent         = TYPE_BCM283X,
+        .registerable   = legacy_binary_is_64bit,
         .class_init     = bcm2837_class_init,
-#endif
     }, {
         .name           = TYPE_BCM283X,
         .parent         = TYPE_BCM283X_BASE,
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index dce35ca11aa..f7e647a9cbf 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -15,6 +15,7 @@
 #include "qemu/osdep.h"
 #include "qemu/units.h"
 #include "qemu/cutils.h"
+#include "qemu/legacy_binary_info.h"
 #include "qapi/error.h"
 #include "hw/arm/boot.h"
 #include "hw/arm/bcm2836.h"
@@ -367,7 +368,6 @@ static void raspi2b_machine_class_init(ObjectClass *oc, void *data)
     raspi_machine_class_init(mc, rmc->board_rev);
 };
 
-#ifdef TARGET_AARCH64
 static void raspi3ap_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -387,7 +387,6 @@ static void raspi3b_machine_class_init(ObjectClass *oc, void *data)
     rmc->board_rev = 0xa02082;
     raspi_machine_class_init(mc, rmc->board_rev);
 };
-#endif /* TARGET_AARCH64 */
 
 static const TypeInfo raspi_machine_types[] = {
     {
@@ -402,16 +401,16 @@ static const TypeInfo raspi_machine_types[] = {
         .name           = MACHINE_TYPE_NAME("raspi2b"),
         .parent         = TYPE_RASPI_MACHINE,
         .class_init     = raspi2b_machine_class_init,
-#ifdef TARGET_AARCH64
     }, {
         .name           = MACHINE_TYPE_NAME("raspi3ap"),
         .parent         = TYPE_RASPI_MACHINE,
+        .registerable   = legacy_binary_is_64bit,
         .class_init     = raspi3ap_machine_class_init,
     }, {
         .name           = MACHINE_TYPE_NAME("raspi3b"),
         .parent         = TYPE_RASPI_MACHINE,
+        .registerable   = legacy_binary_is_64bit,
         .class_init     = raspi3b_machine_class_init,
-#endif
     }, {
         .name           = TYPE_RASPI_MACHINE,
         .parent         = TYPE_RASPI_BASE_MACHINE,
-- 
2.47.1

[RFC PATCH 3/4] hw/arm/aspeed: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[Philippe Mathieu-Daudé]

For legacy ARM binaries, legacy_binary_is_64bit() is
equivalent of the compile time TARGET_AARCH64 definition.

Use it as TypeInfo::registerable() callback to dynamically
add Aarch64 specific types in qemu-system-aarch64 binary,
removing the need of TARGET_AARCH64 #ifdef'ry.

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

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 98bf071139b..3f18a4501e0 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -28,6 +28,7 @@
 #include "hw/loader.h"
 #include "qemu/error-report.h"
 #include "qemu/units.h"
+#include "qemu/legacy_binary_info.h"
 #include "hw/qdev-clock.h"
 #include "system/system.h"
 
@@ -179,11 +180,9 @@ struct AspeedMachineState {
 #define AST2600_EVB_HW_STRAP1 0x000000C0
 #define AST2600_EVB_HW_STRAP2 0x00000003
 
-#ifdef TARGET_AARCH64
 /* AST2700 evb hardware value */
 #define AST2700_EVB_HW_STRAP1 0x000000C0
 #define AST2700_EVB_HW_STRAP2 0x00000003
-#endif
 
 /* Rainier hardware value: (QEMU prototype) */
 #define RAINIER_BMC_HW_STRAP1 (0x00422016 | SCU_AST2600_HW_STRAP_BOOT_SRC_EMMC)
@@ -1661,7 +1660,6 @@ static void aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
     aspeed_machine_class_init_cpus_defaults(mc);
 }
 
-#ifdef TARGET_AARCH64
 static void ast2700_evb_i2c_init(AspeedMachineState *bmc)
 {
     AspeedSoCState *soc = bmc->soc;
@@ -1690,7 +1688,6 @@ static void aspeed_machine_ast2700_evb_class_init(ObjectClass *oc, void *data)
     mc->default_ram_size = 1 * GiB;
     aspeed_machine_class_init_cpus_defaults(mc);
 }
-#endif
 
 static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
                                                      void *data)
@@ -1813,12 +1810,11 @@ static const TypeInfo aspeed_machine_types[] = {
         .name           = MACHINE_TYPE_NAME("ast1030-evb"),
         .parent         = TYPE_ASPEED_MACHINE,
         .class_init     = aspeed_minibmc_machine_ast1030_evb_class_init,
-#ifdef TARGET_AARCH64
     }, {
         .name          = MACHINE_TYPE_NAME("ast2700-evb"),
         .parent        = TYPE_ASPEED_MACHINE,
+        .registerable   = legacy_binary_is_64bit,
         .class_init    = aspeed_machine_ast2700_evb_class_init,
-#endif
     }, {
         .name          = TYPE_ASPEED_MACHINE,
         .parent        = TYPE_MACHINE,
-- 
2.47.1

[RFC PATCH 4/4] hw/ppc: Remove TARGET_PPC64 use in ppc_create_page_sizes_prop()

[Philippe Mathieu-Daudé]

Check the binary is built for 64-bit PPC at runtime,
removing the need for TARGET_PPC64 #ifdef'ry.

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

diff --git a/hw/ppc/fdt.c b/hw/ppc/fdt.c
index 0828ad72548..bae269c72ac 100644
--- a/hw/ppc/fdt.c
+++ b/hw/ppc/fdt.c
@@ -8,12 +8,12 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/legacy_binary_info.h"
 #include "target/ppc/cpu.h"
 #include "target/ppc/mmu-hash64.h"
 
 #include "hw/ppc/fdt.h"
 
-#if defined(TARGET_PPC64)
 size_t ppc_create_page_sizes_prop(PowerPCCPU *cpu, uint32_t *prop,
                                   size_t maxsize)
 {
@@ -21,6 +21,8 @@ size_t ppc_create_page_sizes_prop(PowerPCCPU *cpu, uint32_t *prop,
     int i, j, count;
     uint32_t *p = prop;
 
+    assert(legacy_binary_is_64bit());
+
     for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
         PPCHash64SegmentPageSizes *sps = &cpu->hash64_opts->sps[i];
 
@@ -46,4 +48,3 @@ size_t ppc_create_page_sizes_prop(PowerPCCPU *cpu, uint32_t *prop,
 
     return (p - prop) * sizeof(uint32_t);
 }
-#endif
-- 
2.47.1

Re: [RFC PATCH 3/4] hw/arm/aspeed: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[Cédric Le Goater]

On 3/5/25 17:12, Philippe Mathieu-Daudé wrote:
> For legacy ARM binaries, legacy_binary_is_64bit() is
> equivalent of the compile time TARGET_AARCH64 definition.
> 
> Use it as TypeInfo::registerable() callback to dynamically
> add Aarch64 specific types in qemu-system-aarch64 binary,
> removing the need of TARGET_AARCH64 #ifdef'ry.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/aspeed.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 98bf071139b..3f18a4501e0 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -28,6 +28,7 @@
>   #include "hw/loader.h"
>   #include "qemu/error-report.h"
>   #include "qemu/units.h"
> +#include "qemu/legacy_binary_info.h"
>   #include "hw/qdev-clock.h"
>   #include "system/system.h"
>   
> @@ -179,11 +180,9 @@ struct AspeedMachineState {
>   #define AST2600_EVB_HW_STRAP1 0x000000C0
>   #define AST2600_EVB_HW_STRAP2 0x00000003
>   
> -#ifdef TARGET_AARCH64
>   /* AST2700 evb hardware value */
>   #define AST2700_EVB_HW_STRAP1 0x000000C0
>   #define AST2700_EVB_HW_STRAP2 0x00000003
> -#endif
>   
>   /* Rainier hardware value: (QEMU prototype) */
>   #define RAINIER_BMC_HW_STRAP1 (0x00422016 | SCU_AST2600_HW_STRAP_BOOT_SRC_EMMC)
> @@ -1661,7 +1660,6 @@ static void aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
>       aspeed_machine_class_init_cpus_defaults(mc);
>   }
>   
> -#ifdef TARGET_AARCH64
>   static void ast2700_evb_i2c_init(AspeedMachineState *bmc)
>   {
>       AspeedSoCState *soc = bmc->soc;
> @@ -1690,7 +1688,6 @@ static void aspeed_machine_ast2700_evb_class_init(ObjectClass *oc, void *data)
>       mc->default_ram_size = 1 * GiB;
>       aspeed_machine_class_init_cpus_defaults(mc);
>   }
> -#endif
>   
>   static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
>                                                        void *data)
> @@ -1813,12 +1810,11 @@ static const TypeInfo aspeed_machine_types[] = {
>           .name           = MACHINE_TYPE_NAME("ast1030-evb"),
>           .parent         = TYPE_ASPEED_MACHINE,
>           .class_init     = aspeed_minibmc_machine_ast1030_evb_class_init,
> -#ifdef TARGET_AARCH64
>       }, {
>           .name          = MACHINE_TYPE_NAME("ast2700-evb"),
>           .parent        = TYPE_ASPEED_MACHINE,
> +        .registerable   = legacy_binary_is_64bit,

where is this routine implemented ?

Thanks,

C.


>           .class_init    = aspeed_machine_ast2700_evb_class_init,
> -#endif
>       }, {
>           .name          = TYPE_ASPEED_MACHINE,
>           .parent        = TYPE_MACHINE,

Re: [RFC PATCH 1/4] qom: Introduce TypeInfo::registerable() callback

[Pierrick Bouvier]

On 3/5/25 08:12, Philippe Mathieu-Daudé wrote:
> Introduce the TypeInfo::registerable() callback to allow
> runtime decision on whether register a QOM type or not.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/qom/object.h | 1 +
>   qom/object.c         | 4 ++++
>   qom/trace-events     | 1 +
>   3 files changed, 6 insertions(+)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 9192265db76..f046791f60c 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -476,6 +476,7 @@ struct TypeInfo
>   {
>       const char *name;
>       const char *parent;
> +    bool (*registerable)(void);
>   
>       size_t instance_size;
>       size_t instance_align;
> diff --git a/qom/object.c b/qom/object.c
> index 01618d06bd8..c62b7fd1695 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -168,6 +168,10 @@ static TypeImpl *type_register_internal(const TypeInfo *info)
>           fprintf(stderr, "Registering '%s' with illegal type name\n", info->name);
>           abort();
>       }
> +    if (info->registerable && !info->registerable()) {
> +        trace_object_register_skipped(info->name);
> +        return NULL;
> +    }
>   
>       ti = type_new(info);
>   
> diff --git a/qom/trace-events b/qom/trace-events
> index b2e9f4a7127..29af95d8507 100644
> --- a/qom/trace-events
> +++ b/qom/trace-events
> @@ -3,3 +3,4 @@
>   # object.c
>   object_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
>   object_class_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
> +object_register_skipped(const char *type) "Not registering '%s' type"

That's a good way to be able to select at runtime the objects enabled, 
even though we'll have a binary with all qom objects code present.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [RFC PATCH 2/4] hw/arm/raspi: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[Pierrick Bouvier]

On 3/5/25 08:12, Philippe Mathieu-Daudé wrote:
> For legacy ARM binaries, legacy_binary_is_64bit() is
> equivalent of the compile time TARGET_AARCH64 definition.
> 

I'm not sure where this function comes from.

Anyway, to completely replace TARGET_AARCH64, what we want is something 
like: target_is_aarch64(), because all the objects in this file should 
not be enabled for 64 bits targets who are not arm based.

So it's more easy to introduce a specific function *per* target, and 
enable objects on a per need basis. Some will be enabled for all 
targets, some for only one, some for a specific selection.

> Use it as TypeInfo::registerable() callback to dynamically
> add Aarch64 specific types in qemu-system-aarch64 binary,
> removing the need of TARGET_AARCH64 #ifdef'ry.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/bcm2836.c | 6 ++----
>   hw/arm/raspi.c   | 7 +++----
>   2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 95e16806fa1..88a32e5fc20 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -12,6 +12,7 @@
>   #include "qemu/osdep.h"
>   #include "qapi/error.h"
>   #include "qemu/module.h"
> +#include "qemu/legacy_binary_info.h"
>   #include "hw/arm/bcm2836.h"
>   #include "hw/arm/raspi_platform.h"
>   #include "hw/sysbus.h"
> @@ -195,7 +196,6 @@ static void bcm2836_class_init(ObjectClass *oc, void *data)
>       dc->realize = bcm2836_realize;
>   };
>   
> -#ifdef TARGET_AARCH64
>   static void bcm2837_class_init(ObjectClass *oc, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(oc);
> @@ -208,7 +208,6 @@ static void bcm2837_class_init(ObjectClass *oc, void *data)
>       bc->clusterid = 0x0;
>       dc->realize = bcm2836_realize;
>   };
> -#endif
>   
>   static const TypeInfo bcm283x_types[] = {
>       {
> @@ -219,12 +218,11 @@ static const TypeInfo bcm283x_types[] = {
>           .name           = TYPE_BCM2836,
>           .parent         = TYPE_BCM283X,
>           .class_init     = bcm2836_class_init,
> -#ifdef TARGET_AARCH64
>       }, {
>           .name           = TYPE_BCM2837,
>           .parent         = TYPE_BCM283X,
> +        .registerable   = legacy_binary_is_64bit,
>           .class_init     = bcm2837_class_init,
> -#endif
>       }, {
>           .name           = TYPE_BCM283X,
>           .parent         = TYPE_BCM283X_BASE,
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index dce35ca11aa..f7e647a9cbf 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -15,6 +15,7 @@
>   #include "qemu/osdep.h"
>   #include "qemu/units.h"
>   #include "qemu/cutils.h"
> +#include "qemu/legacy_binary_info.h"
>   #include "qapi/error.h"
>   #include "hw/arm/boot.h"
>   #include "hw/arm/bcm2836.h"
> @@ -367,7 +368,6 @@ static void raspi2b_machine_class_init(ObjectClass *oc, void *data)
>       raspi_machine_class_init(mc, rmc->board_rev);
>   };
>   
> -#ifdef TARGET_AARCH64
>   static void raspi3ap_machine_class_init(ObjectClass *oc, void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> @@ -387,7 +387,6 @@ static void raspi3b_machine_class_init(ObjectClass *oc, void *data)
>       rmc->board_rev = 0xa02082;
>       raspi_machine_class_init(mc, rmc->board_rev);
>   };
> -#endif /* TARGET_AARCH64 */
>   
>   static const TypeInfo raspi_machine_types[] = {
>       {
> @@ -402,16 +401,16 @@ static const TypeInfo raspi_machine_types[] = {
>           .name           = MACHINE_TYPE_NAME("raspi2b"),
>           .parent         = TYPE_RASPI_MACHINE,
>           .class_init     = raspi2b_machine_class_init,
> -#ifdef TARGET_AARCH64
>       }, {
>           .name           = MACHINE_TYPE_NAME("raspi3ap"),
>           .parent         = TYPE_RASPI_MACHINE,
> +        .registerable   = legacy_binary_is_64bit,
>           .class_init     = raspi3ap_machine_class_init,
>       }, {
>           .name           = MACHINE_TYPE_NAME("raspi3b"),
>           .parent         = TYPE_RASPI_MACHINE,
> +        .registerable   = legacy_binary_is_64bit,
>           .class_init     = raspi3b_machine_class_init,
> -#endif
>       }, {
>           .name           = TYPE_RASPI_MACHINE,
>           .parent         = TYPE_RASPI_BASE_MACHINE,

Re: [RFC PATCH 4/4] hw/ppc: Remove TARGET_PPC64 use in ppc_create_page_sizes_prop()

[Pierrick Bouvier]

On 3/5/25 08:12, Philippe Mathieu-Daudé wrote:
> Check the binary is built for 64-bit PPC at runtime,
> removing the need for TARGET_PPC64 #ifdef'ry.
> 

Same goes here, we want a target_is_ppc64() function.
As well, modifying hw/ppc stuff should be in an independent series 
ideally, to keep it more clear.

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/ppc/fdt.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/fdt.c b/hw/ppc/fdt.c
> index 0828ad72548..bae269c72ac 100644
> --- a/hw/ppc/fdt.c
> +++ b/hw/ppc/fdt.c
> @@ -8,12 +8,12 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/legacy_binary_info.h"
>   #include "target/ppc/cpu.h"
>   #include "target/ppc/mmu-hash64.h"
>   
>   #include "hw/ppc/fdt.h"
>   
> -#if defined(TARGET_PPC64)
>   size_t ppc_create_page_sizes_prop(PowerPCCPU *cpu, uint32_t *prop,
>                                     size_t maxsize)
>   {
> @@ -21,6 +21,8 @@ size_t ppc_create_page_sizes_prop(PowerPCCPU *cpu, uint32_t *prop,
>       int i, j, count;
>       uint32_t *p = prop;
>   
> +    assert(legacy_binary_is_64bit());
> +
>       for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
>           PPCHash64SegmentPageSizes *sps = &cpu->hash64_opts->sps[i];
>   
> @@ -46,4 +48,3 @@ size_t ppc_create_page_sizes_prop(PowerPCCPU *cpu, uint32_t *prop,
>   
>       return (p - prop) * sizeof(uint32_t);
>   }
> -#endif

Re: [RFC PATCH 0/4] hw/arm: Register target-specific QOM types at runtime

[Pierrick Bouvier]

On 3/5/25 08:12, Philippe Mathieu-Daudé wrote:
> In preparation of having a single binary for both ARM and
> Aarch64 targets, unify raspi & aspeed by replacing the
> compile-time TARGET_AARCH64 check by a QOM runtime call
> on legacy_binary_is_64bit().
> 
> No behavior change with current binaries:
> 
>    $ ./qemu-system-arm -M help | fgrep raspi
>    raspi0               Raspberry Pi Zero (revision 1.2)
>    raspi1ap             Raspberry Pi A+ (revision 1.1)
>    raspi2b              Raspberry Pi 2B (revision 1.1)
> 
>    $ ./qemu-system-aarch64 -M help | fgrep raspi
>    raspi0               Raspberry Pi Zero (revision 1.2)
>    raspi1ap             Raspberry Pi A+ (revision 1.1)
>    raspi2b              Raspberry Pi 2B (revision 1.1)
>    raspi3ap             Raspberry Pi 3A+ (revision 1.0)
>    raspi3b              Raspberry Pi 3B (revision 1.2)
>    raspi4b              Raspberry Pi 4B (revision 1.5)
> 

To see an effect on those changes, we should ideally make sure those 
files can compile while being common (in meson.build system_ss), so we 
are sure they don't depend on target define anymore.

> Based-on: <20250305153929.43687-1-philmd@linaro.org>
> 
> Philippe Mathieu-Daudé (4):
>    qom: Introduce TypeInfo::registerable() callback
>    hw/arm/raspi: Replace TARGET_AARCH64 by legacy_binary_is_64bit()
>    hw/arm/aspeed: Replace TARGET_AARCH64 by legacy_binary_is_64bit()
>    hw/ppc: Remove TARGET_PPC64 use in ppc_create_page_sizes_prop()
> 
>   include/qom/object.h | 1 +
>   hw/arm/aspeed.c      | 8 ++------
>   hw/arm/bcm2836.c     | 6 ++----
>   hw/arm/raspi.c       | 7 +++----
>   hw/ppc/fdt.c         | 5 +++--
>   qom/object.c         | 4 ++++
>   qom/trace-events     | 1 +
>   7 files changed, 16 insertions(+), 16 deletions(-)
> 

Re: [RFC PATCH 3/4] hw/arm/aspeed: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[Philippe Mathieu-Daudé]

On 5/3/25 17:33, Cédric Le Goater wrote:
> On 3/5/25 17:12, Philippe Mathieu-Daudé wrote:
>> For legacy ARM binaries, legacy_binary_is_64bit() is
>> equivalent of the compile time TARGET_AARCH64 definition.
>>
>> Use it as TypeInfo::registerable() callback to dynamically
>> add Aarch64 specific types in qemu-system-aarch64 binary,
>> removing the need of TARGET_AARCH64 #ifdef'ry.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/arm/aspeed.c | 8 ++------
>>   1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index 98bf071139b..3f18a4501e0 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -28,6 +28,7 @@
>>   #include "hw/loader.h"
>>   #include "qemu/error-report.h"
>>   #include "qemu/units.h"
>> +#include "qemu/legacy_binary_info.h"
>>   #include "hw/qdev-clock.h"
>>   #include "system/system.h"
>> @@ -179,11 +180,9 @@ struct AspeedMachineState {
>>   #define AST2600_EVB_HW_STRAP1 0x000000C0
>>   #define AST2600_EVB_HW_STRAP2 0x00000003
>> -#ifdef TARGET_AARCH64
>>   /* AST2700 evb hardware value */
>>   #define AST2700_EVB_HW_STRAP1 0x000000C0
>>   #define AST2700_EVB_HW_STRAP2 0x00000003
>> -#endif
>>   /* Rainier hardware value: (QEMU prototype) */
>>   #define RAINIER_BMC_HW_STRAP1 (0x00422016 | 
>> SCU_AST2600_HW_STRAP_BOOT_SRC_EMMC)
>> @@ -1661,7 +1660,6 @@ static void 
>> aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
>>       aspeed_machine_class_init_cpus_defaults(mc);
>>   }
>> -#ifdef TARGET_AARCH64
>>   static void ast2700_evb_i2c_init(AspeedMachineState *bmc)
>>   {
>>       AspeedSoCState *soc = bmc->soc;
>> @@ -1690,7 +1688,6 @@ static void 
>> aspeed_machine_ast2700_evb_class_init(ObjectClass *oc, void *data)
>>       mc->default_ram_size = 1 * GiB;
>>       aspeed_machine_class_init_cpus_defaults(mc);
>>   }
>> -#endif
>>   static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
>>                                                        void *data)
>> @@ -1813,12 +1810,11 @@ static const TypeInfo aspeed_machine_types[] = {
>>           .name           = MACHINE_TYPE_NAME("ast1030-evb"),
>>           .parent         = TYPE_ASPEED_MACHINE,
>>           .class_init     = 
>> aspeed_minibmc_machine_ast1030_evb_class_init,
>> -#ifdef TARGET_AARCH64
>>       }, {
>>           .name          = MACHINE_TYPE_NAME("ast2700-evb"),
>>           .parent        = TYPE_ASPEED_MACHINE,
>> +        .registerable   = legacy_binary_is_64bit,
> 
> where is this routine implemented ?

Based-on series mentioned in cover:
https://lore.kernel.org/qemu-devel/20250305153929.43687-6-philmd@linaro.org/

Re: [RFC PATCH 2/4] hw/arm/raspi: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[Thomas Huth]

On 05/03/2025 17.12, Philippe Mathieu-Daudé wrote:
> For legacy ARM binaries, legacy_binary_is_64bit() is
> equivalent of the compile time TARGET_AARCH64 definition.
> 
> Use it as TypeInfo::registerable() callback to dynamically
> add Aarch64 specific types in qemu-system-aarch64 binary,
> removing the need of TARGET_AARCH64 #ifdef'ry.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/bcm2836.c | 6 ++----
>   hw/arm/raspi.c   | 7 +++----
>   2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 95e16806fa1..88a32e5fc20 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -12,6 +12,7 @@
>   #include "qemu/osdep.h"
>   #include "qapi/error.h"
>   #include "qemu/module.h"
> +#include "qemu/legacy_binary_info.h"
>   #include "hw/arm/bcm2836.h"
>   #include "hw/arm/raspi_platform.h"
>   #include "hw/sysbus.h"
> @@ -195,7 +196,6 @@ static void bcm2836_class_init(ObjectClass *oc, void *data)
>       dc->realize = bcm2836_realize;
>   };
>   
> -#ifdef TARGET_AARCH64
>   static void bcm2837_class_init(ObjectClass *oc, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(oc);
> @@ -208,7 +208,6 @@ static void bcm2837_class_init(ObjectClass *oc, void *data)
>       bc->clusterid = 0x0;
>       dc->realize = bcm2836_realize;
>   };
> -#endif
>   
>   static const TypeInfo bcm283x_types[] = {
>       {
> @@ -219,12 +218,11 @@ static const TypeInfo bcm283x_types[] = {
>           .name           = TYPE_BCM2836,
>           .parent         = TYPE_BCM283X,
>           .class_init     = bcm2836_class_init,
> -#ifdef TARGET_AARCH64
>       }, {
>           .name           = TYPE_BCM2837,
>           .parent         = TYPE_BCM283X,
> +        .registerable   = legacy_binary_is_64bit,
>           .class_init     = bcm2837_class_init,
> -#endif
>       }, {
>           .name           = TYPE_BCM283X,
>           .parent         = TYPE_BCM283X_BASE,
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index dce35ca11aa..f7e647a9cbf 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -15,6 +15,7 @@
>   #include "qemu/osdep.h"
>   #include "qemu/units.h"
>   #include "qemu/cutils.h"
> +#include "qemu/legacy_binary_info.h"
>   #include "qapi/error.h"
>   #include "hw/arm/boot.h"
>   #include "hw/arm/bcm2836.h"
> @@ -367,7 +368,6 @@ static void raspi2b_machine_class_init(ObjectClass *oc, void *data)
>       raspi_machine_class_init(mc, rmc->board_rev);
>   };
>   
> -#ifdef TARGET_AARCH64
>   static void raspi3ap_machine_class_init(ObjectClass *oc, void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> @@ -387,7 +387,6 @@ static void raspi3b_machine_class_init(ObjectClass *oc, void *data)
>       rmc->board_rev = 0xa02082;
>       raspi_machine_class_init(mc, rmc->board_rev);
>   };
> -#endif /* TARGET_AARCH64 */
>   
>   static const TypeInfo raspi_machine_types[] = {
>       {
> @@ -402,16 +401,16 @@ static const TypeInfo raspi_machine_types[] = {
>           .name           = MACHINE_TYPE_NAME("raspi2b"),
>           .parent         = TYPE_RASPI_MACHINE,
>           .class_init     = raspi2b_machine_class_init,
> -#ifdef TARGET_AARCH64
>       }, {
>           .name           = MACHINE_TYPE_NAME("raspi3ap"),
>           .parent         = TYPE_RASPI_MACHINE,
> +        .registerable   = legacy_binary_is_64bit,
>           .class_init     = raspi3ap_machine_class_init,
>       }, {
>           .name           = MACHINE_TYPE_NAME("raspi3b"),
>           .parent         = TYPE_RASPI_MACHINE,
> +        .registerable   = legacy_binary_is_64bit,
>           .class_init     = raspi3b_machine_class_init,
> -#endif
>       }, {
>           .name           = TYPE_RASPI_MACHINE,
>           .parent         = TYPE_RASPI_BASE_MACHINE,

Uh, this (together with patch 1) looks very cumbersome. Why don't you simply 
split the array into two, one for 32-bit and one for 64-bit, and then use a 
simply "if (legacy_binary_is_64bit())" in the type_init function instead?

  Thomas

Re: [RFC PATCH 3/4] hw/arm/aspeed: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[Thomas Huth]

On 05/03/2025 17.12, Philippe Mathieu-Daudé wrote:
> For legacy ARM binaries, legacy_binary_is_64bit() is
> equivalent of the compile time TARGET_AARCH64 definition.
> 
> Use it as TypeInfo::registerable() callback to dynamically
> add Aarch64 specific types in qemu-system-aarch64 binary,
> removing the need of TARGET_AARCH64 #ifdef'ry.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/aspeed.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 98bf071139b..3f18a4501e0 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -28,6 +28,7 @@
>   #include "hw/loader.h"
>   #include "qemu/error-report.h"
>   #include "qemu/units.h"
> +#include "qemu/legacy_binary_info.h"
>   #include "hw/qdev-clock.h"
>   #include "system/system.h"
>   
> @@ -179,11 +180,9 @@ struct AspeedMachineState {
>   #define AST2600_EVB_HW_STRAP1 0x000000C0
>   #define AST2600_EVB_HW_STRAP2 0x00000003
>   
> -#ifdef TARGET_AARCH64
>   /* AST2700 evb hardware value */
>   #define AST2700_EVB_HW_STRAP1 0x000000C0
>   #define AST2700_EVB_HW_STRAP2 0x00000003
> -#endif
>   
>   /* Rainier hardware value: (QEMU prototype) */
>   #define RAINIER_BMC_HW_STRAP1 (0x00422016 | SCU_AST2600_HW_STRAP_BOOT_SRC_EMMC)
> @@ -1661,7 +1660,6 @@ static void aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
>       aspeed_machine_class_init_cpus_defaults(mc);
>   }
>   
> -#ifdef TARGET_AARCH64
>   static void ast2700_evb_i2c_init(AspeedMachineState *bmc)
>   {
>       AspeedSoCState *soc = bmc->soc;
> @@ -1690,7 +1688,6 @@ static void aspeed_machine_ast2700_evb_class_init(ObjectClass *oc, void *data)
>       mc->default_ram_size = 1 * GiB;
>       aspeed_machine_class_init_cpus_defaults(mc);
>   }
> -#endif
>   
>   static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
>                                                        void *data)
> @@ -1813,12 +1810,11 @@ static const TypeInfo aspeed_machine_types[] = {
>           .name           = MACHINE_TYPE_NAME("ast1030-evb"),
>           .parent         = TYPE_ASPEED_MACHINE,
>           .class_init     = aspeed_minibmc_machine_ast1030_evb_class_init,
> -#ifdef TARGET_AARCH64
>       }, {
>           .name          = MACHINE_TYPE_NAME("ast2700-evb"),
>           .parent        = TYPE_ASPEED_MACHINE,
> +        .registerable   = legacy_binary_is_64bit,
>           .class_init    = aspeed_machine_ast2700_evb_class_init,
> -#endif
>       }, {
>           .name          = TYPE_ASPEED_MACHINE,
>           .parent        = TYPE_MACHINE,

Same as with previous patch, I'd prefer to split the array, and replace the 
DEFINE_TYPES() with a dynamic type_init() function.

  Thomas

Re: [RFC PATCH 2/4] hw/arm/raspi: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[Cédric Le Goater]

On 3/5/25 18:40, Thomas Huth wrote:
> On 05/03/2025 17.12, Philippe Mathieu-Daudé wrote:
>> For legacy ARM binaries, legacy_binary_is_64bit() is
>> equivalent of the compile time TARGET_AARCH64 definition.
>>
>> Use it as TypeInfo::registerable() callback to dynamically
>> add Aarch64 specific types in qemu-system-aarch64 binary,
>> removing the need of TARGET_AARCH64 #ifdef'ry.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/arm/bcm2836.c | 6 ++----
>>   hw/arm/raspi.c   | 7 +++----
>>   2 files changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
>> index 95e16806fa1..88a32e5fc20 100644
>> --- a/hw/arm/bcm2836.c
>> +++ b/hw/arm/bcm2836.c
>> @@ -12,6 +12,7 @@
>>   #include "qemu/osdep.h"
>>   #include "qapi/error.h"
>>   #include "qemu/module.h"
>> +#include "qemu/legacy_binary_info.h"
>>   #include "hw/arm/bcm2836.h"
>>   #include "hw/arm/raspi_platform.h"
>>   #include "hw/sysbus.h"
>> @@ -195,7 +196,6 @@ static void bcm2836_class_init(ObjectClass *oc, void *data)
>>       dc->realize = bcm2836_realize;
>>   };
>> -#ifdef TARGET_AARCH64
>>   static void bcm2837_class_init(ObjectClass *oc, void *data)
>>   {
>>       DeviceClass *dc = DEVICE_CLASS(oc);
>> @@ -208,7 +208,6 @@ static void bcm2837_class_init(ObjectClass *oc, void *data)
>>       bc->clusterid = 0x0;
>>       dc->realize = bcm2836_realize;
>>   };
>> -#endif
>>   static const TypeInfo bcm283x_types[] = {
>>       {
>> @@ -219,12 +218,11 @@ static const TypeInfo bcm283x_types[] = {
>>           .name           = TYPE_BCM2836,
>>           .parent         = TYPE_BCM283X,
>>           .class_init     = bcm2836_class_init,
>> -#ifdef TARGET_AARCH64
>>       }, {
>>           .name           = TYPE_BCM2837,
>>           .parent         = TYPE_BCM283X,
>> +        .registerable   = legacy_binary_is_64bit,
>>           .class_init     = bcm2837_class_init,
>> -#endif
>>       }, {
>>           .name           = TYPE_BCM283X,
>>           .parent         = TYPE_BCM283X_BASE,
>> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
>> index dce35ca11aa..f7e647a9cbf 100644
>> --- a/hw/arm/raspi.c
>> +++ b/hw/arm/raspi.c
>> @@ -15,6 +15,7 @@
>>   #include "qemu/osdep.h"
>>   #include "qemu/units.h"
>>   #include "qemu/cutils.h"
>> +#include "qemu/legacy_binary_info.h"
>>   #include "qapi/error.h"
>>   #include "hw/arm/boot.h"
>>   #include "hw/arm/bcm2836.h"
>> @@ -367,7 +368,6 @@ static void raspi2b_machine_class_init(ObjectClass *oc, void *data)
>>       raspi_machine_class_init(mc, rmc->board_rev);
>>   };
>> -#ifdef TARGET_AARCH64
>>   static void raspi3ap_machine_class_init(ObjectClass *oc, void *data)
>>   {
>>       MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -387,7 +387,6 @@ static void raspi3b_machine_class_init(ObjectClass *oc, void *data)
>>       rmc->board_rev = 0xa02082;
>>       raspi_machine_class_init(mc, rmc->board_rev);
>>   };
>> -#endif /* TARGET_AARCH64 */
>>   static const TypeInfo raspi_machine_types[] = {
>>       {
>> @@ -402,16 +401,16 @@ static const TypeInfo raspi_machine_types[] = {
>>           .name           = MACHINE_TYPE_NAME("raspi2b"),
>>           .parent         = TYPE_RASPI_MACHINE,
>>           .class_init     = raspi2b_machine_class_init,
>> -#ifdef TARGET_AARCH64
>>       }, {
>>           .name           = MACHINE_TYPE_NAME("raspi3ap"),
>>           .parent         = TYPE_RASPI_MACHINE,
>> +        .registerable   = legacy_binary_is_64bit,
>>           .class_init     = raspi3ap_machine_class_init,
>>       }, {
>>           .name           = MACHINE_TYPE_NAME("raspi3b"),
>>           .parent         = TYPE_RASPI_MACHINE,
>> +        .registerable   = legacy_binary_is_64bit,
>>           .class_init     = raspi3b_machine_class_init,
>> -#endif
>>       }, {
>>           .name           = TYPE_RASPI_MACHINE,
>>           .parent         = TYPE_RASPI_BASE_MACHINE,
> 
> Uh, this (together with patch 1) looks very cumbersome. Why don't you simply split the array into two, one for 32-bit and one for 64-bit, and then use a simply "if (legacy_binary_is_64bit())" in the type_init function instead?

Sounds like a good idea.

So we would have DEFINE_TYPES() and DEFINE_TYPES64() macros ?


C.

Re: [RFC PATCH 2/4] hw/arm/raspi: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[Thomas Huth]

On 05/03/2025 19.12, Cédric Le Goater wrote:
> On 3/5/25 18:40, Thomas Huth wrote:
>> On 05/03/2025 17.12, Philippe Mathieu-Daudé wrote:
>>> For legacy ARM binaries, legacy_binary_is_64bit() is
>>> equivalent of the compile time TARGET_AARCH64 definition.
>>>
>>> Use it as TypeInfo::registerable() callback to dynamically
>>> add Aarch64 specific types in qemu-system-aarch64 binary,
>>> removing the need of TARGET_AARCH64 #ifdef'ry.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   hw/arm/bcm2836.c | 6 ++----
>>>   hw/arm/raspi.c   | 7 +++----
>>>   2 files changed, 5 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
>>> index 95e16806fa1..88a32e5fc20 100644
>>> --- a/hw/arm/bcm2836.c
>>> +++ b/hw/arm/bcm2836.c
>>> @@ -12,6 +12,7 @@
>>>   #include "qemu/osdep.h"
>>>   #include "qapi/error.h"
>>>   #include "qemu/module.h"
>>> +#include "qemu/legacy_binary_info.h"
>>>   #include "hw/arm/bcm2836.h"
>>>   #include "hw/arm/raspi_platform.h"
>>>   #include "hw/sysbus.h"
>>> @@ -195,7 +196,6 @@ static void bcm2836_class_init(ObjectClass *oc, void 
>>> *data)
>>>       dc->realize = bcm2836_realize;
>>>   };
>>> -#ifdef TARGET_AARCH64
>>>   static void bcm2837_class_init(ObjectClass *oc, void *data)
>>>   {
>>>       DeviceClass *dc = DEVICE_CLASS(oc);
>>> @@ -208,7 +208,6 @@ static void bcm2837_class_init(ObjectClass *oc, void 
>>> *data)
>>>       bc->clusterid = 0x0;
>>>       dc->realize = bcm2836_realize;
>>>   };
>>> -#endif
>>>   static const TypeInfo bcm283x_types[] = {
>>>       {
>>> @@ -219,12 +218,11 @@ static const TypeInfo bcm283x_types[] = {
>>>           .name           = TYPE_BCM2836,
>>>           .parent         = TYPE_BCM283X,
>>>           .class_init     = bcm2836_class_init,
>>> -#ifdef TARGET_AARCH64
>>>       }, {
>>>           .name           = TYPE_BCM2837,
>>>           .parent         = TYPE_BCM283X,
>>> +        .registerable   = legacy_binary_is_64bit,
>>>           .class_init     = bcm2837_class_init,
>>> -#endif
>>>       }, {
>>>           .name           = TYPE_BCM283X,
>>>           .parent         = TYPE_BCM283X_BASE,
>>> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
>>> index dce35ca11aa..f7e647a9cbf 100644
>>> --- a/hw/arm/raspi.c
>>> +++ b/hw/arm/raspi.c
>>> @@ -15,6 +15,7 @@
>>>   #include "qemu/osdep.h"
>>>   #include "qemu/units.h"
>>>   #include "qemu/cutils.h"
>>> +#include "qemu/legacy_binary_info.h"
>>>   #include "qapi/error.h"
>>>   #include "hw/arm/boot.h"
>>>   #include "hw/arm/bcm2836.h"
>>> @@ -367,7 +368,6 @@ static void raspi2b_machine_class_init(ObjectClass 
>>> *oc, void *data)
>>>       raspi_machine_class_init(mc, rmc->board_rev);
>>>   };
>>> -#ifdef TARGET_AARCH64
>>>   static void raspi3ap_machine_class_init(ObjectClass *oc, void *data)
>>>   {
>>>       MachineClass *mc = MACHINE_CLASS(oc);
>>> @@ -387,7 +387,6 @@ static void raspi3b_machine_class_init(ObjectClass 
>>> *oc, void *data)
>>>       rmc->board_rev = 0xa02082;
>>>       raspi_machine_class_init(mc, rmc->board_rev);
>>>   };
>>> -#endif /* TARGET_AARCH64 */
>>>   static const TypeInfo raspi_machine_types[] = {
>>>       {
>>> @@ -402,16 +401,16 @@ static const TypeInfo raspi_machine_types[] = {
>>>           .name           = MACHINE_TYPE_NAME("raspi2b"),
>>>           .parent         = TYPE_RASPI_MACHINE,
>>>           .class_init     = raspi2b_machine_class_init,
>>> -#ifdef TARGET_AARCH64
>>>       }, {
>>>           .name           = MACHINE_TYPE_NAME("raspi3ap"),
>>>           .parent         = TYPE_RASPI_MACHINE,
>>> +        .registerable   = legacy_binary_is_64bit,
>>>           .class_init     = raspi3ap_machine_class_init,
>>>       }, {
>>>           .name           = MACHINE_TYPE_NAME("raspi3b"),
>>>           .parent         = TYPE_RASPI_MACHINE,
>>> +        .registerable   = legacy_binary_is_64bit,
>>>           .class_init     = raspi3b_machine_class_init,
>>> -#endif
>>>       }, {
>>>           .name           = TYPE_RASPI_MACHINE,
>>>           .parent         = TYPE_RASPI_BASE_MACHINE,
>>
>> Uh, this (together with patch 1) looks very cumbersome. Why don't you 
>> simply split the array into two, one for 32-bit and one for 64-bit, and 
>> then use a simply "if (legacy_binary_is_64bit())" in the type_init 
>> function instead?
> 
> Sounds like a good idea.
> 
> So we would have DEFINE_TYPES() and DEFINE_TYPES64() macros ?

Either that - or simply use type_init() directly here for the time being.

  Thomas

Re: [RFC PATCH 2/4] hw/arm/raspi: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[Philippe Mathieu-Daudé]

On 5/3/25 19:35, Thomas Huth wrote:
> On 05/03/2025 19.12, Cédric Le Goater wrote:
>> On 3/5/25 18:40, Thomas Huth wrote:
>>> On 05/03/2025 17.12, Philippe Mathieu-Daudé wrote:
>>>> For legacy ARM binaries, legacy_binary_is_64bit() is
>>>> equivalent of the compile time TARGET_AARCH64 definition.
>>>>
>>>> Use it as TypeInfo::registerable() callback to dynamically
>>>> add Aarch64 specific types in qemu-system-aarch64 binary,
>>>> removing the need of TARGET_AARCH64 #ifdef'ry.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>>   hw/arm/bcm2836.c | 6 ++----
>>>>   hw/arm/raspi.c   | 7 +++----
>>>>   2 files changed, 5 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
>>>> index 95e16806fa1..88a32e5fc20 100644
>>>> --- a/hw/arm/bcm2836.c
>>>> +++ b/hw/arm/bcm2836.c
>>>> @@ -12,6 +12,7 @@
>>>>   #include "qemu/osdep.h"
>>>>   #include "qapi/error.h"
>>>>   #include "qemu/module.h"
>>>> +#include "qemu/legacy_binary_info.h"
>>>>   #include "hw/arm/bcm2836.h"
>>>>   #include "hw/arm/raspi_platform.h"
>>>>   #include "hw/sysbus.h"
>>>> @@ -195,7 +196,6 @@ static void bcm2836_class_init(ObjectClass *oc, 
>>>> void *data)
>>>>       dc->realize = bcm2836_realize;
>>>>   };
>>>> -#ifdef TARGET_AARCH64
>>>>   static void bcm2837_class_init(ObjectClass *oc, void *data)
>>>>   {
>>>>       DeviceClass *dc = DEVICE_CLASS(oc);
>>>> @@ -208,7 +208,6 @@ static void bcm2837_class_init(ObjectClass *oc, 
>>>> void *data)
>>>>       bc->clusterid = 0x0;
>>>>       dc->realize = bcm2836_realize;
>>>>   };
>>>> -#endif
>>>>   static const TypeInfo bcm283x_types[] = {
>>>>       {
>>>> @@ -219,12 +218,11 @@ static const TypeInfo bcm283x_types[] = {
>>>>           .name           = TYPE_BCM2836,
>>>>           .parent         = TYPE_BCM283X,
>>>>           .class_init     = bcm2836_class_init,
>>>> -#ifdef TARGET_AARCH64
>>>>       }, {
>>>>           .name           = TYPE_BCM2837,
>>>>           .parent         = TYPE_BCM283X,
>>>> +        .registerable   = legacy_binary_is_64bit,
>>>>           .class_init     = bcm2837_class_init,
>>>> -#endif
>>>>       }, {
>>>>           .name           = TYPE_BCM283X,
>>>>           .parent         = TYPE_BCM283X_BASE,
>>>> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
>>>> index dce35ca11aa..f7e647a9cbf 100644
>>>> --- a/hw/arm/raspi.c
>>>> +++ b/hw/arm/raspi.c
>>>> @@ -15,6 +15,7 @@
>>>>   #include "qemu/osdep.h"
>>>>   #include "qemu/units.h"
>>>>   #include "qemu/cutils.h"
>>>> +#include "qemu/legacy_binary_info.h"
>>>>   #include "qapi/error.h"
>>>>   #include "hw/arm/boot.h"
>>>>   #include "hw/arm/bcm2836.h"
>>>> @@ -367,7 +368,6 @@ static void 
>>>> raspi2b_machine_class_init(ObjectClass *oc, void *data)
>>>>       raspi_machine_class_init(mc, rmc->board_rev);
>>>>   };
>>>> -#ifdef TARGET_AARCH64
>>>>   static void raspi3ap_machine_class_init(ObjectClass *oc, void *data)
>>>>   {
>>>>       MachineClass *mc = MACHINE_CLASS(oc);
>>>> @@ -387,7 +387,6 @@ static void 
>>>> raspi3b_machine_class_init(ObjectClass *oc, void *data)
>>>>       rmc->board_rev = 0xa02082;
>>>>       raspi_machine_class_init(mc, rmc->board_rev);
>>>>   };
>>>> -#endif /* TARGET_AARCH64 */
>>>>   static const TypeInfo raspi_machine_types[] = {
>>>>       {
>>>> @@ -402,16 +401,16 @@ static const TypeInfo raspi_machine_types[] = {
>>>>           .name           = MACHINE_TYPE_NAME("raspi2b"),
>>>>           .parent         = TYPE_RASPI_MACHINE,
>>>>           .class_init     = raspi2b_machine_class_init,
>>>> -#ifdef TARGET_AARCH64
>>>>       }, {
>>>>           .name           = MACHINE_TYPE_NAME("raspi3ap"),
>>>>           .parent         = TYPE_RASPI_MACHINE,
>>>> +        .registerable   = legacy_binary_is_64bit,
>>>>           .class_init     = raspi3ap_machine_class_init,
>>>>       }, {
>>>>           .name           = MACHINE_TYPE_NAME("raspi3b"),
>>>>           .parent         = TYPE_RASPI_MACHINE,
>>>> +        .registerable   = legacy_binary_is_64bit,
>>>>           .class_init     = raspi3b_machine_class_init,
>>>> -#endif
>>>>       }, {
>>>>           .name           = TYPE_RASPI_MACHINE,
>>>>           .parent         = TYPE_RASPI_BASE_MACHINE,
>>>
>>> Uh, this (together with patch 1) looks very cumbersome. Why don't you 
>>> simply split the array into two, one for 32-bit and one for 64-bit, 
>>> and then use a simply "if (legacy_binary_is_64bit())" in the 
>>> type_init function instead?
>>
>> Sounds like a good idea.
>>
>> So we would have DEFINE_TYPES() and DEFINE_TYPES64() macros ?
> 
> Either that - or simply use type_init() directly here for the time being.

As Pierrick noted on private chat, my approach doesn't scale, I should
use smth in the lines of:

     }, {
         .name           = MACHINE_TYPE_NAME("raspi2b"),
         .parent         = TYPE_RASPI_MACHINE,
         .registerable   = qemu_binary_has_target_arm,
         .class_init     = raspi2b_machine_class_init,
     }, {
         .name           = MACHINE_TYPE_NAME("raspi3ap"),
         .parent         = TYPE_RASPI_MACHINE,
         .registerable   = qemu_binary_has_target_aarch64,
         .class_init     = raspi3ap_machine_class_init,
     }, {

Having:

bool qemu_binary_has_target_arm(void)
{
     return qemu_arch_available(QEMU_ARCH_ARM);
}

Now back to Thomas suggestion, we could define 2 TypeInfo arrays,
but I foresee lot of code churn when devices has to be made
available on different setup combinations; so with that in mind
the QOM registerable() callback appears a bit more future proof.

Re: [RFC PATCH 2/4] hw/arm/raspi: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 6841 bytes --]

On Wed, 5 Mar 2025, Philippe Mathieu-Daudé wrote:
> On 5/3/25 19:35, Thomas Huth wrote:
>> On 05/03/2025 19.12, Cédric Le Goater wrote:
>>> On 3/5/25 18:40, Thomas Huth wrote:
>>>> On 05/03/2025 17.12, Philippe Mathieu-Daudé wrote:
>>>>> For legacy ARM binaries, legacy_binary_is_64bit() is
>>>>> equivalent of the compile time TARGET_AARCH64 definition.
>>>>> 
>>>>> Use it as TypeInfo::registerable() callback to dynamically
>>>>> add Aarch64 specific types in qemu-system-aarch64 binary,
>>>>> removing the need of TARGET_AARCH64 #ifdef'ry.
>>>>> 
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> ---
>>>>>   hw/arm/bcm2836.c | 6 ++----
>>>>>   hw/arm/raspi.c   | 7 +++----
>>>>>   2 files changed, 5 insertions(+), 8 deletions(-)
>>>>> 
>>>>> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
>>>>> index 95e16806fa1..88a32e5fc20 100644
>>>>> --- a/hw/arm/bcm2836.c
>>>>> +++ b/hw/arm/bcm2836.c
>>>>> @@ -12,6 +12,7 @@
>>>>>   #include "qemu/osdep.h"
>>>>>   #include "qapi/error.h"
>>>>>   #include "qemu/module.h"
>>>>> +#include "qemu/legacy_binary_info.h"
>>>>>   #include "hw/arm/bcm2836.h"
>>>>>   #include "hw/arm/raspi_platform.h"
>>>>>   #include "hw/sysbus.h"
>>>>> @@ -195,7 +196,6 @@ static void bcm2836_class_init(ObjectClass *oc, void 
>>>>> *data)
>>>>>       dc->realize = bcm2836_realize;
>>>>>   };
>>>>> -#ifdef TARGET_AARCH64
>>>>>   static void bcm2837_class_init(ObjectClass *oc, void *data)
>>>>>   {
>>>>>       DeviceClass *dc = DEVICE_CLASS(oc);
>>>>> @@ -208,7 +208,6 @@ static void bcm2837_class_init(ObjectClass *oc, void 
>>>>> *data)
>>>>>       bc->clusterid = 0x0;
>>>>>       dc->realize = bcm2836_realize;
>>>>>   };
>>>>> -#endif
>>>>>   static const TypeInfo bcm283x_types[] = {
>>>>>       {
>>>>> @@ -219,12 +218,11 @@ static const TypeInfo bcm283x_types[] = {
>>>>>           .name           = TYPE_BCM2836,
>>>>>           .parent         = TYPE_BCM283X,
>>>>>           .class_init     = bcm2836_class_init,
>>>>> -#ifdef TARGET_AARCH64
>>>>>       }, {
>>>>>           .name           = TYPE_BCM2837,
>>>>>           .parent         = TYPE_BCM283X,
>>>>> +        .registerable   = legacy_binary_is_64bit,
>>>>>           .class_init     = bcm2837_class_init,
>>>>> -#endif
>>>>>       }, {
>>>>>           .name           = TYPE_BCM283X,
>>>>>           .parent         = TYPE_BCM283X_BASE,
>>>>> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
>>>>> index dce35ca11aa..f7e647a9cbf 100644
>>>>> --- a/hw/arm/raspi.c
>>>>> +++ b/hw/arm/raspi.c
>>>>> @@ -15,6 +15,7 @@
>>>>>   #include "qemu/osdep.h"
>>>>>   #include "qemu/units.h"
>>>>>   #include "qemu/cutils.h"
>>>>> +#include "qemu/legacy_binary_info.h"
>>>>>   #include "qapi/error.h"
>>>>>   #include "hw/arm/boot.h"
>>>>>   #include "hw/arm/bcm2836.h"
>>>>> @@ -367,7 +368,6 @@ static void raspi2b_machine_class_init(ObjectClass 
>>>>> *oc, void *data)
>>>>>       raspi_machine_class_init(mc, rmc->board_rev);
>>>>>   };
>>>>> -#ifdef TARGET_AARCH64
>>>>>   static void raspi3ap_machine_class_init(ObjectClass *oc, void *data)
>>>>>   {
>>>>>       MachineClass *mc = MACHINE_CLASS(oc);
>>>>> @@ -387,7 +387,6 @@ static void raspi3b_machine_class_init(ObjectClass 
>>>>> *oc, void *data)
>>>>>       rmc->board_rev = 0xa02082;
>>>>>       raspi_machine_class_init(mc, rmc->board_rev);
>>>>>   };
>>>>> -#endif /* TARGET_AARCH64 */
>>>>>   static const TypeInfo raspi_machine_types[] = {
>>>>>       {
>>>>> @@ -402,16 +401,16 @@ static const TypeInfo raspi_machine_types[] = {
>>>>>           .name           = MACHINE_TYPE_NAME("raspi2b"),
>>>>>           .parent         = TYPE_RASPI_MACHINE,
>>>>>           .class_init     = raspi2b_machine_class_init,
>>>>> -#ifdef TARGET_AARCH64
>>>>>       }, {
>>>>>           .name           = MACHINE_TYPE_NAME("raspi3ap"),
>>>>>           .parent         = TYPE_RASPI_MACHINE,
>>>>> +        .registerable   = legacy_binary_is_64bit,
>>>>>           .class_init     = raspi3ap_machine_class_init,
>>>>>       }, {
>>>>>           .name           = MACHINE_TYPE_NAME("raspi3b"),
>>>>>           .parent         = TYPE_RASPI_MACHINE,
>>>>> +        .registerable   = legacy_binary_is_64bit,
>>>>>           .class_init     = raspi3b_machine_class_init,
>>>>> -#endif
>>>>>       }, {
>>>>>           .name           = TYPE_RASPI_MACHINE,
>>>>>           .parent         = TYPE_RASPI_BASE_MACHINE,
>>>> 
>>>> Uh, this (together with patch 1) looks very cumbersome. Why don't you 
>>>> simply split the array into two, one for 32-bit and one for 64-bit, and 
>>>> then use a simply "if (legacy_binary_is_64bit())" in the type_init 
>>>> function instead?
>>> 
>>> Sounds like a good idea.
>>> 
>>> So we would have DEFINE_TYPES() and DEFINE_TYPES64() macros ?
>> 
>> Either that - or simply use type_init() directly here for the time being.
>
> As Pierrick noted on private chat, my approach doesn't scale, I should
> use smth in the lines of:
>
>    }, {
>        .name           = MACHINE_TYPE_NAME("raspi2b"),
>        .parent         = TYPE_RASPI_MACHINE,
>        .registerable   = qemu_binary_has_target_arm,
>        .class_init     = raspi2b_machine_class_init,
>    }, {
>        .name           = MACHINE_TYPE_NAME("raspi3ap"),
>        .parent         = TYPE_RASPI_MACHINE,
>        .registerable   = qemu_binary_has_target_aarch64,
>        .class_init     = raspi3ap_machine_class_init,
>    }, {
>
> Having:
>
> bool qemu_binary_has_target_arm(void)
> {
>    return qemu_arch_available(QEMU_ARCH_ARM);
> }

I don't know where this is going and what are the details here but why add 
yet another one line function that calls another one that's identical to a 
third one. Why not put in TypeInfo .arch = QEMU_ARCH_ARM then compare to 
that when needed? Although it's questionable if arch belongs to QOM 
TypeInfo and not in Device or Machine instead this may be needed if you 
have to decide on this when registering types. I'm not sure about what 
.registerable means here but more common spelling might be .registrable 
(which suggests this could be problematic in practice so maybe try to find 
a better name for this).

Regards,
BALATON Zoltan

> Now back to Thomas suggestion, we could define 2 TypeInfo arrays,
> but I foresee lot of code churn when devices has to be made
> available on different setup combinations; so with that in mind
> the QOM registerable() callback appears a bit more future proof.
>
>

Re: [RFC PATCH 1/4] qom: Introduce TypeInfo::registerable() callback

[Richard Henderson]

On 3/5/25 08:12, Philippe Mathieu-Daudé wrote:
> Introduce theTypeInfo::registerable() callback to allow
> runtime decision on whether register a QOM type or not.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/qom/object.h | 1 +
>   qom/object.c         | 4 ++++
>   qom/trace-events     | 1 +
>   3 files changed, 6 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [RFC PATCH 2/4] hw/arm/raspi: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[Thomas Huth]

On 05/03/2025 20.07, Philippe Mathieu-Daudé wrote:
> On 5/3/25 19:35, Thomas Huth wrote:
>> On 05/03/2025 19.12, Cédric Le Goater wrote:
>>> On 3/5/25 18:40, Thomas Huth wrote:
>>>> On 05/03/2025 17.12, Philippe Mathieu-Daudé wrote:
>>>>> For legacy ARM binaries, legacy_binary_is_64bit() is
>>>>> equivalent of the compile time TARGET_AARCH64 definition.
>>>>>
>>>>> Use it as TypeInfo::registerable() callback to dynamically
>>>>> add Aarch64 specific types in qemu-system-aarch64 binary,
>>>>> removing the need of TARGET_AARCH64 #ifdef'ry.
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> ---
>>>>>   hw/arm/bcm2836.c | 6 ++----
>>>>>   hw/arm/raspi.c   | 7 +++----
>>>>>   2 files changed, 5 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
>>>>> index 95e16806fa1..88a32e5fc20 100644
>>>>> --- a/hw/arm/bcm2836.c
>>>>> +++ b/hw/arm/bcm2836.c
>>>>> @@ -12,6 +12,7 @@
>>>>>   #include "qemu/osdep.h"
>>>>>   #include "qapi/error.h"
>>>>>   #include "qemu/module.h"
>>>>> +#include "qemu/legacy_binary_info.h"
>>>>>   #include "hw/arm/bcm2836.h"
>>>>>   #include "hw/arm/raspi_platform.h"
>>>>>   #include "hw/sysbus.h"
>>>>> @@ -195,7 +196,6 @@ static void bcm2836_class_init(ObjectClass *oc, 
>>>>> void *data)
>>>>>       dc->realize = bcm2836_realize;
>>>>>   };
>>>>> -#ifdef TARGET_AARCH64
>>>>>   static void bcm2837_class_init(ObjectClass *oc, void *data)
>>>>>   {
>>>>>       DeviceClass *dc = DEVICE_CLASS(oc);
>>>>> @@ -208,7 +208,6 @@ static void bcm2837_class_init(ObjectClass *oc, 
>>>>> void *data)
>>>>>       bc->clusterid = 0x0;
>>>>>       dc->realize = bcm2836_realize;
>>>>>   };
>>>>> -#endif
>>>>>   static const TypeInfo bcm283x_types[] = {
>>>>>       {
>>>>> @@ -219,12 +218,11 @@ static const TypeInfo bcm283x_types[] = {
>>>>>           .name           = TYPE_BCM2836,
>>>>>           .parent         = TYPE_BCM283X,
>>>>>           .class_init     = bcm2836_class_init,
>>>>> -#ifdef TARGET_AARCH64
>>>>>       }, {
>>>>>           .name           = TYPE_BCM2837,
>>>>>           .parent         = TYPE_BCM283X,
>>>>> +        .registerable   = legacy_binary_is_64bit,
>>>>>           .class_init     = bcm2837_class_init,
>>>>> -#endif
>>>>>       }, {
>>>>>           .name           = TYPE_BCM283X,
>>>>>           .parent         = TYPE_BCM283X_BASE,
>>>>> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
>>>>> index dce35ca11aa..f7e647a9cbf 100644
>>>>> --- a/hw/arm/raspi.c
>>>>> +++ b/hw/arm/raspi.c
>>>>> @@ -15,6 +15,7 @@
>>>>>   #include "qemu/osdep.h"
>>>>>   #include "qemu/units.h"
>>>>>   #include "qemu/cutils.h"
>>>>> +#include "qemu/legacy_binary_info.h"
>>>>>   #include "qapi/error.h"
>>>>>   #include "hw/arm/boot.h"
>>>>>   #include "hw/arm/bcm2836.h"
>>>>> @@ -367,7 +368,6 @@ static void raspi2b_machine_class_init(ObjectClass 
>>>>> *oc, void *data)
>>>>>       raspi_machine_class_init(mc, rmc->board_rev);
>>>>>   };
>>>>> -#ifdef TARGET_AARCH64
>>>>>   static void raspi3ap_machine_class_init(ObjectClass *oc, void *data)
>>>>>   {
>>>>>       MachineClass *mc = MACHINE_CLASS(oc);
>>>>> @@ -387,7 +387,6 @@ static void raspi3b_machine_class_init(ObjectClass 
>>>>> *oc, void *data)
>>>>>       rmc->board_rev = 0xa02082;
>>>>>       raspi_machine_class_init(mc, rmc->board_rev);
>>>>>   };
>>>>> -#endif /* TARGET_AARCH64 */
>>>>>   static const TypeInfo raspi_machine_types[] = {
>>>>>       {
>>>>> @@ -402,16 +401,16 @@ static const TypeInfo raspi_machine_types[] = {
>>>>>           .name           = MACHINE_TYPE_NAME("raspi2b"),
>>>>>           .parent         = TYPE_RASPI_MACHINE,
>>>>>           .class_init     = raspi2b_machine_class_init,
>>>>> -#ifdef TARGET_AARCH64
>>>>>       }, {
>>>>>           .name           = MACHINE_TYPE_NAME("raspi3ap"),
>>>>>           .parent         = TYPE_RASPI_MACHINE,
>>>>> +        .registerable   = legacy_binary_is_64bit,
>>>>>           .class_init     = raspi3ap_machine_class_init,
>>>>>       }, {
>>>>>           .name           = MACHINE_TYPE_NAME("raspi3b"),
>>>>>           .parent         = TYPE_RASPI_MACHINE,
>>>>> +        .registerable   = legacy_binary_is_64bit,
>>>>>           .class_init     = raspi3b_machine_class_init,
>>>>> -#endif
>>>>>       }, {
>>>>>           .name           = TYPE_RASPI_MACHINE,
>>>>>           .parent         = TYPE_RASPI_BASE_MACHINE,
>>>>
>>>> Uh, this (together with patch 1) looks very cumbersome. Why don't you 
>>>> simply split the array into two, one for 32-bit and one for 64-bit, and 
>>>> then use a simply "if (legacy_binary_is_64bit())" in the type_init 
>>>> function instead?
>>>
>>> Sounds like a good idea.
>>>
>>> So we would have DEFINE_TYPES() and DEFINE_TYPES64() macros ?
>>
>> Either that - or simply use type_init() directly here for the time being.
> 
> As Pierrick noted on private chat, my approach doesn't scale, I should
> use smth in the lines of:
> 
>      }, {
>          .name           = MACHINE_TYPE_NAME("raspi2b"),
>          .parent         = TYPE_RASPI_MACHINE,
>          .registerable   = qemu_binary_has_target_arm,
>          .class_init     = raspi2b_machine_class_init,
>      }, {
>          .name           = MACHINE_TYPE_NAME("raspi3ap"),
>          .parent         = TYPE_RASPI_MACHINE,
>          .registerable   = qemu_binary_has_target_aarch64,
>          .class_init     = raspi3ap_machine_class_init,
>      }, {
> 
> Having:
> 
> bool qemu_binary_has_target_arm(void)
> {
>      return qemu_arch_available(QEMU_ARCH_ARM);
> }
> 
> Now back to Thomas suggestion, we could define 2 TypeInfo arrays,
> but I foresee lot of code churn when devices has to be made
> available on different setup combinations; so with that in mind
> the QOM registerable() callback appears a bit more future proof.

Honestly, in this case, I'd rather prefer some code churn now instead of 
having a unnecessary callback interface until forever! Just my 0.02 €.

  Thomas

Re: [RFC PATCH 2/4] hw/arm/raspi: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[Daniel P. Berrangé]

On Wed, Mar 05, 2025 at 05:12:46PM +0100, Philippe Mathieu-Daudé wrote:
> For legacy ARM binaries, legacy_binary_is_64bit() is
> equivalent of the compile time TARGET_AARCH64 definition.
> 
> Use it as TypeInfo::registerable() callback to dynamically
> add Aarch64 specific types in qemu-system-aarch64 binary,
> removing the need of TARGET_AARCH64 #ifdef'ry.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/arm/bcm2836.c | 6 ++----
>  hw/arm/raspi.c   | 7 +++----
>  2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 95e16806fa1..88a32e5fc20 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c


> @@ -219,12 +218,11 @@ static const TypeInfo bcm283x_types[] = {
>          .name           = TYPE_BCM2836,
>          .parent         = TYPE_BCM283X,
>          .class_init     = bcm2836_class_init,
> -#ifdef TARGET_AARCH64
>      }, {
>          .name           = TYPE_BCM2837,
>          .parent         = TYPE_BCM283X,
> +        .registerable   = legacy_binary_is_64bit,
>          .class_init     = bcm2837_class_init,
> -#endif
>      }, {
>          .name           = TYPE_BCM283X,
>          .parent         = TYPE_BCM283X_BASE,

So historically we have a subset of machines that are only exposed in
the qemu-system-aarch64 binary, and not qemu-system-arm.

You're attempting to build a single binary to cover both 32 & 64 bit
arm, so need to be able to filter what machines are available to
create when the symlink indicates invokation of the 32-bit binary.

If we extend your approach into the future, we may eventually have
a qemu-system-any with all targets, and symlinks from the legacy
binary names, so need to filter based on target as well as on
32/64-bit.


The reason you want the .registerable callback is so that we can
have a single static list of all machine types passed into
DEFINE_TYPES in bulk, instead of implementing a manual constructor
to do type registration and conditionally calling type_register()
for each type.

So I can understand why you took this approach, but conceptually I'm not
comfortable with the semantics of 'type_register' being a method that
registers a type, except when it doesn't register a type. That feels
like dubious semantic design.

If I step back a level, I would ask why do we need to skip the registration
of machine types at all ?

It is because existing code blindly assumes that if a machine class is
registered, then it is valid to instantiate that machine class. IMHO this
is where the current design limitation is.

No matter what we do the code will exist in  memory in the binary. If we
register a type that's a tiny bit of memory to hold the "Class" instance
and ought to be logically harmless if we never instantiate a type.

IMHO we should always unconditionally register all types for which we
have code in memory. If there are types like machine types, where some
Classes should be blocked from instantiation based on certain criteria,
we should instead represent that with a check at that class level in
some manner.

i.e, we could have a helper

    bool machine_is_available(MachineClass *mc)

which we use as a filter when querying machine types that are available to
instantiate (-M help, query-machines), and as a pre-condition prior to
instiating a machine type (-M <name>).

That may in turn imply a callback

   bool (*is_available)(void)

on the MachineClass, for which your legacy_binary_is_64bit() method would
serve as an impl.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [RFC PATCH 2/4] hw/arm/raspi: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

[Peter Maydell]

On Thu, 6 Mar 2025 at 09:21, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Mar 05, 2025 at 05:12:46PM +0100, Philippe Mathieu-Daudé wrote:
> > For legacy ARM binaries, legacy_binary_is_64bit() is
> > equivalent of the compile time TARGET_AARCH64 definition.
> >
> > Use it as TypeInfo::registerable() callback to dynamically
> > add Aarch64 specific types in qemu-system-aarch64 binary,
> > removing the need of TARGET_AARCH64 #ifdef'ry.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >  hw/arm/bcm2836.c | 6 ++----
> >  hw/arm/raspi.c   | 7 +++----
> >  2 files changed, 5 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> > index 95e16806fa1..88a32e5fc20 100644
> > --- a/hw/arm/bcm2836.c
> > +++ b/hw/arm/bcm2836.c
>
>
> > @@ -219,12 +218,11 @@ static const TypeInfo bcm283x_types[] = {
> >          .name           = TYPE_BCM2836,
> >          .parent         = TYPE_BCM283X,
> >          .class_init     = bcm2836_class_init,
> > -#ifdef TARGET_AARCH64
> >      }, {
> >          .name           = TYPE_BCM2837,
> >          .parent         = TYPE_BCM283X,
> > +        .registerable   = legacy_binary_is_64bit,
> >          .class_init     = bcm2837_class_init,
> > -#endif
> >      }, {
> >          .name           = TYPE_BCM283X,
> >          .parent         = TYPE_BCM283X_BASE,
>
> So historically we have a subset of machines that are only exposed in
> the qemu-system-aarch64 binary, and not qemu-system-arm.
>
> You're attempting to build a single binary to cover both 32 & 64 bit
> arm, so need to be able to filter what machines are available to
> create when the symlink indicates invokation of the 32-bit binary.

What machines are there that we don't want to provide, though?
We don't provide for instance raspi4b in today's qemu-system-arm,
but that's because it wouldn't work there: qemu-system-arm
doesn't have the 64-bit CPUs. But if we have a single binary
that has both 32 and 64 bit arm in it, then the 64-bit CPUs
will be in that combined binary. Do we actually need to explicitly
forbid the user from saying 'qemu-system-arm -M raspi4b' when
qemu-system-arm is a symlink and the command would work fine
if it wasn't forbidden?

I had assumed that the motivation here was that we'd like to
be able to build this C file only once. Currently we have to
build it once for the aarch64 target and once for the arm target;
to avoid that we would need to determine "does the binary I'm
in have the AArch64 CPU types this type depends on" at runtime,
not at compile time (which is what the ifdef is doing, effectively).

thanks
-- PMM