| Message ID | 20190508000641.19090-19-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tcg: Move the softmmu tlb to CPUNegativeOffsetState | expand |
On May 8, 2019 2:19 AM, "Richard Henderson" <richard.henderson@linaro.org> wrote: > > > This commit message doesnˊt explain the reason for the change, and why is this an improvement. The underlyng reason for distingishing between env_cpu and env_archcpu cases is not explained too. Thanks, Aleksandar > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/mips/cpu.h | 5 ----- > hw/intc/mips_gic.c | 2 +- > hw/mips/mips_int.c | 2 +- > linux-user/mips/cpu_loop.c | 2 +- > target/mips/helper.c | 15 +++++---------- > target/mips/op_helper.c | 25 +++++++++++-------------- > target/mips/translate.c | 3 +-- > target/mips/translate_init.inc.c | 4 +--- > 8 files changed, 21 insertions(+), 37 deletions(-) > > diff --git a/target/mips/cpu.h b/target/mips/cpu.h > index 31e15834ca..e0645eb1d1 100644 > --- a/target/mips/cpu.h > +++ b/target/mips/cpu.h > @@ -1051,11 +1051,6 @@ struct MIPSCPU { > CPUMIPSState env; > }; > > -static inline MIPSCPU *mips_env_get_cpu(CPUMIPSState *env) > -{ > - return container_of(env, MIPSCPU, env); > -} > - > #define ENV_OFFSET offsetof(MIPSCPU, env) > > void mips_cpu_list(void); > diff --git a/hw/intc/mips_gic.c b/hw/intc/mips_gic.c > index 15e6e40f9f..8f509493ea 100644 > --- a/hw/intc/mips_gic.c > +++ b/hw/intc/mips_gic.c > @@ -44,7 +44,7 @@ static void mips_gic_set_vp_irq(MIPSGICState *gic, int vp, int pin) > GIC_VP_MASK_CMP_SHF; > } > if (kvm_enabled()) { > - kvm_mips_set_ipi_interrupt(mips_env_get_cpu(gic->vps[vp].env), > + kvm_mips_set_ipi_interrupt(env_archcpu(gic->vps[vp].env), > pin + GIC_CPU_PIN_OFFSET, > ored_level); > } else { > diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c > index 5ddeb15848..f899f6ceb3 100644 > --- a/hw/mips/mips_int.c > +++ b/hw/mips/mips_int.c > @@ -76,7 +76,7 @@ void cpu_mips_irq_init_cpu(MIPSCPU *cpu) > qemu_irq *qi; > int i; > > - qi = qemu_allocate_irqs(cpu_mips_irq_request, mips_env_get_cpu(env), 8); > + qi = qemu_allocate_irqs(cpu_mips_irq_request, env_archcpu(env), 8); > for (i = 0; i < 8; i++) { > env->irq[i] = qi[i]; > } > diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c > index 828137cd84..ac6c6d1504 100644 > --- a/linux-user/mips/cpu_loop.c > +++ b/linux-user/mips/cpu_loop.c > @@ -425,7 +425,7 @@ static int do_break(CPUMIPSState *env, target_siginfo_t *info, > > void cpu_loop(CPUMIPSState *env) > { > - CPUState *cs = CPU(mips_env_get_cpu(env)); > + CPUState *cs = env_cpu(env); > target_siginfo_t info; > int trapnr; > abi_long ret; > diff --git a/target/mips/helper.c b/target/mips/helper.c > index c44cdca3b5..1fc0a4ce4b 100644 > --- a/target/mips/helper.c > +++ b/target/mips/helper.c > @@ -336,10 +336,8 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, > > void cpu_mips_tlb_flush(CPUMIPSState *env) > { > - MIPSCPU *cpu = mips_env_get_cpu(env); > - > /* Flush qemu's TLB and discard all shadowed entries. */ > - tlb_flush(CPU(cpu)); > + tlb_flush(env_cpu(env)); > env->tlb->tlb_in_use = env->tlb->nb_tlb; > } > > @@ -401,7 +399,7 @@ void cpu_mips_store_status(CPUMIPSState *env, target_ulong val) > #if defined(TARGET_MIPS64) > if ((env->CP0_Status ^ old) & (old & (7 << CP0St_UX))) { > /* Access to at least one of the 64-bit segments has been disabled */ > - tlb_flush(CPU(mips_env_get_cpu(env))); > + tlb_flush(env_cpu(env)); > } > #endif > if (env->CP0_Config3 & (1 << CP0C3_MT)) { > @@ -446,7 +444,7 @@ void cpu_mips_store_cause(CPUMIPSState *env, target_ulong val) > static void raise_mmu_exception(CPUMIPSState *env, target_ulong address, > int rw, int tlb_error) > { > - CPUState *cs = CPU(mips_env_get_cpu(env)); > + CPUState *cs = env_cpu(env); > int exception = 0, error_code = 0; > > if (rw == MMU_INST_FETCH) { > @@ -1400,8 +1398,7 @@ bool mips_cpu_exec_interrupt(CPUState *cs, int interrupt_request) > #if !defined(CONFIG_USER_ONLY) > void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra) > { > - MIPSCPU *cpu = mips_env_get_cpu(env); > - CPUState *cs; > + CPUState *cs = env_cpu(env); > r4k_tlb_t *tlb; > target_ulong addr; > target_ulong end; > @@ -1427,7 +1424,6 @@ void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra) > /* 1k pages are not supported. */ > mask = tlb->PageMask | ~(TARGET_PAGE_MASK << 1); > if (tlb->V0) { > - cs = CPU(cpu); > addr = tlb->VPN & ~mask; > #if defined(TARGET_MIPS64) > if (addr >= (0xFFFFFFFF80000000ULL & env->SEGMask)) { > @@ -1441,7 +1437,6 @@ void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra) > } > } > if (tlb->V1) { > - cs = CPU(cpu); > addr = (tlb->VPN & ~mask) | ((mask >> 1) + 1); > #if defined(TARGET_MIPS64) > if (addr >= (0xFFFFFFFF80000000ULL & env->SEGMask)) { > @@ -1462,7 +1457,7 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env, > int error_code, > uintptr_t pc) > { > - CPUState *cs = CPU(mips_env_get_cpu(env)); > + CPUState *cs = env_cpu(env); > > qemu_log_mask(CPU_LOG_INT, "%s: %d %d\n", > __func__, exception, error_code); > diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c > index 0f272a5b93..0705e8c686 100644 > --- a/target/mips/op_helper.c > +++ b/target/mips/op_helper.c > @@ -350,7 +350,7 @@ static inline hwaddr do_translate_address(CPUMIPSState *env, > int rw, uintptr_t retaddr) > { > hwaddr paddr; > - CPUState *cs = CPU(mips_env_get_cpu(env)); > + CPUState *cs = env_cpu(env); > > paddr = cpu_mips_translate_address(env, address, rw); > > @@ -699,7 +699,7 @@ static CPUMIPSState *mips_cpu_map_tc(CPUMIPSState *env, int *tc) > return env; > } > > - cs = CPU(mips_env_get_cpu(env)); > + cs = env_cpu(env); > vpe_idx = tc_idx / cs->nr_threads; > *tc = tc_idx % cs->nr_threads; > other_cs = qemu_get_cpu(vpe_idx); > @@ -1298,7 +1298,7 @@ void helper_mttc0_tcrestart(CPUMIPSState *env, target_ulong arg1) > > void helper_mtc0_tchalt(CPUMIPSState *env, target_ulong arg1) > { > - MIPSCPU *cpu = mips_env_get_cpu(env); > + MIPSCPU *cpu = env_archcpu(env); > > env->active_tc.CP0_TCHalt = arg1 & 0x1; > > @@ -1314,7 +1314,7 @@ void helper_mttc0_tchalt(CPUMIPSState *env, target_ulong arg1) > { > int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC); > CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc); > - MIPSCPU *other_cpu = mips_env_get_cpu(other); > + MIPSCPU *other_cpu = env_archcpu(other); > > // TODO: Halt TC / Restart (if allocated+active) TC. > > @@ -1427,7 +1427,7 @@ void helper_mtc0_pagegrain(CPUMIPSState *env, target_ulong arg1) > > void helper_mtc0_segctl0(CPUMIPSState *env, target_ulong arg1) > { > - CPUState *cs = CPU(mips_env_get_cpu(env)); > + CPUState *cs = env_cpu(env); > > env->CP0_SegCtl0 = arg1 & CP0SC0_MASK; > tlb_flush(cs); > @@ -1435,7 +1435,7 @@ void helper_mtc0_segctl0(CPUMIPSState *env, target_ulong arg1) > > void helper_mtc0_segctl1(CPUMIPSState *env, target_ulong arg1) > { > - CPUState *cs = CPU(mips_env_get_cpu(env)); > + CPUState *cs = env_cpu(env); > > env->CP0_SegCtl1 = arg1 & CP0SC1_MASK; > tlb_flush(cs); > @@ -1443,7 +1443,7 @@ void helper_mtc0_segctl1(CPUMIPSState *env, target_ulong arg1) > > void helper_mtc0_segctl2(CPUMIPSState *env, target_ulong arg1) > { > - CPUState *cs = CPU(mips_env_get_cpu(env)); > + CPUState *cs = env_cpu(env); > > env->CP0_SegCtl2 = arg1 & CP0SC2_MASK; > tlb_flush(cs); > @@ -1666,7 +1666,7 @@ void helper_mtc0_entryhi(CPUMIPSState *env, target_ulong arg1) > /* If the ASID changes, flush qemu's TLB. */ > if ((old & env->CP0_EntryHi_ASID_mask) != > (val & env->CP0_EntryHi_ASID_mask)) { > - tlb_flush(CPU(mips_env_get_cpu(env))); > + tlb_flush(env_cpu(env)); > } > } > > @@ -1686,7 +1686,6 @@ void helper_mtc0_compare(CPUMIPSState *env, target_ulong arg1) > > void helper_mtc0_status(CPUMIPSState *env, target_ulong arg1) > { > - MIPSCPU *cpu = mips_env_get_cpu(env); > uint32_t val, old; > > old = env->CP0_Status; > @@ -1706,7 +1705,7 @@ void helper_mtc0_status(CPUMIPSState *env, target_ulong arg1) > case MIPS_HFLAG_SM: qemu_log(", SM\n"); break; > case MIPS_HFLAG_KM: qemu_log("\n"); break; > default: > - cpu_abort(CPU(cpu), "Invalid MMU mode!\n"); > + cpu_abort(env_cpu(env), "Invalid MMU mode!\n"); > break; > } > } > @@ -2485,8 +2484,6 @@ static void debug_pre_eret(CPUMIPSState *env) > > static void debug_post_eret(CPUMIPSState *env) > { > - MIPSCPU *cpu = mips_env_get_cpu(env); > - > if (qemu_loglevel_mask(CPU_LOG_EXEC)) { > qemu_log(" => PC " TARGET_FMT_lx " EPC " TARGET_FMT_lx, > env->active_tc.PC, env->CP0_EPC); > @@ -2502,7 +2499,7 @@ static void debug_post_eret(CPUMIPSState *env) > case MIPS_HFLAG_SM: qemu_log(", SM\n"); break; > case MIPS_HFLAG_KM: qemu_log("\n"); break; > default: > - cpu_abort(CPU(cpu), "Invalid MMU mode!\n"); > + cpu_abort(env_cpu(env), "Invalid MMU mode!\n"); > break; > } > } > @@ -2633,7 +2630,7 @@ void helper_pmon(CPUMIPSState *env, int function) > > void helper_wait(CPUMIPSState *env) > { > - CPUState *cs = CPU(mips_env_get_cpu(env)); > + CPUState *cs = env_cpu(env); > > cs->halted = 1; > cpu_reset_interrupt(cs, CPU_INTERRUPT_WAKE); > diff --git a/target/mips/translate.c b/target/mips/translate.c > index f96c0d01ef..8043e8d398 100644 > --- a/target/mips/translate.c > +++ b/target/mips/translate.c > @@ -29912,8 +29912,7 @@ void cpu_set_exception_base(int vp_index, target_ulong address) > > void cpu_state_reset(CPUMIPSState *env) > { > - MIPSCPU *cpu = mips_env_get_cpu(env); > - CPUState *cs = CPU(cpu); > + CPUState *cs = env_cpu(env); > > /* Reset registers to their default values */ > env->CP0_PRid = env->cpu_model->CP0_PRid; > diff --git a/target/mips/translate_init.inc.c b/target/mips/translate_init.inc.c > index 1c2d017d36..6d145a905a 100644 > --- a/target/mips/translate_init.inc.c > +++ b/target/mips/translate_init.inc.c > @@ -871,8 +871,6 @@ static void r4k_mmu_init (CPUMIPSState *env, const mips_def_t *def) > > static void mmu_init (CPUMIPSState *env, const mips_def_t *def) > { > - MIPSCPU *cpu = mips_env_get_cpu(env); > - > env->tlb = g_malloc0(sizeof(CPUMIPSTLBContext)); > > switch (def->mmu_type) { > @@ -889,7 +887,7 @@ static void mmu_init (CPUMIPSState *env, const mips_def_t *def) > case MMU_TYPE_R6000: > case MMU_TYPE_R8000: > default: > - cpu_abort(CPU(cpu), "MMU type not supported\n"); > + cpu_abort(env_cpu(env), "MMU type not supported\n"); > } > } > #endif /* CONFIG_USER_ONLY */ > -- > 2.17.1 > >
On 5/8/19 1:15 AM, Aleksandar Markovic wrote: > > On May 8, 2019 2:19 AM, "Richard Henderson" <richard.henderson@linaro.org > <mailto:richard.henderson@linaro.org>> wrote: >> >> >> > > This commit message doesnˊt explain the reason for the change, and why is this > an improvement. The underlyng reason for distingishing between env_cpu and > env_archcpu cases is not explained too. It's certainly explained in the preceeding patches that introduce those functions. Are you suggesting that it is beneficial to copy-and-paste a common block explanation into 21 commit messages for each of target/foo/? r~
Hi Richard, Aleksandar. On 5/8/19 4:32 PM, Richard Henderson wrote: > On 5/8/19 1:15 AM, Aleksandar Markovic wrote: >> >> On May 8, 2019 2:19 AM, "Richard Henderson" <richard.henderson@linaro.org >> <mailto:richard.henderson@linaro.org>> wrote: >>> >>> >>> >> >> This commit message doesnˊt explain the reason for the change, and why is this >> an improvement. The underlyng reason for distingishing between env_cpu and >> env_archcpu cases is not explained too. > > It's certainly explained in the preceeding patches that introduce those functions. > > Are you suggesting that it is beneficial to copy-and-paste a common block > explanation into 21 commit messages for each of target/foo/? *) Richard: I tried to put myself in Aleksandar shoes. I believe Aleksandar is worried about his MIPS maintainer duty, wanting to Ack-by this patch. It is true that out of the context of the series, it is hard to see what is the problem you try to solve. You could copy/paste the explanation you used previously, with s/$arch/mips/: "Cleanup in the boilerplate that each target must define." "Combined uses of CPU(mips_env_get_cpu()) were failures to use the more proper, ENV_GET_CPU macro, now replaced by env_cpu." Now to clearly understand this patch we still need to look at the previous two arch-generic patches - "cpu: Replace ENV_GET_CPU with env_cpu" and - "cpu: Introduce env_archcpu". Also, it is tedious to copy/paste the same explanation, but thinking of forks or stable branch that cherry-pick not all but some commits of a series, it might be useful. Another guess is Aleksandar might have looked at the series cover, which is not well explained as your v2: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07635.html I think you mistakenly copied the v1 blurb instead of the v2 one. So at some point I can understand Aleksandar frustation. *) Aleksandar: This series fall under the "Overall Guest CPU cores (TCG)" section maintained by Richard and Paolo. I think you have to see this series as a whole to understand the benefits of it. With the same reasoning, I believe you shouldn't worry to not give your Ack if you don't feel comfortable. I think Richard sent this v3 to simply address comments raised by the previous reviewer during v1/v2, where there was some discussions: I took it as "this is the last round before getting merged" (unless someone object). It is hard to make everybody happy on a such big project, with so many areas, lines of code, people, culture, etc... I believe we all try to give our best, neither the commiters nor the reviewers are perfect, but slowly we help this project to improve :) Best regards, Phil.
On May 8, 2019 11:53 PM, "Philippe Mathieu-Daudé" <philmd@redhat.com> wrote: > > Hi Richard, Aleksandar. > > On 5/8/19 4:32 PM, Richard Henderson wrote: > > On 5/8/19 1:15 AM, Aleksandar Markovic wrote: > >> > >> On May 8, 2019 2:19 AM, "Richard Henderson" < richard.henderson@linaro.org > >> <mailto:richard.henderson@linaro.org>> wrote: > >>> > >>> > >>> > >> > >> This commit message doesnˊt explain the reason for the change, and why is this > >> an improvement. The underlyng reason for distingishing between env_cpu and > >> env_archcpu cases is not explained too. > > > > It's certainly explained in the preceeding patches that introduce those functions. > > > > Are you suggesting that it is beneficial to copy-and-paste a common block > > explanation into 21 commit messages for each of target/foo/? > > > *) Richard: > > I tried to put myself in Aleksandar shoes. I believe Aleksandar is > worried about his MIPS maintainer duty, wanting to Ack-by this patch. > > It is true that out of the context of the series, it is hard to see what > is the problem you try to solve. > > You could copy/paste the explanation you used previously, > with s/$arch/mips/: > > "Cleanup in the boilerplate that each target must define." > > "Combined uses of CPU(mips_env_get_cpu()) were failures to use > the more proper, ENV_GET_CPU macro, now replaced by env_cpu." > > Now to clearly understand this patch we still need to look at the > previous two arch-generic patches > - "cpu: Replace ENV_GET_CPU with env_cpu" and > - "cpu: Introduce env_archcpu". > > Also, it is tedious to copy/paste the same explanation, but thinking of > forks or stable branch that cherry-pick not all but some commits of a > series, it might be useful. > > Another guess is Aleksandar might have looked at the series cover, which > is not well explained as your v2: > https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07635.html > I think you mistakenly copied the v1 blurb instead of the v2 one. > > So at some point I can understand Aleksandar frustation. > > > *) Aleksandar: > > This series fall under the "Overall Guest CPU cores (TCG)" section > maintained by Richard and Paolo. I think you have to see this series as > a whole to understand the benefits of it. > > With the same reasoning, I believe you shouldn't worry to not give your > Ack if you don't feel comfortable. > > I think Richard sent this v3 to simply address comments raised by the > previous reviewer during v1/v2, where there was some discussions: I took > it as "this is the last round before getting merged" (unless someone > object). > > It is hard to make everybody happy on a such big project, with so many > areas, lines of code, people, culture, etc... I believe we all try to > give our best, neither the commiters nor the reviewers are perfect, but > slowly we help this project to improve :) > > > Best regards, > > Phil. Richard, Philippe, A commit message along the line that Philippe put together would be OK. I can talk about this commit only - if other submaintainers are fine with empty commit messages in key files for their target, that is their business. I am certainly opposed to any empty commit messages in MIPS files, and please, Richard, include a decent commit message for this commit. I don't think I am asking much. Thanks, Aleksandar
On May 8, 2019 4:33 PM, "Richard Henderson" <richard.henderson@linaro.org> wrote: > > On 5/8/19 1:15 AM, Aleksandar Markovic wrote: > > > > On May 8, 2019 2:19 AM, "Richard Henderson" < richard.henderson@linaro.org > > <mailto:richard.henderson@linaro.org>> wrote: > >> > >> > >> > > > > This commit message doesnˊt explain the reason for the change, and why is this > > an improvement. The underlyng reason for distingishing between env_cpu and > > env_archcpu cases is not explained too. > > It's certainly explained in the preceeding patches that introduce those functions. > A commit (code+message) should be as standalone as possible, and one should not be forced to resort to reverse-engineering and perusing mailing list or patchwork in order to reveal its true meaning in another commit message altogether. Thanks, Aleksandar > Are you suggesting that it is beneficial to copy-and-paste a common block > explanation into 21 commit messages for each of target/foo/? > > > r~
On May 8, 2019 4:33 PM, "Richard Henderson" <richard.henderson@linaro.org> wrote: > > On 5/8/19 1:15 AM, Aleksandar Markovic wrote: > > > > On May 8, 2019 2:19 AM, "Richard Henderson" < richard.henderson@linaro.org > > <mailto:richard.henderson@linaro.org>> wrote: > >> > >> > >> > > > > This commit message doesnˊt explain the reason for the change, and why is this > > an improvement. The underlyng reason for distingishing between env_cpu and > > env_archcpu cases is not explained too. > > It's certainly explained in the preceeding patches that introduce those functions. > > Are you suggesting that it is beneficial to copy-and-paste a common block > explanation into 21 commit messages for each of target/foo/? My objection, as I am the maintainer for MIPS part, is about this very commit. If you can't put together a classical standalone commit message which will be according to our guidelines for writing a good commit message, the minimum I expect from you is the following commit message: “Please refer to the commit message(s) for commit(s) <here you list the titles of the commits that contain explanation for this commit> for details.” If I were you, I would do the same in all similar cases, but again, at this moment I am talking about this commit only, and I am insisting on not allowing empty commit messages for any code I maintain, without exceptions. Regards, Aleksandar > > r~
diff --git a/target/mips/cpu.h b/target/mips/cpu.h index 31e15834ca..e0645eb1d1 100644 --- a/target/mips/cpu.h +++ b/target/mips/cpu.h @@ -1051,11 +1051,6 @@ struct MIPSCPU { CPUMIPSState env; }; -static inline MIPSCPU *mips_env_get_cpu(CPUMIPSState *env) -{ - return container_of(env, MIPSCPU, env); -} - #define ENV_OFFSET offsetof(MIPSCPU, env) void mips_cpu_list(void); diff --git a/hw/intc/mips_gic.c b/hw/intc/mips_gic.c index 15e6e40f9f..8f509493ea 100644 --- a/hw/intc/mips_gic.c +++ b/hw/intc/mips_gic.c @@ -44,7 +44,7 @@ static void mips_gic_set_vp_irq(MIPSGICState *gic, int vp, int pin) GIC_VP_MASK_CMP_SHF; } if (kvm_enabled()) { - kvm_mips_set_ipi_interrupt(mips_env_get_cpu(gic->vps[vp].env), + kvm_mips_set_ipi_interrupt(env_archcpu(gic->vps[vp].env), pin + GIC_CPU_PIN_OFFSET, ored_level); } else { diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c index 5ddeb15848..f899f6ceb3 100644 --- a/hw/mips/mips_int.c +++ b/hw/mips/mips_int.c @@ -76,7 +76,7 @@ void cpu_mips_irq_init_cpu(MIPSCPU *cpu) qemu_irq *qi; int i; - qi = qemu_allocate_irqs(cpu_mips_irq_request, mips_env_get_cpu(env), 8); + qi = qemu_allocate_irqs(cpu_mips_irq_request, env_archcpu(env), 8); for (i = 0; i < 8; i++) { env->irq[i] = qi[i]; } diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c index 828137cd84..ac6c6d1504 100644 --- a/linux-user/mips/cpu_loop.c +++ b/linux-user/mips/cpu_loop.c @@ -425,7 +425,7 @@ static int do_break(CPUMIPSState *env, target_siginfo_t *info, void cpu_loop(CPUMIPSState *env) { - CPUState *cs = CPU(mips_env_get_cpu(env)); + CPUState *cs = env_cpu(env); target_siginfo_t info; int trapnr; abi_long ret; diff --git a/target/mips/helper.c b/target/mips/helper.c index c44cdca3b5..1fc0a4ce4b 100644 --- a/target/mips/helper.c +++ b/target/mips/helper.c @@ -336,10 +336,8 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, void cpu_mips_tlb_flush(CPUMIPSState *env) { - MIPSCPU *cpu = mips_env_get_cpu(env); - /* Flush qemu's TLB and discard all shadowed entries. */ - tlb_flush(CPU(cpu)); + tlb_flush(env_cpu(env)); env->tlb->tlb_in_use = env->tlb->nb_tlb; } @@ -401,7 +399,7 @@ void cpu_mips_store_status(CPUMIPSState *env, target_ulong val) #if defined(TARGET_MIPS64) if ((env->CP0_Status ^ old) & (old & (7 << CP0St_UX))) { /* Access to at least one of the 64-bit segments has been disabled */ - tlb_flush(CPU(mips_env_get_cpu(env))); + tlb_flush(env_cpu(env)); } #endif if (env->CP0_Config3 & (1 << CP0C3_MT)) { @@ -446,7 +444,7 @@ void cpu_mips_store_cause(CPUMIPSState *env, target_ulong val) static void raise_mmu_exception(CPUMIPSState *env, target_ulong address, int rw, int tlb_error) { - CPUState *cs = CPU(mips_env_get_cpu(env)); + CPUState *cs = env_cpu(env); int exception = 0, error_code = 0; if (rw == MMU_INST_FETCH) { @@ -1400,8 +1398,7 @@ bool mips_cpu_exec_interrupt(CPUState *cs, int interrupt_request) #if !defined(CONFIG_USER_ONLY) void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra) { - MIPSCPU *cpu = mips_env_get_cpu(env); - CPUState *cs; + CPUState *cs = env_cpu(env); r4k_tlb_t *tlb; target_ulong addr; target_ulong end; @@ -1427,7 +1424,6 @@ void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra) /* 1k pages are not supported. */ mask = tlb->PageMask | ~(TARGET_PAGE_MASK << 1); if (tlb->V0) { - cs = CPU(cpu); addr = tlb->VPN & ~mask; #if defined(TARGET_MIPS64) if (addr >= (0xFFFFFFFF80000000ULL & env->SEGMask)) { @@ -1441,7 +1437,6 @@ void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra) } } if (tlb->V1) { - cs = CPU(cpu); addr = (tlb->VPN & ~mask) | ((mask >> 1) + 1); #if defined(TARGET_MIPS64) if (addr >= (0xFFFFFFFF80000000ULL & env->SEGMask)) { @@ -1462,7 +1457,7 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env, int error_code, uintptr_t pc) { - CPUState *cs = CPU(mips_env_get_cpu(env)); + CPUState *cs = env_cpu(env); qemu_log_mask(CPU_LOG_INT, "%s: %d %d\n", __func__, exception, error_code); diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c index 0f272a5b93..0705e8c686 100644 --- a/target/mips/op_helper.c +++ b/target/mips/op_helper.c @@ -350,7 +350,7 @@ static inline hwaddr do_translate_address(CPUMIPSState *env, int rw, uintptr_t retaddr) { hwaddr paddr; - CPUState *cs = CPU(mips_env_get_cpu(env)); + CPUState *cs = env_cpu(env); paddr = cpu_mips_translate_address(env, address, rw); @@ -699,7 +699,7 @@ static CPUMIPSState *mips_cpu_map_tc(CPUMIPSState *env, int *tc) return env; } - cs = CPU(mips_env_get_cpu(env)); + cs = env_cpu(env); vpe_idx = tc_idx / cs->nr_threads; *tc = tc_idx % cs->nr_threads; other_cs = qemu_get_cpu(vpe_idx); @@ -1298,7 +1298,7 @@ void helper_mttc0_tcrestart(CPUMIPSState *env, target_ulong arg1) void helper_mtc0_tchalt(CPUMIPSState *env, target_ulong arg1) { - MIPSCPU *cpu = mips_env_get_cpu(env); + MIPSCPU *cpu = env_archcpu(env); env->active_tc.CP0_TCHalt = arg1 & 0x1; @@ -1314,7 +1314,7 @@ void helper_mttc0_tchalt(CPUMIPSState *env, target_ulong arg1) { int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC); CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc); - MIPSCPU *other_cpu = mips_env_get_cpu(other); + MIPSCPU *other_cpu = env_archcpu(other); // TODO: Halt TC / Restart (if allocated+active) TC. @@ -1427,7 +1427,7 @@ void helper_mtc0_pagegrain(CPUMIPSState *env, target_ulong arg1) void helper_mtc0_segctl0(CPUMIPSState *env, target_ulong arg1) { - CPUState *cs = CPU(mips_env_get_cpu(env)); + CPUState *cs = env_cpu(env); env->CP0_SegCtl0 = arg1 & CP0SC0_MASK; tlb_flush(cs); @@ -1435,7 +1435,7 @@ void helper_mtc0_segctl0(CPUMIPSState *env, target_ulong arg1) void helper_mtc0_segctl1(CPUMIPSState *env, target_ulong arg1) { - CPUState *cs = CPU(mips_env_get_cpu(env)); + CPUState *cs = env_cpu(env); env->CP0_SegCtl1 = arg1 & CP0SC1_MASK; tlb_flush(cs); @@ -1443,7 +1443,7 @@ void helper_mtc0_segctl1(CPUMIPSState *env, target_ulong arg1) void helper_mtc0_segctl2(CPUMIPSState *env, target_ulong arg1) { - CPUState *cs = CPU(mips_env_get_cpu(env)); + CPUState *cs = env_cpu(env); env->CP0_SegCtl2 = arg1 & CP0SC2_MASK; tlb_flush(cs); @@ -1666,7 +1666,7 @@ void helper_mtc0_entryhi(CPUMIPSState *env, target_ulong arg1) /* If the ASID changes, flush qemu's TLB. */ if ((old & env->CP0_EntryHi_ASID_mask) != (val & env->CP0_EntryHi_ASID_mask)) { - tlb_flush(CPU(mips_env_get_cpu(env))); + tlb_flush(env_cpu(env)); } } @@ -1686,7 +1686,6 @@ void helper_mtc0_compare(CPUMIPSState *env, target_ulong arg1) void helper_mtc0_status(CPUMIPSState *env, target_ulong arg1) { - MIPSCPU *cpu = mips_env_get_cpu(env); uint32_t val, old; old = env->CP0_Status; @@ -1706,7 +1705,7 @@ void helper_mtc0_status(CPUMIPSState *env, target_ulong arg1) case MIPS_HFLAG_SM: qemu_log(", SM\n"); break; case MIPS_HFLAG_KM: qemu_log("\n"); break; default: - cpu_abort(CPU(cpu), "Invalid MMU mode!\n"); + cpu_abort(env_cpu(env), "Invalid MMU mode!\n"); break; } } @@ -2485,8 +2484,6 @@ static void debug_pre_eret(CPUMIPSState *env) static void debug_post_eret(CPUMIPSState *env) { - MIPSCPU *cpu = mips_env_get_cpu(env); - if (qemu_loglevel_mask(CPU_LOG_EXEC)) { qemu_log(" => PC " TARGET_FMT_lx " EPC " TARGET_FMT_lx, env->active_tc.PC, env->CP0_EPC); @@ -2502,7 +2499,7 @@ static void debug_post_eret(CPUMIPSState *env) case MIPS_HFLAG_SM: qemu_log(", SM\n"); break; case MIPS_HFLAG_KM: qemu_log("\n"); break; default: - cpu_abort(CPU(cpu), "Invalid MMU mode!\n"); + cpu_abort(env_cpu(env), "Invalid MMU mode!\n"); break; } } @@ -2633,7 +2630,7 @@ void helper_pmon(CPUMIPSState *env, int function) void helper_wait(CPUMIPSState *env) { - CPUState *cs = CPU(mips_env_get_cpu(env)); + CPUState *cs = env_cpu(env); cs->halted = 1; cpu_reset_interrupt(cs, CPU_INTERRUPT_WAKE); diff --git a/target/mips/translate.c b/target/mips/translate.c index f96c0d01ef..8043e8d398 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -29912,8 +29912,7 @@ void cpu_set_exception_base(int vp_index, target_ulong address) void cpu_state_reset(CPUMIPSState *env) { - MIPSCPU *cpu = mips_env_get_cpu(env); - CPUState *cs = CPU(cpu); + CPUState *cs = env_cpu(env); /* Reset registers to their default values */ env->CP0_PRid = env->cpu_model->CP0_PRid; diff --git a/target/mips/translate_init.inc.c b/target/mips/translate_init.inc.c index 1c2d017d36..6d145a905a 100644 --- a/target/mips/translate_init.inc.c +++ b/target/mips/translate_init.inc.c @@ -871,8 +871,6 @@ static void r4k_mmu_init (CPUMIPSState *env, const mips_def_t *def) static void mmu_init (CPUMIPSState *env, const mips_def_t *def) { - MIPSCPU *cpu = mips_env_get_cpu(env); - env->tlb = g_malloc0(sizeof(CPUMIPSTLBContext)); switch (def->mmu_type) { @@ -889,7 +887,7 @@ static void mmu_init (CPUMIPSState *env, const mips_def_t *def) case MMU_TYPE_R6000: case MMU_TYPE_R8000: default: - cpu_abort(CPU(cpu), "MMU type not supported\n"); + cpu_abort(env_cpu(env), "MMU type not supported\n"); } } #endif /* CONFIG_USER_ONLY */