[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-ppc] [PULL 04/12] ppc: tlbie, tlbia and tlbisync
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [Qemu-ppc] [PULL 04/12] ppc: tlbie, tlbia and tlbisync are HV only |
Date: |
Fri, 3 Jun 2016 17:12:10 +1000 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
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 <address@hidden>
> >>>>>>>>
> >>>>>>>> Not that anything remotely recent supports tlbia but ...
> >>>>>>>>
> >>>>>>>> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
> >>>>>>>> Signed-off-by: David Gibson <address@hidden>
> >>>>>>>> ---
> >>>>>>>> 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
- Re: [Qemu-devel] [Qemu-ppc] [PULL 04/12] ppc: tlbie, tlbia and tlbisync are HV only, Mark Cave-Ayland, 2016/06/01
- Re: [Qemu-devel] [Qemu-ppc] [PULL 04/12] ppc: tlbie, tlbia and tlbisync are HV only, David Gibson, 2016/06/01
- Re: [Qemu-devel] [Qemu-ppc] [PULL 04/12] ppc: tlbie, tlbia and tlbisync are HV only, Cédric Le Goater, 2016/06/02
- Re: [Qemu-devel] [Qemu-ppc] [PULL 04/12] ppc: tlbie, tlbia and tlbisync are HV only, Mark Cave-Ayland, 2016/06/02
- Re: [Qemu-devel] [Qemu-ppc] [PULL 04/12] ppc: tlbie, tlbia and tlbisync are HV only, Cédric Le Goater, 2016/06/02
- Re: [Qemu-devel] [Qemu-ppc] [PULL 04/12] ppc: tlbie, tlbia and tlbisync are HV only, Mark Cave-Ayland, 2016/06/02
- Re: [Qemu-devel] [Qemu-ppc] [PULL 04/12] ppc: tlbie, tlbia and tlbisync are HV only, Mark Cave-Ayland, 2016/06/02
- Re: [Qemu-devel] [Qemu-ppc] [PULL 04/12] ppc: tlbie, tlbia and tlbisync are HV only, Cédric Le Goater, 2016/06/02
- Re: [Qemu-devel] [Qemu-ppc] [PULL 04/12] ppc: tlbie, tlbia and tlbisync are HV only,
David Gibson <=
Re: [Qemu-devel] [Qemu-ppc] [PULL 04/12] ppc: tlbie, tlbia and tlbisync are HV only, Thomas Huth, 2016/06/14