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

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

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.