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

[Alexander Graf]

On 23.03.2012, at 14:29, Mark Cave-Ayland wrote:

> On 22/03/12 20:32, Scott Wood wrote:
> 
> Hi Scott,
> 
>> As I think Alex commented recently, we really should be picking the bits
>> we want to keep rather than the ones we want to exclude.  Please keep in
>> mind that this code is used by booke as well.  E.g. on booke exceptions
>> don't normally clear MSR[DE], but it's in your mask of bits to clear.
>> The mask should depend on both the exception model and the specific
>> exception type.
> 
> Well this is the part that is currently confusing me; I know Alex mentioned 
> about picking the bits to keep, however that appears to contradict the 
> reference specification link he pointed me towards at 
> https://www.power.org/resources/downloads/PowerISA_V2.06B_V2_PUBLIC.pdf, page 
> 811. This states "The MSR is set as shown" referencing the table on page 814 
> showing the status of only 8 of the MSR bits. Therefore from this I would 
> naturally conclude that the value of any other MSR bits should be preserved, 
> not zeroed, as per Alex's comment and the existing implementation.

It also explicitly says that reserved bits are set to 0. And all the bits not 
mentioned there are declared as reserved in the MSR definition (page 736) :).

> I've now just realised that the above document also contains the BookE 
> interrupt processing specification, and I can see some of the differences you 
> mention; once we decide what the behaviour should be, I'm happy to rework the 
> patch with an additional switch() for the exception model.
> 
>> It would also be nice to use symbolic names for the MSR bits, rather
>> than magic hex values (even commented ones).
> 
> Yes - that's no problem if everyone prefers that method.
> 
>> BTW, The existing MSR[RI] handling also looks wrong -- MSR[RI] should be
>> preserved rather than set to 1 if it is an exception that does not clear
>> MSR[RI].
> 
> The table on p.814 clearly shows that RI bit should be preserved for a small 
> number of interrupts, and that definitely doesn't agree with the code. One of 
> Alex's changes in commit 41557... was to force the RI bit to zero by default, 
> rather than manually setting it in a majority of codepaths. See 
> http://git.qemu.org/?p=qemu.git;a=commitdiff;h=41557447d30eeb944e42069513df13585f5e6c7f
>  for more details.

Yes, the only cases according to 2.06 where RI is preserved is during HV 
interrupts. Since HV emulation is completely broken atm anyways, I figured we 
can safely ignore that case.


Alex