Re: [PATCH v5 10/10] target/ppc: Validate hflags with CONFIG_DEBUG_TCG

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3597 bytes --]

On Wed, Mar 24, 2021 at 11:12:20AM +1100, David Gibson wrote:
> On Tue, Mar 23, 2021 at 12:43:40PM -0600, Richard Henderson wrote:
> > Verify that hflags was updated correctly whenever we change
> > cpu state that is used by hflags.
> > 
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> 
> Applied to ppc-for-6.0, thanks.

Alas, this appears to cause a failure in the 'build-user' test on
gitlab CI :(.  I haven't managed to reproduce it locally, so I'm not
sure what's going on.

> 
> > ---
> >  target/ppc/cpu.h         |  5 +++++
> >  target/ppc/helper_regs.c | 29 +++++++++++++++++++++++++++--
> >  2 files changed, 32 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > index 3d021f61f3..69fc9a2831 100644
> > --- a/target/ppc/cpu.h
> > +++ b/target/ppc/cpu.h
> > @@ -2425,6 +2425,10 @@ void cpu_write_xer(CPUPPCState *env, target_ulong xer);
> >   */
> >  #define is_book3s_arch2x(ctx) (!!((ctx)->insns_flags & PPC_SEGMENT_64B))
> >  
> > +#ifdef CONFIG_DEBUG_TCG
> > +void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc,
> > +                          target_ulong *cs_base, uint32_t *flags);
> > +#else
> >  static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc,
> >                                          target_ulong *cs_base, uint32_t *flags)
> >  {
> > @@ -2432,6 +2436,7 @@ static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc,
> >      *cs_base = 0;
> >      *flags = env->hflags;
> >  }
> > +#endif
> >  
> >  void QEMU_NORETURN raise_exception(CPUPPCState *env, uint32_t exception);
> >  void QEMU_NORETURN raise_exception_ra(CPUPPCState *env, uint32_t exception,
> > diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> > index 5411a67e9a..3723872aa6 100644
> > --- a/target/ppc/helper_regs.c
> > +++ b/target/ppc/helper_regs.c
> > @@ -43,7 +43,7 @@ void hreg_swap_gpr_tgpr(CPUPPCState *env)
> >      env->tgpr[3] = tmp;
> >  }
> >  
> > -void hreg_compute_hflags(CPUPPCState *env)
> > +static uint32_t hreg_compute_hflags_value(CPUPPCState *env)
> >  {
> >      target_ulong msr = env->msr;
> >      uint32_t ppc_flags = env->flags;
> > @@ -155,9 +155,34 @@ void hreg_compute_hflags(CPUPPCState *env)
> >      hflags |= dmmu_idx << HFLAGS_DMMU_IDX;
> >  #endif
> >  
> > -    env->hflags = hflags | (msr & msr_mask);
> > +    return hflags | (msr & msr_mask);
> >  }
> >  
> > +void hreg_compute_hflags(CPUPPCState *env)
> > +{
> > +    env->hflags = hreg_compute_hflags_value(env);
> > +}
> > +
> > +#ifdef CONFIG_DEBUG_TCG
> > +void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc,
> > +                          target_ulong *cs_base, uint32_t *flags)
> > +{
> > +    uint32_t hflags_current = env->hflags;
> > +    uint32_t hflags_rebuilt;
> > +
> > +    *pc = env->nip;
> > +    *cs_base = 0;
> > +    *flags = hflags_current;
> > +
> > +    hflags_rebuilt = hreg_compute_hflags_value(env);
> > +    if (unlikely(hflags_current != hflags_rebuilt)) {
> > +        cpu_abort(env_cpu(env),
> > +                  "TCG hflags mismatch (current:0x%08x rebuilt:0x%08x)\n",
> > +                  hflags_current, hflags_rebuilt);
> > +    }
> > +}
> > +#endif
> > +
> >  void cpu_interrupt_exittb(CPUState *cs)
> >  {
> >      if (!kvm_enabled()) {
> 



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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]