| Message ID | 20210315184615.1985590-12-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/ppc: Fix truncation of env->hflags | expand |
On Mon, Mar 15, 2021 at 12:46:09PM -0600, Richard Henderson wrote: > Because this bit was not in hflags, the privilege check > for tlb instructions was essentially random. > Recompute hflags when storing to LPCR. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Ouch. Unlike the others which come from ancient strata of qemu code, this one is pretty recent, and demonstrates that I don't really understand how hflags and TCG code generation work. Anyway, Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > target/ppc/cpu.h | 1 + > target/ppc/helper_regs.c | 3 +++ > target/ppc/mmu-hash64.c | 3 +++ > target/ppc/translate.c | 2 +- > 4 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index 2abaf56869..07a4331eec 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -603,6 +603,7 @@ enum { > HFLAGS_TM = 8, /* computed from MSR_TM */ > HFLAGS_BE = 9, /* MSR_BE -- from elsewhere on embedded ppc */ > HFLAGS_SE = 10, /* MSR_SE -- from elsewhere on embedded ppc */ > + HFLAGS_GTSE = 11, /* computed from SPR_LPCR[GTSE] */ > HFLAGS_FP = 13, /* MSR_FP */ > HFLAGS_SA = 22, /* MSR_SA */ > HFLAGS_AP = 23, /* MSR_AP */ > diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c > index c735540333..8479789e24 100644 > --- a/target/ppc/helper_regs.c > +++ b/target/ppc/helper_regs.c > @@ -139,6 +139,9 @@ void hreg_compute_hflags(CPUPPCState *env) > if ((ppc_flags & POWERPC_FLAG_TM) && (msr & (1ull << MSR_TM))) { > hflags |= 1 << HFLAGS_TM; > } > + if (env->spr[SPR_LPCR] & LPCR_GTSE) { > + hflags |= 1 << HFLAGS_GTSE; > + } > > #ifndef CONFIG_USER_ONLY > if (!env->has_hv_mode || (msr & (1ull << MSR_HV))) { > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c > index 0fabc10302..d517a99832 100644 > --- a/target/ppc/mmu-hash64.c > +++ b/target/ppc/mmu-hash64.c > @@ -30,6 +30,7 @@ > #include "exec/log.h" > #include "hw/hw.h" > #include "mmu-book3s-v3.h" > +#include "helper_regs.h" > > /* #define DEBUG_SLB */ > > @@ -1125,6 +1126,8 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val) > CPUPPCState *env = &cpu->env; > > env->spr[SPR_LPCR] = val & pcc->lpcr_mask; > + /* The gtse bit affects hflags */ > + hreg_compute_hflags(env); > } > > void helper_store_lpcr(CPUPPCState *env, target_ulong val) > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index d48c554290..5e629291d3 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -7908,7 +7908,7 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) > ctx->altivec_enabled = (hflags >> HFLAGS_VR) & 1; > ctx->vsx_enabled = (hflags >> HFLAGS_VSX) & 1; > ctx->tm_enabled = (hflags >> HFLAGS_TM) & 1; > - ctx->gtse = !!(env->spr[SPR_LPCR] & LPCR_GTSE); > + ctx->gtse = (hflags >> HFLAGS_GTSE) & 1; > > ctx->singlestep_enabled = 0; > if ((hflags >> HFLAGS_SE) & 1) { -- 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 2abaf56869..07a4331eec 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -603,6 +603,7 @@ enum { HFLAGS_TM = 8, /* computed from MSR_TM */ HFLAGS_BE = 9, /* MSR_BE -- from elsewhere on embedded ppc */ HFLAGS_SE = 10, /* MSR_SE -- from elsewhere on embedded ppc */ + HFLAGS_GTSE = 11, /* computed from SPR_LPCR[GTSE] */ HFLAGS_FP = 13, /* MSR_FP */ HFLAGS_SA = 22, /* MSR_SA */ HFLAGS_AP = 23, /* MSR_AP */ diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c index c735540333..8479789e24 100644 --- a/target/ppc/helper_regs.c +++ b/target/ppc/helper_regs.c @@ -139,6 +139,9 @@ void hreg_compute_hflags(CPUPPCState *env) if ((ppc_flags & POWERPC_FLAG_TM) && (msr & (1ull << MSR_TM))) { hflags |= 1 << HFLAGS_TM; } + if (env->spr[SPR_LPCR] & LPCR_GTSE) { + hflags |= 1 << HFLAGS_GTSE; + } #ifndef CONFIG_USER_ONLY if (!env->has_hv_mode || (msr & (1ull << MSR_HV))) { diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index 0fabc10302..d517a99832 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -30,6 +30,7 @@ #include "exec/log.h" #include "hw/hw.h" #include "mmu-book3s-v3.h" +#include "helper_regs.h" /* #define DEBUG_SLB */ @@ -1125,6 +1126,8 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val) CPUPPCState *env = &cpu->env; env->spr[SPR_LPCR] = val & pcc->lpcr_mask; + /* The gtse bit affects hflags */ + hreg_compute_hflags(env); } void helper_store_lpcr(CPUPPCState *env, target_ulong val) diff --git a/target/ppc/translate.c b/target/ppc/translate.c index d48c554290..5e629291d3 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -7908,7 +7908,7 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) ctx->altivec_enabled = (hflags >> HFLAGS_VR) & 1; ctx->vsx_enabled = (hflags >> HFLAGS_VSX) & 1; ctx->tm_enabled = (hflags >> HFLAGS_TM) & 1; - ctx->gtse = !!(env->spr[SPR_LPCR] & LPCR_GTSE); + ctx->gtse = (hflags >> HFLAGS_GTSE) & 1; ctx->singlestep_enabled = 0; if ((hflags >> HFLAGS_SE) & 1) {
Because this bit was not in hflags, the privilege check for tlb instructions was essentially random. Recompute hflags when storing to LPCR. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/ppc/cpu.h | 1 + target/ppc/helper_regs.c | 3 +++ target/ppc/mmu-hash64.c | 3 +++ target/ppc/translate.c | 2 +- 4 files changed, 8 insertions(+), 1 deletion(-) -- 2.25.1