Re: [PATCH 1/4] target/ppc: Fix instruction loading endianness in alignment interrupt

[Nicholas Piggin]

On Wed Jun 21, 2023 at 2:54 AM AEST, Nicholas Piggin wrote:
> On Wed Jun 21, 2023 at 12:26 AM AEST, BALATON Zoltan wrote:
> > On Tue, 20 Jun 2023, Nicholas Piggin wrote:
> > > powerpc ifetch endianness depends on MSR[LE] so it has to byteswap
> > > after cpu_ldl_code(). This corrects DSISR bits in alignment
> > > interrupts when running in little endian mode.
> > >
> > > Reviewed-by: Fabiano Rosas <farosas@suse.de>
> > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > > ---
> > > target/ppc/excp_helper.c | 22 +++++++++++++++++++++-
> > > 1 file changed, 21 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > > index 12d8a7257b..a2801f6e6b 100644
> > > --- a/target/ppc/excp_helper.c
> > > +++ b/target/ppc/excp_helper.c
> > > @@ -133,6 +133,26 @@ static void dump_hcall(CPUPPCState *env)
> > >                   env->nip);
> > > }
> > >
> > > +#ifdef CONFIG_TCG
> > > +/* Return true iff byteswap is needed to load instruction */
> > > +static inline bool insn_need_byteswap(CPUArchState *env)
> > > +{
> > > +    /* SYSTEM builds TARGET_BIG_ENDIAN. Need to swap when MSR[LE] is set 
> > > */
> > > +    return !!(env->msr & ((target_ulong)1 << MSR_LE));
> > > +}
> >
> > Don't other places typically use FIELD_EX64 to test for msr bits now? If 
>
> Yeah I should use that, good point. There's at least another case in
> that file that doesn't use it but I probably added that too :/

This incremental patch fixes it:

Thanks,
Nick

---
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index ff7166adf9..cfdbeb0da5 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -138,7 +138,7 @@ static void dump_hcall(CPUPPCState *env)
 static inline bool insn_need_byteswap(CPUArchState *env)
 {
     /* SYSTEM builds TARGET_BIG_ENDIAN. Need to swap when MSR[LE] is set */
-    return !!(env->msr & ((target_ulong)1 << MSR_LE));
+    return FIELD_EX64(env->msr, MSR, LE);
 }
 
 static uint32_t ppc_ldl_code(CPUArchState *env, hwaddr addr)