[PATCH-for-10.1 0/8] target/mips: Make 'cpu-qom.h' target agnostic

[PATCH-for-10.1 0/8] target/mips: Make 'cpu-qom.h' target agnostic

[Philippe Mathieu-Daudé]

- Remove the TARGET_MIPS64 use in 'cpu-qom.h' to
  make it target agnostic
- Introduce mips_cpu_is_64bit() as a runtime check
  for compile time TARGET_MIPS64 definition
- Replace the ldtul_p() gdbstub call by ldn_p()
- Rename few symbols to avoid future linkage clash

Philippe Mathieu-Daudé (8):
  cpus: Open code OBJECT_DECLARE_TYPE() in OBJECT_DECLARE_CPU_TYPE()
  target/mips: Declare CPU QOM types using DEFINE_TYPES() macro
  target/mips: Make MIPS_CPU common to new MIPS32_CPU / MIPS64_CPU types
  target/mips: Prefix MMU API with 'mips_'
  target/mips: Replace ldtul_p() -> ldn_p(sizeof(target_ulong))
  target/mips: Introduce mips_cpu_is_64bit() helper
  target/mips: Get CPU register size using mips_cpu_is_64bit()
  target/mips: Introduce mips_env_64bit_enabled() helper

 include/hw/core/cpu.h               |  7 ++++-
 target/mips/cpu-qom.h               | 14 +++++-----
 target/mips/internal.h              |  5 ++++
 target/mips/tcg/tcg-internal.h      |  2 +-
 target/mips/cpu.c                   | 42 ++++++++++++++++++++---------
 target/mips/gdbstub.c               | 14 +++++++---
 target/mips/tcg/system/tlb_helper.c |  2 +-
 7 files changed, 61 insertions(+), 25 deletions(-)

-- 
2.47.1

[PATCH-for-10.1 1/8] cpus: Open code OBJECT_DECLARE_TYPE() in OBJECT_DECLARE_CPU_TYPE()

[Philippe Mathieu-Daudé]

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

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

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

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Acked-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/hw/core/cpu.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 5d11d26556a..01e03f267cc 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -82,7 +82,12 @@ DECLARE_CLASS_CHECKERS(CPUClass, CPU,
  */
 #define OBJECT_DECLARE_CPU_TYPE(CpuInstanceType, CpuClassType, CPU_MODULE_OBJ_NAME) \
     typedef struct ArchCPU CpuInstanceType; \
-    OBJECT_DECLARE_TYPE(ArchCPU, CpuClassType, CPU_MODULE_OBJ_NAME);
+    typedef struct CpuClassType CpuClassType; \
+    \
+    G_DEFINE_AUTOPTR_CLEANUP_FUNC(CpuInstanceType, object_unref) \
+    \
+    DECLARE_OBJ_CHECKERS(CpuInstanceType, CpuClassType, \
+                         CPU_MODULE_OBJ_NAME, TYPE_##CPU_MODULE_OBJ_NAME)
 
 typedef struct CPUWatchpoint CPUWatchpoint;
 
-- 
2.47.1

[PATCH-for-10.1 2/8] target/mips: Declare CPU QOM types using DEFINE_TYPES() macro

[Philippe Mathieu-Daudé]

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

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

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

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

[PATCH-for-10.1 3/8] target/mips: Make MIPS_CPU common to new MIPS32_CPU / MIPS64_CPU types

[Philippe Mathieu-Daudé]

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

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

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

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

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/cpu-qom.h | 12 ++++++------
 target/mips/cpu.c     | 11 ++++++++++-
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/target/mips/cpu-qom.h b/target/mips/cpu-qom.h
index 0eea2a2598e..9acf647420c 100644
--- a/target/mips/cpu-qom.h
+++ b/target/mips/cpu-qom.h
@@ -1,5 +1,5 @@
 /*
- * QEMU MIPS CPU
+ * QEMU MIPS CPU QOM header (target agnostic)
  *
  * Copyright (c) 2012 SUSE LINUX Products GmbH
  *
@@ -22,12 +22,12 @@
 
 #include "hw/core/cpu.h"
 
-#ifdef TARGET_MIPS64
-#define TYPE_MIPS_CPU "mips64-cpu"
-#else
-#define TYPE_MIPS_CPU "mips-cpu"
-#endif
+#define TYPE_MIPS32_CPU "mips32-cpu"
+#define TYPE_MIPS64_CPU "mips64-cpu"
+#define TYPE_MIPS_CPU   "mips-cpu"
 
+OBJECT_DECLARE_CPU_TYPE(MIPS32CPU, MIPSCPUClass, MIPS32_CPU)
+OBJECT_DECLARE_CPU_TYPE(MIPS64CPU, MIPSCPUClass, MIPS64_CPU)
 OBJECT_DECLARE_CPU_TYPE(MIPSCPU, MIPSCPUClass, MIPS_CPU)
 
 #define MIPS_CPU_TYPE_SUFFIX "-" TYPE_MIPS_CPU
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 097554fd8ae..5ed6b3402d3 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -607,6 +607,14 @@ static const TypeInfo mips_cpu_types[] = {
         .abstract       = true,
         .class_size     = sizeof(MIPSCPUClass),
         .class_init     = mips_cpu_class_init,
+    }, {
+        .name           = TYPE_MIPS32_CPU,
+        .parent         = TYPE_MIPS_CPU,
+        .abstract       = true,
+    }, {
+        .name           = TYPE_MIPS64_CPU,
+        .parent         = TYPE_MIPS_CPU,
+        .abstract       = true,
     }
 };
 
@@ -623,7 +631,8 @@ static void mips_register_cpudef_type(const struct mips_def_t *def)
     char *typename = mips_cpu_type_name(def->name);
     TypeInfo ti = {
         .name = typename,
-        .parent = TYPE_MIPS_CPU,
+        .parent = def->insn_flags & CPU_MIPS64
+                  ? TYPE_MIPS64_CPU : TYPE_MIPS32_CPU,
         .class_init = mips_cpu_cpudef_class_init,
         .class_data = (void *)def,
     };
-- 
2.47.1

[PATCH-for-10.1 4/8] target/mips: Prefix MMU API with 'mips_'

[Philippe Mathieu-Daudé]

MIPS MMU API declared in tcg-internal.h has public linkage.
In order to avoid name clashing with other targets, prefix
the API with 'mips_'.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/tcg/tcg-internal.h      | 2 +-
 target/mips/cpu.c                   | 2 +-
 target/mips/tcg/system/tlb_helper.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/mips/tcg/tcg-internal.h b/target/mips/tcg/tcg-internal.h
index 74fc1309a71..a8bf2a5da40 100644
--- a/target/mips/tcg/tcg-internal.h
+++ b/target/mips/tcg/tcg-internal.h
@@ -45,7 +45,7 @@ void do_raise_exception(CPUMIPSState *env,
 void mips_cpu_do_interrupt(CPUState *cpu);
 bool mips_cpu_exec_interrupt(CPUState *cpu, int int_req);
 
-void mmu_init(CPUMIPSState *env, const mips_def_t *def);
+void mips_mmu_init(CPUMIPSState *env, const mips_def_t *def);
 
 void update_pagemask(CPUMIPSState *env, target_ulong arg1, int32_t *pagemask);
 
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 5ed6b3402d3..d8930468b7d 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -485,7 +485,7 @@ static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
     env->exception_base = (int32_t)0xBFC00000;
 
 #if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY)
-    mmu_init(env, env->cpu_model);
+    mips_mmu_init(env, env->cpu_model);
 #endif
     fpu_init(env, env->cpu_model);
     mvp_init(env);
diff --git a/target/mips/tcg/system/tlb_helper.c b/target/mips/tcg/system/tlb_helper.c
index ca4d6b27bc9..1ef2c32cfd4 100644
--- a/target/mips/tcg/system/tlb_helper.c
+++ b/target/mips/tcg/system/tlb_helper.c
@@ -466,7 +466,7 @@ static void r4k_mmu_init(CPUMIPSState *env, const mips_def_t *def)
     env->tlb->helper_tlbinvf = r4k_helper_tlbinvf;
 }
 
-void mmu_init(CPUMIPSState *env, const mips_def_t *def)
+void mips_mmu_init(CPUMIPSState *env, const mips_def_t *def)
 {
     env->tlb = g_malloc0(sizeof(CPUMIPSTLBContext));
 
-- 
2.47.1

[PATCH-for-10.1 5/8] target/mips: Replace ldtul_p() -> ldn_p(sizeof(target_ulong))

[Philippe Mathieu-Daudé]

Replace the single ldtul_p() call by a generic ldn_p() one.
No logical change.

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

diff --git a/target/mips/gdbstub.c b/target/mips/gdbstub.c
index 169d47416a6..b9fc667373e 100644
--- a/target/mips/gdbstub.c
+++ b/target/mips/gdbstub.c
@@ -79,12 +79,13 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
 {
     CPUMIPSState *env = cpu_env(cs);
     target_ulong tmp;
+    size_t regsize = sizeof(tmp);
 
-    tmp = ldtul_p(mem_buf);
+    tmp = ldn_p(mem_buf, regsize);
 
     if (n < 32) {
         env->active_tc.gpr[n] = tmp;
-        return sizeof(target_ulong);
+        return regsize;
     }
     if (env->CP0_Config1 & (1 << CP0C1_FP) && n >= 38 && n < 72) {
         switch (n) {
@@ -104,7 +105,7 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
             }
             break;
         }
-        return sizeof(target_ulong);
+        return regsize;
     }
     switch (n) {
     case 32:
@@ -144,5 +145,5 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
         break;
     }
 
-    return sizeof(target_ulong);
+    return regsize;
 }
-- 
2.47.1

[PATCH-for-10.1 6/8] target/mips: Introduce mips_cpu_is_64bit() helper

[Philippe Mathieu-Daudé]

mips_cpu_is_64bit() returns whether the CPU is a
32-bit or a 64-bit one.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/cpu-qom.h | 2 ++
 target/mips/cpu.c     | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/target/mips/cpu-qom.h b/target/mips/cpu-qom.h
index 9acf647420c..52996e7c354 100644
--- a/target/mips/cpu-qom.h
+++ b/target/mips/cpu-qom.h
@@ -33,4 +33,6 @@ OBJECT_DECLARE_CPU_TYPE(MIPSCPU, MIPSCPUClass, MIPS_CPU)
 #define MIPS_CPU_TYPE_SUFFIX "-" TYPE_MIPS_CPU
 #define MIPS_CPU_TYPE_NAME(model) model MIPS_CPU_TYPE_SUFFIX
 
+bool mips_cpu_is_64bit(MIPSCPU *cpu);
+
 #endif
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index d8930468b7d..05b3ce42af5 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -516,6 +516,12 @@ static void mips_cpu_initfn(Object *obj)
 #endif
 }
 
+bool mips_cpu_is_64bit(MIPSCPU *cpu)
+{
+    return !!object_class_dynamic_cast(OBJECT_CLASS(CPU(cpu)->cc),
+                                       TYPE_MIPS64_CPU);
+}
+
 static char *mips_cpu_type_name(const char *cpu_model)
 {
     return g_strdup_printf(MIPS_CPU_TYPE_NAME("%s"), cpu_model);
-- 
2.47.1

[PATCH-for-10.1 7/8] target/mips: Get CPU register size using mips_cpu_is_64bit()

[Philippe Mathieu-Daudé]

CPU registers size is static and depends on the type of CPU.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/gdbstub.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/mips/gdbstub.c b/target/mips/gdbstub.c
index b9fc667373e..84fd3de4137 100644
--- a/target/mips/gdbstub.c
+++ b/target/mips/gdbstub.c
@@ -75,11 +75,16 @@ int mips_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
     return 0;
 }
 
+static size_t mips_regsize(MIPSCPU *cpu)
+{
+    return mips_cpu_is_64bit(cpu) ? sizeof(uint64_t) : sizeof(uint32_t);
+}
+
 int mips_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
 {
     CPUMIPSState *env = cpu_env(cs);
     target_ulong tmp;
-    size_t regsize = sizeof(tmp);
+    size_t regsize = mips_regsize(MIPS_CPU(cs));
 
     tmp = ldn_p(mem_buf, regsize);
 
-- 
2.47.1

[PATCH-for-10.1 8/8] target/mips: Introduce mips_env_64bit_enabled() helper

[Philippe Mathieu-Daudé]

mips_env_64bit_enabled() returns whether the CPU is running
in 32-bit or 64-bit (behavior which might change at runtime).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/internal.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/mips/internal.h b/target/mips/internal.h
index 28eb28936ba..8107a59b908 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -225,6 +225,11 @@ static inline void mips_env_set_pc(CPUMIPSState *env, target_ulong value)
     }
 }
 
+static inline bool mips_env_64bit_enabled(CPUMIPSState *env)
+{
+    return env->hflags & MIPS_HFLAG_64;
+}
+
 static inline bool mips_env_is_bigendian(CPUMIPSState *env)
 {
     return extract32(env->CP0_Config0, CP0C0_BE, 1);
-- 
2.47.1

Re: [PATCH-for-10.1 2/8] target/mips: Declare CPU QOM types using DEFINE_TYPES() macro

[Pierrick Bouvier]

On 3/25/25 08:40, Philippe Mathieu-Daudé wrote:
> When multiple QOM types are registered in the same file,
> it is simpler to use the the DEFINE_TYPES() macro. In
> particular because type array declared with such macro
> are easier to review.
> 
> In few commits we are going to add more types, so replace
> the type_register_static() to ease further reviews.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/mips/cpu.c | 23 +++++++++++++----------
>   1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index b207106dd79..097554fd8ae 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -597,17 +597,21 @@ static void mips_cpu_class_init(ObjectClass *c, void *data)
>   #endif /* CONFIG_TCG */
>   }
>   
> -static const TypeInfo mips_cpu_type_info = {
> -    .name = TYPE_MIPS_CPU,
> -    .parent = TYPE_CPU,
> -    .instance_size = sizeof(MIPSCPU),
> -    .instance_align = __alignof(MIPSCPU),
> -    .instance_init = mips_cpu_initfn,
> -    .abstract = true,
> -    .class_size = sizeof(MIPSCPUClass),
> -    .class_init = mips_cpu_class_init,
> +static const TypeInfo mips_cpu_types[] = {
> +    {
> +        .name           = TYPE_MIPS_CPU,
> +        .parent         = TYPE_CPU,
> +        .instance_size  = sizeof(MIPSCPU),
> +        .instance_align = __alignof(MIPSCPU),
> +        .instance_init  = mips_cpu_initfn,
> +        .abstract       = true,
> +        .class_size     = sizeof(MIPSCPUClass),
> +        .class_init     = mips_cpu_class_init,
> +    }
>   };
>   
> +DEFINE_TYPES(mips_cpu_types)
> +
>   static void mips_cpu_cpudef_class_init(ObjectClass *oc, void *data)
>   {
>       MIPSCPUClass *mcc = MIPS_CPU_CLASS(oc);
> @@ -632,7 +636,6 @@ static void mips_cpu_register_types(void)
>   {
>       int i;
>   
> -    type_register_static(&mips_cpu_type_info);
>       for (i = 0; i < mips_defs_number; i++) {
>           mips_register_cpudef_type(&mips_defs[i]);
>       }

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH-for-10.1 3/8] target/mips: Make MIPS_CPU common to new MIPS32_CPU / MIPS64_CPU types

[Pierrick Bouvier]

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

I'm not sure we absolutely need to introduce a new common type 
TYPE_MIPS_CPU.

If types don't share any common data, or have specific method applying 
to them, I would just define 32/64 types without a common ancestor.

That said, if you prefer, or if needed, I'm ok with the patch as it is:

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH-for-10.1 4/8] target/mips: Prefix MMU API with 'mips_'

[Pierrick Bouvier]

On 3/25/25 08:40, Philippe Mathieu-Daudé wrote:
> MIPS MMU API declared in tcg-internal.h has public linkage.
> In order to avoid name clashing with other targets, prefix
> the API with 'mips_'.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/mips/tcg/tcg-internal.h      | 2 +-
>   target/mips/cpu.c                   | 2 +-
>   target/mips/tcg/system/tlb_helper.c | 2 +-
>   3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/mips/tcg/tcg-internal.h b/target/mips/tcg/tcg-internal.h
> index 74fc1309a71..a8bf2a5da40 100644
> --- a/target/mips/tcg/tcg-internal.h
> +++ b/target/mips/tcg/tcg-internal.h
> @@ -45,7 +45,7 @@ void do_raise_exception(CPUMIPSState *env,
>   void mips_cpu_do_interrupt(CPUState *cpu);
>   bool mips_cpu_exec_interrupt(CPUState *cpu, int int_req);
>   
> -void mmu_init(CPUMIPSState *env, const mips_def_t *def);
> +void mips_mmu_init(CPUMIPSState *env, const mips_def_t *def);
>   
>   void update_pagemask(CPUMIPSState *env, target_ulong arg1, int32_t *pagemask);
>   
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index 5ed6b3402d3..d8930468b7d 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -485,7 +485,7 @@ static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
>       env->exception_base = (int32_t)0xBFC00000;
>   
>   #if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY)
> -    mmu_init(env, env->cpu_model);
> +    mips_mmu_init(env, env->cpu_model);
>   #endif
>       fpu_init(env, env->cpu_model);
>       mvp_init(env);
> diff --git a/target/mips/tcg/system/tlb_helper.c b/target/mips/tcg/system/tlb_helper.c
> index ca4d6b27bc9..1ef2c32cfd4 100644
> --- a/target/mips/tcg/system/tlb_helper.c
> +++ b/target/mips/tcg/system/tlb_helper.c
> @@ -466,7 +466,7 @@ static void r4k_mmu_init(CPUMIPSState *env, const mips_def_t *def)
>       env->tlb->helper_tlbinvf = r4k_helper_tlbinvf;
>   }
>   
> -void mmu_init(CPUMIPSState *env, const mips_def_t *def)
> +void mips_mmu_init(CPUMIPSState *env, const mips_def_t *def)
>   {
>       env->tlb = g_malloc0(sizeof(CPUMIPSTLBContext));
>   

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH-for-10.1 5/8] target/mips: Replace ldtul_p() -> ldn_p(sizeof(target_ulong))

[Pierrick Bouvier]

On 3/25/25 08:40, Philippe Mathieu-Daudé wrote:
> Replace the single ldtul_p() call by a generic ldn_p() one.
> No logical change.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/mips/gdbstub.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/target/mips/gdbstub.c b/target/mips/gdbstub.c
> index 169d47416a6..b9fc667373e 100644
> --- a/target/mips/gdbstub.c
> +++ b/target/mips/gdbstub.c
> @@ -79,12 +79,13 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>   {
>       CPUMIPSState *env = cpu_env(cs);
>       target_ulong tmp;
> +    size_t regsize = sizeof(tmp);
>   
> -    tmp = ldtul_p(mem_buf);
> +    tmp = ldn_p(mem_buf, regsize);
>   
>       if (n < 32) {
>           env->active_tc.gpr[n] = tmp;
> -        return sizeof(target_ulong);
> +        return regsize;
>       }
>       if (env->CP0_Config1 & (1 << CP0C1_FP) && n >= 38 && n < 72) {
>           switch (n) {
> @@ -104,7 +105,7 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>               }
>               break;
>           }
> -        return sizeof(target_ulong);
> +        return regsize;
>       }
>       switch (n) {
>       case 32:
> @@ -144,5 +145,5 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>           break;
>       }
>   
> -    return sizeof(target_ulong);
> +    return regsize;
>   }

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH-for-10.1 6/8] target/mips: Introduce mips_cpu_is_64bit() helper

[Pierrick Bouvier]

On 3/25/25 08:40, Philippe Mathieu-Daudé wrote:
> mips_cpu_is_64bit() returns whether the CPU is a
> 32-bit or a 64-bit one.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/mips/cpu-qom.h | 2 ++
>   target/mips/cpu.c     | 6 ++++++
>   2 files changed, 8 insertions(+)
> 
> diff --git a/target/mips/cpu-qom.h b/target/mips/cpu-qom.h
> index 9acf647420c..52996e7c354 100644
> --- a/target/mips/cpu-qom.h
> +++ b/target/mips/cpu-qom.h
> @@ -33,4 +33,6 @@ OBJECT_DECLARE_CPU_TYPE(MIPSCPU, MIPSCPUClass, MIPS_CPU)
>   #define MIPS_CPU_TYPE_SUFFIX "-" TYPE_MIPS_CPU
>   #define MIPS_CPU_TYPE_NAME(model) model MIPS_CPU_TYPE_SUFFIX
>   
> +bool mips_cpu_is_64bit(MIPSCPU *cpu);
> +
>   #endif
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index d8930468b7d..05b3ce42af5 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -516,6 +516,12 @@ static void mips_cpu_initfn(Object *obj)
>   #endif
>   }
>   
> +bool mips_cpu_is_64bit(MIPSCPU *cpu)
> +{
> +    return !!object_class_dynamic_cast(OBJECT_CLASS(CPU(cpu)->cc),
> +                                       TYPE_MIPS64_CPU);
> +}
> +
>   static char *mips_cpu_type_name(const char *cpu_model)
>   {
>       return g_strdup_printf(MIPS_CPU_TYPE_NAME("%s"), cpu_model);

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

On the long term, when having the TargetInfo, this is probably an 
information we want to be able to retrieve in the same way for every 
target. That said, we can always replace that later, so this patch is ok 
for now.

Re: [PATCH-for-10.1 7/8] target/mips: Get CPU register size using mips_cpu_is_64bit()

[Pierrick Bouvier]

On 3/25/25 08:40, Philippe Mathieu-Daudé wrote:
> CPU registers size is static and depends on the type of CPU.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/mips/gdbstub.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/target/mips/gdbstub.c b/target/mips/gdbstub.c
> index b9fc667373e..84fd3de4137 100644
> --- a/target/mips/gdbstub.c
> +++ b/target/mips/gdbstub.c
> @@ -75,11 +75,16 @@ int mips_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
>       return 0;
>   }
>   
> +static size_t mips_regsize(MIPSCPU *cpu)
> +{
> +    return mips_cpu_is_64bit(cpu) ? sizeof(uint64_t) : sizeof(uint32_t);
> +}
> +
>   int mips_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>   {
>       CPUMIPSState *env = cpu_env(cs);
>       target_ulong tmp;
> -    size_t regsize = sizeof(tmp);
> +    size_t regsize = mips_regsize(MIPS_CPU(cs));
>   
>       tmp = ldn_p(mem_buf, regsize);
>   

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH-for-10.1 8/8] target/mips: Introduce mips_env_64bit_enabled() helper

[Pierrick Bouvier]

On 3/25/25 08:40, Philippe Mathieu-Daudé wrote:
> mips_env_64bit_enabled() returns whether the CPU is running
> in 32-bit or 64-bit (behavior which might change at runtime).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/mips/internal.h | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/target/mips/internal.h b/target/mips/internal.h
> index 28eb28936ba..8107a59b908 100644
> --- a/target/mips/internal.h
> +++ b/target/mips/internal.h
> @@ -225,6 +225,11 @@ static inline void mips_env_set_pc(CPUMIPSState *env, target_ulong value)
>       }
>   }
>   
> +static inline bool mips_env_64bit_enabled(CPUMIPSState *env)
> +{
> +    return env->hflags & MIPS_HFLAG_64;
> +}
> +
>   static inline bool mips_env_is_bigendian(CPUMIPSState *env)
>   {
>       return extract32(env->CP0_Config0, CP0C0_BE, 1);

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH-for-10.1 0/8] target/mips: Make 'cpu-qom.h' target agnostic

[Anton Johansson via]

On 25/03/25, Philippe Mathieu-Daudé wrote:
> - Remove the TARGET_MIPS64 use in 'cpu-qom.h' to
>   make it target agnostic
> - Introduce mips_cpu_is_64bit() as a runtime check
>   for compile time TARGET_MIPS64 definition
> - Replace the ldtul_p() gdbstub call by ldn_p()
> - Rename few symbols to avoid future linkage clash
> 
> Philippe Mathieu-Daudé (8):
>   cpus: Open code OBJECT_DECLARE_TYPE() in OBJECT_DECLARE_CPU_TYPE()
>   target/mips: Declare CPU QOM types using DEFINE_TYPES() macro
>   target/mips: Make MIPS_CPU common to new MIPS32_CPU / MIPS64_CPU types
>   target/mips: Prefix MMU API with 'mips_'
>   target/mips: Replace ldtul_p() -> ldn_p(sizeof(target_ulong))
>   target/mips: Introduce mips_cpu_is_64bit() helper
>   target/mips: Get CPU register size using mips_cpu_is_64bit()
>   target/mips: Introduce mips_env_64bit_enabled() helper
> 
>  include/hw/core/cpu.h               |  7 ++++-
>  target/mips/cpu-qom.h               | 14 +++++-----
>  target/mips/internal.h              |  5 ++++
>  target/mips/tcg/tcg-internal.h      |  2 +-
>  target/mips/cpu.c                   | 42 ++++++++++++++++++++---------
>  target/mips/gdbstub.c               | 14 +++++++---
>  target/mips/tcg/system/tlb_helper.c |  2 +-
>  7 files changed, 61 insertions(+), 25 deletions(-)
> 
> -- 
> 2.47.1
> 

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH-for-10.1 6/8] target/mips: Introduce mips_cpu_is_64bit() helper

[Richard Henderson]

On 3/25/25 10:40, Philippe Mathieu-Daudé wrote:
> mips_cpu_is_64bit() returns whether the CPU is a
> 32-bit or a 64-bit one.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/mips/cpu-qom.h | 2 ++
>   target/mips/cpu.c     | 6 ++++++
>   2 files changed, 8 insertions(+)
> 
> diff --git a/target/mips/cpu-qom.h b/target/mips/cpu-qom.h
> index 9acf647420c..52996e7c354 100644
> --- a/target/mips/cpu-qom.h
> +++ b/target/mips/cpu-qom.h
> @@ -33,4 +33,6 @@ OBJECT_DECLARE_CPU_TYPE(MIPSCPU, MIPSCPUClass, MIPS_CPU)
>   #define MIPS_CPU_TYPE_SUFFIX "-" TYPE_MIPS_CPU
>   #define MIPS_CPU_TYPE_NAME(model) model MIPS_CPU_TYPE_SUFFIX
>   
> +bool mips_cpu_is_64bit(MIPSCPU *cpu);
> +
>   #endif
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index d8930468b7d..05b3ce42af5 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -516,6 +516,12 @@ static void mips_cpu_initfn(Object *obj)
>   #endif
>   }
>   
> +bool mips_cpu_is_64bit(MIPSCPU *cpu)
> +{
> +    return !!object_class_dynamic_cast(OBJECT_CLASS(CPU(cpu)->cc),
> +                                       TYPE_MIPS64_CPU);
> +}

This is very nearly object_dynamic_cast.  So why not just

   return object_dynamic_cast(OBJECT(cpu), TYPE_MIPS64_CPU);

?


r~

Re: [PATCH-for-10.1 8/8] target/mips: Introduce mips_env_64bit_enabled() helper

[Richard Henderson]

On 3/25/25 10:40, Philippe Mathieu-Daudé wrote:
> mips_env_64bit_enabled() returns whether the CPU is running
> in 32-bit or 64-bit (behavior which might change at runtime).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/mips/internal.h | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/target/mips/internal.h b/target/mips/internal.h
> index 28eb28936ba..8107a59b908 100644
> --- a/target/mips/internal.h
> +++ b/target/mips/internal.h
> @@ -225,6 +225,11 @@ static inline void mips_env_set_pc(CPUMIPSState *env, target_ulong value)
>       }
>   }
>   
> +static inline bool mips_env_64bit_enabled(CPUMIPSState *env)
> +{
> +    return env->hflags & MIPS_HFLAG_64;
> +}
> +
>   static inline bool mips_env_is_bigendian(CPUMIPSState *env)
>   {
>       return extract32(env->CP0_Config0, CP0C0_BE, 1);

Maybe delay this until you need it?

r~

Re: [PATCH-for-10.1 6/8] target/mips: Introduce mips_cpu_is_64bit() helper

[Philippe Mathieu-Daudé]

On 26/3/25 19:22, Richard Henderson wrote:
> On 3/25/25 10:40, Philippe Mathieu-Daudé wrote:
>> mips_cpu_is_64bit() returns whether the CPU is a
>> 32-bit or a 64-bit one.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/mips/cpu-qom.h | 2 ++
>>   target/mips/cpu.c     | 6 ++++++
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/target/mips/cpu-qom.h b/target/mips/cpu-qom.h
>> index 9acf647420c..52996e7c354 100644
>> --- a/target/mips/cpu-qom.h
>> +++ b/target/mips/cpu-qom.h
>> @@ -33,4 +33,6 @@ OBJECT_DECLARE_CPU_TYPE(MIPSCPU, MIPSCPUClass, 
>> MIPS_CPU)
>>   #define MIPS_CPU_TYPE_SUFFIX "-" TYPE_MIPS_CPU
>>   #define MIPS_CPU_TYPE_NAME(model) model MIPS_CPU_TYPE_SUFFIX
>> +bool mips_cpu_is_64bit(MIPSCPU *cpu);
>> +
>>   #endif
>> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
>> index d8930468b7d..05b3ce42af5 100644
>> --- a/target/mips/cpu.c
>> +++ b/target/mips/cpu.c
>> @@ -516,6 +516,12 @@ static void mips_cpu_initfn(Object *obj)
>>   #endif
>>   }
>> +bool mips_cpu_is_64bit(MIPSCPU *cpu)
>> +{
>> +    return !!object_class_dynamic_cast(OBJECT_CLASS(CPU(cpu)->cc),
>> +                                       TYPE_MIPS64_CPU);
>> +}
> 
> This is very nearly object_dynamic_cast.  So why not just
> 
>    return object_dynamic_cast(OBJECT(cpu), TYPE_MIPS64_CPU);
> 
> ?

Good point!