diff mbox series

[PULL,016/110] target/mips: Move 'mvp' field from CPUMIPSState to MIPSCPU

Message ID 20260506135524.20617-17-philmd@linaro.org
State New
Headers show
Series [PULL,001/110] monitor/hmp: : Include missing 'exec/target_long.h' header | expand

Commit Message

Philippe Mathieu-Daudé May 6, 2026, 1:53 p.m. UTC 
From: Peter Maydell <peter.maydell@linaro.org>

The 'mvp' field in the CPUMIPSState is a pointer to memory allocated
in mvp_init().  This is in theory fine, but in practice it clashes
with the current linux-user implementation of cpu_copy(), which
assumes it can do a shallow memcpy() copy of the CPU env struct in
order to clone the CPU when creating a new thread.

Almost all of the MIPS env struct is actually memcpy() copyable;
one of the exceptions is the mvp pointer. We don't need this
to be in the env struct; move it to the CPU object struct instead.

At the moment the memcpy() of the env->mvp pointer doesn't have any
obvious ill-effects, because we never free the memory and it
doesn't contain anything that varies at runtime for user-mode.
So thread 2 ends up pointing at thread 1's mvp struct, but it
still works OK. However, we would like to free the mvp memory to
avoid a leak when a user-mode thread exits, and unless we avoid
the shallow copy this will end up with a double-free when both
thread 1 and thread 2 free the same mvp struct.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20260317175031.3035740-2-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/cpu.h                   |  3 ++-
 target/mips/internal.h              |  3 ++-
 hw/mips/malta.c                     |  4 ++--
 target/mips/cpu.c                   |  2 +-
 target/mips/system/machine.c        |  2 +-
 target/mips/tcg/system/cp0_helper.c | 35 ++++++++++++++++++-----------
 target/mips/tcg/translate.c         |  6 +++--
 target/mips/cpu-defs.c.inc          | 10 +++++----
 8 files changed, 40 insertions(+), 25 deletions(-)
diff mbox series

Patch

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 17e9f6039ec..42c5ca89154 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -1174,7 +1174,6 @@  typedef struct CPUArchState {
     struct {} end_reset_fields;
 
     /* Fields from here on are preserved across CPU reset. */
-    CPUMIPSMVPContext *mvp;
 #if !defined(CONFIG_USER_ONLY)
     CPUMIPSTLBContext *tlb;
     qemu_irq irq[8];
@@ -1209,6 +1208,8 @@  struct ArchCPU {
     Clock *clock;
     Clock *count_div; /* Divider for CP0_Count clock */
 
+    CPUMIPSMVPContext *mvp;
+
     /* Properties */
     bool is_big_endian;
 };
diff --git a/target/mips/internal.h b/target/mips/internal.h
index 28eb28936ba..95b8b7bb9c9 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -246,10 +246,11 @@  static inline void restore_pamask(CPUMIPSState *env)
 
 static inline int mips_vpe_active(CPUMIPSState *env)
 {
+    MIPSCPU *cpu = env_archcpu(env);
     int active = 1;
 
     /* Check that the VPE is enabled.  */
-    if (!(env->mvp->CP0_MVPControl & (1 << CP0MVPCo_EVP))) {
+    if (!(cpu->mvp->CP0_MVPControl & (1 << CP0MVPCo_EVP))) {
         active = 0;
     }
     /* Check that the VPE is activated.  */
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 0cbe9956e74..845b8d339e8 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -968,10 +968,10 @@  static void malta_mips_config(MIPSCPU *cpu)
     CPUState *cs = CPU(cpu);
 
     if (ase_mt_available(env)) {
-        env->mvp->CP0_MVPConf0 = deposit32(env->mvp->CP0_MVPConf0,
+        cpu->mvp->CP0_MVPConf0 = deposit32(cpu->mvp->CP0_MVPConf0,
                                            CP0MVPC0_PTC, 8,
                                            smp_cpus * cs->nr_threads - 1);
-        env->mvp->CP0_MVPConf0 = deposit32(env->mvp->CP0_MVPConf0,
+        cpu->mvp->CP0_MVPConf0 = deposit32(cpu->mvp->CP0_MVPConf0,
                                            CP0MVPC0_PVPE, 4, smp_cpus - 1);
     }
 }
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 5f88c077dbf..789ca188b5e 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -339,7 +339,7 @@  static void mips_cpu_reset_hold(Object *obj, ResetType type)
 
         if (cs->cpu_index == 0) {
             /* VPE0 starts up enabled.  */
-            env->mvp->CP0_MVPControl |= (1 << CP0MVPCo_EVP);
+            cpu->mvp->CP0_MVPControl |= (1 << CP0MVPCo_EVP);
             env->CP0_VPEConf0 |= (1 << CP0VPEC0_MVP) | (1 << CP0VPEC0_VPA);
 
             /* TC0 starts up unhalted.  */
diff --git a/target/mips/system/machine.c b/target/mips/system/machine.c
index 473d3ab0363..0a32665f835 100644
--- a/target/mips/system/machine.c
+++ b/target/mips/system/machine.c
@@ -251,7 +251,7 @@  const VMStateDescription vmstate_mips_cpu = {
                        CPUMIPSFPUContext),
 
         /* MVP */
-        VMSTATE_STRUCT_POINTER(env.mvp, MIPSCPU, vmstate_mvp,
+        VMSTATE_STRUCT_POINTER(mvp, MIPSCPU, vmstate_mvp,
                                CPUMIPSMVPContext),
 
         /* TLB */
diff --git a/target/mips/tcg/system/cp0_helper.c b/target/mips/tcg/system/cp0_helper.c
index b69e70d7fcf..123d5c217c1 100644
--- a/target/mips/tcg/system/cp0_helper.c
+++ b/target/mips/tcg/system/cp0_helper.c
@@ -229,17 +229,20 @@  uint32_t cpu_mips_get_random(CPUMIPSState *env)
 /* CP0 helpers */
 target_ulong helper_mfc0_mvpcontrol(CPUMIPSState *env)
 {
-    return env->mvp->CP0_MVPControl;
+    MIPSCPU *cpu = env_archcpu(env);
+    return cpu->mvp->CP0_MVPControl;
 }
 
 target_ulong helper_mfc0_mvpconf0(CPUMIPSState *env)
 {
-    return env->mvp->CP0_MVPConf0;
+    MIPSCPU *cpu = env_archcpu(env);
+    return cpu->mvp->CP0_MVPConf0;
 }
 
 target_ulong helper_mfc0_mvpconf1(CPUMIPSState *env)
 {
-    return env->mvp->CP0_MVPConf1;
+    MIPSCPU *cpu = env_archcpu(env);
+    return cpu->mvp->CP0_MVPConf1;
 }
 
 target_ulong helper_mfc0_random(CPUMIPSState *env)
@@ -514,6 +517,7 @@  void helper_mtc0_index(CPUMIPSState *env, target_ulong arg1)
 
 void helper_mtc0_mvpcontrol(CPUMIPSState *env, target_ulong arg1)
 {
+    MIPSCPU *cpu = env_archcpu(env);
     uint32_t mask = 0;
     uint32_t newval;
 
@@ -521,14 +525,14 @@  void helper_mtc0_mvpcontrol(CPUMIPSState *env, target_ulong arg1)
         mask |= (1 << CP0MVPCo_CPA) | (1 << CP0MVPCo_VPC) |
                 (1 << CP0MVPCo_EVP);
     }
-    if (env->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC)) {
+    if (cpu->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC)) {
         mask |= (1 << CP0MVPCo_STLB);
     }
-    newval = (env->mvp->CP0_MVPControl & ~mask) | (arg1 & mask);
+    newval = (cpu->mvp->CP0_MVPControl & ~mask) | (arg1 & mask);
 
     /* TODO: Enable/disable shared TLB, enable/disable VPEs. */
 
-    env->mvp->CP0_MVPControl = newval;
+    cpu->mvp->CP0_MVPControl = newval;
 }
 
 void helper_mtc0_vpecontrol(CPUMIPSState *env, target_ulong arg1)
@@ -616,10 +620,11 @@  void helper_mttc0_vpeconf0(CPUMIPSState *env, target_ulong arg1)
 
 void helper_mtc0_vpeconf1(CPUMIPSState *env, target_ulong arg1)
 {
+    MIPSCPU *cpu = env_archcpu(env);
     uint32_t mask = 0;
     uint32_t newval;
 
-    if (env->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC))
+    if (cpu->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC))
         mask |= (0xff << CP0VPEC1_NCX) | (0xff << CP0VPEC1_NCP2) |
                 (0xff << CP0VPEC1_NCP1);
     newval = (env->CP0_VPEConf1 & ~mask) | (arg1 & mask);
@@ -689,10 +694,11 @@  void helper_mttc0_tcstatus(CPUMIPSState *env, target_ulong arg1)
 
 void helper_mtc0_tcbind(CPUMIPSState *env, target_ulong arg1)
 {
+    MIPSCPU *cpu = env_archcpu(env);
     uint32_t mask = (1 << CP0TCBd_TBE);
     uint32_t newval;
 
-    if (env->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC)) {
+    if (cpu->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC)) {
         mask |= (1 << CP0TCBd_CurVPE);
     }
     newval = (env->active_tc.CP0_TCBind & ~mask) | (arg1 & mask);
@@ -705,8 +711,9 @@  void helper_mttc0_tcbind(CPUMIPSState *env, target_ulong arg1)
     uint32_t mask = (1 << CP0TCBd_TBE);
     uint32_t newval;
     CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc);
+    MIPSCPU *other_cpu = env_archcpu(other);
 
-    if (other->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC)) {
+    if (other_cpu->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC)) {
         mask |= (1 << CP0TCBd_CurVPE);
     }
     if (other_tc == other->current_tc) {
@@ -1560,14 +1567,15 @@  target_ulong helper_emt(void)
 target_ulong helper_dvpe(CPUMIPSState *env)
 {
     CPUState *other_cs = first_cpu;
-    target_ulong prev = env->mvp->CP0_MVPControl;
+    MIPSCPU *cpu = env_archcpu(env);
+    target_ulong prev = cpu->mvp->CP0_MVPControl;
 
     if (env->CP0_VPEConf0 & (1 << CP0VPEC0_MVP)) {
         CPU_FOREACH(other_cs) {
             MIPSCPU *other_cpu = MIPS_CPU(other_cs);
             /* Turn off all VPEs except the one executing the dvpe.  */
             if (&other_cpu->env != env) {
-                other_cpu->env.mvp->CP0_MVPControl &= ~(1 << CP0MVPCo_EVP);
+                other_cpu->mvp->CP0_MVPControl &= ~(1 << CP0MVPCo_EVP);
                 mips_vpe_sleep(other_cpu);
             }
         }
@@ -1578,7 +1586,8 @@  target_ulong helper_dvpe(CPUMIPSState *env)
 target_ulong helper_evpe(CPUMIPSState *env)
 {
     CPUState *other_cs = first_cpu;
-    target_ulong prev = env->mvp->CP0_MVPControl;
+    MIPSCPU *cpu = env_archcpu(env);
+    target_ulong prev = cpu->mvp->CP0_MVPControl;
 
     if (env->CP0_VPEConf0 & (1 << CP0VPEC0_MVP)) {
         CPU_FOREACH(other_cs) {
@@ -1588,7 +1597,7 @@  target_ulong helper_evpe(CPUMIPSState *env)
                 /* If the VPE is WFI, don't disturb its sleep.  */
                 && !mips_vpe_is_wfi(other_cpu)) {
                 /* Enable the VPE.  */
-                other_cpu->env.mvp->CP0_MVPControl |= (1 << CP0MVPCo_EVP);
+                other_cpu->mvp->CP0_MVPControl |= (1 << CP0MVPCo_EVP);
                 mips_vpe_wake(other_cpu); /* And wake it up.  */
             }
         }
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 3426acd37b4..e334cf98b78 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -8085,6 +8085,7 @@  cp0_unimplemented:
 static void gen_mftr(CPUMIPSState *env, DisasContext *ctx, int rt, int rd,
                      int u, int sel, int h)
 {
+    MIPSCPU *cpu = env_archcpu(env);
     int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
     TCGv t0 = tcg_temp_new();
 
@@ -8093,7 +8094,7 @@  static void gen_mftr(CPUMIPSState *env, DisasContext *ctx, int rt, int rd,
          (env->active_tc.CP0_TCBind & (0xf << CP0TCBd_CurVPE)))) {
         tcg_gen_movi_tl(t0, -1);
     } else if ((env->CP0_VPEControl & (0xff << CP0VPECo_TargTC)) >
-               (env->mvp->CP0_MVPConf0 & (0xff << CP0MVPC0_PTC))) {
+               (cpu->mvp->CP0_MVPConf0 & (0xff << CP0MVPC0_PTC))) {
         tcg_gen_movi_tl(t0, -1);
     } else if (u == 0) {
         switch (rt) {
@@ -8309,6 +8310,7 @@  die:
 static void gen_mttr(CPUMIPSState *env, DisasContext *ctx, int rd, int rt,
                      int u, int sel, int h)
 {
+    MIPSCPU *cpu = env_archcpu(env);
     int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
     TCGv t0 = tcg_temp_new();
 
@@ -8319,7 +8321,7 @@  static void gen_mttr(CPUMIPSState *env, DisasContext *ctx, int rd, int rt,
         /* NOP */
         ;
     } else if ((env->CP0_VPEControl & (0xff << CP0VPECo_TargTC)) >
-             (env->mvp->CP0_MVPConf0 & (0xff << CP0MVPC0_PTC))) {
+             (cpu->mvp->CP0_MVPConf0 & (0xff << CP0MVPC0_PTC))) {
         /* NOP */
         ;
     } else if (u == 0) {
diff --git a/target/mips/cpu-defs.c.inc b/target/mips/cpu-defs.c.inc
index d93b9d341ac..faefab04738 100644
--- a/target/mips/cpu-defs.c.inc
+++ b/target/mips/cpu-defs.c.inc
@@ -1034,7 +1034,9 @@  static void fpu_init (CPUMIPSState *env, const mips_def_t *def)
 
 static void mvp_init(CPUMIPSState *env)
 {
-    env->mvp = g_malloc0(sizeof(CPUMIPSMVPContext));
+    MIPSCPU *cpu = env_archcpu(env);
+
+    cpu->mvp = g_malloc0(sizeof(CPUMIPSMVPContext));
 
     if (!ase_mt_available(env)) {
         return;
@@ -1044,7 +1046,7 @@  static void mvp_init(CPUMIPSState *env)
        programmable cache partitioning implemented, number of allocatable
        and shareable TLB entries, MVP has allocatable TCs, 2 VPEs
        implemented, 5 TCs implemented. */
-    env->mvp->CP0_MVPConf0 = (1U << CP0MVPC0_M) | (1 << CP0MVPC0_TLBS) |
+    cpu->mvp->CP0_MVPConf0 = (1U << CP0MVPC0_M) | (1 << CP0MVPC0_TLBS) |
                              (0 << CP0MVPC0_GS) | (1 << CP0MVPC0_PCP) |
 // TODO: actually do 2 VPEs.
 //                             (1 << CP0MVPC0_TCA) | (0x1 << CP0MVPC0_PVPE) |
@@ -1053,12 +1055,12 @@  static void mvp_init(CPUMIPSState *env)
                              (0x00 << CP0MVPC0_PTC);
 #if !defined(CONFIG_USER_ONLY)
     /* Usermode has no TLB support */
-    env->mvp->CP0_MVPConf0 |= (env->tlb->nb_tlb << CP0MVPC0_PTLBE);
+    cpu->mvp->CP0_MVPConf0 |= (env->tlb->nb_tlb << CP0MVPC0_PTLBE);
 #endif
 
     /* Allocatable CP1 have media extensions, allocatable CP1 have FP support,
        no UDI implemented, no CP2 implemented, 1 CP1 implemented. */
-    env->mvp->CP0_MVPConf1 = (1U << CP0MVPC1_CIM) | (1 << CP0MVPC1_CIF) |
+    cpu->mvp->CP0_MVPConf1 = (1U << CP0MVPC1_CIM) | (1 << CP0MVPC1_CIF) |
                              (0x0 << CP0MVPC1_PCX) | (0x0 << CP0MVPC1_PCP2) |
                              (0x1 << CP0MVPC1_PCP1);
 }