Re: [RFC PATCH 03/11] system: Introduce QemuArchBit enum

[Pierrick Bouvier]

On 3/4/25 17:31, Philippe Mathieu-Daudé wrote:
On 5/3/25 02:23, Pierrick Bouvier wrote:
On 3/4/25 16:52, Philippe Mathieu-Daudé wrote:
Declare QEMU_ARCH_BIT_$target as QemuArchBit enum.
Use them to declare QEMU_ARCH_$target bitmasks.
Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
   meson.build                |  4 +--
   include/system/arch_init.h | 65 +++++++++++++++++++++++++-------------
   system/arch_init.c         |  2 +-
   3 files changed, 46 insertions(+), 25 deletions(-)

diff --git a/meson.build b/meson.build
index 0a2c61d2bfa..1ab02a5d48d 100644
--- a/meson.build
+++ b/meson.build
@@ -3357,8 +3357,8 @@ foreach target : target_dirs
         config_target_data.set(k, v)
       endif
     endforeach
-  config_target_data.set('QEMU_ARCH',
-                         'QEMU_ARCH_' +
config_target['TARGET_BASE_ARCH'].to_upper())
+  config_target_data.set('QEMU_ARCH_BIT',
+                         'QEMU_ARCH_BIT_' +
config_target['TARGET_BASE_ARCH'].to_upper())
     config_target_h += {target: configure_file(output: target + '-
config-target.h',
                                                  configuration:
config_target_data)}
diff --git a/include/system/arch_init.h b/include/system/arch_init.h
index d8b77440487..06e5527ec88 100644
--- a/include/system/arch_init.h
+++ b/include/system/arch_init.h
@@ -1,29 +1,50 @@
   #ifndef QEMU_ARCH_INIT_H
   #define QEMU_ARCH_INIT_H
+#include "qemu/bitops.h"
-enum {
-    QEMU_ARCH_ALL = -1,
-    QEMU_ARCH_ALPHA = (1 << 0),
-    QEMU_ARCH_ARM = (1 << 1),
-    QEMU_ARCH_I386 = (1 << 3),
-    QEMU_ARCH_M68K = (1 << 4),
-    QEMU_ARCH_MICROBLAZE = (1 << 6),
-    QEMU_ARCH_MIPS = (1 << 7),
-    QEMU_ARCH_PPC = (1 << 8),
-    QEMU_ARCH_S390X = (1 << 9),
-    QEMU_ARCH_SH4 = (1 << 10),
-    QEMU_ARCH_SPARC = (1 << 11),
-    QEMU_ARCH_XTENSA = (1 << 12),
-    QEMU_ARCH_OPENRISC = (1 << 13),
-    QEMU_ARCH_TRICORE = (1 << 16),
-    QEMU_ARCH_HPPA = (1 << 18),
-    QEMU_ARCH_RISCV = (1 << 19),
-    QEMU_ARCH_RX = (1 << 20),
-    QEMU_ARCH_AVR = (1 << 21),
-    QEMU_ARCH_HEXAGON = (1 << 22),
-    QEMU_ARCH_LOONGARCH = (1 << 23),
-};
+typedef enum QemuArchBit {
+    QEMU_ARCH_BIT_ALPHA         = 0,
+    QEMU_ARCH_BIT_ARM           = 1,
+    QEMU_ARCH_BIT_I386          = 3,
+    QEMU_ARCH_BIT_M68K          = 4,
+    QEMU_ARCH_BIT_MICROBLAZE    = 6,
+    QEMU_ARCH_BIT_MIPS          = 7,
+    QEMU_ARCH_BIT_PPC           = 8,
+    QEMU_ARCH_BIT_S390X         = 9,
+    QEMU_ARCH_BIT_SH4           = 10,
+    QEMU_ARCH_BIT_SPARC         = 11,
+    QEMU_ARCH_BIT_XTENSA        = 12,
+    QEMU_ARCH_BIT_OPENRISC      = 13,
+    QEMU_ARCH_BIT_TRICORE       = 16,
+    QEMU_ARCH_BIT_HPPA          = 18,
+    QEMU_ARCH_BIT_RISCV         = 19,
+    QEMU_ARCH_BIT_RX            = 20,
+    QEMU_ARCH_BIT_AVR           = 21,
+    QEMU_ARCH_BIT_HEXAGON       = 22,
+    QEMU_ARCH_BIT_LOONGARCH     = 23,
+} QemuArchBit;
+
+#define QEMU_ARCH_ALPHA         BIT(QEMU_ARCH_BIT_ALPHA)
+#define QEMU_ARCH_ARM           BIT(QEMU_ARCH_BIT_ARM)
+#define QEMU_ARCH_I386          BIT(QEMU_ARCH_BIT_I386)
+#define QEMU_ARCH_M68K          BIT(QEMU_ARCH_BIT_M68K)
+#define QEMU_ARCH_MICROBLAZE    BIT(QEMU_ARCH_BIT_MICROBLAZE)
+#define QEMU_ARCH_MIPS          BIT(QEMU_ARCH_BIT_MIPS)
+#define QEMU_ARCH_PPC           BIT(QEMU_ARCH_BIT_PPC)
+#define QEMU_ARCH_S390X         BIT(QEMU_ARCH_BIT_S390X)
+#define QEMU_ARCH_SH4           BIT(QEMU_ARCH_BIT_SH4)
+#define QEMU_ARCH_SPARC         BIT(QEMU_ARCH_BIT_SPARC)
+#define QEMU_ARCH_XTENSA        BIT(QEMU_ARCH_BIT_XTENSA)
+#define QEMU_ARCH_OPENRISC      BIT(QEMU_ARCH_BIT_OPENRISC)
+#define QEMU_ARCH_TRICORE       BIT(QEMU_ARCH_BIT_TRICORE)
+#define QEMU_ARCH_HPPA          BIT(QEMU_ARCH_BIT_HPPA)
+#define QEMU_ARCH_RISCV         BIT(QEMU_ARCH_BIT_RISCV)
+#define QEMU_ARCH_RX            BIT(QEMU_ARCH_BIT_RX)
+#define QEMU_ARCH_AVR           BIT(QEMU_ARCH_BIT_AVR)
+#define QEMU_ARCH_HEXAGON       BIT(QEMU_ARCH_BIT_HEXAGON)
+#define QEMU_ARCH_LOONGARCH     BIT(QEMU_ARCH_BIT_LOONGARCH)
+#define QEMU_ARCH_ALL           -1
   extern const uint32_t arch_type;

What are we gaining by having a "bit" oriented enum, vs simple values
that can be compared too?

I'm not sure what you are asking here, these definitions are heavily
used in qemu-options.hx, and I don't plan to change them anytime soon.

For the single binary I'll try to keep the command line interface with
no change. For heterogeneous binary I plan to start with no CLI so I'm
not really considering them.


My bad, I thought we were introducing something new here.
Yes, let's stick to a minimal change.

As well, it would make sense to add subvariants (aarch64, x86_64, little
and big endian variants for some arch), so all of them are present and
can be queried easily.

IIUC what you are referring, this is planned for another interface, but
not this series which is focused on introducing qemu_arch_name() and
removing TARGET_NAME. While you don't see any improvement in duplicated
target files as of this series, some reduction should happen in the next
step which remove TARGET_BIG_ENDIAN uses in hw/.


Yes, sounds good. It's just a bit hard to track the changes between the 
cleanup of existing files, and introducing a new API to query this 
information at run time.

diff --git a/system/arch_init.c b/system/arch_init.c
index b9147af93cb..fedbb18e2cc 100644
--- a/system/arch_init.c
+++ b/system/arch_init.c
@@ -24,4 +24,4 @@
   #include "qemu/osdep.h"
   #include "system/arch_init.h"
-const uint32_t arch_type = QEMU_ARCH;
+const uint32_t arch_type = BIT(QEMU_ARCH_BIT);