From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=47892 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OENLU-0002iy-Ol for qemu-devel@nongnu.org; Tue, 18 May 2010 10:00:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OENLS-0003Dd-Av for qemu-devel@nongnu.org; Tue, 18 May 2010 10:00:32 -0400 Received: from mail.corp.accelance.fr ([213.162.48.15]:61102) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OENLS-0003D7-0M for qemu-devel@nongnu.org; Tue, 18 May 2010 10:00:30 -0400 From: Thomas Monjalon Subject: Re: [Qemu-devel] [PATCH v2 4/5] target-ppc: fix RFI by clearing upper bytes of MSR Date: Tue, 18 May 2010 16:00:27 +0200 References: In-Reply-To: MIME-Version: 1.0 Content-Disposition: inline Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable Message-Id: <201005181600.27913.thomas_ml@monjalon.net> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Blue Swirl , qemu-devel@nongnu.org Alexander Graf wrote: > On 27.04.2010, at 17:31, Thomas Monjalon wrote: > > 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. I'm running Linux for SBC834x in Qemu. The interrupt controller and board=20 definition are not yet published. The boot process hang with the actual implementation of RFI. > > Below is a representation of MSR bits: > > 0 .. 12 13 14 15 16 .. 23 24 .. 31 > > =97=97=97=97=97 POW TGPR ILE EE PR FP ME FE0 SE BE FE1 CE IP IR DR =97= =97 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. =46rom the programming manual (MPCFPE32B): "The save/restore register 1 (SRR1) is used to save machine status (selecte= d=20 bits from the MSR and other implementation-specific status bits as well) on= =20 interrupts and to restore those values when rfi is executed. [..] When an interrupt occurs, SRR1 bits 1=964 and 10=9615 are loaded with=20 interrupt-specific information and MSR bits 16=9623, 25=9627, and 30=9631 a= re=20 placed into the corresponding bit positions of SRR1. Depending on the=20 implementation, additional MSR bits may be copied to SRR1." =46rom the e300 reference manual (e300CORERM): "The TGPR bit is cleared by an rfi instruction." My first try was to clear only TGPR. But it doesn't work properly if POW an= d=20 ILE are not cleared. > Have you tried making the SRR1 setting more clever? Yes. POW and TGPR can be filtered-out when saving to SRR1. But it seems tha= t=20 ILE must be cleared in RFI (if not, the Linux PCI scan is an endless loop). I don't if this way is better: =2D-- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -2571,2 +2571,2 @@ static inline void powerpc_excp(CPUState *env, int=20 excp_model, int excp) /* Save MSR */ =2D env->spr[srr1] =3D msr; + env->spr[srr1] =3D msr & ~(1 << MSR_POW | 1 << MSR_TGPR); =2D-- a/target-ppc/op_helper.c +++ b/target-ppc/op_helper.c @@ -1648,2 +1648,2 @@ void helper_rfi (void) do_rfi(env->spr[SPR_SRR0], env->spr[SPR_SRR1], =2D ~((target_ulong)0x0), 1); + ~((target_ulong) 1 << MSR_ILE), 1); =2D-=20 Thomas