Re: [PATCH v4 11/17] target/ppc: Put LPCR[GTSE] in hflags

[David Gibson]

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

signature.asc
Description: PGP signature