qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 00/11] qemu: Remove TARGET_NAME definition
@ 2025-03-05  0:52 Philippe Mathieu-Daudé
  2025-03-05  0:52 ` [RFC PATCH 01/11] system: Extract target-specific globals to their own compilation unit Philippe Mathieu-Daudé
                   ` (10 more replies)
  0 siblings, 11 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  0:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Richard Henderson, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé

Replace all TARGET_NAME uses by target_name() runtime,
then remove the definition and poison it.

While is looks like another cleanup, it is a small step
required for the single binary project.

Alex Bennée (1):
  plugins/loader: populate target_name with target_name()

Philippe Mathieu-Daudé (10):
  system: Extract target-specific globals to their own compilation unit
  system: Open-code qemu_init_arch_modules() using target_name()
  system: Introduce QemuArchBit enum
  system: Replace arch_type global by qemu_arch_available() helper
  include: Expose QemuArchBit enum to user emulation
  include: Declare target_name() in common "qemu/arch_info.h"
  tests/qtest: Replace TARGET_NAME -> target_name()
  user: Replace TARGET_NAME -> target_name()
  qemu: Introduce qemu_arch_name() helper
  qemu: Remove C definitions of TARGET_NAME

 meson.build                     | 10 +++--
 include/exec/poison.h           |  1 -
 include/hw/core/cpu.h           |  2 -
 include/qemu/arch_info.h        | 55 +++++++++++++++++++++++++++
 include/qemu/osdep.h            |  2 +
 include/system/arch_init.h      | 32 ----------------
 arch_info-target.c              | 67 +++++++++++++++++++++++++++++++++
 bsd-user/main.c                 |  9 +++--
 cpu-target.c                    |  5 ---
 hw/core/machine-qmp-cmds.c      |  1 +
 hw/scsi/scsi-disk.c             |  4 +-
 linux-user/main.c               | 12 +++---
 plugins/loader.c                |  3 +-
 system/arch_init.c              | 50 ------------------------
 system/globals-target.c         | 24 ++++++++++++
 system/qdev-monitor.c           |  6 +--
 system/vl.c                     | 14 ++++---
 tests/qtest/fuzz/fuzz.c         |  5 +--
 tests/qtest/fuzz/generic_fuzz.c |  4 +-
 tests/qtest/fuzz/i440fx_fuzz.c  |  5 ++-
 tests/qtest/fuzz/qos_fuzz.c     |  5 ++-
 system/meson.build              |  2 +-
 22 files changed, 195 insertions(+), 123 deletions(-)
 create mode 100644 include/qemu/arch_info.h
 delete mode 100644 include/system/arch_init.h
 create mode 100644 arch_info-target.c
 delete mode 100644 system/arch_init.c
 create mode 100644 system/globals-target.c

-- 
2.47.1



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

* [RFC PATCH 01/11] system: Extract target-specific globals to their own compilation unit
  2025-03-05  0:52 [RFC PATCH 00/11] qemu: Remove TARGET_NAME definition Philippe Mathieu-Daudé
@ 2025-03-05  0:52 ` Philippe Mathieu-Daudé
  2025-03-05  1:20   ` Pierrick Bouvier
  2025-03-11 10:24   ` Alex Bennée
  2025-03-05  0:52 ` [RFC PATCH 02/11] system: Open-code qemu_init_arch_modules() using target_name() Philippe Mathieu-Daudé
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  0:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Richard Henderson, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé

We shouldn't use target specific globals for machine properties.
These ones could be desugarized, as explained in [*]. While
certainly doable, not trivial nor my priority for now. Just move
them to a different file to clarify they are *globals*, like the
generic globals residing in system/globals.c.

[*] https://lore.kernel.org/qemu-devel/e514d6db-781d-4afe-b057-9046c70044dc@redhat.com/

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 system/arch_init.c      | 14 --------------
 system/globals-target.c | 24 ++++++++++++++++++++++++
 system/meson.build      |  1 +
 3 files changed, 25 insertions(+), 14 deletions(-)
 create mode 100644 system/globals-target.c

diff --git a/system/arch_init.c b/system/arch_init.c
index d2c32f84887..ea0842b7410 100644
--- a/system/arch_init.c
+++ b/system/arch_init.c
@@ -25,20 +25,6 @@
 #include "qemu/module.h"
 #include "system/arch_init.h"
 
-#ifdef TARGET_SPARC
-int graphic_width = 1024;
-int graphic_height = 768;
-int graphic_depth = 8;
-#elif defined(TARGET_M68K)
-int graphic_width = 800;
-int graphic_height = 600;
-int graphic_depth = 8;
-#else
-int graphic_width = 800;
-int graphic_height = 600;
-int graphic_depth = 32;
-#endif
-
 const uint32_t arch_type = QEMU_ARCH;
 
 void qemu_init_arch_modules(void)
diff --git a/system/globals-target.c b/system/globals-target.c
new file mode 100644
index 00000000000..989720591e7
--- /dev/null
+++ b/system/globals-target.c
@@ -0,0 +1,24 @@
+/*
+ * Global variables that should not exist (target specific)
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include "qemu/osdep.h"
+#include "system/system.h"
+
+#ifdef TARGET_SPARC
+int graphic_width = 1024;
+int graphic_height = 768;
+int graphic_depth = 8;
+#elif defined(TARGET_M68K)
+int graphic_width = 800;
+int graphic_height = 600;
+int graphic_depth = 8;
+#else
+int graphic_width = 800;
+int graphic_height = 600;
+int graphic_depth = 32;
+#endif
diff --git a/system/meson.build b/system/meson.build
index 4952f4b2c7d..181195410fb 100644
--- a/system/meson.build
+++ b/system/meson.build
@@ -1,6 +1,7 @@
 specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
   'arch_init.c',
   'ioport.c',
+  'globals-target.c',
   'memory.c',
   'physmem.c',
   'watchpoint.c',
-- 
2.47.1



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

* [RFC PATCH 02/11] system: Open-code qemu_init_arch_modules() using target_name()
  2025-03-05  0:52 [RFC PATCH 00/11] qemu: Remove TARGET_NAME definition Philippe Mathieu-Daudé
  2025-03-05  0:52 ` [RFC PATCH 01/11] system: Extract target-specific globals to their own compilation unit Philippe Mathieu-Daudé
@ 2025-03-05  0:52 ` Philippe Mathieu-Daudé
  2025-03-05  2:27   ` Richard Henderson
  2025-03-05  0:52 ` [RFC PATCH 03/11] system: Introduce QemuArchBit enum Philippe Mathieu-Daudé
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  0:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Richard Henderson, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé

Mostly revert commit c80cafa0c73 ("system: Add qemu_init_arch_modules")
but using target_name() instead of the target specific 'TARGET_NAME'
definition.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/system/arch_init.h | 2 --
 system/arch_init.c         | 9 ---------
 system/vl.c                | 7 ++++++-
 3 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/include/system/arch_init.h b/include/system/arch_init.h
index 5b1c1026f3a..d8b77440487 100644
--- a/include/system/arch_init.h
+++ b/include/system/arch_init.h
@@ -27,6 +27,4 @@ enum {
 
 extern const uint32_t arch_type;
 
-void qemu_init_arch_modules(void);
-
 #endif
diff --git a/system/arch_init.c b/system/arch_init.c
index ea0842b7410..b9147af93cb 100644
--- a/system/arch_init.c
+++ b/system/arch_init.c
@@ -22,15 +22,6 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
-#include "qemu/module.h"
 #include "system/arch_init.h"
 
 const uint32_t arch_type = QEMU_ARCH;
-
-void qemu_init_arch_modules(void)
-{
-#ifdef CONFIG_MODULES
-    module_init_info(qemu_modinfo);
-    module_allow_arch(TARGET_NAME);
-#endif
-}
diff --git a/system/vl.c b/system/vl.c
index 8f776684ec8..04f78466c41 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -26,6 +26,7 @@
 #include "qemu/help-texts.h"
 #include "qemu/datadir.h"
 #include "qemu/units.h"
+#include "qemu/module.h"
 #include "exec/cpu-common.h"
 #include "exec/page-vary.h"
 #include "hw/qdev-properties.h"
@@ -78,6 +79,7 @@
 #include "hw/block/block.h"
 #include "hw/i386/x86.h"
 #include "hw/i386/pc.h"
+#include "hw/core/cpu.h"
 #include "migration/cpr.h"
 #include "migration/misc.h"
 #include "migration/snapshot.h"
@@ -2885,7 +2887,10 @@ void qemu_init(int argc, char **argv)
 
     os_setup_limits();
 
-    qemu_init_arch_modules();
+#ifdef CONFIG_MODULES
+    module_init_info(qemu_modinfo);
+    module_allow_arch(target_name());
+#endif
 
     qemu_init_subsystems();
 
-- 
2.47.1



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

* [RFC PATCH 03/11] system: Introduce QemuArchBit enum
  2025-03-05  0:52 [RFC PATCH 00/11] qemu: Remove TARGET_NAME definition Philippe Mathieu-Daudé
  2025-03-05  0:52 ` [RFC PATCH 01/11] system: Extract target-specific globals to their own compilation unit Philippe Mathieu-Daudé
  2025-03-05  0:52 ` [RFC PATCH 02/11] system: Open-code qemu_init_arch_modules() using target_name() Philippe Mathieu-Daudé
@ 2025-03-05  0:52 ` Philippe Mathieu-Daudé
  2025-03-05  1:23   ` Pierrick Bouvier
                     ` (2 more replies)
  2025-03-05  0:52 ` [RFC PATCH 04/11] system: Replace arch_type global by qemu_arch_available() helper Philippe Mathieu-Daudé
                   ` (7 subsequent siblings)
  10 siblings, 3 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  0:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Richard Henderson, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé

Declare QEMU_ARCH_BIT_$target as QemuArchBit enum.
Use them to declare QEMU_ARCH_$target bitmasks.

Signed-off-by: Philippe Mathieu-Daudé <philmd@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;
 
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);
-- 
2.47.1



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

* [RFC PATCH 04/11] system: Replace arch_type global by qemu_arch_available() helper
  2025-03-05  0:52 [RFC PATCH 00/11] qemu: Remove TARGET_NAME definition Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2025-03-05  0:52 ` [RFC PATCH 03/11] system: Introduce QemuArchBit enum Philippe Mathieu-Daudé
@ 2025-03-05  0:52 ` Philippe Mathieu-Daudé
  2025-03-05  2:29   ` Richard Henderson
  2025-03-05  0:52 ` [RFC PATCH 05/11] include: Expose QemuArchBit enum to user emulation Philippe Mathieu-Daudé
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  0:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Richard Henderson, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé

qemu_arch_available() is a bit simpler to understand while
reviewing than the undocumented arch_type variable.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/system/arch_init.h | 2 +-
 hw/scsi/scsi-disk.c        | 2 +-
 system/arch_init.c         | 5 ++++-
 system/qdev-monitor.c      | 4 ++--
 system/vl.c                | 6 +++---
 5 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/system/arch_init.h b/include/system/arch_init.h
index 06e5527ec88..023d27d5bd7 100644
--- a/include/system/arch_init.h
+++ b/include/system/arch_init.h
@@ -46,6 +46,6 @@ typedef enum QemuArchBit {
 #define QEMU_ARCH_LOONGARCH     BIT(QEMU_ARCH_BIT_LOONGARCH)
 #define QEMU_ARCH_ALL           -1
 
-extern const uint32_t arch_type;
+bool qemu_arch_available(unsigned qemu_arch_mask);
 
 #endif
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e7f738b4841..7c87b20e694 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -3165,7 +3165,7 @@ static void scsi_property_add_specifics(DeviceClass *dc)
     ObjectClass *oc = OBJECT_CLASS(dc);
 
     /* The loadparm property is only supported on s390x */
-    if (arch_type & QEMU_ARCH_S390X) {
+    if (qemu_arch_available(QEMU_ARCH_S390X)) {
         object_class_property_add_str(oc, "loadparm",
                                       scsi_property_get_loadparm,
                                       scsi_property_set_loadparm);
diff --git a/system/arch_init.c b/system/arch_init.c
index fedbb18e2cc..0fec0cd416b 100644
--- a/system/arch_init.c
+++ b/system/arch_init.c
@@ -24,4 +24,7 @@
 #include "qemu/osdep.h"
 #include "system/arch_init.h"
 
-const uint32_t arch_type = BIT(QEMU_ARCH_BIT);
+bool qemu_arch_available(unsigned qemu_arch_mask)
+{
+    return qemu_arch_mask & BIT(QEMU_ARCH_BIT);
+}
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 856c9e8c32e..5588ed2047d 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -132,7 +132,7 @@ static const char *qdev_class_get_alias(DeviceClass *dc)
 
     for (i = 0; qdev_alias_table[i].typename; i++) {
         if (qdev_alias_table[i].arch_mask &&
-            !(qdev_alias_table[i].arch_mask & arch_type)) {
+            !qemu_arch_available(qdev_alias_table[i].arch_mask)) {
             continue;
         }
 
@@ -218,7 +218,7 @@ static const char *find_typename_by_alias(const char *alias)
 
     for (i = 0; qdev_alias_table[i].alias; i++) {
         if (qdev_alias_table[i].arch_mask &&
-            !(qdev_alias_table[i].arch_mask & arch_type)) {
+            !qemu_arch_available(qdev_alias_table[i].arch_mask)) {
             continue;
         }
 
diff --git a/system/vl.c b/system/vl.c
index 04f78466c41..ec93988a03a 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -878,11 +878,11 @@ static void help(int exitcode)
             g_get_prgname());
 
 #define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)    \
-    if ((arch_mask) & arch_type)                               \
+    if (qemu_arch_available(arch_mask)) \
         fputs(opt_help, stdout);
 
 #define ARCHHEADING(text, arch_mask) \
-    if ((arch_mask) & arch_type)    \
+    if (qemu_arch_available(arch_mask)) \
         puts(stringify(text));
 
 #define DEFHEADING(text) ARCHHEADING(text, QEMU_ARCH_ALL)
@@ -2929,7 +2929,7 @@ void qemu_init(int argc, char **argv)
             const QEMUOption *popt;
 
             popt = lookup_opt(argc, argv, &optarg, &optind);
-            if (!(popt->arch_mask & arch_type)) {
+            if (!qemu_arch_available(popt->arch_mask)) {
                 error_report("Option not supported for this target");
                 exit(1);
             }
-- 
2.47.1



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

* [RFC PATCH 05/11] include: Expose QemuArchBit enum to user emulation
  2025-03-05  0:52 [RFC PATCH 00/11] qemu: Remove TARGET_NAME definition Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2025-03-05  0:52 ` [RFC PATCH 04/11] system: Replace arch_type global by qemu_arch_available() helper Philippe Mathieu-Daudé
@ 2025-03-05  0:52 ` Philippe Mathieu-Daudé
  2025-03-05  2:43   ` Richard Henderson
  2025-03-05  0:52 ` [RFC PATCH 06/11] include: Declare target_name() in common "qemu/arch_info.h" Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  0:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Richard Henderson, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé

QemuArchBit isn't really specific to system emulation,
move the "system/arch_init.h" header to "qemu/arch_info.h",
a more generic include path.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 meson.build                                      | 2 +-
 include/{system/arch_init.h => qemu/arch_info.h} | 4 ++--
 system/arch_init.c => arch_info-target.c         | 2 +-
 hw/scsi/scsi-disk.c                              | 2 +-
 system/qdev-monitor.c                            | 2 +-
 system/vl.c                                      | 2 +-
 system/meson.build                               | 1 -
 7 files changed, 7 insertions(+), 8 deletions(-)
 rename include/{system/arch_init.h => qemu/arch_info.h} (97%)
 rename system/arch_init.c => arch_info-target.c (97%)

diff --git a/meson.build b/meson.build
index 1ab02a5d48d..04d8c477644 100644
--- a/meson.build
+++ b/meson.build
@@ -3766,7 +3766,7 @@ if have_block
 endif
 
 common_ss.add(files('cpu-common.c'))
-specific_ss.add(files('cpu-target.c'))
+specific_ss.add(files('cpu-target.c', 'arch_info-target.c'))
 
 subdir('system')
 
diff --git a/include/system/arch_init.h b/include/qemu/arch_info.h
similarity index 97%
rename from include/system/arch_init.h
rename to include/qemu/arch_info.h
index 023d27d5bd7..3cb95926e27 100644
--- a/include/system/arch_init.h
+++ b/include/qemu/arch_info.h
@@ -1,5 +1,5 @@
-#ifndef QEMU_ARCH_INIT_H
-#define QEMU_ARCH_INIT_H
+#ifndef QEMU_ARCH_INFO_H
+#define QEMU_ARCH_INFO_H
 
 #include "qemu/bitops.h"
 
diff --git a/system/arch_init.c b/arch_info-target.c
similarity index 97%
rename from system/arch_init.c
rename to arch_info-target.c
index 0fec0cd416b..6c11c73feb9 100644
--- a/system/arch_init.c
+++ b/arch_info-target.c
@@ -22,7 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
-#include "system/arch_init.h"
+#include "qemu/arch_info.h"
 
 bool qemu_arch_available(unsigned qemu_arch_mask)
 {
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 7c87b20e694..bafa9f39497 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -27,12 +27,12 @@
 #include "qemu/module.h"
 #include "qemu/hw-version.h"
 #include "qemu/memalign.h"
+#include "qemu/arch_info.h"
 #include "hw/scsi/scsi.h"
 #include "migration/qemu-file-types.h"
 #include "migration/vmstate.h"
 #include "hw/scsi/emulation.h"
 #include "scsi/constants.h"
-#include "system/arch_init.h"
 #include "system/block-backend.h"
 #include "system/blockdev.h"
 #include "hw/block/block.h"
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 5588ed2047d..b88e6e83512 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -22,7 +22,6 @@
 #include "monitor/hmp.h"
 #include "monitor/monitor.h"
 #include "monitor/qdev.h"
-#include "system/arch_init.h"
 #include "system/runstate.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-qdev.h"
@@ -37,6 +36,7 @@
 #include "qemu/option.h"
 #include "qemu/qemu-print.h"
 #include "qemu/option_int.h"
+#include "qemu/arch_info.h"
 #include "system/block-backend.h"
 #include "migration/misc.h"
 #include "qemu/cutils.h"
diff --git a/system/vl.c b/system/vl.c
index ec93988a03a..e96c72e5400 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -27,6 +27,7 @@
 #include "qemu/datadir.h"
 #include "qemu/units.h"
 #include "qemu/module.h"
+#include "qemu/arch_info.h"
 #include "exec/cpu-common.h"
 #include "exec/page-vary.h"
 #include "hw/qdev-properties.h"
@@ -110,7 +111,6 @@
 #include "trace/control.h"
 #include "qemu/plugin.h"
 #include "qemu/queue.h"
-#include "system/arch_init.h"
 #include "system/confidential-guest-support.h"
 
 #include "ui/qemu-spice.h"
diff --git a/system/meson.build b/system/meson.build
index 181195410fb..900c5d52ee4 100644
--- a/system/meson.build
+++ b/system/meson.build
@@ -1,5 +1,4 @@
 specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
-  'arch_init.c',
   'ioport.c',
   'globals-target.c',
   'memory.c',
-- 
2.47.1



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

* [RFC PATCH 06/11] include: Declare target_name() in common "qemu/arch_info.h"
  2025-03-05  0:52 [RFC PATCH 00/11] qemu: Remove TARGET_NAME definition Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2025-03-05  0:52 ` [RFC PATCH 05/11] include: Expose QemuArchBit enum to user emulation Philippe Mathieu-Daudé
@ 2025-03-05  0:52 ` Philippe Mathieu-Daudé
  2025-03-05  2:44   ` Richard Henderson
  2025-03-05  0:52 ` [RFC PATCH 07/11] plugins/loader: populate target_name with target_name() Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  0:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Richard Henderson, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé

No need to include the huge "hw/core/cpu.h" header to
get a simple prototype declaration such target_name().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/cpu.h      | 2 --
 include/qemu/arch_info.h   | 2 ++
 arch_info-target.c         | 5 +++++
 cpu-target.c               | 5 -----
 hw/core/machine-qmp-cmds.c | 1 +
 system/vl.c                | 1 -
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 5d11d26556a..4f538a8f96d 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1162,8 +1162,6 @@ bool cpu_exec_realizefn(CPUState *cpu, Error **errp);
 void cpu_exec_unrealizefn(CPUState *cpu);
 void cpu_exec_reset_hold(CPUState *cpu);
 
-const char *target_name(void);
-
 #ifdef COMPILING_PER_TARGET
 
 #ifndef CONFIG_USER_ONLY
diff --git a/include/qemu/arch_info.h b/include/qemu/arch_info.h
index 3cb95926e27..613dc2037db 100644
--- a/include/qemu/arch_info.h
+++ b/include/qemu/arch_info.h
@@ -46,6 +46,8 @@ typedef enum QemuArchBit {
 #define QEMU_ARCH_LOONGARCH     BIT(QEMU_ARCH_BIT_LOONGARCH)
 #define QEMU_ARCH_ALL           -1
 
+const char *target_name(void);
+
 bool qemu_arch_available(unsigned qemu_arch_mask);
 
 #endif
diff --git a/arch_info-target.c b/arch_info-target.c
index 6c11c73feb9..61007415b30 100644
--- a/arch_info-target.c
+++ b/arch_info-target.c
@@ -24,6 +24,11 @@
 #include "qemu/osdep.h"
 #include "qemu/arch_info.h"
 
+const char *target_name(void)
+{
+    return TARGET_NAME;
+}
+
 bool qemu_arch_available(unsigned qemu_arch_mask)
 {
     return qemu_arch_mask & BIT(QEMU_ARCH_BIT);
diff --git a/cpu-target.c b/cpu-target.c
index f7e17a650c0..1177e93c444 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -158,8 +158,3 @@ bool target_words_bigendian(void)
 {
     return TARGET_BIG_ENDIAN;
 }
-
-const char *target_name(void)
-{
-    return TARGET_NAME;
-}
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 3130c5cd456..3bf280cef16 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -19,6 +19,7 @@
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/type-helpers.h"
 #include "qemu/uuid.h"
+#include "qemu/arch_info.h"
 #include "qom/qom-qobject.h"
 #include "system/hostmem.h"
 #include "system/hw_accel.h"
diff --git a/system/vl.c b/system/vl.c
index e96c72e5400..a41ba4a2d5f 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -80,7 +80,6 @@
 #include "hw/block/block.h"
 #include "hw/i386/x86.h"
 #include "hw/i386/pc.h"
-#include "hw/core/cpu.h"
 #include "migration/cpr.h"
 #include "migration/misc.h"
 #include "migration/snapshot.h"
-- 
2.47.1



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

* [RFC PATCH 07/11] plugins/loader: populate target_name with target_name()
  2025-03-05  0:52 [RFC PATCH 00/11] qemu: Remove TARGET_NAME definition Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2025-03-05  0:52 ` [RFC PATCH 06/11] include: Declare target_name() in common "qemu/arch_info.h" Philippe Mathieu-Daudé
@ 2025-03-05  0:52 ` Philippe Mathieu-Daudé
  2025-03-05  0:52 ` [RFC PATCH 08/11] tests/qtest: Replace TARGET_NAME -> target_name() Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  0:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Richard Henderson, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé

From: Alex Bennée <alex.bennee@linaro.org>

We have a function we can call for this, lets not rely on macros that
stop us building once.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 plugins/loader.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/plugins/loader.c b/plugins/loader.c
index 99686b54666..e56c1003c27 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -29,6 +29,7 @@
 #include "qemu/xxhash.h"
 #include "qemu/plugin.h"
 #include "qemu/memalign.h"
+#include "qemu/arch_info.h"
 #include "hw/core/cpu.h"
 #include "exec/tb-flush.h"
 #ifndef CONFIG_USER_ONLY
@@ -297,7 +298,7 @@ int qemu_plugin_load_list(QemuPluginList *head, Error **errp)
     struct qemu_plugin_desc *desc, *next;
     g_autofree qemu_info_t *info = g_new0(qemu_info_t, 1);
 
-    info->target_name = TARGET_NAME;
+    info->target_name = target_name();
     info->version.min = QEMU_PLUGIN_MIN_VERSION;
     info->version.cur = QEMU_PLUGIN_VERSION;
 #ifndef CONFIG_USER_ONLY
-- 
2.47.1



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

* [RFC PATCH 08/11] tests/qtest: Replace TARGET_NAME -> target_name()
  2025-03-05  0:52 [RFC PATCH 00/11] qemu: Remove TARGET_NAME definition Philippe Mathieu-Daudé
                   ` (6 preceding siblings ...)
  2025-03-05  0:52 ` [RFC PATCH 07/11] plugins/loader: populate target_name with target_name() Philippe Mathieu-Daudé
@ 2025-03-05  0:52 ` Philippe Mathieu-Daudé
  2025-03-05  3:02   ` Richard Henderson
  2025-03-05  0:52 ` [RFC PATCH 09/11] user: " Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  0:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Richard Henderson, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé

In order to avoid the target-specific TARGET_NAME definition,
replace it by the generic target_name() helper.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/fuzz/fuzz.c         | 5 ++---
 tests/qtest/fuzz/generic_fuzz.c | 4 ++--
 tests/qtest/fuzz/i440fx_fuzz.c  | 5 +++--
 tests/qtest/fuzz/qos_fuzz.c     | 5 +++--
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index ca248a51a6c..84cef4907f9 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -17,6 +17,7 @@
 
 #include "qemu/cutils.h"
 #include "qemu/datadir.h"
+#include "qemu/arch_info.h"
 #include "system/system.h"
 #include "system/qtest.h"
 #include "system/runstate.h"
@@ -35,8 +36,6 @@ typedef struct FuzzTargetState {
 
 typedef QSLIST_HEAD(, FuzzTargetState) FuzzTargetList;
 
-static const char *fuzz_arch = TARGET_NAME;
-
 static FuzzTargetList *fuzz_target_list;
 static FuzzTarget *fuzz_target;
 static QTestState *fuzz_qts;
@@ -61,7 +60,7 @@ void fuzz_reset(QTestState *s)
 static QTestState *qtest_setup(void)
 {
     qtest_server_set_send_handler(&qtest_client_inproc_recv, &fuzz_qts);
-    return qtest_inproc_init(&fuzz_qts, false, fuzz_arch,
+    return qtest_inproc_init(&fuzz_qts, false, target_name(),
             &qtest_server_inproc_recv);
 }
 
diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index d107a496da6..89a9c16ce1f 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -12,10 +12,10 @@
 
 #include "qemu/osdep.h"
 #include "qemu/range.h"
+#include "qemu/arch_info.h"
 
 #include <wordexp.h>
 
-#include "hw/core/cpu.h"
 #include "tests/qtest/libqtest.h"
 #include "tests/qtest/libqos/pci-pc.h"
 #include "fuzz.h"
@@ -914,7 +914,7 @@ static size_t generic_fuzz_crossover(const uint8_t *data1, size_t size1, const
 
 static GString *generic_fuzz_cmdline(FuzzTarget *t)
 {
-    GString *cmd_line = g_string_new(TARGET_NAME);
+    GString *cmd_line = g_string_new(target_name());
     if (!getenv("QEMU_FUZZ_ARGS")) {
         usage();
     }
diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
index 155fe018f80..7b39a2ad085 100644
--- a/tests/qtest/fuzz/i440fx_fuzz.c
+++ b/tests/qtest/fuzz/i440fx_fuzz.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 
 #include "qemu/main-loop.h"
+#include "qemu/arch_info.h"
 #include "tests/qtest/libqtest.h"
 #include "tests/qtest/libqos/pci.h"
 #include "tests/qtest/libqos/pci-pc.h"
@@ -145,11 +146,11 @@ static void i440fx_fuzz_qos(QTestState *s,
     pciconfig_fuzz_qos(s, bus, Data, Size);
 }
 
-static const char *i440fx_qtest_argv = TARGET_NAME " -machine accel=qtest"
+static const char *i440fx_qtest_argv = " -machine accel=qtest"
                                        " -m 0 -display none";
 static GString *i440fx_argv(FuzzTarget *t)
 {
-    return g_string_new(i440fx_qtest_argv);
+    return g_string_append(g_string_new(target_name()), i440fx_qtest_argv);
 }
 
 
diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index d3839bf9994..fd93c405f2f 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -18,6 +18,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/units.h"
+#include "qemu/arch_info.h"
 #include "qapi/error.h"
 #include "exec/memory.h"
 #include "qemu/main-loop.h"
@@ -83,8 +84,8 @@ static GString *qos_build_main_args(void)
         test_arg = test_node->u.test.before(cmd_line, test_arg);
     }
     /* Prepend the arguments that we need */
-    g_string_prepend(cmd_line,
-            TARGET_NAME " -display none -machine accel=qtest -m 64 ");
+    g_string_prepend(cmd_line, target_name());
+    g_string_prepend(cmd_line, " -display none -machine accel=qtest -m 64 ");
     return cmd_line;
 }
 
-- 
2.47.1



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

* [RFC PATCH 09/11] user: Replace TARGET_NAME -> target_name()
  2025-03-05  0:52 [RFC PATCH 00/11] qemu: Remove TARGET_NAME definition Philippe Mathieu-Daudé
                   ` (7 preceding siblings ...)
  2025-03-05  0:52 ` [RFC PATCH 08/11] tests/qtest: Replace TARGET_NAME -> target_name() Philippe Mathieu-Daudé
@ 2025-03-05  0:52 ` Philippe Mathieu-Daudé
  2025-03-05  0:52 ` [RFC PATCH 10/11] qemu: Introduce qemu_arch_name() helper Philippe Mathieu-Daudé
  2025-03-05  0:52 ` [RFC PATCH 11/11] qemu: Remove C definitions of TARGET_NAME Philippe Mathieu-Daudé
  10 siblings, 0 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  0:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Richard Henderson, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé

In order to avoid the target-specific TARGET_NAME definition,
replace it by the generic target_name() helper.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 bsd-user/main.c   |  9 ++++++---
 linux-user/main.c | 12 +++++++-----
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index fdb160bed0f..683003c2d52 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -25,6 +25,7 @@
 #include "qemu/help-texts.h"
 #include "qemu/units.h"
 #include "qemu/accel.h"
+#include "qemu/arch_info.h"
 #include "qemu-version.h"
 #include <machine/trap.h>
 
@@ -150,9 +151,11 @@ void cpu_loop(CPUArchState *env)
 
 static void usage(void)
 {
-    printf("qemu-" TARGET_NAME " version " QEMU_FULL_VERSION
+    const char *target = target_name();
+
+    printf("qemu-%s version " QEMU_FULL_VERSION
            "\n" QEMU_COPYRIGHT "\n"
-           "usage: qemu-" TARGET_NAME " [options] program [arguments...]\n"
+           "usage: qemu-%s [options] program [arguments...]\n"
            "BSD CPU emulator (compiled for %s emulation)\n"
            "\n"
            "Standard options:\n"
@@ -188,7 +191,7 @@ static void usage(void)
            "\n"
            QEMU_HELP_BOTTOM "\n"
            ,
-           TARGET_NAME,
+           target, target, target,
            interp_prefix,
            target_dflssiz);
     exit(1);
diff --git a/linux-user/main.c b/linux-user/main.c
index 5c74c52cc52..0e4722ea273 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -21,6 +21,7 @@
 #include "qemu/help-texts.h"
 #include "qemu/units.h"
 #include "qemu/accel.h"
+#include "qemu/arch_info.h"
 #include "qemu-version.h"
 #include <sys/syscall.h>
 #include <sys/resource.h>
@@ -432,8 +433,8 @@ static void handle_arg_strace(const char *arg)
 
 static void handle_arg_version(const char *arg)
 {
-    printf("qemu-" TARGET_NAME " version " QEMU_FULL_VERSION
-           "\n" QEMU_COPYRIGHT "\n");
+    printf("qemu-%s version " QEMU_FULL_VERSION
+           "\n" QEMU_COPYRIGHT "\n", target_name());
     exit(EXIT_SUCCESS);
 }
 
@@ -543,14 +544,15 @@ static const struct qemu_argument arg_table[] = {
 static void usage(int exitcode)
 {
     const struct qemu_argument *arginfo;
+    const char *target = target_name();
     int maxarglen;
     int maxenvlen;
 
-    printf("usage: qemu-" TARGET_NAME " [options] program [arguments...]\n"
-           "Linux CPU emulator (compiled for " TARGET_NAME " emulation)\n"
+    printf("usage: qemu-%s [options] program [arguments...]\n"
+           "Linux CPU emulator (compiled for %s emulation)\n"
            "\n"
            "Options and associated environment variables:\n"
-           "\n");
+           "\n", target, target);
 
     /* Calculate column widths. We must always have at least enough space
      * for the column header.
-- 
2.47.1



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

* [RFC PATCH 10/11] qemu: Introduce qemu_arch_name() helper
  2025-03-05  0:52 [RFC PATCH 00/11] qemu: Remove TARGET_NAME definition Philippe Mathieu-Daudé
                   ` (8 preceding siblings ...)
  2025-03-05  0:52 ` [RFC PATCH 09/11] user: " Philippe Mathieu-Daudé
@ 2025-03-05  0:52 ` Philippe Mathieu-Daudé
  2025-03-05  1:32   ` BALATON Zoltan
                     ` (2 more replies)
  2025-03-05  0:52 ` [RFC PATCH 11/11] qemu: Remove C definitions of TARGET_NAME Philippe Mathieu-Daudé
  10 siblings, 3 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  0:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Richard Henderson, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé

Introduce a generic helper to get the target name of a QemuArchBit.
(This will be used for single / heterogeneous binaries).
Use it in target_name(), removing the last use of the TARGET_NAME
definition.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qemu/arch_info.h |  2 ++
 arch_info-target.c       | 34 +++++++++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/include/qemu/arch_info.h b/include/qemu/arch_info.h
index 613dc2037db..7e3192f590f 100644
--- a/include/qemu/arch_info.h
+++ b/include/qemu/arch_info.h
@@ -46,6 +46,8 @@ typedef enum QemuArchBit {
 #define QEMU_ARCH_LOONGARCH     BIT(QEMU_ARCH_BIT_LOONGARCH)
 #define QEMU_ARCH_ALL           -1
 
+const char *qemu_arch_name(QemuArchBit qemu_arch_bit);
+
 const char *target_name(void);
 
 bool qemu_arch_available(unsigned qemu_arch_mask);
diff --git a/arch_info-target.c b/arch_info-target.c
index 61007415b30..9b19fe8d56d 100644
--- a/arch_info-target.c
+++ b/arch_info-target.c
@@ -24,9 +24,41 @@
 #include "qemu/osdep.h"
 #include "qemu/arch_info.h"
 
+const char *qemu_arch_name(QemuArchBit qemu_arch_bit)
+{
+    static const char *legacy_target_names[] = {
+        [QEMU_ARCH_ALPHA] = "alpha",
+        [QEMU_ARCH_BIT_ARM] = TARGET_LONG_BITS == 32 ? "arm" : "aarch64",
+        [QEMU_ARCH_BIT_AVR] = "avr",
+        [QEMU_ARCH_BIT_HEXAGON] = "hexagon",
+        [QEMU_ARCH_BIT_HPPA] = "hppa",
+        [QEMU_ARCH_BIT_I386] = TARGET_LONG_BITS == 32 ? "i386" : "x86_64",
+        [QEMU_ARCH_BIT_LOONGARCH] = "loongarch64",
+        [QEMU_ARCH_BIT_M68K] = "m68k",
+        [QEMU_ARCH_BIT_MICROBLAZE] = TARGET_BIG_ENDIAN ? "microblaze"
+                                                       : "microblazeel",
+        [QEMU_ARCH_BIT_MIPS] = TARGET_BIG_ENDIAN
+                             ? (TARGET_LONG_BITS == 32 ? "mips" : "mips64")
+                             : (TARGET_LONG_BITS == 32 ? "mipsel" : "mips64el"),
+        [QEMU_ARCH_BIT_OPENRISC] = "or1k",
+        [QEMU_ARCH_BIT_PPC] = TARGET_LONG_BITS == 32 ? "ppc" : "ppc64",
+        [QEMU_ARCH_BIT_RISCV] = TARGET_LONG_BITS == 32 ? "riscv32" : "riscv64",
+        [QEMU_ARCH_BIT_RX] = "rx",
+        [QEMU_ARCH_BIT_S390X] = "s390x",
+        [QEMU_ARCH_BIT_SH4] = TARGET_BIG_ENDIAN ? "sh4eb" : "sh4",
+        [QEMU_ARCH_BIT_SPARC] = TARGET_LONG_BITS == 32 ? "sparc" : "sparc64",
+        [QEMU_ARCH_BIT_TRICORE] = "tricore",
+        [QEMU_ARCH_BIT_XTENSA] = TARGET_BIG_ENDIAN ? "xtensaeb" : "xtensa",
+    };
+
+    assert(qemu_arch_bit < ARRAY_SIZE(legacy_target_names));
+    assert(legacy_target_names[qemu_arch_bit]);
+    return legacy_target_names[qemu_arch_bit];
+}
+
 const char *target_name(void)
 {
-    return TARGET_NAME;
+    return qemu_arch_name(QEMU_ARCH_BIT);
 }
 
 bool qemu_arch_available(unsigned qemu_arch_mask)
-- 
2.47.1



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

* [RFC PATCH 11/11] qemu: Remove C definitions of TARGET_NAME
  2025-03-05  0:52 [RFC PATCH 00/11] qemu: Remove TARGET_NAME definition Philippe Mathieu-Daudé
                   ` (9 preceding siblings ...)
  2025-03-05  0:52 ` [RFC PATCH 10/11] qemu: Introduce qemu_arch_name() helper Philippe Mathieu-Daudé
@ 2025-03-05  0:52 ` Philippe Mathieu-Daudé
  10 siblings, 0 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  0:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Richard Henderson, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé

We don't use TARGET_NAME anymore, remove and poison it globally.

The symbol is still defined in Makefiles by the configure script
(it is required by tests/tcg/).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 meson.build           | 4 +++-
 include/exec/poison.h | 1 -
 include/qemu/osdep.h  | 2 ++
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 04d8c477644..eaae1da2e92 100644
--- a/meson.build
+++ b/meson.build
@@ -3347,7 +3347,9 @@ foreach target : target_dirs
       # Note that TARGET_BASE_ARCH ends up in config-target.h but it is
       # not used to select files from sourcesets.
       config_target_data.set('TARGET_' + v.to_upper(), 1)
-    elif k == 'TARGET_NAME' or k == 'CONFIG_QEMU_INTERP_PREFIX'
+    elif k == 'TARGET_NAME'
+      # do nothing
+    elif k == 'CONFIG_QEMU_INTERP_PREFIX'
       config_target_data.set_quoted(k, v)
     elif v == 'y'
       config_target_data.set(k, 1)
diff --git a/include/exec/poison.h b/include/exec/poison.h
index f4283f693af..0c4ad04eb97 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -34,7 +34,6 @@
 #pragma GCC poison TARGET_XTENSA
 
 #pragma GCC poison TARGET_HAS_BFLT
-#pragma GCC poison TARGET_NAME
 #pragma GCC poison TARGET_SUPPORTS_MTTCG
 #pragma GCC poison TARGET_BIG_ENDIAN
 #pragma GCC poison BSWAP_NEEDED
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 4397a906807..3167fda68e4 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -50,6 +50,8 @@
  */
 #pragma GCC poison TARGET_WORDS_BIGENDIAN
 
+#pragma GCC poison TARGET_NAME
+
 #include "qemu/compiler.h"
 
 /* Older versions of C++ don't get definitions of various macros from
-- 
2.47.1



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

* Re: [RFC PATCH 01/11] system: Extract target-specific globals to their own compilation unit
  2025-03-05  0:52 ` [RFC PATCH 01/11] system: Extract target-specific globals to their own compilation unit Philippe Mathieu-Daudé
@ 2025-03-05  1:20   ` Pierrick Bouvier
  2025-03-05  1:33     ` Philippe Mathieu-Daudé
  2025-03-11 10:24   ` Alex Bennée
  1 sibling, 1 reply; 33+ messages in thread
From: Pierrick Bouvier @ 2025-03-05  1:20 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé, Richard Henderson,
	Thomas Huth, Alex Bennée

On 3/4/25 16:52, Philippe Mathieu-Daudé wrote:
> We shouldn't use target specific globals for machine properties.
> These ones could be desugarized, as explained in [*]. While
> certainly doable, not trivial nor my priority for now. Just move
> them to a different file to clarify they are *globals*, like the
> generic globals residing in system/globals.c.
> 

Maybe those could be set globally to the default:
int graphic_width = 800;
int graphic_height = 600;
int graphic_depth = 32;

And override them for sparc and m68k in qemu_init_arch_modules function, 
using qemu_arch_name().
This way, no need to have a new file to hold them.

> [*] https://lore.kernel.org/qemu-devel/e514d6db-781d-4afe-b057-9046c70044dc@redhat.com/
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   system/arch_init.c      | 14 --------------
>   system/globals-target.c | 24 ++++++++++++++++++++++++
>   system/meson.build      |  1 +
>   3 files changed, 25 insertions(+), 14 deletions(-)
>   create mode 100644 system/globals-target.c
> 
> diff --git a/system/arch_init.c b/system/arch_init.c
> index d2c32f84887..ea0842b7410 100644
> --- a/system/arch_init.c
> +++ b/system/arch_init.c
> @@ -25,20 +25,6 @@
>   #include "qemu/module.h"
>   #include "system/arch_init.h"
>   
> -#ifdef TARGET_SPARC
> -int graphic_width = 1024;
> -int graphic_height = 768;
> -int graphic_depth = 8;
> -#elif defined(TARGET_M68K)
> -int graphic_width = 800;
> -int graphic_height = 600;
> -int graphic_depth = 8;
> -#else
> -int graphic_width = 800;
> -int graphic_height = 600;
> -int graphic_depth = 32;
> -#endif
> -
>   const uint32_t arch_type = QEMU_ARCH;
>   
>   void qemu_init_arch_modules(void)
> diff --git a/system/globals-target.c b/system/globals-target.c
> new file mode 100644
> index 00000000000..989720591e7
> --- /dev/null
> +++ b/system/globals-target.c
> @@ -0,0 +1,24 @@
> +/*
> + * Global variables that should not exist (target specific)
> + *
> + * Copyright (c) 2003-2008 Fabrice Bellard
> + *
> + * SPDX-License-Identifier: MIT
> + */
> +
> +#include "qemu/osdep.h"
> +#include "system/system.h"
> +
> +#ifdef TARGET_SPARC
> +int graphic_width = 1024;
> +int graphic_height = 768;
> +int graphic_depth = 8;
> +#elif defined(TARGET_M68K)
> +int graphic_width = 800;
> +int graphic_height = 600;
> +int graphic_depth = 8;
> +#else
> +int graphic_width = 800;
> +int graphic_height = 600;
> +int graphic_depth = 32;
> +#endif
> diff --git a/system/meson.build b/system/meson.build
> index 4952f4b2c7d..181195410fb 100644
> --- a/system/meson.build
> +++ b/system/meson.build
> @@ -1,6 +1,7 @@
>   specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
>     'arch_init.c',
>     'ioport.c',
> +  'globals-target.c',
>     'memory.c',
>     'physmem.c',
>     'watchpoint.c',


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

* Re: [RFC PATCH 03/11] system: Introduce QemuArchBit enum
  2025-03-05  0:52 ` [RFC PATCH 03/11] system: Introduce QemuArchBit enum Philippe Mathieu-Daudé
@ 2025-03-05  1:23   ` Pierrick Bouvier
  2025-03-05  1:31     ` Philippe Mathieu-Daudé
  2025-03-05  2:27   ` Richard Henderson
  2025-03-05  8:55   ` Daniel P. Berrangé
  2 siblings, 1 reply; 33+ messages in thread
From: Pierrick Bouvier @ 2025-03-05  1:23 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé, Richard Henderson,
	Thomas Huth, Alex Bennée

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é <philmd@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?
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.

> 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);


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

* Re: [RFC PATCH 03/11] system: Introduce QemuArchBit enum
  2025-03-05  1:23   ` Pierrick Bouvier
@ 2025-03-05  1:31     ` Philippe Mathieu-Daudé
  2025-03-05  1:36       ` Pierrick Bouvier
  0 siblings, 1 reply; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  1:31 UTC (permalink / raw)
  To: Pierrick Bouvier, qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé, Richard Henderson,
	Thomas Huth, Alex Bennée, Markus Armbruster

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

> 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/.

>> 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);
> 



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

* Re: [RFC PATCH 10/11] qemu: Introduce qemu_arch_name() helper
  2025-03-05  0:52 ` [RFC PATCH 10/11] qemu: Introduce qemu_arch_name() helper Philippe Mathieu-Daudé
@ 2025-03-05  1:32   ` BALATON Zoltan
  2025-03-05  1:36     ` Philippe Mathieu-Daudé
  2025-03-05  2:23   ` Richard Henderson
  2025-03-05  8:59   ` Daniel P. Berrangé
  2 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2025-03-05  1:32 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Paolo Bonzini, Pierrick Bouvier,
	Daniel P. Berrangé, Richard Henderson, Thomas Huth,
	Alex Bennée

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

On Wed, 5 Mar 2025, Philippe Mathieu-Daudé wrote:
> Introduce a generic helper to get the target name of a QemuArchBit.
> (This will be used for single / heterogeneous binaries).
> Use it in target_name(), removing the last use of the TARGET_NAME
> definition.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> include/qemu/arch_info.h |  2 ++
> arch_info-target.c       | 34 +++++++++++++++++++++++++++++++++-
> 2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/include/qemu/arch_info.h b/include/qemu/arch_info.h
> index 613dc2037db..7e3192f590f 100644
> --- a/include/qemu/arch_info.h
> +++ b/include/qemu/arch_info.h
> @@ -46,6 +46,8 @@ typedef enum QemuArchBit {
> #define QEMU_ARCH_LOONGARCH     BIT(QEMU_ARCH_BIT_LOONGARCH)
> #define QEMU_ARCH_ALL           -1
>
> +const char *qemu_arch_name(QemuArchBit qemu_arch_bit);
> +
> const char *target_name(void);
>
> bool qemu_arch_available(unsigned qemu_arch_mask);
> diff --git a/arch_info-target.c b/arch_info-target.c
> index 61007415b30..9b19fe8d56d 100644
> --- a/arch_info-target.c
> +++ b/arch_info-target.c
> @@ -24,9 +24,41 @@
> #include "qemu/osdep.h"
> #include "qemu/arch_info.h"
>
> +const char *qemu_arch_name(QemuArchBit qemu_arch_bit)
> +{
> +    static const char *legacy_target_names[] = {
> +        [QEMU_ARCH_ALPHA] = "alpha",
> +        [QEMU_ARCH_BIT_ARM] = TARGET_LONG_BITS == 32 ? "arm" : "aarch64",
> +        [QEMU_ARCH_BIT_AVR] = "avr",
> +        [QEMU_ARCH_BIT_HEXAGON] = "hexagon",
> +        [QEMU_ARCH_BIT_HPPA] = "hppa",
> +        [QEMU_ARCH_BIT_I386] = TARGET_LONG_BITS == 32 ? "i386" : "x86_64",
> +        [QEMU_ARCH_BIT_LOONGARCH] = "loongarch64",
> +        [QEMU_ARCH_BIT_M68K] = "m68k",
> +        [QEMU_ARCH_BIT_MICROBLAZE] = TARGET_BIG_ENDIAN ? "microblaze"
> +                                                       : "microblazeel",
> +        [QEMU_ARCH_BIT_MIPS] = TARGET_BIG_ENDIAN
> +                             ? (TARGET_LONG_BITS == 32 ? "mips" : "mips64")
> +                             : (TARGET_LONG_BITS == 32 ? "mipsel" : "mips64el"),
> +        [QEMU_ARCH_BIT_OPENRISC] = "or1k",
> +        [QEMU_ARCH_BIT_PPC] = TARGET_LONG_BITS == 32 ? "ppc" : "ppc64",
> +        [QEMU_ARCH_BIT_RISCV] = TARGET_LONG_BITS == 32 ? "riscv32" : "riscv64",
> +        [QEMU_ARCH_BIT_RX] = "rx",
> +        [QEMU_ARCH_BIT_S390X] = "s390x",
> +        [QEMU_ARCH_BIT_SH4] = TARGET_BIG_ENDIAN ? "sh4eb" : "sh4",
> +        [QEMU_ARCH_BIT_SPARC] = TARGET_LONG_BITS == 32 ? "sparc" : "sparc64",
> +        [QEMU_ARCH_BIT_TRICORE] = "tricore",
> +        [QEMU_ARCH_BIT_XTENSA] = TARGET_BIG_ENDIAN ? "xtensaeb" : "xtensa",
> +    };
> +
> +    assert(qemu_arch_bit < ARRAY_SIZE(legacy_target_names));
> +    assert(legacy_target_names[qemu_arch_bit]);
> +    return legacy_target_names[qemu_arch_bit];
> +}
> +
> const char *target_name(void)
> {
> -    return TARGET_NAME;
> +    return qemu_arch_name(QEMU_ARCH_BIT);
> }

Why two functions that do the same? Do you plan to remove target_name 
later or it should just do what the new function does?

Regards,
BALATON Zoltan

> bool qemu_arch_available(unsigned qemu_arch_mask)
>

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

* Re: [RFC PATCH 01/11] system: Extract target-specific globals to their own compilation unit
  2025-03-05  1:20   ` Pierrick Bouvier
@ 2025-03-05  1:33     ` Philippe Mathieu-Daudé
  2025-03-05  1:41       ` Pierrick Bouvier
  0 siblings, 1 reply; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  1:33 UTC (permalink / raw)
  To: Pierrick Bouvier, qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé, Richard Henderson,
	Thomas Huth, Alex Bennée

On 5/3/25 02:20, Pierrick Bouvier wrote:
> On 3/4/25 16:52, Philippe Mathieu-Daudé wrote:
>> We shouldn't use target specific globals for machine properties.
>> These ones could be desugarized, as explained in [*]. While
>> certainly doable, not trivial nor my priority for now. Just move
>> them to a different file to clarify they are *globals*, like the
>> generic globals residing in system/globals.c.
>>
> 
> Maybe those could be set globally to the default:
> int graphic_width = 800;
> int graphic_height = 600;
> int graphic_depth = 32;
> 
> And override them for sparc and m68k in qemu_init_arch_modules function, 
> using qemu_arch_name().
> This way, no need to have a new file to hold them.

This is not what Paolo explained ...

> 
>> [*] https://lore.kernel.org/qemu-devel/e514d6db-781d-4afe- 
>> b057-9046c70044dc@redhat.com/

... here ^, but maybe I misunderstood him.

Doing the '-g' CLI change is not my priority, and I welcome
any developer willing to help :) Here I'm just trying to make
sense of that arch_init.c file.

>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   system/arch_init.c      | 14 --------------
>>   system/globals-target.c | 24 ++++++++++++++++++++++++
>>   system/meson.build      |  1 +
>>   3 files changed, 25 insertions(+), 14 deletions(-)
>>   create mode 100644 system/globals-target.c
>>
>> diff --git a/system/arch_init.c b/system/arch_init.c
>> index d2c32f84887..ea0842b7410 100644
>> --- a/system/arch_init.c
>> +++ b/system/arch_init.c
>> @@ -25,20 +25,6 @@
>>   #include "qemu/module.h"
>>   #include "system/arch_init.h"
>> -#ifdef TARGET_SPARC
>> -int graphic_width = 1024;
>> -int graphic_height = 768;
>> -int graphic_depth = 8;
>> -#elif defined(TARGET_M68K)
>> -int graphic_width = 800;
>> -int graphic_height = 600;
>> -int graphic_depth = 8;
>> -#else
>> -int graphic_width = 800;
>> -int graphic_height = 600;
>> -int graphic_depth = 32;
>> -#endif
>> -
>>   const uint32_t arch_type = QEMU_ARCH;
>>   void qemu_init_arch_modules(void)
>> diff --git a/system/globals-target.c b/system/globals-target.c
>> new file mode 100644
>> index 00000000000..989720591e7
>> --- /dev/null
>> +++ b/system/globals-target.c
>> @@ -0,0 +1,24 @@
>> +/*
>> + * Global variables that should not exist (target specific)
>> + *
>> + * Copyright (c) 2003-2008 Fabrice Bellard
>> + *
>> + * SPDX-License-Identifier: MIT
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "system/system.h"
>> +
>> +#ifdef TARGET_SPARC
>> +int graphic_width = 1024;
>> +int graphic_height = 768;
>> +int graphic_depth = 8;
>> +#elif defined(TARGET_M68K)
>> +int graphic_width = 800;
>> +int graphic_height = 600;
>> +int graphic_depth = 8;
>> +#else
>> +int graphic_width = 800;
>> +int graphic_height = 600;
>> +int graphic_depth = 32;
>> +#endif
>> diff --git a/system/meson.build b/system/meson.build
>> index 4952f4b2c7d..181195410fb 100644
>> --- a/system/meson.build
>> +++ b/system/meson.build
>> @@ -1,6 +1,7 @@
>>   specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
>>     'arch_init.c',
>>     'ioport.c',
>> +  'globals-target.c',
>>     'memory.c',
>>     'physmem.c',
>>     'watchpoint.c',
> 



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

* Re: [RFC PATCH 10/11] qemu: Introduce qemu_arch_name() helper
  2025-03-05  1:32   ` BALATON Zoltan
@ 2025-03-05  1:36     ` Philippe Mathieu-Daudé
  2025-03-05  2:05       ` BALATON Zoltan
  0 siblings, 1 reply; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  1:36 UTC (permalink / raw)
  To: BALATON Zoltan
  Cc: qemu-devel, Paolo Bonzini, Pierrick Bouvier,
	Daniel P. Berrangé, Richard Henderson, Thomas Huth,
	Alex Bennée

On 5/3/25 02:32, BALATON Zoltan wrote:
> On Wed, 5 Mar 2025, Philippe Mathieu-Daudé wrote:
>> Introduce a generic helper to get the target name of a QemuArchBit.
>> (This will be used for single / heterogeneous binaries).
>> Use it in target_name(), removing the last use of the TARGET_NAME
>> definition.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> include/qemu/arch_info.h |  2 ++
>> arch_info-target.c       | 34 +++++++++++++++++++++++++++++++++-
>> 2 files changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/qemu/arch_info.h b/include/qemu/arch_info.h
>> index 613dc2037db..7e3192f590f 100644
>> --- a/include/qemu/arch_info.h
>> +++ b/include/qemu/arch_info.h
>> @@ -46,6 +46,8 @@ typedef enum QemuArchBit {
>> #define QEMU_ARCH_LOONGARCH     BIT(QEMU_ARCH_BIT_LOONGARCH)
>> #define QEMU_ARCH_ALL           -1
>>
>> +const char *qemu_arch_name(QemuArchBit qemu_arch_bit);
>> +
>> const char *target_name(void);
>>
>> bool qemu_arch_available(unsigned qemu_arch_mask);
>> diff --git a/arch_info-target.c b/arch_info-target.c
>> index 61007415b30..9b19fe8d56d 100644
>> --- a/arch_info-target.c
>> +++ b/arch_info-target.c
>> @@ -24,9 +24,41 @@
>> #include "qemu/osdep.h"
>> #include "qemu/arch_info.h"
>>
>> +const char *qemu_arch_name(QemuArchBit qemu_arch_bit)
>> +{
>> +    static const char *legacy_target_names[] = {
>> +        [QEMU_ARCH_ALPHA] = "alpha",
>> +        [QEMU_ARCH_BIT_ARM] = TARGET_LONG_BITS == 32 ? "arm" : 
>> "aarch64",
>> +        [QEMU_ARCH_BIT_AVR] = "avr",
>> +        [QEMU_ARCH_BIT_HEXAGON] = "hexagon",
>> +        [QEMU_ARCH_BIT_HPPA] = "hppa",
>> +        [QEMU_ARCH_BIT_I386] = TARGET_LONG_BITS == 32 ? "i386" : 
>> "x86_64",
>> +        [QEMU_ARCH_BIT_LOONGARCH] = "loongarch64",
>> +        [QEMU_ARCH_BIT_M68K] = "m68k",
>> +        [QEMU_ARCH_BIT_MICROBLAZE] = TARGET_BIG_ENDIAN ? "microblaze"
>> +                                                       : "microblazeel",
>> +        [QEMU_ARCH_BIT_MIPS] = TARGET_BIG_ENDIAN
>> +                             ? (TARGET_LONG_BITS == 32 ? "mips" : 
>> "mips64")
>> +                             : (TARGET_LONG_BITS == 32 ? "mipsel" : 
>> "mips64el"),
>> +        [QEMU_ARCH_BIT_OPENRISC] = "or1k",
>> +        [QEMU_ARCH_BIT_PPC] = TARGET_LONG_BITS == 32 ? "ppc" : "ppc64",
>> +        [QEMU_ARCH_BIT_RISCV] = TARGET_LONG_BITS == 32 ? "riscv32" : 
>> "riscv64",
>> +        [QEMU_ARCH_BIT_RX] = "rx",
>> +        [QEMU_ARCH_BIT_S390X] = "s390x",
>> +        [QEMU_ARCH_BIT_SH4] = TARGET_BIG_ENDIAN ? "sh4eb" : "sh4",
>> +        [QEMU_ARCH_BIT_SPARC] = TARGET_LONG_BITS == 32 ? "sparc" : 
>> "sparc64",
>> +        [QEMU_ARCH_BIT_TRICORE] = "tricore",
>> +        [QEMU_ARCH_BIT_XTENSA] = TARGET_BIG_ENDIAN ? "xtensaeb" : 
>> "xtensa",
>> +    };
>> +
>> +    assert(qemu_arch_bit < ARRAY_SIZE(legacy_target_names));
>> +    assert(legacy_target_names[qemu_arch_bit]);
>> +    return legacy_target_names[qemu_arch_bit];
>> +}
>> +
>> const char *target_name(void)
>> {
>> -    return TARGET_NAME;
>> +    return qemu_arch_name(QEMU_ARCH_BIT);
>> }
> 
> Why two functions that do the same? Do you plan to remove target_name 
> later or it should just do what the new function does?

target_name() is a no-op for current binaries, where one binary include
a single target.

The "single binary" will include multiple targets. We plan to symlink it
to the previous binaries and use argv[0] to mimic previous behavior.


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

* Re: [RFC PATCH 03/11] system: Introduce QemuArchBit enum
  2025-03-05  1:31     ` Philippe Mathieu-Daudé
@ 2025-03-05  1:36       ` Pierrick Bouvier
  2025-03-05  1:39         ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 33+ messages in thread
From: Pierrick Bouvier @ 2025-03-05  1:36 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé, Richard Henderson,
	Thomas Huth, Alex Bennée, Markus Armbruster

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é <philmd@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);
>>
> 


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

* Re: [RFC PATCH 03/11] system: Introduce QemuArchBit enum
  2025-03-05  1:36       ` Pierrick Bouvier
@ 2025-03-05  1:39         ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-05  1:39 UTC (permalink / raw)
  To: Pierrick Bouvier, qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé, Richard Henderson,
	Thomas Huth, Alex Bennée, Markus Armbruster

On 5/3/25 02:36, Pierrick Bouvier wrote:
> 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é <philmd@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.

OK, in the next version I'll try to better explain where this series is
going in the patch description.


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

* Re: [RFC PATCH 01/11] system: Extract target-specific globals to their own compilation unit
  2025-03-05  1:33     ` Philippe Mathieu-Daudé
@ 2025-03-05  1:41       ` Pierrick Bouvier
  0 siblings, 0 replies; 33+ messages in thread
From: Pierrick Bouvier @ 2025-03-05  1:41 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé, Richard Henderson,
	Thomas Huth, Alex Bennée

On 3/4/25 17:33, Philippe Mathieu-Daudé wrote:
> On 5/3/25 02:20, Pierrick Bouvier wrote:
>> On 3/4/25 16:52, Philippe Mathieu-Daudé wrote:
>>> We shouldn't use target specific globals for machine properties.
>>> These ones could be desugarized, as explained in [*]. While
>>> certainly doable, not trivial nor my priority for now. Just move
>>> them to a different file to clarify they are *globals*, like the
>>> generic globals residing in system/globals.c.
>>>
>>
>> Maybe those could be set globally to the default:
>> int graphic_width = 800;
>> int graphic_height = 600;
>> int graphic_depth = 32;
>>
>> And override them for sparc and m68k in qemu_init_arch_modules function,
>> using qemu_arch_name().
>> This way, no need to have a new file to hold them.
> 
> This is not what Paolo explained ...
> 
>>
>>> [*] https://lore.kernel.org/qemu-devel/e514d6db-781d-4afe-
>>> b057-9046c70044dc@redhat.com/
> 
> ... here ^, but maybe I misunderstood him.
> 
> Doing the '-g' CLI change is not my priority, and I welcome
> any developer willing to help :) Here I'm just trying to make
> sense of that arch_init.c file.
> 

It's not what I suggested, and the final state (globals are set with a 
value), should be the same.

It's just
Eventually, the
if (sparc) {
    graphic_* = ...'
} else if (m86k) {
    graphic_* = ...'
}

Can be moved somewhere else (sooner) if needed.
It's just to get rid of TARGET_SPARC and TARGET_M68K on the way, not to 
change anything else.

>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    system/arch_init.c      | 14 --------------
>>>    system/globals-target.c | 24 ++++++++++++++++++++++++
>>>    system/meson.build      |  1 +
>>>    3 files changed, 25 insertions(+), 14 deletions(-)
>>>    create mode 100644 system/globals-target.c
>>>
>>> diff --git a/system/arch_init.c b/system/arch_init.c
>>> index d2c32f84887..ea0842b7410 100644
>>> --- a/system/arch_init.c
>>> +++ b/system/arch_init.c
>>> @@ -25,20 +25,6 @@
>>>    #include "qemu/module.h"
>>>    #include "system/arch_init.h"
>>> -#ifdef TARGET_SPARC
>>> -int graphic_width = 1024;
>>> -int graphic_height = 768;
>>> -int graphic_depth = 8;
>>> -#elif defined(TARGET_M68K)
>>> -int graphic_width = 800;
>>> -int graphic_height = 600;
>>> -int graphic_depth = 8;
>>> -#else
>>> -int graphic_width = 800;
>>> -int graphic_height = 600;
>>> -int graphic_depth = 32;
>>> -#endif
>>> -
>>>    const uint32_t arch_type = QEMU_ARCH;
>>>    void qemu_init_arch_modules(void)
>>> diff --git a/system/globals-target.c b/system/globals-target.c
>>> new file mode 100644
>>> index 00000000000..989720591e7
>>> --- /dev/null
>>> +++ b/system/globals-target.c
>>> @@ -0,0 +1,24 @@
>>> +/*
>>> + * Global variables that should not exist (target specific)
>>> + *
>>> + * Copyright (c) 2003-2008 Fabrice Bellard
>>> + *
>>> + * SPDX-License-Identifier: MIT
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "system/system.h"
>>> +
>>> +#ifdef TARGET_SPARC
>>> +int graphic_width = 1024;
>>> +int graphic_height = 768;
>>> +int graphic_depth = 8;
>>> +#elif defined(TARGET_M68K)
>>> +int graphic_width = 800;
>>> +int graphic_height = 600;
>>> +int graphic_depth = 8;
>>> +#else
>>> +int graphic_width = 800;
>>> +int graphic_height = 600;
>>> +int graphic_depth = 32;
>>> +#endif
>>> diff --git a/system/meson.build b/system/meson.build
>>> index 4952f4b2c7d..181195410fb 100644
>>> --- a/system/meson.build
>>> +++ b/system/meson.build
>>> @@ -1,6 +1,7 @@
>>>    specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
>>>      'arch_init.c',
>>>      'ioport.c',
>>> +  'globals-target.c',
>>>      'memory.c',
>>>      'physmem.c',
>>>      'watchpoint.c',
>>
> 


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

* Re: [RFC PATCH 10/11] qemu: Introduce qemu_arch_name() helper
  2025-03-05  1:36     ` Philippe Mathieu-Daudé
@ 2025-03-05  2:05       ` BALATON Zoltan
  0 siblings, 0 replies; 33+ messages in thread
From: BALATON Zoltan @ 2025-03-05  2:05 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Paolo Bonzini, Pierrick Bouvier,
	Daniel P. Berrangé, Richard Henderson, Thomas Huth,
	Alex Bennée

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

On Wed, 5 Mar 2025, Philippe Mathieu-Daudé wrote:
> On 5/3/25 02:32, BALATON Zoltan wrote:
>> On Wed, 5 Mar 2025, Philippe Mathieu-Daudé wrote:
>>> Introduce a generic helper to get the target name of a QemuArchBit.
>>> (This will be used for single / heterogeneous binaries).
>>> Use it in target_name(), removing the last use of the TARGET_NAME
>>> definition.
>>> 
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> include/qemu/arch_info.h |  2 ++
>>> arch_info-target.c       | 34 +++++++++++++++++++++++++++++++++-
>>> 2 files changed, 35 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/include/qemu/arch_info.h b/include/qemu/arch_info.h
>>> index 613dc2037db..7e3192f590f 100644
>>> --- a/include/qemu/arch_info.h
>>> +++ b/include/qemu/arch_info.h
>>> @@ -46,6 +46,8 @@ typedef enum QemuArchBit {
>>> #define QEMU_ARCH_LOONGARCH     BIT(QEMU_ARCH_BIT_LOONGARCH)
>>> #define QEMU_ARCH_ALL           -1
>>> 
>>> +const char *qemu_arch_name(QemuArchBit qemu_arch_bit);
>>> +
>>> const char *target_name(void);
>>> 
>>> bool qemu_arch_available(unsigned qemu_arch_mask);
>>> diff --git a/arch_info-target.c b/arch_info-target.c
>>> index 61007415b30..9b19fe8d56d 100644
>>> --- a/arch_info-target.c
>>> +++ b/arch_info-target.c
>>> @@ -24,9 +24,41 @@
>>> #include "qemu/osdep.h"
>>> #include "qemu/arch_info.h"
>>> 
>>> +const char *qemu_arch_name(QemuArchBit qemu_arch_bit)
>>> +{
>>> +    static const char *legacy_target_names[] = {
>>> +        [QEMU_ARCH_ALPHA] = "alpha",
>>> +        [QEMU_ARCH_BIT_ARM] = TARGET_LONG_BITS == 32 ? "arm" : "aarch64",
>>> +        [QEMU_ARCH_BIT_AVR] = "avr",
>>> +        [QEMU_ARCH_BIT_HEXAGON] = "hexagon",
>>> +        [QEMU_ARCH_BIT_HPPA] = "hppa",
>>> +        [QEMU_ARCH_BIT_I386] = TARGET_LONG_BITS == 32 ? "i386" : 
>>> "x86_64",
>>> +        [QEMU_ARCH_BIT_LOONGARCH] = "loongarch64",
>>> +        [QEMU_ARCH_BIT_M68K] = "m68k",
>>> +        [QEMU_ARCH_BIT_MICROBLAZE] = TARGET_BIG_ENDIAN ? "microblaze"
>>> +                                                       : "microblazeel",
>>> +        [QEMU_ARCH_BIT_MIPS] = TARGET_BIG_ENDIAN
>>> +                             ? (TARGET_LONG_BITS == 32 ? "mips" : 
>>> "mips64")
>>> +                             : (TARGET_LONG_BITS == 32 ? "mipsel" : 
>>> "mips64el"),
>>> +        [QEMU_ARCH_BIT_OPENRISC] = "or1k",
>>> +        [QEMU_ARCH_BIT_PPC] = TARGET_LONG_BITS == 32 ? "ppc" : "ppc64",
>>> +        [QEMU_ARCH_BIT_RISCV] = TARGET_LONG_BITS == 32 ? "riscv32" : 
>>> "riscv64",
>>> +        [QEMU_ARCH_BIT_RX] = "rx",
>>> +        [QEMU_ARCH_BIT_S390X] = "s390x",
>>> +        [QEMU_ARCH_BIT_SH4] = TARGET_BIG_ENDIAN ? "sh4eb" : "sh4",
>>> +        [QEMU_ARCH_BIT_SPARC] = TARGET_LONG_BITS == 32 ? "sparc" : 
>>> "sparc64",
>>> +        [QEMU_ARCH_BIT_TRICORE] = "tricore",
>>> +        [QEMU_ARCH_BIT_XTENSA] = TARGET_BIG_ENDIAN ? "xtensaeb" : 
>>> "xtensa",
>>> +    };
>>> +
>>> +    assert(qemu_arch_bit < ARRAY_SIZE(legacy_target_names));
>>> +    assert(legacy_target_names[qemu_arch_bit]);
>>> +    return legacy_target_names[qemu_arch_bit];
>>> +}
>>> +
>>> const char *target_name(void)
>>> {
>>> -    return TARGET_NAME;
>>> +    return qemu_arch_name(QEMU_ARCH_BIT);
>>> }
>> 
>> Why two functions that do the same? Do you plan to remove target_name later 
>> or it should just do what the new function does?
>
> target_name() is a no-op for current binaries, where one binary include
> a single target.
>
> The "single binary" will include multiple targets. We plan to symlink it
> to the previous binaries and use argv[0] to mimic previous behavior.

This didn't answer the question for me why this would need target_name() 
and qemu_arch_name() which are the same function with two names. If these 
do the same why not only have one of them?

Regards,
BALATON Zoltan

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

* Re: [RFC PATCH 10/11] qemu: Introduce qemu_arch_name() helper
  2025-03-05  0:52 ` [RFC PATCH 10/11] qemu: Introduce qemu_arch_name() helper Philippe Mathieu-Daudé
  2025-03-05  1:32   ` BALATON Zoltan
@ 2025-03-05  2:23   ` Richard Henderson
  2025-03-05  8:59   ` Daniel P. Berrangé
  2 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2025-03-05  2:23 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Thomas Huth, Alex Bennée

On 3/4/25 16:52, Philippe Mathieu-Daudé wrote:
> Introduce a generic helper to get the target name of a QemuArchBit.
> (This will be used for single / heterogeneous binaries).
> Use it in target_name(), removing the last use of the TARGET_NAME
> definition.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/qemu/arch_info.h |  2 ++
>   arch_info-target.c       | 34 +++++++++++++++++++++++++++++++++-
>   2 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/include/qemu/arch_info.h b/include/qemu/arch_info.h
> index 613dc2037db..7e3192f590f 100644
> --- a/include/qemu/arch_info.h
> +++ b/include/qemu/arch_info.h
> @@ -46,6 +46,8 @@ typedef enum QemuArchBit {
>   #define QEMU_ARCH_LOONGARCH     BIT(QEMU_ARCH_BIT_LOONGARCH)
>   #define QEMU_ARCH_ALL           -1
>   
> +const char *qemu_arch_name(QemuArchBit qemu_arch_bit);
> +
>   const char *target_name(void);
>   
>   bool qemu_arch_available(unsigned qemu_arch_mask);
> diff --git a/arch_info-target.c b/arch_info-target.c
> index 61007415b30..9b19fe8d56d 100644
> --- a/arch_info-target.c
> +++ b/arch_info-target.c
> @@ -24,9 +24,41 @@
>   #include "qemu/osdep.h"
>   #include "qemu/arch_info.h"
>   
> +const char *qemu_arch_name(QemuArchBit qemu_arch_bit)
> +{
> +    static const char *legacy_target_names[] = {
> +        [QEMU_ARCH_ALPHA] = "alpha",
> +        [QEMU_ARCH_BIT_ARM] = TARGET_LONG_BITS == 32 ? "arm" : "aarch64",
> +        [QEMU_ARCH_BIT_AVR] = "avr",
> +        [QEMU_ARCH_BIT_HEXAGON] = "hexagon",
> +        [QEMU_ARCH_BIT_HPPA] = "hppa",
> +        [QEMU_ARCH_BIT_I386] = TARGET_LONG_BITS == 32 ? "i386" : "x86_64",
> +        [QEMU_ARCH_BIT_LOONGARCH] = "loongarch64",
> +        [QEMU_ARCH_BIT_M68K] = "m68k",
> +        [QEMU_ARCH_BIT_MICROBLAZE] = TARGET_BIG_ENDIAN ? "microblaze"
> +                                                       : "microblazeel",
> +        [QEMU_ARCH_BIT_MIPS] = TARGET_BIG_ENDIAN
> +                             ? (TARGET_LONG_BITS == 32 ? "mips" : "mips64")
> +                             : (TARGET_LONG_BITS == 32 ? "mipsel" : "mips64el"),
> +        [QEMU_ARCH_BIT_OPENRISC] = "or1k",
> +        [QEMU_ARCH_BIT_PPC] = TARGET_LONG_BITS == 32 ? "ppc" : "ppc64",
> +        [QEMU_ARCH_BIT_RISCV] = TARGET_LONG_BITS == 32 ? "riscv32" : "riscv64",
> +        [QEMU_ARCH_BIT_RX] = "rx",
> +        [QEMU_ARCH_BIT_S390X] = "s390x",
> +        [QEMU_ARCH_BIT_SH4] = TARGET_BIG_ENDIAN ? "sh4eb" : "sh4",
> +        [QEMU_ARCH_BIT_SPARC] = TARGET_LONG_BITS == 32 ? "sparc" : "sparc64",
> +        [QEMU_ARCH_BIT_TRICORE] = "tricore",
> +        [QEMU_ARCH_BIT_XTENSA] = TARGET_BIG_ENDIAN ? "xtensaeb" : "xtensa",
> +    };
> +
> +    assert(qemu_arch_bit < ARRAY_SIZE(legacy_target_names));
> +    assert(legacy_target_names[qemu_arch_bit]);
> +    return legacy_target_names[qemu_arch_bit];
> +}

Given all of the other #defines checked in this function,
I don't think it's helpful at all.

I think you should drop this patch for now.


r~


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

* Re: [RFC PATCH 02/11] system: Open-code qemu_init_arch_modules() using target_name()
  2025-03-05  0:52 ` [RFC PATCH 02/11] system: Open-code qemu_init_arch_modules() using target_name() Philippe Mathieu-Daudé
@ 2025-03-05  2:27   ` Richard Henderson
  0 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2025-03-05  2:27 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Thomas Huth, Alex Bennée

On 3/4/25 16:52, Philippe Mathieu-Daudé wrote:
> Mostly revert commit c80cafa0c73 ("system: Add qemu_init_arch_modules")
> but using target_name() instead of the target specific 'TARGET_NAME'
> definition.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/system/arch_init.h | 2 --
>   system/arch_init.c         | 9 ---------
>   system/vl.c                | 7 ++++++-
>   3 files changed, 6 insertions(+), 12 deletions(-)

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

r~


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

* Re: [RFC PATCH 03/11] system: Introduce QemuArchBit enum
  2025-03-05  0:52 ` [RFC PATCH 03/11] system: Introduce QemuArchBit enum Philippe Mathieu-Daudé
  2025-03-05  1:23   ` Pierrick Bouvier
@ 2025-03-05  2:27   ` Richard Henderson
  2025-03-05  8:55   ` Daniel P. Berrangé
  2 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2025-03-05  2:27 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Thomas Huth, Alex Bennée

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é<philmd@linaro.org>
> ---
>   meson.build                |  4 +--
>   include/system/arch_init.h | 65 +++++++++++++++++++++++++-------------
>   system/arch_init.c         |  2 +-
>   3 files changed, 46 insertions(+), 25 deletions(-)

If we ignore the array index in patch 10, is there any other reason for this?


r~


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

* Re: [RFC PATCH 04/11] system: Replace arch_type global by qemu_arch_available() helper
  2025-03-05  0:52 ` [RFC PATCH 04/11] system: Replace arch_type global by qemu_arch_available() helper Philippe Mathieu-Daudé
@ 2025-03-05  2:29   ` Richard Henderson
  0 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2025-03-05  2:29 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Thomas Huth, Alex Bennée

On 3/4/25 16:52, Philippe Mathieu-Daudé wrote:
> qemu_arch_available() is a bit simpler to understand while
> reviewing than the undocumented arch_type variable.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/system/arch_init.h | 2 +-
>   hw/scsi/scsi-disk.c        | 2 +-
>   system/arch_init.c         | 5 ++++-
>   system/qdev-monitor.c      | 4 ++--
>   system/vl.c                | 6 +++---
>   5 files changed, 11 insertions(+), 8 deletions(-)

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

r~


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

* Re: [RFC PATCH 05/11] include: Expose QemuArchBit enum to user emulation
  2025-03-05  0:52 ` [RFC PATCH 05/11] include: Expose QemuArchBit enum to user emulation Philippe Mathieu-Daudé
@ 2025-03-05  2:43   ` Richard Henderson
  0 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2025-03-05  2:43 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Thomas Huth, Alex Bennée

On 3/4/25 16:52, Philippe Mathieu-Daudé wrote:
> QemuArchBit isn't really specific to system emulation,
> move the "system/arch_init.h" header to "qemu/arch_info.h",
> a more generic include path.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   meson.build                                      | 2 +-
>   include/{system/arch_init.h => qemu/arch_info.h} | 4 ++--
>   system/arch_init.c => arch_info-target.c         | 2 +-
>   hw/scsi/scsi-disk.c                              | 2 +-
>   system/qdev-monitor.c                            | 2 +-
>   system/vl.c                                      | 2 +-
>   system/meson.build                               | 1 -
>   7 files changed, 7 insertions(+), 8 deletions(-)
>   rename include/{system/arch_init.h => qemu/arch_info.h} (97%)
>   rename system/arch_init.c => arch_info-target.c (97%)

I can't think of a reason to use QemuArch(Bit) in user-mode.
But I can't really justify splitting the header either.
It's certainly not "init" related anyway.

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


r~


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

* Re: [RFC PATCH 06/11] include: Declare target_name() in common "qemu/arch_info.h"
  2025-03-05  0:52 ` [RFC PATCH 06/11] include: Declare target_name() in common "qemu/arch_info.h" Philippe Mathieu-Daudé
@ 2025-03-05  2:44   ` Richard Henderson
  0 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2025-03-05  2:44 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Thomas Huth, Alex Bennée

On 3/4/25 16:52, Philippe Mathieu-Daudé wrote:
> No need to include the huge "hw/core/cpu.h" header to
> get a simple prototype declaration such target_name().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/core/cpu.h      | 2 --
>   include/qemu/arch_info.h   | 2 ++
>   arch_info-target.c         | 5 +++++
>   cpu-target.c               | 5 -----
>   hw/core/machine-qmp-cmds.c | 1 +
>   system/vl.c                | 1 -
>   6 files changed, 8 insertions(+), 8 deletions(-)

You should mention moving the function as well.  Otherwise,

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


r~


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

* Re: [RFC PATCH 08/11] tests/qtest: Replace TARGET_NAME -> target_name()
  2025-03-05  0:52 ` [RFC PATCH 08/11] tests/qtest: Replace TARGET_NAME -> target_name() Philippe Mathieu-Daudé
@ 2025-03-05  3:02   ` Richard Henderson
  0 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2025-03-05  3:02 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, Pierrick Bouvier, Daniel P. Berrangé,
	Thomas Huth, Alex Bennée

On 3/4/25 16:52, Philippe Mathieu-Daudé wrote:
> +++ b/tests/qtest/fuzz/qos_fuzz.c
> @@ -18,6 +18,7 @@
>   
>   #include "qemu/osdep.h"
>   #include "qemu/units.h"
> +#include "qemu/arch_info.h"
>   #include "qapi/error.h"
>   #include "exec/memory.h"
>   #include "qemu/main-loop.h"
> @@ -83,8 +84,8 @@ static GString *qos_build_main_args(void)
>           test_arg = test_node->u.test.before(cmd_line, test_arg);
>       }
>       /* Prepend the arguments that we need */
> -    g_string_prepend(cmd_line,
> -            TARGET_NAME " -display none -machine accel=qtest -m 64 ");
> +    g_string_prepend(cmd_line, target_name());
> +    g_string_prepend(cmd_line, " -display none -machine accel=qtest -m 64 ");
>       return cmd_line;
>   }

You're not constructing in the same order.
Swap the two lines with prepend.


r~


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

* Re: [RFC PATCH 03/11] system: Introduce QemuArchBit enum
  2025-03-05  0:52 ` [RFC PATCH 03/11] system: Introduce QemuArchBit enum Philippe Mathieu-Daudé
  2025-03-05  1:23   ` Pierrick Bouvier
  2025-03-05  2:27   ` Richard Henderson
@ 2025-03-05  8:55   ` Daniel P. Berrangé
  2025-03-05  9:00     ` Daniel P. Berrangé
  2 siblings, 1 reply; 33+ messages in thread
From: Daniel P. Berrangé @ 2025-03-05  8:55 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Paolo Bonzini, Pierrick Bouvier, Richard Henderson,
	Thomas Huth, Alex Bennée

On Wed, Mar 05, 2025 at 01:52:17AM +0100, 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é <philmd@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;

I'm somewhat inclined to say we should be defining this as a QemuArch
enum in QAPI, especially because that gives us the string <> int
conversion that you hand-code in a latter patch.


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 :|



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

* Re: [RFC PATCH 10/11] qemu: Introduce qemu_arch_name() helper
  2025-03-05  0:52 ` [RFC PATCH 10/11] qemu: Introduce qemu_arch_name() helper Philippe Mathieu-Daudé
  2025-03-05  1:32   ` BALATON Zoltan
  2025-03-05  2:23   ` Richard Henderson
@ 2025-03-05  8:59   ` Daniel P. Berrangé
  2 siblings, 0 replies; 33+ messages in thread
From: Daniel P. Berrangé @ 2025-03-05  8:59 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Paolo Bonzini, Pierrick Bouvier, Richard Henderson,
	Thomas Huth, Alex Bennée

On Wed, Mar 05, 2025 at 01:52:24AM +0100, Philippe Mathieu-Daudé wrote:
> Introduce a generic helper to get the target name of a QemuArchBit.
> (This will be used for single / heterogeneous binaries).
> Use it in target_name(), removing the last use of the TARGET_NAME
> definition.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/qemu/arch_info.h |  2 ++
>  arch_info-target.c       | 34 +++++++++++++++++++++++++++++++++-
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/include/qemu/arch_info.h b/include/qemu/arch_info.h
> index 613dc2037db..7e3192f590f 100644
> --- a/include/qemu/arch_info.h
> +++ b/include/qemu/arch_info.h
> @@ -46,6 +46,8 @@ typedef enum QemuArchBit {
>  #define QEMU_ARCH_LOONGARCH     BIT(QEMU_ARCH_BIT_LOONGARCH)
>  #define QEMU_ARCH_ALL           -1
>  
> +const char *qemu_arch_name(QemuArchBit qemu_arch_bit);
> +
>  const char *target_name(void);
>  
>  bool qemu_arch_available(unsigned qemu_arch_mask);
> diff --git a/arch_info-target.c b/arch_info-target.c
> index 61007415b30..9b19fe8d56d 100644
> --- a/arch_info-target.c
> +++ b/arch_info-target.c
> @@ -24,9 +24,41 @@
>  #include "qemu/osdep.h"
>  #include "qemu/arch_info.h"
>  
> +const char *qemu_arch_name(QemuArchBit qemu_arch_bit)
> +{
> +    static const char *legacy_target_names[] = {
> +        [QEMU_ARCH_ALPHA] = "alpha",

All the others you've used QEMU_ARCH_BIT except for this. Yes, it happens
to have the same value either way, but it still looks wrong.

> +        [QEMU_ARCH_BIT_ARM] = TARGET_LONG_BITS == 32 ? "arm" : "aarch64",
> +        [QEMU_ARCH_BIT_AVR] = "avr",
> +        [QEMU_ARCH_BIT_HEXAGON] = "hexagon",
> +        [QEMU_ARCH_BIT_HPPA] = "hppa",
> +        [QEMU_ARCH_BIT_I386] = TARGET_LONG_BITS == 32 ? "i386" : "x86_64",
> +        [QEMU_ARCH_BIT_LOONGARCH] = "loongarch64",
> +        [QEMU_ARCH_BIT_M68K] = "m68k",
> +        [QEMU_ARCH_BIT_MICROBLAZE] = TARGET_BIG_ENDIAN ? "microblaze"
> +                                                       : "microblazeel",
> +        [QEMU_ARCH_BIT_MIPS] = TARGET_BIG_ENDIAN
> +                             ? (TARGET_LONG_BITS == 32 ? "mips" : "mips64")
> +                             : (TARGET_LONG_BITS == 32 ? "mipsel" : "mips64el"),
> +        [QEMU_ARCH_BIT_OPENRISC] = "or1k",
> +        [QEMU_ARCH_BIT_PPC] = TARGET_LONG_BITS == 32 ? "ppc" : "ppc64",
> +        [QEMU_ARCH_BIT_RISCV] = TARGET_LONG_BITS == 32 ? "riscv32" : "riscv64",
> +        [QEMU_ARCH_BIT_RX] = "rx",
> +        [QEMU_ARCH_BIT_S390X] = "s390x",
> +        [QEMU_ARCH_BIT_SH4] = TARGET_BIG_ENDIAN ? "sh4eb" : "sh4",
> +        [QEMU_ARCH_BIT_SPARC] = TARGET_LONG_BITS == 32 ? "sparc" : "sparc64",
> +        [QEMU_ARCH_BIT_TRICORE] = "tricore",
> +        [QEMU_ARCH_BIT_XTENSA] = TARGET_BIG_ENDIAN ? "xtensaeb" : "xtensa",

Why do we need to give arches different names based on endian/bits, as
opposed to a fixed int -> name mapping. What's the intended  usage of
this ?

Since you're calling this a legacy_target_names, should the method also
be call qemu_legacy_arch_name() rather than qemu_arch_name - I would
have naively expected qemu_arch_name to be a plain mapping.

> +    };
> +
> +    assert(qemu_arch_bit < ARRAY_SIZE(legacy_target_names));
> +    assert(legacy_target_names[qemu_arch_bit]);
> +    return legacy_target_names[qemu_arch_bit];
> +}
> +
>  const char *target_name(void)
>  {
> -    return TARGET_NAME;
> +    return qemu_arch_name(QEMU_ARCH_BIT);
>  }
>  
>  bool qemu_arch_available(unsigned qemu_arch_mask)
> -- 
> 2.47.1
> 

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 :|



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

* Re: [RFC PATCH 03/11] system: Introduce QemuArchBit enum
  2025-03-05  8:55   ` Daniel P. Berrangé
@ 2025-03-05  9:00     ` Daniel P. Berrangé
  0 siblings, 0 replies; 33+ messages in thread
From: Daniel P. Berrangé @ 2025-03-05  9:00 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel, Paolo Bonzini,
	Pierrick Bouvier, Richard Henderson, Thomas Huth,
	Alex Bennée

On Wed, Mar 05, 2025 at 08:55:58AM +0000, Daniel P. Berrangé wrote:
> On Wed, Mar 05, 2025 at 01:52:17AM +0100, 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é <philmd@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;
> 
> I'm somewhat inclined to say we should be defining this as a QemuArch
> enum in QAPI, especially because that gives us the string <> int
> conversion that you hand-code in a latter patch.

Having said that, the auto-generated string/int conversion won't work
if we have differing name mappings based on endian/target bits size.
So scratch that idea.

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 :|



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

* Re: [RFC PATCH 01/11] system: Extract target-specific globals to their own compilation unit
  2025-03-05  0:52 ` [RFC PATCH 01/11] system: Extract target-specific globals to their own compilation unit Philippe Mathieu-Daudé
  2025-03-05  1:20   ` Pierrick Bouvier
@ 2025-03-11 10:24   ` Alex Bennée
  1 sibling, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2025-03-11 10:24 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Paolo Bonzini, Pierrick Bouvier,
	Daniel P. Berrangé, Richard Henderson, Thomas Huth

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> We shouldn't use target specific globals for machine properties.
> These ones could be desugarized, as explained in [*]. While
> certainly doable, not trivial nor my priority for now. Just move
> them to a different file to clarify they are *globals*, like the
> generic globals residing in system/globals.c.
>
> [*] https://lore.kernel.org/qemu-devel/e514d6db-781d-4afe-b057-9046c70044dc@redhat.com/
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


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

end of thread, other threads:[~2025-03-11 10:25 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-05  0:52 [RFC PATCH 00/11] qemu: Remove TARGET_NAME definition Philippe Mathieu-Daudé
2025-03-05  0:52 ` [RFC PATCH 01/11] system: Extract target-specific globals to their own compilation unit Philippe Mathieu-Daudé
2025-03-05  1:20   ` Pierrick Bouvier
2025-03-05  1:33     ` Philippe Mathieu-Daudé
2025-03-05  1:41       ` Pierrick Bouvier
2025-03-11 10:24   ` Alex Bennée
2025-03-05  0:52 ` [RFC PATCH 02/11] system: Open-code qemu_init_arch_modules() using target_name() Philippe Mathieu-Daudé
2025-03-05  2:27   ` Richard Henderson
2025-03-05  0:52 ` [RFC PATCH 03/11] system: Introduce QemuArchBit enum Philippe Mathieu-Daudé
2025-03-05  1:23   ` Pierrick Bouvier
2025-03-05  1:31     ` Philippe Mathieu-Daudé
2025-03-05  1:36       ` Pierrick Bouvier
2025-03-05  1:39         ` Philippe Mathieu-Daudé
2025-03-05  2:27   ` Richard Henderson
2025-03-05  8:55   ` Daniel P. Berrangé
2025-03-05  9:00     ` Daniel P. Berrangé
2025-03-05  0:52 ` [RFC PATCH 04/11] system: Replace arch_type global by qemu_arch_available() helper Philippe Mathieu-Daudé
2025-03-05  2:29   ` Richard Henderson
2025-03-05  0:52 ` [RFC PATCH 05/11] include: Expose QemuArchBit enum to user emulation Philippe Mathieu-Daudé
2025-03-05  2:43   ` Richard Henderson
2025-03-05  0:52 ` [RFC PATCH 06/11] include: Declare target_name() in common "qemu/arch_info.h" Philippe Mathieu-Daudé
2025-03-05  2:44   ` Richard Henderson
2025-03-05  0:52 ` [RFC PATCH 07/11] plugins/loader: populate target_name with target_name() Philippe Mathieu-Daudé
2025-03-05  0:52 ` [RFC PATCH 08/11] tests/qtest: Replace TARGET_NAME -> target_name() Philippe Mathieu-Daudé
2025-03-05  3:02   ` Richard Henderson
2025-03-05  0:52 ` [RFC PATCH 09/11] user: " Philippe Mathieu-Daudé
2025-03-05  0:52 ` [RFC PATCH 10/11] qemu: Introduce qemu_arch_name() helper Philippe Mathieu-Daudé
2025-03-05  1:32   ` BALATON Zoltan
2025-03-05  1:36     ` Philippe Mathieu-Daudé
2025-03-05  2:05       ` BALATON Zoltan
2025-03-05  2:23   ` Richard Henderson
2025-03-05  8:59   ` Daniel P. Berrangé
2025-03-05  0:52 ` [RFC PATCH 11/11] qemu: Remove C definitions of TARGET_NAME Philippe Mathieu-Daudé

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).