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 <b...@kernel.crashing.org> > >>>>>>>> > >>>>>>>> Not that anything remotely recent supports tlbia but ... > >>>>>>>> > >>>>>>>> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org> > >>>>>>>> Signed-off-by: David Gibson <da...@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. > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature