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

[Mark Cave-Ayland]

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.

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.


Many thanks,

Mark.