Re: What's the correct way to implement rfi and related instruction.

[Yonggang Luo]

On Tue, Jan 12, 2021 at 1:23 AM Cédric Le Goater <clg@kaod.org> wrote:
>
> > QEMU 5.2.x, an e300 based machine ppc603 are impacted.
> > Here is my fix, narrowed down to  MSR_TGPR and  MSR_ILE
> > ```
> > From 42ce41671f1e6c4dd44e6fb481bbda9df09320bd Mon Sep 17 00:00:00 2001
> > From: Yonggang Luo <luoyonggang@gmail.com <mailto:luoyonggang@gmail.com>>
> > Date: Sun, 10 Jan 2021 00:08:00 -0800
> > Subject: [PATCH] ppc: Fix rfi/rfid/hrfi/... emulation again
> >
> > This revert part mask bits for ppc603/ppc4x that disabled in  a2e71b28e832346409efc795ecd1f0a2bcb705a3.
> > Remove redundant macro MSR_BOOK3S_MASK.
> > Fixes boot VxWorks on e300
> >
> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com <mailto:luoyonggang@gmail.com>>
> > ---
> >  target/ppc/excp_helper.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > index 1c48b9fdf6..df70c5a4e8 100644
> > --- a/target/ppc/excp_helper.c
> > +++ b/target/ppc/excp_helper.c
> > @@ -1156,8 +1156,10 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
> >  {
> >      CPUState *cs = env_cpu(env);
> >  
> > -    /* MSR:POW cannot be set by any form of rfi */
> > +    /* MSR:POW,TGPR,ILE cannot be set by any form of rfi */
> >      msr &= ~(1ULL << MSR_POW);
> > +    msr &= ~(1ULL << MSR_TGPR);
>
> Indeed. The e300 user manual says that TGPR is cleared by rfi. We should
> add a per-cpu family mask and not a global setting.
>
> > +    msr &= ~(1ULL << MSR_ILE);
>
> that's curious. I am still trying to understand that part. May be this is
> due to the lack of HID2 modeling which contains a "True little-endian" bit.
>
> Is your image Little endian ?

Hi, According to ` Table 2-4. MSR Bit Settings (continued) ` in https://www.nxp.com/docs/en/reference-manual/e300coreRM.pdf

```
  Interrupt little-endian mode. When an interrupt occurs, this bit is copied into MSR[LE] to select the endian mode for the context established by the interrupt.  

```

Does this means MSR[LE] =  MSR[LE]

>
> C.
>
> >  
> >  #if defined(TARGET_PPC64)
> >      /* Switching to 32-bit ? Crop the nip */
> > @@ -1190,7 +1192,6 @@ void helper_rfi(CPUPPCState *env)
> >      do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xfffffffful);
> >  }
> >  
> > -#define MSR_BOOK3S_MASK
> >  #if defined(TARGET_PPC64)
> >  void helper_rfid(CPUPPCState *env)
> >  {
> > --
> > 2.29.2.windows.3
> >
> > ```
> >
> > --
> >          此致
> > 礼
> > 罗勇刚
> > Yours
> >     sincerely,
> > Yonggang Luo
>


--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo