1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH 00/19] ppc: support for the baremetal XIVE interrupt controller (POWER9)
  4. View patch

[Qemu-devel] [PATCH 07/19] target/ppc: Make special ORs match x86 pause and don't generate on mttcg

Cédric Le Goater posted 19 patches 6 years, 9 months ago
[Qemu-devel] [PATCH 07/19] target/ppc: Make special ORs match x86 pause and don't generate on mttcg
Posted by Cédric Le Goater 6 years, 9 months ago 
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

There's no point in going out of translation on an SMT OR with
mttcg since the backend won't do anything useful such as pausing,
it's only useful on traditional TCG to give time to other processors.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/translate.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index e169c43643a1..7d40a1fbe6bd 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1580,7 +1580,7 @@ static void gen_pause(DisasContext *ctx)
     tcg_temp_free_i32(t0);
 
     /* Stop translation, this gives other CPUs a chance to run */
-    gen_exception_nip(ctx, EXCP_HLT, ctx->base.pc_next);
+    gen_exception_nip(ctx, EXCP_INTERRUPT, ctx->base.pc_next);
 }
 #endif /* defined(TARGET_PPC64) */
 
@@ -1662,7 +1662,9 @@ static void gen_or(DisasContext *ctx)
          * than no-op, e.g., miso(rs=26), yield(27), mdoio(29), mdoom(30),
          * and all currently undefined.
          */
-        gen_pause(ctx);
+        if (!mttcg_enabled) {
+            gen_pause(ctx);
+        }
 #endif
 #endif
     }
-- 
2.20.1
Re: [Qemu-devel] [PATCH 07/19] target/ppc: Make special ORs match x86 pause and don't generate on mttcg
Posted by David Gibson 6 years, 8 months ago 
On Mon, Jan 28, 2019 at 10:46:13AM +0100, Cédric Le Goater wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> There's no point in going out of translation on an SMT OR with
> mttcg since the backend won't do anything useful such as pausing,
> it's only useful on traditional TCG to give time to other
> processors.

Is it actively harmful in the MTTCG case, or just pointless?

> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  target/ppc/translate.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index e169c43643a1..7d40a1fbe6bd 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -1580,7 +1580,7 @@ static void gen_pause(DisasContext *ctx)
>      tcg_temp_free_i32(t0);
>  
>      /* Stop translation, this gives other CPUs a chance to run */
> -    gen_exception_nip(ctx, EXCP_HLT, ctx->base.pc_next);
> +    gen_exception_nip(ctx, EXCP_INTERRUPT, ctx->base.pc_next);

I don't see how this change relates to the rest.

>  }
>  #endif /* defined(TARGET_PPC64) */
>  
> @@ -1662,7 +1662,9 @@ static void gen_or(DisasContext *ctx)
>           * than no-op, e.g., miso(rs=26), yield(27), mdoio(29), mdoom(30),
>           * and all currently undefined.
>           */
> -        gen_pause(ctx);
> +        if (!mttcg_enabled) {
> +            gen_pause(ctx);
> +        }
>  #endif
>  #endif
>      }

-- 
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
Re: [Qemu-devel] [PATCH 07/19] target/ppc: Make special ORs match x86 pause and don't generate on mttcg
Posted by Benjamin Herrenschmidt 6 years, 8 months ago 
On Tue, 2019-02-12 at 16:59 +1100, David Gibson wrote:
> On Mon, Jan 28, 2019 at 10:46:13AM +0100, Cédric Le Goater wrote:
> > From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > 
> > There's no point in going out of translation on an SMT OR with
> > mttcg since the backend won't do anything useful such as pausing,
> > it's only useful on traditional TCG to give time to other
> > processors.
> 
> Is it actively harmful in the MTTCG case, or just pointless?

I think it can hurt performance, I don't remember for sure :)

> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
> >  target/ppc/translate.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> > index e169c43643a1..7d40a1fbe6bd 100644
> > --- a/target/ppc/translate.c
> > +++ b/target/ppc/translate.c
> > @@ -1580,7 +1580,7 @@ static void gen_pause(DisasContext *ctx)
> >      tcg_temp_free_i32(t0);
> >  
> >      /* Stop translation, this gives other CPUs a chance to run */
> > -    gen_exception_nip(ctx, EXCP_HLT, ctx->base.pc_next);
> > +    gen_exception_nip(ctx, EXCP_INTERRUPT, ctx->base.pc_next);
> 
> I don't see how this change relates to the rest.

Yeah not sure anymore :-)

> >  }
> >  #endif /* defined(TARGET_PPC64) */
> >  
> > @@ -1662,7 +1662,9 @@ static void gen_or(DisasContext *ctx)
> >           * than no-op, e.g., miso(rs=26), yield(27), mdoio(29), mdoom(30),
> >           * and all currently undefined.
> >           */
> > -        gen_pause(ctx);
> > +        if (!mttcg_enabled) {
> > +            gen_pause(ctx);
> > +        }
> >  #endif
> >  #endif
> >      }
Re: [Qemu-devel] [PATCH 07/19] target/ppc: Make special ORs match x86 pause and don't generate on mttcg
Posted by David Gibson 6 years, 8 months ago 
On Wed, Feb 13, 2019 at 11:03:12AM +1100, Benjamin Herrenschmidt wrote:
> On Tue, 2019-02-12 at 16:59 +1100, David Gibson wrote:
> > On Mon, Jan 28, 2019 at 10:46:13AM +0100, Cédric Le Goater wrote:
> > > From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > 
> > > There's no point in going out of translation on an SMT OR with
> > > mttcg since the backend won't do anything useful such as pausing,
> > > it's only useful on traditional TCG to give time to other
> > > processors.
> > 
> > Is it actively harmful in the MTTCG case, or just pointless?
> 
> I think it can hurt performance, I don't remember for sure :)
> 
> > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > ---
> > >  target/ppc/translate.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> > > index e169c43643a1..7d40a1fbe6bd 100644
> > > --- a/target/ppc/translate.c
> > > +++ b/target/ppc/translate.c
> > > @@ -1580,7 +1580,7 @@ static void gen_pause(DisasContext *ctx)
> > >      tcg_temp_free_i32(t0);
> > >  
> > >      /* Stop translation, this gives other CPUs a chance to run */
> > > -    gen_exception_nip(ctx, EXCP_HLT, ctx->base.pc_next);
> > > +    gen_exception_nip(ctx, EXCP_INTERRUPT, ctx->base.pc_next);
> > 
> > I don't see how this change relates to the rest.
> 
> Yeah not sure anymore :-)

Oh.  That certainly doesn't make this easier to review.

So, all these target/ppc patches are only indirectly related to XIVE
pnv support.  Cédric, can you split them out into their own series on
the next spin.

> 
> > >  }
> > >  #endif /* defined(TARGET_PPC64) */
> > >  
> > > @@ -1662,7 +1662,9 @@ static void gen_or(DisasContext *ctx)
> > >           * than no-op, e.g., miso(rs=26), yield(27), mdoio(29), mdoom(30),
> > >           * and all currently undefined.
> > >           */
> > > -        gen_pause(ctx);
> > > +        if (!mttcg_enabled) {
> > > +            gen_pause(ctx);
> > > +        }
> > >  #endif
> > >  #endif
> > >      }
> 

-- 
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
Re: [Qemu-devel] [PATCH 07/19] target/ppc: Make special ORs match x86 pause and don't generate on mttcg
Posted by Cédric Le Goater 6 years, 8 months ago 
On 2/13/19 5:54 AM, David Gibson wrote:
> On Wed, Feb 13, 2019 at 11:03:12AM +1100, Benjamin Herrenschmidt wrote:
>> On Tue, 2019-02-12 at 16:59 +1100, David Gibson wrote:
>>> On Mon, Jan 28, 2019 at 10:46:13AM +0100, Cédric Le Goater wrote:
>>>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>>>
>>>> There's no point in going out of translation on an SMT OR with
>>>> mttcg since the backend won't do anything useful such as pausing,
>>>> it's only useful on traditional TCG to give time to other
>>>> processors.
>>>
>>> Is it actively harmful in the MTTCG case, or just pointless?
>>
>> I think it can hurt performance, I don't remember for sure :)
>>
>>>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> ---
>>>>  target/ppc/translate.c | 6 ++++--
>>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>>>> index e169c43643a1..7d40a1fbe6bd 100644
>>>> --- a/target/ppc/translate.c
>>>> +++ b/target/ppc/translate.c
>>>> @@ -1580,7 +1580,7 @@ static void gen_pause(DisasContext *ctx)
>>>>      tcg_temp_free_i32(t0);
>>>>  
>>>>      /* Stop translation, this gives other CPUs a chance to run */
>>>> -    gen_exception_nip(ctx, EXCP_HLT, ctx->base.pc_next);
>>>> +    gen_exception_nip(ctx, EXCP_INTERRUPT, ctx->base.pc_next);
>>>
>>> I don't see how this change relates to the rest.
>>
>> Yeah not sure anymore :-)
> 
> Oh.  That certainly doesn't make this easier to review.
> 
> So, all these target/ppc patches are only indirectly related to XIVE
> pnv support.  Cédric, can you split them out into their own series on
> the next spin.

Sure. I will address your comments and resend them first.
 
Thanks,

C.

> 
>>
>>>>  }
>>>>  #endif /* defined(TARGET_PPC64) */
>>>>  
>>>> @@ -1662,7 +1662,9 @@ static void gen_or(DisasContext *ctx)
>>>>           * than no-op, e.g., miso(rs=26), yield(27), mdoio(29), mdoom(30),
>>>>           * and all currently undefined.
>>>>           */
>>>> -        gen_pause(ctx);
>>>> +        if (!mttcg_enabled) {
>>>> +            gen_pause(ctx);
>>>> +        }
>>>>  #endif
>>>>  #endif
>>>>      }
>>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.