| Message ID | 1464655277-14748-5-git-send-email-david@gibson.dropbear.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 31/05/16 01:41, David Gibson wrote: > From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > Not that anything remotely recent supports tlbia but ... > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- > target-ppc/translate.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index dfd3010..690ffd2 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -4858,7 +4858,7 @@ static void gen_tlbie(DisasContext *ctx) > #if defined(CONFIG_USER_ONLY) > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > #else > - if (unlikely(ctx->pr)) { > + if (unlikely(ctx->pr || !ctx->hv)) { > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > return; > } > @@ -4879,7 +4879,7 @@ static void gen_tlbsync(DisasContext *ctx) > #if defined(CONFIG_USER_ONLY) > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > #else > - if (unlikely(ctx->pr)) { > + if (unlikely(ctx->pr || !ctx->hv)) { > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > return; > } > @@ -4898,7 +4898,7 @@ static void gen_slbia(DisasContext *ctx) > #if defined(CONFIG_USER_ONLY) > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > #else > - if (unlikely(ctx->pr)) { > + if (unlikely(ctx->pr || !ctx->hv)) { > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > return; > } Unfortunately this patch breaks qemu-system-ppc for both g3beige and mac99 under TCG causing a freeze in OpenBIOS when starting qemu-system-ppc with no parameters. Note that there is also another regression that has recently landed in git master so you'll also need to revert e7c9136977cb99c6eb52c9139f7b8d8b5fa87db9 in order to get back to a functioning OpenBIOS. ATB, Mark.
On Tue, May 31, 2016 at 11:28:49PM +0100, Mark Cave-Ayland wrote: > On 31/05/16 01:41, David Gibson wrote: > > > From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > > > Not that anything remotely recent supports tlbia but ... > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > --- > > target-ppc/translate.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > > index dfd3010..690ffd2 100644 > > --- a/target-ppc/translate.c > > +++ b/target-ppc/translate.c > > @@ -4858,7 +4858,7 @@ static void gen_tlbie(DisasContext *ctx) > > #if defined(CONFIG_USER_ONLY) > > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > #else > > - if (unlikely(ctx->pr)) { > > + if (unlikely(ctx->pr || !ctx->hv)) { > > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > return; > > } > > @@ -4879,7 +4879,7 @@ static void gen_tlbsync(DisasContext *ctx) > > #if defined(CONFIG_USER_ONLY) > > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > #else > > - if (unlikely(ctx->pr)) { > > + if (unlikely(ctx->pr || !ctx->hv)) { > > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > return; > > } > > @@ -4898,7 +4898,7 @@ static void gen_slbia(DisasContext *ctx) > > #if defined(CONFIG_USER_ONLY) > > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > #else > > - if (unlikely(ctx->pr)) { > > + if (unlikely(ctx->pr || !ctx->hv)) { > > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > return; > > } > > Unfortunately this patch breaks qemu-system-ppc for both g3beige and > mac99 under TCG causing a freeze in OpenBIOS when starting > qemu-system-ppc with no parameters. Bother, sorry. I think this is because I applied this without the patch that treats machines with no hypervisor mode (e.g. Apples) as always being in hypervisor mode. > Note that there is also another regression that has recently landed in > git master so you'll also need to revert > e7c9136977cb99c6eb52c9139f7b8d8b5fa87db9 in order to get back to a > functioning OpenBIOS. I'd preter to see it fixed rather than just reverted..
On 01/06/16 03:15, David Gibson wrote: > On Tue, May 31, 2016 at 11:28:49PM +0100, Mark Cave-Ayland wrote: >> On 31/05/16 01:41, David Gibson wrote: >> >>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>> >>> Not that anything remotely recent supports tlbia but ... >>> >>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >>> --- >>> target-ppc/translate.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c >>> index dfd3010..690ffd2 100644 >>> --- a/target-ppc/translate.c >>> +++ b/target-ppc/translate.c >>> @@ -4858,7 +4858,7 @@ static void gen_tlbie(DisasContext *ctx) >>> #if defined(CONFIG_USER_ONLY) >>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>> #else >>> - if (unlikely(ctx->pr)) { >>> + if (unlikely(ctx->pr || !ctx->hv)) { >>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>> return; >>> } >>> @@ -4879,7 +4879,7 @@ static void gen_tlbsync(DisasContext *ctx) >>> #if defined(CONFIG_USER_ONLY) >>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>> #else >>> - if (unlikely(ctx->pr)) { >>> + if (unlikely(ctx->pr || !ctx->hv)) { >>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>> return; >>> } >>> @@ -4898,7 +4898,7 @@ static void gen_slbia(DisasContext *ctx) >>> #if defined(CONFIG_USER_ONLY) >>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>> #else >>> - if (unlikely(ctx->pr)) { >>> + if (unlikely(ctx->pr || !ctx->hv)) { >>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>> return; >>> } >> >> Unfortunately this patch breaks qemu-system-ppc for both g3beige and >> mac99 under TCG causing a freeze in OpenBIOS when starting >> qemu-system-ppc with no parameters. > > Bother, sorry. > > I think this is because I applied this without the patch that treats > machines with no hypervisor mode (e.g. Apples) as always being in > hypervisor mode. No problem, I can cope for a couple of days or so. >> Note that there is also another regression that has recently landed in >> git master so you'll also need to revert >> e7c9136977cb99c6eb52c9139f7b8d8b5fa87db9 in order to get back to a >> functioning OpenBIOS. > > I'd preter to see it fixed rather than just reverted.. Looks like the original author has found the bug, so there should be a fix coming up for this soon (I only included it here in case you needed an explicit test case). ATB, Mark.
On Wed, Jun 01, 2016 at 08:03:08AM +0100, Mark Cave-Ayland wrote: > On 01/06/16 03:15, David Gibson wrote: > > > On Tue, May 31, 2016 at 11:28:49PM +0100, Mark Cave-Ayland wrote: > >> On 31/05/16 01:41, David Gibson wrote: > >> > >>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > >>> > >>> Not that anything remotely recent supports tlbia but ... > >>> > >>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > >>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > >>> --- > >>> target-ppc/translate.c | 6 +++--- > >>> 1 file changed, 3 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c > >>> index dfd3010..690ffd2 100644 > >>> --- a/target-ppc/translate.c > >>> +++ b/target-ppc/translate.c > >>> @@ -4858,7 +4858,7 @@ static void gen_tlbie(DisasContext *ctx) > >>> #if defined(CONFIG_USER_ONLY) > >>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > >>> #else > >>> - if (unlikely(ctx->pr)) { > >>> + if (unlikely(ctx->pr || !ctx->hv)) { > >>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > >>> return; > >>> } > >>> @@ -4879,7 +4879,7 @@ static void gen_tlbsync(DisasContext *ctx) > >>> #if defined(CONFIG_USER_ONLY) > >>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > >>> #else > >>> - if (unlikely(ctx->pr)) { > >>> + if (unlikely(ctx->pr || !ctx->hv)) { > >>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > >>> return; > >>> } > >>> @@ -4898,7 +4898,7 @@ static void gen_slbia(DisasContext *ctx) > >>> #if defined(CONFIG_USER_ONLY) > >>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > >>> #else > >>> - if (unlikely(ctx->pr)) { > >>> + if (unlikely(ctx->pr || !ctx->hv)) { > >>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > >>> return; > >>> } > >> > >> Unfortunately this patch breaks qemu-system-ppc for both g3beige and > >> mac99 under TCG causing a freeze in OpenBIOS when starting > >> qemu-system-ppc with no parameters. > > > > Bother, sorry. > > > > I think this is because I applied this without the patch that treats > > machines with no hypervisor mode (e.g. Apples) as always being in > > hypervisor mode. > > No problem, I can cope for a couple of days or so. Cédric, Not sure if you've seen this thread, but one of the HV-mode patches caused a regression on Mac. I think it's because I didn't include the other patch which treats Apple-mode PPCs as always having HV=1. Can you make sending your updated version of that patch a priority, even if the rest of the batch of HV patches isn't ready yet. > >> Note that there is also another regression that has recently landed in > >> git master so you'll also need to revert > >> e7c9136977cb99c6eb52c9139f7b8d8b5fa87db9 in order to get back to a > >> functioning OpenBIOS. > > > > I'd preter to see it fixed rather than just reverted.. > > Looks like the original author has found the bug, so there should be a > fix coming up for this soon (I only included it here in case you needed > an explicit test case). Ok. So, yeah, I'm not really set up to test Mac machines which means I don't easily catch regressions like this. Mark, Could you look into adding a testcase to "make check" that will at least catch these unsubtle breaks boot type regressions?
On 06/02/2016 05:17 AM, David Gibson wrote: > On Wed, Jun 01, 2016 at 08:03:08AM +0100, Mark Cave-Ayland wrote: >> On 01/06/16 03:15, David Gibson wrote: >> >>> On Tue, May 31, 2016 at 11:28:49PM +0100, Mark Cave-Ayland wrote: >>>> On 31/05/16 01:41, David Gibson wrote: >>>> >>>>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>>> >>>>> Not that anything remotely recent supports tlbia but ... >>>>> >>>>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >>>>> --- >>>>> target-ppc/translate.c | 6 +++--- >>>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c >>>>> index dfd3010..690ffd2 100644 >>>>> --- a/target-ppc/translate.c >>>>> +++ b/target-ppc/translate.c >>>>> @@ -4858,7 +4858,7 @@ static void gen_tlbie(DisasContext *ctx) >>>>> #if defined(CONFIG_USER_ONLY) >>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>> #else >>>>> - if (unlikely(ctx->pr)) { >>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>> return; >>>>> } >>>>> @@ -4879,7 +4879,7 @@ static void gen_tlbsync(DisasContext *ctx) >>>>> #if defined(CONFIG_USER_ONLY) >>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>> #else >>>>> - if (unlikely(ctx->pr)) { >>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>> return; >>>>> } >>>>> @@ -4898,7 +4898,7 @@ static void gen_slbia(DisasContext *ctx) >>>>> #if defined(CONFIG_USER_ONLY) >>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>> #else >>>>> - if (unlikely(ctx->pr)) { >>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>> return; >>>>> } >>>> >>>> Unfortunately this patch breaks qemu-system-ppc for both g3beige and >>>> mac99 under TCG causing a freeze in OpenBIOS when starting >>>> qemu-system-ppc with no parameters. >>> >>> Bother, sorry. >>> >>> I think this is because I applied this without the patch that treats >>> machines with no hypervisor mode (e.g. Apples) as always being in >>> hypervisor mode. >> >> No problem, I can cope for a couple of days or so. > > Cédric, > > Not sure if you've seen this thread, but one of the HV-mode patches > caused a regression on Mac. I think it's because I didn't include the > other patch which treats Apple-mode PPCs as always having HV=1. I missed that as I didn't put myself in Cc :/ > Can you make sending your updated version of that patch a priority, > even if the rest of the batch of HV patches isn't ready yet. sure. I will/should today or tomorrow. I suppose we want these patches : [05/12] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV http://patchwork.ozlabs.org/patch/618083/ [07/12] ppc: Better figure out if processor has HV mode http://patchwork.ozlabs.org/patch/618089/ Mark, I tried to boot a darwinppc-602.iso with : qemu-system-ppc -M g3beige -cdrom darwinx86-602.iso -boot d but I get a : "No valid state has been set by load or ..." or we don't need to go further ? may be I need a newer FW. Could you try the two patches above please ? They apply on top of Dave's ppc-for-2.7-20160531 and seem to have a good behavior with the small test I could do. Thanks, C. >>>> Note that there is also another regression that has recently landed in >>>> git master so you'll also need to revert >>>> e7c9136977cb99c6eb52c9139f7b8d8b5fa87db9 in order to get back to a >>>> functioning OpenBIOS. >>> >>> I'd preter to see it fixed rather than just reverted.. >> >> Looks like the original author has found the bug, so there should be a >> fix coming up for this soon (I only included it here in case you needed >> an explicit test case). > > Ok. > > So, yeah, I'm not really set up to test Mac machines which means I > don't easily catch regressions like this. > > Mark, > > Could you look into adding a testcase to "make check" that will at > least catch these unsubtle breaks boot type regressions? >
On 02/06/16 08:37, Cédric Le Goater wrote: > On 06/02/2016 05:17 AM, David Gibson wrote: >> On Wed, Jun 01, 2016 at 08:03:08AM +0100, Mark Cave-Ayland wrote: >>> On 01/06/16 03:15, David Gibson wrote: >>> >>>> On Tue, May 31, 2016 at 11:28:49PM +0100, Mark Cave-Ayland wrote: >>>>> On 31/05/16 01:41, David Gibson wrote: >>>>> >>>>>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>>>> >>>>>> Not that anything remotely recent supports tlbia but ... >>>>>> >>>>>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >>>>>> --- >>>>>> target-ppc/translate.c | 6 +++--- >>>>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c >>>>>> index dfd3010..690ffd2 100644 >>>>>> --- a/target-ppc/translate.c >>>>>> +++ b/target-ppc/translate.c >>>>>> @@ -4858,7 +4858,7 @@ static void gen_tlbie(DisasContext *ctx) >>>>>> #if defined(CONFIG_USER_ONLY) >>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>> #else >>>>>> - if (unlikely(ctx->pr)) { >>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>> return; >>>>>> } >>>>>> @@ -4879,7 +4879,7 @@ static void gen_tlbsync(DisasContext *ctx) >>>>>> #if defined(CONFIG_USER_ONLY) >>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>> #else >>>>>> - if (unlikely(ctx->pr)) { >>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>> return; >>>>>> } >>>>>> @@ -4898,7 +4898,7 @@ static void gen_slbia(DisasContext *ctx) >>>>>> #if defined(CONFIG_USER_ONLY) >>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>> #else >>>>>> - if (unlikely(ctx->pr)) { >>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>> return; >>>>>> } >>>>> >>>>> Unfortunately this patch breaks qemu-system-ppc for both g3beige and >>>>> mac99 under TCG causing a freeze in OpenBIOS when starting >>>>> qemu-system-ppc with no parameters. >>>> >>>> Bother, sorry. >>>> >>>> I think this is because I applied this without the patch that treats >>>> machines with no hypervisor mode (e.g. Apples) as always being in >>>> hypervisor mode. >>> >>> No problem, I can cope for a couple of days or so. >> >> Cédric, >> >> Not sure if you've seen this thread, but one of the HV-mode patches >> caused a regression on Mac. I think it's because I didn't include the >> other patch which treats Apple-mode PPCs as always having HV=1. > > I missed that as I didn't put myself in Cc :/ > >> Can you make sending your updated version of that patch a priority, >> even if the rest of the batch of HV patches isn't ready yet. > > sure. I will/should today or tomorrow. I suppose we want these patches : > > [05/12] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV > http://patchwork.ozlabs.org/patch/618083/ > > [07/12] ppc: Better figure out if processor has HV mode > http://patchwork.ozlabs.org/patch/618089/ > > > Mark, > > I tried to boot a darwinppc-602.iso with : > > qemu-system-ppc -M g3beige -cdrom darwinx86-602.iso -boot d > > but I get a : > > "No valid state has been set by load or ..." > > or we don't need to go further ? may be I need a newer FW. Hmmm that looks like you've got an x86 ISO there which is why OpenBIOS/PPC fails to execute the bootloader. The image I use for testing can be found here: https://opensource.apple.com/static/iso/darwinppc-602.cdr.gz (simply gunzip and then rename to .iso). > Could you try the two patches above please ? They apply on top of Dave's > ppc-for-2.7-20160531 and seem to have a good behavior with the small test > I could do. I'll try and take a look tomorrow, however in the meantime see if the above image enables you to replicate the issue locally. ATB, Mark.
On 06/02/2016 09:45 AM, Mark Cave-Ayland wrote: > On 02/06/16 08:37, Cédric Le Goater wrote: >> On 06/02/2016 05:17 AM, David Gibson wrote: >>> On Wed, Jun 01, 2016 at 08:03:08AM +0100, Mark Cave-Ayland wrote: >>>> On 01/06/16 03:15, David Gibson wrote: >>>> >>>>> On Tue, May 31, 2016 at 11:28:49PM +0100, Mark Cave-Ayland wrote: >>>>>> On 31/05/16 01:41, David Gibson wrote: >>>>>> >>>>>>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>>>>> >>>>>>> Not that anything remotely recent supports tlbia but ... >>>>>>> >>>>>>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >>>>>>> --- >>>>>>> target-ppc/translate.c | 6 +++--- >>>>>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>>>>> >>>>>>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c >>>>>>> index dfd3010..690ffd2 100644 >>>>>>> --- a/target-ppc/translate.c >>>>>>> +++ b/target-ppc/translate.c >>>>>>> @@ -4858,7 +4858,7 @@ static void gen_tlbie(DisasContext *ctx) >>>>>>> #if defined(CONFIG_USER_ONLY) >>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>> #else >>>>>>> - if (unlikely(ctx->pr)) { >>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>> return; >>>>>>> } >>>>>>> @@ -4879,7 +4879,7 @@ static void gen_tlbsync(DisasContext *ctx) >>>>>>> #if defined(CONFIG_USER_ONLY) >>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>> #else >>>>>>> - if (unlikely(ctx->pr)) { >>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>> return; >>>>>>> } >>>>>>> @@ -4898,7 +4898,7 @@ static void gen_slbia(DisasContext *ctx) >>>>>>> #if defined(CONFIG_USER_ONLY) >>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>> #else >>>>>>> - if (unlikely(ctx->pr)) { >>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>> return; >>>>>>> } >>>>>> >>>>>> Unfortunately this patch breaks qemu-system-ppc for both g3beige and >>>>>> mac99 under TCG causing a freeze in OpenBIOS when starting >>>>>> qemu-system-ppc with no parameters. >>>>> >>>>> Bother, sorry. >>>>> >>>>> I think this is because I applied this without the patch that treats >>>>> machines with no hypervisor mode (e.g. Apples) as always being in >>>>> hypervisor mode. >>>> >>>> No problem, I can cope for a couple of days or so. >>> >>> Cédric, >>> >>> Not sure if you've seen this thread, but one of the HV-mode patches >>> caused a regression on Mac. I think it's because I didn't include the >>> other patch which treats Apple-mode PPCs as always having HV=1. >> >> I missed that as I didn't put myself in Cc :/ >> >>> Can you make sending your updated version of that patch a priority, >>> even if the rest of the batch of HV patches isn't ready yet. >> >> sure. I will/should today or tomorrow. I suppose we want these patches : >> >> [05/12] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV >> http://patchwork.ozlabs.org/patch/618083/ >> >> [07/12] ppc: Better figure out if processor has HV mode >> http://patchwork.ozlabs.org/patch/618089/ >> >> >> Mark, >> >> I tried to boot a darwinppc-602.iso with : >> >> qemu-system-ppc -M g3beige -cdrom darwinx86-602.iso -boot d >> >> but I get a : >> >> "No valid state has been set by load or ..." >> >> or we don't need to go further ? may be I need a newer FW. > > Hmmm that looks like you've got an x86 ISO there which is why > OpenBIOS/PPC fails to execute the bootloader. The image I use for > testing can be found here: > https://opensource.apple.com/static/iso/darwinppc-602.cdr.gz (simply > gunzip and then rename to .iso). Got it. much better with ppc :) ppc is not that omnipotent. >> Could you try the two patches above please ? They apply on top of Dave's >> ppc-for-2.7-20160531 and seem to have a good behavior with the small test >> I could do. > > I'll try and take a look tomorrow, however in the meantime see if the > above image enables you to replicate the issue locally. so, on top of ppc-for-2.7-20160531, with your fix for : ppc: Use split I/D mmu modes to avoid flushes on interrupts and these two patches : [05/12] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV http://patchwork.ozlabs.org/patch/618083/ [07/12] ppc: Better figure out if processor has HV mode http://patchwork.ozlabs.org/patch/618089/ The darwin cd boots correctly up to : ... The following devices are available for installation : and then loops on something. But I don't get a kernel panic anymore. Dave, I still need to dig the !msr_pr removal. Thanks, C. > ATB, > > Mark. >
On 02/06/16 09:23, Cédric Le Goater wrote: > On 06/02/2016 09:45 AM, Mark Cave-Ayland wrote: >> On 02/06/16 08:37, Cédric Le Goater wrote: >>> On 06/02/2016 05:17 AM, David Gibson wrote: >>>> On Wed, Jun 01, 2016 at 08:03:08AM +0100, Mark Cave-Ayland wrote: >>>>> On 01/06/16 03:15, David Gibson wrote: >>>>> >>>>>> On Tue, May 31, 2016 at 11:28:49PM +0100, Mark Cave-Ayland wrote: >>>>>>> On 31/05/16 01:41, David Gibson wrote: >>>>>>> >>>>>>>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>>>>>> >>>>>>>> Not that anything remotely recent supports tlbia but ... >>>>>>>> >>>>>>>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>>>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >>>>>>>> --- >>>>>>>> target-ppc/translate.c | 6 +++--- >>>>>>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>>>>>> >>>>>>>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c >>>>>>>> index dfd3010..690ffd2 100644 >>>>>>>> --- a/target-ppc/translate.c >>>>>>>> +++ b/target-ppc/translate.c >>>>>>>> @@ -4858,7 +4858,7 @@ static void gen_tlbie(DisasContext *ctx) >>>>>>>> #if defined(CONFIG_USER_ONLY) >>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>> #else >>>>>>>> - if (unlikely(ctx->pr)) { >>>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>> return; >>>>>>>> } >>>>>>>> @@ -4879,7 +4879,7 @@ static void gen_tlbsync(DisasContext *ctx) >>>>>>>> #if defined(CONFIG_USER_ONLY) >>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>> #else >>>>>>>> - if (unlikely(ctx->pr)) { >>>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>> return; >>>>>>>> } >>>>>>>> @@ -4898,7 +4898,7 @@ static void gen_slbia(DisasContext *ctx) >>>>>>>> #if defined(CONFIG_USER_ONLY) >>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>> #else >>>>>>>> - if (unlikely(ctx->pr)) { >>>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>> return; >>>>>>>> } >>>>>>> >>>>>>> Unfortunately this patch breaks qemu-system-ppc for both g3beige and >>>>>>> mac99 under TCG causing a freeze in OpenBIOS when starting >>>>>>> qemu-system-ppc with no parameters. >>>>>> >>>>>> Bother, sorry. >>>>>> >>>>>> I think this is because I applied this without the patch that treats >>>>>> machines with no hypervisor mode (e.g. Apples) as always being in >>>>>> hypervisor mode. >>>>> >>>>> No problem, I can cope for a couple of days or so. >>>> >>>> Cédric, >>>> >>>> Not sure if you've seen this thread, but one of the HV-mode patches >>>> caused a regression on Mac. I think it's because I didn't include the >>>> other patch which treats Apple-mode PPCs as always having HV=1. >>> >>> I missed that as I didn't put myself in Cc :/ >>> >>>> Can you make sending your updated version of that patch a priority, >>>> even if the rest of the batch of HV patches isn't ready yet. >>> >>> sure. I will/should today or tomorrow. I suppose we want these patches : >>> >>> [05/12] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV >>> http://patchwork.ozlabs.org/patch/618083/ >>> >>> [07/12] ppc: Better figure out if processor has HV mode >>> http://patchwork.ozlabs.org/patch/618089/ >>> >>> >>> Mark, >>> >>> I tried to boot a darwinppc-602.iso with : >>> >>> qemu-system-ppc -M g3beige -cdrom darwinx86-602.iso -boot d >>> >>> but I get a : >>> >>> "No valid state has been set by load or ..." >>> >>> or we don't need to go further ? may be I need a newer FW. >> >> Hmmm that looks like you've got an x86 ISO there which is why >> OpenBIOS/PPC fails to execute the bootloader. The image I use for >> testing can be found here: >> https://opensource.apple.com/static/iso/darwinppc-602.cdr.gz (simply >> gunzip and then rename to .iso). > > Got it. much better with ppc :) ppc is not that omnipotent. :) >>> Could you try the two patches above please ? They apply on top of Dave's >>> ppc-for-2.7-20160531 and seem to have a good behavior with the small test >>> I could do. >> >> I'll try and take a look tomorrow, however in the meantime see if the >> above image enables you to replicate the issue locally. > > > so, on top of ppc-for-2.7-20160531, with your fix for : > > ppc: Use split I/D mmu modes to avoid flushes on interrupts Unfortunately this isn't really a fix: the whole point of splitting the MMU modes is to be able to avoid these expensive cache flushes in the first place. Then again it could be that this is exposing an existing bug elsewhere... > and these two patches : > > [05/12] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV > http://patchwork.ozlabs.org/patch/618083/ > > [07/12] ppc: Better figure out if processor has HV mode > http://patchwork.ozlabs.org/patch/618089/ > > The darwin cd boots correctly up to : > > ... > The following devices are available for installation : > > and then loops on something. But I don't get a kernel panic anymore. Yes, that effectively matches what I see here - glad that you are now able to reproduce this. ATB, Mark.
On 02/06/16 09:47, Mark Cave-Ayland wrote: > On 02/06/16 09:23, Cédric Le Goater wrote: > >> On 06/02/2016 09:45 AM, Mark Cave-Ayland wrote: >>> On 02/06/16 08:37, Cédric Le Goater wrote: >>>> On 06/02/2016 05:17 AM, David Gibson wrote: >>>>> On Wed, Jun 01, 2016 at 08:03:08AM +0100, Mark Cave-Ayland wrote: >>>>>> On 01/06/16 03:15, David Gibson wrote: >>>>>> >>>>>>> On Tue, May 31, 2016 at 11:28:49PM +0100, Mark Cave-Ayland wrote: >>>>>>>> On 31/05/16 01:41, David Gibson wrote: >>>>>>>> >>>>>>>>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>>>>>>> >>>>>>>>> Not that anything remotely recent supports tlbia but ... >>>>>>>>> >>>>>>>>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>>>>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >>>>>>>>> --- >>>>>>>>> target-ppc/translate.c | 6 +++--- >>>>>>>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c >>>>>>>>> index dfd3010..690ffd2 100644 >>>>>>>>> --- a/target-ppc/translate.c >>>>>>>>> +++ b/target-ppc/translate.c >>>>>>>>> @@ -4858,7 +4858,7 @@ static void gen_tlbie(DisasContext *ctx) >>>>>>>>> #if defined(CONFIG_USER_ONLY) >>>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>>> #else >>>>>>>>> - if (unlikely(ctx->pr)) { >>>>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>>> return; >>>>>>>>> } >>>>>>>>> @@ -4879,7 +4879,7 @@ static void gen_tlbsync(DisasContext *ctx) >>>>>>>>> #if defined(CONFIG_USER_ONLY) >>>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>>> #else >>>>>>>>> - if (unlikely(ctx->pr)) { >>>>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>>> return; >>>>>>>>> } >>>>>>>>> @@ -4898,7 +4898,7 @@ static void gen_slbia(DisasContext *ctx) >>>>>>>>> #if defined(CONFIG_USER_ONLY) >>>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>>> #else >>>>>>>>> - if (unlikely(ctx->pr)) { >>>>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>>> return; >>>>>>>>> } >>>>>>>> >>>>>>>> Unfortunately this patch breaks qemu-system-ppc for both g3beige and >>>>>>>> mac99 under TCG causing a freeze in OpenBIOS when starting >>>>>>>> qemu-system-ppc with no parameters. >>>>>>> >>>>>>> Bother, sorry. >>>>>>> >>>>>>> I think this is because I applied this without the patch that treats >>>>>>> machines with no hypervisor mode (e.g. Apples) as always being in >>>>>>> hypervisor mode. >>>>>> >>>>>> No problem, I can cope for a couple of days or so. >>>>> >>>>> Cédric, >>>>> >>>>> Not sure if you've seen this thread, but one of the HV-mode patches >>>>> caused a regression on Mac. I think it's because I didn't include the >>>>> other patch which treats Apple-mode PPCs as always having HV=1. >>>> >>>> I missed that as I didn't put myself in Cc :/ >>>> >>>>> Can you make sending your updated version of that patch a priority, >>>>> even if the rest of the batch of HV patches isn't ready yet. >>>> >>>> sure. I will/should today or tomorrow. I suppose we want these patches : >>>> >>>> [05/12] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV >>>> http://patchwork.ozlabs.org/patch/618083/ >>>> >>>> [07/12] ppc: Better figure out if processor has HV mode >>>> http://patchwork.ozlabs.org/patch/618089/ >>>> >>>> >>>> Mark, >>>> >>>> I tried to boot a darwinppc-602.iso with : >>>> >>>> qemu-system-ppc -M g3beige -cdrom darwinx86-602.iso -boot d >>>> >>>> but I get a : >>>> >>>> "No valid state has been set by load or ..." >>>> >>>> or we don't need to go further ? may be I need a newer FW. >>> >>> Hmmm that looks like you've got an x86 ISO there which is why >>> OpenBIOS/PPC fails to execute the bootloader. The image I use for >>> testing can be found here: >>> https://opensource.apple.com/static/iso/darwinppc-602.cdr.gz (simply >>> gunzip and then rename to .iso). >> >> Got it. much better with ppc :) ppc is not that omnipotent. > > :) > >>>> Could you try the two patches above please ? They apply on top of Dave's >>>> ppc-for-2.7-20160531 and seem to have a good behavior with the small test >>>> I could do. >>> >>> I'll try and take a look tomorrow, however in the meantime see if the >>> above image enables you to replicate the issue locally. >> >> >> so, on top of ppc-for-2.7-20160531, with your fix for : >> >> ppc: Use split I/D mmu modes to avoid flushes on interrupts > > Unfortunately this isn't really a fix: the whole point of splitting the > MMU modes is to be able to avoid these expensive cache flushes in the > first place. Then again it could be that this is exposing an existing > bug elsewhere... > >> and these two patches : >> >> [05/12] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV >> http://patchwork.ozlabs.org/patch/618083/ >> >> [07/12] ppc: Better figure out if processor has HV mode >> http://patchwork.ozlabs.org/patch/618089/ >> >> The darwin cd boots correctly up to : >> >> ... >> The following devices are available for installation : >> >> and then loops on something. But I don't get a kernel panic anymore. > > Yes, that effectively matches what I see here - glad that you are now > able to reproduce this. Just to add in case it wasn't clear from my previous reply - this is actually the correct and expected behaviour. If you add a hard disk device with -hda then it will appear on-screen below the prompt at which point you can proceed with the installation as normal. ATB, Mark.
On 06/02/2016 08:09 PM, Mark Cave-Ayland wrote: > On 02/06/16 09:47, Mark Cave-Ayland wrote: > >> On 02/06/16 09:23, Cédric Le Goater wrote: >> >>> On 06/02/2016 09:45 AM, Mark Cave-Ayland wrote: >>>> On 02/06/16 08:37, Cédric Le Goater wrote: >>>>> On 06/02/2016 05:17 AM, David Gibson wrote: >>>>>> On Wed, Jun 01, 2016 at 08:03:08AM +0100, Mark Cave-Ayland wrote: >>>>>>> On 01/06/16 03:15, David Gibson wrote: >>>>>>> >>>>>>>> On Tue, May 31, 2016 at 11:28:49PM +0100, Mark Cave-Ayland wrote: >>>>>>>>> On 31/05/16 01:41, David Gibson wrote: >>>>>>>>> >>>>>>>>>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>>>>>>>> >>>>>>>>>> Not that anything remotely recent supports tlbia but ... >>>>>>>>>> >>>>>>>>>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>>>>>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >>>>>>>>>> --- >>>>>>>>>> target-ppc/translate.c | 6 +++--- >>>>>>>>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>>>>>>>> >>>>>>>>>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c >>>>>>>>>> index dfd3010..690ffd2 100644 >>>>>>>>>> --- a/target-ppc/translate.c >>>>>>>>>> +++ b/target-ppc/translate.c >>>>>>>>>> @@ -4858,7 +4858,7 @@ static void gen_tlbie(DisasContext *ctx) >>>>>>>>>> #if defined(CONFIG_USER_ONLY) >>>>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>>>> #else >>>>>>>>>> - if (unlikely(ctx->pr)) { >>>>>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>>>> return; >>>>>>>>>> } >>>>>>>>>> @@ -4879,7 +4879,7 @@ static void gen_tlbsync(DisasContext *ctx) >>>>>>>>>> #if defined(CONFIG_USER_ONLY) >>>>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>>>> #else >>>>>>>>>> - if (unlikely(ctx->pr)) { >>>>>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>>>> return; >>>>>>>>>> } >>>>>>>>>> @@ -4898,7 +4898,7 @@ static void gen_slbia(DisasContext *ctx) >>>>>>>>>> #if defined(CONFIG_USER_ONLY) >>>>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>>>> #else >>>>>>>>>> - if (unlikely(ctx->pr)) { >>>>>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { >>>>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); >>>>>>>>>> return; >>>>>>>>>> } >>>>>>>>> >>>>>>>>> Unfortunately this patch breaks qemu-system-ppc for both g3beige and >>>>>>>>> mac99 under TCG causing a freeze in OpenBIOS when starting >>>>>>>>> qemu-system-ppc with no parameters. >>>>>>>> >>>>>>>> Bother, sorry. >>>>>>>> >>>>>>>> I think this is because I applied this without the patch that treats >>>>>>>> machines with no hypervisor mode (e.g. Apples) as always being in >>>>>>>> hypervisor mode. >>>>>>> >>>>>>> No problem, I can cope for a couple of days or so. >>>>>> >>>>>> Cédric, >>>>>> >>>>>> Not sure if you've seen this thread, but one of the HV-mode patches >>>>>> caused a regression on Mac. I think it's because I didn't include the >>>>>> other patch which treats Apple-mode PPCs as always having HV=1. >>>>> >>>>> I missed that as I didn't put myself in Cc :/ >>>>> >>>>>> Can you make sending your updated version of that patch a priority, >>>>>> even if the rest of the batch of HV patches isn't ready yet. >>>>> >>>>> sure. I will/should today or tomorrow. I suppose we want these patches : >>>>> >>>>> [05/12] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV >>>>> http://patchwork.ozlabs.org/patch/618083/ >>>>> >>>>> [07/12] ppc: Better figure out if processor has HV mode >>>>> http://patchwork.ozlabs.org/patch/618089/ >>>>> >>>>> >>>>> Mark, >>>>> >>>>> I tried to boot a darwinppc-602.iso with : >>>>> >>>>> qemu-system-ppc -M g3beige -cdrom darwinx86-602.iso -boot d >>>>> >>>>> but I get a : >>>>> >>>>> "No valid state has been set by load or ..." >>>>> >>>>> or we don't need to go further ? may be I need a newer FW. >>>> >>>> Hmmm that looks like you've got an x86 ISO there which is why >>>> OpenBIOS/PPC fails to execute the bootloader. The image I use for >>>> testing can be found here: >>>> https://opensource.apple.com/static/iso/darwinppc-602.cdr.gz (simply >>>> gunzip and then rename to .iso). >>> >>> Got it. much better with ppc :) ppc is not that omnipotent. >> >> :) >> >>>>> Could you try the two patches above please ? They apply on top of Dave's >>>>> ppc-for-2.7-20160531 and seem to have a good behavior with the small test >>>>> I could do. >>>> >>>> I'll try and take a look tomorrow, however in the meantime see if the >>>> above image enables you to replicate the issue locally. >>> >>> >>> so, on top of ppc-for-2.7-20160531, with your fix for : >>> >>> ppc: Use split I/D mmu modes to avoid flushes on interrupts >> >> Unfortunately this isn't really a fix: the whole point of splitting the >> MMU modes is to be able to avoid these expensive cache flushes in the >> first place. Then again it could be that this is exposing an existing >> bug elsewhere... >> >>> and these two patches : >>> >>> [05/12] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV >>> http://patchwork.ozlabs.org/patch/618083/ >>> >>> [07/12] ppc: Better figure out if processor has HV mode >>> http://patchwork.ozlabs.org/patch/618089/ >>> >>> The darwin cd boots correctly up to : >>> >>> ... >>> The following devices are available for installation : >>> >>> and then loops on something. But I don't get a kernel panic anymore. >> >> Yes, that effectively matches what I see here - glad that you are now >> able to reproduce this. > > Just to add in case it wasn't clear from my previous reply - this is > actually the correct and expected behaviour. If you add a hard disk > device with -hda then it will appear on-screen below the prompt at which > point you can proceed with the installation as normal. yes. I tried that and it did. C.
On Thu, Jun 02, 2016 at 09:47:01AM +0100, Mark Cave-Ayland wrote: > On 02/06/16 09:23, Cédric Le Goater wrote: > > > On 06/02/2016 09:45 AM, Mark Cave-Ayland wrote: > >> On 02/06/16 08:37, Cédric Le Goater wrote: > >>> On 06/02/2016 05:17 AM, David Gibson wrote: > >>>> On Wed, Jun 01, 2016 at 08:03:08AM +0100, Mark Cave-Ayland wrote: > >>>>> On 01/06/16 03:15, David Gibson wrote: > >>>>> > >>>>>> On Tue, May 31, 2016 at 11:28:49PM +0100, Mark Cave-Ayland wrote: > >>>>>>> On 31/05/16 01:41, David Gibson wrote: > >>>>>>> > >>>>>>>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > >>>>>>>> > >>>>>>>> Not that anything remotely recent supports tlbia but ... > >>>>>>>> > >>>>>>>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > >>>>>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > >>>>>>>> --- > >>>>>>>> target-ppc/translate.c | 6 +++--- > >>>>>>>> 1 file changed, 3 insertions(+), 3 deletions(-) > >>>>>>>> > >>>>>>>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c > >>>>>>>> index dfd3010..690ffd2 100644 > >>>>>>>> --- a/target-ppc/translate.c > >>>>>>>> +++ b/target-ppc/translate.c > >>>>>>>> @@ -4858,7 +4858,7 @@ static void gen_tlbie(DisasContext *ctx) > >>>>>>>> #if defined(CONFIG_USER_ONLY) > >>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > >>>>>>>> #else > >>>>>>>> - if (unlikely(ctx->pr)) { > >>>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { > >>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > >>>>>>>> return; > >>>>>>>> } > >>>>>>>> @@ -4879,7 +4879,7 @@ static void gen_tlbsync(DisasContext *ctx) > >>>>>>>> #if defined(CONFIG_USER_ONLY) > >>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > >>>>>>>> #else > >>>>>>>> - if (unlikely(ctx->pr)) { > >>>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { > >>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > >>>>>>>> return; > >>>>>>>> } > >>>>>>>> @@ -4898,7 +4898,7 @@ static void gen_slbia(DisasContext *ctx) > >>>>>>>> #if defined(CONFIG_USER_ONLY) > >>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > >>>>>>>> #else > >>>>>>>> - if (unlikely(ctx->pr)) { > >>>>>>>> + if (unlikely(ctx->pr || !ctx->hv)) { > >>>>>>>> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > >>>>>>>> return; > >>>>>>>> } > >>>>>>> > >>>>>>> Unfortunately this patch breaks qemu-system-ppc for both g3beige and > >>>>>>> mac99 under TCG causing a freeze in OpenBIOS when starting > >>>>>>> qemu-system-ppc with no parameters. > >>>>>> > >>>>>> Bother, sorry. > >>>>>> > >>>>>> I think this is because I applied this without the patch that treats > >>>>>> machines with no hypervisor mode (e.g. Apples) as always being in > >>>>>> hypervisor mode. > >>>>> > >>>>> No problem, I can cope for a couple of days or so. > >>>> > >>>> Cédric, > >>>> > >>>> Not sure if you've seen this thread, but one of the HV-mode patches > >>>> caused a regression on Mac. I think it's because I didn't include the > >>>> other patch which treats Apple-mode PPCs as always having HV=1. > >>> > >>> I missed that as I didn't put myself in Cc :/ > >>> > >>>> Can you make sending your updated version of that patch a priority, > >>>> even if the rest of the batch of HV patches isn't ready yet. > >>> > >>> sure. I will/should today or tomorrow. I suppose we want these patches : > >>> > >>> [05/12] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV > >>> http://patchwork.ozlabs.org/patch/618083/ > >>> > >>> [07/12] ppc: Better figure out if processor has HV mode > >>> http://patchwork.ozlabs.org/patch/618089/ > >>> > >>> > >>> Mark, > >>> > >>> I tried to boot a darwinppc-602.iso with : > >>> > >>> qemu-system-ppc -M g3beige -cdrom darwinx86-602.iso -boot d > >>> > >>> but I get a : > >>> > >>> "No valid state has been set by load or ..." > >>> > >>> or we don't need to go further ? may be I need a newer FW. > >> > >> Hmmm that looks like you've got an x86 ISO there which is why > >> OpenBIOS/PPC fails to execute the bootloader. The image I use for > >> testing can be found here: > >> https://opensource.apple.com/static/iso/darwinppc-602.cdr.gz (simply > >> gunzip and then rename to .iso). > > > > Got it. much better with ppc :) ppc is not that omnipotent. > > :) > > >>> Could you try the two patches above please ? They apply on top of Dave's > >>> ppc-for-2.7-20160531 and seem to have a good behavior with the small test > >>> I could do. > >> > >> I'll try and take a look tomorrow, however in the meantime see if the > >> above image enables you to replicate the issue locally. > > > > > > so, on top of ppc-for-2.7-20160531, with your fix for : > > > > ppc: Use split I/D mmu modes to avoid flushes on interrupts > > Unfortunately this isn't really a fix: the whole point of splitting the > MMU modes is to be able to avoid these expensive cache flushes in the > first place. Yeah, the "fix" makes the I/D split patch basically worthless. > Then again it could be that this is exposing an existing > bug elsewhere... I strongly suspect that's the case, we just need to work out what. > > > and these two patches : > > > > [05/12] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV > > http://patchwork.ozlabs.org/patch/618083/ > > > > [07/12] ppc: Better figure out if processor has HV mode > > http://patchwork.ozlabs.org/patch/618089/ > > > > The darwin cd boots correctly up to : > > > > ... > > The following devices are available for installation : > > > > and then loops on something. But I don't get a kernel panic anymore. > > Yes, that effectively matches what I see here - glad that you are now > able to reproduce this. > > > ATB, > > Mark. >
On 02.06.2016 05:17, David Gibson wrote: > On Wed, Jun 01, 2016 at 08:03:08AM +0100, Mark Cave-Ayland wrote: >> On 01/06/16 03:15, David Gibson wrote: >> >>> On Tue, May 31, 2016 at 11:28:49PM +0100, Mark Cave-Ayland wrote: [...] >>>> Note that there is also another regression that has recently landed in >>>> git master so you'll also need to revert >>>> e7c9136977cb99c6eb52c9139f7b8d8b5fa87db9 in order to get back to a >>>> functioning OpenBIOS. >>> >>> I'd preter to see it fixed rather than just reverted.. >> >> Looks like the original author has found the bug, so there should be a >> fix coming up for this soon (I only included it here in case you needed >> an explicit test case). > > Ok. > > So, yeah, I'm not really set up to test Mac machines which means I > don't easily catch regressions like this. > > Mark, > > Could you look into adding a testcase to "make check" that will at > least catch these unsubtle breaks boot type regressions? I think I've just found a nice way to check whether the OpenBIOS machines can at least successfully run through the OpenBIOS boot sequence: You can use the "-prom-env" parameter of QEMU to execute some Forth code there, so this can be used to signal a successful test to the qtest environment. Unless you've got a better test in the works already, I polish up my patch and submit it later today or tomorrow... Thomas
diff --git a/target-ppc/translate.c b/target-ppc/translate.c index dfd3010..690ffd2 100644 --- a/target-ppc/translate.c +++ b/target-ppc/translate.c @@ -4858,7 +4858,7 @@ static void gen_tlbie(DisasContext *ctx) #if defined(CONFIG_USER_ONLY) gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); #else - if (unlikely(ctx->pr)) { + if (unlikely(ctx->pr || !ctx->hv)) { gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); return; } @@ -4879,7 +4879,7 @@ static void gen_tlbsync(DisasContext *ctx) #if defined(CONFIG_USER_ONLY) gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); #else - if (unlikely(ctx->pr)) { + if (unlikely(ctx->pr || !ctx->hv)) { gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); return; } @@ -4898,7 +4898,7 @@ static void gen_slbia(DisasContext *ctx) #if defined(CONFIG_USER_ONLY) gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); #else - if (unlikely(ctx->pr)) { + if (unlikely(ctx->pr || !ctx->hv)) { gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); return; }