| Message ID | 20240710032814.104643-7-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fixes for user-only munmap races | expand |
On Tue, 9 Jul 2024, Richard Henderson wrote: > The 970 logic does not apply to dcbzep, which is an e500 insn. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu> As all callers pass GETPC() to retaddr maybe that could be moved into dcbz_common() as i had in my patch. Regards, BALATON Zoltan > --- > target/ppc/mem_helper.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c > index 361fd72226..5067919ff8 100644 > --- a/target/ppc/mem_helper.c > +++ b/target/ppc/mem_helper.c > @@ -271,22 +271,12 @@ void helper_stsw(CPUPPCState *env, target_ulong addr, uint32_t nb, > } > > static void dcbz_common(CPUPPCState *env, target_ulong addr, > - uint32_t opcode, int mmu_idx, uintptr_t retaddr) > + int dcbz_size, int mmu_idx, uintptr_t retaddr) > { > - target_ulong mask, dcbz_size = env->dcache_line_size; > - uint32_t i; > + target_ulong mask = ~(target_ulong)(dcbz_size - 1); > void *haddr; > > -#if defined(TARGET_PPC64) > - /* Check for dcbz vs dcbzl on 970 */ > - if (env->excp_model == POWERPC_EXCP_970 && > - !(opcode & 0x00200000) && ((env->spr[SPR_970_HID5] >> 7) & 0x3) == 1) { > - dcbz_size = 32; > - } > -#endif > - > /* Align address */ > - mask = ~(dcbz_size - 1); > addr &= mask; > > /* Check reservation */ > @@ -300,7 +290,7 @@ static void dcbz_common(CPUPPCState *env, target_ulong addr, > memset(haddr, 0, dcbz_size); > } else { > /* Slow path */ > - for (i = 0; i < dcbz_size; i += 8) { > + for (int i = 0; i < dcbz_size; i += 8) { > cpu_stq_mmuidx_ra(env, addr + i, 0, mmu_idx, retaddr); > } > } > @@ -308,12 +298,22 @@ static void dcbz_common(CPUPPCState *env, target_ulong addr, > > void helper_dcbz(CPUPPCState *env, target_ulong addr, uint32_t opcode) > { > - dcbz_common(env, addr, opcode, ppc_env_mmu_index(env, false), GETPC()); > + int dcbz_size = env->dcache_line_size; > + > +#if defined(TARGET_PPC64) > + /* Check for dcbz vs dcbzl on 970 */ > + if (env->excp_model == POWERPC_EXCP_970 && > + !(opcode & 0x00200000) && ((env->spr[SPR_970_HID5] >> 7) & 0x3) == 1) { > + dcbz_size = 32; > + } > +#endif > + > + dcbz_common(env, addr, dcbz_size, ppc_env_mmu_index(env, false), GETPC()); > } > > void helper_dcbzep(CPUPPCState *env, target_ulong addr, uint32_t opcode) > { > - dcbz_common(env, addr, opcode, PPC_TLB_EPID_STORE, GETPC()); > + dcbz_common(env, addr, env->dcache_line_size, PPC_TLB_EPID_STORE, GETPC()); > } > > void helper_icbi(CPUPPCState *env, target_ulong addr) >
On 7/10/24 05:11, BALATON Zoltan wrote: > On Tue, 9 Jul 2024, Richard Henderson wrote: >> The 970 logic does not apply to dcbzep, which is an e500 insn. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > > Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu> > > As all callers pass GETPC() to retaddr maybe that could be moved into dcbz_common() as i > had in my patch. No, that would be incorrect usage of GETPC. It *must* be used in the HELPER directly called from TCG. r~
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c index 361fd72226..5067919ff8 100644 --- a/target/ppc/mem_helper.c +++ b/target/ppc/mem_helper.c @@ -271,22 +271,12 @@ void helper_stsw(CPUPPCState *env, target_ulong addr, uint32_t nb, } static void dcbz_common(CPUPPCState *env, target_ulong addr, - uint32_t opcode, int mmu_idx, uintptr_t retaddr) + int dcbz_size, int mmu_idx, uintptr_t retaddr) { - target_ulong mask, dcbz_size = env->dcache_line_size; - uint32_t i; + target_ulong mask = ~(target_ulong)(dcbz_size - 1); void *haddr; -#if defined(TARGET_PPC64) - /* Check for dcbz vs dcbzl on 970 */ - if (env->excp_model == POWERPC_EXCP_970 && - !(opcode & 0x00200000) && ((env->spr[SPR_970_HID5] >> 7) & 0x3) == 1) { - dcbz_size = 32; - } -#endif - /* Align address */ - mask = ~(dcbz_size - 1); addr &= mask; /* Check reservation */ @@ -300,7 +290,7 @@ static void dcbz_common(CPUPPCState *env, target_ulong addr, memset(haddr, 0, dcbz_size); } else { /* Slow path */ - for (i = 0; i < dcbz_size; i += 8) { + for (int i = 0; i < dcbz_size; i += 8) { cpu_stq_mmuidx_ra(env, addr + i, 0, mmu_idx, retaddr); } } @@ -308,12 +298,22 @@ static void dcbz_common(CPUPPCState *env, target_ulong addr, void helper_dcbz(CPUPPCState *env, target_ulong addr, uint32_t opcode) { - dcbz_common(env, addr, opcode, ppc_env_mmu_index(env, false), GETPC()); + int dcbz_size = env->dcache_line_size; + +#if defined(TARGET_PPC64) + /* Check for dcbz vs dcbzl on 970 */ + if (env->excp_model == POWERPC_EXCP_970 && + !(opcode & 0x00200000) && ((env->spr[SPR_970_HID5] >> 7) & 0x3) == 1) { + dcbz_size = 32; + } +#endif + + dcbz_common(env, addr, dcbz_size, ppc_env_mmu_index(env, false), GETPC()); } void helper_dcbzep(CPUPPCState *env, target_ulong addr, uint32_t opcode) { - dcbz_common(env, addr, opcode, PPC_TLB_EPID_STORE, GETPC()); + dcbz_common(env, addr, env->dcache_line_size, PPC_TLB_EPID_STORE, GETPC()); } void helper_icbi(CPUPPCState *env, target_ulong addr)
The 970 logic does not apply to dcbzep, which is an e500 insn. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/ppc/mem_helper.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)