[RFC PATCH-for-9.1 00/21] qapi: Make @query-cpu-definitions command target-agnostic

[RFC PATCH-for-9.1 00/21] qapi: Make @query-cpu-definitions command target-agnostic

[Philippe Mathieu-Daudé]

Hi Alex, Markus,

Markus mentioned QAPI problems with the heterogeneous emulation
binary. My understanding is, while QAPI can use host-specific
conditional (OS, library available, configure option), it
shouldn't use target-specific ones.

This series is an example on how to remove target specific
bits from the @query-cpu-definitions command. Target specific
code is registered as CPUClass handlers, then a generic method
is used, iterating over all targets built in.

The first set of patches were already posted / reviewed last
year.

The PPC and S390X targets still need work (help welcomed),
however the code is useful enough to be tested and see if this
is a good approach.

The only drawback is a change in QAPI introspection, because
targets not implementing @query-cpu-definitions were returning
"CommandNotFound". My view is this was an incomplete
implementation, rather than a feature.

Regards,

Phil.

Philippe Mathieu-Daudé (21):
  target/i386: Declare CPU QOM types using DEFINE_TYPES() macro
  target/mips: Declare CPU QOM types using DEFINE_TYPES() macro
  target/ppc: Declare CPU QOM types using DEFINE_TYPES() macro
  target/sparc: Declare CPU QOM types using DEFINE_TYPES() macro
  cpus: Open code OBJECT_DECLARE_TYPE() in OBJECT_DECLARE_CPU_TYPE()
  target/i386: Make X86_CPU common to new I386_CPU / X86_64_CPU types
  target/mips: Make MIPS_CPU common to new MIPS32_CPU / MIPS64_CPU types
  target/sparc: Make SPARC_CPU common to new SPARC32_CPU/SPARC64_CPU
    types
  qapi: Merge machine-common.json with qapi/machine.json
  qapi: Make CpuModel* definitions target agnostic
  qapi: Make CpuDefinitionInfo target agnostic
  system: Introduce QemuArchBit enum
  system: Introduce cpu_typename_by_arch_bit()
  system: Introduce QMP generic_query_cpu_definitions()
  target/arm: Use QMP generic_query_cpu_definitions()
  target/loongarch: Use QMP generic_query_cpu_definitions()
  target/riscv: Use QMP generic_query_cpu_definitions()
  target/i386: Use QMP generic_query_cpu_definitions()
  target/ppc: Factor ppc_add_alias_definitions() out
  target/ppc: Use QMP generic_query_cpu_definitions()
  qapi: Make @query-cpu-definitions target-agnostic

 MAINTAINERS                           |   3 +-
 qapi/machine-common.json              |  21 ----
 qapi/machine-target.json              | 167 +-------------------------
 qapi/machine.json                     | 166 ++++++++++++++++++++++++-
 qapi/qapi-schema.json                 |   1 -
 include/hw/core/cpu.h                 |   7 +-
 include/hw/core/sysemu-cpu-ops.h      |  14 +++
 include/sysemu/arch_init.h            |  71 +++++++----
 target/i386/cpu-qom.h                 |  16 ++-
 target/mips/cpu-qom.h                 |  13 +-
 target/ppc/cpu-models.h               |   4 +
 target/riscv/cpu.h                    |   2 +
 target/s390x/cpu.h                    |   2 +-
 target/sparc/cpu-qom.h                |   9 +-
 system/cpu-qmp-cmds.c                 |  71 +++++++++++
 system/cpu-qom-helpers.c              |  58 +++++++++
 target/arm/arm-qmp-cmds.c             |  27 -----
 target/i386/cpu.c                     |  77 ++++++------
 target/loongarch/loongarch-qmp-cmds.c |  25 ----
 target/mips/cpu.c                     |  34 ++++--
 target/mips/sysemu/mips-qmp-cmds.c    |  31 -----
 target/ppc/cpu_init.c                 |  53 ++++----
 target/ppc/ppc-qmp-cmds.c             |  31 +----
 target/riscv/cpu.c                    |   1 +
 target/riscv/riscv-qmp-cmds.c         |  13 +-
 target/sparc/cpu.c                    |  35 ++++--
 tests/qtest/cpu-plug-test.c           |   2 +-
 qapi/meson.build                      |   1 -
 system/meson.build                    |   2 +
 29 files changed, 515 insertions(+), 442 deletions(-)
 delete mode 100644 qapi/machine-common.json
 create mode 100644 system/cpu-qmp-cmds.c
 create mode 100644 system/cpu-qom-helpers.c

-- 
2.41.0

[PATCH-for-9.1 01/21] target/i386: Declare CPU QOM types using DEFINE_TYPES() macro

[Philippe Mathieu-Daudé]

When multiple QOM types are registered in the same file,
it is simpler to use the the DEFINE_TYPES() macro. In
particular because type array declared with such macro
are easier to review.

In few commits we are going to add more types, so replace
the type_register_static() to ease further reviews.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Message-Id: <20231013140116.255-14-philmd@linaro.org>
---
 target/i386/cpu.c | 50 ++++++++++++++++++++++-------------------------
 1 file changed, 23 insertions(+), 27 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9a210d8d92..ebf555f50f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4991,13 +4991,6 @@ static void max_x86_cpu_initfn(Object *obj)
                             &error_abort);
 }
 
-static const TypeInfo max_x86_cpu_type_info = {
-    .name = X86_CPU_TYPE_NAME("max"),
-    .parent = TYPE_X86_CPU,
-    .instance_init = max_x86_cpu_initfn,
-    .class_init = max_x86_cpu_class_init,
-};
-
 static char *feature_word_description(FeatureWordInfo *f, uint32_t bit)
 {
     assert(f->type == CPUID_FEATURE_WORD || f->type == MSR_FEATURE_WORD);
@@ -8041,19 +8034,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     }
 }
 
-static const TypeInfo x86_cpu_type_info = {
-    .name = TYPE_X86_CPU,
-    .parent = TYPE_CPU,
-    .instance_size = sizeof(X86CPU),
-    .instance_align = __alignof(X86CPU),
-    .instance_init = x86_cpu_initfn,
-    .instance_post_init = x86_cpu_post_initfn,
-
-    .abstract = true,
-    .class_size = sizeof(X86CPUClass),
-    .class_init = x86_cpu_common_class_init,
-};
-
 /* "base" CPU model, used by query-cpu-model-expansion */
 static void x86_cpu_base_class_init(ObjectClass *oc, void *data)
 {
@@ -8065,22 +8045,38 @@ static void x86_cpu_base_class_init(ObjectClass *oc, void *data)
     xcc->ordering = 8;
 }
 
-static const TypeInfo x86_base_cpu_type_info = {
-        .name = X86_CPU_TYPE_NAME("base"),
-        .parent = TYPE_X86_CPU,
-        .class_init = x86_cpu_base_class_init,
+static const TypeInfo x86_cpu_types[] = {
+    {
+        .name           = TYPE_X86_CPU,
+        .parent         = TYPE_CPU,
+        .abstract       = true,
+        .instance_size  = sizeof(X86CPU),
+        .instance_align = __alignof(X86CPU),
+        .instance_init  = x86_cpu_initfn,
+        .instance_post_init = x86_cpu_post_initfn,
+        .class_size     = sizeof(X86CPUClass),
+        .class_init     = x86_cpu_common_class_init,
+    }, {
+        .name           = X86_CPU_TYPE_NAME("base"),
+        .parent         = TYPE_X86_CPU,
+        .class_init     = x86_cpu_base_class_init,
+    }, {
+        .name           = X86_CPU_TYPE_NAME("max"),
+        .parent         = TYPE_X86_CPU,
+        .instance_init  = max_x86_cpu_initfn,
+        .class_init     = max_x86_cpu_class_init,
+    }
 };
 
+DEFINE_TYPES(x86_cpu_types)
+
 static void x86_cpu_register_types(void)
 {
     int i;
 
-    type_register_static(&x86_cpu_type_info);
     for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
         x86_register_cpudef_types(&builtin_x86_defs[i]);
     }
-    type_register_static(&max_x86_cpu_type_info);
-    type_register_static(&x86_base_cpu_type_info);
 }
 
 type_init(x86_cpu_register_types)
-- 
2.41.0

[PATCH-for-9.1 02/21] target/mips: Declare CPU QOM types using DEFINE_TYPES() macro

[Philippe Mathieu-Daudé]

When multiple QOM types are registered in the same file,
it is simpler to use the the DEFINE_TYPES() macro. In
particular because type array declared with such macro
are easier to review.

In few commits we are going to add more types, so replace
the type_register_static() to ease further reviews.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20231013140116.255-15-philmd@linaro.org>
---
 target/mips/cpu.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 8d8f690a53..c096d97fe3 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -594,17 +594,21 @@ static void mips_cpu_class_init(ObjectClass *c, void *data)
 #endif /* CONFIG_TCG */
 }
 
-static const TypeInfo mips_cpu_type_info = {
-    .name = TYPE_MIPS_CPU,
-    .parent = TYPE_CPU,
-    .instance_size = sizeof(MIPSCPU),
-    .instance_align = __alignof(MIPSCPU),
-    .instance_init = mips_cpu_initfn,
-    .abstract = true,
-    .class_size = sizeof(MIPSCPUClass),
-    .class_init = mips_cpu_class_init,
+static const TypeInfo mips_cpu_types[] = {
+    {
+        .name           = TYPE_MIPS_CPU,
+        .parent         = TYPE_CPU,
+        .instance_size  = sizeof(MIPSCPU),
+        .instance_align = __alignof(MIPSCPU),
+        .instance_init  = mips_cpu_initfn,
+        .abstract       = true,
+        .class_size     = sizeof(MIPSCPUClass),
+        .class_init     = mips_cpu_class_init,
+    }
 };
 
+DEFINE_TYPES(mips_cpu_types)
+
 static void mips_cpu_cpudef_class_init(ObjectClass *oc, void *data)
 {
     MIPSCPUClass *mcc = MIPS_CPU_CLASS(oc);
@@ -629,7 +633,6 @@ static void mips_cpu_register_types(void)
 {
     int i;
 
-    type_register_static(&mips_cpu_type_info);
     for (i = 0; i < mips_defs_number; i++) {
         mips_register_cpudef_type(&mips_defs[i]);
     }
-- 
2.41.0

Re: [PATCH-for-9.1 02/21] target/mips: Declare CPU QOM types using DEFINE_TYPES() macro

[Zhao Liu]

On Fri, Mar 15, 2024 at 02:08:50PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Fri, 15 Mar 2024 14:08:50 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH-for-9.1 02/21] target/mips: Declare CPU QOM types using
>  DEFINE_TYPES() macro
> X-Mailer: git-send-email 2.41.0
> 
> When multiple QOM types are registered in the same file,
> it is simpler to use the the DEFINE_TYPES() macro. In
> particular because type array declared with such macro
> are easier to review.
> 
> In few commits we are going to add more types, so replace
> the type_register_static() to ease further reviews.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20231013140116.255-15-philmd@linaro.org>
> ---
>  target/mips/cpu.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

[PATCH-for-9.1 03/21] target/ppc: Declare CPU QOM types using DEFINE_TYPES() macro

[Philippe Mathieu-Daudé]

When multiple QOM types are registered in the same file,
it is simpler to use the the DEFINE_TYPES() macro. In
particular because type array declared with such macro
are easier to review.

In few commits we are going to add more types, so replace
the type_register_static() to ease further reviews.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20231013140116.255-16-philmd@linaro.org>
---
 target/ppc/cpu_init.c | 52 +++++++++++++++++++------------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 7e65f08147..d5e227a6fb 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7432,39 +7432,34 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
 #endif /* CONFIG_TCG */
 }
 
-static const TypeInfo ppc_cpu_type_info = {
-    .name = TYPE_POWERPC_CPU,
-    .parent = TYPE_CPU,
-    .instance_size = sizeof(PowerPCCPU),
-    .instance_align = __alignof__(PowerPCCPU),
-    .instance_init = ppc_cpu_instance_init,
-    .instance_finalize = ppc_cpu_instance_finalize,
-    .abstract = true,
-    .class_size = sizeof(PowerPCCPUClass),
-    .class_init = ppc_cpu_class_init,
+static const TypeInfo ppc_cpu_types[] = {
+    {
+        .name           = TYPE_POWERPC_CPU,
+        .parent         = TYPE_CPU,
+        .abstract       = true,
+        .instance_size  = sizeof(PowerPCCPU),
+        .instance_align = __alignof__(PowerPCCPU),
+        .instance_init  = ppc_cpu_instance_init,
+        .instance_finalize = ppc_cpu_instance_finalize,
+        .class_size     = sizeof(PowerPCCPUClass),
+        .class_init     = ppc_cpu_class_init,
 #ifndef CONFIG_USER_ONLY
-    .interfaces = (InterfaceInfo[]) {
-          { TYPE_INTERRUPT_STATS_PROVIDER },
-          { }
+        .interfaces     = (InterfaceInfo[]) {
+              { TYPE_INTERRUPT_STATS_PROVIDER },
+              { }
+        },
+#endif
+    },
+#ifndef CONFIG_USER_ONLY
+    {
+        .name           = TYPE_PPC_VIRTUAL_HYPERVISOR,
+        .parent         = TYPE_INTERFACE,
+        .class_size     = sizeof(PPCVirtualHypervisorClass),
     },
 #endif
 };
 
-#ifndef CONFIG_USER_ONLY
-static const TypeInfo ppc_vhyp_type_info = {
-    .name = TYPE_PPC_VIRTUAL_HYPERVISOR,
-    .parent = TYPE_INTERFACE,
-    .class_size = sizeof(PPCVirtualHypervisorClass),
-};
-#endif
-
-static void ppc_cpu_register_types(void)
-{
-    type_register_static(&ppc_cpu_type_info);
-#ifndef CONFIG_USER_ONLY
-    type_register_static(&ppc_vhyp_type_info);
-#endif
-}
+DEFINE_TYPES(ppc_cpu_types)
 
 void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
 {
@@ -7658,4 +7653,3 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
 #undef RGPL
 #undef RFPL
 }
-type_init(ppc_cpu_register_types)
-- 
2.41.0

Re: [PATCH-for-9.1 03/21] target/ppc: Declare CPU QOM types using DEFINE_TYPES() macro

[Zhao Liu]

On Fri, Mar 15, 2024 at 02:08:51PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Fri, 15 Mar 2024 14:08:51 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH-for-9.1 03/21] target/ppc: Declare CPU QOM types using
>  DEFINE_TYPES() macro
> X-Mailer: git-send-email 2.41.0
> 
> When multiple QOM types are registered in the same file,
> it is simpler to use the the DEFINE_TYPES() macro. In
> particular because type array declared with such macro
> are easier to review.
> 
> In few commits we are going to add more types, so replace
> the type_register_static() to ease further reviews.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20231013140116.255-16-philmd@linaro.org>
> ---
>  target/ppc/cpu_init.c | 52 +++++++++++++++++++------------------------
>  1 file changed, 23 insertions(+), 29 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

[PATCH-for-9.1 04/21] target/sparc: Declare CPU QOM types using DEFINE_TYPES() macro

[Philippe Mathieu-Daudé]

When multiple QOM types are registered in the same file,
it is simpler to use the the DEFINE_TYPES() macro. In
particular because type array declared with such macro
are easier to review.

In few commits we are going to add more types, so replace
the type_register_static() to ease further reviews.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-Id: <20231013140116.255-17-philmd@linaro.org>
---
 target/sparc/cpu.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index dc9ead21fc..42b13ab63f 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -949,17 +949,21 @@ static void sparc_cpu_class_init(ObjectClass *oc, void *data)
     cc->tcg_ops = &sparc_tcg_ops;
 }
 
-static const TypeInfo sparc_cpu_type_info = {
-    .name = TYPE_SPARC_CPU,
-    .parent = TYPE_CPU,
-    .instance_size = sizeof(SPARCCPU),
-    .instance_align = __alignof(SPARCCPU),
-    .instance_init = sparc_cpu_initfn,
-    .abstract = true,
-    .class_size = sizeof(SPARCCPUClass),
-    .class_init = sparc_cpu_class_init,
+static const TypeInfo sparc_cpu_types[] = {
+    {
+        .name           = TYPE_SPARC_CPU,
+        .parent         = TYPE_CPU,
+        .instance_size  = sizeof(SPARCCPU),
+        .instance_align = __alignof(SPARCCPU),
+        .instance_init  = sparc_cpu_initfn,
+        .abstract       = true,
+        .class_size     = sizeof(SPARCCPUClass),
+        .class_init     = sparc_cpu_class_init,
+    }
 };
 
+DEFINE_TYPES(sparc_cpu_types)
+
 static void sparc_cpu_cpudef_class_init(ObjectClass *oc, void *data)
 {
     SPARCCPUClass *scc = SPARC_CPU_CLASS(oc);
@@ -984,7 +988,6 @@ static void sparc_cpu_register_types(void)
 {
     int i;
 
-    type_register_static(&sparc_cpu_type_info);
     for (i = 0; i < ARRAY_SIZE(sparc_defs); i++) {
         sparc_register_cpudef_type(&sparc_defs[i]);
     }
-- 
2.41.0

Re: [PATCH-for-9.1 04/21] target/sparc: Declare CPU QOM types using DEFINE_TYPES() macro

[Zhao Liu]

On Fri, Mar 15, 2024 at 02:08:52PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Fri, 15 Mar 2024 14:08:52 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH-for-9.1 04/21] target/sparc: Declare CPU QOM types using
>  DEFINE_TYPES() macro
> X-Mailer: git-send-email 2.41.0
> 
> When multiple QOM types are registered in the same file,
> it is simpler to use the the DEFINE_TYPES() macro. In
> particular because type array declared with such macro
> are easier to review.
> 
> In few commits we are going to add more types, so replace
> the type_register_static() to ease further reviews.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Message-Id: <20231013140116.255-17-philmd@linaro.org>
> ---
>  target/sparc/cpu.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)

Reviewed-by: Zhao Liu <zhao1liu@intel.com>

[PATCH-for-9.1 05/21] cpus: Open code OBJECT_DECLARE_TYPE() in OBJECT_DECLARE_CPU_TYPE()

[Philippe Mathieu-Daudé]

Since the OBJECT_DECLARE_CPU_TYPE() macro uses the abstract ArchCPU
type, when declaring multiple CPUs of the same ArchCPU type we get
an error related to the indirect G_DEFINE_AUTOPTR_CLEANUP_FUNC()
use within OBJECT_DECLARE_TYPE():

  target/mips/cpu-qom.h:31:1: error: redefinition of 'glib_autoptr_clear_ArchCPU'
  OBJECT_DECLARE_CPU_TYPE(MIPS64CPU, MIPSCPUClass, MIPS64_CPU)
  ^
  include/hw/core/cpu.h:82:5: note: expanded from macro 'OBJECT_DECLARE_CPU_TYPE'
      OBJECT_DECLARE_TYPE(ArchCPU, CpuClassType, CPU_MODULE_OBJ_NAME);
      ^
  include/qom/object.h:237:5: note: expanded from macro 'OBJECT_DECLARE_TYPE'
      G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \
      ^
  /usr/include/glib-2.0/glib/gmacros.h:1371:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
    _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func)
    ^
  /usr/include/glib-2.0/glib/gmacros.h:1354:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS'
    static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) (TypeName *_ptr)                     \
                                     ^
  /usr/include/glib-2.0/glib/gmacros.h:1338:49: note: expanded from macro '_GLIB_AUTOPTR_CLEAR_FUNC_NAME'
  #define _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) glib_autoptr_clear_##TypeName
                                                  ^
  <scratch space>:54:1: note: expanded from here
  glib_autoptr_clear_ArchCPU
  ^
  target/mips/cpu-qom.h:30:1: note: previous definition is here
  OBJECT_DECLARE_CPU_TYPE(MIPS32CPU, MIPSCPUClass, MIPS32_CPU)
  ^

Avoid that problem by expanding the OBJECT_DECLARE_TYPE() macro
within OBJECT_DECLARE_CPU_TYPE().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Acked-by: Richard Henderson <richard.henderson@linaro.org>
---
TODO: check rth comment:
What about adding an OBJECT_DECLARE_CPU_SUBTYPE that omits half the stuff instead?
We don't need another object typedef at all, for instance.
---
 include/hw/core/cpu.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index ec14f74ce5..4c2e5095bf 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -78,7 +78,12 @@ DECLARE_CLASS_CHECKERS(CPUClass, CPU,
  */
 #define OBJECT_DECLARE_CPU_TYPE(CpuInstanceType, CpuClassType, CPU_MODULE_OBJ_NAME) \
     typedef struct ArchCPU CpuInstanceType; \
-    OBJECT_DECLARE_TYPE(ArchCPU, CpuClassType, CPU_MODULE_OBJ_NAME);
+    typedef struct CpuClassType CpuClassType; \
+    \
+    G_DEFINE_AUTOPTR_CLEANUP_FUNC(CpuInstanceType, object_unref) \
+    \
+    DECLARE_OBJ_CHECKERS(CpuInstanceType, CpuClassType, \
+                         CPU_MODULE_OBJ_NAME, TYPE_##CPU_MODULE_OBJ_NAME)
 
 typedef enum MMUAccessType {
     MMU_DATA_LOAD  = 0,
-- 
2.41.0

Re: [PATCH-for-9.1 05/21] cpus: Open code OBJECT_DECLARE_TYPE() in OBJECT_DECLARE_CPU_TYPE()

[Zhao Liu]

Hi Philippe,

On Fri, Mar 15, 2024 at 02:08:53PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Fri, 15 Mar 2024 14:08:53 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH-for-9.1 05/21] cpus: Open code OBJECT_DECLARE_TYPE() in
>  OBJECT_DECLARE_CPU_TYPE()
> X-Mailer: git-send-email 2.41.0
> 
> Since the OBJECT_DECLARE_CPU_TYPE() macro uses the abstract ArchCPU
> type, when declaring multiple CPUs of the same ArchCPU type we get
> an error related to the indirect G_DEFINE_AUTOPTR_CLEANUP_FUNC()
> use within OBJECT_DECLARE_TYPE():
> 
>   target/mips/cpu-qom.h:31:1: error: redefinition of 'glib_autoptr_clear_ArchCPU'
>   OBJECT_DECLARE_CPU_TYPE(MIPS64CPU, MIPSCPUClass, MIPS64_CPU)
>   ^
>   include/hw/core/cpu.h:82:5: note: expanded from macro 'OBJECT_DECLARE_CPU_TYPE'
>       OBJECT_DECLARE_TYPE(ArchCPU, CpuClassType, CPU_MODULE_OBJ_NAME);
>       ^
>   include/qom/object.h:237:5: note: expanded from macro 'OBJECT_DECLARE_TYPE'
>       G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \
>       ^
>   /usr/include/glib-2.0/glib/gmacros.h:1371:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
>     _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func)
>     ^
>   /usr/include/glib-2.0/glib/gmacros.h:1354:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS'
>     static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) (TypeName *_ptr)                     \
>                                      ^
>   /usr/include/glib-2.0/glib/gmacros.h:1338:49: note: expanded from macro '_GLIB_AUTOPTR_CLEAR_FUNC_NAME'
>   #define _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) glib_autoptr_clear_##TypeName
>                                                   ^
>   <scratch space>:54:1: note: expanded from here
>   glib_autoptr_clear_ArchCPU
>   ^
>   target/mips/cpu-qom.h:30:1: note: previous definition is here
>   OBJECT_DECLARE_CPU_TYPE(MIPS32CPU, MIPSCPUClass, MIPS32_CPU)
>   ^
> 
> Avoid that problem by expanding the OBJECT_DECLARE_TYPE() macro
> within OBJECT_DECLARE_CPU_TYPE().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> TODO: check rth comment:
> What about adding an OBJECT_DECLARE_CPU_SUBTYPE that omits half the stuff instead?
> We don't need another object typedef at all, for instance.
> ---
>  include/hw/core/cpu.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index ec14f74ce5..4c2e5095bf 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -78,7 +78,12 @@ DECLARE_CLASS_CHECKERS(CPUClass, CPU,
>   */
>  #define OBJECT_DECLARE_CPU_TYPE(CpuInstanceType, CpuClassType, CPU_MODULE_OBJ_NAME) \
>      typedef struct ArchCPU CpuInstanceType; \
> -    OBJECT_DECLARE_TYPE(ArchCPU, CpuClassType, CPU_MODULE_OBJ_NAME);
> +    typedef struct CpuClassType CpuClassType; \
> +    \
> +    G_DEFINE_AUTOPTR_CLEANUP_FUNC(CpuInstanceType, object_unref) \
> +    \
> +    DECLARE_OBJ_CHECKERS(CpuInstanceType, CpuClassType, \
> +                         CPU_MODULE_OBJ_NAME, TYPE_##CPU_MODULE_OBJ_NAME)
>

The OBJECT_DECLARE_TYPE is expaneded as the following:

#define OBJECT_DECLARE_TYPE(InstanceType, ClassType, MODULE_OBJ_NAME) \
    typedef struct InstanceType InstanceType; \
    typedef struct ClassType ClassType; \
    \
    G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \
    \
    DECLARE_OBJ_CHECKERS(InstanceType, ClassType, \
                         MODULE_OBJ_NAME, TYPE_##MODULE_OBJ_NAME)

So the above code change deletes a typedef:

    typedef struct ArchCPU ArchCPU;

Will this deletion break the direct uses of ArchCPU? e.g., in
accel/tcg/translator.c:

static void set_can_do_io(DisasContextBase *db, bool val)
{
    if (db->saved_can_do_io != val) {
        ...
        tcg_gen_st8_i32(tcg_constant_i32(val), tcg_env,
                        offsetof(ArchCPU, parent_obj.neg.can_do_io) -
                        offsetof(ArchCPU, env));
    }
}

Thanks,
Zhao

[PATCH-for-9.1 06/21] target/i386: Make X86_CPU common to new I386_CPU / X86_64_CPU types

[Philippe Mathieu-Daudé]

"target/foo/cpu-qom.h" can not use any target specific definitions.

Currently "target/i386/cpu-qom.h" defines TYPE_X86_CPU depending
on the i386/x86_64 build type. This doesn't scale in a heterogeneous
context where we need to access both types concurrently.

In order to do that, introduce the new I386_CPU / X86_64_CPU
types, both inheriting a common TYPE_X86_CPU base type.

Keep the current "base" and "max" CPU types as 32 or 64-bit,
depending on the binary built.

Adapt the cpu-plug-test, since the 'base' architecture is now
common to both 32/64-bit x86 targets.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Acked-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/i386/cpu-qom.h       | 16 ++++++++++------
 target/i386/cpu.c           | 20 ++++++++++++++++++--
 tests/qtest/cpu-plug-test.c |  2 +-
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/target/i386/cpu-qom.h b/target/i386/cpu-qom.h
index d4e216d000..de28d7ea20 100644
--- a/target/i386/cpu-qom.h
+++ b/target/i386/cpu-qom.h
@@ -1,5 +1,5 @@
 /*
- * QEMU x86 CPU
+ * QEMU x86 CPU QOM header (target agnostic)
  *
  * Copyright (c) 2012 SUSE LINUX Products GmbH
  *
@@ -22,14 +22,18 @@
 
 #include "hw/core/cpu.h"
 
-#ifdef TARGET_X86_64
-#define TYPE_X86_CPU "x86_64-cpu"
-#else
-#define TYPE_X86_CPU "i386-cpu"
-#endif
+#define TYPE_X86_CPU    "x86-cpu"
+#define TYPE_I386_CPU   "i386-cpu"
+#define TYPE_X86_64_CPU "x86_64-cpu"
 
 OBJECT_DECLARE_CPU_TYPE(X86CPU, X86CPUClass, X86_CPU)
 
+OBJECT_DECLARE_CPU_TYPE(I386CPU, X86CPUClass, I386_CPU)
+OBJECT_DECLARE_CPU_TYPE(X86_64CPU, X86CPUClass, X86_64_CPU)
+
+#define X86_CPU_TYPE_SUFFIX "-" TYPE_X86_CPU
+#define X86_CPU_TYPE_NAME(name) (name X86_CPU_TYPE_SUFFIX)
+
 #define X86_CPU_TYPE_SUFFIX "-" TYPE_X86_CPU
 #define X86_CPU_TYPE_NAME(name) (name X86_CPU_TYPE_SUFFIX)
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ebf555f50f..07f64c1ea5 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8057,12 +8057,28 @@ static const TypeInfo x86_cpu_types[] = {
         .class_size     = sizeof(X86CPUClass),
         .class_init     = x86_cpu_common_class_init,
     }, {
-        .name           = X86_CPU_TYPE_NAME("base"),
+        .name           = TYPE_I386_CPU,
         .parent         = TYPE_X86_CPU,
+        .abstract       = true,
+    }, {
+        .name           = TYPE_X86_64_CPU,
+        .parent         = TYPE_X86_CPU,
+        .abstract       = true,
+    }, {
+        .name           = X86_CPU_TYPE_NAME("base"),
+#ifdef TARGET_X86_64
+        .parent         = TYPE_X86_64_CPU,
+#else
+        .parent         = TYPE_I386_CPU,
+#endif
         .class_init     = x86_cpu_base_class_init,
     }, {
         .name           = X86_CPU_TYPE_NAME("max"),
-        .parent         = TYPE_X86_CPU,
+#ifdef TARGET_X86_64
+        .parent         = TYPE_X86_64_CPU,
+#else
+        .parent         = TYPE_I386_CPU,
+#endif
         .instance_init  = max_x86_cpu_initfn,
         .class_init     = max_x86_cpu_class_init,
     }
diff --git a/tests/qtest/cpu-plug-test.c b/tests/qtest/cpu-plug-test.c
index 7f5dd5f85a..97316d131f 100644
--- a/tests/qtest/cpu-plug-test.c
+++ b/tests/qtest/cpu-plug-test.c
@@ -90,7 +90,7 @@ static void add_pc_test_case(const char *mname)
     data->machine = g_strdup(mname);
     data->cpu_model = "Haswell"; /* 1.3+ theoretically */
     data->device_model = g_strdup_printf("%s-%s-cpu", data->cpu_model,
-                                         qtest_get_arch());
+                                         qtest_get_base_arch());
     data->sockets = 1;
     data->cores = 3;
     data->threads = 2;
-- 
2.41.0

Re: [PATCH-for-9.1 06/21] target/i386: Make X86_CPU common to new I386_CPU / X86_64_CPU types

[Zhao Liu]

Hi Philippe,

On Fri, Mar 15, 2024 at 02:08:54PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Fri, 15 Mar 2024 14:08:54 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH-for-9.1 06/21] target/i386: Make X86_CPU common to new
>  I386_CPU / X86_64_CPU types
> X-Mailer: git-send-email 2.41.0
> 
> "target/foo/cpu-qom.h" can not use any target specific definitions.
> 
> Currently "target/i386/cpu-qom.h" defines TYPE_X86_CPU depending
> on the i386/x86_64 build type. This doesn't scale in a heterogeneous
> context where we need to access both types concurrently.

Does this mean that there would be a TCG case contains both 64-bit and
32-bit i386 core? ;-)

> In order to do that, introduce the new I386_CPU / X86_64_CPU
> types, both inheriting a common TYPE_X86_CPU base type.
> 
> Keep the current "base" and "max" CPU types as 32 or 64-bit,
> depending on the binary built.
> 
> Adapt the cpu-plug-test, since the 'base' architecture is now
> common to both 32/64-bit x86 targets.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/i386/cpu-qom.h       | 16 ++++++++++------
>  target/i386/cpu.c           | 20 ++++++++++++++++++--
>  tests/qtest/cpu-plug-test.c |  2 +-
>  3 files changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/target/i386/cpu-qom.h b/target/i386/cpu-qom.h
> index d4e216d000..de28d7ea20 100644
> --- a/target/i386/cpu-qom.h
> +++ b/target/i386/cpu-qom.h
> @@ -1,5 +1,5 @@
>  /*
> - * QEMU x86 CPU
> + * QEMU x86 CPU QOM header (target agnostic)
>   *
>   * Copyright (c) 2012 SUSE LINUX Products GmbH
>   *
> @@ -22,14 +22,18 @@
>  
>  #include "hw/core/cpu.h"
>  
> -#ifdef TARGET_X86_64
> -#define TYPE_X86_CPU "x86_64-cpu"
> -#else
> -#define TYPE_X86_CPU "i386-cpu"
> -#endif
> +#define TYPE_X86_CPU    "x86-cpu"
> +#define TYPE_I386_CPU   "i386-cpu"
> +#define TYPE_X86_64_CPU "x86_64-cpu"
>  
>  OBJECT_DECLARE_CPU_TYPE(X86CPU, X86CPUClass, X86_CPU)
>  
> +OBJECT_DECLARE_CPU_TYPE(I386CPU, X86CPUClass, I386_CPU)
> +OBJECT_DECLARE_CPU_TYPE(X86_64CPU, X86CPUClass, X86_64_CPU)
> +
> +#define X86_CPU_TYPE_SUFFIX "-" TYPE_X86_CPU
> +#define X86_CPU_TYPE_NAME(name) (name X86_CPU_TYPE_SUFFIX)
> +

X86_CPU_TYPE_NAME seems to be duplicated because the following line is
the existing X86_CPU_TYPE_NAME definition.

>  #define X86_CPU_TYPE_SUFFIX "-" TYPE_X86_CPU
>  #define X86_CPU_TYPE_NAME(name) (name X86_CPU_TYPE_SUFFIX)
>  
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index ebf555f50f..07f64c1ea5 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -8057,12 +8057,28 @@ static const TypeInfo x86_cpu_types[] = {
>          .class_size     = sizeof(X86CPUClass),
>          .class_init     = x86_cpu_common_class_init,
>      }, {
> -        .name           = X86_CPU_TYPE_NAME("base"),
> +        .name           = TYPE_I386_CPU,
>          .parent         = TYPE_X86_CPU,
> +        .abstract       = true,
> +    }, {
> +        .name           = TYPE_X86_64_CPU,
> +        .parent         = TYPE_X86_CPU,
> +        .abstract       = true,
> +    }, {

Should TYPE_I386_CPU/TYPE_X86_64_CPU be also wrapped with TARGET_X86_64?

Otherwise, we would keep the 32-bit CPU type definition of TYPE_I386_CPU
in the 64-bit case.

> +        .name           = X86_CPU_TYPE_NAME("base"),
> +#ifdef TARGET_X86_64
> +        .parent         = TYPE_X86_64_CPU,
> +#else
> +        .parent         = TYPE_I386_CPU,
> +#endif
>          .class_init     = x86_cpu_base_class_init,
>      }, {
>          .name           = X86_CPU_TYPE_NAME("max"),
> -        .parent         = TYPE_X86_CPU,
> +#ifdef TARGET_X86_64
> +        .parent         = TYPE_X86_64_CPU,
> +#else
> +        .parent         = TYPE_I386_CPU,
> +#endif
>          .instance_init  = max_x86_cpu_initfn,
>          .class_init     = max_x86_cpu_class_init,
>      }

Re: [PATCH-for-9.1 06/21] target/i386: Make X86_CPU common to new I386_CPU / X86_64_CPU types

[Markus Armbruster]

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

> "target/foo/cpu-qom.h" can not use any target specific definitions.
>
> Currently "target/i386/cpu-qom.h" defines TYPE_X86_CPU depending
> on the i386/x86_64 build type. This doesn't scale in a heterogeneous
> context where we need to access both types concurrently.
>
> In order to do that, introduce the new I386_CPU / X86_64_CPU
> types, both inheriting a common TYPE_X86_CPU base type.
>
> Keep the current "base" and "max" CPU types as 32 or 64-bit,
> depending on the binary built.
>
> Adapt the cpu-plug-test, since the 'base' architecture is now
> common to both 32/64-bit x86 targets.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>

[...]

> diff --git a/tests/qtest/cpu-plug-test.c b/tests/qtest/cpu-plug-test.c
> index 7f5dd5f85a..97316d131f 100644
> --- a/tests/qtest/cpu-plug-test.c
> +++ b/tests/qtest/cpu-plug-test.c
> @@ -90,7 +90,7 @@ static void add_pc_test_case(const char *mname)
>      data->machine = g_strdup(mname);
>      data->cpu_model = "Haswell"; /* 1.3+ theoretically */
>      data->device_model = g_strdup_printf("%s-%s-cpu", data->cpu_model,
> -                                         qtest_get_arch());
> +                                         qtest_get_base_arch());
>      data->sockets = 1;
>      data->cores = 3;
>      data->threads = 2;

Doesn't build for me:

../tests/qtest/cpu-plug-test.c: In function ‘add_pc_test_case’:
../tests/qtest/cpu-plug-test.c:93:42: error: implicit declaration of function ‘qtest_get_base_arch’; did you mean ‘qtest_get_arch’? [-Werror=implicit-function-declaration]
   93 |                                          qtest_get_base_arch());
      |                                          ^~~~~~~~~~~~~~~~~~~
      |                                          qtest_get_arch
../tests/qtest/cpu-plug-test.c:93:42: error: nested extern declaration of ‘qtest_get_base_arch’ [-Werror=nested-externs]
../tests/qtest/cpu-plug-test.c:92:47: error: format ‘%s’ expects argument of type ‘char *’, but argument 3 has type ‘int’ [-Werror=format=]
   92 |     data->device_model = g_strdup_printf("%s-%s-cpu", data->cpu_model,
      |                                              ~^
      |                                               |
      |                                               char *
      |                                              %d
   93 |                                          qtest_get_base_arch());
      |                                          ~~~~~~~~~~~~~~~~~~~~~
      |                                          |
      |                                          int

Re: [PATCH-for-9.1 06/21] target/i386: Make X86_CPU common to new I386_CPU / X86_64_CPU types

[Philippe Mathieu-Daudé]

On 26/3/24 11:57, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> "target/foo/cpu-qom.h" can not use any target specific definitions.
>>
>> Currently "target/i386/cpu-qom.h" defines TYPE_X86_CPU depending
>> on the i386/x86_64 build type. This doesn't scale in a heterogeneous
>> context where we need to access both types concurrently.
>>
>> In order to do that, introduce the new I386_CPU / X86_64_CPU
>> types, both inheriting a common TYPE_X86_CPU base type.
>>
>> Keep the current "base" and "max" CPU types as 32 or 64-bit,
>> depending on the binary built.
>>
>> Adapt the cpu-plug-test, since the 'base' architecture is now
>> common to both 32/64-bit x86 targets.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> 
> [...]
> 
>> diff --git a/tests/qtest/cpu-plug-test.c b/tests/qtest/cpu-plug-test.c
>> index 7f5dd5f85a..97316d131f 100644
>> --- a/tests/qtest/cpu-plug-test.c
>> +++ b/tests/qtest/cpu-plug-test.c
>> @@ -90,7 +90,7 @@ static void add_pc_test_case(const char *mname)
>>       data->machine = g_strdup(mname);
>>       data->cpu_model = "Haswell"; /* 1.3+ theoretically */
>>       data->device_model = g_strdup_printf("%s-%s-cpu", data->cpu_model,
>> -                                         qtest_get_arch());
>> +                                         qtest_get_base_arch());
>>       data->sockets = 1;
>>       data->cores = 3;
>>       data->threads = 2;
> 
> Doesn't build for me:
> 
> ../tests/qtest/cpu-plug-test.c: In function ‘add_pc_test_case’:
> ../tests/qtest/cpu-plug-test.c:93:42: error: implicit declaration of function ‘qtest_get_base_arch’; did you mean ‘qtest_get_arch’? [-Werror=implicit-function-declaration]
>     93 |                                          qtest_get_base_arch());
>        |                                          ^~~~~~~~~~~~~~~~~~~
>        |                                          qtest_get_arch
> ../tests/qtest/cpu-plug-test.c:93:42: error: nested extern declaration of ‘qtest_get_base_arch’ [-Werror=nested-externs]
> ../tests/qtest/cpu-plug-test.c:92:47: error: format ‘%s’ expects argument of type ‘char *’, but argument 3 has type ‘int’ [-Werror=format=]
>     92 |     data->device_model = g_strdup_printf("%s-%s-cpu", data->cpu_model,
>        |                                              ~^
>        |                                               |
>        |                                               char *
>        |                                              %d
>     93 |                                          qtest_get_base_arch());
>        |                                          ~~~~~~~~~~~~~~~~~~~~~
>        |                                          |
>        |                                          int

Sorry, I forgot to mention this series is based on:
https://lore.kernel.org/qemu-devel/20231010074952.79165-1-philmd@linaro.org/
"qtest: Introduce qtest_get_base_arch() and qtest_get_arch_bits()"

[PATCH-for-9.1 07/21] target/mips: Make MIPS_CPU common to new MIPS32_CPU / MIPS64_CPU types

[Philippe Mathieu-Daudé]

"target/foo/cpu-qom.h" can not use any target specific definitions.

Currently "target/mips/cpu-qom.h" defines TYPE_MIPS_CPU depending
on the mips(32)/mips64 build type. This doesn't scale in a
heterogeneous context where we need to access both types concurrently.

In order to do that, introduce the new MIPS32_CPU / MIPS64_CPU types,
both inheriting a common TYPE_MIPS_CPU base type.

Keep the current CPU types registered in mips_register_cpudef_type()
as 32 or 64-bit, but instead of depending on the binary built being
targeting 32/64-bit, check whether the CPU is 64-bit by looking at
the CPU_MIPS64 bit.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/cpu-qom.h              | 13 +++++++------
 target/mips/cpu.c                  | 11 ++++++++++-
 target/mips/sysemu/mips-qmp-cmds.c | 26 ++------------------------
 3 files changed, 19 insertions(+), 31 deletions(-)

diff --git a/target/mips/cpu-qom.h b/target/mips/cpu-qom.h
index 0eea2a2598..bf464f16b6 100644
--- a/target/mips/cpu-qom.h
+++ b/target/mips/cpu-qom.h
@@ -1,5 +1,5 @@
 /*
- * QEMU MIPS CPU
+ * QEMU MIPS CPU QOM header (target agnostic)
  *
  * Copyright (c) 2012 SUSE LINUX Products GmbH
  *
@@ -22,14 +22,15 @@
 
 #include "hw/core/cpu.h"
 
-#ifdef TARGET_MIPS64
-#define TYPE_MIPS_CPU "mips64-cpu"
-#else
-#define TYPE_MIPS_CPU "mips-cpu"
-#endif
+#define TYPE_MIPS_CPU   "mips-cpu"
+#define TYPE_MIPS32_CPU "mips32-cpu"
+#define TYPE_MIPS64_CPU "mips64-cpu"
 
 OBJECT_DECLARE_CPU_TYPE(MIPSCPU, MIPSCPUClass, MIPS_CPU)
 
+OBJECT_DECLARE_CPU_TYPE(MIPS32CPU, MIPSCPUClass, MIPS32_CPU)
+OBJECT_DECLARE_CPU_TYPE(MIPS64CPU, MIPSCPUClass, MIPS64_CPU)
+
 #define MIPS_CPU_TYPE_SUFFIX "-" TYPE_MIPS_CPU
 #define MIPS_CPU_TYPE_NAME(model) model MIPS_CPU_TYPE_SUFFIX
 
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index c096d97fe3..f3ea6175f2 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -604,6 +604,14 @@ static const TypeInfo mips_cpu_types[] = {
         .abstract       = true,
         .class_size     = sizeof(MIPSCPUClass),
         .class_init     = mips_cpu_class_init,
+    }, {
+        .name           = TYPE_MIPS32_CPU,
+        .parent         = TYPE_MIPS_CPU,
+        .abstract       = true,
+    }, {
+        .name           = TYPE_MIPS64_CPU,
+        .parent         = TYPE_MIPS_CPU,
+        .abstract       = true,
     }
 };
 
@@ -620,7 +628,8 @@ static void mips_register_cpudef_type(const struct mips_def_t *def)
     char *typename = mips_cpu_type_name(def->name);
     TypeInfo ti = {
         .name = typename,
-        .parent = TYPE_MIPS_CPU,
+        .parent = def->insn_flags & CPU_MIPS64
+                  ? TYPE_MIPS64_CPU : TYPE_MIPS32_CPU,
         .class_init = mips_cpu_cpudef_class_init,
         .class_data = (void *)def,
     };
diff --git a/target/mips/sysemu/mips-qmp-cmds.c b/target/mips/sysemu/mips-qmp-cmds.c
index 7340ac70ba..329db3a028 100644
--- a/target/mips/sysemu/mips-qmp-cmds.c
+++ b/target/mips/sysemu/mips-qmp-cmds.c
@@ -8,31 +8,9 @@
 
 #include "qemu/osdep.h"
 #include "qapi/qapi-commands-machine-target.h"
-#include "cpu.h"
-
-static void mips_cpu_add_definition(gpointer data, gpointer user_data)
-{
-    ObjectClass *oc = data;
-    CpuDefinitionInfoList **cpu_list = user_data;
-    CpuDefinitionInfo *info;
-    const char *typename;
-
-    typename = object_class_get_name(oc);
-    info = g_malloc0(sizeof(*info));
-    info->name = cpu_model_from_type(typename);
-    info->q_typename = g_strdup(typename);
-
-    QAPI_LIST_PREPEND(*cpu_list, info);
-}
+#include "qapi/commands-target-compat.h"
 
 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
 {
-    CpuDefinitionInfoList *cpu_list = NULL;
-    GSList *list;
-
-    list = object_class_get_list(TYPE_MIPS_CPU, false);
-    g_slist_foreach(list, mips_cpu_add_definition, &cpu_list);
-    g_slist_free(list);
-
-    return cpu_list;
+    return generic_query_cpu_definitions(errp);
 }
-- 
2.41.0

Re: [PATCH-for-9.1 07/21] target/mips: Make MIPS_CPU common to new MIPS32_CPU / MIPS64_CPU types

[Philippe Mathieu-Daudé]

On 15/3/24 14:08, Philippe Mathieu-Daudé wrote:
> "target/foo/cpu-qom.h" can not use any target specific definitions.
> 
> Currently "target/mips/cpu-qom.h" defines TYPE_MIPS_CPU depending
> on the mips(32)/mips64 build type. This doesn't scale in a
> heterogeneous context where we need to access both types concurrently.
> 
> In order to do that, introduce the new MIPS32_CPU / MIPS64_CPU types,
> both inheriting a common TYPE_MIPS_CPU base type.
> 
> Keep the current CPU types registered in mips_register_cpudef_type()
> as 32 or 64-bit, but instead of depending on the binary built being
> targeting 32/64-bit, check whether the CPU is 64-bit by looking at
> the CPU_MIPS64 bit.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/mips/cpu-qom.h              | 13 +++++++------
>   target/mips/cpu.c                  | 11 ++++++++++-
>   target/mips/sysemu/mips-qmp-cmds.c | 26 ++------------------------
>   3 files changed, 19 insertions(+), 31 deletions(-)
> 
> diff --git a/target/mips/cpu-qom.h b/target/mips/cpu-qom.h
> index 0eea2a2598..bf464f16b6 100644
> --- a/target/mips/cpu-qom.h
> +++ b/target/mips/cpu-qom.h
> @@ -1,5 +1,5 @@
>   /*
> - * QEMU MIPS CPU
> + * QEMU MIPS CPU QOM header (target agnostic)
>    *
>    * Copyright (c) 2012 SUSE LINUX Products GmbH
>    *
> @@ -22,14 +22,15 @@
>   
>   #include "hw/core/cpu.h"
>   
> -#ifdef TARGET_MIPS64
> -#define TYPE_MIPS_CPU "mips64-cpu"
> -#else
> -#define TYPE_MIPS_CPU "mips-cpu"
> -#endif
> +#define TYPE_MIPS_CPU   "mips-cpu"
> +#define TYPE_MIPS32_CPU "mips32-cpu"
> +#define TYPE_MIPS64_CPU "mips64-cpu"
>   
>   OBJECT_DECLARE_CPU_TYPE(MIPSCPU, MIPSCPUClass, MIPS_CPU)
>   
> +OBJECT_DECLARE_CPU_TYPE(MIPS32CPU, MIPSCPUClass, MIPS32_CPU)
> +OBJECT_DECLARE_CPU_TYPE(MIPS64CPU, MIPSCPUClass, MIPS64_CPU)
> +
>   #define MIPS_CPU_TYPE_SUFFIX "-" TYPE_MIPS_CPU
>   #define MIPS_CPU_TYPE_NAME(model) model MIPS_CPU_TYPE_SUFFIX
>   
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index c096d97fe3..f3ea6175f2 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -604,6 +604,14 @@ static const TypeInfo mips_cpu_types[] = {
>           .abstract       = true,
>           .class_size     = sizeof(MIPSCPUClass),
>           .class_init     = mips_cpu_class_init,
> +    }, {
> +        .name           = TYPE_MIPS32_CPU,
> +        .parent         = TYPE_MIPS_CPU,
> +        .abstract       = true,
> +    }, {
> +        .name           = TYPE_MIPS64_CPU,
> +        .parent         = TYPE_MIPS_CPU,
> +        .abstract       = true,
>       }
>   };
>   
> @@ -620,7 +628,8 @@ static void mips_register_cpudef_type(const struct mips_def_t *def)
>       char *typename = mips_cpu_type_name(def->name);
>       TypeInfo ti = {
>           .name = typename,
> -        .parent = TYPE_MIPS_CPU,
> +        .parent = def->insn_flags & CPU_MIPS64
> +                  ? TYPE_MIPS64_CPU : TYPE_MIPS32_CPU,
>           .class_init = mips_cpu_cpudef_class_init,
>           .class_data = (void *)def,
>       };

Oops, I got 2 commits squashed by mistake here...

> diff --git a/target/mips/sysemu/mips-qmp-cmds.c b/target/mips/sysemu/mips-qmp-cmds.c
> index 7340ac70ba..329db3a028 100644
> --- a/target/mips/sysemu/mips-qmp-cmds.c
> +++ b/target/mips/sysemu/mips-qmp-cmds.c
> @@ -8,31 +8,9 @@
>   
>   #include "qemu/osdep.h"
>   #include "qapi/qapi-commands-machine-target.h"
> -#include "cpu.h"
> -
> -static void mips_cpu_add_definition(gpointer data, gpointer user_data)
> -{
> -    ObjectClass *oc = data;
> -    CpuDefinitionInfoList **cpu_list = user_data;
> -    CpuDefinitionInfo *info;
> -    const char *typename;
> -
> -    typename = object_class_get_name(oc);
> -    info = g_malloc0(sizeof(*info));
> -    info->name = cpu_model_from_type(typename);
> -    info->q_typename = g_strdup(typename);
> -
> -    QAPI_LIST_PREPEND(*cpu_list, info);
> -}
> +#include "qapi/commands-target-compat.h"
>   
>   CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
>   {
> -    CpuDefinitionInfoList *cpu_list = NULL;
> -    GSList *list;
> -
> -    list = object_class_get_list(TYPE_MIPS_CPU, false);
> -    g_slist_foreach(list, mips_cpu_add_definition, &cpu_list);
> -    g_slist_free(list);
> -
> -    return cpu_list;
> +    return generic_query_cpu_definitions(errp);
>   }

[PATCH-for-9.1 08/21] target/sparc: Make SPARC_CPU common to new SPARC32_CPU/SPARC64_CPU types

[Philippe Mathieu-Daudé]

"target/foo/cpu-qom.h" can not use any target specific definitions.

Currently "target/sparc/cpu-qom.h" defines TYPE_SPARC_CPU
depending on the sparc(32)/sparc64 build type. This doesn't
scale in a heterogeneous context where we need to access both
types concurrently.

In order to do that, introduce the new SPARC32_CPU / SPARC64_CPU
types, both inheriting a common TYPE_SPARC_CPU base type.

Keep the current CPU types registered in sparc_register_cpudef_type()
as 32 or 64-bit, depending on the binary built.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 target/sparc/cpu-qom.h |  9 +++++----
 target/sparc/cpu.c     | 12 +++++++++++-
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/target/sparc/cpu-qom.h b/target/sparc/cpu-qom.h
index a86331bd58..6ad283506e 100644
--- a/target/sparc/cpu-qom.h
+++ b/target/sparc/cpu-qom.h
@@ -22,14 +22,15 @@
 
 #include "hw/core/cpu.h"
 
-#ifdef TARGET_SPARC64
-#define TYPE_SPARC_CPU "sparc64-cpu"
-#else
 #define TYPE_SPARC_CPU "sparc-cpu"
-#endif
+#define TYPE_SPARC32_CPU "sparc32-cpu"
+#define TYPE_SPARC64_CPU "sparc64-cpu"
 
 OBJECT_DECLARE_CPU_TYPE(SPARCCPU, SPARCCPUClass, SPARC_CPU)
 
+OBJECT_DECLARE_CPU_TYPE(SPARC32CPU, SPARCCPUClass, SPARC32_CPU)
+OBJECT_DECLARE_CPU_TYPE(SPARC64CPU, SPARCCPUClass, SPARC64_CPU)
+
 #define SPARC_CPU_TYPE_SUFFIX "-" TYPE_SPARC_CPU
 #define SPARC_CPU_TYPE_NAME(model) model SPARC_CPU_TYPE_SUFFIX
 
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 42b13ab63f..9e27e16b75 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -959,6 +959,12 @@ static const TypeInfo sparc_cpu_types[] = {
         .abstract       = true,
         .class_size     = sizeof(SPARCCPUClass),
         .class_init     = sparc_cpu_class_init,
+    }, {
+        .name           = TYPE_SPARC32_CPU,
+        .parent         = TYPE_SPARC_CPU,
+    }, {
+        .name           = TYPE_SPARC64_CPU,
+        .parent         = TYPE_SPARC_CPU,
     }
 };
 
@@ -975,7 +981,11 @@ static void sparc_register_cpudef_type(const struct sparc_def_t *def)
     char *typename = sparc_cpu_type_name(def->name);
     TypeInfo ti = {
         .name = typename,
-        .parent = TYPE_SPARC_CPU,
+#ifdef TARGET_SPARC64
+        .parent = TYPE_SPARC64_CPU,
+#else
+        .parent = TYPE_SPARC32_CPU,
+#endif
         .class_init = sparc_cpu_cpudef_class_init,
         .class_data = (void *)def,
     };
-- 
2.41.0

[PATCH-for-9.1 09/21] qapi: Merge machine-common.json with qapi/machine.json

[Philippe Mathieu-Daudé]

machine-common.json declares a single type, which isn't
restricted to a particular target. Move this type in
machine.json.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 MAINTAINERS              |  1 -
 qapi/machine-common.json | 21 ---------------------
 qapi/machine-target.json |  2 +-
 qapi/machine.json        | 13 ++++++++++++-
 qapi/qapi-schema.json    |  1 -
 target/s390x/cpu.h       |  2 +-
 qapi/meson.build         |  1 -
 7 files changed, 14 insertions(+), 27 deletions(-)
 delete mode 100644 qapi/machine-common.json

diff --git a/MAINTAINERS b/MAINTAINERS
index a3130f64fd..ed98814398 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1889,7 +1889,6 @@ F: hw/core/null-machine.c
 F: hw/core/numa.c
 F: hw/cpu/cluster.c
 F: qapi/machine.json
-F: qapi/machine-common.json
 F: qapi/machine-target.json
 F: include/hw/boards.h
 F: include/hw/core/cpu.h
diff --git a/qapi/machine-common.json b/qapi/machine-common.json
deleted file mode 100644
index fa6bd71d12..0000000000
--- a/qapi/machine-common.json
+++ /dev/null
@@ -1,21 +0,0 @@
-# -*- Mode: Python -*-
-# vim: filetype=python
-#
-# This work is licensed under the terms of the GNU GPL, version 2 or later.
-# See the COPYING file in the top-level directory.
-
-##
-# = Machines S390 data types
-##
-
-##
-# @CpuS390Entitlement:
-#
-# An enumeration of CPU entitlements that can be assumed by a virtual
-# S390 CPU
-#
-# Since: 8.2
-##
-{ 'enum': 'CpuS390Entitlement',
-  'prefix': 'S390_CPU_ENTITLEMENT',
-  'data': [ 'auto', 'low', 'medium', 'high' ] }
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 519adf3220..5f17b25d50 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -4,7 +4,7 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or later.
 # See the COPYING file in the top-level directory.
 
-{ 'include': 'machine-common.json' }
+{ 'include': 'machine.json' }
 
 ##
 # @CpuModelInfo:
diff --git a/qapi/machine.json b/qapi/machine.json
index bb5a178909..4bc38e86fd 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -9,7 +9,6 @@
 ##
 
 { 'include': 'common.json' }
-{ 'include': 'machine-common.json' }
 
 ##
 # @SysEmuTarget:
@@ -50,6 +49,18 @@
   'prefix': 'S390_CPU_STATE',
   'data': [ 'uninitialized', 'stopped', 'check-stop', 'operating', 'load' ] }
 
+##
+# @CpuS390Entitlement:
+#
+# An enumeration of CPU entitlements that can be assumed by a virtual
+# S390 CPU
+#
+# Since: 8.2
+##
+{ 'enum': 'CpuS390Entitlement',
+  'prefix': 'S390_CPU_ENTITLEMENT',
+  'data': [ 'auto', 'low', 'medium', 'high' ] }
+
 ##
 # @CpuInfoS390:
 #
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 8304d45625..2c82a49bae 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -67,7 +67,6 @@
 { 'include': 'introspect.json' }
 { 'include': 'qom.json' }
 { 'include': 'qdev.json' }
-{ 'include': 'machine-common.json' }
 { 'include': 'machine.json' }
 { 'include': 'machine-target.json' }
 { 'include': 'replay.json' }
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 43a46a5a06..b46339bd7c 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -29,7 +29,7 @@
 #include "cpu_models.h"
 #include "exec/cpu-defs.h"
 #include "qemu/cpu-float.h"
-#include "qapi/qapi-types-machine-common.h"
+#include "qapi/qapi-types-machine.h"
 
 #define ELF_MACHINE_UNAME "S390X"
 
diff --git a/qapi/meson.build b/qapi/meson.build
index 375d564277..90047dae1c 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -37,7 +37,6 @@ qapi_all_modules = [
   'error',
   'introspect',
   'job',
-  'machine-common',
   'machine',
   'machine-target',
   'migration',
-- 
2.41.0

Re: [PATCH-for-9.1 09/21] qapi: Merge machine-common.json with qapi/machine.json

[Markus Armbruster]

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

> machine-common.json declares a single type, which isn't
> restricted to a particular target. Move this type in
> machine.json.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Previous discussion at
https://lore.kernel.org/qemu-devel/875y45kt8i.fsf@pond.sub.org/

CpuS390Entitlement is specific to s390x.  We need it for
set-cpu-topology and the s390x-specific part of query-cpus-fast.  The
former is conditional on TARGET_S390X, and therefore must be in
machine-target.json.  The latter is not conditional, and in
machine.json.  If I remember correctly, I briefly explored making it
conditional, but it was too messy to be worth the bother.

Anyway.  We have a target-specific type we want to use both in
machine.json and machine-target.json.  Neither of the two includes the
other.  Target-independent machine.json cannot include target-specific
machine-target.json.  Two solutions:

1. Define the type in machine.json, have machine-target.json include
machine.json.  Ugly: even more target-stuff in machine.json.
Potentially slow: including qapi/qapi-*-machine-target.h now includes
more.

2. Define the type in a submodule both include.  Since nothing they
include is a fitting home for the type, create one: machine-common.json.
Ugly & potentially slow: yet another submodule.

Nina chose to do 2.

I figure I'd leave it as is until we get to the point where we can get
rid of the *-target.json entirely.

[PATCH-for-9.1 10/21] qapi: Make CpuModel* definitions target agnostic

[Philippe Mathieu-Daudé]

CpuModelInfo, CpuModelExpansionType and CpuModelCompareResult
are not restricted to any particular target. Define them in
machine.json to generate them once.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 qapi/machine-target.json | 78 ---------------------------------------
 qapi/machine.json        | 79 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 78 deletions(-)

diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 5f17b25d50..0412400df3 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -6,84 +6,6 @@
 
 { 'include': 'machine.json' }
 
-##
-# @CpuModelInfo:
-#
-# Virtual CPU model.
-#
-# A CPU model consists of the name of a CPU definition, to which delta
-# changes are applied (e.g. features added/removed). Most magic values
-# that an architecture might require should be hidden behind the name.
-# However, if required, architectures can expose relevant properties.
-#
-# @name: the name of the CPU definition the model is based on
-#
-# @props: a dictionary of QOM properties to be applied
-#
-# Since: 2.8
-##
-{ 'struct': 'CpuModelInfo',
-  'data': { 'name': 'str',
-            '*props': 'any' } }
-
-##
-# @CpuModelExpansionType:
-#
-# An enumeration of CPU model expansion types.
-#
-# @static: Expand to a static CPU model, a combination of a static
-#     base model name and property delta changes.  As the static base
-#     model will never change, the expanded CPU model will be the
-#     same, independent of QEMU version, machine type, machine
-#     options, and accelerator options.  Therefore, the resulting
-#     model can be used by tooling without having to specify a
-#     compatibility machine - e.g. when displaying the "host" model.
-#     The @static CPU models are migration-safe.
-#
-# @full: Expand all properties.  The produced model is not guaranteed
-#     to be migration-safe, but allows tooling to get an insight and
-#     work with model details.
-#
-# Note: When a non-migration-safe CPU model is expanded in static
-#     mode, some features enabled by the CPU model may be omitted,
-#     because they can't be implemented by a static CPU model
-#     definition (e.g. cache info passthrough and PMU passthrough in
-#     x86). If you need an accurate representation of the features
-#     enabled by a non-migration-safe CPU model, use @full.  If you
-#     need a static representation that will keep ABI compatibility
-#     even when changing QEMU version or machine-type, use @static
-#     (but keep in mind that some features may be omitted).
-#
-# Since: 2.8
-##
-{ 'enum': 'CpuModelExpansionType',
-  'data': [ 'static', 'full' ] }
-
-##
-# @CpuModelCompareResult:
-#
-# An enumeration of CPU model comparison results.  The result is
-# usually calculated using e.g. CPU features or CPU generations.
-#
-# @incompatible: If model A is incompatible to model B, model A is not
-#     guaranteed to run where model B runs and the other way around.
-#
-# @identical: If model A is identical to model B, model A is
-#     guaranteed to run where model B runs and the other way around.
-#
-# @superset: If model A is a superset of model B, model B is
-#     guaranteed to run where model A runs.  There are no guarantees
-#     about the other way.
-#
-# @subset: If model A is a subset of model B, model A is guaranteed to
-#     run where model B runs.  There are no guarantees about the other
-#     way.
-#
-# Since: 2.8
-##
-{ 'enum': 'CpuModelCompareResult',
-  'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }
-
 ##
 # @CpuModelBaselineInfo:
 #
diff --git a/qapi/machine.json b/qapi/machine.json
index 4bc38e86fd..65702c2c78 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -10,6 +10,85 @@
 
 { 'include': 'common.json' }
 
+##
+# @CpuModelInfo:
+#
+# Virtual CPU model.
+#
+# A CPU model consists of the name of a CPU definition, to which delta
+# changes are applied (e.g. features added/removed). Most magic values
+# that an architecture might require should be hidden behind the name.
+# However, if required, architectures can expose relevant properties.
+#
+# @name: the name of the CPU definition the model is based on
+#
+# @props: a dictionary of QOM properties to be applied
+#
+# Since: 2.8
+##
+{ 'struct': 'CpuModelInfo',
+  'data': { 'name': 'str',
+            '*props': 'any' }
+}
+
+##
+# @CpuModelExpansionType:
+#
+# An enumeration of CPU model expansion types.
+#
+# @static: Expand to a static CPU model, a combination of a static
+#     base model name and property delta changes.  As the static base
+#     model will never change, the expanded CPU model will be the
+#     same, independent of QEMU version, machine type, machine
+#     options, and accelerator options.  Therefore, the resulting
+#     model can be used by tooling without having to specify a
+#     compatibility machine - e.g. when displaying the "host" model.
+#     The @static CPU models are migration-safe.
+#
+# @full: Expand all properties.  The produced model is not guaranteed
+#     to be migration-safe, but allows tooling to get an insight and
+#     work with model details.
+#
+# Note: When a non-migration-safe CPU model is expanded in static
+#     mode, some features enabled by the CPU model may be omitted,
+#     because they can't be implemented by a static CPU model
+#     definition (e.g. cache info passthrough and PMU passthrough in
+#     x86). If you need an accurate representation of the features
+#     enabled by a non-migration-safe CPU model, use @full.  If you
+#     need a static representation that will keep ABI compatibility
+#     even when changing QEMU version or machine-type, use @static
+#     (but keep in mind that some features may be omitted).
+#
+# Since: 2.8
+##
+{ 'enum': 'CpuModelExpansionType',
+  'data': [ 'static', 'full' ] }
+
+##
+# @CpuModelCompareResult:
+#
+# An enumeration of CPU model comparison results.  The result is
+# usually calculated using e.g. CPU features or CPU generations.
+#
+# @incompatible: If model A is incompatible to model B, model A is not
+#     guaranteed to run where model B runs and the other way around.
+#
+# @identical: If model A is identical to model B, model A is
+#     guaranteed to run where model B runs and the other way around.
+#
+# @superset: If model A is a superset of model B, model B is
+#     guaranteed to run where model A runs.  There are no guarantees
+#     about the other way.
+#
+# @subset: If model A is a subset of model B, model A is guaranteed to
+#     run where model B runs.  There are no guarantees about the other
+#     way.
+#
+# Since: 2.8
+##
+{ 'enum': 'CpuModelCompareResult',
+  'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }
+
 ##
 # @SysEmuTarget:
 #
-- 
2.41.0

Re: [PATCH-for-9.1 10/21] qapi: Make CpuModel* definitions target agnostic

[Philippe Mathieu-Daudé]

On 15/3/24 14:08, Philippe Mathieu-Daudé wrote:
> CpuModelInfo, CpuModelExpansionType and CpuModelCompareResult
> are not restricted to any particular target. Define them in
> machine.json to generate them once.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   qapi/machine-target.json | 78 ---------------------------------------
>   qapi/machine.json        | 79 ++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 79 insertions(+), 78 deletions(-)
> 
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index 5f17b25d50..0412400df3 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -6,84 +6,6 @@
>   
>   { 'include': 'machine.json' }
>   
> -##
> -# @CpuModelInfo:
> -#
> -# Virtual CPU model.
> -#
> -# A CPU model consists of the name of a CPU definition, to which delta
> -# changes are applied (e.g. features added/removed). Most magic values
> -# that an architecture might require should be hidden behind the name.
> -# However, if required, architectures can expose relevant properties.
> -#
> -# @name: the name of the CPU definition the model is based on
> -#
> -# @props: a dictionary of QOM properties to be applied
> -#
> -# Since: 2.8
> -##
> -{ 'struct': 'CpuModelInfo',
> -  'data': { 'name': 'str',
> -            '*props': 'any' } }
> -
> -##
> -# @CpuModelExpansionType:
> -#
> -# An enumeration of CPU model expansion types.
> -#
> -# @static: Expand to a static CPU model, a combination of a static
> -#     base model name and property delta changes.  As the static base
> -#     model will never change, the expanded CPU model will be the
> -#     same, independent of QEMU version, machine type, machine
> -#     options, and accelerator options.  Therefore, the resulting
> -#     model can be used by tooling without having to specify a
> -#     compatibility machine - e.g. when displaying the "host" model.
> -#     The @static CPU models are migration-safe.
> -#
> -# @full: Expand all properties.  The produced model is not guaranteed
> -#     to be migration-safe, but allows tooling to get an insight and
> -#     work with model details.
> -#
> -# Note: When a non-migration-safe CPU model is expanded in static
> -#     mode, some features enabled by the CPU model may be omitted,
> -#     because they can't be implemented by a static CPU model
> -#     definition (e.g. cache info passthrough and PMU passthrough in
> -#     x86). If you need an accurate representation of the features
> -#     enabled by a non-migration-safe CPU model, use @full.  If you
> -#     need a static representation that will keep ABI compatibility
> -#     even when changing QEMU version or machine-type, use @static
> -#     (but keep in mind that some features may be omitted).
> -#
> -# Since: 2.8
> -##
> -{ 'enum': 'CpuModelExpansionType',
> -  'data': [ 'static', 'full' ] }
> -
> -##
> -# @CpuModelCompareResult:
> -#
> -# An enumeration of CPU model comparison results.  The result is
> -# usually calculated using e.g. CPU features or CPU generations.
> -#
> -# @incompatible: If model A is incompatible to model B, model A is not
> -#     guaranteed to run where model B runs and the other way around.
> -#
> -# @identical: If model A is identical to model B, model A is
> -#     guaranteed to run where model B runs and the other way around.
> -#
> -# @superset: If model A is a superset of model B, model B is
> -#     guaranteed to run where model A runs.  There are no guarantees
> -#     about the other way.
> -#
> -# @subset: If model A is a subset of model B, model A is guaranteed to
> -#     run where model B runs.  There are no guarantees about the other
> -#     way.
> -#
> -# Since: 2.8
> -##
> -{ 'enum': 'CpuModelCompareResult',
> -  'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }

Otherwise I'm getting when linking:

   duplicate symbol '_CpuModelCompareResult_lookup'
   duplicate symbol '_CpuModelExpansionType_lookup'

Re: [PATCH-for-9.1 10/21] qapi: Make CpuModel* definitions target agnostic

[Markus Armbruster]

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

> CpuModelInfo, CpuModelExpansionType and CpuModelCompareResult
> are not restricted to any particular target. Define them in
> machine.json to generate them once.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Well, the stuff in machine-target.json is generated once just fine.  But
that single copy is then compiled once per target.

On the one hand, less target-specific code is good.  On the other hand,
I'm reluctant to move these types away from their users.

[PATCH-for-9.1 11/21] qapi: Make CpuDefinitionInfo target agnostic

[Philippe Mathieu-Daudé]

Generate the CpuDefinitionInfo type once for all targets.
In few commits @query-cpu-definitions will become generic
and all target will return their CPUs list.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 qapi/machine-target.json | 69 ----------------------------------------
 qapi/machine.json        | 63 ++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 69 deletions(-)

diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 0412400df3..7480921d33 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -212,75 +212,6 @@
                    'TARGET_LOONGARCH64',
                    'TARGET_RISCV' ] } }
 
-##
-# @CpuDefinitionInfo:
-#
-# Virtual CPU definition.
-#
-# @name: the name of the CPU definition
-#
-# @migration-safe: whether a CPU definition can be safely used for
-#     migration in combination with a QEMU compatibility machine when
-#     migrating between different QEMU versions and between hosts with
-#     different sets of (hardware or software) capabilities.  If not
-#     provided, information is not available and callers should not
-#     assume the CPU definition to be migration-safe.  (since 2.8)
-#
-# @static: whether a CPU definition is static and will not change
-#     depending on QEMU version, machine type, machine options and
-#     accelerator options.  A static model is always migration-safe.
-#     (since 2.8)
-#
-# @unavailable-features: List of properties that prevent the CPU model
-#     from running in the current host.  (since 2.8)
-#
-# @typename: Type name that can be used as argument to
-#     @device-list-properties, to introspect properties configurable
-#     using -cpu or -global.  (since 2.9)
-#
-# @alias-of: Name of CPU model this model is an alias for.  The target
-#     of the CPU model alias may change depending on the machine type.
-#     Management software is supposed to translate CPU model aliases
-#     in the VM configuration, because aliases may stop being
-#     migration-safe in the future (since 4.1)
-#
-# @deprecated: If true, this CPU model is deprecated and may be
-#     removed in in some future version of QEMU according to the QEMU
-#     deprecation policy.  (since 5.2)
-#
-# @unavailable-features is a list of QOM property names that represent
-# CPU model attributes that prevent the CPU from running.  If the QOM
-# property is read-only, that means there's no known way to make the
-# CPU model run in the current host.  Implementations that choose not
-# to provide specific information return the property name "type". If
-# the property is read-write, it means that it MAY be possible to run
-# the CPU model in the current host if that property is changed.
-# Management software can use it as hints to suggest or choose an
-# alternative for the user, or just to generate meaningful error
-# messages explaining why the CPU model can't be used.  If
-# @unavailable-features is an empty list, the CPU model is runnable
-# using the current host and machine-type.  If @unavailable-features
-# is not present, runnability information for the CPU is not
-# available.
-#
-# Since: 1.2
-##
-{ 'struct': 'CpuDefinitionInfo',
-  'data': { 'name': 'str',
-            '*migration-safe': 'bool',
-            'static': 'bool',
-            '*unavailable-features': [ 'str' ],
-            'typename': 'str',
-            '*alias-of' : 'str',
-            'deprecated' : 'bool' },
-  'if': { 'any': [ 'TARGET_PPC',
-                   'TARGET_ARM',
-                   'TARGET_I386',
-                   'TARGET_S390X',
-                   'TARGET_MIPS',
-                   'TARGET_LOONGARCH64',
-                   'TARGET_RISCV' ] } }
-
 ##
 # @query-cpu-definitions:
 #
diff --git a/qapi/machine.json b/qapi/machine.json
index 65702c2c78..9c44b8fa82 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -10,6 +10,69 @@
 
 { 'include': 'common.json' }
 
+##
+# @CpuDefinitionInfo:
+#
+# Virtual CPU definition.
+#
+# @name: the name of the CPU definition
+#
+# @migration-safe: whether a CPU definition can be safely used for
+#     migration in combination with a QEMU compatibility machine when
+#     migrating between different QEMU versions and between hosts with
+#     different sets of (hardware or software) capabilities.  If not
+#     provided, information is not available and callers should not
+#     assume the CPU definition to be migration-safe.  (since 2.8)
+#
+# @static: whether a CPU definition is static and will not change
+#     depending on QEMU version, machine type, machine options and
+#     accelerator options.  A static model is always migration-safe.
+#     (since 2.8)
+#
+# @unavailable-features: List of properties that prevent the CPU model
+#     from running in the current host.  (since 2.8)
+#
+# @typename: Type name that can be used as argument to
+#     @device-list-properties, to introspect properties configurable
+#     using -cpu or -global.  (since 2.9)
+#
+# @alias-of: Name of CPU model this model is an alias for.  The target
+#     of the CPU model alias may change depending on the machine type.
+#     Management software is supposed to translate CPU model aliases
+#     in the VM configuration, because aliases may stop being
+#     migration-safe in the future (since 4.1)
+#
+# @deprecated: If true, this CPU model is deprecated and may be
+#     removed in in some future version of QEMU according to the QEMU
+#     deprecation policy.  (since 5.2)
+#
+# @unavailable-features is a list of QOM property names that represent
+# CPU model attributes that prevent the CPU from running.  If the QOM
+# property is read-only, that means there's no known way to make the
+# CPU model run in the current host.  Implementations that choose not
+# to provide specific information return the property name "type". If
+# the property is read-write, it means that it MAY be possible to run
+# the CPU model in the current host if that property is changed.
+# Management software can use it as hints to suggest or choose an
+# alternative for the user, or just to generate meaningful error
+# messages explaining why the CPU model can't be used.  If
+# @unavailable-features is an empty list, the CPU model is runnable
+# using the current host and machine-type.  If @unavailable-features
+# is not present, runnability information for the CPU is not
+# available.
+#
+# Since: 1.2
+##
+{ 'struct': 'CpuDefinitionInfo',
+  'data': { 'name': 'str',
+            '*migration-safe': 'bool',
+            'static': 'bool',
+            '*unavailable-features': [ 'str' ],
+            'typename': 'str',
+            '*alias-of' : 'str',
+            'deprecated' : 'bool' }
+}
+
 ##
 # @CpuModelInfo:
 #
-- 
2.41.0

[PATCH-for-9.1 12/21] system: Introduce QemuArchBit enum

[Philippe Mathieu-Daudé]

Current QEMU_ARCH_foo definitions are used as masks.
Extract the bit values, so we can easily iterate over.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/sysemu/arch_init.h | 69 ++++++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 22 deletions(-)

diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 8850cb1a14..cf597c40a3 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -1,30 +1,55 @@
 #ifndef QEMU_ARCH_INIT_H
 #define QEMU_ARCH_INIT_H
 
+typedef enum QemuArchBit {
+    QEMU_ARCH_BIT_ALPHA         = 0,
+    QEMU_ARCH_BIT_ARM           = 1,
+    QEMU_ARCH_BIT_CRIS          = 2,
+    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_NIOS2         = 17,
+    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,
 
-enum {
+    QEMU_ARCH_BIT_LAST          = QEMU_ARCH_BIT_LOONGARCH
+} QemuArchBit;
+
+enum QemuArchMask {
     QEMU_ARCH_ALL = -1,
-    QEMU_ARCH_ALPHA = (1 << 0),
-    QEMU_ARCH_ARM = (1 << 1),
-    QEMU_ARCH_CRIS = (1 << 2),
-    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_NIOS2 = (1 << 17),
-    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),
+    QEMU_ARCH_ALPHA             = (1 << QEMU_ARCH_BIT_ALPHA),
+    QEMU_ARCH_ARM               = (1 << QEMU_ARCH_BIT_ARM),
+    QEMU_ARCH_CRIS              = (1 << QEMU_ARCH_BIT_CRIS),
+    QEMU_ARCH_I386              = (1 << QEMU_ARCH_BIT_I386),
+    QEMU_ARCH_M68K              = (1 << QEMU_ARCH_BIT_M68K),
+    QEMU_ARCH_MICROBLAZE        = (1 << QEMU_ARCH_BIT_MICROBLAZE),
+    QEMU_ARCH_MIPS              = (1 << QEMU_ARCH_BIT_MIPS),
+    QEMU_ARCH_PPC               = (1 << QEMU_ARCH_BIT_PPC),
+    QEMU_ARCH_S390X             = (1 << QEMU_ARCH_BIT_S390X),
+    QEMU_ARCH_SH4               = (1 << QEMU_ARCH_BIT_SH4),
+    QEMU_ARCH_SPARC             = (1 << QEMU_ARCH_BIT_SPARC),
+    QEMU_ARCH_XTENSA            = (1 << QEMU_ARCH_BIT_XTENSA),
+    QEMU_ARCH_OPENRISC          = (1 << QEMU_ARCH_BIT_OPENRISC),
+    QEMU_ARCH_TRICORE           = (1 << QEMU_ARCH_BIT_TRICORE),
+    QEMU_ARCH_NIOS2             = (1 << QEMU_ARCH_BIT_NIOS2),
+    QEMU_ARCH_HPPA              = (1 << QEMU_ARCH_BIT_HPPA),
+    QEMU_ARCH_RISCV             = (1 << QEMU_ARCH_BIT_RISCV),
+    QEMU_ARCH_RX                = (1 << QEMU_ARCH_BIT_RX),
+    QEMU_ARCH_AVR               = (1 << QEMU_ARCH_BIT_AVR),
+    QEMU_ARCH_HEXAGON           = (1 << QEMU_ARCH_BIT_HEXAGON),
+    QEMU_ARCH_LOONGARCH         = (1 << QEMU_ARCH_BIT_LOONGARCH),
 };
 
 extern const uint32_t arch_type;
-- 
2.41.0

[PATCH-for-9.1 13/21] system: Introduce cpu_typename_by_arch_bit()

[Philippe Mathieu-Daudé]

Introduce a helper to return the CPU type name given a QemuArchBit.

The TYPE_PPC_CPU target have different 32/64-bit definitions
so we can not include it yet.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
---
 MAINTAINERS                |  1 +
 include/sysemu/arch_init.h |  2 ++
 system/cpu-qom-helpers.c   | 58 ++++++++++++++++++++++++++++++++++++++
 system/meson.build         |  1 +
 4 files changed, 62 insertions(+)
 create mode 100644 system/cpu-qom-helpers.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ed98814398..af27490243 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -148,6 +148,7 @@ M: Richard Henderson <richard.henderson@linaro.org>
 R: Paolo Bonzini <pbonzini@redhat.com>
 S: Maintained
 F: system/cpus.c
+F: system/cpu-qom-helpers.c
 F: system/watchpoint.c
 F: cpu-common.c
 F: cpu-target.c
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index cf597c40a3..1874f18e67 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -27,6 +27,8 @@ typedef enum QemuArchBit {
     QEMU_ARCH_BIT_LAST          = QEMU_ARCH_BIT_LOONGARCH
 } QemuArchBit;
 
+const char *cpu_typename_by_arch_bit(QemuArchBit arch_bit);
+
 enum QemuArchMask {
     QEMU_ARCH_ALL = -1,
     QEMU_ARCH_ALPHA             = (1 << QEMU_ARCH_BIT_ALPHA),
diff --git a/system/cpu-qom-helpers.c b/system/cpu-qom-helpers.c
new file mode 100644
index 0000000000..0d402ee3a0
--- /dev/null
+++ b/system/cpu-qom-helpers.c
@@ -0,0 +1,58 @@
+/*
+ * Helpers for CPU QOM types
+ *
+ * SPDX-FileCopyrightText: 2024 Linaro Ltd.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/arch_init.h"
+
+#include "target/alpha/cpu-qom.h"
+#include "target/arm/cpu-qom.h"
+#include "target/avr/cpu-qom.h"
+#include "target/cris/cpu-qom.h"
+#include "target/hexagon/cpu-qom.h"
+#include "target/hppa/cpu-qom.h"
+#include "target/i386/cpu-qom.h"
+#include "target/loongarch/cpu-qom.h"
+#include "target/m68k/cpu-qom.h"
+#include "target/microblaze/cpu-qom.h"
+#include "target/mips/cpu-qom.h"
+#include "target/nios2/cpu-qom.h"
+#include "target/openrisc/cpu-qom.h"
+#include "target/riscv/cpu-qom.h"
+#include "target/rx/cpu-qom.h"
+#include "target/s390x/cpu-qom.h"
+#include "target/sparc/cpu-qom.h"
+#include "target/sh4/cpu-qom.h"
+#include "target/tricore/cpu-qom.h"
+#include "target/xtensa/cpu-qom.h"
+
+const char *cpu_typename_by_arch_bit(QemuArchBit arch_bit)
+{
+    static const char *cpu_bit_to_typename[QEMU_ARCH_BIT_LAST + 1] = {
+        [QEMU_ARCH_BIT_ALPHA]       = TYPE_ALPHA_CPU,
+        [QEMU_ARCH_BIT_ARM]         = TYPE_ARM_CPU,
+        [QEMU_ARCH_BIT_CRIS]        = TYPE_CRIS_CPU,
+        [QEMU_ARCH_BIT_I386]        = TYPE_I386_CPU,
+        [QEMU_ARCH_BIT_M68K]        = TYPE_M68K_CPU,
+        [QEMU_ARCH_BIT_MICROBLAZE]  = TYPE_MICROBLAZE_CPU,
+        [QEMU_ARCH_BIT_MIPS]        = TYPE_MIPS_CPU,
+        /* TODO:                      TYPE_PPC_CPU */
+        [QEMU_ARCH_BIT_S390X]       = TYPE_S390_CPU,
+        [QEMU_ARCH_BIT_SH4]         = TYPE_SUPERH_CPU,
+        [QEMU_ARCH_BIT_SPARC]       = TYPE_SPARC_CPU,
+        [QEMU_ARCH_BIT_XTENSA]      = TYPE_XTENSA_CPU,
+        [QEMU_ARCH_BIT_OPENRISC]    = TYPE_OPENRISC_CPU,
+        [QEMU_ARCH_BIT_TRICORE]     = TYPE_TRICORE_CPU,
+        [QEMU_ARCH_BIT_NIOS2]       = TYPE_NIOS2_CPU,
+        [QEMU_ARCH_BIT_HPPA]        = TYPE_HPPA_CPU,
+        [QEMU_ARCH_BIT_RISCV]       = TYPE_RISCV_CPU,
+        [QEMU_ARCH_BIT_RX]          = TYPE_RX_CPU,
+        [QEMU_ARCH_BIT_AVR]         = TYPE_AVR_CPU,
+        [QEMU_ARCH_BIT_HEXAGON]     = TYPE_HEXAGON_CPU,
+        [QEMU_ARCH_BIT_LOONGARCH]   = TYPE_LOONGARCH_CPU,
+    };
+    return cpu_bit_to_typename[arch_bit];
+}
diff --git a/system/meson.build b/system/meson.build
index 25e2117250..c6ee97e3b2 100644
--- a/system/meson.build
+++ b/system/meson.build
@@ -10,6 +10,7 @@ system_ss.add(files(
   'balloon.c',
   'bootdevice.c',
   'cpus.c',
+  'cpu-qom-helpers.c',
   'cpu-throttle.c',
   'cpu-timers.c',
   'datadir.c',
-- 
2.41.0

[RFC PATCH-for-9.1 14/21] system: Introduce QMP generic_query_cpu_definitions()

[Philippe Mathieu-Daudé]

Each target use a common template for qmp_query_cpu_definitions().

Extract it as generic_query_cpu_definitions(), keeping the
target-specific implementations as the following SysemuCPUOps
handlers:
 - cpu_list_compare()
 - add_definition()
 - add_alias_definitions()

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 MAINTAINERS                           |  2 +
 include/hw/core/sysemu-cpu-ops.h      | 14 ++++++
 include/qapi/commands-target-compat.h | 14 ++++++
 system/cpu-qmp-cmds.c                 | 71 +++++++++++++++++++++++++++
 system/meson.build                    |  1 +
 5 files changed, 102 insertions(+)
 create mode 100644 include/qapi/commands-target-compat.h
 create mode 100644 system/cpu-qmp-cmds.c

diff --git a/MAINTAINERS b/MAINTAINERS
index af27490243..39d7c14d98 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -148,6 +148,7 @@ M: Richard Henderson <richard.henderson@linaro.org>
 R: Paolo Bonzini <pbonzini@redhat.com>
 S: Maintained
 F: system/cpus.c
+F: system/cpu-qmp-cmds.c
 F: system/cpu-qom-helpers.c
 F: system/watchpoint.c
 F: cpu-common.c
@@ -1894,6 +1895,7 @@ F: qapi/machine-target.json
 F: include/hw/boards.h
 F: include/hw/core/cpu.h
 F: include/hw/cpu/cluster.h
+F: include/qapi/commands-target-compat.h
 F: include/sysemu/numa.h
 F: tests/unit/test-smp-parse.c
 T: git https://gitlab.com/ehabkost/qemu.git machine-next
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index 24d003fe04..2173226e97 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -11,6 +11,7 @@
 #define SYSEMU_CPU_OPS_H
 
 #include "hw/core/cpu.h"
+#include "qapi/qapi-types-machine.h"
 
 /*
  * struct SysemuCPUOps: System operations specific to a CPU class
@@ -81,6 +82,19 @@ typedef struct SysemuCPUOps {
      */
     bool (*virtio_is_big_endian)(CPUState *cpu);
 
+    /**
+     * @cpu_list_compare: Sort alphabetically by type name,
+     *                    respecting CPUClass::ordering.
+     */
+    gint (*cpu_list_compare)(gconstpointer cpu_class_a, gconstpointer cpu_class_b);
+    /**
+     * @add_definition: Add the @cpu_class definition to @cpu_list.
+     */
+    void (*add_definition)(gpointer cpu_class, gpointer cpu_list);
+    /**
+     * @add_alias_definitions: Add CPU alias definitions to @cpu_list.
+     */
+    void (*add_alias_definitions)(CpuDefinitionInfoList **cpu_list);
     /**
      * @legacy_vmsd: Legacy state for migration.
      *               Do not use in new targets, use #DeviceClass::vmsd instead.
diff --git a/include/qapi/commands-target-compat.h b/include/qapi/commands-target-compat.h
new file mode 100644
index 0000000000..86d45d8fcc
--- /dev/null
+++ b/include/qapi/commands-target-compat.h
@@ -0,0 +1,14 @@
+/*
+ * QAPI helpers for target specific QMP commands
+ *
+ * SPDX-FileCopyrightText: 2024 Linaro Ltd.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef QAPI_COMPAT_TARGET_H
+#define QAPI_COMPAT_TARGET_H
+
+#include "qapi/qapi-types-machine.h"
+
+CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp);
+
+#endif
diff --git a/system/cpu-qmp-cmds.c b/system/cpu-qmp-cmds.c
new file mode 100644
index 0000000000..daeb131159
--- /dev/null
+++ b/system/cpu-qmp-cmds.c
@@ -0,0 +1,71 @@
+/*
+ * QAPI helpers for target specific QMP commands
+ *
+ * SPDX-FileCopyrightText: 2024 Linaro Ltd.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qom/object.h"
+#include "qapi/commands-target-compat.h"
+#include "sysemu/arch_init.h"
+#include "hw/core/cpu.h"
+#include "hw/core/sysemu-cpu-ops.h"
+
+static void cpu_common_add_definition(gpointer data, gpointer user_data)
+{
+    ObjectClass *oc = data;
+    CpuDefinitionInfoList **cpu_list = user_data;
+    CpuDefinitionInfo *info;
+    const char *typename;
+
+    typename = object_class_get_name(oc);
+    info = g_malloc0(sizeof(*info));
+    info->name = cpu_model_from_type(typename);
+    info->q_typename = g_strdup(typename);
+
+    QAPI_LIST_PREPEND(*cpu_list, info);
+}
+
+static void arch_add_cpu_definitions(CpuDefinitionInfoList **cpu_list,
+                                     const char *cpu_typename)
+{
+    ObjectClass *oc;
+    GSList *list;
+    const struct SysemuCPUOps *ops;
+
+    oc = object_class_by_name(cpu_typename);
+    if (!oc) {
+        return;
+    }
+    ops = CPU_CLASS(oc)->sysemu_ops;
+
+    list = object_class_get_list(cpu_typename, false);
+    if (ops->cpu_list_compare) {
+        list = g_slist_sort(list, ops->cpu_list_compare);
+    }
+    g_slist_foreach(list, ops->add_definition ? : cpu_common_add_definition,
+                    cpu_list);
+    g_slist_free(list);
+
+    if (ops->add_alias_definitions) {
+        ops->add_alias_definitions(cpu_list);
+    }
+}
+
+CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp)
+{
+    CpuDefinitionInfoList *cpu_list = NULL;
+
+    for (unsigned i = 0; i <= QEMU_ARCH_BIT_LAST; i++) {
+        const char *cpu_typename;
+
+        cpu_typename = cpu_typename_by_arch_bit(i);
+        if (!cpu_typename) {
+            continue;
+        }
+        arch_add_cpu_definitions(&cpu_list, cpu_typename);
+    }
+
+    return cpu_list;
+}
diff --git a/system/meson.build b/system/meson.build
index c6ee97e3b2..dd78caa9b7 100644
--- a/system/meson.build
+++ b/system/meson.build
@@ -10,6 +10,7 @@ system_ss.add(files(
   'balloon.c',
   'bootdevice.c',
   'cpus.c',
+  'cpu-qmp-cmds.c',
   'cpu-qom-helpers.c',
   'cpu-throttle.c',
   'cpu-timers.c',
-- 
2.41.0

Re: [RFC PATCH-for-9.1 14/21] system: Introduce QMP generic_query_cpu_definitions()

[Markus Armbruster]

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

> Each target use a common template for qmp_query_cpu_definitions().
>
> Extract it as generic_query_cpu_definitions(), keeping the
> target-specific implementations as the following SysemuCPUOps
> handlers:
>  - cpu_list_compare()
>  - add_definition()
>  - add_alias_definitions()
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  MAINTAINERS                           |  2 +
>  include/hw/core/sysemu-cpu-ops.h      | 14 ++++++
>  include/qapi/commands-target-compat.h | 14 ++++++
>  system/cpu-qmp-cmds.c                 | 71 +++++++++++++++++++++++++++
>  system/meson.build                    |  1 +
>  5 files changed, 102 insertions(+)
>  create mode 100644 include/qapi/commands-target-compat.h
>  create mode 100644 system/cpu-qmp-cmds.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index af27490243..39d7c14d98 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -148,6 +148,7 @@ M: Richard Henderson <richard.henderson@linaro.org>
>  R: Paolo Bonzini <pbonzini@redhat.com>
>  S: Maintained
>  F: system/cpus.c
> +F: system/cpu-qmp-cmds.c
>  F: system/cpu-qom-helpers.c
>  F: system/watchpoint.c
>  F: cpu-common.c
> @@ -1894,6 +1895,7 @@ F: qapi/machine-target.json
>  F: include/hw/boards.h
>  F: include/hw/core/cpu.h
>  F: include/hw/cpu/cluster.h
> +F: include/qapi/commands-target-compat.h
>  F: include/sysemu/numa.h
>  F: tests/unit/test-smp-parse.c
>  T: git https://gitlab.com/ehabkost/qemu.git machine-next
> diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
> index 24d003fe04..2173226e97 100644
> --- a/include/hw/core/sysemu-cpu-ops.h
> +++ b/include/hw/core/sysemu-cpu-ops.h
> @@ -11,6 +11,7 @@
>  #define SYSEMU_CPU_OPS_H
>  
>  #include "hw/core/cpu.h"
> +#include "qapi/qapi-types-machine.h"
>  
>  /*
>   * struct SysemuCPUOps: System operations specific to a CPU class
> @@ -81,6 +82,19 @@ typedef struct SysemuCPUOps {
>       */
>      bool (*virtio_is_big_endian)(CPUState *cpu);
>  
> +    /**
> +     * @cpu_list_compare: Sort alphabetically by type name,
> +     *                    respecting CPUClass::ordering.
> +     */
> +    gint (*cpu_list_compare)(gconstpointer cpu_class_a, gconstpointer cpu_class_b);

Peeking ahead...  This is used for sorting the subtypes of the CPU type
returned by cpu_typename_by_arch_bit().  Implementing the callback is
optional, and when absent, we don't sort, i.e. we get hash table order.

Worth mentioning it's optional?

> +    /**
> +     * @add_definition: Add the @cpu_class definition to @cpu_list.
> +     */
> +    void (*add_definition)(gpointer cpu_class, gpointer cpu_list);

This one appears to default to cpu_common_add_definition().  Worth
mentioning?

I despise Glib's pointless typedefs for ordinary C types.

> +    /**
> +     * @add_alias_definitions: Add CPU alias definitions to @cpu_list.
> +     */
> +    void (*add_alias_definitions)(CpuDefinitionInfoList **cpu_list);
>      /**
>       * @legacy_vmsd: Legacy state for migration.
>       *               Do not use in new targets, use #DeviceClass::vmsd instead.
> diff --git a/include/qapi/commands-target-compat.h b/include/qapi/commands-target-compat.h
> new file mode 100644
> index 0000000000..86d45d8fcc
> --- /dev/null
> +++ b/include/qapi/commands-target-compat.h

Why "compat"?

> @@ -0,0 +1,14 @@
> +/*
> + * QAPI helpers for target specific QMP commands
> + *
> + * SPDX-FileCopyrightText: 2024 Linaro Ltd.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef QAPI_COMPAT_TARGET_H
> +#define QAPI_COMPAT_TARGET_H
> +
> +#include "qapi/qapi-types-machine.h"
> +
> +CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp);
> +
> +#endif
> diff --git a/system/cpu-qmp-cmds.c b/system/cpu-qmp-cmds.c
> new file mode 100644
> index 0000000000..daeb131159
> --- /dev/null
> +++ b/system/cpu-qmp-cmds.c
> @@ -0,0 +1,71 @@
> +/*
> + * QAPI helpers for target specific QMP commands
> + *
> + * SPDX-FileCopyrightText: 2024 Linaro Ltd.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qom/object.h"
> +#include "qapi/commands-target-compat.h"
> +#include "sysemu/arch_init.h"
> +#include "hw/core/cpu.h"
> +#include "hw/core/sysemu-cpu-ops.h"
> +
> +static void cpu_common_add_definition(gpointer data, gpointer user_data)
> +{
> +    ObjectClass *oc = data;
> +    CpuDefinitionInfoList **cpu_list = user_data;
> +    CpuDefinitionInfo *info;
> +    const char *typename;
> +
> +    typename = object_class_get_name(oc);
> +    info = g_malloc0(sizeof(*info));
> +    info->name = cpu_model_from_type(typename);
> +    info->q_typename = g_strdup(typename);
> +
> +    QAPI_LIST_PREPEND(*cpu_list, info);
> +}
> +
> +static void arch_add_cpu_definitions(CpuDefinitionInfoList **cpu_list,
> +                                     const char *cpu_typename)
> +{
> +    ObjectClass *oc;
> +    GSList *list;
> +    const struct SysemuCPUOps *ops;
> +
> +    oc = object_class_by_name(cpu_typename);
> +    if (!oc) {
> +        return;
> +    }
> +    ops = CPU_CLASS(oc)->sysemu_ops;
> +
> +    list = object_class_get_list(cpu_typename, false);
> +    if (ops->cpu_list_compare) {
> +        list = g_slist_sort(list, ops->cpu_list_compare);
> +    }
> +    g_slist_foreach(list, ops->add_definition ? : cpu_common_add_definition,
> +                    cpu_list);
> +    g_slist_free(list);
> +
> +    if (ops->add_alias_definitions) {
> +        ops->add_alias_definitions(cpu_list);
> +    }
> +}
> +
> +CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp)
> +{
> +    CpuDefinitionInfoList *cpu_list = NULL;
> +
> +    for (unsigned i = 0; i <= QEMU_ARCH_BIT_LAST; i++) {
> +        const char *cpu_typename;
> +
> +        cpu_typename = cpu_typename_by_arch_bit(i);
> +        if (!cpu_typename) {
> +            continue;
> +        }
> +        arch_add_cpu_definitions(&cpu_list, cpu_typename);
> +    }
> +
> +    return cpu_list;
> +}
> diff --git a/system/meson.build b/system/meson.build
> index c6ee97e3b2..dd78caa9b7 100644
> --- a/system/meson.build
> +++ b/system/meson.build
> @@ -10,6 +10,7 @@ system_ss.add(files(
>    'balloon.c',
>    'bootdevice.c',
>    'cpus.c',
> +  'cpu-qmp-cmds.c',
>    'cpu-qom-helpers.c',
>    'cpu-throttle.c',
>    'cpu-timers.c',

The commit message made me expect the complete refactoring in this
patch.  In fact, it's just a first step: the new
generic_query_cpu_definitions() remains unused, and no target implements
the new callbacks.  We can worry about this later.

Subsequent patches convert targets to generic_query_cpu_definitions()
one by one.  To convince myself the replacement is faithful, I'll have
to refer back to this patch.  That's fine.

Re: [RFC PATCH-for-9.1 14/21] system: Introduce QMP generic_query_cpu_definitions()

[Markus Armbruster]

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

> Each target use a common template for qmp_query_cpu_definitions().
>
> Extract it as generic_query_cpu_definitions(), keeping the
> target-specific implementations as the following SysemuCPUOps
> handlers:
>  - cpu_list_compare()
>  - add_definition()
>  - add_alias_definitions()
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  MAINTAINERS                           |  2 +
>  include/hw/core/sysemu-cpu-ops.h      | 14 ++++++
>  include/qapi/commands-target-compat.h | 14 ++++++
>  system/cpu-qmp-cmds.c                 | 71 +++++++++++++++++++++++++++
>  system/meson.build                    |  1 +
>  5 files changed, 102 insertions(+)
>  create mode 100644 include/qapi/commands-target-compat.h
>  create mode 100644 system/cpu-qmp-cmds.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index af27490243..39d7c14d98 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -148,6 +148,7 @@ M: Richard Henderson <richard.henderson@linaro.org>
>  R: Paolo Bonzini <pbonzini@redhat.com>
>  S: Maintained
>  F: system/cpus.c
> +F: system/cpu-qmp-cmds.c
>  F: system/cpu-qom-helpers.c
>  F: system/watchpoint.c
>  F: cpu-common.c
> @@ -1894,6 +1895,7 @@ F: qapi/machine-target.json
>  F: include/hw/boards.h
>  F: include/hw/core/cpu.h
>  F: include/hw/cpu/cluster.h
> +F: include/qapi/commands-target-compat.h
>  F: include/sysemu/numa.h
>  F: tests/unit/test-smp-parse.c
>  T: git https://gitlab.com/ehabkost/qemu.git machine-next
> diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
> index 24d003fe04..2173226e97 100644
> --- a/include/hw/core/sysemu-cpu-ops.h
> +++ b/include/hw/core/sysemu-cpu-ops.h
> @@ -11,6 +11,7 @@
>  #define SYSEMU_CPU_OPS_H
>  
>  #include "hw/core/cpu.h"
> +#include "qapi/qapi-types-machine.h"
>  
>  /*
>   * struct SysemuCPUOps: System operations specific to a CPU class
> @@ -81,6 +82,19 @@ typedef struct SysemuCPUOps {
>       */
>      bool (*virtio_is_big_endian)(CPUState *cpu);
>  
> +    /**
> +     * @cpu_list_compare: Sort alphabetically by type name,
> +     *                    respecting CPUClass::ordering.
> +     */
> +    gint (*cpu_list_compare)(gconstpointer cpu_class_a, gconstpointer cpu_class_b);
> +    /**
> +     * @add_definition: Add the @cpu_class definition to @cpu_list.
> +     */
> +    void (*add_definition)(gpointer cpu_class, gpointer cpu_list);
> +    /**
> +     * @add_alias_definitions: Add CPU alias definitions to @cpu_list.
> +     */
> +    void (*add_alias_definitions)(CpuDefinitionInfoList **cpu_list);
>      /**
>       * @legacy_vmsd: Legacy state for migration.
>       *               Do not use in new targets, use #DeviceClass::vmsd instead.
> diff --git a/include/qapi/commands-target-compat.h b/include/qapi/commands-target-compat.h
> new file mode 100644
> index 0000000000..86d45d8fcc
> --- /dev/null
> +++ b/include/qapi/commands-target-compat.h
> @@ -0,0 +1,14 @@
> +/*
> + * QAPI helpers for target specific QMP commands
> + *
> + * SPDX-FileCopyrightText: 2024 Linaro Ltd.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef QAPI_COMPAT_TARGET_H
> +#define QAPI_COMPAT_TARGET_H
> +
> +#include "qapi/qapi-types-machine.h"
> +
> +CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp);
> +
> +#endif
> diff --git a/system/cpu-qmp-cmds.c b/system/cpu-qmp-cmds.c
> new file mode 100644
> index 0000000000..daeb131159
> --- /dev/null
> +++ b/system/cpu-qmp-cmds.c
> @@ -0,0 +1,71 @@
> +/*
> + * QAPI helpers for target specific QMP commands
> + *
> + * SPDX-FileCopyrightText: 2024 Linaro Ltd.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qom/object.h"
> +#include "qapi/commands-target-compat.h"
> +#include "sysemu/arch_init.h"
> +#include "hw/core/cpu.h"
> +#include "hw/core/sysemu-cpu-ops.h"
> +
> +static void cpu_common_add_definition(gpointer data, gpointer user_data)
> +{
> +    ObjectClass *oc = data;
> +    CpuDefinitionInfoList **cpu_list = user_data;
> +    CpuDefinitionInfo *info;
> +    const char *typename;
> +
> +    typename = object_class_get_name(oc);
> +    info = g_malloc0(sizeof(*info));
> +    info->name = cpu_model_from_type(typename);
> +    info->q_typename = g_strdup(typename);
> +
> +    QAPI_LIST_PREPEND(*cpu_list, info);
> +}
> +
> +static void arch_add_cpu_definitions(CpuDefinitionInfoList **cpu_list,
> +                                     const char *cpu_typename)
> +{
> +    ObjectClass *oc;
> +    GSList *list;
> +    const struct SysemuCPUOps *ops;
> +
> +    oc = object_class_by_name(cpu_typename);
> +    if (!oc) {
> +        return;
> +    }
> +    ops = CPU_CLASS(oc)->sysemu_ops;
> +
> +    list = object_class_get_list(cpu_typename, false);
> +    if (ops->cpu_list_compare) {
> +        list = g_slist_sort(list, ops->cpu_list_compare);
> +    }
> +    g_slist_foreach(list, ops->add_definition ? : cpu_common_add_definition,
> +                    cpu_list);
> +    g_slist_free(list);
> +
> +    if (ops->add_alias_definitions) {
> +        ops->add_alias_definitions(cpu_list);
> +    }
> +}
> +
> +CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp)
> +{
> +    CpuDefinitionInfoList *cpu_list = NULL;
> +
> +    for (unsigned i = 0; i <= QEMU_ARCH_BIT_LAST; i++) {
> +        const char *cpu_typename;
> +
> +        cpu_typename = cpu_typename_by_arch_bit(i);
> +        if (!cpu_typename) {
> +            continue;
> +        }
> +        arch_add_cpu_definitions(&cpu_list, cpu_typename);
> +    }
> +
> +    return cpu_list;
> +}

The target-specific qmp_query_cpu_definitions() this is going to replace
each execute the equivalent of *one* loop iteration: the one
corresponding to their own arch bit.

For the replacement to be faithful, as cpu_typename_by_arch_bit() must
return non-null exactly once.

This is the case for the qemu-system-TARGET.  The solution feels
overengineered there.

I figure cpu_typename_by_arch_bit() will return non-null multiple times
in a future single binary supporting heterogeneous machines.

Such a single binary then can't serve as drop-in replacement for the
qemu-system-TARGET, because query-cpu-definitions returns more.

To get a drop-in replacement, we'll need additional logic to restrict
the query for the homogeneous use case.

I think this needs to be discussed in the commit message.

Possibly easier: don't loop over the bits, relying on
cpu_typename_by_arch_bit() to select the right one.  Instead get the
right bit from somewhere.

We can switch to a loop when we need it for the heterogeneous case.

> diff --git a/system/meson.build b/system/meson.build
> index c6ee97e3b2..dd78caa9b7 100644
> --- a/system/meson.build
> +++ b/system/meson.build
> @@ -10,6 +10,7 @@ system_ss.add(files(
>    'balloon.c',
>    'bootdevice.c',
>    'cpus.c',
> +  'cpu-qmp-cmds.c',
>    'cpu-qom-helpers.c',
>    'cpu-throttle.c',
>    'cpu-timers.c',

Re: [RFC PATCH-for-9.1 14/21] system: Introduce QMP generic_query_cpu_definitions()

[Philippe Mathieu-Daudé]

Hi Markus,

On 26/3/24 14:28, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> Each target use a common template for qmp_query_cpu_definitions().
>>
>> Extract it as generic_query_cpu_definitions(), keeping the
>> target-specific implementations as the following SysemuCPUOps
>> handlers:
>>   - cpu_list_compare()
>>   - add_definition()
>>   - add_alias_definitions()
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   MAINTAINERS                           |  2 +
>>   include/hw/core/sysemu-cpu-ops.h      | 14 ++++++
>>   include/qapi/commands-target-compat.h | 14 ++++++
>>   system/cpu-qmp-cmds.c                 | 71 +++++++++++++++++++++++++++
>>   system/meson.build                    |  1 +
>>   5 files changed, 102 insertions(+)
>>   create mode 100644 include/qapi/commands-target-compat.h
>>   create mode 100644 system/cpu-qmp-cmds.c


>> diff --git a/system/cpu-qmp-cmds.c b/system/cpu-qmp-cmds.c
>> new file mode 100644
>> index 0000000000..daeb131159
>> --- /dev/null
>> +++ b/system/cpu-qmp-cmds.c
>> @@ -0,0 +1,71 @@
>> +/*
>> + * QAPI helpers for target specific QMP commands
>> + *
>> + * SPDX-FileCopyrightText: 2024 Linaro Ltd.
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qom/object.h"
>> +#include "qapi/commands-target-compat.h"
>> +#include "sysemu/arch_init.h"
>> +#include "hw/core/cpu.h"
>> +#include "hw/core/sysemu-cpu-ops.h"
>> +
>> +static void cpu_common_add_definition(gpointer data, gpointer user_data)
>> +{
>> +    ObjectClass *oc = data;
>> +    CpuDefinitionInfoList **cpu_list = user_data;
>> +    CpuDefinitionInfo *info;
>> +    const char *typename;
>> +
>> +    typename = object_class_get_name(oc);
>> +    info = g_malloc0(sizeof(*info));
>> +    info->name = cpu_model_from_type(typename);
>> +    info->q_typename = g_strdup(typename);
>> +
>> +    QAPI_LIST_PREPEND(*cpu_list, info);
>> +}
>> +
>> +static void arch_add_cpu_definitions(CpuDefinitionInfoList **cpu_list,
>> +                                     const char *cpu_typename)
>> +{
>> +    ObjectClass *oc;
>> +    GSList *list;
>> +    const struct SysemuCPUOps *ops;
>> +
>> +    oc = object_class_by_name(cpu_typename);
>> +    if (!oc) {
>> +        return;
>> +    }
>> +    ops = CPU_CLASS(oc)->sysemu_ops;
>> +
>> +    list = object_class_get_list(cpu_typename, false);
>> +    if (ops->cpu_list_compare) {
>> +        list = g_slist_sort(list, ops->cpu_list_compare);
>> +    }
>> +    g_slist_foreach(list, ops->add_definition ? : cpu_common_add_definition,
>> +                    cpu_list);
>> +    g_slist_free(list);
>> +
>> +    if (ops->add_alias_definitions) {
>> +        ops->add_alias_definitions(cpu_list);
>> +    }
>> +}
>> +
>> +CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp)
>> +{
>> +    CpuDefinitionInfoList *cpu_list = NULL;
>> +
>> +    for (unsigned i = 0; i <= QEMU_ARCH_BIT_LAST; i++) {
>> +        const char *cpu_typename;
>> +
>> +        cpu_typename = cpu_typename_by_arch_bit(i);
>> +        if (!cpu_typename) {
>> +            continue;
>> +        }
>> +        arch_add_cpu_definitions(&cpu_list, cpu_typename);
>> +    }
>> +
>> +    return cpu_list;
>> +}
> 
> The target-specific qmp_query_cpu_definitions() this is going to replace
> each execute the equivalent of *one* loop iteration: the one
> corresponding to their own arch bit.
> 
> For the replacement to be faithful, as cpu_typename_by_arch_bit() must
> return non-null exactly once.
> 
> This is the case for the qemu-system-TARGET.  The solution feels
> overengineered there.
> 
> I figure cpu_typename_by_arch_bit() will return non-null multiple times
> in a future single binary supporting heterogeneous machines.
> 
> Such a single binary then can't serve as drop-in replacement for the
> qemu-system-TARGET, because query-cpu-definitions returns more.
> 
> To get a drop-in replacement, we'll need additional logic to restrict
> the query for the homogeneous use case.

Can we ask the management layer to provide the current homogeneous
target via argument? Otherwise we can add a new query-cpu-definitions-v2
command requiring an explicit target argument, allowing 'all', and
deprecate the current query-cpu-definitions.

> I think this needs to be discussed in the commit message.
> 
> Possibly easier: don't loop over the bits, relying on
> cpu_typename_by_arch_bit() to select the right one.  Instead get the
> right bit from somewhere.
> 
> We can switch to a loop when we need it for the heterogeneous case.

Alex suggested to consider heterogeneous emulation the new default,
and the current homogeneous use as a particular case. I'd rather not
plan on a "heterogeneous switch day" and get things integrated in
the way, otherwise we'll never get there...

Regards,

Phil.

Re: [RFC PATCH-for-9.1 14/21] system: Introduce QMP generic_query_cpu_definitions()

[Markus Armbruster]

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

> Hi Markus,
>
> On 26/3/24 14:28, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>> 
>>> Each target use a common template for qmp_query_cpu_definitions().
>>>
>>> Extract it as generic_query_cpu_definitions(), keeping the
>>> target-specific implementations as the following SysemuCPUOps
>>> handlers:
>>>   - cpu_list_compare()
>>>   - add_definition()
>>>   - add_alias_definitions()
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   MAINTAINERS                           |  2 +
>>>   include/hw/core/sysemu-cpu-ops.h      | 14 ++++++
>>>   include/qapi/commands-target-compat.h | 14 ++++++
>>>   system/cpu-qmp-cmds.c                 | 71 +++++++++++++++++++++++++++
>>>   system/meson.build                    |  1 +
>>>   5 files changed, 102 insertions(+)
>>>   create mode 100644 include/qapi/commands-target-compat.h
>>>   create mode 100644 system/cpu-qmp-cmds.c
>
>
>>> diff --git a/system/cpu-qmp-cmds.c b/system/cpu-qmp-cmds.c
>>> new file mode 100644
>>> index 0000000000..daeb131159
>>> --- /dev/null
>>> +++ b/system/cpu-qmp-cmds.c
>>> @@ -0,0 +1,71 @@
>>> +/*
>>> + * QAPI helpers for target specific QMP commands
>>> + *
>>> + * SPDX-FileCopyrightText: 2024 Linaro Ltd.
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "qom/object.h"
>>> +#include "qapi/commands-target-compat.h"
>>> +#include "sysemu/arch_init.h"
>>> +#include "hw/core/cpu.h"
>>> +#include "hw/core/sysemu-cpu-ops.h"
>>> +
>>> +static void cpu_common_add_definition(gpointer data, gpointer user_data)
>>> +{
>>> +    ObjectClass *oc = data;
>>> +    CpuDefinitionInfoList **cpu_list = user_data;
>>> +    CpuDefinitionInfo *info;
>>> +    const char *typename;
>>> +
>>> +    typename = object_class_get_name(oc);
>>> +    info = g_malloc0(sizeof(*info));
>>> +    info->name = cpu_model_from_type(typename);
>>> +    info->q_typename = g_strdup(typename);
>>> +
>>> +    QAPI_LIST_PREPEND(*cpu_list, info);
>>> +}
>>> +
>>> +static void arch_add_cpu_definitions(CpuDefinitionInfoList **cpu_list,
>>> +                                     const char *cpu_typename)
>>> +{
>>> +    ObjectClass *oc;
>>> +    GSList *list;
>>> +    const struct SysemuCPUOps *ops;
>>> +
>>> +    oc = object_class_by_name(cpu_typename);
>>> +    if (!oc) {
>>> +        return;
>>> +    }
>>> +    ops = CPU_CLASS(oc)->sysemu_ops;
>>> +
>>> +    list = object_class_get_list(cpu_typename, false);
>>> +    if (ops->cpu_list_compare) {
>>> +        list = g_slist_sort(list, ops->cpu_list_compare);
>>> +    }
>>> +    g_slist_foreach(list, ops->add_definition ? : cpu_common_add_definition,
>>> +                    cpu_list);
>>> +    g_slist_free(list);
>>> +
>>> +    if (ops->add_alias_definitions) {
>>> +        ops->add_alias_definitions(cpu_list);
>>> +    }
>>> +}
>>> +
>>> +CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp)
>>> +{
>>> +    CpuDefinitionInfoList *cpu_list = NULL;
>>> +
>>> +    for (unsigned i = 0; i <= QEMU_ARCH_BIT_LAST; i++) {
>>> +        const char *cpu_typename;
>>> +
>>> +        cpu_typename = cpu_typename_by_arch_bit(i);
>>> +        if (!cpu_typename) {
>>> +            continue;
>>> +        }
>>> +        arch_add_cpu_definitions(&cpu_list, cpu_typename);
>>> +    }
>>> +
>>> +    return cpu_list;
>>> +}
>>
>> The target-specific qmp_query_cpu_definitions() this is going to replace
>> each execute the equivalent of *one* loop iteration: the one
>> corresponding to their own arch bit.
>>
>> For the replacement to be faithful, as cpu_typename_by_arch_bit() must
>> return non-null exactly once.
>>
>> This is the case for the qemu-system-TARGET.  The solution feels
>> overengineered there.
>>
>> I figure cpu_typename_by_arch_bit() will return non-null multiple times
>> in a future single binary supporting heterogeneous machines.
>>
>> Such a single binary then can't serve as drop-in replacement for the
>> qemu-system-TARGET, because query-cpu-definitions returns more.
>>
>> To get a drop-in replacement, we'll need additional logic to restrict
>> the query for the homogeneous use case.
>
> Can we ask the management layer to provide the current homogeneous
> target via argument? Otherwise we can add a new query-cpu-definitions-v2
> command requiring an explicit target argument, allowing 'all', and
> deprecate the current query-cpu-definitions.

Is "new binary can serve as drop-in replacement for the
qemu-system-TARGET" a goal?

I'm asking because a new binary is also an opportunity to improve
things, and backward compatibility conflicts with that.

Where would we want to go if backward compatibility was not a concern?
I guess we'd want a single binary capable of running any machine,
homogeneous or not.

Your query-cpu-definitions would be fine for that binary, as long as its
users can filter out the CPUs they're interested in.

I prefer client-side filtering whenever practical, since it keeps the
interface simple.  We do have to provide clients the information they
need to filter.  How would a client filter the result of your
query-cpu-definitions for target?

Since backward compatibility is a concern, and we don't want to provide
per target binaries forever, we need to think about how to provide
suitable replacements based on the single binary.  I'm not sure drop-in
replacement is feasible, given the complexity of the external
interfaces.

>> I think this needs to be discussed in the commit message.
>>
>> Possibly easier: don't loop over the bits, relying on
>> cpu_typename_by_arch_bit() to select the right one.  Instead get the
>> right bit from somewhere.
>>
>> We can switch to a loop when we need it for the heterogeneous case.
>
> Alex suggested to consider heterogeneous emulation the new default,
> and the current homogeneous use as a particular case. I'd rather not
> plan on a "heterogeneous switch day" and get things integrated in
> the way, otherwise we'll never get there...

I guess it's okay to overengineer certain things so they're ready for
the heterogenous case when it comes.  We may want to explain this in
comments, so people don't simplify the overengineered code :)

[RFC PATCH-for-9.1 15/21] target/arm: Use QMP generic_query_cpu_definitions()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/arm-qmp-cmds.c | 25 ++-----------------------
 1 file changed, 2 insertions(+), 23 deletions(-)

diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
index 3cc8cc738b..c5091e64ec 100644
--- a/target/arm/arm-qmp-cmds.c
+++ b/target/arm/arm-qmp-cmds.c
@@ -28,6 +28,7 @@
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qapi-commands-machine-target.h"
 #include "qapi/qapi-commands-misc-target.h"
+#include "qapi/commands-target-compat.h"
 #include "qapi/qmp/qdict.h"
 #include "qom/qom-qobject.h"
 
@@ -220,29 +221,7 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
     return expansion_info;
 }
 
-static void arm_cpu_add_definition(gpointer data, gpointer user_data)
-{
-    ObjectClass *oc = data;
-    CpuDefinitionInfoList **cpu_list = user_data;
-    CpuDefinitionInfo *info;
-    const char *typename;
-
-    typename = object_class_get_name(oc);
-    info = g_malloc0(sizeof(*info));
-    info->name = cpu_model_from_type(typename);
-    info->q_typename = g_strdup(typename);
-
-    QAPI_LIST_PREPEND(*cpu_list, info);
-}
-
 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
 {
-    CpuDefinitionInfoList *cpu_list = NULL;
-    GSList *list;
-
-    list = object_class_get_list(TYPE_ARM_CPU, false);
-    g_slist_foreach(list, arm_cpu_add_definition, &cpu_list);
-    g_slist_free(list);
-
-    return cpu_list;
+    return generic_query_cpu_definitions(errp);
 }
-- 
2.41.0

[RFC PATCH-for-9.1 16/21] target/loongarch: Use QMP generic_query_cpu_definitions()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/loongarch/loongarch-qmp-cmds.c | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/target/loongarch/loongarch-qmp-cmds.c b/target/loongarch/loongarch-qmp-cmds.c
index 8721a5eb13..ef5aedc1cd 100644
--- a/target/loongarch/loongarch-qmp-cmds.c
+++ b/target/loongarch/loongarch-qmp-cmds.c
@@ -9,34 +9,15 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-machine-target.h"
+#include "qapi/commands-target-compat.h"
 #include "cpu.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qobject-input-visitor.h"
 #include "qom/qom-qobject.h"
 
-static void loongarch_cpu_add_definition(gpointer data, gpointer user_data)
-{
-    ObjectClass *oc = data;
-    CpuDefinitionInfoList **cpu_list = user_data;
-    CpuDefinitionInfo *info = g_new0(CpuDefinitionInfo, 1);
-    const char *typename = object_class_get_name(oc);
-
-    info->name = cpu_model_from_type(typename);
-    info->q_typename = g_strdup(typename);
-
-    QAPI_LIST_PREPEND(*cpu_list, info);
-}
-
 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
 {
-    CpuDefinitionInfoList *cpu_list = NULL;
-    GSList *list;
-
-    list = object_class_get_list(TYPE_LOONGARCH_CPU, false);
-    g_slist_foreach(list, loongarch_cpu_add_definition, &cpu_list);
-    g_slist_free(list);
-
-    return cpu_list;
+    return generic_query_cpu_definitions(errp);
 }
 
 static const char *cpu_model_advertised_features[] = {
-- 
2.41.0

[RFC PATCH-for-9.1 17/21] target/riscv: Use QMP generic_query_cpu_definitions()

[Philippe Mathieu-Daudé]

Expose riscv_cpu_add_definition() and use it as add_definition()
handler, then use the QMP generic_query_cpu_definitions() method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/riscv/cpu.h            |  2 ++
 target/riscv/cpu.c            |  1 +
 target/riscv/riscv-qmp-cmds.c | 11 +++--------
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 3b1a02b944..15fc287680 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -824,4 +824,6 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
 uint8_t satp_mode_max_from_map(uint32_t map);
 const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
 
+void riscv_cpu_add_definition(gpointer data, gpointer user_data);
+
 #endif /* RISCV_CPU_H */
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c160b9216b..2da9364335 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2341,6 +2341,7 @@ static int64_t riscv_get_arch_id(CPUState *cs)
 #include "hw/core/sysemu-cpu-ops.h"
 
 static const struct SysemuCPUOps riscv_sysemu_ops = {
+    .add_definition = riscv_cpu_add_definition,
     .get_phys_page_debug = riscv_cpu_get_phys_page_debug,
     .write_elf64_note = riscv_cpu_write_elf64_note,
     .write_elf32_note = riscv_cpu_write_elf32_note,
diff --git a/target/riscv/riscv-qmp-cmds.c b/target/riscv/riscv-qmp-cmds.c
index d363dc318d..45adc90d3b 100644
--- a/target/riscv/riscv-qmp-cmds.c
+++ b/target/riscv/riscv-qmp-cmds.c
@@ -26,6 +26,7 @@
 
 #include "qapi/error.h"
 #include "qapi/qapi-commands-machine-target.h"
+#include "qapi/commands-target-compat.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qobject-input-visitor.h"
@@ -36,7 +37,7 @@
 #include "cpu-qom.h"
 #include "cpu.h"
 
-static void riscv_cpu_add_definition(gpointer data, gpointer user_data)
+void riscv_cpu_add_definition(gpointer data, gpointer user_data)
 {
     ObjectClass *oc = data;
     CpuDefinitionInfoList **cpu_list = user_data;
@@ -55,13 +56,7 @@ static void riscv_cpu_add_definition(gpointer data, gpointer user_data)
 
 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
 {
-    CpuDefinitionInfoList *cpu_list = NULL;
-    GSList *list = object_class_get_list(TYPE_RISCV_CPU, false);
-
-    g_slist_foreach(list, riscv_cpu_add_definition, &cpu_list);
-    g_slist_free(list);
-
-    return cpu_list;
+    return generic_query_cpu_definitions(errp);
 }
 
 static void riscv_check_if_cpu_available(RISCVCPU *cpu, Error **errp)
-- 
2.41.0

[RFC PATCH-for-9.1 18/21] target/i386: Use QMP generic_query_cpu_definitions()

[Philippe Mathieu-Daudé]

Register x86_cpu_definition_entry() and x86_cpu_list_compare()
as handler so we can use the QMP generic_query_cpu_definitions()
method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/i386/cpu.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 07f64c1ea5..e5dbd307d8 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -38,6 +38,7 @@
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/reset.h"
 #include "qapi/qapi-commands-machine-target.h"
+#include "qapi/commands-target-compat.h"
 #include "exec/address-spaces.h"
 #include "hw/boards.h"
 #include "hw/i386/sgx-epc.h"
@@ -5667,11 +5668,7 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
 
 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
 {
-    CpuDefinitionInfoList *cpu_list = NULL;
-    GSList *list = get_sorted_cpu_model_list();
-    g_slist_foreach(list, x86_cpu_definition_entry, &cpu_list);
-    g_slist_free(list);
-    return cpu_list;
+    return generic_query_cpu_definitions(errp);
 }
 
 #endif /* !CONFIG_USER_ONLY */
@@ -7937,6 +7934,8 @@ static Property x86_cpu_properties[] = {
 #include "hw/core/sysemu-cpu-ops.h"
 
 static const struct SysemuCPUOps i386_sysemu_ops = {
+    .add_definition = x86_cpu_definition_entry,
+    .cpu_list_compare = x86_cpu_list_compare,
     .get_memory_mapping = x86_cpu_get_memory_mapping,
     .get_paging_enabled = x86_cpu_get_paging_enabled,
     .get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug,
-- 
2.41.0

[PATCH-for-9.1 19/21] target/ppc: Factor ppc_add_alias_definitions() out

[Philippe Mathieu-Daudé]

Factor ppc_add_alias_definitions() out of qmp_query_cpu_definitions()
to clearly see the generic pattern used in all targets.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/ppc/cpu-models.h   |  4 ++++
 target/ppc/ppc-qmp-cmds.c | 26 +++++++++++++++-----------
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
index 0229ef3a9a..89a5e232b7 100644
--- a/target/ppc/cpu-models.h
+++ b/target/ppc/cpu-models.h
@@ -21,6 +21,8 @@
 #ifndef TARGET_PPC_CPU_MODELS_H
 #define TARGET_PPC_CPU_MODELS_H
 
+#include "qapi/qapi-types-machine.h"
+
 /**
  * PowerPCCPUAlias:
  * @alias: The alias name.
@@ -480,4 +482,6 @@ enum {
     POWERPC_SVR_8641D              = 0x80900121,
 };
 
+void ppc_add_alias_definitions(CpuDefinitionInfoList **cpu_list);
+
 #endif
diff --git a/target/ppc/ppc-qmp-cmds.c b/target/ppc/ppc-qmp-cmds.c
index a25d86a8d1..528cc3e4af 100644
--- a/target/ppc/ppc-qmp-cmds.c
+++ b/target/ppc/ppc-qmp-cmds.c
@@ -189,17 +189,9 @@ static void ppc_cpu_defs_entry(gpointer data, gpointer user_data)
     QAPI_LIST_PREPEND(*first, info);
 }
 
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+void ppc_add_alias_definitions(CpuDefinitionInfoList **cpu_list)
 {
-    CpuDefinitionInfoList *cpu_list = NULL;
-    GSList *list;
-    int i;
-
-    list = object_class_get_list(TYPE_POWERPC_CPU, false);
-    g_slist_foreach(list, ppc_cpu_defs_entry, &cpu_list);
-    g_slist_free(list);
-
-    for (i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
+    for (unsigned i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
         PowerPCCPUAlias *alias = &ppc_cpu_aliases[i];
         ObjectClass *oc;
         CpuDefinitionInfo *info;
@@ -213,8 +205,20 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
         info->name = g_strdup(alias->alias);
         info->q_typename = g_strdup(object_class_get_name(oc));
 
-        QAPI_LIST_PREPEND(cpu_list, info);
+        QAPI_LIST_PREPEND(*cpu_list, info);
     }
+}
+
+CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+{
+    CpuDefinitionInfoList *cpu_list = NULL;
+    GSList *list;
+
+    list = object_class_get_list(TYPE_POWERPC_CPU, false);
+    g_slist_foreach(list, ppc_cpu_defs_entry, &cpu_list);
+    g_slist_free(list);
+
+    ppc_add_alias_definitions(&cpu_list);
 
     return cpu_list;
 }
-- 
2.41.0

Re: [PATCH-for-9.1 19/21] target/ppc: Factor ppc_add_alias_definitions() out

[Nicholas Piggin]

On Fri Mar 15, 2024 at 11:09 PM AEST, Philippe Mathieu-Daudé wrote:
> Factor ppc_add_alias_definitions() out of qmp_query_cpu_definitions()
> to clearly see the generic pattern used in all targets.

Looks equivalent.

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/ppc/cpu-models.h   |  4 ++++
>  target/ppc/ppc-qmp-cmds.c | 26 +++++++++++++++-----------
>  2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
> index 0229ef3a9a..89a5e232b7 100644
> --- a/target/ppc/cpu-models.h
> +++ b/target/ppc/cpu-models.h
> @@ -21,6 +21,8 @@
>  #ifndef TARGET_PPC_CPU_MODELS_H
>  #define TARGET_PPC_CPU_MODELS_H
>  
> +#include "qapi/qapi-types-machine.h"
> +
>  /**
>   * PowerPCCPUAlias:
>   * @alias: The alias name.
> @@ -480,4 +482,6 @@ enum {
>      POWERPC_SVR_8641D              = 0x80900121,
>  };
>  
> +void ppc_add_alias_definitions(CpuDefinitionInfoList **cpu_list);
> +
>  #endif
> diff --git a/target/ppc/ppc-qmp-cmds.c b/target/ppc/ppc-qmp-cmds.c
> index a25d86a8d1..528cc3e4af 100644
> --- a/target/ppc/ppc-qmp-cmds.c
> +++ b/target/ppc/ppc-qmp-cmds.c
> @@ -189,17 +189,9 @@ static void ppc_cpu_defs_entry(gpointer data, gpointer user_data)
>      QAPI_LIST_PREPEND(*first, info);
>  }
>  
> -CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
> +void ppc_add_alias_definitions(CpuDefinitionInfoList **cpu_list)
>  {
> -    CpuDefinitionInfoList *cpu_list = NULL;
> -    GSList *list;
> -    int i;
> -
> -    list = object_class_get_list(TYPE_POWERPC_CPU, false);
> -    g_slist_foreach(list, ppc_cpu_defs_entry, &cpu_list);
> -    g_slist_free(list);
> -
> -    for (i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
> +    for (unsigned i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
>          PowerPCCPUAlias *alias = &ppc_cpu_aliases[i];
>          ObjectClass *oc;
>          CpuDefinitionInfo *info;
> @@ -213,8 +205,20 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
>          info->name = g_strdup(alias->alias);
>          info->q_typename = g_strdup(object_class_get_name(oc));
>  
> -        QAPI_LIST_PREPEND(cpu_list, info);
> +        QAPI_LIST_PREPEND(*cpu_list, info);
>      }
> +}
> +
> +CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
> +{
> +    CpuDefinitionInfoList *cpu_list = NULL;
> +    GSList *list;
> +
> +    list = object_class_get_list(TYPE_POWERPC_CPU, false);
> +    g_slist_foreach(list, ppc_cpu_defs_entry, &cpu_list);
> +    g_slist_free(list);
> +
> +    ppc_add_alias_definitions(&cpu_list);
>  
>      return cpu_list;
>  }

[RFC PATCH-for-9.1 20/21] target/ppc: Use QMP generic_query_cpu_definitions()

[Philippe Mathieu-Daudé]

Register ppc_add_alias_definitions() as handler so we can
use the QMP generic_query_cpu_definitions() method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/ppc/cpu_init.c     |  1 +
 target/ppc/ppc-qmp-cmds.c | 26 ++------------------------
 2 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index d5e227a6fb..3b500c95a4 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7348,6 +7348,7 @@ static Property ppc_cpu_properties[] = {
 #include "hw/core/sysemu-cpu-ops.h"
 
 static const struct SysemuCPUOps ppc_sysemu_ops = {
+    .add_alias_definitions = ppc_add_alias_definitions,
     .get_phys_page_debug = ppc_cpu_get_phys_page_debug,
     .write_elf32_note = ppc32_cpu_write_elf32_note,
     .write_elf64_note = ppc64_cpu_write_elf64_note,
diff --git a/target/ppc/ppc-qmp-cmds.c b/target/ppc/ppc-qmp-cmds.c
index 528cc3e4af..267dd84718 100644
--- a/target/ppc/ppc-qmp-cmds.c
+++ b/target/ppc/ppc-qmp-cmds.c
@@ -29,6 +29,7 @@
 #include "monitor/hmp-target.h"
 #include "monitor/hmp.h"
 #include "qapi/qapi-commands-machine-target.h"
+#include "qapi/commands-target-compat.h"
 #include "cpu-models.h"
 #include "cpu-qom.h"
 
@@ -175,20 +176,6 @@ int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
     return -EINVAL;
 }
 
-static void ppc_cpu_defs_entry(gpointer data, gpointer user_data)
-{
-    ObjectClass *oc = data;
-    CpuDefinitionInfoList **first = user_data;
-    const char *typename;
-    CpuDefinitionInfo *info;
-
-    typename = object_class_get_name(oc);
-    info = g_malloc0(sizeof(*info));
-    info->name = cpu_model_from_type(typename);
-
-    QAPI_LIST_PREPEND(*first, info);
-}
-
 void ppc_add_alias_definitions(CpuDefinitionInfoList **cpu_list)
 {
     for (unsigned i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
@@ -211,14 +198,5 @@ void ppc_add_alias_definitions(CpuDefinitionInfoList **cpu_list)
 
 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
 {
-    CpuDefinitionInfoList *cpu_list = NULL;
-    GSList *list;
-
-    list = object_class_get_list(TYPE_POWERPC_CPU, false);
-    g_slist_foreach(list, ppc_cpu_defs_entry, &cpu_list);
-    g_slist_free(list);
-
-    ppc_add_alias_definitions(&cpu_list);
-
-    return cpu_list;
+    return generic_query_cpu_definitions(errp);
 }
-- 
2.41.0

[RFC PATCH-for-9.1 21/21] qapi: Make @query-cpu-definitions target-agnostic

[Philippe Mathieu-Daudé]

All targets use the generic_query_cpu_definitions() method,
which is not target-specific. Make the command target agnostic
by moving it to machine.json. Rename generic_query_cpu_definitions
as qmp_query_cpu_definitions.

This is an introspection change for the target that were not
implementing qmp_query_cpu_definitions(): now query-cpu-definitions
returns an their CPUs list.

Example with SH4 before:

  { "execute": "query-cpu-definitions" }

  { "error": {"class": "CommandNotFound", "desc": "The command query-cpu-definitions has not been found"} }

and after:

  { "execute": "query-cpu-definitions" }

  { "return": [
          {"name": "sh7751r", "typename": "sh7751r-superh-cpu", "static": false, "deprecated": false},
          {"name": "sh7750r", "typename": "sh7750r-superh-cpu", "static": false, "deprecated": false},
          {"name": "sh7785", "typename": "sh7785-superh-cpu", "static": false, "deprecated": false}
      ]
  }

However this allows heterogeneous emulation to return a correct list.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Well, not all target got converted, I left the s390x one for later :)
---
 MAINTAINERS                           |  1 -
 qapi/machine-target.json              | 18 ------------------
 qapi/machine.json                     | 11 +++++++++++
 include/qapi/commands-target-compat.h | 14 --------------
 system/cpu-qmp-cmds.c                 |  4 ++--
 target/arm/arm-qmp-cmds.c             |  6 ------
 target/i386/cpu.c                     |  6 ------
 target/loongarch/loongarch-qmp-cmds.c |  6 ------
 target/mips/sysemu/mips-qmp-cmds.c    |  9 ---------
 target/ppc/ppc-qmp-cmds.c             |  7 -------
 target/riscv/riscv-qmp-cmds.c         |  6 ------
 11 files changed, 13 insertions(+), 75 deletions(-)
 delete mode 100644 include/qapi/commands-target-compat.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 39d7c14d98..71f446311b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1895,7 +1895,6 @@ F: qapi/machine-target.json
 F: include/hw/boards.h
 F: include/hw/core/cpu.h
 F: include/hw/cpu/cluster.h
-F: include/qapi/commands-target-compat.h
 F: include/sysemu/numa.h
 F: tests/unit/test-smp-parse.c
 T: git https://gitlab.com/ehabkost/qemu.git machine-next
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 7480921d33..2065972d8a 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -212,24 +212,6 @@
                    'TARGET_LOONGARCH64',
                    'TARGET_RISCV' ] } }
 
-##
-# @query-cpu-definitions:
-#
-# Return a list of supported virtual CPU definitions
-#
-# Returns: a list of CpuDefinitionInfo
-#
-# Since: 1.2
-##
-{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
-  'if': { 'any': [ 'TARGET_PPC',
-                   'TARGET_ARM',
-                   'TARGET_I386',
-                   'TARGET_S390X',
-                   'TARGET_MIPS',
-                   'TARGET_LOONGARCH64',
-                   'TARGET_RISCV' ] } }
-
 ##
 # @CpuS390Polarization:
 #
diff --git a/qapi/machine.json b/qapi/machine.json
index 9c44b8fa82..987c64f8e7 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -73,6 +73,17 @@
             'deprecated' : 'bool' }
 }
 
+##
+# @query-cpu-definitions:
+#
+# Return a list of supported virtual CPU definitions
+#
+# Returns: a list of CpuDefinitionInfo
+#
+# Since: 1.2
+##
+{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
+
 ##
 # @CpuModelInfo:
 #
diff --git a/include/qapi/commands-target-compat.h b/include/qapi/commands-target-compat.h
deleted file mode 100644
index 86d45d8fcc..0000000000
--- a/include/qapi/commands-target-compat.h
+++ /dev/null
@@ -1,14 +0,0 @@
-/*
- * QAPI helpers for target specific QMP commands
- *
- * SPDX-FileCopyrightText: 2024 Linaro Ltd.
- * SPDX-License-Identifier: GPL-2.0-or-later
- */
-#ifndef QAPI_COMPAT_TARGET_H
-#define QAPI_COMPAT_TARGET_H
-
-#include "qapi/qapi-types-machine.h"
-
-CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp);
-
-#endif
diff --git a/system/cpu-qmp-cmds.c b/system/cpu-qmp-cmds.c
index daeb131159..049e8b9d35 100644
--- a/system/cpu-qmp-cmds.c
+++ b/system/cpu-qmp-cmds.c
@@ -7,7 +7,7 @@
 
 #include "qemu/osdep.h"
 #include "qom/object.h"
-#include "qapi/commands-target-compat.h"
+#include "qapi/qapi-commands-machine.h"
 #include "sysemu/arch_init.h"
 #include "hw/core/cpu.h"
 #include "hw/core/sysemu-cpu-ops.h"
@@ -53,7 +53,7 @@ static void arch_add_cpu_definitions(CpuDefinitionInfoList **cpu_list,
     }
 }
 
-CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
 {
     CpuDefinitionInfoList *cpu_list = NULL;
 
diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
index c5091e64ec..ac8d890bc0 100644
--- a/target/arm/arm-qmp-cmds.c
+++ b/target/arm/arm-qmp-cmds.c
@@ -28,7 +28,6 @@
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qapi-commands-machine-target.h"
 #include "qapi/qapi-commands-misc-target.h"
-#include "qapi/commands-target-compat.h"
 #include "qapi/qmp/qdict.h"
 #include "qom/qom-qobject.h"
 
@@ -220,8 +219,3 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
 
     return expansion_info;
 }
-
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-    return generic_query_cpu_definitions(errp);
-}
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index e5dbd307d8..f26adefd37 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -38,7 +38,6 @@
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/reset.h"
 #include "qapi/qapi-commands-machine-target.h"
-#include "qapi/commands-target-compat.h"
 #include "exec/address-spaces.h"
 #include "hw/boards.h"
 #include "hw/i386/sgx-epc.h"
@@ -5666,11 +5665,6 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
     QAPI_LIST_PREPEND(*cpu_list, info);
 }
 
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-    return generic_query_cpu_definitions(errp);
-}
-
 #endif /* !CONFIG_USER_ONLY */
 
 uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
diff --git a/target/loongarch/loongarch-qmp-cmds.c b/target/loongarch/loongarch-qmp-cmds.c
index ef5aedc1cd..aaed7bbf93 100644
--- a/target/loongarch/loongarch-qmp-cmds.c
+++ b/target/loongarch/loongarch-qmp-cmds.c
@@ -9,17 +9,11 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-machine-target.h"
-#include "qapi/commands-target-compat.h"
 #include "cpu.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qobject-input-visitor.h"
 #include "qom/qom-qobject.h"
 
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-    return generic_query_cpu_definitions(errp);
-}
-
 static const char *cpu_model_advertised_features[] = {
     "lsx", "lasx", NULL
 };
diff --git a/target/mips/sysemu/mips-qmp-cmds.c b/target/mips/sysemu/mips-qmp-cmds.c
index 329db3a028..c3c775acc6 100644
--- a/target/mips/sysemu/mips-qmp-cmds.c
+++ b/target/mips/sysemu/mips-qmp-cmds.c
@@ -5,12 +5,3 @@
  *
  * SPDX-License-Identifier: LGPL-2.1-or-later
  */
-
-#include "qemu/osdep.h"
-#include "qapi/qapi-commands-machine-target.h"
-#include "qapi/commands-target-compat.h"
-
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-    return generic_query_cpu_definitions(errp);
-}
diff --git a/target/ppc/ppc-qmp-cmds.c b/target/ppc/ppc-qmp-cmds.c
index 267dd84718..f85c4560f9 100644
--- a/target/ppc/ppc-qmp-cmds.c
+++ b/target/ppc/ppc-qmp-cmds.c
@@ -28,8 +28,6 @@
 #include "qemu/ctype.h"
 #include "monitor/hmp-target.h"
 #include "monitor/hmp.h"
-#include "qapi/qapi-commands-machine-target.h"
-#include "qapi/commands-target-compat.h"
 #include "cpu-models.h"
 #include "cpu-qom.h"
 
@@ -195,8 +193,3 @@ void ppc_add_alias_definitions(CpuDefinitionInfoList **cpu_list)
         QAPI_LIST_PREPEND(*cpu_list, info);
     }
 }
-
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-    return generic_query_cpu_definitions(errp);
-}
diff --git a/target/riscv/riscv-qmp-cmds.c b/target/riscv/riscv-qmp-cmds.c
index 45adc90d3b..170d720b24 100644
--- a/target/riscv/riscv-qmp-cmds.c
+++ b/target/riscv/riscv-qmp-cmds.c
@@ -26,7 +26,6 @@
 
 #include "qapi/error.h"
 #include "qapi/qapi-commands-machine-target.h"
-#include "qapi/commands-target-compat.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qobject-input-visitor.h"
@@ -54,11 +53,6 @@ void riscv_cpu_add_definition(gpointer data, gpointer user_data)
     QAPI_LIST_PREPEND(*cpu_list, info);
 }
 
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-    return generic_query_cpu_definitions(errp);
-}
-
 static void riscv_check_if_cpu_available(RISCVCPU *cpu, Error **errp)
 {
     if (!riscv_cpu_accelerator_compatible(cpu)) {
-- 
2.41.0

Re: [RFC PATCH-for-9.1 21/21] qapi: Make @query-cpu-definitions target-agnostic

[Markus Armbruster]

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

> All targets use the generic_query_cpu_definitions() method,
> which is not target-specific. Make the command target agnostic
> by moving it to machine.json. Rename generic_query_cpu_definitions
> as qmp_query_cpu_definitions.
>
> This is an introspection change for the target that were not
> implementing qmp_query_cpu_definitions(): now query-cpu-definitions
> returns an their CPUs list.

Do you mean "returns their CPUs list"?

>
> Example with SH4 before:
>
>   { "execute": "query-cpu-definitions" }
>
>   { "error": {"class": "CommandNotFound", "desc": "The command query-cpu-definitions has not been found"} }
>
> and after:
>
>   { "execute": "query-cpu-definitions" }
>
>   { "return": [
>           {"name": "sh7751r", "typename": "sh7751r-superh-cpu", "static": false, "deprecated": false},
>           {"name": "sh7750r", "typename": "sh7750r-superh-cpu", "static": false, "deprecated": false},
>           {"name": "sh7785", "typename": "sh7785-superh-cpu", "static": false, "deprecated": false}
>       ]
>   }

Does the result make sense?  What do the callbacks add for the targets
that define them, and why do the other targets don't need them?

Conversion steps:

0. Create a generic version of the command, with optional callbacks
   [PATCH 14]

1. Turn the target-specific versions of the command into thin wrappers
   around the generic version one by one, supplying callbacks to
   preserve behavior [PATCH 15-20]

2. Peel off the wrappers [this patch]

3. Enable the command for all the other targets [also this patch]

Correct?

Split this patch?  Question, not a demand.

I think the commit messages of the step 1 patches should explain the
target-specific behavior, i.e. the difference between their callbacks
and the default behavior.

Obvious for cpu_list_compare(): sorted vs. unsorted.

Not obvious for add_definition().

> However this allows heterogeneous emulation to return a correct list.

Cryptic.  It made me go back to PATCH 14, where I discovered the loop
over the arch bits.

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Well, not all target got converted, I left the s390x one for later :)

Haha!

Re: [RFC PATCH-for-9.1 00/21] qapi: Make @query-cpu-definitions command target-agnostic

[Markus Armbruster]

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

> Hi Alex, Markus,
>
> Markus mentioned QAPI problems with the heterogeneous emulation
> binary. My understanding is, while QAPI can use host-specific
> conditional (OS, library available, configure option), it
> shouldn't use target-specific ones.

"Shouldn't" is too strong in the current state of things.  It can be
awkward, though.

Target-specific macros may be used only in target-specific code,
i.e. code that is compiled per target.  To catch uses in
target-independent code, we poison them there; see exec/poison.h.

QAPI-generated .c are not normally compiled per target.  To enable use
of target-specific macros in conditionals, we compile .c generated QAPI
submodules named *-target.json per target.  This is a bit of a hack.

Since the same conditionals will also appear in the .h generated from
them, these headers can only be used in target-specific code.

Sometimes, these headers "infect" target-independent code: we need to
compile per target just for the headers.  Awkward.  I can dig out an
example if there's interest.

But what about possible future state of things?

The QAPI schema is compile-time static by design.  QAPI conditionals
permit adjusting the schema for build configuration.  Target-specific
conditionals adjust it for the build configuration of the
target-specific part.  Each QEMU binary contains just one target's
target-specific part.

However, a single QEMU binary will contain several target-specific
parts, one per target it supports.  The targets' QAPI schema adjustments
may conflict.

For a single binary, we need to resolve these conflicts.

Special case: QAPI definitions that exist only for some targets.

Example: command query-sev exists only for TARGET_I386.  It's actually a
stub when CONFIG_SEV is off.

Obvious solution: make it exist if it needs to exist for any target
compiled into the binary.  Requires command stubs for the other targets.

Example: query-sev now exists when the single binary supports x86.  It's
a stub when CONFIG_SEV is off.  It behaves like a stub when CONFIG_SEV
is on, but the machine isn't x86.

Drawback: introspection with query-qmp-schema becomes less precise.
When the binary supports just one target, introspection can answer
target-specific questions like "does this target support SEV?"  With a
single binary, that's no longer possible.

Harmless enough for SEV, but consider query-cpu-model-expansion.  The
command may support expansion types "full" and "static".  Currently,
s390x supports both, ARM supports only "full", Loongarch only "static",
RISC-V only "full", and x86 supports both.

(I think.  The documentation is incomplete:

    # Some architectures may not support all expansion types.  s390x
    # supports "full" and "static". Arm only supports "full".
)

A management application may want to find out which expansion type is
supported.  Right now, it can't.  But we could improve the schema so it
can find out via introspection: define the expansion types only when
they're actually supported.

With a single binary, that's no longer possible: we have to define an
expansion type when *any* target supports it.

Such loss of introspection power is not a show-stopper, just something
we need to keep in mind.

> This series is an example on how to remove target specific
> bits from the @query-cpu-definitions command.

This is an instance of the special case with an additional twist: each
target defines its own QMP command handler.

>                                               Target specific
> code is registered as CPUClass handlers, then a generic method
> is used, iterating over all targets built in.
>
> The first set of patches were already posted / reviewed last
> year.
>
> The PPC and S390X targets still need work (help welcomed),
> however the code is useful enough to be tested and see if this
> is a good approach.
>
> The only drawback is a change in QAPI introspection, because
> targets not implementing @query-cpu-definitions were returning
> "CommandNotFound".

The change is introspection is actually something else.  Before the
series, query-qmp-schema returns a description of command
query-cpu-definitions exactly when the (single) target supports it.
Afterwards, it always returns one (I think).

The CommandNotFound change is a change in behavior when you try to
execute query-cpu-definitions, but the target doesn't support it.
Before, the command doesn't exist, and the QMP core duly replies
CommandNotFound.  Afterwards, it does exist, but the target doesn't
implement the call back, so the command fails.  I guess it fails with
GenericError.  We could make it fail with CommandNotFound if we care.

>                    My view is this was an incomplete
> implementation, rather than a feature.

Feels fair (but I'm not an expert in this area).

Re: [RFC PATCH-for-9.1 00/21] qapi: Make @query-cpu-definitions command target-agnostic

[Markus Armbruster]

The general approach looks sane enough to me.