Re: [Qemu-devel] [PATCH v2 4/5] target-ppc: fix RFI by clearing upper bytes of MSR

[Alexander Graf]

On 27.04.2010, at 17:31, Thomas Monjalon wrote:

> From: Thomas Monjalon <address@hidden>
> 
> Since commit 2ada0ed, "Return From Interrupt" is broken for PPC processors
> because the upper bits (POW, TGPR, ILE) of MSR were not cleared.

May I ask for your test case or how you stumbled over this? I haven't seen any 
OS rely on this yet.

> 
> Below is a representation of MSR bits:
> 0 .. 12  13  14  15  16         ..         23  24        ..      31
> —————  POW TGPR ILE EE PR FP ME FE0 SE BE FE1 CE IP IR DR —— RI LE
> 
> Only the 2 lower bytes (16-31) of MSR are saved to SRR1 before an interrupt.
> So only these bytes should be restored and the upper ones (0-15) cleared.
> But, referring to commit 2ada0ed, clearing all the upper bytes breaks Altivec.
> The compromise is to clear the well known bits (13-15).

IIRC this is vastly implementation dependent. Book3 lists the bits saved when 
setting SRR1 explicitly and I haven't found a good rule of thumb yet.
RFI on the other hand is described as MSR <- SRR1. So I'm fairly sure RFI is 
implemented correctly.

Have you tried making the SRR1 setting more clever?

> 
> Regarding RFID, since the 32 lower bits of MSR are the same in 64-bit,
> the same mask as RFI should apply to RFID.
> 
> Signed-off-by: Thomas Monjalon <address@hidden>
> Cc: Blue Swirl <address@hidden>
> ---
> target-ppc/op_helper.c |    6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
> index 8f2ee98..2bf2ce1 100644
> --- a/target-ppc/op_helper.c
> +++ b/target-ppc/op_helper.c
> @@ -1646,20 +1646,20 @@ static inline void do_rfi(target_ulong nip, 
> target_ulong msr,
> void helper_rfi (void)
> {
>     do_rfi(env->spr[SPR_SRR0], env->spr[SPR_SRR1],
> -           ~((target_ulong)0x0), 1);
> +           ~((target_ulong)0x00070000), 1);

Please use constant defines here. We have MSR_XX.

Alex