@samitouri / QOSamiQemu / commits / 1d5a312675

target/mips: Move 'mvp' field from CPUMIPSState to MIPSCPU

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>

Peter Maydell committed Mar 17, 2026 at 17:50 UTC 1d5a312675f069d5c98b642032823929715f243d
8 files changed +40 -25
hw/mips/malta.c
+2 -2
@@ -968,10 +968,10 @@ static void malta_mips_config(MIPSCPU *cpu)
968 CPUState *cs = CPU(cpu);
969
970 if (ase_mt_available(env)) {
971 - env->mvp->CP0_MVPConf0 = deposit32(env->mvp->CP0_MVPConf0,
971 + cpu->mvp->CP0_MVPConf0 = deposit32(cpu->mvp->CP0_MVPConf0,
972 CP0MVPC0_PTC, 8,
973 smp_cpus * cs->nr_threads - 1);
974 - env->mvp->CP0_MVPConf0 = deposit32(env->mvp->CP0_MVPConf0,
974 + cpu->mvp->CP0_MVPConf0 = deposit32(cpu->mvp->CP0_MVPConf0,
975 CP0MVPC0_PVPE, 4, smp_cpus - 1);
976 }
977 }
target/mips/cpu-defs.c.inc
+6 -4
@@ -1034,7 +1034,9 @@ static void fpu_init (CPUMIPSState *env, const mips_def_t *def)
1034
1035 static void mvp_init(CPUMIPSState *env)
1036 {
1037 - env->mvp = g_malloc0(sizeof(CPUMIPSMVPContext));
1037 + MIPSCPU *cpu = env_archcpu(env);
1038 +
1039 + cpu->mvp = g_malloc0(sizeof(CPUMIPSMVPContext));
1040
1041 if (!ase_mt_available(env)) {
1042 return;
@@ -1044,7 +1046,7 @@ static void mvp_init(CPUMIPSState *env)
1046 programmable cache partitioning implemented, number of allocatable
1047 and shareable TLB entries, MVP has allocatable TCs, 2 VPEs
1048 implemented, 5 TCs implemented. */
1047 - env->mvp->CP0_MVPConf0 = (1U << CP0MVPC0_M) | (1 << CP0MVPC0_TLBS) |
1049 + cpu->mvp->CP0_MVPConf0 = (1U << CP0MVPC0_M) | (1 << CP0MVPC0_TLBS) |
1050 (0 << CP0MVPC0_GS) | (1 << CP0MVPC0_PCP) |
1051 // TODO: actually do 2 VPEs.
1052 // (1 << CP0MVPC0_TCA) | (0x1 << CP0MVPC0_PVPE) |
@@ -1053,12 +1055,12 @@ static void mvp_init(CPUMIPSState *env)
1055 (0x00 << CP0MVPC0_PTC);
1056 #if !defined(CONFIG_USER_ONLY)
1057 /* Usermode has no TLB support */
1056 - env->mvp->CP0_MVPConf0 |= (env->tlb->nb_tlb << CP0MVPC0_PTLBE);
1058 + cpu->mvp->CP0_MVPConf0 |= (env->tlb->nb_tlb << CP0MVPC0_PTLBE);
1059 #endif
1060
1061 /* Allocatable CP1 have media extensions, allocatable CP1 have FP support,
1062 no UDI implemented, no CP2 implemented, 1 CP1 implemented. */
1061 - env->mvp->CP0_MVPConf1 = (1U << CP0MVPC1_CIM) | (1 << CP0MVPC1_CIF) |
1063 + cpu->mvp->CP0_MVPConf1 = (1U << CP0MVPC1_CIM) | (1 << CP0MVPC1_CIF) |
1064 (0x0 << CP0MVPC1_PCX) | (0x0 << CP0MVPC1_PCP2) |
1065 (0x1 << CP0MVPC1_PCP1);
1066 }
target/mips/cpu.c
+1 -1
@@ -339,7 +339,7 @@ static void mips_cpu_reset_hold(Object *obj, ResetType type)
339
340 if (cs->cpu_index == 0) {
341 /* VPE0 starts up enabled. */
342 - env->mvp->CP0_MVPControl |= (1 << CP0MVPCo_EVP);
342 + cpu->mvp->CP0_MVPControl |= (1 << CP0MVPCo_EVP);
343 env->CP0_VPEConf0 |= (1 << CP0VPEC0_MVP) | (1 << CP0VPEC0_VPA);
344
345 /* TC0 starts up unhalted. */
target/mips/cpu.h
+2 -1
@@ -1174,7 +1174,6 @@ typedef struct CPUArchState {
1174 struct {} end_reset_fields;
1175
1176 /* Fields from here on are preserved across CPU reset. */
1177 - CPUMIPSMVPContext *mvp;
1177 #if !defined(CONFIG_USER_ONLY)
1178 CPUMIPSTLBContext *tlb;
1179 qemu_irq irq[8];
@@ -1209,6 +1208,8 @@ struct ArchCPU {
1208 Clock *clock;
1209 Clock *count_div; /* Divider for CP0_Count clock */
1210
1211 + CPUMIPSMVPContext *mvp;
1212 +
1213 /* Properties */
1214 bool is_big_endian;
1215 };
target/mips/internal.h
+2 -1
@@ -246,10 +246,11 @@ static inline void restore_pamask(CPUMIPSState *env)
246
247 static inline int mips_vpe_active(CPUMIPSState *env)
248 {
249 + MIPSCPU *cpu = env_archcpu(env);
250 int active = 1;
251
252 /* Check that the VPE is enabled. */
252 - if (!(env->mvp->CP0_MVPControl & (1 << CP0MVPCo_EVP))) {
253 + if (!(cpu->mvp->CP0_MVPControl & (1 << CP0MVPCo_EVP))) {
254 active = 0;
255 }
256 /* Check that the VPE is activated. */
target/mips/system/machine.c
+1 -1
@@ -251,7 +251,7 @@ const VMStateDescription vmstate_mips_cpu = {
251 CPUMIPSFPUContext),
252
253 /* MVP */
254 - VMSTATE_STRUCT_POINTER(env.mvp, MIPSCPU, vmstate_mvp,
254 + VMSTATE_STRUCT_POINTER(mvp, MIPSCPU, vmstate_mvp,
255 CPUMIPSMVPContext),
256
257 /* TLB */
target/mips/tcg/system/cp0_helper.c
+22 -13
@@ -229,17 +229,20 @@ uint32_t cpu_mips_get_random(CPUMIPSState *env)
229 /* CP0 helpers */
230 target_ulong helper_mfc0_mvpcontrol(CPUMIPSState *env)
231 {
232 - return env->mvp->CP0_MVPControl;
232 + MIPSCPU *cpu = env_archcpu(env);
233 + return cpu->mvp->CP0_MVPControl;
234 }
235
236 target_ulong helper_mfc0_mvpconf0(CPUMIPSState *env)
237 {
237 - return env->mvp->CP0_MVPConf0;
238 + MIPSCPU *cpu = env_archcpu(env);
239 + return cpu->mvp->CP0_MVPConf0;
240 }
241
242 target_ulong helper_mfc0_mvpconf1(CPUMIPSState *env)
243 {
242 - return env->mvp->CP0_MVPConf1;
244 + MIPSCPU *cpu = env_archcpu(env);
245 + return cpu->mvp->CP0_MVPConf1;
246 }
247
248 target_ulong helper_mfc0_random(CPUMIPSState *env)
@@ -514,6 +517,7 @@ void helper_mtc0_index(CPUMIPSState *env, target_ulong arg1)
517
518 void helper_mtc0_mvpcontrol(CPUMIPSState *env, target_ulong arg1)
519 {
520 + MIPSCPU *cpu = env_archcpu(env);
521 uint32_t mask = 0;
522 uint32_t newval;
523
@@ -521,14 +525,14 @@ void helper_mtc0_mvpcontrol(CPUMIPSState *env, target_ulong arg1)
525 mask |= (1 << CP0MVPCo_CPA) | (1 << CP0MVPCo_VPC) |
526 (1 << CP0MVPCo_EVP);
527 }
524 - if (env->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC)) {
528 + if (cpu->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC)) {
529 mask |= (1 << CP0MVPCo_STLB);
530 }
527 - newval = (env->mvp->CP0_MVPControl & ~mask) | (arg1 & mask);
531 + newval = (cpu->mvp->CP0_MVPControl & ~mask) | (arg1 & mask);
532
533 /* TODO: Enable/disable shared TLB, enable/disable VPEs. */
534
531 - env->mvp->CP0_MVPControl = newval;
535 + cpu->mvp->CP0_MVPControl = newval;
536 }
537
538 void helper_mtc0_vpecontrol(CPUMIPSState *env, target_ulong arg1)
@@ -616,10 +620,11 @@ void helper_mttc0_vpeconf0(CPUMIPSState *env, target_ulong arg1)
620
621 void helper_mtc0_vpeconf1(CPUMIPSState *env, target_ulong arg1)
622 {
623 + MIPSCPU *cpu = env_archcpu(env);
624 uint32_t mask = 0;
625 uint32_t newval;
626
622 - if (env->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC))
627 + if (cpu->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC))
628 mask |= (0xff << CP0VPEC1_NCX) | (0xff << CP0VPEC1_NCP2) |
629 (0xff << CP0VPEC1_NCP1);
630 newval = (env->CP0_VPEConf1 & ~mask) | (arg1 & mask);
@@ -689,10 +694,11 @@ void helper_mttc0_tcstatus(CPUMIPSState *env, target_ulong arg1)
694
695 void helper_mtc0_tcbind(CPUMIPSState *env, target_ulong arg1)
696 {
697 + MIPSCPU *cpu = env_archcpu(env);
698 uint32_t mask = (1 << CP0TCBd_TBE);
699 uint32_t newval;
700
695 - if (env->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC)) {
701 + if (cpu->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC)) {
702 mask |= (1 << CP0TCBd_CurVPE);
703 }
704 newval = (env->active_tc.CP0_TCBind & ~mask) | (arg1 & mask);
@@ -705,8 +711,9 @@ void helper_mttc0_tcbind(CPUMIPSState *env, target_ulong arg1)
711 uint32_t mask = (1 << CP0TCBd_TBE);
712 uint32_t newval;
713 CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc);
714 + MIPSCPU *other_cpu = env_archcpu(other);
715
709 - if (other->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC)) {
716 + if (other_cpu->mvp->CP0_MVPControl & (1 << CP0MVPCo_VPC)) {
717 mask |= (1 << CP0TCBd_CurVPE);
718 }
719 if (other_tc == other->current_tc) {
@@ -1560,14 +1567,15 @@ target_ulong helper_emt(void)
1567 target_ulong helper_dvpe(CPUMIPSState *env)
1568 {
1569 CPUState *other_cs = first_cpu;
1563 - target_ulong prev = env->mvp->CP0_MVPControl;
1570 + MIPSCPU *cpu = env_archcpu(env);
1571 + target_ulong prev = cpu->mvp->CP0_MVPControl;
1572
1573 if (env->CP0_VPEConf0 & (1 << CP0VPEC0_MVP)) {
1574 CPU_FOREACH(other_cs) {
1575 MIPSCPU *other_cpu = MIPS_CPU(other_cs);
1576 /* Turn off all VPEs except the one executing the dvpe. */
1577 if (&other_cpu->env != env) {
1570 - other_cpu->env.mvp->CP0_MVPControl &= ~(1 << CP0MVPCo_EVP);
1578 + other_cpu->mvp->CP0_MVPControl &= ~(1 << CP0MVPCo_EVP);
1579 mips_vpe_sleep(other_cpu);
1580 }
1581 }
@@ -1578,7 +1586,8 @@ target_ulong helper_dvpe(CPUMIPSState *env)
1586 target_ulong helper_evpe(CPUMIPSState *env)
1587 {
1588 CPUState *other_cs = first_cpu;
1581 - target_ulong prev = env->mvp->CP0_MVPControl;
1589 + MIPSCPU *cpu = env_archcpu(env);
1590 + target_ulong prev = cpu->mvp->CP0_MVPControl;
1591
1592 if (env->CP0_VPEConf0 & (1 << CP0VPEC0_MVP)) {
1593 CPU_FOREACH(other_cs) {
@@ -1588,7 +1597,7 @@ target_ulong helper_evpe(CPUMIPSState *env)
1597 /* If the VPE is WFI, don't disturb its sleep. */
1598 && !mips_vpe_is_wfi(other_cpu)) {
1599 /* Enable the VPE. */
1591 - other_cpu->env.mvp->CP0_MVPControl |= (1 << CP0MVPCo_EVP);
1600 + other_cpu->mvp->CP0_MVPControl |= (1 << CP0MVPCo_EVP);
1601 mips_vpe_wake(other_cpu); /* And wake it up. */
1602 }
1603 }
target/mips/tcg/translate.c
+4 -2
@@ -8085,6 +8085,7 @@ cp0_unimplemented:
8085 static void gen_mftr(CPUMIPSState *env, DisasContext *ctx, int rt, int rd,
8086 int u, int sel, int h)
8087 {
8088 + MIPSCPU *cpu = env_archcpu(env);
8089 int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
8090 TCGv t0 = tcg_temp_new();
8091
@@ -8093,7 +8094,7 @@ static void gen_mftr(CPUMIPSState *env, DisasContext *ctx, int rt, int rd,
8094 (env->active_tc.CP0_TCBind & (0xf << CP0TCBd_CurVPE)))) {
8095 tcg_gen_movi_tl(t0, -1);
8096 } else if ((env->CP0_VPEControl & (0xff << CP0VPECo_TargTC)) >
8096 - (env->mvp->CP0_MVPConf0 & (0xff << CP0MVPC0_PTC))) {
8097 + (cpu->mvp->CP0_MVPConf0 & (0xff << CP0MVPC0_PTC))) {
8098 tcg_gen_movi_tl(t0, -1);
8099 } else if (u == 0) {
8100 switch (rt) {
@@ -8309,6 +8310,7 @@ die:
8310 static void gen_mttr(CPUMIPSState *env, DisasContext *ctx, int rd, int rt,
8311 int u, int sel, int h)
8312 {
8313 + MIPSCPU *cpu = env_archcpu(env);
8314 int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
8315 TCGv t0 = tcg_temp_new();
8316
@@ -8319,7 +8321,7 @@ static void gen_mttr(CPUMIPSState *env, DisasContext *ctx, int rd, int rt,
8321 /* NOP */
8322 ;
8323 } else if ((env->CP0_VPEControl & (0xff << CP0VPECo_TargTC)) >
8322 - (env->mvp->CP0_MVPConf0 & (0xff << CP0MVPC0_PTC))) {
8324 + (cpu->mvp->CP0_MVPConf0 & (0xff << CP0MVPC0_PTC))) {
8325 /* NOP */
8326 ;
8327 } else if (u == 0) {