| Message ID | 201005181600.27913.thomas_ml@monjalon.net |
|---|---|
| State | New |
| Headers | show |
Thomas Monjalon wrote: > 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 > definition are not yet published. > Wow, I didn't know there were still new products based on e300. > The boot process hang with the actual implementation of RFI. > Hrm. > >>> 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. >> > > From the programming manual (MPCFPE32B): > "The save/restore register 1 (SRR1) is used to save machine status (selected > bits from the MSR and other implementation-specific status bits as well) on > interrupts and to restore those values when rfi is executed. > [..] > When an interrupt occurs, SRR1 bits 1–4 and 10–15 are loaded with > interrupt-specific information and MSR bits 16–23, 25–27, and 30–31 are > placed into the corresponding bit positions of SRR1. Depending on the > implementation, additional MSR bits may be copied to SRR1." > According to the 2.06 ISA, for example the DSI sets SRR1 to: 33:36 Set to 0 42:47 Set to 0 Others Loaded from the MSR > From 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 and > ILE are not cleared. > According to the 2.06 ISA again, rfi does the following: The contents of SRR1 are placed into the MSR. If the new MSR value does not enable any pending exceptions, then the next instruction is fetched, under control of the new MSR value, from the address SRR0 0:64 || 0b00. (Note: VLE behavior may be different; see Book VLE). Book VLE is the Variable Length Encoded Instructions Architecture, so that doesn't come into play here. If rfi would clear ILE, how would it be enabled then? Alex
Alexander Graf wrote: > Thomas Monjalon wrote: > > I'm running Linux for SBC834x in Qemu. The interrupt controller and board > > definition are not yet published. > > Wow, I didn't know there were still new products based on e300. Sorry, I was not clear. By "not yet published", I mean that I've written Qemu code to emulate e300 but I haven't yet send it to the ML. I would prefer to fix this RFI issue first. SBC834x is not a new product. > > From 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 > > and ILE are not cleared. > > According to the 2.06 ISA again, rfi does the following: > > The contents of SRR1 are placed into the MSR. If the new MSR value does > not enable any pending exceptions, then the next instruction is fetched, > under control of the new MSR value, from the address SRR0 0:64 || 0b00. > > If rfi would clear ILE, how would it be enabled then? You should be right. I have to fix a bug elsewhere.
--- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -2571,2 +2571,2 @@ static inline void powerpc_excp(CPUState *env, int excp_model, int excp) /* Save MSR */ - env->spr[srr1] = msr; + env->spr[srr1] = msr & ~(1 << MSR_POW | 1 << MSR_TGPR); --- 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], - ~((target_ulong)0x0), 1); + ~((target_ulong) 1 << MSR_ILE), 1);