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é <phi...@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.