| Message ID | 1467355319-28406-6-git-send-email-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
On 01/07/16 07:41, David Gibson wrote: > From: Benjamin Herrenschmidt <b378bb0948277d71c78bc6d0c1ef80a253aafc80> > > The architecture specifies that any instruction that sets MSR:PR will also > set MSR:EE, IR and DR. > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Signed-off-by: Cédric Le Goater <clg@kaod.org> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- > target-ppc/helper_regs.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h > index 8fc0934..8fdfa5c 100644 > --- a/target-ppc/helper_regs.h > +++ b/target-ppc/helper_regs.h > @@ -136,6 +136,10 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value, > /* Change the exception prefix on PowerPC 601 */ > env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000; > } > + /* If PR=1 then EE, IR and DR must be 1 */ > + if ((value >> MSR_PR) & 1) { > + value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR); > + } > #endif > env->msr = value; > hreg_compute_hflags(env); > Unfortunately this patch causes a regression and breaks booting OS 9 and OS X under qemu-system-ppc. ATB, Mark.
On Sat, 2016-07-09 at 01:43 +0100, Mark Cave-Ayland wrote: > On 01/07/16 07:41, David Gibson wrote: > > > From: Benjamin Herrenschmidt > > > > The architecture specifies that any instruction that sets MSR:PR will also > > set MSR:EE, IR and DR. .../... > Unfortunately this patch causes a regression and breaks booting OS 9 and > OS X under qemu-system-ppc. Any idea what is breaking specifically ? The architecture is pretty clear here, could it be that they rely on old implementations allowing the incorrect combination ? Maybe we can make the restriction 64-bit server only... Cheers, Ben.
On Sat, 2016-07-09 at 12:46 +1000, Benjamin Herrenschmidt wrote: > On Sat, 2016-07-09 at 01:43 +0100, Mark Cave-Ayland wrote: > > On 01/07/16 07:41, David Gibson wrote: > > > > > From: Benjamin Herrenschmidt > > > > > > The architecture specifies that any instruction that sets MSR:PR > > > will also > > > set MSR:EE, IR and DR. > > .../... > > > Unfortunately this patch causes a regression and breaks booting OS > > 9 and > > OS X under qemu-system-ppc. > > Any idea what is breaking specifically ? The architecture is pretty > clear > here, could it be that they rely on old implementations allowing the > incorrect combination ? > > Maybe we can make the restriction 64-bit server only... Additionally, hreg_compute_mem_idx() will treat PR=1 as DR=1/IR=1 as well ! That means that if those old processors allow PR=1 and IR or DR=0 and MacOS uses it, we do have a TLB coherency problem in qemu. Cheers, Ben.
On Sat, 2016-07-09 at 12:52 +1000, Benjamin Herrenschmidt wrote: > > Additionally, hreg_compute_mem_idx() will treat PR=1 as DR=1/IR=1 > as well ! That means that if those old processors allow PR=1 and IR > or DR=0 and MacOS uses it, we do have a TLB coherency problem in > qemu. Wow, yes indeed, I see an MSR with PR=1 IR=0, IR=1 and EE=0 .. ugh. Cheers, Ben.
On Sat, 2016-07-09 at 13:00 +1000, Benjamin Herrenschmidt wrote: > > Additionally, hreg_compute_mem_idx() will treat PR=1 as DR=1/IR=1 > > as well ! That means that if those old processors allow PR=1 and IR > > or DR=0 and MacOS uses it, we do have a TLB coherency problem in > > qemu. > > Wow, yes indeed, I see an MSR with PR=1 IR=0, IR=1 and EE=0 .. ugh. Note that I see that happening with OS 9, but not with Darwin ... are you sure about OS X ? Cheers, Ben.
On 07/09/2016 02:43 AM, Mark Cave-Ayland wrote: > On 01/07/16 07:41, David Gibson wrote: > >> From: Benjamin Herrenschmidt <b378bb0948277d71c78bc6d0c1ef80a253aafc80> >> >> The architecture specifies that any instruction that sets MSR:PR will also >> set MSR:EE, IR and DR. >> >> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >> --- >> target-ppc/helper_regs.h | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h >> index 8fc0934..8fdfa5c 100644 >> --- a/target-ppc/helper_regs.h >> +++ b/target-ppc/helper_regs.h >> @@ -136,6 +136,10 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value, >> /* Change the exception prefix on PowerPC 601 */ >> env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000; >> } >> + /* If PR=1 then EE, IR and DR must be 1 */ >> + if ((value >> MSR_PR) & 1) { >> + value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR); >> + } >> #endif >> env->msr = value; >> hreg_compute_hflags(env); >> > > Unfortunately this patch causes a regression and breaks booting OS 9 and > OS X under qemu-system-ppc. Ah This is curious. I used : qemu-system-ppc -M g3beige -cdrom darwinppc-602.cdr -boot d qemu-system-ppc -M mac99 -cdrom darwinppc-602.cdr -boot d qemu-system-ppc64 -M g3beige -cdrom darwinppc-602.cdr -boot d which "work" as they reach the installation prompt : The following devices are available for installation. This one hangs : qemu-system-ppc64 -M mac99 -cdrom darwinppc-602.cdr -boot d But that is expected for a 970 cpu. The login prompt is reached with a full Darwin disk image. So I must be missing a scenario :/ Thanks, C. > > ATB, > > Mark. >
On Sat, 2016-07-09 at 10:16 +0200, Cédric Le Goater wrote: > The login prompt is reached with a full Darwin disk image. > > So I must be missing a scenario :/ Yes same here, Darwin worked fine, but I did reproduce the problem with OS 9. See the fix I sent. Cheers, Ben.
On 07/09/2016 10:25 AM, Benjamin Herrenschmidt wrote: > On Sat, 2016-07-09 at 10:16 +0200, Cédric Le Goater wrote: >> The login prompt is reached with a full Darwin disk image. >> >> So I must be missing a scenario :/ > > Yes same here, Darwin worked fine, but I did reproduce the problem with > OS 9. See the fix I sent. Yep. I need to get one of these installers then. Thanks, C.
On 09/07/16 04:08, Benjamin Herrenschmidt wrote: > On Sat, 2016-07-09 at 13:00 +1000, Benjamin Herrenschmidt wrote: >>> Additionally, hreg_compute_mem_idx() will treat PR=1 as DR=1/IR=1 >>> as well ! That means that if those old processors allow PR=1 and IR >>> or DR=0 and MacOS uses it, we do have a TLB coherency problem in >>> qemu. >> >> Wow, yes indeed, I see an MSR with PR=1 IR=0, IR=1 and EE=0 .. ugh. > > Note that I see that happening with OS 9, but not with Darwin ... are > you sure about OS X ? > > Cheers, > Ben. Hmmm actually I think OS X might have been a red herring - I double-checked and it looks like I was accidentally testing an illegal combination, i.e. OS X 10.2 with -M mac99 which of course doesn't work. ATB, Mark.
diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h index 8fc0934..8fdfa5c 100644 --- a/target-ppc/helper_regs.h +++ b/target-ppc/helper_regs.h @@ -136,6 +136,10 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value, /* Change the exception prefix on PowerPC 601 */ env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000; } + /* If PR=1 then EE, IR and DR must be 1 */ + if ((value >> MSR_PR) & 1) { + value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR); + } #endif env->msr = value; hreg_compute_hflags(env);