Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] PPC: Fix interrupt MSR value within the PPC interrupt handler.

[Mark Cave-Ayland]

On 28/03/12 01:46, David Gibson wrote:

Hi David,

> > If we're going to make this specific to MSRs, might as well cut down on
> > the user's verbosity:
> >
> > #define MSR_BIT(x) ((target_ulong)1<<  MSR_##x)
> >
> > ...and move it to a header file.
> >
> > Or possibly have the header file define a set of MSRBIT_IR, MSRBIT_DR, etc.

I think I prefer your macro above and move it to a relevant part of 
target-ppc/cpu.h with the other MSR defines.

> > >   static inline void powerpc_excp(CPUPPCState *env, int excp_model, int excp)
> > >   {
> > >       target_ulong msr, new_msr, vector;
> > > @@ -2478,11 +2480,26 @@ static inline void powerpc_excp(CPUPPCState *env, int 
> > > excp_model, int excp)
> > >       qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
> > >                     " =>  %08x (%02x)\n", env->nip, excp, env->error_code);
> > >
> > > -    /* new srr1 value excluding must-be-zero bits */
> > > +    /* new srr1 value with interrupt-specific bits defaulting to zero */
> > >       msr = env->msr&  ~0x783f0000ULL;
> > >
> > > -    /* new interrupt handler msr */
> > > -    new_msr = env->msr&  ((target_ulong)1<<  MSR_ME);
> > > +    switch (excp_model) {
> > > +    case POWERPC_EXCP_BOOKE:
> > > +        /* new interrupt handler msr */
> > > +        new_msr = env->msr&  ((target_ulong)1<<  MSR_ME);
> > > +        break;
> > > +
> > > +    default:
> > > +        /* new interrupt handler msr (as per PowerISA 2.06B p.811 and p.814):
> > > +           1) force the following bits to zero
> > > +              IR, DR, FE0, FE1, EE, BE, FP, PMM, PR, SE
> > > +           2) default the following bits to zero (can be overidden later on)
> > > +              RI */
> > > +        new_msr = env->msr&  ~(MSR_BIT(MSR_IR) | MSR_BIT(MSR_DR)
> > > +                      | MSR_BIT(MSR_FE0)| MSR_BIT(MSR_FE1) | MSR_BIT(MSR_EE)
> > > +                      | MSR_BIT(MSR_BE) | MSR_BIT(MSR_FP) | MSR_BIT(MSR_PMM)
> > > +                      | MSR_BIT(MSR_PR) | MSR_BIT(MSR_SE) | MSR_BIT(MSR_RI));
> > > +    }
> >
> > What about POWERPC_EXCP_40x?  And are all the classic chips OK with the
> > 2.06B implementation?
>
> Hrm, yeah.  I think what you ought to do is to use the new logic just
> for the "classic" exception models.  Have the default branch remain
> the one that just masks ME.  That's wrong, but it's the same wrong as
> we have already, and we can fix it later once we've verified what the
> right thing to do is for 40x and BookE.

I'm actually coming at this from a fixing what was potentially an 
OpenBIOS bug rather than a PPC angle, so I have to admit I have no I 
idea which ones are the "classic" exception models. Would you consider 
this to be just EXCP_STD, EXCP_6* and EXCP_7*?


Many thanks,

Mark.