[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] target-ppc: fix RFI by clearing some bits of MS
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH] target-ppc: fix RFI by clearing some bits of MSR |
Date: |
Mon, 31 May 2010 20:45:34 +0200 |
User-agent: |
Mutt/1.5.20 (2009-06-14) |
On Fri, May 28, 2010 at 09:07:32PM +0200, Thomas Monjalon wrote:
> From: Thomas Monjalon <address@hidden>
>
> Since commit 2ada0ed, "Return From Interrupt" is broken for PPC processors
> because some interrupt specifics bits of SRR1 are copied to MSR.
>
> SRR1 is a save of MSR during interrupt.
> During RFI, MSR must be restored from SRR1.
> But some bits of SRR1 are interrupt-specific and are not used for MSR saving.
>
> This is the specification (ISA 2.06) at chapter 6.4.3 (Interrupt Processing):
> "2. Bits 33:36 and 42:47 of SRR1 or HSRR1 are loaded with information specific
> to the interrupt type.
> 3. Bits 0:32, 37:41, and 48:63 of SRR1 or HSRR1 are loaded with a copy of the
> corresponding bits of the MSR."
>
> Below is a representation of MSR bits which are not saved:
> 0:15 16:31 32 33:36 37:41 42:47 48:63
> ——— | ——— | — X X X X — — — — — X X X X X X | ————
> 0000 0000 | 7 | 8 | 3 | F | 0000
>
> History:
> In the initial Qemu implementation (e1833e1), the mask 0x783F0000 was used for
> saving MSR in SRR1. But all the bits 32:47 were cleared during RFI restoring.
> This was wrong. The commit 2ada0ed explains that this breaks Altivec.
> Indeed, bit 38 (for Altivec support) must be saved and restored.
> The change of 2ada0ed was to restore all the bits of SRR1 to MSR.
> But it's also wrong.
>
> Explanation:
> As an example, let's see what's happening after a TLB miss.
> According to the e300 manual (E300CORERM table 5-6), the TLB miss interrupts
> set the bits 44-47 for KEY, I/D, WAY and S/L. These bits are specifics to the
> interrupt and must not be copied into MSR at the end of the interrupt.
> With the current implementation, a TLB miss overwrite bits POW, TGPR and ILE.
>
> Fix:
> It shouldn't be needed to filter-out bits on MSR saving when interrupt occurs.
> Specific bits overwrite MSR ones in SRR1.
> But at the end of interrupt (RFI), specifics bits must be cleared before
> restoring MSR from SRR1. The mask 0x783F0000 apply here.
>
> Discussion:
> The bits of the mask 0x783F0000 are cleared after an interrupt.
> I cannot find a specification which talks about this
> but I assume it is the truth since Linux can run this way.
> Maybe it's not perfect but it's better (works for e300).
>
> Signed-off-by: Thomas Monjalon <address@hidden>
> Cc: Alexander Graf <address@hidden>
> ---
> target-ppc/helper.c | 1 -
> target-ppc/op_helper.c | 6 +++---
> 2 files changed, 3 insertions(+), 4 deletions(-)
Thanks, applied.
> diff --git a/target-ppc/helper.c b/target-ppc/helper.c
> index dabf1fd..5035d92 100644
> --- a/target-ppc/helper.c
> +++ b/target-ppc/helper.c
> @@ -2080,7 +2080,6 @@ static inline void powerpc_excp(CPUState *env, int
> excp_model, int excp)
> srr1 = SPR_SRR1;
> asrr0 = -1;
> asrr1 = -1;
> - msr &= ~((target_ulong)0x783F0000);
> switch (excp) {
> case POWERPC_EXCP_NONE:
> /* Should never happen */
> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
> index 8f2ee98..3c3aa60 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)0x783F0000), 1);
> }
>
> #if defined(TARGET_PPC64)
> void helper_rfid (void)
> {
> do_rfi(env->spr[SPR_SRR0], env->spr[SPR_SRR1],
> - ~((target_ulong)0x0), 0);
> + ~((target_ulong)0x783F0000), 0);
> }
>
> void helper_hrfid (void)
> {
> do_rfi(env->spr[SPR_HSRR0], env->spr[SPR_HSRR1],
> - ~((target_ulong)0x0), 0);
> + ~((target_ulong)0x783F0000), 0);
> }
> #endif
> #endif
> --
> 1.7.1
>
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
address@hidden http://www.aurel32.net