Re: [Qemu-devel] [Qemu-ppc] [PATCH 09/77] ppc: Fix do_rfi() for rfi emulation

[Benjamin Herrenschmidt]

On Thu, 2015-11-19 at 17:19 +1100, David Gibson wrote:
> On Wed, Nov 11, 2015 at 11:27:22AM +1100, Benjamin Herrenschmidt
> wrote:
> > XXX This patch needs double checking... It fixed 32-bit userspace
> > but I'm not sure it's right. I wonder whether msr_is_64bit() should
> > be applied to env->msr, not msr, but I need to double check the
> > architecture.
> 
> Hrm, I'm not really sure where I'd look in the arch, but
> msr_is_64bit(env->msr) seems like it would make more sense to me.
> The current logic means that rfi, ostensibly a 32-bit instruction
> will
> have different behaviour depending on the upper bits of SRR1, which
> seems a unexpected.

I only just discovered that rfi is actually gone from arch 2.07 :-)

I'll dig a bit more tomorrow.

Cheers,
Ben.

> 
> > 
> > Signed-off-by: Benjamin Herrenschmidt <address@hidden>
> > ---
> >  target-ppc/excp_helper.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
> > index c1d6605..00fae60 100644
> > --- a/target-ppc/excp_helper.c
> > +++ b/target-ppc/excp_helper.c
> > @@ -878,13 +878,13 @@ static inline void do_rfi(CPUPPCState *env,
> > target_ulong nip, target_ulong msr,
> >      CPUState *cs = CPU(ppc_env_get_cpu(env));
> >  
> >  #if defined(TARGET_PPC64)
> > +    msr = msr & msrm;
> >      if (msr_is_64bit(env, msr)) {
> >          nip = (uint64_t)nip;
> > -        msr &= (uint64_t)msrm;
> >      } else {
> >          nip = (uint32_t)nip;
> > -        msr = (uint32_t)(msr & msrm);
> >          if (keep_msrh) {
> > +       msr &= 0xffffffff;
> >              msr |= env->msr & ~((uint64_t)0xFFFFFFFF);
> >          }
> >      }
>