| Message ID | 20191216221158.29572-27-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cputlb: Remove support for MMU_MODE*_SUFFIX | expand |
Richard Henderson <richard.henderson@linaro.org> writes: > There are only two uses. Within dcbz_common, the local variable > mmu_idx already contains the epid computation, and we can avoid > repeating it for the store. Within helper_icbiep, the usage is > trivially expanded using PPC_TLB_EPID_LOAD. > > Acked-by: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/ppc/cpu.h | 2 -- > target/ppc/mem_helper.c | 11 ++--------- > 2 files changed, 2 insertions(+), 11 deletions(-) > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index e3e82327b7..3bd983adaa 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -951,8 +951,6 @@ struct ppc_radix_page_info { > * + real/paged mode combinations. The other two modes are for > * external PID load/store. > */ > -#define MMU_MODE8_SUFFIX _epl > -#define MMU_MODE9_SUFFIX _eps > #define PPC_TLB_EPID_LOAD 8 > #define PPC_TLB_EPID_STORE 9 > > diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c > index 1351b53f28..56855f2381 100644 > --- a/target/ppc/mem_helper.c > +++ b/target/ppc/mem_helper.c > @@ -177,14 +177,7 @@ static void dcbz_common(CPUPPCState *env, target_ulong addr, > } else { > /* Slow path */ > for (i = 0; i < dcbz_size; i += 8) { > - if (epid) { > -#if !defined(CONFIG_USER_ONLY) > - /* Does not make sense on USER_ONLY config */ > - cpu_stq_eps_ra(env, addr + i, 0, retaddr); > -#endif > - } else { > - cpu_stq_data_ra(env, addr + i, 0, retaddr); > - } > + cpu_stq_mmuidx_ra(env, addr + i, 0, mmu_idx, retaddr); I assume the possibility of a user-mode with epid is elided in the translation phase by avoiding gen_dcbzep although I can't quite see where they get called from. Anyway: Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > } > } > } > @@ -216,7 +209,7 @@ void helper_icbiep(CPUPPCState *env, target_ulong addr) > #if !defined(CONFIG_USER_ONLY) > /* See comments above */ > addr &= ~(env->dcache_line_size - 1); > - cpu_ldl_epl_ra(env, addr, GETPC()); > + cpu_ldl_mmuidx_ra(env, addr, PPC_TLB_EPID_LOAD, GETPC()); > #endif > } -- Alex Bennée
On 12/21/19 6:51 AM, Alex Bennée wrote: >> --- a/target/ppc/mem_helper.c >> +++ b/target/ppc/mem_helper.c >> @@ -177,14 +177,7 @@ static void dcbz_common(CPUPPCState *env, target_ulong addr, >> } else { >> /* Slow path */ >> for (i = 0; i < dcbz_size; i += 8) { >> - if (epid) { >> -#if !defined(CONFIG_USER_ONLY) >> - /* Does not make sense on USER_ONLY config */ >> - cpu_stq_eps_ra(env, addr + i, 0, retaddr); >> -#endif >> - } else { >> - cpu_stq_data_ra(env, addr + i, 0, retaddr); >> - } >> + cpu_stq_mmuidx_ra(env, addr + i, 0, mmu_idx, retaddr); > > I assume the possibility of a user-mode with epid is elided in the > translation phase by avoiding gen_dcbzep although I can't quite see > where they get called from. Anyway: I suspect that dcbzep (vs dcbze) is supposed to be privileged, but I can't see that enforced anywhere. Certainly one can't write to the EPSC register from userspace... r~
On Sun, Dec 29, 2019 at 08:18:35AM +1100, Richard Henderson wrote: > On 12/21/19 6:51 AM, Alex Bennée wrote: > >> --- a/target/ppc/mem_helper.c > >> +++ b/target/ppc/mem_helper.c > >> @@ -177,14 +177,7 @@ static void dcbz_common(CPUPPCState *env, target_ulong addr, > >> } else { > >> /* Slow path */ > >> for (i = 0; i < dcbz_size; i += 8) { > >> - if (epid) { > >> -#if !defined(CONFIG_USER_ONLY) > >> - /* Does not make sense on USER_ONLY config */ > >> - cpu_stq_eps_ra(env, addr + i, 0, retaddr); > >> -#endif > >> - } else { > >> - cpu_stq_data_ra(env, addr + i, 0, retaddr); > >> - } > >> + cpu_stq_mmuidx_ra(env, addr + i, 0, mmu_idx, retaddr); > > > > I assume the possibility of a user-mode with epid is elided in the > > translation phase by avoiding gen_dcbzep although I can't quite see > > where they get called from. Anyway: > > I suspect that dcbzep (vs dcbze) is supposed to be privileged, but I can't see > that enforced anywhere. Certainly one can't write to the EPSC register from > userspace... So... it's true that dcbzep is privileged (as are all the external PID instructions, I believe). I'm not certain if the reasoning you used to guess that was correct, though. In this case the suffix is "ep" for "External PID" not "p" for "Privileged". There is no "dcbze" instruction, only "dcbz" which happens not to be privileged. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index e3e82327b7..3bd983adaa 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -951,8 +951,6 @@ struct ppc_radix_page_info { * + real/paged mode combinations. The other two modes are for * external PID load/store. */ -#define MMU_MODE8_SUFFIX _epl -#define MMU_MODE9_SUFFIX _eps #define PPC_TLB_EPID_LOAD 8 #define PPC_TLB_EPID_STORE 9 diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c index 1351b53f28..56855f2381 100644 --- a/target/ppc/mem_helper.c +++ b/target/ppc/mem_helper.c @@ -177,14 +177,7 @@ static void dcbz_common(CPUPPCState *env, target_ulong addr, } else { /* Slow path */ for (i = 0; i < dcbz_size; i += 8) { - if (epid) { -#if !defined(CONFIG_USER_ONLY) - /* Does not make sense on USER_ONLY config */ - cpu_stq_eps_ra(env, addr + i, 0, retaddr); -#endif - } else { - cpu_stq_data_ra(env, addr + i, 0, retaddr); - } + cpu_stq_mmuidx_ra(env, addr + i, 0, mmu_idx, retaddr); } } } @@ -216,7 +209,7 @@ void helper_icbiep(CPUPPCState *env, target_ulong addr) #if !defined(CONFIG_USER_ONLY) /* See comments above */ addr &= ~(env->dcache_line_size - 1); - cpu_ldl_epl_ra(env, addr, GETPC()); + cpu_ldl_mmuidx_ra(env, addr, PPC_TLB_EPID_LOAD, GETPC()); #endif }