[PATCH 0/4] hw/core/cpu-common: Consolidate cpu_class_by_name()

[PATCH 0/4] hw/core/cpu-common: Consolidate cpu_class_by_name()

[Philippe Mathieu-Daudé]

Gavin noticed the same pattern is duplicated in many
CPUClass::class_by_name() handlers [*].
This series consolidate the calls to
 - object_class_is_abstract()
 - object_class_dynamic_cast()
in the common cpu_class_by_name(), by introducing
the CPUClass::cpu_resolving_type field.

[*] https://lore.kernel.org/qemu-devel/ab07d81c-da98-a270-c3f6-0625912c6d0b@redhat.com/

Philippe Mathieu-Daudé (4):
  target/alpha: Tidy up alpha_cpu_class_by_name()
  hw/cpu: Call object_class_is_abstract() once in cpu_class_by_name()
  hw/cpu: Introduce CPUClass::cpu_resolving_type field
  hw/cpu: Call object_class_dynamic_cast() once in cpu_class_by_name()

 include/hw/core/cpu.h   |  9 ++++++---
 hw/core/cpu-common.c    | 15 ++++++++++++---
 target/alpha/cpu.c      | 11 ++++-------
 target/arm/cpu.c        |  6 ++----
 target/avr/cpu.c        |  6 ++----
 target/cris/cpu.c       |  6 ++----
 target/hexagon/cpu.c    |  6 ++----
 target/hppa/cpu.c       |  1 +
 target/i386/cpu.c       |  1 +
 target/loongarch/cpu.c  |  7 ++-----
 target/m68k/cpu.c       |  6 ++----
 target/microblaze/cpu.c |  1 +
 target/mips/cpu.c       |  1 +
 target/nios2/cpu.c      |  1 +
 target/openrisc/cpu.c   |  6 ++----
 target/ppc/cpu_init.c   |  1 +
 target/riscv/cpu.c      |  6 ++----
 target/rx/cpu.c         |  7 ++-----
 target/s390x/cpu.c      |  1 +
 target/sh4/cpu.c        |  4 +---
 target/sparc/cpu.c      |  1 +
 target/tricore/cpu.c    |  6 ++----
 target/xtensa/cpu.c     |  6 ++----
 23 files changed, 53 insertions(+), 62 deletions(-)

-- 
2.41.0

[PATCH 1/4] target/alpha: Tidy up alpha_cpu_class_by_name()

[Philippe Mathieu-Daudé]

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

diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 270ae787b1..351ee2e9f2 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -142,13 +142,10 @@ static ObjectClass *alpha_cpu_class_by_name(const char *cpu_model)
     typename = g_strdup_printf(ALPHA_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;
-    }
 
     /* TODO: remove match everything nonsense */
-    /* Default to ev67; no reason not to emulate insns by default. */
-    if (!oc) {
+    if (!oc || object_class_is_abstract(oc)) {
+        /* Default to ev67; no reason not to emulate insns by default. */
         oc = object_class_by_name(ALPHA_CPU_TYPE_NAME("ev67"));
     }
 
-- 
2.41.0

Re: [PATCH 1/4] target/alpha: Tidy up alpha_cpu_class_by_name()

[Richard Henderson]

On 9/8/23 04:22, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/alpha/cpu.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)

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

r~

Re: [PATCH 1/4] target/alpha: Tidy up alpha_cpu_class_by_name()

[Gavin Shan]

On 9/8/23 21:22, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/alpha/cpu.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
> 

Reviewed-by: Gavin Shan <gshan@redhat.com>

> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
> index 270ae787b1..351ee2e9f2 100644
> --- a/target/alpha/cpu.c
> +++ b/target/alpha/cpu.c
> @@ -142,13 +142,10 @@ static ObjectClass *alpha_cpu_class_by_name(const char *cpu_model)
>       typename = g_strdup_printf(ALPHA_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;
> -    }
>   
>       /* TODO: remove match everything nonsense */
> -    /* Default to ev67; no reason not to emulate insns by default. */
> -    if (!oc) {
> +    if (!oc || object_class_is_abstract(oc)) {
> +        /* Default to ev67; no reason not to emulate insns by default. */
>           oc = object_class_by_name(ALPHA_CPU_TYPE_NAME("ev67"));
>       }
>   

[PATCH 2/4] hw/cpu: Call object_class_is_abstract() once in cpu_class_by_name()

[Philippe Mathieu-Daudé]

Let CPUClass::class_by_name() handlers to return abstract classes,
and filter them once in the public cpu_class_by_name() method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/cpu.h  |  7 ++++---
 hw/core/cpu-common.c   | 14 +++++++++++---
 target/arm/cpu.c       |  3 +--
 target/avr/cpu.c       |  3 +--
 target/cris/cpu.c      |  3 +--
 target/hexagon/cpu.c   |  3 +--
 target/loongarch/cpu.c |  3 +--
 target/m68k/cpu.c      |  3 +--
 target/openrisc/cpu.c  |  3 +--
 target/riscv/cpu.c     |  3 +--
 target/rx/cpu.c        |  6 +-----
 target/sh4/cpu.c       |  3 ---
 target/tricore/cpu.c   |  3 +--
 target/xtensa/cpu.c    |  3 +--
 14 files changed, 26 insertions(+), 34 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 92a4234439..129d179937 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -101,7 +101,7 @@ struct SysemuCPUOps;
 /**
  * CPUClass:
  * @class_by_name: Callback to map -cpu command line model name to an
- * instantiatable CPU type.
+ *                 instantiatable CPU type.
  * @parse_features: Callback to parse command line arguments.
  * @reset_dump_flags: #CPUDumpFlags to use for reset logging.
  * @has_work: Callback for checking if there is work to do.
@@ -630,9 +630,10 @@ void cpu_reset(CPUState *cpu);
  * @typename: The CPU base type.
  * @cpu_model: The model string without any parameters.
  *
- * Looks up a CPU #ObjectClass matching name @cpu_model.
+ * Looks up a concrete CPU #ObjectClass matching name @cpu_model.
  *
- * Returns: A #CPUClass or %NULL if not matching class is found.
+ * Returns: A concrete #CPUClass or %NULL if no matching class is found
+ *          or if the matching class is abstract.
  */
 ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model);
 
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index ced66c2b34..c6a0c9390c 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -149,10 +149,18 @@ static bool cpu_common_has_work(CPUState *cs)
 
 ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
 {
-    CPUClass *cc = CPU_CLASS(object_class_by_name(typename));
+    ObjectClass *oc;
+    CPUClass *cc;
 
-    assert(cpu_model && cc->class_by_name);
-    return cc->class_by_name(cpu_model);
+    assert(cpu_model);
+    oc = object_class_by_name(typename);
+    cc = CPU_CLASS(oc);
+    assert(cc->class_by_name);
+    oc = cc->class_by_name(cpu_model);
+    if (oc == NULL || object_class_is_abstract(oc)) {
+        return NULL;
+    }
+    return oc;
 }
 
 static void cpu_common_parse_features(const char *typename, char *features,
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 0bb0585441..42e29816cc 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2300,8 +2300,7 @@ static ObjectClass *arm_cpu_class_by_name(const char *cpu_model)
     oc = object_class_by_name(typename);
     g_strfreev(cpuname);
     g_free(typename);
-    if (!oc || !object_class_dynamic_cast(oc, TYPE_ARM_CPU) ||
-        object_class_is_abstract(oc)) {
+    if (!oc || !object_class_dynamic_cast(oc, TYPE_ARM_CPU)) {
         return NULL;
     }
     return oc;
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 8f741f258c..4b255eade1 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -159,8 +159,7 @@ static ObjectClass *avr_cpu_class_by_name(const char *cpu_model)
     ObjectClass *oc;
 
     oc = object_class_by_name(cpu_model);
-    if (object_class_dynamic_cast(oc, TYPE_AVR_CPU) == NULL ||
-        object_class_is_abstract(oc)) {
+    if (object_class_dynamic_cast(oc, TYPE_AVR_CPU) == NULL) {
         oc = NULL;
     }
     return oc;
diff --git a/target/cris/cpu.c b/target/cris/cpu.c
index a6a93c2359..115f6e2ea2 100644
--- a/target/cris/cpu.c
+++ b/target/cris/cpu.c
@@ -95,8 +95,7 @@ static ObjectClass *cris_cpu_class_by_name(const char *cpu_model)
     typename = g_strdup_printf(CRIS_CPU_TYPE_NAME("%s"), cpu_model);
     oc = object_class_by_name(typename);
     g_free(typename);
-    if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_CRIS_CPU) ||
-                       object_class_is_abstract(oc))) {
+    if (oc != NULL && !object_class_dynamic_cast(oc, TYPE_CRIS_CPU)) {
         oc = NULL;
     }
     return oc;
diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index f155936289..5e301327d3 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -63,8 +63,7 @@ static ObjectClass *hexagon_cpu_class_by_name(const char *cpu_model)
     oc = object_class_by_name(typename);
     g_strfreev(cpuname);
     g_free(typename);
-    if (!oc || !object_class_dynamic_cast(oc, TYPE_HEXAGON_CPU) ||
-        object_class_is_abstract(oc)) {
+    if (!oc || !object_class_dynamic_cast(oc, TYPE_HEXAGON_CPU)) {
         return NULL;
     }
     return oc;
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 65f9320e34..fe2e5ecc46 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -646,8 +646,7 @@ static ObjectClass *loongarch_cpu_class_by_name(const char *cpu_model)
         }
     }
 
-    if (object_class_dynamic_cast(oc, TYPE_LOONGARCH_CPU)
-        && !object_class_is_abstract(oc)) {
+    if (object_class_dynamic_cast(oc, TYPE_LOONGARCH_CPU)) {
         return oc;
     }
     return NULL;
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 70d58471dc..004f3d6265 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -111,8 +111,7 @@ static ObjectClass *m68k_cpu_class_by_name(const char *cpu_model)
     typename = g_strdup_printf(M68K_CPU_TYPE_NAME("%s"), cpu_model);
     oc = object_class_by_name(typename);
     g_free(typename);
-    if (oc != NULL && (object_class_dynamic_cast(oc, TYPE_M68K_CPU) == NULL ||
-                       object_class_is_abstract(oc))) {
+    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_M68K_CPU) == NULL) {
         return NULL;
     }
     return oc;
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index 61d748cfdc..3bbbcc4e63 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -168,8 +168,7 @@ 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))) {
+    if (oc != NULL && !object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU)) {
         return NULL;
     }
     return oc;
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 6b93b04453..17b00eb7c0 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -619,8 +619,7 @@ static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
     oc = object_class_by_name(typename);
     g_strfreev(cpuname);
     g_free(typename);
-    if (!oc || !object_class_dynamic_cast(oc, TYPE_RISCV_CPU) ||
-        object_class_is_abstract(oc)) {
+    if (!oc || !object_class_dynamic_cast(oc, TYPE_RISCV_CPU)) {
         return NULL;
     }
     return oc;
diff --git a/target/rx/cpu.c b/target/rx/cpu.c
index 157e57da0f..c98034540d 100644
--- a/target/rx/cpu.c
+++ b/target/rx/cpu.c
@@ -111,16 +111,12 @@ 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 &&
-        !object_class_is_abstract(oc)) {
+    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_RX_CPU) != NULL) {
         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;
-    }
 
     return oc;
 }
diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index 61769ffdfa..bc112776fc 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -152,9 +152,6 @@ static ObjectClass *superh_cpu_class_by_name(const char *cpu_model)
 
     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);
diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
index 133a9ac70e..a2381b0dc1 100644
--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -140,8 +140,7 @@ 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)) {
+    if (!oc || !object_class_dynamic_cast(oc, TYPE_TRICORE_CPU)) {
         return NULL;
     }
     return oc;
diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index acaf8c905f..a31825a2b5 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -141,8 +141,7 @@ 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)) {
+    if (oc == NULL || !object_class_dynamic_cast(oc, TYPE_XTENSA_CPU)) {
         return NULL;
     }
     return oc;
-- 
2.41.0

Re: [PATCH 2/4] hw/cpu: Call object_class_is_abstract() once in cpu_class_by_name()

[Richard Henderson]

On 9/8/23 04:22, Philippe Mathieu-Daudé wrote:
> Let CPUClass::class_by_name() handlers to return abstract classes,
> and filter them once in the public cpu_class_by_name() method.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/hw/core/cpu.h  |  7 ++++---
>   hw/core/cpu-common.c   | 14 +++++++++++---
>   target/arm/cpu.c       |  3 +--
>   target/avr/cpu.c       |  3 +--
>   target/cris/cpu.c      |  3 +--
>   target/hexagon/cpu.c   |  3 +--
>   target/loongarch/cpu.c |  3 +--
>   target/m68k/cpu.c      |  3 +--
>   target/openrisc/cpu.c  |  3 +--
>   target/riscv/cpu.c     |  3 +--
>   target/rx/cpu.c        |  6 +-----
>   target/sh4/cpu.c       |  3 ---
>   target/tricore/cpu.c   |  3 +--
>   target/xtensa/cpu.c    |  3 +--
>   14 files changed, 26 insertions(+), 34 deletions(-)

Missed the cleanup for alpha, which you introduced in patch 1.

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


r~

[PATCH 3/4] hw/cpu: Introduce CPUClass::cpu_resolving_type field

[Philippe Mathieu-Daudé]

Add a field to return the QOM type name of a CPU class.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/cpu.h   | 2 ++
 hw/core/cpu-common.c    | 2 +-
 target/alpha/cpu.c      | 1 +
 target/arm/cpu.c        | 1 +
 target/avr/cpu.c        | 1 +
 target/cris/cpu.c       | 1 +
 target/hexagon/cpu.c    | 1 +
 target/hppa/cpu.c       | 1 +
 target/i386/cpu.c       | 1 +
 target/loongarch/cpu.c  | 1 +
 target/m68k/cpu.c       | 1 +
 target/microblaze/cpu.c | 1 +
 target/mips/cpu.c       | 1 +
 target/nios2/cpu.c      | 1 +
 target/openrisc/cpu.c   | 1 +
 target/ppc/cpu_init.c   | 1 +
 target/riscv/cpu.c      | 1 +
 target/rx/cpu.c         | 1 +
 target/s390x/cpu.c      | 1 +
 target/sh4/cpu.c        | 1 +
 target/sparc/cpu.c      | 1 +
 target/tricore/cpu.c    | 1 +
 target/xtensa/cpu.c     | 1 +
 23 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 129d179937..e469efd409 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -100,6 +100,7 @@ struct SysemuCPUOps;
 
 /**
  * CPUClass:
+ * @cpu_resolving_type: CPU QOM type name
  * @class_by_name: Callback to map -cpu command line model name to an
  *                 instantiatable CPU type.
  * @parse_features: Callback to parse command line arguments.
@@ -148,6 +149,7 @@ struct CPUClass {
     DeviceClass parent_class;
     /*< public >*/
 
+    const char *cpu_resolving_type;
     ObjectClass *(*class_by_name)(const char *cpu_model);
     void (*parse_features)(const char *typename, char *str, Error **errp);
 
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index c6a0c9390c..2d24261a6a 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -155,7 +155,7 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
     assert(cpu_model);
     oc = object_class_by_name(typename);
     cc = CPU_CLASS(oc);
-    assert(cc->class_by_name);
+    assert(cc->cpu_resolving_type && cc->class_by_name);
     oc = cc->class_by_name(cpu_model);
     if (oc == NULL || object_class_is_abstract(oc)) {
         return NULL;
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 351ee2e9f2..0ddea8004c 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -254,6 +254,7 @@ static void alpha_cpu_class_init(ObjectClass *oc, void *data)
     device_class_set_parent_realize(dc, alpha_cpu_realizefn,
                                     &acc->parent_realize);
 
+    cc->cpu_resolving_type = TYPE_ALPHA_CPU;
     cc->class_by_name = alpha_cpu_class_by_name;
     cc->has_work = alpha_cpu_has_work;
     cc->dump_state = alpha_cpu_dump_state;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 42e29816cc..9e51bde170 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2377,6 +2377,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
     resettable_class_set_parent_phases(rc, NULL, arm_cpu_reset_hold, NULL,
                                        &acc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_ARM_CPU;
     cc->class_by_name = arm_cpu_class_by_name;
     cc->has_work = arm_cpu_has_work;
     cc->dump_state = arm_cpu_dump_state;
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 4b255eade1..f6004169ac 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -233,6 +233,7 @@ static void avr_cpu_class_init(ObjectClass *oc, void *data)
     resettable_class_set_parent_phases(rc, NULL, avr_cpu_reset_hold, NULL,
                                        &mcc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_AVR_CPU;
     cc->class_by_name = avr_cpu_class_by_name;
 
     cc->has_work = avr_cpu_has_work;
diff --git a/target/cris/cpu.c b/target/cris/cpu.c
index 115f6e2ea2..adde4f599d 100644
--- a/target/cris/cpu.c
+++ b/target/cris/cpu.c
@@ -314,6 +314,7 @@ static void cris_cpu_class_init(ObjectClass *oc, void *data)
     resettable_class_set_parent_phases(rc, NULL, cris_cpu_reset_hold, NULL,
                                        &ccc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_CRIS_CPU;
     cc->class_by_name = cris_cpu_class_by_name;
     cc->has_work = cris_cpu_has_work;
     cc->dump_state = cris_cpu_dump_state;
diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 5e301327d3..2d4fed838d 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -381,6 +381,7 @@ static void hexagon_cpu_class_init(ObjectClass *c, void *data)
     resettable_class_set_parent_phases(rc, NULL, hexagon_cpu_reset_hold, NULL,
                                        &mcc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_HEXAGON_CPU;
     cc->class_by_name = hexagon_cpu_class_by_name;
     cc->has_work = hexagon_cpu_has_work;
     cc->dump_state = hexagon_dump_state;
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 11022f9c99..47950a15ae 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -192,6 +192,7 @@ static void hppa_cpu_class_init(ObjectClass *oc, void *data)
     device_class_set_parent_realize(dc, hppa_cpu_realizefn,
                                     &acc->parent_realize);
 
+    cc->cpu_resolving_type = TYPE_HPPA_CPU;
     cc->class_by_name = hppa_cpu_class_by_name;
     cc->has_work = hppa_cpu_has_work;
     cc->dump_state = hppa_cpu_dump_state;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 00f913b638..9979464420 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7951,6 +7951,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
                                        &xcc->parent_phases);
     cc->reset_dump_flags = CPU_DUMP_FPU | CPU_DUMP_CCOP;
 
+    cc->cpu_resolving_type = TYPE_X86_CPU;
     cc->class_by_name = x86_cpu_class_by_name;
     cc->parse_features = x86_cpu_parse_featurestr;
     cc->has_work = x86_cpu_has_work;
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index fe2e5ecc46..189dfd32d1 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -743,6 +743,7 @@ static void loongarch_cpu_class_init(ObjectClass *c, void *data)
     resettable_class_set_parent_phases(rc, NULL, loongarch_cpu_reset_hold, NULL,
                                        &lacc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_LOONGARCH_CPU;
     cc->class_by_name = loongarch_cpu_class_by_name;
     cc->has_work = loongarch_cpu_has_work;
     cc->dump_state = loongarch_cpu_dump_state;
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 004f3d6265..bd7bb103d7 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -558,6 +558,7 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data)
     resettable_class_set_parent_phases(rc, NULL, m68k_cpu_reset_hold, NULL,
                                        &mcc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_M68K_CPU;
     cc->class_by_name = m68k_cpu_class_by_name;
     cc->has_work = m68k_cpu_has_work;
     cc->dump_state = m68k_cpu_dump_state;
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 03c2c4db1f..bb5f2c1f00 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -414,6 +414,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
     resettable_class_set_parent_phases(rc, NULL, mb_cpu_reset_hold, NULL,
                                        &mcc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_MICROBLAZE_CPU;
     cc->class_by_name = mb_cpu_class_by_name;
     cc->has_work = mb_cpu_has_work;
 
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 63da1948fd..649147df2e 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -578,6 +578,7 @@ static void mips_cpu_class_init(ObjectClass *c, void *data)
     resettable_class_set_parent_phases(rc, NULL, mips_cpu_reset_hold, NULL,
                                        &mcc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_MIPS_CPU;
     cc->class_by_name = mips_cpu_class_by_name;
     cc->has_work = mips_cpu_has_work;
     cc->dump_state = mips_cpu_dump_state;
diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
index bc5cbf81c2..fc7c6a83ee 100644
--- a/target/nios2/cpu.c
+++ b/target/nios2/cpu.c
@@ -381,6 +381,7 @@ static void nios2_cpu_class_init(ObjectClass *oc, void *data)
     resettable_class_set_parent_phases(rc, NULL, nios2_cpu_reset_hold, NULL,
                                        &ncc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_NIOS2_CPU;
     cc->class_by_name = nios2_cpu_class_by_name;
     cc->has_work = nios2_cpu_has_work;
     cc->dump_state = nios2_cpu_dump_state;
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index 3bbbcc4e63..5e1e0576e0 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -243,6 +243,7 @@ static void openrisc_cpu_class_init(ObjectClass *oc, void *data)
     resettable_class_set_parent_phases(rc, NULL, openrisc_cpu_reset_hold, NULL,
                                        &occ->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_OPENRISC_CPU;
     cc->class_by_name = openrisc_cpu_class_by_name;
     cc->has_work = openrisc_cpu_has_work;
     cc->dump_state = openrisc_cpu_dump_state;
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 02b7aad9b0..bc106d01a2 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7357,6 +7357,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     resettable_class_set_parent_phases(rc, NULL, ppc_cpu_reset_hold, NULL,
                                        &pcc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_POWERPC_CPU;
     cc->class_by_name = ppc_cpu_class_by_name;
     cc->has_work = ppc_cpu_has_work;
     cc->dump_state = ppc_cpu_dump_state;
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 17b00eb7c0..e8f04ef82b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2130,6 +2130,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
     resettable_class_set_parent_phases(rc, NULL, riscv_cpu_reset_hold, NULL,
                                        &mcc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_RISCV_CPU;
     cc->class_by_name = riscv_cpu_class_by_name;
     cc->has_work = riscv_cpu_has_work;
     cc->dump_state = riscv_cpu_dump_state;
diff --git a/target/rx/cpu.c b/target/rx/cpu.c
index c98034540d..2a6df418a8 100644
--- a/target/rx/cpu.c
+++ b/target/rx/cpu.c
@@ -222,6 +222,7 @@ static void rx_cpu_class_init(ObjectClass *klass, void *data)
     resettable_class_set_parent_phases(rc, NULL, rx_cpu_reset_hold, NULL,
                                        &rcc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_RX_CPU;
     cc->class_by_name = rx_cpu_class_by_name;
     cc->has_work = rx_cpu_has_work;
     cc->dump_state = rx_cpu_dump_state;
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index df167493c3..bcba466bb4 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -336,6 +336,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
     device_class_set_parent_reset(dc, s390_cpu_reset_full, &scc->parent_reset);
 
     scc->reset = s390_cpu_reset;
+    cc->cpu_resolving_type = TYPE_S390_CPU;
     cc->class_by_name = s390_cpu_class_by_name,
     cc->has_work = s390_cpu_has_work;
     cc->dump_state = s390_cpu_dump_state;
diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index bc112776fc..17c87f15f2 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -283,6 +283,7 @@ static void superh_cpu_class_init(ObjectClass *oc, void *data)
     resettable_class_set_parent_phases(rc, NULL, superh_cpu_reset_hold, NULL,
                                        &scc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_SUPERH_CPU;
     cc->class_by_name = superh_cpu_class_by_name;
     cc->has_work = superh_cpu_has_work;
     cc->dump_state = superh_cpu_dump_state;
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 130ab8f578..e41a9f4ee2 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -902,6 +902,7 @@ static void sparc_cpu_class_init(ObjectClass *oc, void *data)
     resettable_class_set_parent_phases(rc, NULL, sparc_cpu_reset_hold, NULL,
                                        &scc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_SPARC_CPU;
     cc->class_by_name = sparc_cpu_class_by_name;
     cc->parse_features = sparc_cpu_parse_features;
     cc->has_work = sparc_cpu_has_work;
diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
index a2381b0dc1..ffe5158786 100644
--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -202,6 +202,7 @@ static void tricore_cpu_class_init(ObjectClass *c, void *data)
 
     resettable_class_set_parent_phases(rc, NULL, tricore_cpu_reset_hold, NULL,
                                        &mcc->parent_phases);
+    cc->cpu_resolving_type = TYPE_TRICORE_CPU;
     cc->class_by_name = tricore_cpu_class_by_name;
     cc->has_work = tricore_cpu_has_work;
 
diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index a31825a2b5..13bed05d0c 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -252,6 +252,7 @@ static void xtensa_cpu_class_init(ObjectClass *oc, void *data)
     resettable_class_set_parent_phases(rc, NULL, xtensa_cpu_reset_hold, NULL,
                                        &xcc->parent_phases);
 
+    cc->cpu_resolving_type = TYPE_XTENSA_CPU;
     cc->class_by_name = xtensa_cpu_class_by_name;
     cc->has_work = xtensa_cpu_has_work;
     cc->dump_state = xtensa_cpu_dump_state;
-- 
2.41.0

Re: [PATCH 3/4] hw/cpu: Introduce CPUClass::cpu_resolving_type field

[Richard Henderson]

On 9/8/23 04:22, Philippe Mathieu-Daudé wrote:
> Add a field to return the QOM type name of a CPU class.

It isn't the type name of the cpu class, it's the name of the base class that one 
particular use case expects.

I don't think this is a good idea.


r~

Re: [PATCH 3/4] hw/cpu: Introduce CPUClass::cpu_resolving_type field

[Gavin Shan]

Hi Philippe,

On 9/8/23 21:22, Philippe Mathieu-Daudé wrote:
> Add a field to return the QOM type name of a CPU class.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/core/cpu.h   | 2 ++
>   hw/core/cpu-common.c    | 2 +-
>   target/alpha/cpu.c      | 1 +
>   target/arm/cpu.c        | 1 +
>   target/avr/cpu.c        | 1 +
>   target/cris/cpu.c       | 1 +
>   target/hexagon/cpu.c    | 1 +
>   target/hppa/cpu.c       | 1 +
>   target/i386/cpu.c       | 1 +
>   target/loongarch/cpu.c  | 1 +
>   target/m68k/cpu.c       | 1 +
>   target/microblaze/cpu.c | 1 +
>   target/mips/cpu.c       | 1 +
>   target/nios2/cpu.c      | 1 +
>   target/openrisc/cpu.c   | 1 +
>   target/ppc/cpu_init.c   | 1 +
>   target/riscv/cpu.c      | 1 +
>   target/rx/cpu.c         | 1 +
>   target/s390x/cpu.c      | 1 +
>   target/sh4/cpu.c        | 1 +
>   target/sparc/cpu.c      | 1 +
>   target/tricore/cpu.c    | 1 +
>   target/xtensa/cpu.c     | 1 +
>   23 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 129d179937..e469efd409 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -100,6 +100,7 @@ struct SysemuCPUOps;
>   
>   /**
>    * CPUClass:
> + * @cpu_resolving_type: CPU QOM type name
>    * @class_by_name: Callback to map -cpu command line model name to an
>    *                 instantiatable CPU type.
>    * @parse_features: Callback to parse command line arguments.
> @@ -148,6 +149,7 @@ struct CPUClass {
>       DeviceClass parent_class;
>       /*< public >*/
>   
> +    const char *cpu_resolving_type;
>       ObjectClass *(*class_by_name)(const char *cpu_model);
>       void (*parse_features)(const char *typename, char *str, Error **errp);
>   

The question is why not use CPU_RESOLVING_TYPE directly? It seems CPU_RESOLVING_TYPE
is exactly what you want here. Besides, I guess the changes can be squeezed into two
patches (commits) as below:

PATCH[1] target/alpha: Tidy up alpha_cpu_class_by_name()
PATCH[2] Move the checks (oc == NULL || object_class_is_abstract() || !object_class_dynamic_cast())
          from individual targets to hw/core/cpu-common.c::cpu_class_by_name()

I rebase my series of '[PATCH v3 00/32] Unified CPU type check' on top of yours. Please
let me know if I need to include your patch into my v4 series for review. In that case,
I can include your patches with above changes applied.

Thanks,
Gavin

> diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
> index c6a0c9390c..2d24261a6a 100644
> --- a/hw/core/cpu-common.c
> +++ b/hw/core/cpu-common.c
> @@ -155,7 +155,7 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
>       assert(cpu_model);
>       oc = object_class_by_name(typename);
>       cc = CPU_CLASS(oc);
> -    assert(cc->class_by_name);
> +    assert(cc->cpu_resolving_type && cc->class_by_name);
>       oc = cc->class_by_name(cpu_model);
>       if (oc == NULL || object_class_is_abstract(oc)) {
>           return NULL;
> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
> index 351ee2e9f2..0ddea8004c 100644
> --- a/target/alpha/cpu.c
> +++ b/target/alpha/cpu.c
> @@ -254,6 +254,7 @@ static void alpha_cpu_class_init(ObjectClass *oc, void *data)
>       device_class_set_parent_realize(dc, alpha_cpu_realizefn,
>                                       &acc->parent_realize);
>   
> +    cc->cpu_resolving_type = TYPE_ALPHA_CPU;
>       cc->class_by_name = alpha_cpu_class_by_name;
>       cc->has_work = alpha_cpu_has_work;
>       cc->dump_state = alpha_cpu_dump_state;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 42e29816cc..9e51bde170 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -2377,6 +2377,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
>       resettable_class_set_parent_phases(rc, NULL, arm_cpu_reset_hold, NULL,
>                                          &acc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_ARM_CPU;
>       cc->class_by_name = arm_cpu_class_by_name;
>       cc->has_work = arm_cpu_has_work;
>       cc->dump_state = arm_cpu_dump_state;
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index 4b255eade1..f6004169ac 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -233,6 +233,7 @@ static void avr_cpu_class_init(ObjectClass *oc, void *data)
>       resettable_class_set_parent_phases(rc, NULL, avr_cpu_reset_hold, NULL,
>                                          &mcc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_AVR_CPU;
>       cc->class_by_name = avr_cpu_class_by_name;
>   
>       cc->has_work = avr_cpu_has_work;
> diff --git a/target/cris/cpu.c b/target/cris/cpu.c
> index 115f6e2ea2..adde4f599d 100644
> --- a/target/cris/cpu.c
> +++ b/target/cris/cpu.c
> @@ -314,6 +314,7 @@ static void cris_cpu_class_init(ObjectClass *oc, void *data)
>       resettable_class_set_parent_phases(rc, NULL, cris_cpu_reset_hold, NULL,
>                                          &ccc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_CRIS_CPU;
>       cc->class_by_name = cris_cpu_class_by_name;
>       cc->has_work = cris_cpu_has_work;
>       cc->dump_state = cris_cpu_dump_state;
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
> index 5e301327d3..2d4fed838d 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -381,6 +381,7 @@ static void hexagon_cpu_class_init(ObjectClass *c, void *data)
>       resettable_class_set_parent_phases(rc, NULL, hexagon_cpu_reset_hold, NULL,
>                                          &mcc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_HEXAGON_CPU;
>       cc->class_by_name = hexagon_cpu_class_by_name;
>       cc->has_work = hexagon_cpu_has_work;
>       cc->dump_state = hexagon_dump_state;
> diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
> index 11022f9c99..47950a15ae 100644
> --- a/target/hppa/cpu.c
> +++ b/target/hppa/cpu.c
> @@ -192,6 +192,7 @@ static void hppa_cpu_class_init(ObjectClass *oc, void *data)
>       device_class_set_parent_realize(dc, hppa_cpu_realizefn,
>                                       &acc->parent_realize);
>   
> +    cc->cpu_resolving_type = TYPE_HPPA_CPU;
>       cc->class_by_name = hppa_cpu_class_by_name;
>       cc->has_work = hppa_cpu_has_work;
>       cc->dump_state = hppa_cpu_dump_state;
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 00f913b638..9979464420 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -7951,6 +7951,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>                                          &xcc->parent_phases);
>       cc->reset_dump_flags = CPU_DUMP_FPU | CPU_DUMP_CCOP;
>   
> +    cc->cpu_resolving_type = TYPE_X86_CPU;
>       cc->class_by_name = x86_cpu_class_by_name;
>       cc->parse_features = x86_cpu_parse_featurestr;
>       cc->has_work = x86_cpu_has_work;
> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
> index fe2e5ecc46..189dfd32d1 100644
> --- a/target/loongarch/cpu.c
> +++ b/target/loongarch/cpu.c
> @@ -743,6 +743,7 @@ static void loongarch_cpu_class_init(ObjectClass *c, void *data)
>       resettable_class_set_parent_phases(rc, NULL, loongarch_cpu_reset_hold, NULL,
>                                          &lacc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_LOONGARCH_CPU;
>       cc->class_by_name = loongarch_cpu_class_by_name;
>       cc->has_work = loongarch_cpu_has_work;
>       cc->dump_state = loongarch_cpu_dump_state;
> diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
> index 004f3d6265..bd7bb103d7 100644
> --- a/target/m68k/cpu.c
> +++ b/target/m68k/cpu.c
> @@ -558,6 +558,7 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data)
>       resettable_class_set_parent_phases(rc, NULL, m68k_cpu_reset_hold, NULL,
>                                          &mcc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_M68K_CPU;
>       cc->class_by_name = m68k_cpu_class_by_name;
>       cc->has_work = m68k_cpu_has_work;
>       cc->dump_state = m68k_cpu_dump_state;
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 03c2c4db1f..bb5f2c1f00 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -414,6 +414,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
>       resettable_class_set_parent_phases(rc, NULL, mb_cpu_reset_hold, NULL,
>                                          &mcc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_MICROBLAZE_CPU;
>       cc->class_by_name = mb_cpu_class_by_name;
>       cc->has_work = mb_cpu_has_work;
>   
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index 63da1948fd..649147df2e 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -578,6 +578,7 @@ static void mips_cpu_class_init(ObjectClass *c, void *data)
>       resettable_class_set_parent_phases(rc, NULL, mips_cpu_reset_hold, NULL,
>                                          &mcc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_MIPS_CPU;
>       cc->class_by_name = mips_cpu_class_by_name;
>       cc->has_work = mips_cpu_has_work;
>       cc->dump_state = mips_cpu_dump_state;
> diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
> index bc5cbf81c2..fc7c6a83ee 100644
> --- a/target/nios2/cpu.c
> +++ b/target/nios2/cpu.c
> @@ -381,6 +381,7 @@ static void nios2_cpu_class_init(ObjectClass *oc, void *data)
>       resettable_class_set_parent_phases(rc, NULL, nios2_cpu_reset_hold, NULL,
>                                          &ncc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_NIOS2_CPU;
>       cc->class_by_name = nios2_cpu_class_by_name;
>       cc->has_work = nios2_cpu_has_work;
>       cc->dump_state = nios2_cpu_dump_state;
> diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
> index 3bbbcc4e63..5e1e0576e0 100644
> --- a/target/openrisc/cpu.c
> +++ b/target/openrisc/cpu.c
> @@ -243,6 +243,7 @@ static void openrisc_cpu_class_init(ObjectClass *oc, void *data)
>       resettable_class_set_parent_phases(rc, NULL, openrisc_cpu_reset_hold, NULL,
>                                          &occ->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_OPENRISC_CPU;
>       cc->class_by_name = openrisc_cpu_class_by_name;
>       cc->has_work = openrisc_cpu_has_work;
>       cc->dump_state = openrisc_cpu_dump_state;
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 02b7aad9b0..bc106d01a2 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -7357,6 +7357,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
>       resettable_class_set_parent_phases(rc, NULL, ppc_cpu_reset_hold, NULL,
>                                          &pcc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_POWERPC_CPU;
>       cc->class_by_name = ppc_cpu_class_by_name;
>       cc->has_work = ppc_cpu_has_work;
>       cc->dump_state = ppc_cpu_dump_state;
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 17b00eb7c0..e8f04ef82b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -2130,6 +2130,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
>       resettable_class_set_parent_phases(rc, NULL, riscv_cpu_reset_hold, NULL,
>                                          &mcc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_RISCV_CPU;
>       cc->class_by_name = riscv_cpu_class_by_name;
>       cc->has_work = riscv_cpu_has_work;
>       cc->dump_state = riscv_cpu_dump_state;
> diff --git a/target/rx/cpu.c b/target/rx/cpu.c
> index c98034540d..2a6df418a8 100644
> --- a/target/rx/cpu.c
> +++ b/target/rx/cpu.c
> @@ -222,6 +222,7 @@ static void rx_cpu_class_init(ObjectClass *klass, void *data)
>       resettable_class_set_parent_phases(rc, NULL, rx_cpu_reset_hold, NULL,
>                                          &rcc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_RX_CPU;
>       cc->class_by_name = rx_cpu_class_by_name;
>       cc->has_work = rx_cpu_has_work;
>       cc->dump_state = rx_cpu_dump_state;
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index df167493c3..bcba466bb4 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -336,6 +336,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
>       device_class_set_parent_reset(dc, s390_cpu_reset_full, &scc->parent_reset);
>   
>       scc->reset = s390_cpu_reset;
> +    cc->cpu_resolving_type = TYPE_S390_CPU;
>       cc->class_by_name = s390_cpu_class_by_name,
>       cc->has_work = s390_cpu_has_work;
>       cc->dump_state = s390_cpu_dump_state;
> diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
> index bc112776fc..17c87f15f2 100644
> --- a/target/sh4/cpu.c
> +++ b/target/sh4/cpu.c
> @@ -283,6 +283,7 @@ static void superh_cpu_class_init(ObjectClass *oc, void *data)
>       resettable_class_set_parent_phases(rc, NULL, superh_cpu_reset_hold, NULL,
>                                          &scc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_SUPERH_CPU;
>       cc->class_by_name = superh_cpu_class_by_name;
>       cc->has_work = superh_cpu_has_work;
>       cc->dump_state = superh_cpu_dump_state;
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index 130ab8f578..e41a9f4ee2 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -902,6 +902,7 @@ static void sparc_cpu_class_init(ObjectClass *oc, void *data)
>       resettable_class_set_parent_phases(rc, NULL, sparc_cpu_reset_hold, NULL,
>                                          &scc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_SPARC_CPU;
>       cc->class_by_name = sparc_cpu_class_by_name;
>       cc->parse_features = sparc_cpu_parse_features;
>       cc->has_work = sparc_cpu_has_work;
> diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
> index a2381b0dc1..ffe5158786 100644
> --- a/target/tricore/cpu.c
> +++ b/target/tricore/cpu.c
> @@ -202,6 +202,7 @@ static void tricore_cpu_class_init(ObjectClass *c, void *data)
>   
>       resettable_class_set_parent_phases(rc, NULL, tricore_cpu_reset_hold, NULL,
>                                          &mcc->parent_phases);
> +    cc->cpu_resolving_type = TYPE_TRICORE_CPU;
>       cc->class_by_name = tricore_cpu_class_by_name;
>       cc->has_work = tricore_cpu_has_work;
>   
> diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
> index a31825a2b5..13bed05d0c 100644
> --- a/target/xtensa/cpu.c
> +++ b/target/xtensa/cpu.c
> @@ -252,6 +252,7 @@ static void xtensa_cpu_class_init(ObjectClass *oc, void *data)
>       resettable_class_set_parent_phases(rc, NULL, xtensa_cpu_reset_hold, NULL,
>                                          &xcc->parent_phases);
>   
> +    cc->cpu_resolving_type = TYPE_XTENSA_CPU;
>       cc->class_by_name = xtensa_cpu_class_by_name;
>       cc->has_work = xtensa_cpu_has_work;
>       cc->dump_state = xtensa_cpu_dump_state;

Re: [PATCH 3/4] hw/cpu: Introduce CPUClass::cpu_resolving_type field

[Philippe Mathieu-Daudé]

On 11/9/23 01:28, Gavin Shan wrote:
> Hi Philippe,
> 
> On 9/8/23 21:22, Philippe Mathieu-Daudé wrote:
>> Add a field to return the QOM type name of a CPU class.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/hw/core/cpu.h   | 2 ++
>>   hw/core/cpu-common.c    | 2 +-
>>   target/alpha/cpu.c      | 1 +
>>   target/arm/cpu.c        | 1 +
>>   target/avr/cpu.c        | 1 +
>>   target/cris/cpu.c       | 1 +
>>   target/hexagon/cpu.c    | 1 +
>>   target/hppa/cpu.c       | 1 +
>>   target/i386/cpu.c       | 1 +
>>   target/loongarch/cpu.c  | 1 +
>>   target/m68k/cpu.c       | 1 +
>>   target/microblaze/cpu.c | 1 +
>>   target/mips/cpu.c       | 1 +
>>   target/nios2/cpu.c      | 1 +
>>   target/openrisc/cpu.c   | 1 +
>>   target/ppc/cpu_init.c   | 1 +
>>   target/riscv/cpu.c      | 1 +
>>   target/rx/cpu.c         | 1 +
>>   target/s390x/cpu.c      | 1 +
>>   target/sh4/cpu.c        | 1 +
>>   target/sparc/cpu.c      | 1 +
>>   target/tricore/cpu.c    | 1 +
>>   target/xtensa/cpu.c     | 1 +
>>   23 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>> index 129d179937..e469efd409 100644
>> --- a/include/hw/core/cpu.h
>> +++ b/include/hw/core/cpu.h
>> @@ -100,6 +100,7 @@ struct SysemuCPUOps;
>>   /**
>>    * CPUClass:
>> + * @cpu_resolving_type: CPU QOM type name
>>    * @class_by_name: Callback to map -cpu command line model name to an
>>    *                 instantiatable CPU type.
>>    * @parse_features: Callback to parse command line arguments.
>> @@ -148,6 +149,7 @@ struct CPUClass {
>>       DeviceClass parent_class;
>>       /*< public >*/
>> +    const char *cpu_resolving_type;
>>       ObjectClass *(*class_by_name)(const char *cpu_model);
>>       void (*parse_features)(const char *typename, char *str, Error 
>> **errp);
> 
> The question is why not use CPU_RESOLVING_TYPE directly? It seems 
> CPU_RESOLVING_TYPE
> is exactly what you want here.

Unfortunately CPU_RESOLVING_TYPE is target-specific, we want
hw/core/cpu-common.c to be target-agnostic (build once for all
targets). This is particularly important in the context of
heterogeneous QEMU, where a single binary will be able to create
CPUs from different targets.

> Besides, I guess the changes can be 
> squeezed into two
> patches (commits) as below:
> 
> PATCH[1] target/alpha: Tidy up alpha_cpu_class_by_name()
> PATCH[2] Move the checks (oc == NULL || object_class_is_abstract() || 
> !object_class_dynamic_cast())
>           from individual targets to 
> hw/core/cpu-common.c::cpu_class_by_name()
> 
> I rebase my series of '[PATCH v3 00/32] Unified CPU type check' on top 
> of yours. Please
> let me know if I need to include your patch into my v4 series for 
> review. In that case,
> I can include your patches with above changes applied.
> 
> Thanks,
> Gavin
> 
>> diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
>> index c6a0c9390c..2d24261a6a 100644
>> --- a/hw/core/cpu-common.c
>> +++ b/hw/core/cpu-common.c
>> @@ -155,7 +155,7 @@ ObjectClass *cpu_class_by_name(const char 
>> *typename, const char *cpu_model)
>>       assert(cpu_model);
>>       oc = object_class_by_name(typename);
>>       cc = CPU_CLASS(oc);
>> -    assert(cc->class_by_name);
>> +    assert(cc->cpu_resolving_type && cc->class_by_name);
>>       oc = cc->class_by_name(cpu_model);
>>       if (oc == NULL || object_class_is_abstract(oc)) {
>>           return NULL;

Re: [PATCH 3/4] hw/cpu: Introduce CPUClass::cpu_resolving_type field

[Igor Mammedov]

On Mon, 11 Sep 2023 11:43:00 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> On 11/9/23 01:28, Gavin Shan wrote:
> > Hi Philippe,
> > 
> > On 9/8/23 21:22, Philippe Mathieu-Daudé wrote:  
> >> Add a field to return the QOM type name of a CPU class.
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >> ---
> >>   include/hw/core/cpu.h   | 2 ++
> >>   hw/core/cpu-common.c    | 2 +-
> >>   target/alpha/cpu.c      | 1 +
> >>   target/arm/cpu.c        | 1 +
> >>   target/avr/cpu.c        | 1 +
> >>   target/cris/cpu.c       | 1 +
> >>   target/hexagon/cpu.c    | 1 +
> >>   target/hppa/cpu.c       | 1 +
> >>   target/i386/cpu.c       | 1 +
> >>   target/loongarch/cpu.c  | 1 +
> >>   target/m68k/cpu.c       | 1 +
> >>   target/microblaze/cpu.c | 1 +
> >>   target/mips/cpu.c       | 1 +
> >>   target/nios2/cpu.c      | 1 +
> >>   target/openrisc/cpu.c   | 1 +
> >>   target/ppc/cpu_init.c   | 1 +
> >>   target/riscv/cpu.c      | 1 +
> >>   target/rx/cpu.c         | 1 +
> >>   target/s390x/cpu.c      | 1 +
> >>   target/sh4/cpu.c        | 1 +
> >>   target/sparc/cpu.c      | 1 +
> >>   target/tricore/cpu.c    | 1 +
> >>   target/xtensa/cpu.c     | 1 +
> >>   23 files changed, 24 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> >> index 129d179937..e469efd409 100644
> >> --- a/include/hw/core/cpu.h
> >> +++ b/include/hw/core/cpu.h
> >> @@ -100,6 +100,7 @@ struct SysemuCPUOps;
> >>   /**
> >>    * CPUClass:
> >> + * @cpu_resolving_type: CPU QOM type name
> >>    * @class_by_name: Callback to map -cpu command line model name to an
> >>    *                 instantiatable CPU type.
> >>    * @parse_features: Callback to parse command line arguments.
> >> @@ -148,6 +149,7 @@ struct CPUClass {
> >>       DeviceClass parent_class;
> >>       /*< public >*/
> >> +    const char *cpu_resolving_type;
> >>       ObjectClass *(*class_by_name)(const char *cpu_model);
> >>       void (*parse_features)(const char *typename, char *str, Error 
> >> **errp);  
> > 
> > The question is why not use CPU_RESOLVING_TYPE directly? It seems 
> > CPU_RESOLVING_TYPE
> > is exactly what you want here.  
> 
> Unfortunately CPU_RESOLVING_TYPE is target-specific, we want
> hw/core/cpu-common.c to be target-agnostic (build once for all
> targets). This is particularly important in the context of
> heterogeneous QEMU, where a single binary will be able to create
> CPUs from different targets.

Well cpu model resolving ain't going to work with heterogeneous env.
But otherwise it's argument towards dropping CPU model and use
cpu types directly (then we can get rid all this resolving nonsense).

Though for Gavin's purposes, given existing cpu type naming convention
it's sufficient to just cut of the last 2 '-' from typename to get
cpumodel (target independent and no need for resolving type). 


> 
> > Besides, I guess the changes can be 
> > squeezed into two
> > patches (commits) as below:
> > 
> > PATCH[1] target/alpha: Tidy up alpha_cpu_class_by_name()
> > PATCH[2] Move the checks (oc == NULL || object_class_is_abstract() || 
> > !object_class_dynamic_cast())
> >           from individual targets to 
> > hw/core/cpu-common.c::cpu_class_by_name()
> > 
> > I rebase my series of '[PATCH v3 00/32] Unified CPU type check' on top 
> > of yours. Please
> > let me know if I need to include your patch into my v4 series for 
> > review. In that case,
> > I can include your patches with above changes applied.
> > 
> > Thanks,
> > Gavin
> >   
> >> diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
> >> index c6a0c9390c..2d24261a6a 100644
> >> --- a/hw/core/cpu-common.c
> >> +++ b/hw/core/cpu-common.c
> >> @@ -155,7 +155,7 @@ ObjectClass *cpu_class_by_name(const char 
> >> *typename, const char *cpu_model)
> >>       assert(cpu_model);
> >>       oc = object_class_by_name(typename);
> >>       cc = CPU_CLASS(oc);
> >> -    assert(cc->class_by_name);
> >> +    assert(cc->cpu_resolving_type && cc->class_by_name);
> >>       oc = cc->class_by_name(cpu_model);
> >>       if (oc == NULL || object_class_is_abstract(oc)) {
> >>           return NULL;  
> 
> 

Re: [PATCH 3/4] hw/cpu: Introduce CPUClass::cpu_resolving_type field

[Gavin Shan]

On 9/11/23 19:43, Philippe Mathieu-Daudé wrote:
> On 11/9/23 01:28, Gavin Shan wrote:
>> On 9/8/23 21:22, Philippe Mathieu-Daudé wrote:
>>> Add a field to return the QOM type name of a CPU class.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   include/hw/core/cpu.h   | 2 ++
>>>   hw/core/cpu-common.c    | 2 +-
>>>   target/alpha/cpu.c      | 1 +
>>>   target/arm/cpu.c        | 1 +
>>>   target/avr/cpu.c        | 1 +
>>>   target/cris/cpu.c       | 1 +
>>>   target/hexagon/cpu.c    | 1 +
>>>   target/hppa/cpu.c       | 1 +
>>>   target/i386/cpu.c       | 1 +
>>>   target/loongarch/cpu.c  | 1 +
>>>   target/m68k/cpu.c       | 1 +
>>>   target/microblaze/cpu.c | 1 +
>>>   target/mips/cpu.c       | 1 +
>>>   target/nios2/cpu.c      | 1 +
>>>   target/openrisc/cpu.c   | 1 +
>>>   target/ppc/cpu_init.c   | 1 +
>>>   target/riscv/cpu.c      | 1 +
>>>   target/rx/cpu.c         | 1 +
>>>   target/s390x/cpu.c      | 1 +
>>>   target/sh4/cpu.c        | 1 +
>>>   target/sparc/cpu.c      | 1 +
>>>   target/tricore/cpu.c    | 1 +
>>>   target/xtensa/cpu.c     | 1 +
>>>   23 files changed, 24 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>>> index 129d179937..e469efd409 100644
>>> --- a/include/hw/core/cpu.h
>>> +++ b/include/hw/core/cpu.h
>>> @@ -100,6 +100,7 @@ struct SysemuCPUOps;
>>>   /**
>>>    * CPUClass:
>>> + * @cpu_resolving_type: CPU QOM type name
>>>    * @class_by_name: Callback to map -cpu command line model name to an
>>>    *                 instantiatable CPU type.
>>>    * @parse_features: Callback to parse command line arguments.
>>> @@ -148,6 +149,7 @@ struct CPUClass {
>>>       DeviceClass parent_class;
>>>       /*< public >*/
>>> +    const char *cpu_resolving_type;
>>>       ObjectClass *(*class_by_name)(const char *cpu_model);
>>>       void (*parse_features)(const char *typename, char *str, Error **errp);
>>
>> The question is why not use CPU_RESOLVING_TYPE directly? It seems CPU_RESOLVING_TYPE
>> is exactly what you want here.
> 
> Unfortunately CPU_RESOLVING_TYPE is target-specific, we want
> hw/core/cpu-common.c to be target-agnostic (build once for all
> targets). This is particularly important in the context of
> heterogeneous QEMU, where a single binary will be able to create
> CPUs from different targets.
> 

CPU_RESOLVING_TYPE and CPUClass::cpu_resolving_type is duplicate to
each other. There are two options I can figure out to avoid the
duplication.

(a) move cpu_class_by_name() from hw/core/cpu-common.c to cpu.c, so that
     CPU_RESOLVING_TYPE can be seen. cpu.c::list_cpus() is the example.

(b) remove hw/core/cpu-common.c::cpu_calss_by_name() and squeeze its
     logic to cpu.c::parse_cpu_option() since there are not too much
     users for it. target/arm and target/s390 needs some tweaks so that
     hw/core/cpu-common.c::cpu_calss_by_name() can be removed.

     [gshan@gshan q]$ git grep \ cpu_class_by_name\(
     cpu.c:    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
     target/arm/arm-qmp-cmds.c:    oc = cpu_class_by_name(TYPE_ARM_CPU, model->name);
     target/s390x/cpu_models_sysemu.c:    oc = cpu_class_by_name(TYPE_S390_CPU, info->name);

When option (b) is taken, this series to have the checks against @oc
in hw/core/cpu-common.c::cpu_calss_by_name() becomes non-sense. Instead,
we need the same (and complete) checks in CPUClass::class_by_name() for
individual targets. Further more, an inline helper can be provided to do
the check in CPUClass::class_by_name() for individual targets.

    include/hw/core/cpu.h

    static inline bool cpu_class_is_valid(ObjectClass *oc, const char *parent)
    {
        if (!object_class_dynamic_cast(oc, parent) ||
            object_class_is_abstract(oc)) {
            return false;
        }

        return true;
    }

Thanks,
Gavin

Re: [PATCH 3/4] hw/cpu: Introduce CPUClass::cpu_resolving_type field

[Gavin Shan]

Hi Philippe,

On 9/12/23 08:40, Gavin Shan wrote:
> On 9/11/23 19:43, Philippe Mathieu-Daudé wrote:
>> On 11/9/23 01:28, Gavin Shan wrote:
>>> On 9/8/23 21:22, Philippe Mathieu-Daudé wrote:
>>>> Add a field to return the QOM type name of a CPU class.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>>   include/hw/core/cpu.h   | 2 ++
>>>>   hw/core/cpu-common.c    | 2 +-
>>>>   target/alpha/cpu.c      | 1 +
>>>>   target/arm/cpu.c        | 1 +
>>>>   target/avr/cpu.c        | 1 +
>>>>   target/cris/cpu.c       | 1 +
>>>>   target/hexagon/cpu.c    | 1 +
>>>>   target/hppa/cpu.c       | 1 +
>>>>   target/i386/cpu.c       | 1 +
>>>>   target/loongarch/cpu.c  | 1 +
>>>>   target/m68k/cpu.c       | 1 +
>>>>   target/microblaze/cpu.c | 1 +
>>>>   target/mips/cpu.c       | 1 +
>>>>   target/nios2/cpu.c      | 1 +
>>>>   target/openrisc/cpu.c   | 1 +
>>>>   target/ppc/cpu_init.c   | 1 +
>>>>   target/riscv/cpu.c      | 1 +
>>>>   target/rx/cpu.c         | 1 +
>>>>   target/s390x/cpu.c      | 1 +
>>>>   target/sh4/cpu.c        | 1 +
>>>>   target/sparc/cpu.c      | 1 +
>>>>   target/tricore/cpu.c    | 1 +
>>>>   target/xtensa/cpu.c     | 1 +
>>>>   23 files changed, 24 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>>>> index 129d179937..e469efd409 100644
>>>> --- a/include/hw/core/cpu.h
>>>> +++ b/include/hw/core/cpu.h
>>>> @@ -100,6 +100,7 @@ struct SysemuCPUOps;
>>>>   /**
>>>>    * CPUClass:
>>>> + * @cpu_resolving_type: CPU QOM type name
>>>>    * @class_by_name: Callback to map -cpu command line model name to an
>>>>    *                 instantiatable CPU type.
>>>>    * @parse_features: Callback to parse command line arguments.
>>>> @@ -148,6 +149,7 @@ struct CPUClass {
>>>>       DeviceClass parent_class;
>>>>       /*< public >*/
>>>> +    const char *cpu_resolving_type;
>>>>       ObjectClass *(*class_by_name)(const char *cpu_model);
>>>>       void (*parse_features)(const char *typename, char *str, Error **errp);
>>>
>>> The question is why not use CPU_RESOLVING_TYPE directly? It seems CPU_RESOLVING_TYPE
>>> is exactly what you want here.
>>
>> Unfortunately CPU_RESOLVING_TYPE is target-specific, we want
>> hw/core/cpu-common.c to be target-agnostic (build once for all
>> targets). This is particularly important in the context of
>> heterogeneous QEMU, where a single binary will be able to create
>> CPUs from different targets.
>>
> 
> CPU_RESOLVING_TYPE and CPUClass::cpu_resolving_type is duplicate to
> each other. There are two options I can figure out to avoid the
> duplication.
> 
> (a) move cpu_class_by_name() from hw/core/cpu-common.c to cpu.c, so that
>      CPU_RESOLVING_TYPE can be seen. cpu.c::list_cpus() is the example.
> 
> (b) remove hw/core/cpu-common.c::cpu_calss_by_name() and squeeze its
>      logic to cpu.c::parse_cpu_option() since there are not too much
>      users for it. target/arm and target/s390 needs some tweaks so that
>      hw/core/cpu-common.c::cpu_calss_by_name() can be removed.
> 
>      [gshan@gshan q]$ git grep \ cpu_class_by_name\(
>      cpu.c:    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
>      target/arm/arm-qmp-cmds.c:    oc = cpu_class_by_name(TYPE_ARM_CPU, model->name);
>      target/s390x/cpu_models_sysemu.c:    oc = cpu_class_by_name(TYPE_S390_CPU, info->name);
> 
> When option (b) is taken, this series to have the checks against @oc
> in hw/core/cpu-common.c::cpu_calss_by_name() becomes non-sense. Instead,
> we need the same (and complete) checks in CPUClass::class_by_name() for
> individual targets. Further more, an inline helper can be provided to do
> the check in CPUClass::class_by_name() for individual targets.
> 
>     include/hw/core/cpu.h
> 
>     static inline bool cpu_class_is_valid(ObjectClass *oc, const char *parent)
>     {
>         if (!object_class_dynamic_cast(oc, parent) ||
>             object_class_is_abstract(oc)) {
>             return false;
>         }
> 
>         return true;
>     }
> 

Since my series to make CPU type check unified depends on this series, could
you please share your thoughts? If you don't have bandwidth for this, I can
improve the code based on your thoughts, and include your patches to my series
so that they can be reviewed at once. Please just let me know.

Thanks,
Gavin

Re: [PATCH 3/4] hw/cpu: Introduce CPUClass::cpu_resolving_type field

[Philippe Mathieu-Daudé]

Hi Gavin,

On 25/9/23 02:24, Gavin Shan wrote:
> On 9/12/23 08:40, Gavin Shan wrote:
>> On 9/11/23 19:43, Philippe Mathieu-Daudé wrote:
>>> On 11/9/23 01:28, Gavin Shan wrote:
>>>> On 9/8/23 21:22, Philippe Mathieu-Daudé wrote:
>>>>> Add a field to return the QOM type name of a CPU class.
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> ---


>>>>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>>>>> index 129d179937..e469efd409 100644
>>>>> --- a/include/hw/core/cpu.h
>>>>> +++ b/include/hw/core/cpu.h
>>>>> @@ -100,6 +100,7 @@ struct SysemuCPUOps;
>>>>>   /**
>>>>>    * CPUClass:
>>>>> + * @cpu_resolving_type: CPU QOM type name
>>>>>    * @class_by_name: Callback to map -cpu command line model name 
>>>>> to an
>>>>>    *                 instantiatable CPU type.
>>>>>    * @parse_features: Callback to parse command line arguments.
>>>>> @@ -148,6 +149,7 @@ struct CPUClass {
>>>>>       DeviceClass parent_class;
>>>>>       /*< public >*/
>>>>> +    const char *cpu_resolving_type;
>>>>>       ObjectClass *(*class_by_name)(const char *cpu_model);
>>>>>       void (*parse_features)(const char *typename, char *str, Error 
>>>>> **errp);
>>>>
>>>> The question is why not use CPU_RESOLVING_TYPE directly? It seems 
>>>> CPU_RESOLVING_TYPE
>>>> is exactly what you want here.
>>>
>>> Unfortunately CPU_RESOLVING_TYPE is target-specific, we want
>>> hw/core/cpu-common.c to be target-agnostic (build once for all
>>> targets). This is particularly important in the context of
>>> heterogeneous QEMU, where a single binary will be able to create
>>> CPUs from different targets.
>>>
>>
>> CPU_RESOLVING_TYPE and CPUClass::cpu_resolving_type is duplicate to
>> each other. There are two options I can figure out to avoid the
>> duplication.
>>
>> (a) move cpu_class_by_name() from hw/core/cpu-common.c to cpu.c, so that
>>      CPU_RESOLVING_TYPE can be seen. cpu.c::list_cpus() is the example.
>>
>> (b) remove hw/core/cpu-common.c::cpu_calss_by_name() and squeeze its
>>      logic to cpu.c::parse_cpu_option() since there are not too much
>>      users for it. target/arm and target/s390 needs some tweaks so that
>>      hw/core/cpu-common.c::cpu_calss_by_name() can be removed.
>>
>>      [gshan@gshan q]$ git grep \ cpu_class_by_name\(
>>      cpu.c:    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, 
>> model_pieces[0]);
>>      target/arm/arm-qmp-cmds.c:    oc = 
>> cpu_class_by_name(TYPE_ARM_CPU, model->name);
>>      target/s390x/cpu_models_sysemu.c:    oc = 
>> cpu_class_by_name(TYPE_S390_CPU, info->name);
>>
>> When option (b) is taken, this series to have the checks against @oc
>> in hw/core/cpu-common.c::cpu_calss_by_name() becomes non-sense. Instead,
>> we need the same (and complete) checks in CPUClass::class_by_name() for
>> individual targets. Further more, an inline helper can be provided to do
>> the check in CPUClass::class_by_name() for individual targets.
>>
>>     include/hw/core/cpu.h
>>
>>     static inline bool cpu_class_is_valid(ObjectClass *oc, const char 
>> *parent)
>>     {
>>         if (!object_class_dynamic_cast(oc, parent) ||
>>             object_class_is_abstract(oc)) {
>>             return false;
>>         }
>>
>>         return true;
>>     }
>>
> 
> Since my series to make CPU type check unified depends on this series, 
> could
> you please share your thoughts? If you don't have bandwidth for this, I can
> improve the code based on your thoughts, and include your patches to my 
> series
> so that they can be reviewed at once. Please just let me know.

You seem to prove (b) is not useful, so we have to do (a).

Unfortunately at this moment I feel hopeless with this topic.

I don't want to delay your work further. If you find a way to integrate
both series, please go ahead. Otherwise let's drop my approach and
continue with your previous work.

I apologize I kept you waiting that long.

Regards,

Phil.

Re: [PATCH 3/4] hw/cpu: Introduce CPUClass::cpu_resolving_type field

[Gavin Shan]

Hi Philippe,

On 10/11/23 13:28, Philippe Mathieu-Daudé wrote:
> On 25/9/23 02:24, Gavin Shan wrote:
>> On 9/12/23 08:40, Gavin Shan wrote:
>>> On 9/11/23 19:43, Philippe Mathieu-Daudé wrote:
>>>> On 11/9/23 01:28, Gavin Shan wrote:
>>>>> On 9/8/23 21:22, Philippe Mathieu-Daudé wrote:
>>>>>> Add a field to return the QOM type name of a CPU class.
>>>>>>
>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>>> ---
> 
> 
>>>>>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>>>>>> index 129d179937..e469efd409 100644
>>>>>> --- a/include/hw/core/cpu.h
>>>>>> +++ b/include/hw/core/cpu.h
>>>>>> @@ -100,6 +100,7 @@ struct SysemuCPUOps;
>>>>>>   /**
>>>>>>    * CPUClass:
>>>>>> + * @cpu_resolving_type: CPU QOM type name
>>>>>>    * @class_by_name: Callback to map -cpu command line model name to an
>>>>>>    *                 instantiatable CPU type.
>>>>>>    * @parse_features: Callback to parse command line arguments.
>>>>>> @@ -148,6 +149,7 @@ struct CPUClass {
>>>>>>       DeviceClass parent_class;
>>>>>>       /*< public >*/
>>>>>> +    const char *cpu_resolving_type;
>>>>>>       ObjectClass *(*class_by_name)(const char *cpu_model);
>>>>>>       void (*parse_features)(const char *typename, char *str, Error **errp);
>>>>>
>>>>> The question is why not use CPU_RESOLVING_TYPE directly? It seems CPU_RESOLVING_TYPE
>>>>> is exactly what you want here.
>>>>
>>>> Unfortunately CPU_RESOLVING_TYPE is target-specific, we want
>>>> hw/core/cpu-common.c to be target-agnostic (build once for all
>>>> targets). This is particularly important in the context of
>>>> heterogeneous QEMU, where a single binary will be able to create
>>>> CPUs from different targets.
>>>>
>>>
>>> CPU_RESOLVING_TYPE and CPUClass::cpu_resolving_type is duplicate to
>>> each other. There are two options I can figure out to avoid the
>>> duplication.
>>>
>>> (a) move cpu_class_by_name() from hw/core/cpu-common.c to cpu.c, so that
>>>      CPU_RESOLVING_TYPE can be seen. cpu.c::list_cpus() is the example.
>>>
>>> (b) remove hw/core/cpu-common.c::cpu_calss_by_name() and squeeze its
>>>      logic to cpu.c::parse_cpu_option() since there are not too much
>>>      users for it. target/arm and target/s390 needs some tweaks so that
>>>      hw/core/cpu-common.c::cpu_calss_by_name() can be removed.
>>>
>>>      [gshan@gshan q]$ git grep \ cpu_class_by_name\(
>>>      cpu.c:    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
>>>      target/arm/arm-qmp-cmds.c:    oc = cpu_class_by_name(TYPE_ARM_CPU, model->name);
>>>      target/s390x/cpu_models_sysemu.c:    oc = cpu_class_by_name(TYPE_S390_CPU, info->name);
>>>
>>> When option (b) is taken, this series to have the checks against @oc
>>> in hw/core/cpu-common.c::cpu_calss_by_name() becomes non-sense. Instead,
>>> we need the same (and complete) checks in CPUClass::class_by_name() for
>>> individual targets. Further more, an inline helper can be provided to do
>>> the check in CPUClass::class_by_name() for individual targets.
>>>
>>>     include/hw/core/cpu.h
>>>
>>>     static inline bool cpu_class_is_valid(ObjectClass *oc, const char *parent)
>>>     {
>>>         if (!object_class_dynamic_cast(oc, parent) ||
>>>             object_class_is_abstract(oc)) {
>>>             return false;
>>>         }
>>>
>>>         return true;
>>>     }
>>>
>>
>> Since my series to make CPU type check unified depends on this series, could
>> you please share your thoughts? If you don't have bandwidth for this, I can
>> improve the code based on your thoughts, and include your patches to my series
>> so that they can be reviewed at once. Please just let me know.
> 
> You seem to prove (b) is not useful, so we have to do (a).
> 
> Unfortunately at this moment I feel hopeless with this topic.
> 
> I don't want to delay your work further. If you find a way to integrate
> both series, please go ahead. Otherwise let's drop my approach and
> continue with your previous work.
> 
> I apologize I kept you waiting that long.
> 

Ah, nope, nothing went to wrong here. Thanks for your reply, I will try
to follow (a) and integrate your patches to my series. Please help to
review my series when it's posted :)

Thanks,
Gavin

[PATCH 4/4] hw/cpu: Call object_class_dynamic_cast() once in cpu_class_by_name()

[Philippe Mathieu-Daudé]

Leverage the public CPUClass::cpu_resolving_type field and
call object_class_dynamic_cast() once in cpu_class_by_name().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/core/cpu-common.c   | 3 ++-
 target/alpha/cpu.c     | 3 +--
 target/arm/cpu.c       | 4 +---
 target/avr/cpu.c       | 4 +---
 target/cris/cpu.c      | 4 +---
 target/hexagon/cpu.c   | 4 +---
 target/loongarch/cpu.c | 5 +----
 target/m68k/cpu.c      | 4 +---
 target/openrisc/cpu.c  | 4 +---
 target/riscv/cpu.c     | 4 +---
 target/rx/cpu.c        | 2 +-
 target/tricore/cpu.c   | 4 +---
 target/xtensa/cpu.c    | 4 +---
 13 files changed, 14 insertions(+), 35 deletions(-)

diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index 2d24261a6a..f4a2ccebea 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -157,7 +157,8 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
     cc = CPU_CLASS(oc);
     assert(cc->cpu_resolving_type && cc->class_by_name);
     oc = cc->class_by_name(cpu_model);
-    if (oc == NULL || object_class_is_abstract(oc)) {
+    if (oc == NULL || object_class_is_abstract(oc)
+                   || !object_class_dynamic_cast(oc, cc->cpu_resolving_type)) {
         return NULL;
     }
     return oc;
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 0ddea8004c..b184fcc123 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -126,8 +126,7 @@ static ObjectClass *alpha_cpu_class_by_name(const char *cpu_model)
     int i;
 
     oc = object_class_by_name(cpu_model);
-    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_ALPHA_CPU) != NULL &&
-        !object_class_is_abstract(oc)) {
+    if (oc != NULL && !object_class_is_abstract(oc)) {
         return oc;
     }
 
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 9e51bde170..d29040cd8c 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2300,9 +2300,7 @@ static ObjectClass *arm_cpu_class_by_name(const char *cpu_model)
     oc = object_class_by_name(typename);
     g_strfreev(cpuname);
     g_free(typename);
-    if (!oc || !object_class_dynamic_cast(oc, TYPE_ARM_CPU)) {
-        return NULL;
-    }
+
     return oc;
 }
 
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index f6004169ac..53735ff1dd 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -159,9 +159,7 @@ static ObjectClass *avr_cpu_class_by_name(const char *cpu_model)
     ObjectClass *oc;
 
     oc = object_class_by_name(cpu_model);
-    if (object_class_dynamic_cast(oc, TYPE_AVR_CPU) == NULL) {
-        oc = NULL;
-    }
+
     return oc;
 }
 
diff --git a/target/cris/cpu.c b/target/cris/cpu.c
index adde4f599d..b307d0b9db 100644
--- a/target/cris/cpu.c
+++ b/target/cris/cpu.c
@@ -95,9 +95,7 @@ static ObjectClass *cris_cpu_class_by_name(const char *cpu_model)
     typename = g_strdup_printf(CRIS_CPU_TYPE_NAME("%s"), cpu_model);
     oc = object_class_by_name(typename);
     g_free(typename);
-    if (oc != NULL && !object_class_dynamic_cast(oc, TYPE_CRIS_CPU)) {
-        oc = NULL;
-    }
+
     return oc;
 }
 
diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 2d4fed838d..4b8d63c4a7 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -63,9 +63,7 @@ static ObjectClass *hexagon_cpu_class_by_name(const char *cpu_model)
     oc = object_class_by_name(typename);
     g_strfreev(cpuname);
     g_free(typename);
-    if (!oc || !object_class_dynamic_cast(oc, TYPE_HEXAGON_CPU)) {
-        return NULL;
-    }
+
     return oc;
 }
 
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 189dfd32d1..1eb2c579eb 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -646,10 +646,7 @@ static ObjectClass *loongarch_cpu_class_by_name(const char *cpu_model)
         }
     }
 
-    if (object_class_dynamic_cast(oc, TYPE_LOONGARCH_CPU)) {
-        return oc;
-    }
-    return NULL;
+    return oc;
 }
 
 void loongarch_cpu_dump_state(CPUState *cs, FILE *f, int flags)
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index bd7bb103d7..e8b86c80f1 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -111,9 +111,7 @@ static ObjectClass *m68k_cpu_class_by_name(const char *cpu_model)
     typename = g_strdup_printf(M68K_CPU_TYPE_NAME("%s"), cpu_model);
     oc = object_class_by_name(typename);
     g_free(typename);
-    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_M68K_CPU) == NULL) {
-        return NULL;
-    }
+
     return oc;
 }
 
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index 5e1e0576e0..7aac9105bd 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -168,9 +168,7 @@ 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)) {
-        return NULL;
-    }
+
     return oc;
 }
 
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index e8f04ef82b..0170e288e7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -619,9 +619,7 @@ static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
     oc = object_class_by_name(typename);
     g_strfreev(cpuname);
     g_free(typename);
-    if (!oc || !object_class_dynamic_cast(oc, TYPE_RISCV_CPU)) {
-        return NULL;
-    }
+
     return oc;
 }
 
diff --git a/target/rx/cpu.c b/target/rx/cpu.c
index 2a6df418a8..879d4fcdef 100644
--- a/target/rx/cpu.c
+++ b/target/rx/cpu.c
@@ -111,7 +111,7 @@ 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 (oc != NULL) {
         return oc;
     }
     typename = g_strdup_printf(RX_CPU_TYPE_NAME("%s"), cpu_model);
diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
index ffe5158786..f65b8761b0 100644
--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -140,9 +140,7 @@ 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)) {
-        return NULL;
-    }
+
     return oc;
 }
 
diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index 13bed05d0c..6d96e5ab27 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -141,9 +141,7 @@ 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)) {
-        return NULL;
-    }
+
     return oc;
 }
 
-- 
2.41.0

Re: [PATCH 4/4] hw/cpu: Call object_class_dynamic_cast() once in cpu_class_by_name()

[Richard Henderson]

On 9/8/23 04:22, Philippe Mathieu-Daudé wrote:
> +++ b/hw/core/cpu-common.c
> @@ -157,7 +157,8 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
>       cc = CPU_CLASS(oc);
>       assert(cc->cpu_resolving_type && cc->class_by_name);
>       oc = cc->class_by_name(cpu_model);
> -    if (oc == NULL || object_class_is_abstract(oc)) {
> +    if (oc == NULL || object_class_is_abstract(oc)
> +                   || !object_class_dynamic_cast(oc, cc->cpu_resolving_type)) {
>           return NULL;
>       }

cc->cpu_resolving_type, as constructed in patch 3, is always @typename.
In the only (!) caller in toplevel/cpu.c, this is CPU_RESOLVING_TYPE.

Indentation:

     if (oc == NULL
         || object_class_is_abstract(oc)
         || !object_class_dynamic_cast(oc, typename)


r~

Re: [PATCH 4/4] hw/cpu: Call object_class_dynamic_cast() once in cpu_class_by_name()

[Gavin Shan]

On 9/8/23 21:22, Philippe Mathieu-Daudé wrote:
> Leverage the public CPUClass::cpu_resolving_type field and
> call object_class_dynamic_cast() once in cpu_class_by_name().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/core/cpu-common.c   | 3 ++-
>   target/alpha/cpu.c     | 3 +--
>   target/arm/cpu.c       | 4 +---
>   target/avr/cpu.c       | 4 +---
>   target/cris/cpu.c      | 4 +---
>   target/hexagon/cpu.c   | 4 +---
>   target/loongarch/cpu.c | 5 +----
>   target/m68k/cpu.c      | 4 +---
>   target/openrisc/cpu.c  | 4 +---
>   target/riscv/cpu.c     | 4 +---
>   target/rx/cpu.c        | 2 +-
>   target/tricore/cpu.c   | 4 +---
>   target/xtensa/cpu.c    | 4 +---
>   13 files changed, 14 insertions(+), 35 deletions(-)
> 
> diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
> index 2d24261a6a..f4a2ccebea 100644
> --- a/hw/core/cpu-common.c
> +++ b/hw/core/cpu-common.c
> @@ -157,7 +157,8 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
>       cc = CPU_CLASS(oc);
>       assert(cc->cpu_resolving_type && cc->class_by_name);
>       oc = cc->class_by_name(cpu_model);
> -    if (oc == NULL || object_class_is_abstract(oc)) {
> +    if (oc == NULL || object_class_is_abstract(oc)
> +                   || !object_class_dynamic_cast(oc, cc->cpu_resolving_type)) {
>           return NULL;
>       }
>       return oc;

Alignment.

     if (!oc || object_class_is_abstract(oc) ||
         !object_class_dynamic_cast(oc, cc->cpu_resolving_type)) {

> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
> index 0ddea8004c..b184fcc123 100644
> --- a/target/alpha/cpu.c
> +++ b/target/alpha/cpu.c
> @@ -126,8 +126,7 @@ static ObjectClass *alpha_cpu_class_by_name(const char *cpu_model)
>       int i;
>   
>       oc = object_class_by_name(cpu_model);
> -    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_ALPHA_CPU) != NULL &&
> -        !object_class_is_abstract(oc)) {
> +    if (oc != NULL && !object_class_is_abstract(oc)) {
>           return oc;
>       }
>   

No, the check "!object_class_is_abstract(oc)" for this level can't be dropped.
The same check for the last level can be dropped because it's going to be
covered by hw/core/cpu-common.c::cpu_class_by_name().

> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 9e51bde170..d29040cd8c 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -2300,9 +2300,7 @@ static ObjectClass *arm_cpu_class_by_name(const char *cpu_model)
>       oc = object_class_by_name(typename);
>       g_strfreev(cpuname);
>       g_free(typename);
> -    if (!oc || !object_class_dynamic_cast(oc, TYPE_ARM_CPU)) {
> -        return NULL;
> -    }
> +
>       return oc;
>   }
>   
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index f6004169ac..53735ff1dd 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -159,9 +159,7 @@ static ObjectClass *avr_cpu_class_by_name(const char *cpu_model)
>       ObjectClass *oc;
>   
>       oc = object_class_by_name(cpu_model);
> -    if (object_class_dynamic_cast(oc, TYPE_AVR_CPU) == NULL) {
> -        oc = NULL;
> -    }
> +
>       return oc;
>   }
>   
> diff --git a/target/cris/cpu.c b/target/cris/cpu.c
> index adde4f599d..b307d0b9db 100644
> --- a/target/cris/cpu.c
> +++ b/target/cris/cpu.c
> @@ -95,9 +95,7 @@ static ObjectClass *cris_cpu_class_by_name(const char *cpu_model)
>       typename = g_strdup_printf(CRIS_CPU_TYPE_NAME("%s"), cpu_model);
>       oc = object_class_by_name(typename);
>       g_free(typename);
> -    if (oc != NULL && !object_class_dynamic_cast(oc, TYPE_CRIS_CPU)) {
> -        oc = NULL;
> -    }
> +
>       return oc;
>   }
>   
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
> index 2d4fed838d..4b8d63c4a7 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -63,9 +63,7 @@ static ObjectClass *hexagon_cpu_class_by_name(const char *cpu_model)
>       oc = object_class_by_name(typename);
>       g_strfreev(cpuname);
>       g_free(typename);
> -    if (!oc || !object_class_dynamic_cast(oc, TYPE_HEXAGON_CPU)) {
> -        return NULL;
> -    }
> +
>       return oc;
>   }
>   
> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
> index 189dfd32d1..1eb2c579eb 100644
> --- a/target/loongarch/cpu.c
> +++ b/target/loongarch/cpu.c
> @@ -646,10 +646,7 @@ static ObjectClass *loongarch_cpu_class_by_name(const char *cpu_model)
>           }
>       }
>   
> -    if (object_class_dynamic_cast(oc, TYPE_LOONGARCH_CPU)) {
> -        return oc;
> -    }
> -    return NULL;
> +    return oc;
>   }
>   
>   void loongarch_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
> index bd7bb103d7..e8b86c80f1 100644
> --- a/target/m68k/cpu.c
> +++ b/target/m68k/cpu.c
> @@ -111,9 +111,7 @@ static ObjectClass *m68k_cpu_class_by_name(const char *cpu_model)
>       typename = g_strdup_printf(M68K_CPU_TYPE_NAME("%s"), cpu_model);
>       oc = object_class_by_name(typename);
>       g_free(typename);
> -    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_M68K_CPU) == NULL) {
> -        return NULL;
> -    }
> +
>       return oc;
>   }
>   
> diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
> index 5e1e0576e0..7aac9105bd 100644
> --- a/target/openrisc/cpu.c
> +++ b/target/openrisc/cpu.c
> @@ -168,9 +168,7 @@ 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)) {
> -        return NULL;
> -    }
> +
>       return oc;
>   }
>   
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index e8f04ef82b..0170e288e7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -619,9 +619,7 @@ static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
>       oc = object_class_by_name(typename);
>       g_strfreev(cpuname);
>       g_free(typename);
> -    if (!oc || !object_class_dynamic_cast(oc, TYPE_RISCV_CPU)) {
> -        return NULL;
> -    }
> +
>       return oc;
>   }
>   
> diff --git a/target/rx/cpu.c b/target/rx/cpu.c
> index 2a6df418a8..879d4fcdef 100644
> --- a/target/rx/cpu.c
> +++ b/target/rx/cpu.c
> @@ -111,7 +111,7 @@ 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 (oc != NULL) {
>           return oc;
>       }
>       typename = g_strdup_printf(RX_CPU_TYPE_NAME("%s"), cpu_model);

Same as above. The check (object_class_dynamic_cast()) can't be dropped in
this level. Besides, the check object_class_is_abstract() is missed for this
level.

> diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
> index ffe5158786..f65b8761b0 100644
> --- a/target/tricore/cpu.c
> +++ b/target/tricore/cpu.c
> @@ -140,9 +140,7 @@ 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)) {
> -        return NULL;
> -    }
> +
>       return oc;
>   }
>   
> diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
> index 13bed05d0c..6d96e5ab27 100644
> --- a/target/xtensa/cpu.c
> +++ b/target/xtensa/cpu.c
> @@ -141,9 +141,7 @@ 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)) {
> -        return NULL;
> -    }
> +
>       return oc;
>   }
>   

Thanks,
Gavin