[PATCH v3 00/32] Unified CPU type check

[PATCH v3 00/32] Unified CPU type check

[Gavin Shan]

There are two places where the user specified CPU type is checked to see
if it's supported or allowed by the board: machine_run_board_init() and
mc->init(). We don't have to maintain two duplicate sets of logic. This
series intends to move the check to machine_run_board_init().

PATCH[01]    Adds a generic helper cpu_model_from_type() to convert CPU
             type name to CPU model name.
PATCH[02-19] Uses cpu_model_from_type() in the individual targets
PATCH[20-23] Implements cpu_list() for the missed targets
PATCH[24-27] Improves the CPU type validation in machine_run_board_init()
PATCH[28-32] Validate the CPU type in machine_run_board_init() for the
             individual boards

v1: https://lists.nongnu.org/archive/html/qemu-arm/2023-07/msg00302.html
v2: https://lists.nongnu.org/archive/html/qemu-arm/2023-07/msg00528.html

Testing
=======

With the following command lines, the output messages are varied before
and after the series is applied.

  ./build/qemu-system-aarch64            \
  -accel tcg -machine virt,gic-version=3 \
  -cpu cortex-a8 -smp maxcpus=2,cpus=1   \
    :

Before the series is applied:

  qemu-system-aarch64: mach-virt: CPU type cortex-a8-arm-cpu not supported

After the series is applied:

  qemu-system-aarch64: Invalid CPU type: cortex-a8-arm-cpu
  The valid models are: cortex-a7, cortex-a15, cortex-a35, cortex-a55,
                        cortex-a72, cortex-a76, a64fx, neoverse-n1,
                        neoverse-v1, cortex-a53, cortex-a57, max

Changelog
=========
v3:
  * Generic helper cpu_model_from_type()                        (Igor)
  * Apply cpu_model_from_type() to the individual targets       (Igor)
  * Implement cpu_list() for the missed targets                 (Gavin)
  * Remove mc->valid_cpu_models                                 (Richard)
  * Separate patch to constify mc->validate_cpu_types           (Gavin)
v2:
  * Constify mc->valid_cpu_types                                (Richard)
  * Print the supported CPU models, instead of typenames        (Peter)
  * Misc improvements for the hleper to do the check            (Igor)
  * More patches to move the check                              (Marcin)

Gavin Shan (32):
  cpu: Add helper cpu_model_from_type()
  target/alpha: Use generic helper to show CPU model names
  target/arm: Use generic helper to show CPU model names
  target/avr: Use generic helper to show CPU model names
  target/cris: Use generic helper to show CPU model names
  target/hexagon: Use generic helper to show CPU model names
  target/i386: Use generic helper to show CPU model names
  target/loongarch: Use generic helper to show CPU model names
  target/m68k: Use generic helper to show CPU model names
  target/mips: Use generic helper to show CPU model names
  target/openrisc: Use generic helper to show CPU model names
  target/ppc: Use generic helper to show CPU model names
  target/riscv: Use generic helper to show CPU model names
  target/rx: Use generic helper to show CPU model names
  target/s390x: Use generic helper to show CPU model names
  target/sh4: Use generic helper to show CPU model names
  target/tricore: Use generic helper to show CPU model names
  target/sparc: Improve sparc_cpu_class_by_name()
  target/xtensa: Improve xtensa_cpu_class_by_name()
  target/hppa: Implement hppa_cpu_list()
  target/microblaze: Implement microblaze_cpu_list()
  target/nios2: Implement nios2_cpu_list()
  Mark cpu_list() supported on all targets
  machine: Constify MachineClass::valid_cpu_types[i]
  machine: Use error handling when CPU type is checked
  machine: Introduce helper is_cpu_type_supported()
  machine: Print CPU model name instead of CPU type name
  hw/arm/virt: Check CPU type in machine_run_board_init()
  hw/arm/virt: Hide host CPU model for tcg
  hw/arm/sbsa-ref: Check CPU type in machine_run_board_init()
  hw/arm: Check CPU type in machine_run_board_init()
  hw/riscv/shakti_c: Check CPU type in machine_run_board_init()

 bsd-user/main.c                       |  3 -
 cpu.c                                 | 19 +++++-
 hw/arm/bananapi_m2u.c                 | 12 ++--
 hw/arm/cubieboard.c                   | 12 ++--
 hw/arm/mps2-tz.c                      | 20 ++++--
 hw/arm/mps2.c                         | 25 ++++++--
 hw/arm/msf2-som.c                     | 12 ++--
 hw/arm/musca.c                        | 13 ++--
 hw/arm/npcm7xx_boards.c               | 13 ++--
 hw/arm/orangepi.c                     | 12 ++--
 hw/arm/sbsa-ref.c                     | 21 +------
 hw/arm/virt.c                         | 23 ++-----
 hw/core/machine.c                     | 90 ++++++++++++++++-----------
 hw/m68k/q800.c                        |  2 +-
 hw/riscv/shakti_c.c                   | 11 ++--
 include/hw/boards.h                   |  2 +-
 include/hw/core/cpu.h                 | 12 ++++
 target/alpha/cpu.c                    |  6 +-
 target/arm/arm-qmp-cmds.c             |  6 +-
 target/arm/helper.c                   | 12 ++--
 target/avr/cpu.c                      | 23 +++++--
 target/cris/cpu.c                     |  7 +--
 target/hexagon/cpu.c                  | 12 ++--
 target/hppa/cpu.c                     | 19 ++++++
 target/hppa/cpu.h                     |  3 +
 target/i386/cpu.c                     |  7 +--
 target/loongarch/cpu.c                |  5 +-
 target/loongarch/loongarch-qmp-cmds.c |  3 +-
 target/m68k/helper.c                  | 14 ++---
 target/microblaze/cpu.c               | 20 ++++++
 target/microblaze/cpu.h               |  3 +
 target/mips/cpu-defs.c.inc            |  9 ---
 target/mips/cpu.c                     | 25 +++++++-
 target/mips/sysemu/mips-qmp-cmds.c    |  3 +-
 target/nios2/cpu.c                    | 20 ++++++
 target/nios2/cpu.h                    |  3 +
 target/openrisc/cpu.c                 | 23 +++----
 target/ppc/cpu_init.c                 | 11 ++--
 target/riscv/cpu.c                    | 23 ++++---
 target/riscv/riscv-qmp-cmds.c         |  3 +-
 target/rx/cpu.c                       | 16 +++--
 target/s390x/cpu_models.c             | 18 +++---
 target/s390x/cpu_models_sysemu.c      |  9 ++-
 target/sh4/cpu.c                      | 24 +++----
 target/sparc/cpu.c                    |  7 ++-
 target/tricore/cpu.c                  |  9 +--
 target/tricore/helper.c               | 13 ++--
 target/xtensa/cpu.c                   |  9 +--
 48 files changed, 397 insertions(+), 270 deletions(-)

-- 
2.41.0

[PATCH v3 01/32] cpu: Add helper cpu_model_from_type()

[Gavin Shan]

Add helper cpu_model_from_type() to extract the CPU model name from
the CPU type name in two circumstances: (1) The CPU type name is the
combination of the CPU model name and suffix. (2) The CPU type name
is same to the CPU model name.

The helper will be used in the subsequent patches to conver the
CPU type name to the CPU model name.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 cpu.c                 | 16 ++++++++++++++++
 include/hw/core/cpu.h | 12 ++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/cpu.c b/cpu.c
index 1c948d1161..a19e33ff96 100644
--- a/cpu.c
+++ b/cpu.c
@@ -284,6 +284,22 @@ const char *parse_cpu_option(const char *cpu_option)
     return cpu_type;
 }
 
+char *cpu_model_from_type(const char *typename)
+{
+    const char *suffix = "-" CPU_RESOLVING_TYPE;
+
+    if (!object_class_by_name(typename)) {
+        return NULL;
+    }
+
+    if (strlen(typename) > strlen(suffix) &&
+        !strcmp(typename + strlen(typename) - strlen(suffix), suffix)) {
+        return g_strndup(typename, strlen(typename) - strlen(suffix));
+    }
+
+    return g_strdup(typename);
+}
+
 void list_cpus(void)
 {
     /* XXX: implement xxx_cpu_list for targets that still miss it */
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 92a4234439..6e76d95490 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -657,6 +657,18 @@ CPUState *cpu_create(const char *typename);
  */
 const char *parse_cpu_option(const char *cpu_option);
 
+/**
+ * cpu_model_from_type:
+ * @typename: The CPU type name
+ *
+ * Extract the CPU model name from the CPU type name. The
+ * CPU type name is either the combination of the CPU model
+ * name and suffix, or same to the CPU model name.
+ *
+ * Returns: CPU model name
+ */
+char *cpu_model_from_type(const char *typename);
+
 /**
  * cpu_has_work:
  * @cpu: The vCPU to check.
-- 
2.41.0

Re: [PATCH v3 01/32] cpu: Add helper cpu_model_from_type()

[Philippe Mathieu-Daudé]

On 7/9/23 02:35, Gavin Shan wrote:
> Add helper cpu_model_from_type() to extract the CPU model name from
> the CPU type name in two circumstances: (1) The CPU type name is the
> combination of the CPU model name and suffix. (2) The CPU type name
> is same to the CPU model name.
> 
> The helper will be used in the subsequent patches to conver the

"patches to conver" -> "commits to convert"

> CPU type name to the CPU model name.
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>   cpu.c                 | 16 ++++++++++++++++
>   include/hw/core/cpu.h | 12 ++++++++++++
>   2 files changed, 28 insertions(+)

Re: [PATCH v3 01/32] cpu: Add helper cpu_model_from_type()

[Gavin Shan]

On 9/7/23 18:54, Philippe Mathieu-Daudé wrote:
> On 7/9/23 02:35, Gavin Shan wrote:
>> Add helper cpu_model_from_type() to extract the CPU model name from
>> the CPU type name in two circumstances: (1) The CPU type name is the
>> combination of the CPU model name and suffix. (2) The CPU type name
>> is same to the CPU model name.
>>
>> The helper will be used in the subsequent patches to conver the
> 
> "patches to conver" -> "commits to convert"
> 

Thanks, it will be fixed in next respin.

>> CPU type name to the CPU model name.
>>
>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   cpu.c                 | 16 ++++++++++++++++
>>   include/hw/core/cpu.h | 12 ++++++++++++
>>   2 files changed, 28 insertions(+)
> 

Thanks,
Gavin

[PATCH v3 02/32] target/alpha: Use generic helper to show CPU model names

[Gavin Shan]

For target/alpha, the CPU type name can be: (1) the combination of
the CPU model name and suffix; (2) same to the CPU model name; (3)
pre-defined aliases. It's correct to show the CPU model name for (1)
or the CPU type name for (2) in cpu_list() because both of them can
be resolved by alpha_cpu_class_by_name() successfully.

Lets follow (1) to show the CPU model names, with the suffix stripped.
With this, the output is compabitle with all of most cases.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/alpha/cpu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 270ae787b1..9e15c3245b 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -89,9 +89,11 @@ static void alpha_cpu_realizefn(DeviceState *dev, Error **errp)
 
 static void alpha_cpu_list_entry(gpointer data, gpointer user_data)
 {
-    ObjectClass *oc = data;
+    const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    char *model = cpu_model_from_type(typename);
 
-    qemu_printf("  %s\n", object_class_get_name(oc));
+    qemu_printf("  %s\n", model);
+    g_free(model);
 }
 
 void alpha_cpu_list(void)
-- 
2.41.0

[PATCH v3 03/32] target/arm: Use generic helper to show CPU model names

[Gavin Shan]

For target/arm, the CPU type name can be: (1) the combination of
the CPU model name and suffix; (2) alias "any" corresponding to
"max-arm-cpu" when CONFIG_USER_ONLY is enabled. The CPU model names
have been already shown in cpu_list() and query_cpu_definitions()
by following (1).

Use generic helper cpu_model_from_type() to show the CPU model names.
The variable @name is renamed to @model in arm_cpu_list_entry() since
it points to the CPU model name instead of the CPU type name.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/arm/arm-qmp-cmds.c |  6 ++----
 target/arm/helper.c       | 12 +++++-------
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
index c8fa524002..51fddaefc3 100644
--- a/target/arm/arm-qmp-cmds.c
+++ b/target/arm/arm-qmp-cmds.c
@@ -233,12 +233,10 @@ static void arm_cpu_add_definition(gpointer data, gpointer user_data)
     ObjectClass *oc = data;
     CpuDefinitionInfoList **cpu_list = user_data;
     CpuDefinitionInfo *info;
-    const char *typename;
+    const char *typename = object_class_get_name(oc);
 
-    typename = object_class_get_name(oc);
     info = g_malloc0(sizeof(*info));
-    info->name = g_strndup(typename,
-                           strlen(typename) - strlen("-" TYPE_ARM_CPU));
+    info->name = cpu_model_from_type(typename);
     info->q_typename = g_strdup(typename);
 
     QAPI_LIST_PREPEND(*cpu_list, info);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index e3f5a7d2bd..7b8257b496 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9409,17 +9409,15 @@ static void arm_cpu_list_entry(gpointer data, gpointer user_data)
 {
     ObjectClass *oc = data;
     CPUClass *cc = CPU_CLASS(oc);
-    const char *typename;
-    char *name;
+    const char *typename = object_class_get_name(oc);
+    char *model = cpu_model_from_type(typename);
 
-    typename = object_class_get_name(oc);
-    name = g_strndup(typename, strlen(typename) - strlen("-" TYPE_ARM_CPU));
     if (cc->deprecation_note) {
-        qemu_printf("  %s (deprecated)\n", name);
+        qemu_printf("  %s (deprecated)\n", model);
     } else {
-        qemu_printf("  %s\n", name);
+        qemu_printf("  %s\n", model);
     }
-    g_free(name);
+    g_free(model);
 }
 
 void arm_cpu_list(void)
-- 
2.41.0

[PATCH v3 04/32] target/avr: Use generic helper to show CPU model names

[Gavin Shan]

For target/avr, the CPU model name is resolved as same to the CPU
type name in avr_cpu_class_by_name(). Actually, the CPU model name
is the combination of the CPU model name and suffix.

Support the resolution from the combination of CPU model name and
suffix to the CPU type name in avr_cpu_class_by_name(), and use
the generic helper cpu_model_from_type() to show CPU model names
in cpu_list(), with adjusted format to match with other targets.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/avr/cpu.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 8f741f258c..cef9f84e32 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -157,13 +157,23 @@ static void avr_cpu_initfn(Object *obj)
 static ObjectClass *avr_cpu_class_by_name(const char *cpu_model)
 {
     ObjectClass *oc;
+    char *typename;
 
     oc = object_class_by_name(cpu_model);
-    if (object_class_dynamic_cast(oc, TYPE_AVR_CPU) == NULL ||
-        object_class_is_abstract(oc)) {
-        oc = NULL;
+    if (object_class_dynamic_cast(oc, TYPE_AVR_CPU) &&
+        !object_class_is_abstract(oc)) {
+        return oc;
     }
-    return oc;
+
+    typename = g_strdup_printf(AVR_CPU_TYPE_NAME("%s"), cpu_model);
+    oc = object_class_by_name(typename);
+    g_free(typename);
+    if (object_class_dynamic_cast(oc, TYPE_AVR_CPU) &&
+        !object_class_is_abstract(oc)) {
+        return oc;
+    }
+
+    return NULL;
 }
 
 static void avr_cpu_dump_state(CPUState *cs, FILE *f, int flags)
@@ -366,14 +376,17 @@ typedef struct AVRCPUInfo {
 static void avr_cpu_list_entry(gpointer data, gpointer user_data)
 {
     const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    char *model = cpu_model_from_type(typename);
 
-    qemu_printf("%s\n", typename);
+    qemu_printf("  %s\n", model);
+    g_free(model);
 }
 
 void avr_cpu_list(void)
 {
     GSList *list;
     list = object_class_get_list_sorted(TYPE_AVR_CPU, false);
+    qemu_printf("Available CPUs:\n");
     g_slist_foreach(list, avr_cpu_list_entry, NULL);
     g_slist_free(list);
 }
-- 
2.41.0

[PATCH v3 05/32] target/cris: Use generic helper to show CPU model names

[Gavin Shan]

For target/cris, the CPU type name can be: (1) the combination of
the CPU model name and suffix; (2) alias "any" corresponding to
"crisv32-cris-cpu". The CPU model names have been shown correctly
by following (1).

Use generic helper cpu_model_from_type() to show the CPU model
names in arm_cpu_list_entry(), and rename @name to @model since
it's points to the CPU model name instead of the CPU type name.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/cris/cpu.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/target/cris/cpu.c b/target/cris/cpu.c
index a6a93c2359..420a2f75fb 100644
--- a/target/cris/cpu.c
+++ b/target/cris/cpu.c
@@ -122,11 +122,10 @@ static void cris_cpu_list_entry(gpointer data, gpointer user_data)
 {
     ObjectClass *oc = data;
     const char *typename = object_class_get_name(oc);
-    char *name;
+    char *model = cpu_model_from_type(typename);
 
-    name = g_strndup(typename, strlen(typename) - strlen(CRIS_CPU_TYPE_SUFFIX));
-    qemu_printf("  %s\n", name);
-    g_free(name);
+    qemu_printf("  %s\n", model);
+    g_free(model);
 }
 
 void cris_cpu_list(void)
-- 
2.41.0

[PATCH v3 06/32] target/hexagon: Use generic helper to show CPU model names

[Gavin Shan]

For target/hexagon, the CPU type name is always the combination of
the CPU model name and suffix. The CPU model names have been shown
correctly in hexagon_cpu_list_entry().

Use generic helper cpu_model_from_type() to show the CPU model names
in hexagon_cpu_list_entry(), and rename @name to @model since it points
to the CPU model name instead of the CPU type name.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/hexagon/cpu.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index f155936289..3d0174e6f1 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -34,13 +34,11 @@ static void hexagon_v73_cpu_init(Object *obj) { }
 
 static void hexagon_cpu_list_entry(gpointer data, gpointer user_data)
 {
-    ObjectClass *oc = data;
-    char *name = g_strdup(object_class_get_name(oc));
-    if (g_str_has_suffix(name, HEXAGON_CPU_TYPE_SUFFIX)) {
-        name[strlen(name) - strlen(HEXAGON_CPU_TYPE_SUFFIX)] = '\0';
-    }
-    qemu_printf("  %s\n", name);
-    g_free(name);
+    const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    char *model = cpu_model_from_type(typename);
+
+    qemu_printf("  %s\n", model);
+    g_free(model);
 }
 
 void hexagon_cpu_list(void)
-- 
2.41.0

[PATCH v3 07/32] target/i386: Use generic helper to show CPU model names

[Gavin Shan]

For target/i386, the CPU type name is always the combination of the
CPU model name and suffix. The CPU model names have been shown
correctly in x86_cpu_list_entry().

Use generic helper cpu_model_from_type() to get the CPU model name
from the CPU type name in x86_cpu_class_get_model_name(), and rename
@name to @model in x86_cpu_list_entry() since it points to the CPU
model name instead of the CPU type name.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/i386/cpu.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 00f913b638..31f1d0379e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1741,8 +1741,7 @@ static char *x86_cpu_class_get_model_name(X86CPUClass *cc)
 {
     const char *class_name = object_class_get_name(OBJECT_CLASS(cc));
     assert(g_str_has_suffix(class_name, X86_CPU_TYPE_SUFFIX));
-    return g_strndup(class_name,
-                     strlen(class_name) - strlen(X86_CPU_TYPE_SUFFIX));
+    return cpu_model_from_type(class_name);
 }
 
 typedef struct X86CPUVersionDefinition {
@@ -5544,7 +5543,7 @@ static void x86_cpu_list_entry(gpointer data, gpointer user_data)
 {
     ObjectClass *oc = data;
     X86CPUClass *cc = X86_CPU_CLASS(oc);
-    g_autofree char *name = x86_cpu_class_get_model_name(cc);
+    g_autofree char *model = x86_cpu_class_get_model_name(cc);
     g_autofree char *desc = g_strdup(cc->model_description);
     g_autofree char *alias_of = x86_cpu_class_get_alias_of(cc);
     g_autofree char *model_id = x86_cpu_class_get_model_id(cc);
@@ -5568,7 +5567,7 @@ static void x86_cpu_list_entry(gpointer data, gpointer user_data)
         desc = g_strdup_printf("%s (deprecated)", olddesc);
     }
 
-    qemu_printf("x86 %-20s  %s\n", name, desc);
+    qemu_printf("x86 %-20s  %s\n", model, desc);
 }
 
 /* list available CPU models and flags */
-- 
2.41.0

[PATCH v3 08/32] target/loongarch: Use generic helper to show CPU model names

[Gavin Shan]

For target/loongarch, the CPU type name can be: (1) the combination of
the CPU model name and suffix; (2) same to the CPU model name. The CPU
model names have been shown correctly in loongarch_cpu_list_entry()
and loongarch_cpu_add_definition() by following (1).

Use generic helper cpu_model_from_type() in above two functions to
show the CPU model names. The format of the output from cpu_list()
is also adjusted to match with other targets.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/loongarch/cpu.c                | 5 ++++-
 target/loongarch/loongarch-qmp-cmds.c | 3 +--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 65f9320e34..3ab8e4f792 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -473,14 +473,17 @@ static void loongarch_la132_initfn(Object *obj)
 static void loongarch_cpu_list_entry(gpointer data, gpointer user_data)
 {
     const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    char *model = cpu_model_from_type(typename);
 
-    qemu_printf("%s\n", typename);
+    qemu_printf("  %s\n", model);
+    g_free(model);
 }
 
 void loongarch_cpu_list(void)
 {
     GSList *list;
     list = object_class_get_list_sorted(TYPE_LOONGARCH_CPU, false);
+    qemu_printf("Available CPUs:\n");
     g_slist_foreach(list, loongarch_cpu_list_entry, NULL);
     g_slist_free(list);
 }
diff --git a/target/loongarch/loongarch-qmp-cmds.c b/target/loongarch/loongarch-qmp-cmds.c
index 6c25957881..815ceaf0ea 100644
--- a/target/loongarch/loongarch-qmp-cmds.c
+++ b/target/loongarch/loongarch-qmp-cmds.c
@@ -17,8 +17,7 @@ static void loongarch_cpu_add_definition(gpointer data, gpointer user_data)
     CpuDefinitionInfo *info = g_new0(CpuDefinitionInfo, 1);
     const char *typename = object_class_get_name(oc);
 
-    info->name = g_strndup(typename,
-                           strlen(typename) - strlen("-" TYPE_LOONGARCH_CPU));
+    info->name = cpu_model_from_type(typename);
     info->q_typename = g_strdup(typename);
 
     QAPI_LIST_PREPEND(*cpu_list, info);
-- 
2.41.0

[PATCH v3 09/32] target/m68k: Use generic helper to show CPU model names

[Gavin Shan]

For target/m68k, the CPU type name is always the combination of the
CPU model name and suffix. The CPU model names have been shown
correctly in m68k_cpu_list_entry().

Use generic helper cpu_model_from_type() to show the CPU model name
in m68k_cpu_list_entry(), rename @name to @model since it's for the
CPU model name instead of the CPU type name, and adjusted output
format to match with other targets.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/m68k/helper.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 0a1544cd68..47f2cee69a 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -49,14 +49,11 @@ static gint m68k_cpu_list_compare(gconstpointer a, gconstpointer b)
 
 static void m68k_cpu_list_entry(gpointer data, gpointer user_data)
 {
-    ObjectClass *c = data;
-    const char *typename;
-    char *name;
-
-    typename = object_class_get_name(c);
-    name = g_strndup(typename, strlen(typename) - strlen("-" TYPE_M68K_CPU));
-    qemu_printf("%s\n", name);
-    g_free(name);
+    const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    char *model = cpu_model_from_type(typename);
+
+    qemu_printf("  %s\n", model);
+    g_free(model);
 }
 
 void m68k_cpu_list(void)
@@ -65,6 +62,7 @@ void m68k_cpu_list(void)
 
     list = object_class_get_list(TYPE_M68K_CPU, false);
     list = g_slist_sort(list, m68k_cpu_list_compare);
+    qemu_printf("Available CPUs:\n");
     g_slist_foreach(list, m68k_cpu_list_entry, NULL);
     g_slist_free(list);
 }
-- 
2.41.0

[PATCH v3 10/32] target/mips: Use generic helper to show CPU model names

[Gavin Shan]

For target/mips, the CPU type name is always the combination of the
CPU model name and suffix. The CPU model names have been shown
correctly in mips_cpu_list(), which fetches the CPU model names from
the pre-defined array. It's different from other targets and lack
of flexibility.

Implement mips_cpu_list() by fetching the CPU model names from the
available CPU classes. Besides, the retrieved class needs to be
validated before it's returned in mips_cpu_class_by_name(), as other
targets do.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/mips/cpu-defs.c.inc         |  9 ---------
 target/mips/cpu.c                  | 25 ++++++++++++++++++++++++-
 target/mips/sysemu/mips-qmp-cmds.c |  3 +--
 3 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/target/mips/cpu-defs.c.inc b/target/mips/cpu-defs.c.inc
index c0c389c59a..fbf787d8ce 100644
--- a/target/mips/cpu-defs.c.inc
+++ b/target/mips/cpu-defs.c.inc
@@ -1018,15 +1018,6 @@ const mips_def_t mips_defs[] =
 };
 const int mips_defs_number = ARRAY_SIZE(mips_defs);
 
-void mips_cpu_list(void)
-{
-    int i;
-
-    for (i = 0; i < ARRAY_SIZE(mips_defs); i++) {
-        qemu_printf("MIPS '%s'\n", mips_defs[i].name);
-    }
-}
-
 static void fpu_init (CPUMIPSState *env, const mips_def_t *def)
 {
     int i;
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 63da1948fd..3431acbd99 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -532,7 +532,12 @@ static ObjectClass *mips_cpu_class_by_name(const char *cpu_model)
     typename = mips_cpu_type_name(cpu_model);
     oc = object_class_by_name(typename);
     g_free(typename);
-    return oc;
+    if (object_class_dynamic_cast(oc, TYPE_MIPS_CPU) &&
+        !object_class_is_abstract(oc)) {
+        return oc;
+    }
+
+    return NULL;
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -566,6 +571,24 @@ static const struct TCGCPUOps mips_tcg_ops = {
 };
 #endif /* CONFIG_TCG */
 
+static void mips_cpu_list_entry(gpointer data, gpointer user_data)
+{
+    const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    char *model = cpu_model_from_type(typename);
+
+    qemu_printf("  %s\n", model);
+    g_free(model);
+}
+
+void mips_cpu_list(void)
+{
+    GSList *list;
+    list = object_class_get_list_sorted(TYPE_MIPS_CPU, false);
+    qemu_printf("Available CPUs:\n");
+    g_slist_foreach(list, mips_cpu_list_entry, NULL);
+    g_slist_free(list);
+}
+
 static void mips_cpu_class_init(ObjectClass *c, void *data)
 {
     MIPSCPUClass *mcc = MIPS_CPU_CLASS(c);
diff --git a/target/mips/sysemu/mips-qmp-cmds.c b/target/mips/sysemu/mips-qmp-cmds.c
index 6db4626412..7340ac70ba 100644
--- a/target/mips/sysemu/mips-qmp-cmds.c
+++ b/target/mips/sysemu/mips-qmp-cmds.c
@@ -19,8 +19,7 @@ static void mips_cpu_add_definition(gpointer data, gpointer user_data)
 
     typename = object_class_get_name(oc);
     info = g_malloc0(sizeof(*info));
-    info->name = g_strndup(typename,
-                           strlen(typename) - strlen("-" TYPE_MIPS_CPU));
+    info->name = cpu_model_from_type(typename);
     info->q_typename = g_strdup(typename);
 
     QAPI_LIST_PREPEND(*cpu_list, info);
-- 
2.41.0

[PATCH v3 11/32] target/openrisc: Use generic helper to show CPU model names

[Gavin Shan]

For target/openrisc, the CPU type name is always the combination of
the CPU model name and suffix. The CPU model names have been correctly
shown in openrisc_cpu_list_entry().

Use generic helper cpu_model_from_type() to show the CPU model names
in openrisc_cpu_list_entry(), and @name is renamed to @model since it
points to the CPU model name instead of the CPU type name. Besides,
openrisc_cpu_class_by_name() is simplified since the condtion of
'@oc == NULL' has been covered by object_class_dynamic_cast().

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/openrisc/cpu.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index 61d748cfdc..2284c0187b 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -168,11 +168,12 @@ static ObjectClass *openrisc_cpu_class_by_name(const char *cpu_model)
     typename = g_strdup_printf(OPENRISC_CPU_TYPE_NAME("%s"), cpu_model);
     oc = object_class_by_name(typename);
     g_free(typename);
-    if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU) ||
-                       object_class_is_abstract(oc))) {
-        return NULL;
+    if (object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU) &&
+        !object_class_is_abstract(oc)) {
+        return oc;
     }
-    return oc;
+
+    return NULL;
 }
 
 static void or1200_initfn(Object *obj)
@@ -280,15 +281,11 @@ static gint openrisc_cpu_list_compare(gconstpointer a, gconstpointer b)
 
 static void openrisc_cpu_list_entry(gpointer data, gpointer user_data)
 {
-    ObjectClass *oc = data;
-    const char *typename;
-    char *name;
-
-    typename = object_class_get_name(oc);
-    name = g_strndup(typename,
-                     strlen(typename) - strlen("-" TYPE_OPENRISC_CPU));
-    qemu_printf("  %s\n", name);
-    g_free(name);
+    const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    char *model = cpu_model_from_type(typename);
+
+    qemu_printf("  %s\n", model);
+    g_free(model);
 }
 
 void cpu_openrisc_list(void)
-- 
2.41.0

[PATCH v3 12/32] target/ppc: Use generic helper to show CPU model names

[Gavin Shan]

For target/ppc, the CPU type name can be: (1) The combination of
the CPU model name and suffix; (2) the type name of the class whose
PVR matches with the specified one; (3) alias of the CPU model, plus
suffix; (4) MachineClass::default_cpu_type when the CPU model name
is "max". All the possible information, the CPU model name, aliases
of the CPU models and PVRs are all shown in ppc_cpu_list_entry().

Use generic helper cpu_model_from_type() in ppc_cpu_list_entry(),
and rename @name to @model since it points to the CPU model name
instead of the CPU type name.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/ppc/cpu_init.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 02b7aad9b0..7281402331 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7019,16 +7019,15 @@ static void ppc_cpu_list_entry(gpointer data, gpointer user_data)
     PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
     DeviceClass *family = DEVICE_CLASS(ppc_cpu_get_family_class(pcc));
     const char *typename = object_class_get_name(oc);
-    char *name;
+    char *model;
     int i;
 
     if (unlikely(strcmp(typename, TYPE_HOST_POWERPC_CPU) == 0)) {
         return;
     }
 
-    name = g_strndup(typename,
-                     strlen(typename) - strlen(POWERPC_CPU_TYPE_SUFFIX));
-    qemu_printf("PowerPC %-16s PVR %08x\n", name, pcc->pvr);
+    model = cpu_model_from_type(typename);
+    qemu_printf("PowerPC %-16s PVR %08x\n", model, pcc->pvr);
     for (i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
         PowerPCCPUAlias *alias = &ppc_cpu_aliases[i];
         ObjectClass *alias_oc = ppc_cpu_class_by_name(alias->model);
@@ -7045,10 +7044,10 @@ static void ppc_cpu_list_entry(gpointer data, gpointer user_data)
                         alias->alias, family->desc);
         } else {
             qemu_printf("PowerPC %-16s (alias for %s)\n",
-                        alias->alias, name);
+                        alias->alias, model);
         }
     }
-    g_free(name);
+    g_free(model);
 }
 
 void ppc_cpu_list(void)
-- 
2.41.0

Re: [PATCH v3 12/32] target/ppc: Use generic helper to show CPU model names

[Cédric Le Goater]

On 9/7/23 02:35, Gavin Shan wrote:
> For target/ppc, the CPU type name can be: (1) The combination of
> the CPU model name and suffix; (2) the type name of the class whose
> PVR matches with the specified one; (3) alias of the CPU model, plus
> suffix; (4) MachineClass::default_cpu_type when the CPU model name
> is "max". All the possible information, the CPU model name, aliases
> of the CPU models and PVRs are all shown in ppc_cpu_list_entry().
> 
> Use generic helper cpu_model_from_type() in ppc_cpu_list_entry(),
> and rename @name to @model since it points to the CPU model name
> instead of the CPU type name.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>

Looks good.

Reviewed-by: Cédric Le Goater <clg@kaod.org>


With a quick hack adding a valid_cpu_types list to the pseries machine,
QEMU reported:

   qemu-system-ppc64 -M pseries -cpu 460exb
   qemu-system-ppc64: Invalid CPU type: 460exb
   The valid types are: 970_v2.2, 970mp_v1.0, 970mp_v1.1, power5+_v2.1, power7_v2.3, power7+_v2.1, power8_v2.0, power8e_v2.1, power8nvl_v1.0, power9_v1.0, power9_v2.0, power9_v2.2, power10_v1.0, power10_v2.0

This would be a great improvement for the PPC boards since the CPU
definitions are numerous :

   $ install/bin/qemu-system-ppc64 -cpu ?  | wc -l
   418

and usually, the board only reports that the CPU specified on the
command line is not supported.

Thanks,

C.


> ---
>   target/ppc/cpu_init.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 02b7aad9b0..7281402331 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -7019,16 +7019,15 @@ static void ppc_cpu_list_entry(gpointer data, gpointer user_data)
>       PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
>       DeviceClass *family = DEVICE_CLASS(ppc_cpu_get_family_class(pcc));
>       const char *typename = object_class_get_name(oc);
> -    char *name;
> +    char *model;
>       int i;
>   
>       if (unlikely(strcmp(typename, TYPE_HOST_POWERPC_CPU) == 0)) {
>           return;
>       }
>   
> -    name = g_strndup(typename,
> -                     strlen(typename) - strlen(POWERPC_CPU_TYPE_SUFFIX));
> -    qemu_printf("PowerPC %-16s PVR %08x\n", name, pcc->pvr);
> +    model = cpu_model_from_type(typename);
> +    qemu_printf("PowerPC %-16s PVR %08x\n", model, pcc->pvr);
>       for (i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
>           PowerPCCPUAlias *alias = &ppc_cpu_aliases[i];
>           ObjectClass *alias_oc = ppc_cpu_class_by_name(alias->model);
> @@ -7045,10 +7044,10 @@ static void ppc_cpu_list_entry(gpointer data, gpointer user_data)
>                           alias->alias, family->desc);
>           } else {
>               qemu_printf("PowerPC %-16s (alias for %s)\n",
> -                        alias->alias, name);
> +                        alias->alias, model);
>           }
>       }
> -    g_free(name);
> +    g_free(model);
>   }
>   
>   void ppc_cpu_list(void)

[PATCH v3 13/32] target/riscv: Use generic helper to show CPU model names

[Gavin Shan]

For target/riscv, the CPU type name is always the combination of the
CPU model name and suffix. The CPU model names have been correctly
shown in riscv_cpu_list_entry() and riscv_cpu_add_definition()

Use generic helper cpu_mdoel_from_type() to show the CPU model names
in the above two functions, and adjusted format of the output from
riscv_cpu_list_entry() to match with other targets. Besides, the
function riscv_cpu_class_by_name() is improved by renaming @cpuname
to @model since it's for the CPU model name, and merging the condtion
of "@oc == NULL" to object_class_dynamic_cast().

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/riscv/cpu.c            | 23 +++++++++++++----------
 target/riscv/riscv-qmp-cmds.c |  3 +--
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 6b93b04453..a525e24c5a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -612,18 +612,19 @@ static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
 {
     ObjectClass *oc;
     char *typename;
-    char **cpuname;
+    char **model;
 
-    cpuname = g_strsplit(cpu_model, ",", 1);
-    typename = g_strdup_printf(RISCV_CPU_TYPE_NAME("%s"), cpuname[0]);
+    model = g_strsplit(cpu_model, ",", 1);
+    typename = g_strdup_printf(RISCV_CPU_TYPE_NAME("%s"), model[0]);
     oc = object_class_by_name(typename);
-    g_strfreev(cpuname);
+    g_strfreev(model);
     g_free(typename);
-    if (!oc || !object_class_dynamic_cast(oc, TYPE_RISCV_CPU) ||
-        object_class_is_abstract(oc)) {
-        return NULL;
+    if (object_class_dynamic_cast(oc, TYPE_RISCV_CPU) &&
+        !object_class_is_abstract(oc)) {
+        return oc;
     }
-    return oc;
+
+    return NULL;
 }
 
 static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
@@ -2211,9 +2212,10 @@ static gint riscv_cpu_list_compare(gconstpointer a, gconstpointer b)
 static void riscv_cpu_list_entry(gpointer data, gpointer user_data)
 {
     const char *typename = object_class_get_name(OBJECT_CLASS(data));
-    int len = strlen(typename) - strlen(RISCV_CPU_TYPE_SUFFIX);
+    char *model = cpu_model_from_type(typename);
 
-    qemu_printf("%.*s\n", len, typename);
+    qemu_printf("  %s\n", model);
+    g_free(model);
 }
 
 void riscv_cpu_list(void)
@@ -2222,6 +2224,7 @@ void riscv_cpu_list(void)
 
     list = object_class_get_list(TYPE_RISCV_CPU, false);
     list = g_slist_sort(list, riscv_cpu_list_compare);
+    qemu_printf("Available CPUs:\n");
     g_slist_foreach(list, riscv_cpu_list_entry, NULL);
     g_slist_free(list);
 }
diff --git a/target/riscv/riscv-qmp-cmds.c b/target/riscv/riscv-qmp-cmds.c
index 5ecff1afb3..22f728673f 100644
--- a/target/riscv/riscv-qmp-cmds.c
+++ b/target/riscv/riscv-qmp-cmds.c
@@ -35,8 +35,7 @@ static void riscv_cpu_add_definition(gpointer data, gpointer user_data)
     const char *typename = object_class_get_name(oc);
     ObjectClass *dyn_class;
 
-    info->name = g_strndup(typename,
-                           strlen(typename) - strlen("-" TYPE_RISCV_CPU));
+    info->name = cpu_model_from_type(typename);
     info->q_typename = g_strdup(typename);
 
     dyn_class = object_class_dynamic_cast(oc, TYPE_RISCV_DYNAMIC_CPU);
-- 
2.41.0

Re: [PATCH v3 13/32] target/riscv: Use generic helper to show CPU model names

[Daniel Henrique Barboza]

On 9/6/23 21:35, Gavin Shan wrote:
> For target/riscv, the CPU type name is always the combination of the
> CPU model name and suffix. The CPU model names have been correctly
> shown in riscv_cpu_list_entry() and riscv_cpu_add_definition()
> 
> Use generic helper cpu_mdoel_from_type() to show the CPU model names
> in the above two functions, and adjusted format of the output from
> riscv_cpu_list_entry() to match with other targets. Besides, the
> function riscv_cpu_class_by_name() is improved by renaming @cpuname
> to @model since it's for the CPU model name, and merging the condtion

typo: "condition"

> of "@oc == NULL" to object_class_dynamic_cast().
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---


Tested with "-cpu help" and "query-cpu-definitions". LGTM


Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>


>   target/riscv/cpu.c            | 23 +++++++++++++----------
>   target/riscv/riscv-qmp-cmds.c |  3 +--
>   2 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 6b93b04453..a525e24c5a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -612,18 +612,19 @@ static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
>   {
>       ObjectClass *oc;
>       char *typename;
> -    char **cpuname;
> +    char **model;
>   
> -    cpuname = g_strsplit(cpu_model, ",", 1);
> -    typename = g_strdup_printf(RISCV_CPU_TYPE_NAME("%s"), cpuname[0]);
> +    model = g_strsplit(cpu_model, ",", 1);
> +    typename = g_strdup_printf(RISCV_CPU_TYPE_NAME("%s"), model[0]);
>       oc = object_class_by_name(typename);
> -    g_strfreev(cpuname);
> +    g_strfreev(model);
>       g_free(typename);
> -    if (!oc || !object_class_dynamic_cast(oc, TYPE_RISCV_CPU) ||
> -        object_class_is_abstract(oc)) {
> -        return NULL;
> +    if (object_class_dynamic_cast(oc, TYPE_RISCV_CPU) &&
> +        !object_class_is_abstract(oc)) {
> +        return oc;
>       }
> -    return oc;
> +
> +    return NULL;
>   }
>   
>   static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> @@ -2211,9 +2212,10 @@ static gint riscv_cpu_list_compare(gconstpointer a, gconstpointer b)
>   static void riscv_cpu_list_entry(gpointer data, gpointer user_data)
>   {
>       const char *typename = object_class_get_name(OBJECT_CLASS(data));
> -    int len = strlen(typename) - strlen(RISCV_CPU_TYPE_SUFFIX);
> +    char *model = cpu_model_from_type(typename);
>   
> -    qemu_printf("%.*s\n", len, typename);
> +    qemu_printf("  %s\n", model);
> +    g_free(model);
>   }
>   
>   void riscv_cpu_list(void)
> @@ -2222,6 +2224,7 @@ void riscv_cpu_list(void)
>   
>       list = object_class_get_list(TYPE_RISCV_CPU, false);
>       list = g_slist_sort(list, riscv_cpu_list_compare);
> +    qemu_printf("Available CPUs:\n");
>       g_slist_foreach(list, riscv_cpu_list_entry, NULL);
>       g_slist_free(list);
>   }
> diff --git a/target/riscv/riscv-qmp-cmds.c b/target/riscv/riscv-qmp-cmds.c
> index 5ecff1afb3..22f728673f 100644
> --- a/target/riscv/riscv-qmp-cmds.c
> +++ b/target/riscv/riscv-qmp-cmds.c
> @@ -35,8 +35,7 @@ static void riscv_cpu_add_definition(gpointer data, gpointer user_data)
>       const char *typename = object_class_get_name(oc);
>       ObjectClass *dyn_class;
>   
> -    info->name = g_strndup(typename,
> -                           strlen(typename) - strlen("-" TYPE_RISCV_CPU));
> +    info->name = cpu_model_from_type(typename);
>       info->q_typename = g_strdup(typename);
>   
>       dyn_class = object_class_dynamic_cast(oc, TYPE_RISCV_DYNAMIC_CPU);

[PATCH v3 14/32] target/rx: Use generic helper to show CPU model names

[Gavin Shan]

For target/rx, the CPU type name can be: (1) the combination of the
CPU model name and suffix; (2) same to the CPU model name. The CPU
type names have been shown in rx_cpu_list_entry().

Use generic helper cpu_model_from_type() to show the CPU model names
in rx_cpu_list_entry(). Besides, rx_cpu_class_by_name() is improved
by merging the condition of '@oc == NULL' to object_class_dynamic_cast().

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/rx/cpu.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/target/rx/cpu.c b/target/rx/cpu.c
index 157e57da0f..ff0ced1f3d 100644
--- a/target/rx/cpu.c
+++ b/target/rx/cpu.c
@@ -91,9 +91,11 @@ static void rx_cpu_reset_hold(Object *obj)
 
 static void rx_cpu_list_entry(gpointer data, gpointer user_data)
 {
-    ObjectClass *oc = data;
+    const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    char *model = cpu_model_from_type(typename);
 
-    qemu_printf("  %s\n", object_class_get_name(oc));
+    qemu_printf("  %s\n", model);
+    g_free(model);
 }
 
 void rx_cpu_list(void)
@@ -111,18 +113,20 @@ static ObjectClass *rx_cpu_class_by_name(const char *cpu_model)
     char *typename;
 
     oc = object_class_by_name(cpu_model);
-    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_RX_CPU) != NULL &&
+    if (object_class_dynamic_cast(oc, TYPE_RX_CPU) &&
         !object_class_is_abstract(oc)) {
         return oc;
     }
+
     typename = g_strdup_printf(RX_CPU_TYPE_NAME("%s"), cpu_model);
     oc = object_class_by_name(typename);
     g_free(typename);
-    if (oc != NULL && object_class_is_abstract(oc)) {
-        oc = NULL;
+    if (object_class_dynamic_cast(oc, TYPE_RX_CPU) &&
+        !object_class_is_abstract(oc)) {
+        return oc;
     }
 
-    return oc;
+    return NULL;
 }
 
 static void rx_cpu_realize(DeviceState *dev, Error **errp)
-- 
2.41.0

[PATCH v3 15/32] target/s390x: Use generic helper to show CPU model names

[Gavin Shan]

For target/s390x, the CPU type name is always the combination of the
CPU modle name and suffix. The CPU model names have been correctly
shown in s390_print_cpu_model_list_entry() and create_cpu_model_list().

Use generic helper cpu_model_from_type() to show the CPU model names
in the above two functions. Besides, we need validate the CPU class
in s390_cpu_class_by_name(), as other targets do.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/s390x/cpu_models.c        | 18 +++++++++++-------
 target/s390x/cpu_models_sysemu.c |  9 ++++-----
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 91ce896491..103e9072b8 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -338,7 +338,8 @@ static void s390_print_cpu_model_list_entry(gpointer data, gpointer user_data)
 {
     const S390CPUClass *scc = S390_CPU_CLASS((ObjectClass *)data);
     CPUClass *cc = CPU_CLASS(scc);
-    char *name = g_strdup(object_class_get_name((ObjectClass *)data));
+    const char *typename = object_class_get_name((ObjectClass *)data);
+    char *model = cpu_model_from_type(typename);
     g_autoptr(GString) details = g_string_new("");
 
     if (scc->is_static) {
@@ -355,14 +356,12 @@ static void s390_print_cpu_model_list_entry(gpointer data, gpointer user_data)
         g_string_truncate(details, details->len - 2);
     }
 
-    /* strip off the -s390x-cpu */
-    g_strrstr(name, "-" TYPE_S390_CPU)[0] = 0;
     if (details->len) {
-        qemu_printf("s390 %-15s %-35s (%s)\n", name, scc->desc, details->str);
+        qemu_printf("s390 %-15s %-35s (%s)\n", model, scc->desc, details->str);
     } else {
-        qemu_printf("s390 %-15s %-35s\n", name, scc->desc);
+        qemu_printf("s390 %-15s %-35s\n", model, scc->desc);
     }
-    g_free(name);
+    g_free(model);
 }
 
 static gint s390_cpu_list_compare(gconstpointer a, gconstpointer b)
@@ -916,7 +915,12 @@ ObjectClass *s390_cpu_class_by_name(const char *name)
 
     oc = object_class_by_name(typename);
     g_free(typename);
-    return oc;
+    if (object_class_dynamic_cast(oc, TYPE_S390_CPU) &&
+        !object_class_is_abstract(oc)) {
+        return oc;
+    }
+
+    return NULL;
 }
 
 static const TypeInfo qemu_s390_cpu_type_info = {
diff --git a/target/s390x/cpu_models_sysemu.c b/target/s390x/cpu_models_sysemu.c
index 63981bf36b..c41af253d3 100644
--- a/target/s390x/cpu_models_sysemu.c
+++ b/target/s390x/cpu_models_sysemu.c
@@ -55,17 +55,16 @@ static void create_cpu_model_list(ObjectClass *klass, void *opaque)
     struct CpuDefinitionInfoListData *cpu_list_data = opaque;
     CpuDefinitionInfoList **cpu_list = &cpu_list_data->list;
     CpuDefinitionInfo *info;
-    char *name = g_strdup(object_class_get_name(klass));
+    const char *typename = object_class_get_name(klass);
+    char *model = cpu_model_from_type(typename);
     S390CPUClass *scc = S390_CPU_CLASS(klass);
 
-    /* strip off the -s390x-cpu */
-    g_strrstr(name, "-" TYPE_S390_CPU)[0] = 0;
     info = g_new0(CpuDefinitionInfo, 1);
-    info->name = name;
+    info->name = model;
     info->has_migration_safe = true;
     info->migration_safe = scc->is_migration_safe;
     info->q_static = scc->is_static;
-    info->q_typename = g_strdup(object_class_get_name(klass));
+    info->q_typename = g_strdup(typename);
     /* check for unavailable features */
     if (cpu_list_data->model) {
         Object *obj;
-- 
2.41.0

Re: [PATCH v3 15/32] target/s390x: Use generic helper to show CPU model names

[David Hildenbrand]

On 07.09.23 02:35, Gavin Shan wrote:
> For target/s390x, the CPU type name is always the combination of the
> CPU modle name and suffix. The CPU model names have been correctly
> shown in s390_print_cpu_model_list_entry() and create_cpu_model_list().
> 
> Use generic helper cpu_model_from_type() to show the CPU model names
> in the above two functions. Besides, we need validate the CPU class
> in s390_cpu_class_by_name(), as other targets do.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>   target/s390x/cpu_models.c        | 18 +++++++++++-------
>   target/s390x/cpu_models_sysemu.c |  9 ++++-----
>   2 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 91ce896491..103e9072b8 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -338,7 +338,8 @@ static void s390_print_cpu_model_list_entry(gpointer data, gpointer user_data)
>   {
>       const S390CPUClass *scc = S390_CPU_CLASS((ObjectClass *)data);
>       CPUClass *cc = CPU_CLASS(scc);
> -    char *name = g_strdup(object_class_get_name((ObjectClass *)data));
> +    const char *typename = object_class_get_name((ObjectClass *)data);
> +    char *model = cpu_model_from_type(typename);
>       g_autoptr(GString) details = g_string_new("");
>   
>       if (scc->is_static) {
> @@ -355,14 +356,12 @@ static void s390_print_cpu_model_list_entry(gpointer data, gpointer user_data)
>           g_string_truncate(details, details->len - 2);
>       }
>   
> -    /* strip off the -s390x-cpu */
> -    g_strrstr(name, "-" TYPE_S390_CPU)[0] = 0;
>       if (details->len) {
> -        qemu_printf("s390 %-15s %-35s (%s)\n", name, scc->desc, details->str);
> +        qemu_printf("s390 %-15s %-35s (%s)\n", model, scc->desc, details->str);
>       } else {
> -        qemu_printf("s390 %-15s %-35s\n", name, scc->desc);
> +        qemu_printf("s390 %-15s %-35s\n", model, scc->desc);
>       }
> -    g_free(name);
> +    g_free(model);
>   }
>   
>   static gint s390_cpu_list_compare(gconstpointer a, gconstpointer b)
> @@ -916,7 +915,12 @@ ObjectClass *s390_cpu_class_by_name(const char *name)
>   
>       oc = object_class_by_name(typename);
>       g_free(typename);
> -    return oc;
> +    if (object_class_dynamic_cast(oc, TYPE_S390_CPU) &&
> +        !object_class_is_abstract(oc)) {
> +        return oc;
> +    }
> +
> +    return NULL;

Why is that change required?

-- 
Cheers,

David / dhildenb

Re: [PATCH v3 15/32] target/s390x: Use generic helper to show CPU model names

[Gavin Shan]

On 9/7/23 18:20, David Hildenbrand wrote:
> On 07.09.23 02:35, Gavin Shan wrote:
>> For target/s390x, the CPU type name is always the combination of the
>> CPU modle name and suffix. The CPU model names have been correctly
>> shown in s390_print_cpu_model_list_entry() and create_cpu_model_list().
>>
>> Use generic helper cpu_model_from_type() to show the CPU model names
>> in the above two functions. Besides, we need validate the CPU class
>> in s390_cpu_class_by_name(), as other targets do.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   target/s390x/cpu_models.c        | 18 +++++++++++-------
>>   target/s390x/cpu_models_sysemu.c |  9 ++++-----
>>   2 files changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>> index 91ce896491..103e9072b8 100644
>> --- a/target/s390x/cpu_models.c
>> +++ b/target/s390x/cpu_models.c
>> @@ -338,7 +338,8 @@ static void s390_print_cpu_model_list_entry(gpointer data, gpointer user_data)
>>   {
>>       const S390CPUClass *scc = S390_CPU_CLASS((ObjectClass *)data);
>>       CPUClass *cc = CPU_CLASS(scc);
>> -    char *name = g_strdup(object_class_get_name((ObjectClass *)data));
>> +    const char *typename = object_class_get_name((ObjectClass *)data);
>> +    char *model = cpu_model_from_type(typename);
>>       g_autoptr(GString) details = g_string_new("");
>>       if (scc->is_static) {
>> @@ -355,14 +356,12 @@ static void s390_print_cpu_model_list_entry(gpointer data, gpointer user_data)
>>           g_string_truncate(details, details->len - 2);
>>       }
>> -    /* strip off the -s390x-cpu */
>> -    g_strrstr(name, "-" TYPE_S390_CPU)[0] = 0;
>>       if (details->len) {
>> -        qemu_printf("s390 %-15s %-35s (%s)\n", name, scc->desc, details->str);
>> +        qemu_printf("s390 %-15s %-35s (%s)\n", model, scc->desc, details->str);
>>       } else {
>> -        qemu_printf("s390 %-15s %-35s\n", name, scc->desc);
>> +        qemu_printf("s390 %-15s %-35s\n", model, scc->desc);
>>       }
>> -    g_free(name);
>> +    g_free(model);
>>   }
>>   static gint s390_cpu_list_compare(gconstpointer a, gconstpointer b)
>> @@ -916,7 +915,12 @@ ObjectClass *s390_cpu_class_by_name(const char *name)
>>       oc = object_class_by_name(typename);
>>       g_free(typename);
>> -    return oc;
>> +    if (object_class_dynamic_cast(oc, TYPE_S390_CPU) &&
>> +        !object_class_is_abstract(oc)) {
>> +        return oc;
>> +    }
>> +
>> +    return NULL;
> 
> Why is that change required?
> 

Good question. It's possible that other class's name conflicts with
CPU class's name. For example, class "abc-base-s390x-cpu" has been
registered for a misc class other than a CPU class. We don't want
s390_cpu_class_by_name() return the class for "abc-base-s390x-cpu".
Almost all other target does similar check.

Thanks,
Gavin

Re: [PATCH v3 15/32] target/s390x: Use generic helper to show CPU model names

[Philippe Mathieu-Daudé]

On 8/9/23 01:44, Gavin Shan wrote:
> 
> On 9/7/23 18:20, David Hildenbrand wrote:
>> On 07.09.23 02:35, Gavin Shan wrote:
>>> For target/s390x, the CPU type name is always the combination of the
>>> CPU modle name and suffix. The CPU model names have been correctly
>>> shown in s390_print_cpu_model_list_entry() and create_cpu_model_list().
>>>
>>> Use generic helper cpu_model_from_type() to show the CPU model names
>>> in the above two functions. Besides, we need validate the CPU class
>>> in s390_cpu_class_by_name(), as other targets do.
>>>
>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>> ---
>>>   target/s390x/cpu_models.c        | 18 +++++++++++-------
>>>   target/s390x/cpu_models_sysemu.c |  9 ++++-----
>>>   2 files changed, 15 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>>> index 91ce896491..103e9072b8 100644
>>> --- a/target/s390x/cpu_models.c
>>> +++ b/target/s390x/cpu_models.c
>>> @@ -338,7 +338,8 @@ static void 
>>> s390_print_cpu_model_list_entry(gpointer data, gpointer user_data)
>>>   {
>>>       const S390CPUClass *scc = S390_CPU_CLASS((ObjectClass *)data);
>>>       CPUClass *cc = CPU_CLASS(scc);
>>> -    char *name = g_strdup(object_class_get_name((ObjectClass *)data));
>>> +    const char *typename = object_class_get_name((ObjectClass *)data);
>>> +    char *model = cpu_model_from_type(typename);
>>>       g_autoptr(GString) details = g_string_new("");
>>>       if (scc->is_static) {
>>> @@ -355,14 +356,12 @@ static void 
>>> s390_print_cpu_model_list_entry(gpointer data, gpointer user_data)
>>>           g_string_truncate(details, details->len - 2);
>>>       }
>>> -    /* strip off the -s390x-cpu */
>>> -    g_strrstr(name, "-" TYPE_S390_CPU)[0] = 0;
>>>       if (details->len) {
>>> -        qemu_printf("s390 %-15s %-35s (%s)\n", name, scc->desc, 
>>> details->str);
>>> +        qemu_printf("s390 %-15s %-35s (%s)\n", model, scc->desc, 
>>> details->str);
>>>       } else {
>>> -        qemu_printf("s390 %-15s %-35s\n", name, scc->desc);
>>> +        qemu_printf("s390 %-15s %-35s\n", model, scc->desc);
>>>       }
>>> -    g_free(name);
>>> +    g_free(model);
>>>   }
>>>   static gint s390_cpu_list_compare(gconstpointer a, gconstpointer b)
>>> @@ -916,7 +915,12 @@ ObjectClass *s390_cpu_class_by_name(const char 
>>> *name)
>>>       oc = object_class_by_name(typename);
>>>       g_free(typename);
>>> -    return oc;
>>> +    if (object_class_dynamic_cast(oc, TYPE_S390_CPU) &&
>>> +        !object_class_is_abstract(oc)) {
>>> +        return oc;
>>> +    }
>>> +
>>> +    return NULL;
>>
>> Why is that change required?
>>
> 
> Good question. It's possible that other class's name conflicts with
> CPU class's name. For example, class "abc-base-s390x-cpu" has been
> registered for a misc class other than a CPU class. We don't want
> s390_cpu_class_by_name() return the class for "abc-base-s390x-cpu".
> Almost all other target does similar check.

I agree with David there is some code smell here.

IMO this check belong to cpu_class_by_name(), not to each impl.

Re: [PATCH v3 15/32] target/s390x: Use generic helper to show CPU model names

[Philippe Mathieu-Daudé]

On 8/9/23 10:04, Philippe Mathieu-Daudé wrote:
> On 8/9/23 01:44, Gavin Shan wrote:
>>
>> On 9/7/23 18:20, David Hildenbrand wrote:
>>> On 07.09.23 02:35, Gavin Shan wrote:
>>>> For target/s390x, the CPU type name is always the combination of the
>>>> CPU modle name and suffix. The CPU model names have been correctly
>>>> shown in s390_print_cpu_model_list_entry() and create_cpu_model_list().
>>>>
>>>> Use generic helper cpu_model_from_type() to show the CPU model names
>>>> in the above two functions. Besides, we need validate the CPU class
>>>> in s390_cpu_class_by_name(), as other targets do.
>>>>
>>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>>> ---
>>>>   target/s390x/cpu_models.c        | 18 +++++++++++-------
>>>>   target/s390x/cpu_models_sysemu.c |  9 ++++-----
>>>>   2 files changed, 15 insertions(+), 12 deletions(-)


>>>>   static gint s390_cpu_list_compare(gconstpointer a, gconstpointer b)
>>>> @@ -916,7 +915,12 @@ ObjectClass *s390_cpu_class_by_name(const char 
>>>> *name)
>>>>       oc = object_class_by_name(typename);
>>>>       g_free(typename);
>>>> -    return oc;
>>>> +    if (object_class_dynamic_cast(oc, TYPE_S390_CPU) &&
>>>> +        !object_class_is_abstract(oc)) {
>>>> +        return oc;
>>>> +    }
>>>> +
>>>> +    return NULL;
>>>
>>> Why is that change required?
>>>
>>
>> Good question. It's possible that other class's name conflicts with
>> CPU class's name. For example, class "abc-base-s390x-cpu" has been
>> registered for a misc class other than a CPU class. We don't want
>> s390_cpu_class_by_name() return the class for "abc-base-s390x-cpu".
>> Almost all other target does similar check.
> 
> I agree with David there is some code smell here.
> 
> IMO this check belong to cpu_class_by_name(), not to each impl.

Suggestion implemented here:
https://lore.kernel.org/qemu-devel/20230908112235.75914-1-philmd@linaro.org/

Re: [PATCH v3 15/32] target/s390x: Use generic helper to show CPU model names

[Gavin Shan]

On 9/8/23 21:23, Philippe Mathieu-Daudé wrote:
> On 8/9/23 10:04, Philippe Mathieu-Daudé wrote:
>> On 8/9/23 01:44, Gavin Shan wrote:
>>>
>>> On 9/7/23 18:20, David Hildenbrand wrote:
>>>> On 07.09.23 02:35, Gavin Shan wrote:
>>>>> For target/s390x, the CPU type name is always the combination of the
>>>>> CPU modle name and suffix. The CPU model names have been correctly
>>>>> shown in s390_print_cpu_model_list_entry() and create_cpu_model_list().
>>>>>
>>>>> Use generic helper cpu_model_from_type() to show the CPU model names
>>>>> in the above two functions. Besides, we need validate the CPU class
>>>>> in s390_cpu_class_by_name(), as other targets do.
>>>>>
>>>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>>>> ---
>>>>>   target/s390x/cpu_models.c        | 18 +++++++++++-------
>>>>>   target/s390x/cpu_models_sysemu.c |  9 ++++-----
>>>>>   2 files changed, 15 insertions(+), 12 deletions(-)
> 
> 
>>>>>   static gint s390_cpu_list_compare(gconstpointer a, gconstpointer b)
>>>>> @@ -916,7 +915,12 @@ ObjectClass *s390_cpu_class_by_name(const char *name)
>>>>>       oc = object_class_by_name(typename);
>>>>>       g_free(typename);
>>>>> -    return oc;
>>>>> +    if (object_class_dynamic_cast(oc, TYPE_S390_CPU) &&
>>>>> +        !object_class_is_abstract(oc)) {
>>>>> +        return oc;
>>>>> +    }
>>>>> +
>>>>> +    return NULL;
>>>>
>>>> Why is that change required?
>>>>
>>>
>>> Good question. It's possible that other class's name conflicts with
>>> CPU class's name. For example, class "abc-base-s390x-cpu" has been
>>> registered for a misc class other than a CPU class. We don't want
>>> s390_cpu_class_by_name() return the class for "abc-base-s390x-cpu".
>>> Almost all other target does similar check.
>>
>> I agree with David there is some code smell here.
>>
>> IMO this check belong to cpu_class_by_name(), not to each impl.
> 
> Suggestion implemented here:
> https://lore.kernel.org/qemu-devel/20230908112235.75914-1-philmd@linaro.org/
> 

Right. That is better way to consolidate the checks. I've provided my
comments for your code changes. I believe I need to rebase my v4 series
on top your changes. Philippe, please let me know if I need include your
patches to my v4 series, modify the code accordingly and send them together
for review.

Thanks,
Gavin

Re: [PATCH v3 15/32] target/s390x: Use generic helper to show CPU model names

[Thomas Huth]

On 07/09/2023 02.35, Gavin Shan wrote:
> For target/s390x, the CPU type name is always the combination of the
> CPU modle name and suffix. The CPU model names have been correctly

s/modle/model/

  Thomas

Re: [PATCH v3 15/32] target/s390x: Use generic helper to show CPU model names

[Gavin Shan]

On 9/7/23 18:31, Thomas Huth wrote:
> On 07/09/2023 02.35, Gavin Shan wrote:
>> For target/s390x, the CPU type name is always the combination of the
>> CPU modle name and suffix. The CPU model names have been correctly
> 
> s/modle/model/
> 

Thanks, will be fixed in next respin.

Thanks,
Gavin

[PATCH v3 16/32] target/sh4: Use generic helper to show CPU model names

[Gavin Shan]

For target/sh4, the CPU type name can be: (1) the combination of the
CPU model name and suffix; (2) TYPE_SH7750R_CPU when the CPU model
name is "any". The CPU model names have been correctly shown in
superh_cpu_list_entry().

Use generic helper cpu_model_from_type() to show the CPU model name
in the above function. Besides, superh_cpu_class_by_name() is improved
by avoiding "goto out" and validating the CPU class.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/sh4/cpu.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index 61769ffdfa..ca06e2ce99 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -125,9 +125,10 @@ static void superh_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
 static void superh_cpu_list_entry(gpointer data, gpointer user_data)
 {
     const char *typename = object_class_get_name(OBJECT_CLASS(data));
-    int len = strlen(typename) - strlen(SUPERH_CPU_TYPE_SUFFIX);
+    char *model = cpu_model_from_type(typename);
 
-    qemu_printf("%.*s\n", len, typename);
+    qemu_printf("  %s\n", model);
+    g_free(model);
 }
 
 void sh4_cpu_list(void)
@@ -135,6 +136,7 @@ void sh4_cpu_list(void)
     GSList *list;
 
     list = object_class_get_list_sorted(TYPE_SUPERH_CPU, false);
+    qemu_printf("Available CPUs:\n");
     g_slist_foreach(list, superh_cpu_list_entry, NULL);
     g_slist_free(list);
 }
@@ -146,20 +148,20 @@ static ObjectClass *superh_cpu_class_by_name(const char *cpu_model)
 
     s = g_ascii_strdown(cpu_model, -1);
     if (strcmp(s, "any") == 0) {
-        oc = object_class_by_name(TYPE_SH7750R_CPU);
-        goto out;
+        typename = g_strdup(TYPE_SH7750R_CPU);
+    } else {
+        typename = g_strdup_printf(SUPERH_CPU_TYPE_NAME("%s"), s);
     }
 
-    typename = g_strdup_printf(SUPERH_CPU_TYPE_NAME("%s"), s);
     oc = object_class_by_name(typename);
-    if (oc != NULL && object_class_is_abstract(oc)) {
-        oc = NULL;
-    }
-
-out:
     g_free(s);
     g_free(typename);
-    return oc;
+    if (object_class_dynamic_cast(oc, TYPE_SUPERH_CPU) &&
+        !object_class_is_abstract(oc)) {
+        return oc;
+    }
+
+    return NULL;
 }
 
 static void sh7750r_cpu_initfn(Object *obj)
-- 
2.41.0

[PATCH v3 17/32] target/tricore: Use generic helper to show CPU model names

[Gavin Shan]

For target/tricore, the CPU type name is always the combination of the
CPU model name and suffix. The CPU model names have been correctly
shown in tricore_cpu_list_entry().

Use generic helper cpu_model_from_type() to show the CPU model names
in the above function. tricore_cpu_class_by_name() is also improved
by merging the condition of '@oc == NULL' to object_class_dynamic_cast().

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/tricore/cpu.c    |  9 +++++----
 target/tricore/helper.c | 13 +++++--------
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
index 133a9ac70e..066249e50d 100644
--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -140,11 +140,12 @@ static ObjectClass *tricore_cpu_class_by_name(const char *cpu_model)
     typename = g_strdup_printf(TRICORE_CPU_TYPE_NAME("%s"), cpu_model);
     oc = object_class_by_name(typename);
     g_free(typename);
-    if (!oc || !object_class_dynamic_cast(oc, TYPE_TRICORE_CPU) ||
-        object_class_is_abstract(oc)) {
-        return NULL;
+    if (object_class_dynamic_cast(oc, TYPE_TRICORE_CPU) &&
+        !object_class_is_abstract(oc)) {
+        return oc;
     }
-    return oc;
+
+    return NULL;
 }
 
 static void tc1796_initfn(Object *obj)
diff --git a/target/tricore/helper.c b/target/tricore/helper.c
index 6d076ac36f..21f4e1f1a3 100644
--- a/target/tricore/helper.c
+++ b/target/tricore/helper.c
@@ -98,14 +98,11 @@ bool tricore_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
 
 static void tricore_cpu_list_entry(gpointer data, gpointer user_data)
 {
-    ObjectClass *oc = data;
-    const char *typename;
-    char *name;
-
-    typename = object_class_get_name(oc);
-    name = g_strndup(typename, strlen(typename) - strlen("-" TYPE_TRICORE_CPU));
-    qemu_printf("  %s\n", name);
-    g_free(name);
+    const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    char *model = cpu_model_from_type(typename);
+
+    qemu_printf("  %s\n", model);
+    g_free(model);
 }
 
 void tricore_cpu_list(void)
-- 
2.41.0

Re: [PATCH v3 17/32] target/tricore: Use generic helper to show CPU model names

[Bastian Koppelmann]

On Thu, Sep 07, 2023 at 10:35:38AM +1000, Gavin Shan wrote:
> For target/tricore, the CPU type name is always the combination of the
> CPU model name and suffix. The CPU model names have been correctly
> shown in tricore_cpu_list_entry().
> 
> Use generic helper cpu_model_from_type() to show the CPU model names
> in the above function. tricore_cpu_class_by_name() is also improved
> by merging the condition of '@oc == NULL' to object_class_dynamic_cast().
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>  target/tricore/cpu.c    |  9 +++++----
>  target/tricore/helper.c | 13 +++++--------
>  2 files changed, 10 insertions(+), 12 deletions(-)

Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

Cheers,
Bastian

[PATCH v3 18/32] target/sparc: Improve sparc_cpu_class_by_name()

[Gavin Shan]

Improve sparc_cpu_class_by_name() by validating @oc, to ensure it's
child of TYPE_SPARC_CPU since it's possible for other types of classes
to have TYPE_SPARC_CPU as the suffix of their type names.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/sparc/cpu.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 130ab8f578..20417707da 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -745,7 +745,12 @@ static ObjectClass *sparc_cpu_class_by_name(const char *cpu_model)
     typename = sparc_cpu_type_name(cpu_model);
     oc = object_class_by_name(typename);
     g_free(typename);
-    return oc;
+    if (object_class_dynamic_cast(oc, TYPE_SPARC_CPU) &&
+        !object_class_is_abstract(oc)) {
+        return oc;
+    }
+
+    return NULL;
 }
 
 static void sparc_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.41.0

[PATCH v3 19/32] target/xtensa: Improve xtensa_cpu_class_by_name()

[Gavin Shan]

Improve xtensa_cpu_class_by_name() by merging the condition of
'@oc == NULL' to object_class_dynamic_cast().

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/xtensa/cpu.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index acaf8c905f..9d682611aa 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -141,11 +141,12 @@ static ObjectClass *xtensa_cpu_class_by_name(const char *cpu_model)
     typename = g_strdup_printf(XTENSA_CPU_TYPE_NAME("%s"), cpu_model);
     oc = object_class_by_name(typename);
     g_free(typename);
-    if (oc == NULL || !object_class_dynamic_cast(oc, TYPE_XTENSA_CPU) ||
-        object_class_is_abstract(oc)) {
-        return NULL;
+    if (object_class_dynamic_cast(oc, TYPE_XTENSA_CPU) &&
+        !object_class_is_abstract(oc)) {
+        return oc;
     }
-    return oc;
+
+    return NULL;
 }
 
 static void xtensa_cpu_disas_set_info(CPUState *cs, disassemble_info *info)
-- 
2.41.0

[PATCH v3 20/32] target/hppa: Implement hppa_cpu_list()

[Gavin Shan]

Implement hppa_cpu_list() to support cpu_list(). With this applied,
the available CPU model names, same to the CPU type names, are shown
as below.

  $ ./build/qemu-system-hppa -cpu ?
  Available CPUs:
    hppa-cpu

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/hppa/cpu.c | 19 +++++++++++++++++++
 target/hppa/cpu.h |  3 +++
 2 files changed, 22 insertions(+)

diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 11022f9c99..873402bf9c 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -143,6 +143,25 @@ static void hppa_cpu_realizefn(DeviceState *dev, Error **errp)
 #endif
 }
 
+static void hppa_cpu_list_entry(gpointer data, gpointer user_data)
+{
+    const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    char *model = cpu_model_from_type(typename);
+
+    qemu_printf("  %s\n", model);
+    g_free(model);
+}
+
+void hppa_cpu_list(void)
+{
+    GSList *list;
+
+    list = object_class_get_list_sorted(TYPE_HPPA_CPU, false);
+    qemu_printf("Available CPUs:\n");
+    g_slist_foreach(list, hppa_cpu_list_entry, NULL);
+    g_slist_free(list);
+}
+
 static void hppa_cpu_initfn(Object *obj)
 {
     CPUState *cs = CPU(obj);
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index fa13694dab..19759f5f62 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -351,5 +351,8 @@ void hppa_cpu_alarm_timer(void *);
 int hppa_artype_for_page(CPUHPPAState *env, target_ulong vaddr);
 #endif
 G_NORETURN void hppa_dynamic_excp(CPUHPPAState *env, int excp, uintptr_t ra);
+void hppa_cpu_list(void);
+
+#define cpu_list hppa_cpu_list
 
 #endif /* HPPA_CPU_H */
-- 
2.41.0

[PATCH v3 21/32] target/microblaze: Implement microblaze_cpu_list()

[Gavin Shan]

Implement microblaze_cpu_list() to support cpu_list(). With this applied,
the available CPU model names, same to the CPU type names, are shown
as below.

  $ ./build/qemu-system-hppa -cpu ?
  Available CPUs:
    microblaze-cpu

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/microblaze/cpu.c | 20 ++++++++++++++++++++
 target/microblaze/cpu.h |  3 +++
 2 files changed, 23 insertions(+)

diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 03c2c4db1f..fc7a5dee5b 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -24,6 +24,7 @@
 #include "qemu/osdep.h"
 #include "qemu/log.h"
 #include "qapi/error.h"
+#include "qemu/qemu-print.h"
 #include "cpu.h"
 #include "qemu/module.h"
 #include "hw/qdev-properties.h"
@@ -291,6 +292,25 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
     mcc->parent_realize(dev, errp);
 }
 
+static void microblaze_cpu_list_entry(gpointer data, gpointer user_data)
+{
+    const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    char *model = cpu_model_from_type(typename);
+
+    qemu_printf("  %s\n", model);
+    g_free(model);
+}
+
+void microblaze_cpu_list(void)
+{
+    GSList *list;
+
+    list = object_class_get_list_sorted(TYPE_MICROBLAZE_CPU, false);
+    qemu_printf("Available CPUs:\n");
+    g_slist_foreach(list, microblaze_cpu_list_entry, NULL);
+    g_slist_free(list);
+}
+
 static void mb_cpu_initfn(Object *obj)
 {
     MicroBlazeCPU *cpu = MICROBLAZE_CPU(obj);
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index f6cab6ce19..b5775c2966 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -372,6 +372,9 @@ int mb_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int mb_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 int mb_cpu_gdb_read_stack_protect(CPUArchState *cpu, GByteArray *buf, int reg);
 int mb_cpu_gdb_write_stack_protect(CPUArchState *cpu, uint8_t *buf, int reg);
+void microblaze_cpu_list(void);
+
+#define cpu_list microblaze_cpu_list
 
 static inline uint32_t mb_cpu_read_msr(const CPUMBState *env)
 {
-- 
2.41.0

[PATCH v3 22/32] target/nios2: Implement nios2_cpu_list()

[Gavin Shan]

Implement nios2_cpu_list() to support cpu_list(). With this applied,
the available CPU model names, same to the CPU type names, are shown
as below.

  $ ./build/qemu-system-nios2 -cpu ?
  Available CPUs:
    nios2-cpu

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/nios2/cpu.c | 20 ++++++++++++++++++++
 target/nios2/cpu.h |  3 +++
 2 files changed, 23 insertions(+)

diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
index bc5cbf81c2..80af24eb69 100644
--- a/target/nios2/cpu.c
+++ b/target/nios2/cpu.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "qemu/module.h"
 #include "qapi/error.h"
+#include "qemu/qemu-print.h"
 #include "cpu.h"
 #include "exec/log.h"
 #include "gdbstub/helpers.h"
@@ -111,6 +112,25 @@ static void iic_set_irq(void *opaque, int irq, int level)
 }
 #endif
 
+static void nios2_cpu_list_entry(gpointer data, gpointer user_data)
+{
+    const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    char *model = cpu_model_from_type(typename);
+
+    qemu_printf("  %s\n", model);
+    g_free(model);
+}
+
+void nios2_cpu_list(void)
+{
+    GSList *list;
+
+    list = object_class_get_list_sorted(TYPE_NIOS2_CPU, false);
+    qemu_printf("Available CPUs:\n");
+    g_slist_foreach(list, nios2_cpu_list_entry, NULL);
+    g_slist_free(list);
+}
+
 static void nios2_cpu_initfn(Object *obj)
 {
     Nios2CPU *cpu = NIOS2_CPU(obj);
diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
index 477a3161fd..6d21b7e8f4 100644
--- a/target/nios2/cpu.h
+++ b/target/nios2/cpu.h
@@ -292,6 +292,9 @@ bool nios2_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                         MMUAccessType access_type, int mmu_idx,
                         bool probe, uintptr_t retaddr);
 #endif
+void nios2_cpu_list(void);
+
+#define cpu_list nios2_cpu_list
 
 typedef CPUNios2State CPUArchState;
 typedef Nios2CPU ArchCPU;
-- 
2.41.0

[PATCH v3 23/32] Mark cpu_list() supported on all targets

[Gavin Shan]

Remove the false conditions and comments since cpu_list() has been
supported on all targets.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 bsd-user/main.c | 3 ---
 cpu.c           | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index f913cb55a7..3a2d84f14b 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -378,10 +378,7 @@ int main(int argc, char **argv)
         } else if (!strcmp(r, "cpu")) {
             cpu_model = argv[optind++];
             if (is_help_option(cpu_model)) {
-                /* XXX: implement xxx_cpu_list for targets that still miss it */
-#if defined(cpu_list)
                 cpu_list();
-#endif
                 exit(1);
             }
         } else if (!strcmp(r, "B")) {
diff --git a/cpu.c b/cpu.c
index a19e33ff96..01bff086f8 100644
--- a/cpu.c
+++ b/cpu.c
@@ -302,10 +302,7 @@ char *cpu_model_from_type(const char *typename)
 
 void list_cpus(void)
 {
-    /* XXX: implement xxx_cpu_list for targets that still miss it */
-#if defined(cpu_list)
     cpu_list();
-#endif
 }
 
 #if defined(CONFIG_USER_ONLY)
-- 
2.41.0

[PATCH v3 24/32] machine: Constify MachineClass::valid_cpu_types[i]

[Gavin Shan]

Constify MachineClass::valid_cpu_types[i], as suggested by Richard
Henderson.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 hw/m68k/q800.c      | 2 +-
 include/hw/boards.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index b770b71d54..45f2b58b26 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -596,7 +596,7 @@ static GlobalProperty hw_compat_q800[] = {
 };
 static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
 
-static const char *q800_machine_valid_cpu_types[] = {
+static const char * const q800_machine_valid_cpu_types[] = {
     M68K_CPU_TYPE_NAME("m68040"),
     NULL
 };
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 3b541ffd24..49c328d928 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -268,7 +268,7 @@ struct MachineClass {
     bool has_hotpluggable_cpus;
     bool ignore_memory_transaction_failures;
     int numa_mem_align_shift;
-    const char **valid_cpu_types;
+    const char * const *valid_cpu_types;
     strList *allowed_dynamic_sysbus_devices;
     bool auto_enable_numa_with_memhp;
     bool auto_enable_numa_with_memdev;
-- 
2.41.0

Re: [PATCH v3 24/32] machine: Constify MachineClass::valid_cpu_types[i]

[Philippe Mathieu-Daudé]

On 7/9/23 02:35, Gavin Shan wrote:
> Constify MachineClass::valid_cpu_types[i], as suggested by Richard
> Henderson.
> 
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>   hw/m68k/q800.c      | 2 +-
>   include/hw/boards.h | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)

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

[PATCH v3 25/32] machine: Use error handling when CPU type is checked

[Gavin Shan]

QEMU will be terminated if the specified CPU type isn't supported
in machine_run_board_init(). The list of supported CPU type names
is tracked by mc->valid_cpu_types.

The error handling can be used to propagate error messages, to be
consistent how the errors are handled for other situations in the
same function.

No functional change intended.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 hw/core/machine.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index da699cf4e1..6d3f8e133f 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1358,6 +1358,7 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
     MachineClass *machine_class = MACHINE_GET_CLASS(machine);
     ObjectClass *oc = object_class_by_name(machine->cpu_type);
     CPUClass *cc;
+    Error *local_err = NULL;
 
     /* This checkpoint is required by replay to separate prior clock
        reading from the other reads, because timer polling functions query
@@ -1426,15 +1427,16 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
 
         if (!machine_class->valid_cpu_types[i]) {
             /* The user specified CPU is not valid */
-            error_report("Invalid CPU type: %s", machine->cpu_type);
-            error_printf("The valid types are: %s",
-                         machine_class->valid_cpu_types[0]);
+            error_setg(&local_err, "Invalid CPU type: %s", machine->cpu_type);
+            error_append_hint(&local_err, "The valid types are: %s",
+                              machine_class->valid_cpu_types[0]);
             for (i = 1; machine_class->valid_cpu_types[i]; i++) {
-                error_printf(", %s", machine_class->valid_cpu_types[i]);
+                error_append_hint(&local_err, ", %s",
+                                  machine_class->valid_cpu_types[i]);
             }
-            error_printf("\n");
+            error_append_hint(&local_err, "\n");
 
-            exit(1);
+            error_propagate(errp, local_err);
         }
     }
 
-- 
2.41.0

[PATCH v3 26/32] machine: Introduce helper is_cpu_type_supported()

[Gavin Shan]

The logic, to check if the specified CPU type is supported in
machine_run_board_init(), is independent enough. Factor it out
into helper is_cpu_type_supported(). With this, machine_run_board_init()
looks a bit clean. Since we're here, @machine_class is renamed to
@mc to avoid multiple line spanning of code. The comments are tweaked
a bit either.

No functional change intended.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 hw/core/machine.c | 82 +++++++++++++++++++++++++----------------------
 1 file changed, 44 insertions(+), 38 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 6d3f8e133f..93a327927f 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1352,12 +1352,50 @@ out:
     return r;
 }
 
+static void is_cpu_type_supported(MachineState *machine, Error **errp)
+{
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
+    ObjectClass *oc = object_class_by_name(machine->cpu_type);
+    CPUClass *cc;
+    int i;
+
+    /*
+     * Check if the user specified CPU type is supported when the valid
+     * CPU types have been determined. Note that the user specified CPU
+     * type is provided through '-cpu' option.
+     */
+    if (mc->valid_cpu_types && machine->cpu_type) {
+        for (i = 0; mc->valid_cpu_types[i]; i++) {
+            if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
+                break;
+            }
+        }
+
+        /* The user specified CPU type isn't valid */
+        if (!mc->valid_cpu_types[i]) {
+            error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
+            error_append_hint(errp, "The valid types are: %s",
+                              mc->valid_cpu_types[0]);
+            for (i = 1; mc->valid_cpu_types[i]; i++) {
+                error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
+            }
+
+            error_append_hint(errp, "\n");
+            return;
+        }
+    }
+
+    /* Check if CPU type is deprecated and warn if so */
+    cc = CPU_CLASS(oc);
+    if (cc && cc->deprecation_note) {
+        warn_report("CPU model %s is deprecated -- %s",
+                    machine->cpu_type, cc->deprecation_note);
+    }
+}
 
 void machine_run_board_init(MachineState *machine, const char *mem_path, Error **errp)
 {
     MachineClass *machine_class = MACHINE_GET_CLASS(machine);
-    ObjectClass *oc = object_class_by_name(machine->cpu_type);
-    CPUClass *cc;
     Error *local_err = NULL;
 
     /* This checkpoint is required by replay to separate prior clock
@@ -1409,42 +1447,10 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
         machine->ram = machine_consume_memdev(machine, machine->memdev);
     }
 
-    /* If the machine supports the valid_cpu_types check and the user
-     * specified a CPU with -cpu check here that the user CPU is supported.
-     */
-    if (machine_class->valid_cpu_types && machine->cpu_type) {
-        int i;
-
-        for (i = 0; machine_class->valid_cpu_types[i]; i++) {
-            if (object_class_dynamic_cast(oc,
-                                          machine_class->valid_cpu_types[i])) {
-                /* The user specificed CPU is in the valid field, we are
-                 * good to go.
-                 */
-                break;
-            }
-        }
-
-        if (!machine_class->valid_cpu_types[i]) {
-            /* The user specified CPU is not valid */
-            error_setg(&local_err, "Invalid CPU type: %s", machine->cpu_type);
-            error_append_hint(&local_err, "The valid types are: %s",
-                              machine_class->valid_cpu_types[0]);
-            for (i = 1; machine_class->valid_cpu_types[i]; i++) {
-                error_append_hint(&local_err, ", %s",
-                                  machine_class->valid_cpu_types[i]);
-            }
-            error_append_hint(&local_err, "\n");
-
-            error_propagate(errp, local_err);
-        }
-    }
-
-    /* Check if CPU type is deprecated and warn if so */
-    cc = CPU_CLASS(oc);
-    if (cc && cc->deprecation_note) {
-        warn_report("CPU model %s is deprecated -- %s", machine->cpu_type,
-                    cc->deprecation_note);
+    /* Check if the CPU type is supported */
+    is_cpu_type_supported(machine, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
     }
 
     if (machine->cgs) {
-- 
2.41.0

[PATCH v3 27/32] machine: Print CPU model name instead of CPU type name

[Gavin Shan]

The names of supported CPU models instead of CPU types should be
printed when the user specified CPU type isn't supported, to be
consistent with the output from '-cpu ?'.

Correct the error messages to print CPU model names instead of CPU
type names.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 hw/core/machine.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 93a327927f..6b701526ae 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1357,6 +1357,7 @@ static void is_cpu_type_supported(MachineState *machine, Error **errp)
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     ObjectClass *oc = object_class_by_name(machine->cpu_type);
     CPUClass *cc;
+    char *model;
     int i;
 
     /*
@@ -1373,11 +1374,18 @@ static void is_cpu_type_supported(MachineState *machine, Error **errp)
 
         /* The user specified CPU type isn't valid */
         if (!mc->valid_cpu_types[i]) {
-            error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
-            error_append_hint(errp, "The valid types are: %s",
-                              mc->valid_cpu_types[0]);
+            model = cpu_model_from_type(machine->cpu_type);
+            error_setg(errp, "Invalid CPU type: %s", model);
+            g_free(model);
+
+            model = cpu_model_from_type(mc->valid_cpu_types[0]);
+            error_append_hint(errp, "The valid types are: %s", model);
+            g_free(model);
+
             for (i = 1; mc->valid_cpu_types[i]; i++) {
-                error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
+                model = cpu_model_from_type(mc->valid_cpu_types[i]);
+                error_append_hint(errp, ", %s", model);
+                g_free(model);
             }
 
             error_append_hint(errp, "\n");
-- 
2.41.0

Re: [PATCH v3 27/32] machine: Print CPU model name instead of CPU type name

[Philippe Mathieu-Daudé]

On 7/9/23 02:35, Gavin Shan wrote:
> The names of supported CPU models instead of CPU types should be
> printed when the user specified CPU type isn't supported, to be
> consistent with the output from '-cpu ?'.
> 
> Correct the error messages to print CPU model names instead of CPU
> type names.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>   hw/core/machine.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)


> @@ -1373,11 +1374,18 @@ static void is_cpu_type_supported(MachineState *machine, Error **errp)
>   
>           /* The user specified CPU type isn't valid */
>           if (!mc->valid_cpu_types[i]) {
> -            error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
> -            error_append_hint(errp, "The valid types are: %s",
> -                              mc->valid_cpu_types[0]);
> +            model = cpu_model_from_type(machine->cpu_type);
> +            error_setg(errp, "Invalid CPU type: %s", model);
> +            g_free(model);
> +
> +            model = cpu_model_from_type(mc->valid_cpu_types[0]);
> +            error_append_hint(errp, "The valid types are: %s", model);
> +            g_free(model);
> +
>               for (i = 1; mc->valid_cpu_types[i]; i++) {
> -                error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
> +                model = cpu_model_from_type(mc->valid_cpu_types[i]);

cpu_model_from_type() can return NULL:

  char *cpu_model_from_type(const char *typename)
  {
      const char *suffix = "-" CPU_RESOLVING_TYPE;

      if (!object_class_by_name(typename)) {
          return NULL;
      }

Don't we want to skip that case?

                    if (!model) {
                        continue;
                    }

> +                error_append_hint(errp, ", %s", model);
> +                g_free(model);
>               }
>   
>               error_append_hint(errp, "\n");

Re: [PATCH v3 27/32] machine: Print CPU model name instead of CPU type name

[Gavin Shan]

On 9/7/23 19:05, Philippe Mathieu-Daudé wrote:
> On 7/9/23 02:35, Gavin Shan wrote:
>> The names of supported CPU models instead of CPU types should be
>> printed when the user specified CPU type isn't supported, to be
>> consistent with the output from '-cpu ?'.
>>
>> Correct the error messages to print CPU model names instead of CPU
>> type names.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   hw/core/machine.c | 16 ++++++++++++----
>>   1 file changed, 12 insertions(+), 4 deletions(-)
> 
> 
>> @@ -1373,11 +1374,18 @@ static void is_cpu_type_supported(MachineState *machine, Error **errp)
>>           /* The user specified CPU type isn't valid */
>>           if (!mc->valid_cpu_types[i]) {
>> -            error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
>> -            error_append_hint(errp, "The valid types are: %s",
>> -                              mc->valid_cpu_types[0]);
>> +            model = cpu_model_from_type(machine->cpu_type);
>> +            error_setg(errp, "Invalid CPU type: %s", model);
>> +            g_free(model);
>> +
>> +            model = cpu_model_from_type(mc->valid_cpu_types[0]);
>> +            error_append_hint(errp, "The valid types are: %s", model);
>> +            g_free(model);
>> +
>>               for (i = 1; mc->valid_cpu_types[i]; i++) {
>> -                error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
>> +                model = cpu_model_from_type(mc->valid_cpu_types[i]);
> 
> cpu_model_from_type() can return NULL:
> 
>   char *cpu_model_from_type(const char *typename)
>   {
>       const char *suffix = "-" CPU_RESOLVING_TYPE;
> 
>       if (!object_class_by_name(typename)) {
>           return NULL;
>       }
> 
> Don't we want to skip that case?
> 
>                     if (!model) {
>                         continue;
>                     }
> 

No, it's intentional. "(null)" will be printed in this specific case so that
it can be identified quickly and mc->valid_cpu_types[] need to be fixed by
developers.

>> +                error_append_hint(errp, ", %s", model);
>> +                g_free(model);
>>               }
>>               error_append_hint(errp, "\n");
> 

Thanks,
Gavin

Re: [PATCH v3 27/32] machine: Print CPU model name instead of CPU type name

[Philippe Mathieu-Daudé]

On 8/9/23 01:49, Gavin Shan wrote:
> On 9/7/23 19:05, Philippe Mathieu-Daudé wrote:
>> On 7/9/23 02:35, Gavin Shan wrote:
>>> The names of supported CPU models instead of CPU types should be
>>> printed when the user specified CPU type isn't supported, to be
>>> consistent with the output from '-cpu ?'.
>>>
>>> Correct the error messages to print CPU model names instead of CPU
>>> type names.
>>>
>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>> ---
>>>   hw/core/machine.c | 16 ++++++++++++----
>>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>
>>
>>> @@ -1373,11 +1374,18 @@ static void 
>>> is_cpu_type_supported(MachineState *machine, Error **errp)
>>>           /* The user specified CPU type isn't valid */
>>>           if (!mc->valid_cpu_types[i]) {
>>> -            error_setg(errp, "Invalid CPU type: %s", 
>>> machine->cpu_type);
>>> -            error_append_hint(errp, "The valid types are: %s",
>>> -                              mc->valid_cpu_types[0]);
>>> +            model = cpu_model_from_type(machine->cpu_type);
>>> +            error_setg(errp, "Invalid CPU type: %s", model);
>>> +            g_free(model);
>>> +
>>> +            model = cpu_model_from_type(mc->valid_cpu_types[0]);
>>> +            error_append_hint(errp, "The valid types are: %s", model);
>>> +            g_free(model);
>>> +
>>>               for (i = 1; mc->valid_cpu_types[i]; i++) {
>>> -                error_append_hint(errp, ", %s", 
>>> mc->valid_cpu_types[i]);
>>> +                model = cpu_model_from_type(mc->valid_cpu_types[i]);
>>
>> cpu_model_from_type() can return NULL:
>>
>>   char *cpu_model_from_type(const char *typename)
>>   {
>>       const char *suffix = "-" CPU_RESOLVING_TYPE;
>>
>>       if (!object_class_by_name(typename)) {
>>           return NULL;
>>       }
>>
>> Don't we want to skip that case?
>>
>>                     if (!model) {
>>                         continue;
>>                     }
>>
> 
> No, it's intentional. "(null)" will be printed in this specific case so 
> that
> it can be identified quickly and mc->valid_cpu_types[] need to be fixed by
> developers.

If you want to help developers, use g_assert().

Re: [PATCH v3 27/32] machine: Print CPU model name instead of CPU type name

[Gavin Shan]

On 9/8/23 17:56, Philippe Mathieu-Daudé wrote:
> On 8/9/23 01:49, Gavin Shan wrote:
>> On 9/7/23 19:05, Philippe Mathieu-Daudé wrote:
>>> On 7/9/23 02:35, Gavin Shan wrote:
>>>> The names of supported CPU models instead of CPU types should be
>>>> printed when the user specified CPU type isn't supported, to be
>>>> consistent with the output from '-cpu ?'.
>>>>
>>>> Correct the error messages to print CPU model names instead of CPU
>>>> type names.
>>>>
>>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>>> ---
>>>>   hw/core/machine.c | 16 ++++++++++++----
>>>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>>
>>>> @@ -1373,11 +1374,18 @@ static void is_cpu_type_supported(MachineState *machine, Error **errp)
>>>>           /* The user specified CPU type isn't valid */
>>>>           if (!mc->valid_cpu_types[i]) {
>>>> -            error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
>>>> -            error_append_hint(errp, "The valid types are: %s",
>>>> -                              mc->valid_cpu_types[0]);
>>>> +            model = cpu_model_from_type(machine->cpu_type);
>>>> +            error_setg(errp, "Invalid CPU type: %s", model);
>>>> +            g_free(model);
>>>> +
>>>> +            model = cpu_model_from_type(mc->valid_cpu_types[0]);
>>>> +            error_append_hint(errp, "The valid types are: %s", model);
>>>> +            g_free(model);
>>>> +
>>>>               for (i = 1; mc->valid_cpu_types[i]; i++) {
>>>> -                error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
>>>> +                model = cpu_model_from_type(mc->valid_cpu_types[i]);
>>>
>>> cpu_model_from_type() can return NULL:
>>>
>>>   char *cpu_model_from_type(const char *typename)
>>>   {
>>>       const char *suffix = "-" CPU_RESOLVING_TYPE;
>>>
>>>       if (!object_class_by_name(typename)) {
>>>           return NULL;
>>>       }
>>>
>>> Don't we want to skip that case?
>>>
>>>                     if (!model) {
>>>                         continue;
>>>                     }
>>>
>>
>> No, it's intentional. "(null)" will be printed in this specific case so that
>> it can be identified quickly and mc->valid_cpu_types[] need to be fixed by
>> developers.
> 
> If you want to help developers, use g_assert().
> 

g_assert() wins. It will be included into v4 :)

Thanks,
Gavin

[PATCH v3 28/32] hw/arm/virt: Check CPU type in machine_run_board_init()

[Gavin Shan]

Set mc->valid_cpu_types so that the user specified CPU type can be
validated in machine_run_board_init(). We needn't to do the check
by ourselves.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 hw/arm/virt.c | 21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a13c658bbf..762780e677 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -203,7 +203,7 @@ static const int a15irqmap[] = {
     [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
 };
 
-static const char *valid_cpus[] = {
+static const char * const valid_cpu_types[] = {
 #ifdef CONFIG_TCG
     ARM_CPU_TYPE_NAME("cortex-a7"),
     ARM_CPU_TYPE_NAME("cortex-a15"),
@@ -219,20 +219,9 @@ static const char *valid_cpus[] = {
     ARM_CPU_TYPE_NAME("cortex-a57"),
     ARM_CPU_TYPE_NAME("host"),
     ARM_CPU_TYPE_NAME("max"),
+    NULL
 };
 
-static bool cpu_type_valid(const char *cpu)
-{
-    int i;
-
-    for (i = 0; i < ARRAY_SIZE(valid_cpus); i++) {
-        if (strcmp(cpu, valid_cpus[i]) == 0) {
-            return true;
-        }
-    }
-    return false;
-}
-
 static void create_randomness(MachineState *ms, const char *node)
 {
     struct {
@@ -2030,11 +2019,6 @@ static void machvirt_init(MachineState *machine)
     unsigned int smp_cpus = machine->smp.cpus;
     unsigned int max_cpus = machine->smp.max_cpus;
 
-    if (!cpu_type_valid(machine->cpu_type)) {
-        error_report("mach-virt: CPU type %s not supported", machine->cpu_type);
-        exit(1);
-    }
-
     possible_cpus = mc->possible_cpu_arch_ids(machine);
 
     /*
@@ -2953,6 +2937,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
 #else
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("max");
 #endif
+    mc->valid_cpu_types = valid_cpu_types;
     mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
     mc->kvm_type = virt_kvm_type;
     assert(!mc->get_hotplug_handler);
-- 
2.41.0

Re: [PATCH v3 28/32] hw/arm/virt: Check CPU type in machine_run_board_init()

[Philippe Mathieu-Daudé]

On 7/9/23 02:35, Gavin Shan wrote:
> Set mc->valid_cpu_types so that the user specified CPU type can be
> validated in machine_run_board_init(). We needn't to do the check
> by ourselves.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>   hw/arm/virt.c | 21 +++------------------
>   1 file changed, 3 insertions(+), 18 deletions(-)

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

[PATCH v3 29/32] hw/arm/virt: Hide host CPU model for tcg

[Gavin Shan]

The 'host' CPU model isn't available until KVM or HVF is enabled.
For example, the following error messages are seen when the guest
is started with option '-cpu cortex-a8' on tcg.

  qemu-system-aarch64: Invalid CPU type: cortex-a8
  The valid types are: cortex-a7, cortex-a15, cortex-a35, cortex-a55,
                       cortex-a72, cortex-a76, a64fx, neoverse-n1,
                       neoverse-v1, cortex-a53, cortex-a57, (null),
                       max

Hide 'host' CPU model until KVM or HVF is enabled.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 hw/arm/virt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 762780e677..bd0ad15028 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -217,7 +217,9 @@ static const char * const valid_cpu_types[] = {
 #endif
     ARM_CPU_TYPE_NAME("cortex-a53"),
     ARM_CPU_TYPE_NAME("cortex-a57"),
+#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
     ARM_CPU_TYPE_NAME("host"),
+#endif
     ARM_CPU_TYPE_NAME("max"),
     NULL
 };
-- 
2.41.0

[PATCH v3 30/32] hw/arm/sbsa-ref: Check CPU type in machine_run_board_init()

[Gavin Shan]

Set mc->valid_cpu_types so that the user specified CPU type can
be validated in machine_run_board_init(). We needn't to do it
by ourselves.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 hw/arm/sbsa-ref.c | 21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index bc89eb4806..f24be53ea2 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -149,26 +149,15 @@ static const int sbsa_ref_irqmap[] = {
     [SBSA_GWDT_WS0] = 16,
 };
 
-static const char * const valid_cpus[] = {
+static const char * const valid_cpu_types[] = {
     ARM_CPU_TYPE_NAME("cortex-a57"),
     ARM_CPU_TYPE_NAME("cortex-a72"),
     ARM_CPU_TYPE_NAME("neoverse-n1"),
     ARM_CPU_TYPE_NAME("neoverse-v1"),
     ARM_CPU_TYPE_NAME("max"),
+    NULL,
 };
 
-static bool cpu_type_valid(const char *cpu)
-{
-    int i;
-
-    for (i = 0; i < ARRAY_SIZE(valid_cpus); i++) {
-        if (strcmp(cpu, valid_cpus[i]) == 0) {
-            return true;
-        }
-    }
-    return false;
-}
-
 static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
 {
     uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER;
@@ -730,11 +719,6 @@ static void sbsa_ref_init(MachineState *machine)
     const CPUArchIdList *possible_cpus;
     int n, sbsa_max_cpus;
 
-    if (!cpu_type_valid(machine->cpu_type)) {
-        error_report("sbsa-ref: CPU type %s not supported", machine->cpu_type);
-        exit(1);
-    }
-
     if (kvm_enabled()) {
         error_report("sbsa-ref: KVM is not supported for this machine");
         exit(1);
@@ -899,6 +883,7 @@ static void sbsa_ref_class_init(ObjectClass *oc, void *data)
     mc->init = sbsa_ref_init;
     mc->desc = "QEMU 'SBSA Reference' ARM Virtual Machine";
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("neoverse-n1");
+    mc->valid_cpu_types = valid_cpu_types;
     mc->max_cpus = 512;
     mc->pci_allow_0_address = true;
     mc->minimum_page_bits = 12;
-- 
2.41.0

Re: [PATCH v3 30/32] hw/arm/sbsa-ref: Check CPU type in machine_run_board_init()

[Philippe Mathieu-Daudé]

On 7/9/23 02:35, Gavin Shan wrote:
> Set mc->valid_cpu_types so that the user specified CPU type can
> be validated in machine_run_board_init(). We needn't to do it
> by ourselves.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>   hw/arm/sbsa-ref.c | 21 +++------------------
>   1 file changed, 3 insertions(+), 18 deletions(-)

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

Re: [PATCH v3 30/32] hw/arm/sbsa-ref: Check CPU type in machine_run_board_init()

[Leif Lindholm]

On Thu, Sep 07, 2023 at 10:35:51 +1000, Gavin Shan wrote:
> Set mc->valid_cpu_types so that the user specified CPU type can
> be validated in machine_run_board_init(). We needn't to do it
> by ourselves.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>

Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>

> ---
>  hw/arm/sbsa-ref.c | 21 +++------------------
>  1 file changed, 3 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index bc89eb4806..f24be53ea2 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -149,26 +149,15 @@ static const int sbsa_ref_irqmap[] = {
>      [SBSA_GWDT_WS0] = 16,
>  };
>  
> -static const char * const valid_cpus[] = {
> +static const char * const valid_cpu_types[] = {
>      ARM_CPU_TYPE_NAME("cortex-a57"),
>      ARM_CPU_TYPE_NAME("cortex-a72"),
>      ARM_CPU_TYPE_NAME("neoverse-n1"),
>      ARM_CPU_TYPE_NAME("neoverse-v1"),
>      ARM_CPU_TYPE_NAME("max"),
> +    NULL,
>  };
>  
> -static bool cpu_type_valid(const char *cpu)
> -{
> -    int i;
> -
> -    for (i = 0; i < ARRAY_SIZE(valid_cpus); i++) {
> -        if (strcmp(cpu, valid_cpus[i]) == 0) {
> -            return true;
> -        }
> -    }
> -    return false;
> -}
> -
>  static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
>  {
>      uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER;
> @@ -730,11 +719,6 @@ static void sbsa_ref_init(MachineState *machine)
>      const CPUArchIdList *possible_cpus;
>      int n, sbsa_max_cpus;
>  
> -    if (!cpu_type_valid(machine->cpu_type)) {
> -        error_report("sbsa-ref: CPU type %s not supported", machine->cpu_type);
> -        exit(1);
> -    }
> -
>      if (kvm_enabled()) {
>          error_report("sbsa-ref: KVM is not supported for this machine");
>          exit(1);
> @@ -899,6 +883,7 @@ static void sbsa_ref_class_init(ObjectClass *oc, void *data)
>      mc->init = sbsa_ref_init;
>      mc->desc = "QEMU 'SBSA Reference' ARM Virtual Machine";
>      mc->default_cpu_type = ARM_CPU_TYPE_NAME("neoverse-n1");
> +    mc->valid_cpu_types = valid_cpu_types;
>      mc->max_cpus = 512;
>      mc->pci_allow_0_address = true;
>      mc->minimum_page_bits = 12;
> -- 
> 2.41.0
> 

[PATCH v3 31/32] hw/arm: Check CPU type in machine_run_board_init()

[Gavin Shan]

Set mc->valid_cpu_types so that the user specified CPU type can
be validated in machine_run_board_init(). We needn't to do it by
ourselves.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 hw/arm/bananapi_m2u.c   | 12 ++++++------
 hw/arm/cubieboard.c     | 12 ++++++------
 hw/arm/mps2-tz.c        | 20 ++++++++++++++------
 hw/arm/mps2.c           | 25 +++++++++++++++++++------
 hw/arm/msf2-som.c       | 12 ++++++------
 hw/arm/musca.c          | 13 ++++++-------
 hw/arm/npcm7xx_boards.c | 13 ++++++-------
 hw/arm/orangepi.c       | 12 ++++++------
 8 files changed, 69 insertions(+), 50 deletions(-)

diff --git a/hw/arm/bananapi_m2u.c b/hw/arm/bananapi_m2u.c
index 74121d8966..2d8551aa67 100644
--- a/hw/arm/bananapi_m2u.c
+++ b/hw/arm/bananapi_m2u.c
@@ -29,6 +29,11 @@
 
 static struct arm_boot_info bpim2u_binfo;
 
+static const char * const valid_cpu_types[] = {
+    ARM_CPU_TYPE_NAME("cortex-a7"),
+    NULL
+};
+
 /*
  * R40 can boot from mmc0 and mmc2, and bpim2u has two mmc interface, one is
  * connected to sdcard and another mount an emmc media.
@@ -70,12 +75,6 @@ static void bpim2u_init(MachineState *machine)
         exit(1);
     }
 
-    /* Only allow Cortex-A7 for this board */
-    if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a7")) != 0) {
-        error_report("This board can only be used with cortex-a7 CPU");
-        exit(1);
-    }
-
     r40 = AW_R40(object_new(TYPE_AW_R40));
     object_property_add_child(OBJECT(machine), "soc", OBJECT(r40));
     object_unref(OBJECT(r40));
@@ -138,6 +137,7 @@ static void bpim2u_machine_init(MachineClass *mc)
     mc->max_cpus = AW_R40_NUM_CPUS;
     mc->default_cpus = AW_R40_NUM_CPUS;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a7");
+    mc->valid_cpu_types = valid_cpu_types;
     mc->default_ram_size = 1 * GiB;
     mc->default_ram_id = "bpim2u.ram";
 }
diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index 8c7fa91529..f77fd5fe6c 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -28,6 +28,11 @@ static struct arm_boot_info cubieboard_binfo = {
     .board_id = 0x1008,
 };
 
+static const char * const valid_cpu_types[] = {
+    ARM_CPU_TYPE_NAME("cortex-a8"),
+    NULL
+};
+
 static void cubieboard_init(MachineState *machine)
 {
     AwA10State *a10;
@@ -51,12 +56,6 @@ static void cubieboard_init(MachineState *machine)
         exit(1);
     }
 
-    /* Only allow Cortex-A8 for this board */
-    if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a8")) != 0) {
-        error_report("This board can only be used with cortex-a8 CPU");
-        exit(1);
-    }
-
     a10 = AW_A10(object_new(TYPE_AW_A10));
     object_property_add_child(OBJECT(machine), "soc", OBJECT(a10));
     object_unref(OBJECT(a10));
@@ -115,6 +114,7 @@ static void cubieboard_machine_init(MachineClass *mc)
 {
     mc->desc = "cubietech cubieboard (Cortex-A8)";
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
+    mc->valid_cpu_types = valid_cpu_types;
     mc->default_ram_size = 1 * GiB;
     mc->init = cubieboard_init;
     mc->block_default_type = IF_IDE;
diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index eae3639da2..d7bb6d965f 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -190,6 +190,16 @@ OBJECT_DECLARE_TYPE(MPS2TZMachineState, MPS2TZMachineClass, MPS2TZ_MACHINE)
 /* For cpu{0,1}_mpu_{ns,s}, means "leave at SSE's default value" */
 #define MPU_REGION_DEFAULT UINT32_MAX
 
+static const char * const valid_cpu_types[] = {
+    ARM_CPU_TYPE_NAME("cortex-m33"),
+    NULL
+};
+
+static const char * const mps3tz_an547_valid_cpu_types[] = {
+    ARM_CPU_TYPE_NAME("cortex-m55"),
+    NULL
+};
+
 static const uint32_t an505_oscclk[] = {
     40000000,
     24580000,
@@ -809,12 +819,6 @@ static void mps2tz_common_init(MachineState *machine)
     int num_ppcs;
     int i;
 
-    if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
-        error_report("This board can only be used with CPU %s",
-                     mc->default_cpu_type);
-        exit(1);
-    }
-
     if (machine->ram_size != mc->default_ram_size) {
         char *sz = size_to_str(mc->default_ram_size);
         error_report("Invalid RAM size, should be %s", sz);
@@ -1321,6 +1325,7 @@ static void mps2tz_an505_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = mc->default_cpus;
     mmc->fpga_type = FPGA_AN505;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m33");
+    mc->valid_cpu_types = valid_cpu_types;
     mmc->scc_id = 0x41045050;
     mmc->sysclk_frq = 20 * 1000 * 1000; /* 20MHz */
     mmc->apb_periph_frq = mmc->sysclk_frq;
@@ -1350,6 +1355,7 @@ static void mps2tz_an521_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = mc->default_cpus;
     mmc->fpga_type = FPGA_AN521;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m33");
+    mc->valid_cpu_types = valid_cpu_types;
     mmc->scc_id = 0x41045210;
     mmc->sysclk_frq = 20 * 1000 * 1000; /* 20MHz */
     mmc->apb_periph_frq = mmc->sysclk_frq;
@@ -1379,6 +1385,7 @@ static void mps3tz_an524_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = mc->default_cpus;
     mmc->fpga_type = FPGA_AN524;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m33");
+    mc->valid_cpu_types = valid_cpu_types;
     mmc->scc_id = 0x41045240;
     mmc->sysclk_frq = 32 * 1000 * 1000; /* 32MHz */
     mmc->apb_periph_frq = mmc->sysclk_frq;
@@ -1413,6 +1420,7 @@ static void mps3tz_an547_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = mc->default_cpus;
     mmc->fpga_type = FPGA_AN547;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m55");
+    mc->valid_cpu_types = mps3tz_an547_valid_cpu_types;
     mmc->scc_id = 0x41055470;
     mmc->sysclk_frq = 32 * 1000 * 1000; /* 32MHz */
     mmc->apb_periph_frq = 25 * 1000 * 1000; /* 25MHz */
diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index d92fd60684..691b0803cf 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -110,6 +110,21 @@ OBJECT_DECLARE_TYPE(MPS2MachineState, MPS2MachineClass, MPS2_MACHINE)
  */
 #define REFCLK_FRQ (1 * 1000 * 1000)
 
+static const char * const valid_cpu_types[] = {
+    ARM_CPU_TYPE_NAME("cortex-m3"),
+    NULL
+};
+
+static const char * const mps2_an386_valid_cpu_types[] = {
+    ARM_CPU_TYPE_NAME("cortex-m4"),
+    NULL
+};
+
+static const char * const mps2_an500_valid_cpu_types[] = {
+    ARM_CPU_TYPE_NAME("cortex-m7"),
+    NULL
+};
+
 /* Initialize the auxiliary RAM region @mr and map it into
  * the memory map at @base.
  */
@@ -140,12 +155,6 @@ static void mps2_common_init(MachineState *machine)
     DeviceState *armv7m, *sccdev;
     int i;
 
-    if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
-        error_report("This board can only be used with CPU %s",
-                     mc->default_cpu_type);
-        exit(1);
-    }
-
     if (machine->ram_size != mc->default_ram_size) {
         char *sz = size_to_str(mc->default_ram_size);
         error_report("Invalid RAM size, should be %s", sz);
@@ -484,6 +493,7 @@ static void mps2_an385_class_init(ObjectClass *oc, void *data)
     mc->desc = "ARM MPS2 with AN385 FPGA image for Cortex-M3";
     mmc->fpga_type = FPGA_AN385;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m3");
+    mc->valid_cpu_types = valid_cpu_types;
     mmc->scc_id = 0x41043850;
     mmc->psram_base = 0x21000000;
     mmc->ethernet_base = 0x40200000;
@@ -498,6 +508,7 @@ static void mps2_an386_class_init(ObjectClass *oc, void *data)
     mc->desc = "ARM MPS2 with AN386 FPGA image for Cortex-M4";
     mmc->fpga_type = FPGA_AN386;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
+    mc->valid_cpu_types = mps2_an386_valid_cpu_types;
     mmc->scc_id = 0x41043860;
     mmc->psram_base = 0x21000000;
     mmc->ethernet_base = 0x40200000;
@@ -512,6 +523,7 @@ static void mps2_an500_class_init(ObjectClass *oc, void *data)
     mc->desc = "ARM MPS2 with AN500 FPGA image for Cortex-M7";
     mmc->fpga_type = FPGA_AN500;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m7");
+    mc->valid_cpu_types = mps2_an500_valid_cpu_types;
     mmc->scc_id = 0x41045000;
     mmc->psram_base = 0x60000000;
     mmc->ethernet_base = 0xa0000000;
@@ -526,6 +538,7 @@ static void mps2_an511_class_init(ObjectClass *oc, void *data)
     mc->desc = "ARM MPS2 with AN511 DesignStart FPGA image for Cortex-M3";
     mmc->fpga_type = FPGA_AN511;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m3");
+    mc->valid_cpu_types = valid_cpu_types;
     mmc->scc_id = 0x41045110;
     mmc->psram_base = 0x21000000;
     mmc->ethernet_base = 0x40200000;
diff --git a/hw/arm/msf2-som.c b/hw/arm/msf2-som.c
index 7b3106c790..470951a006 100644
--- a/hw/arm/msf2-som.c
+++ b/hw/arm/msf2-som.c
@@ -42,6 +42,11 @@
 #define M2S010_ENVM_SIZE      (256 * KiB)
 #define M2S010_ESRAM_SIZE     (64 * KiB)
 
+static const char * const valid_cpu_types[] = {
+    ARM_CPU_TYPE_NAME("cortex-m3"),
+    NULL
+};
+
 static void emcraft_sf2_s2s010_init(MachineState *machine)
 {
     DeviceState *dev;
@@ -55,12 +60,6 @@ static void emcraft_sf2_s2s010_init(MachineState *machine)
     MemoryRegion *ddr = g_new(MemoryRegion, 1);
     Clock *m3clk;
 
-    if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
-        error_report("This board can only be used with CPU %s",
-                     mc->default_cpu_type);
-        exit(1);
-    }
-
     memory_region_init_ram(ddr, NULL, "ddr-ram", DDR_SIZE,
                            &error_fatal);
     memory_region_add_subregion(sysmem, DDR_BASE_ADDRESS, ddr);
@@ -109,6 +108,7 @@ static void emcraft_sf2_machine_init(MachineClass *mc)
     mc->desc = "SmartFusion2 SOM kit from Emcraft (M2S010)";
     mc->init = emcraft_sf2_s2s010_init;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m3");
+    mc->valid_cpu_types = valid_cpu_types;
 }
 
 DEFINE_MACHINE("emcraft-sf2", emcraft_sf2_machine_init)
diff --git a/hw/arm/musca.c b/hw/arm/musca.c
index 6eeee57c9d..9bd78fd531 100644
--- a/hw/arm/musca.c
+++ b/hw/arm/musca.c
@@ -102,6 +102,11 @@ OBJECT_DECLARE_TYPE(MuscaMachineState, MuscaMachineClass, MUSCA_MACHINE)
 /* Slow 32Khz S32KCLK frequency in Hz */
 #define S32KCLK_FRQ (32 * 1000)
 
+static const char * const valid_cpu_types[] = {
+    ARM_CPU_TYPE_NAME("cortex-m33"),
+    NULL
+};
+
 static qemu_irq get_sse_irq_in(MuscaMachineState *mms, int irqno)
 {
     /* Return a qemu_irq which will signal IRQ n to all CPUs in the SSE. */
@@ -355,7 +360,6 @@ static void musca_init(MachineState *machine)
 {
     MuscaMachineState *mms = MUSCA_MACHINE(machine);
     MuscaMachineClass *mmc = MUSCA_MACHINE_GET_CLASS(mms);
-    MachineClass *mc = MACHINE_GET_CLASS(machine);
     MemoryRegion *system_memory = get_system_memory();
     DeviceState *ssedev;
     DeviceState *dev_splitter;
@@ -366,12 +370,6 @@ static void musca_init(MachineState *machine)
     assert(mmc->num_irqs <= MUSCA_NUMIRQ_MAX);
     assert(mmc->num_mpcs <= MUSCA_MPC_MAX);
 
-    if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
-        error_report("This board can only be used with CPU %s",
-                     mc->default_cpu_type);
-        exit(1);
-    }
-
     mms->sysclk = clock_new(OBJECT(machine), "SYSCLK");
     clock_set_hz(mms->sysclk, SYSCLK_FRQ);
     mms->s32kclk = clock_new(OBJECT(machine), "S32KCLK");
@@ -609,6 +607,7 @@ static void musca_class_init(ObjectClass *oc, void *data)
     mc->min_cpus = mc->default_cpus;
     mc->max_cpus = mc->default_cpus;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m33");
+    mc->valid_cpu_types = valid_cpu_types;
     mc->init = musca_init;
 }
 
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
index 2aef579aac..ca37c893b4 100644
--- a/hw/arm/npcm7xx_boards.c
+++ b/hw/arm/npcm7xx_boards.c
@@ -55,6 +55,11 @@
 
 static const char npcm7xx_default_bootrom[] = "npcm7xx_bootrom.bin";
 
+static const char * const valid_cpu_types[] = {
+    ARM_CPU_TYPE_NAME("cortex-a9"),
+    NULL
+};
+
 static void npcm7xx_load_bootrom(MachineState *machine, NPCM7xxState *soc)
 {
     const char *bios_name = machine->firmware ?: npcm7xx_default_bootrom;
@@ -121,15 +126,8 @@ static NPCM7xxState *npcm7xx_create_soc(MachineState *machine,
                                         uint32_t hw_straps)
 {
     NPCM7xxMachineClass *nmc = NPCM7XX_MACHINE_GET_CLASS(machine);
-    MachineClass *mc = MACHINE_CLASS(nmc);
     Object *obj;
 
-    if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
-        error_report("This board can only be used with %s",
-                     mc->default_cpu_type);
-        exit(1);
-    }
-
     obj = object_new_with_props(nmc->soc_type, OBJECT(machine), "soc",
                                 &error_abort, NULL);
     object_property_set_uint(obj, "power-on-straps", hw_straps, &error_abort);
@@ -469,6 +467,7 @@ static void npcm7xx_machine_class_init(ObjectClass *oc, void *data)
     mc->no_parallel = 1;
     mc->default_ram_id = "ram";
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
+    mc->valid_cpu_types = valid_cpu_types;
 }
 
 /*
diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c
index 10653361ed..5c6d4538b9 100644
--- a/hw/arm/orangepi.c
+++ b/hw/arm/orangepi.c
@@ -28,6 +28,11 @@
 
 static struct arm_boot_info orangepi_binfo;
 
+static const char * const valid_cpu_types[] = {
+    ARM_CPU_TYPE_NAME("cortex-a7"),
+    NULL
+};
+
 static void orangepi_init(MachineState *machine)
 {
     AwH3State *h3;
@@ -48,12 +53,6 @@ static void orangepi_init(MachineState *machine)
         exit(1);
     }
 
-    /* Only allow Cortex-A7 for this board */
-    if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a7")) != 0) {
-        error_report("This board can only be used with cortex-a7 CPU");
-        exit(1);
-    }
-
     h3 = AW_H3(object_new(TYPE_AW_H3));
     object_property_add_child(OBJECT(machine), "soc", OBJECT(h3));
     object_unref(OBJECT(h3));
@@ -118,6 +117,7 @@ static void orangepi_machine_init(MachineClass *mc)
     mc->max_cpus = AW_H3_NUM_CPUS;
     mc->default_cpus = AW_H3_NUM_CPUS;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a7");
+    mc->valid_cpu_types = valid_cpu_types;
     mc->default_ram_size = 1 * GiB;
     mc->default_ram_id = "orangepi.ram";
 }
-- 
2.41.0

[PATCH v3 32/32] hw/riscv/shakti_c: Check CPU type in machine_run_board_init()

[Gavin Shan]

Set mc->valid_cpu_types so that the user specified CPU type can
be validated in machine_run_board_init(). We needn't to do it
by ourselves.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 hw/riscv/shakti_c.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
index 12ea74b032..fc83ed4db4 100644
--- a/hw/riscv/shakti_c.c
+++ b/hw/riscv/shakti_c.c
@@ -28,6 +28,10 @@
 #include "exec/address-spaces.h"
 #include "hw/riscv/boot.h"
 
+static const char * const valid_cpu_types[] = {
+    RISCV_CPU_TYPE_NAME("shakti-c"),
+    NULL
+};
 
 static const struct MemmapEntry {
     hwaddr base;
@@ -47,12 +51,6 @@ static void shakti_c_machine_state_init(MachineState *mstate)
     ShaktiCMachineState *sms = RISCV_SHAKTI_MACHINE(mstate);
     MemoryRegion *system_memory = get_system_memory();
 
-    /* Allow only Shakti C CPU for this platform */
-    if (strcmp(mstate->cpu_type, TYPE_RISCV_CPU_SHAKTI_C) != 0) {
-        error_report("This board can only be used with Shakti C CPU");
-        exit(1);
-    }
-
     /* Initialize SoC */
     object_initialize_child(OBJECT(mstate), "soc", &sms->soc,
                             TYPE_RISCV_SHAKTI_SOC);
@@ -85,6 +83,7 @@ static void shakti_c_machine_class_init(ObjectClass *klass, void *data)
     mc->desc = "RISC-V Board compatible with Shakti SDK";
     mc->init = shakti_c_machine_state_init;
     mc->default_cpu_type = TYPE_RISCV_CPU_SHAKTI_C;
+    mc->valid_cpu_types = valid_cpu_types;
     mc->default_ram_id = "riscv.shakti.c.ram";
 }
 
-- 
2.41.0

Re: [PATCH v3 32/32] hw/riscv/shakti_c: Check CPU type in machine_run_board_init()

[Philippe Mathieu-Daudé]

On 7/9/23 02:35, Gavin Shan wrote:
> Set mc->valid_cpu_types so that the user specified CPU type can
> be validated in machine_run_board_init(). We needn't to do it
> by ourselves.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>   hw/riscv/shakti_c.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)

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