diff mbox series

[v4,07/17] target/ppc: Disconnect hflags from MSR

Message ID 20210315184615.1985590-8-richard.henderson@linaro.org
State Superseded
Headers show
Series target/ppc: Fix truncation of env->hflags | expand

Commit Message

Richard Henderson March 15, 2021, 6:46 p.m. UTC
Copying flags directly from msr has drawbacks: (1) msr bits
mean different things per cpu, (2) msr has 64 bits on 64 cpus
while tb->flags has only 32 bits.

Create a enum to define these bits.  Document the origin of each bit.
This fixes the truncation of env->hflags to tb->flags, because we no
longer have hflags bits set above bit 31.

Most of the code in ppc_tr_init_disas_context is moved over to
hreg_compute_hflags.  Some of it is simple extractions from msr,
some requires examining other cpu flags.  Anything that is moved
becomes a simple extract from hflags in ppc_tr_init_disas_context.

Several existing bugs are left in ppc_tr_init_disas_context, where
additional changes are required -- to be addressed in future patches.

Remove a broken #if 0 block.

Reported-by: Ivan Warren <ivan@vmfacility.fr>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/ppc/cpu.h         | 24 ++++++++++++++++++
 target/ppc/helper_regs.c | 55 ++++++++++++++++++++++++++++++++--------
 target/ppc/translate.c   | 55 ++++++++++++----------------------------
 3 files changed, 84 insertions(+), 50 deletions(-)

-- 
2.25.1

Comments

David Gibson March 22, 2021, 3:52 a.m. UTC | #1
On Mon, Mar 15, 2021 at 12:46:05PM -0600, Richard Henderson wrote:
> Copying flags directly from msr has drawbacks: (1) msr bits

> mean different things per cpu, (2) msr has 64 bits on 64 cpus

> while tb->flags has only 32 bits.

> 

> Create a enum to define these bits.  Document the origin of each bit.

> This fixes the truncation of env->hflags to tb->flags, because we no

> longer have hflags bits set above bit 31.

> 

> Most of the code in ppc_tr_init_disas_context is moved over to

> hreg_compute_hflags.  Some of it is simple extractions from msr,

> some requires examining other cpu flags.  Anything that is moved

> becomes a simple extract from hflags in ppc_tr_init_disas_context.

> 

> Several existing bugs are left in ppc_tr_init_disas_context, where

> additional changes are required -- to be addressed in future patches.

> 

> Remove a broken #if 0 block.

> 

> Reported-by: Ivan Warren <ivan@vmfacility.fr>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  target/ppc/cpu.h         | 24 ++++++++++++++++++

>  target/ppc/helper_regs.c | 55 ++++++++++++++++++++++++++++++++--------

>  target/ppc/translate.c   | 55 ++++++++++++----------------------------

>  3 files changed, 84 insertions(+), 50 deletions(-)

> 

> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h

> index fd13489dce..39f35b570c 100644

> --- a/target/ppc/cpu.h

> +++ b/target/ppc/cpu.h

> @@ -585,6 +585,30 @@ enum {

>      POWERPC_FLAG_HID0_LE  = 0x00400000,

>  };

>  

> +/*

> + * Bits for env->hflags.

> + *

> + * Most of these bits overlap with corresponding bits in MSR,

> + * but some come from other sources.  Be cautious when modifying.


Yeah.. I'm not sure "be cautious" is enough of a warning.  The exact
value of some but not all of these flags must equal that for the
corresponding MSR bits, which is terrifyingly subtle.

> + */

> +enum {

> +    HFLAGS_LE = 0,   /* MSR_LE -- comes from elsewhere on 601 */

> +    HFLAGS_HV = 1,   /* computed from MSR_HV and other state */

> +    HFLAGS_64 = 2,   /* computed from MSR_CE and MSR_SF */

> +    HFLAGS_PR = 3,   /* MSR_PR */

> +    HFLAGS_DR = 4,   /* MSR_DR */

> +    HFLAGS_IR = 5,   /* MSR_IR */

> +    HFLAGS_SPE = 6,  /* from MSR_SPE if cpu has SPE; avoid overlap w/ MSR_VR */

> +    HFLAGS_VSX = 7,  /* from MSR_VSX if cpu has VSX; avoid overlap w/ MSR_AP */

> +    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_FP = 13,  /* MSR_FP */

> +    HFLAGS_SA = 22,  /* MSR_SA */

> +    HFLAGS_AP = 23,  /* MSR_AP */

> +    HFLAGS_VR = 25,  /* MSR_VR if cpu has VRE */

> +};

> +

>  /*****************************************************************************/

>  /* Floating point status and control register                                */

>  #define FPSCR_DRN2   34 /* Decimal Floating-Point rounding control           */

> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c

> index a87e354ca2..0a746bffd7 100644

> --- a/target/ppc/helper_regs.c

> +++ b/target/ppc/helper_regs.c

> @@ -18,6 +18,7 @@

>   */

>  

>  #include "qemu/osdep.h"

> +#include "cpu.h"

>  #include "qemu/main-loop.h"

>  #include "exec/exec-all.h"

>  #include "sysemu/kvm.h"

> @@ -87,24 +88,56 @@ void hreg_compute_mem_idx(CPUPPCState *env)

>  

>  void hreg_compute_hflags(CPUPPCState *env)

>  {

> -    target_ulong hflags_mask;

> +    target_ulong msr = env->msr;

> +    uint32_t ppc_flags = env->flags;

> +    uint32_t hflags = 0;

> +    uint32_t msr_mask;

>  

> -    /* We 'forget' FE0 & FE1: we'll never generate imprecise exceptions */

> -    hflags_mask = (1 << MSR_VR) | (1 << MSR_AP) | (1 << MSR_SA) |

> -        (1 << MSR_PR) | (1 << MSR_FP) | (1 << MSR_SE) | (1 << MSR_BE) |

> -        (1 << MSR_LE) | (1 << MSR_VSX) | (1 << MSR_IR) | (1 << MSR_DR);

> -    hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;

> -    hreg_compute_mem_idx(env);

> -    env->hflags = env->msr & hflags_mask;

> +    /* Some bits come straight across from MSR. */

> +    msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |

> +                (1 << MSR_DR) | (1 << MSR_IR) |

> +                (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));

>  

> -    if (env->flags & POWERPC_FLAG_HID0_LE) {

> +    if (ppc_flags & POWERPC_FLAG_HID0_LE) {

>          /*

>           * Note that MSR_LE is not set in env->msr_mask for this cpu,

> -         * and so will never be set in msr or hflags at this point.

> +         * and so will never be set in msr.

>           */

>          uint32_t le = extract32(env->spr[SPR_HID0], 3, 1);

> -        env->hflags |= le << MSR_LE;

> +        hflags |= le << MSR_LE;

>      }

> +

> +    if (ppc_flags & POWERPC_FLAG_BE) {

> +        msr_mask |= 1 << MSR_BE;

> +    }

> +    if (ppc_flags & POWERPC_FLAG_SE) {

> +        msr_mask |= 1 << MSR_SE;

> +    }

> +

> +    if (msr_is_64bit(env, msr)) {

> +        hflags |= 1 << HFLAGS_64;

> +    }

> +    if ((ppc_flags & POWERPC_FLAG_SPE) && (msr & (1 << MSR_SPE))) {

> +        hflags |= 1 << HFLAGS_SPE;

> +    }

> +    if (ppc_flags & POWERPC_FLAG_VRE) {

> +        msr_mask |= 1 << MSR_VR;

> +    }

> +    if ((ppc_flags & POWERPC_FLAG_VSX) && (msr & (1 << MSR_VSX))) {

> +        hflags |= 1 << HFLAGS_VSX;

> +    }

> +    if ((ppc_flags & POWERPC_FLAG_TM) && (msr & (1ull << MSR_TM))) {

> +        hflags |= 1 << HFLAGS_TM;

> +    }

> +

> +#ifndef CONFIG_USER_ONLY

> +    if (!env->has_hv_mode || (msr & (1ull << MSR_HV))) {

> +        hflags |= 1 << HFLAGS_HV;

> +    }

> +#endif

> +

> +    env->hflags = hflags | (msr & msr_mask);


I feel like it would be better to explicitly translate *all* the bits,
so that the hflags bits numbers really are disentangled from the MSR
bit numbers.  Or if we really must do this, then have some build time
asserts to ensure that (MSR_LE == HFLAGS_LE) and so forth.

> +    hreg_compute_mem_idx(env);

>  }

>  

>  void cpu_interrupt_exittb(CPUState *cs)

> diff --git a/target/ppc/translate.c b/target/ppc/translate.c

> index 0984ce637b..a9325a12e5 100644

> --- a/target/ppc/translate.c

> +++ b/target/ppc/translate.c

> @@ -7879,67 +7879,48 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)

>  {

>      DisasContext *ctx = container_of(dcbase, DisasContext, base);

>      CPUPPCState *env = cs->env_ptr;

> +    uint32_t hflags = ctx->base.tb->flags;

>      int bound;

>  

>      ctx->exception = POWERPC_EXCP_NONE;

>      ctx->spr_cb = env->spr_cb;

> -    ctx->pr = msr_pr;

> +    ctx->pr = (hflags >> HFLAGS_PR) & 1;

>      ctx->mem_idx = env->dmmu_idx;

> -    ctx->dr = msr_dr;

> -#if !defined(CONFIG_USER_ONLY)

> -    ctx->hv = msr_hv || !env->has_hv_mode;

> -#endif

> +    ctx->dr = (hflags >> HFLAGS_DR) & 1;

> +    ctx->hv = (hflags >> HFLAGS_HV) & 1;

>      ctx->insns_flags = env->insns_flags;

>      ctx->insns_flags2 = env->insns_flags2;

>      ctx->access_type = -1;

>      ctx->need_access_type = !mmu_is_64bit(env->mmu_model);

> -    ctx->le_mode = !!(env->hflags & (1 << MSR_LE));

> +    ctx->le_mode = (hflags >> HFLAGS_LE) & 1;

>      ctx->default_tcg_memop_mask = ctx->le_mode ? MO_LE : MO_BE;

>      ctx->flags = env->flags;

>  #if defined(TARGET_PPC64)

> -    ctx->sf_mode = msr_is_64bit(env, env->msr);

> +    ctx->sf_mode = (hflags >> HFLAGS_64) & 1;

>      ctx->has_cfar = !!(env->flags & POWERPC_FLAG_CFAR);

>  #endif

>      ctx->lazy_tlb_flush = env->mmu_model == POWERPC_MMU_32B

>          || env->mmu_model == POWERPC_MMU_601

>          || env->mmu_model & POWERPC_MMU_64;

>  

> -    ctx->fpu_enabled = !!msr_fp;

> -    if ((env->flags & POWERPC_FLAG_SPE) && msr_spe) {

> -        ctx->spe_enabled = !!msr_spe;

> -    } else {

> -        ctx->spe_enabled = false;

> -    }

> -    if ((env->flags & POWERPC_FLAG_VRE) && msr_vr) {

> -        ctx->altivec_enabled = !!msr_vr;

> -    } else {

> -        ctx->altivec_enabled = false;

> -    }

> -    if ((env->flags & POWERPC_FLAG_VSX) && msr_vsx) {

> -        ctx->vsx_enabled = !!msr_vsx;

> -    } else {

> -        ctx->vsx_enabled = false;

> -    }

> +    ctx->fpu_enabled = (hflags >> HFLAGS_FP) & 1;

> +    ctx->spe_enabled = (hflags >> HFLAGS_SPE) & 1;

> +    ctx->altivec_enabled = (hflags >> HFLAGS_VR) & 1;

> +    ctx->vsx_enabled = (hflags >> HFLAGS_VSX) & 1;

>      if ((env->flags & POWERPC_FLAG_SCV)

>          && (env->spr[SPR_FSCR] & (1ull << FSCR_SCV))) {

>          ctx->scv_enabled = true;

>      } else {

>          ctx->scv_enabled = false;

>      }

> -#if defined(TARGET_PPC64)

> -    if ((env->flags & POWERPC_FLAG_TM) && msr_tm) {

> -        ctx->tm_enabled = !!msr_tm;

> -    } else {

> -        ctx->tm_enabled = false;

> -    }

> -#endif

> +    ctx->tm_enabled = (hflags >> HFLAGS_TM) & 1;

>      ctx->gtse = !!(env->spr[SPR_LPCR] & LPCR_GTSE);

> -    if ((env->flags & POWERPC_FLAG_SE) && msr_se) {

> -        ctx->singlestep_enabled = CPU_SINGLE_STEP;

> -    } else {

> -        ctx->singlestep_enabled = 0;

> +

> +    ctx->singlestep_enabled = 0;

> +    if ((hflags >> HFLAGS_SE) & 1) {

> +        ctx->singlestep_enabled |= CPU_SINGLE_STEP;

>      }

> -    if ((env->flags & POWERPC_FLAG_BE) && msr_be) {

> +    if ((hflags >> HFLAGS_BE) & 1) {

>          ctx->singlestep_enabled |= CPU_BRANCH_STEP;

>      }

>      if ((env->flags & POWERPC_FLAG_DE) && msr_de) {

> @@ -7956,10 +7937,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)

>      if (unlikely(ctx->base.singlestep_enabled)) {

>          ctx->singlestep_enabled |= GDBSTUB_SINGLE_STEP;

>      }

> -#if defined(DO_SINGLE_STEP) && 0

> -    /* Single step trace mode */

> -    msr_se = 1;

> -#endif

>  

>      bound = -(ctx->base.pc_first | TARGET_PAGE_MASK) / 4;

>      ctx->base.max_insns = MIN(ctx->base.max_insns, bound);


-- 
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
Richard Henderson March 22, 2021, 4:55 p.m. UTC | #2
On 3/21/21 9:52 PM, David Gibson wrote:
>> +/*

>> + * Bits for env->hflags.

>> + *

>> + * Most of these bits overlap with corresponding bits in MSR,

>> + * but some come from other sources.  Be cautious when modifying.

> 

> Yeah.. I'm not sure "be cautious" is enough of a warning.  The exact

> value of some but not all of these flags must equal that for the

> corresponding MSR bits, which is terrifyingly subtle.


Fair.  How about, for the comment here, "This is validated in hreg_compute_hflags."

>> +    /* Some bits come straight across from MSR. */

>> +    msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |

>> +                (1 << MSR_DR) | (1 << MSR_IR) |

>> +                (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));


Here, and in every other spot within this function where we manipulate msr_mask,

     QEMU_BUILD_BUG_ON(MSR_LE != HFLAGS_LE);

etc.


r~
David Gibson March 22, 2021, 11:54 p.m. UTC | #3
On Mon, Mar 22, 2021 at 10:55:46AM -0600, Richard Henderson wrote:
> On 3/21/21 9:52 PM, David Gibson wrote:

> > > +/*

> > > + * Bits for env->hflags.

> > > + *

> > > + * Most of these bits overlap with corresponding bits in MSR,

> > > + * but some come from other sources.  Be cautious when modifying.

> > 

> > Yeah.. I'm not sure "be cautious" is enough of a warning.  The exact

> > value of some but not all of these flags must equal that for the

> > corresponding MSR bits, which is terrifyingly subtle.

> 

> Fair.  How about, for the comment here, "This is validated in hreg_compute_hflags."

> 

> > > +    /* Some bits come straight across from MSR. */

> > > +    msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |

> > > +                (1 << MSR_DR) | (1 << MSR_IR) |

> > > +                (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));

> 

> Here, and in every other spot within this function where we manipulate msr_mask,

> 

>     QEMU_BUILD_BUG_ON(MSR_LE != HFLAGS_LE);


Seems reasonable.

-- 
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
Richard Henderson March 23, 2021, 5:04 p.m. UTC | #4
On 3/22/21 5:54 PM, David Gibson wrote:
> On Mon, Mar 22, 2021 at 10:55:46AM -0600, Richard Henderson wrote:

>> On 3/21/21 9:52 PM, David Gibson wrote:

>>>> +/*

>>>> + * Bits for env->hflags.

>>>> + *

>>>> + * Most of these bits overlap with corresponding bits in MSR,

>>>> + * but some come from other sources.  Be cautious when modifying.

>>>

>>> Yeah.. I'm not sure "be cautious" is enough of a warning.  The exact

>>> value of some but not all of these flags must equal that for the

>>> corresponding MSR bits, which is terrifyingly subtle.

>>

>> Fair.  How about, for the comment here, "This is validated in hreg_compute_hflags."

>>

>>>> +    /* Some bits come straight across from MSR. */

>>>> +    msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |

>>>> +                (1 << MSR_DR) | (1 << MSR_IR) |

>>>> +                (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));

>>

>> Here, and in every other spot within this function where we manipulate msr_mask,

>>

>>      QEMU_BUILD_BUG_ON(MSR_LE != HFLAGS_LE);

> 

> Seems reasonable.

> 


Hah.  Your paranoia was well-founded.  Typo in HFLAGS_PR.  :-)


r~
David Gibson March 24, 2021, 1:42 a.m. UTC | #5
On Tue, Mar 23, 2021 at 11:04:03AM -0600, Richard Henderson wrote:
> On 3/22/21 5:54 PM, David Gibson wrote:

> > On Mon, Mar 22, 2021 at 10:55:46AM -0600, Richard Henderson wrote:

> > > On 3/21/21 9:52 PM, David Gibson wrote:

> > > > > +/*

> > > > > + * Bits for env->hflags.

> > > > > + *

> > > > > + * Most of these bits overlap with corresponding bits in MSR,

> > > > > + * but some come from other sources.  Be cautious when modifying.

> > > > 

> > > > Yeah.. I'm not sure "be cautious" is enough of a warning.  The exact

> > > > value of some but not all of these flags must equal that for the

> > > > corresponding MSR bits, which is terrifyingly subtle.

> > > 

> > > Fair.  How about, for the comment here, "This is validated in hreg_compute_hflags."

> > > 

> > > > > +    /* Some bits come straight across from MSR. */

> > > > > +    msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |

> > > > > +                (1 << MSR_DR) | (1 << MSR_IR) |

> > > > > +                (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));

> > > 

> > > Here, and in every other spot within this function where we manipulate msr_mask,

> > > 

> > >      QEMU_BUILD_BUG_ON(MSR_LE != HFLAGS_LE);

> > 

> > Seems reasonable.

> 

> Hah.  Your paranoia was well-founded.  Typo in HFLAGS_PR.  :-)


Heh :).

-- 
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 mbox series

Patch

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index fd13489dce..39f35b570c 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -585,6 +585,30 @@  enum {
     POWERPC_FLAG_HID0_LE  = 0x00400000,
 };
 
+/*
+ * Bits for env->hflags.
+ *
+ * Most of these bits overlap with corresponding bits in MSR,
+ * but some come from other sources.  Be cautious when modifying.
+ */
+enum {
+    HFLAGS_LE = 0,   /* MSR_LE -- comes from elsewhere on 601 */
+    HFLAGS_HV = 1,   /* computed from MSR_HV and other state */
+    HFLAGS_64 = 2,   /* computed from MSR_CE and MSR_SF */
+    HFLAGS_PR = 3,   /* MSR_PR */
+    HFLAGS_DR = 4,   /* MSR_DR */
+    HFLAGS_IR = 5,   /* MSR_IR */
+    HFLAGS_SPE = 6,  /* from MSR_SPE if cpu has SPE; avoid overlap w/ MSR_VR */
+    HFLAGS_VSX = 7,  /* from MSR_VSX if cpu has VSX; avoid overlap w/ MSR_AP */
+    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_FP = 13,  /* MSR_FP */
+    HFLAGS_SA = 22,  /* MSR_SA */
+    HFLAGS_AP = 23,  /* MSR_AP */
+    HFLAGS_VR = 25,  /* MSR_VR if cpu has VRE */
+};
+
 /*****************************************************************************/
 /* Floating point status and control register                                */
 #define FPSCR_DRN2   34 /* Decimal Floating-Point rounding control           */
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index a87e354ca2..0a746bffd7 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -18,6 +18,7 @@ 
  */
 
 #include "qemu/osdep.h"
+#include "cpu.h"
 #include "qemu/main-loop.h"
 #include "exec/exec-all.h"
 #include "sysemu/kvm.h"
@@ -87,24 +88,56 @@  void hreg_compute_mem_idx(CPUPPCState *env)
 
 void hreg_compute_hflags(CPUPPCState *env)
 {
-    target_ulong hflags_mask;
+    target_ulong msr = env->msr;
+    uint32_t ppc_flags = env->flags;
+    uint32_t hflags = 0;
+    uint32_t msr_mask;
 
-    /* We 'forget' FE0 & FE1: we'll never generate imprecise exceptions */
-    hflags_mask = (1 << MSR_VR) | (1 << MSR_AP) | (1 << MSR_SA) |
-        (1 << MSR_PR) | (1 << MSR_FP) | (1 << MSR_SE) | (1 << MSR_BE) |
-        (1 << MSR_LE) | (1 << MSR_VSX) | (1 << MSR_IR) | (1 << MSR_DR);
-    hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
-    hreg_compute_mem_idx(env);
-    env->hflags = env->msr & hflags_mask;
+    /* Some bits come straight across from MSR. */
+    msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |
+                (1 << MSR_DR) | (1 << MSR_IR) |
+                (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));
 
-    if (env->flags & POWERPC_FLAG_HID0_LE) {
+    if (ppc_flags & POWERPC_FLAG_HID0_LE) {
         /*
          * Note that MSR_LE is not set in env->msr_mask for this cpu,
-         * and so will never be set in msr or hflags at this point.
+         * and so will never be set in msr.
          */
         uint32_t le = extract32(env->spr[SPR_HID0], 3, 1);
-        env->hflags |= le << MSR_LE;
+        hflags |= le << MSR_LE;
     }
+
+    if (ppc_flags & POWERPC_FLAG_BE) {
+        msr_mask |= 1 << MSR_BE;
+    }
+    if (ppc_flags & POWERPC_FLAG_SE) {
+        msr_mask |= 1 << MSR_SE;
+    }
+
+    if (msr_is_64bit(env, msr)) {
+        hflags |= 1 << HFLAGS_64;
+    }
+    if ((ppc_flags & POWERPC_FLAG_SPE) && (msr & (1 << MSR_SPE))) {
+        hflags |= 1 << HFLAGS_SPE;
+    }
+    if (ppc_flags & POWERPC_FLAG_VRE) {
+        msr_mask |= 1 << MSR_VR;
+    }
+    if ((ppc_flags & POWERPC_FLAG_VSX) && (msr & (1 << MSR_VSX))) {
+        hflags |= 1 << HFLAGS_VSX;
+    }
+    if ((ppc_flags & POWERPC_FLAG_TM) && (msr & (1ull << MSR_TM))) {
+        hflags |= 1 << HFLAGS_TM;
+    }
+
+#ifndef CONFIG_USER_ONLY
+    if (!env->has_hv_mode || (msr & (1ull << MSR_HV))) {
+        hflags |= 1 << HFLAGS_HV;
+    }
+#endif
+
+    env->hflags = hflags | (msr & msr_mask);
+    hreg_compute_mem_idx(env);
 }
 
 void cpu_interrupt_exittb(CPUState *cs)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 0984ce637b..a9325a12e5 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7879,67 +7879,48 @@  static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     CPUPPCState *env = cs->env_ptr;
+    uint32_t hflags = ctx->base.tb->flags;
     int bound;
 
     ctx->exception = POWERPC_EXCP_NONE;
     ctx->spr_cb = env->spr_cb;
-    ctx->pr = msr_pr;
+    ctx->pr = (hflags >> HFLAGS_PR) & 1;
     ctx->mem_idx = env->dmmu_idx;
-    ctx->dr = msr_dr;
-#if !defined(CONFIG_USER_ONLY)
-    ctx->hv = msr_hv || !env->has_hv_mode;
-#endif
+    ctx->dr = (hflags >> HFLAGS_DR) & 1;
+    ctx->hv = (hflags >> HFLAGS_HV) & 1;
     ctx->insns_flags = env->insns_flags;
     ctx->insns_flags2 = env->insns_flags2;
     ctx->access_type = -1;
     ctx->need_access_type = !mmu_is_64bit(env->mmu_model);
-    ctx->le_mode = !!(env->hflags & (1 << MSR_LE));
+    ctx->le_mode = (hflags >> HFLAGS_LE) & 1;
     ctx->default_tcg_memop_mask = ctx->le_mode ? MO_LE : MO_BE;
     ctx->flags = env->flags;
 #if defined(TARGET_PPC64)
-    ctx->sf_mode = msr_is_64bit(env, env->msr);
+    ctx->sf_mode = (hflags >> HFLAGS_64) & 1;
     ctx->has_cfar = !!(env->flags & POWERPC_FLAG_CFAR);
 #endif
     ctx->lazy_tlb_flush = env->mmu_model == POWERPC_MMU_32B
         || env->mmu_model == POWERPC_MMU_601
         || env->mmu_model & POWERPC_MMU_64;
 
-    ctx->fpu_enabled = !!msr_fp;
-    if ((env->flags & POWERPC_FLAG_SPE) && msr_spe) {
-        ctx->spe_enabled = !!msr_spe;
-    } else {
-        ctx->spe_enabled = false;
-    }
-    if ((env->flags & POWERPC_FLAG_VRE) && msr_vr) {
-        ctx->altivec_enabled = !!msr_vr;
-    } else {
-        ctx->altivec_enabled = false;
-    }
-    if ((env->flags & POWERPC_FLAG_VSX) && msr_vsx) {
-        ctx->vsx_enabled = !!msr_vsx;
-    } else {
-        ctx->vsx_enabled = false;
-    }
+    ctx->fpu_enabled = (hflags >> HFLAGS_FP) & 1;
+    ctx->spe_enabled = (hflags >> HFLAGS_SPE) & 1;
+    ctx->altivec_enabled = (hflags >> HFLAGS_VR) & 1;
+    ctx->vsx_enabled = (hflags >> HFLAGS_VSX) & 1;
     if ((env->flags & POWERPC_FLAG_SCV)
         && (env->spr[SPR_FSCR] & (1ull << FSCR_SCV))) {
         ctx->scv_enabled = true;
     } else {
         ctx->scv_enabled = false;
     }
-#if defined(TARGET_PPC64)
-    if ((env->flags & POWERPC_FLAG_TM) && msr_tm) {
-        ctx->tm_enabled = !!msr_tm;
-    } else {
-        ctx->tm_enabled = false;
-    }
-#endif
+    ctx->tm_enabled = (hflags >> HFLAGS_TM) & 1;
     ctx->gtse = !!(env->spr[SPR_LPCR] & LPCR_GTSE);
-    if ((env->flags & POWERPC_FLAG_SE) && msr_se) {
-        ctx->singlestep_enabled = CPU_SINGLE_STEP;
-    } else {
-        ctx->singlestep_enabled = 0;
+
+    ctx->singlestep_enabled = 0;
+    if ((hflags >> HFLAGS_SE) & 1) {
+        ctx->singlestep_enabled |= CPU_SINGLE_STEP;
     }
-    if ((env->flags & POWERPC_FLAG_BE) && msr_be) {
+    if ((hflags >> HFLAGS_BE) & 1) {
         ctx->singlestep_enabled |= CPU_BRANCH_STEP;
     }
     if ((env->flags & POWERPC_FLAG_DE) && msr_de) {
@@ -7956,10 +7937,6 @@  static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     if (unlikely(ctx->base.singlestep_enabled)) {
         ctx->singlestep_enabled |= GDBSTUB_SINGLE_STEP;
     }
-#if defined(DO_SINGLE_STEP) && 0
-    /* Single step trace mode */
-    msr_se = 1;
-#endif
 
     bound = -(ctx->base.pc_first | TARGET_PAGE_MASK) / 4;
     ctx->base.max_insns = MIN(ctx->base.max_insns, bound);