* [PATCH] target/ppc: Fix lxvx/stxvx facility check
@ 2024-09-11 14:16 Fabiano Rosas
2024-09-11 16:19 ` Claudio Fontana
0 siblings, 1 reply; 6+ messages in thread
From: Fabiano Rosas @ 2024-09-11 14:16 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Nicholas Piggin, Daniel Henrique Barboza
The XT check for the lxvx/stxvx instructions is currently
inverted. This was introduced during the move to decodetree.
From the ISA:
Chapter 7. Vector-Scalar Extension Facility
Load VSX Vector Indexed X-form
lxvx XT,RA,RB
if TX=0 & MSR.VSX=0 then VSX_Unavailable()
if TX=1 & MSR.VEC=0 then Vector_Unavailable()
...
Let XT be the value 32×TX + T.
The code currently does the opposite:
if (paired || a->rt >= 32) {
REQUIRE_VSX(ctx);
} else {
REQUIRE_VECTOR(ctx);
}
This was already fixed for lxv/stxv at commit "2cc0e449d1 (target/ppc:
Fix lxv/stxv MSR facility check)", but the indexed forms were missed.
Cc: qemu-stable@nongnu.org
Fixes: 70426b5bb7 ("target/ppc: moved stxvx and lxvx from legacy to decodtree")
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
target/ppc/translate/vsx-impl.c.inc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
index 40a87ddc4a..a869f30e86 100644
--- a/target/ppc/translate/vsx-impl.c.inc
+++ b/target/ppc/translate/vsx-impl.c.inc
@@ -2244,7 +2244,7 @@ static bool do_lstxv_PLS_D(DisasContext *ctx, arg_PLS_D *a,
static bool do_lstxv_X(DisasContext *ctx, arg_X *a, bool store, bool paired)
{
- if (paired || a->rt >= 32) {
+ if (paired || a->rt < 32) {
REQUIRE_VSX(ctx);
} else {
REQUIRE_VECTOR(ctx);
--
2.35.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] target/ppc: Fix lxvx/stxvx facility check 2024-09-11 14:16 [PATCH] target/ppc: Fix lxvx/stxvx facility check Fabiano Rosas @ 2024-09-11 16:19 ` Claudio Fontana 2024-09-18 15:11 ` Claudio Fontana 0 siblings, 1 reply; 6+ messages in thread From: Claudio Fontana @ 2024-09-11 16:19 UTC (permalink / raw) To: Fabiano Rosas, qemu-devel Cc: qemu-stable, Nicholas Piggin, Daniel Henrique Barboza On 9/11/24 16:16, Fabiano Rosas wrote: > The XT check for the lxvx/stxvx instructions is currently > inverted. This was introduced during the move to decodetree. > > From the ISA: > Chapter 7. Vector-Scalar Extension Facility > Load VSX Vector Indexed X-form > > lxvx XT,RA,RB > if TX=0 & MSR.VSX=0 then VSX_Unavailable() > if TX=1 & MSR.VEC=0 then Vector_Unavailable() > ... > Let XT be the value 32×TX + T. > > The code currently does the opposite: > > if (paired || a->rt >= 32) { > REQUIRE_VSX(ctx); > } else { > REQUIRE_VECTOR(ctx); > } > > This was already fixed for lxv/stxv at commit "2cc0e449d1 (target/ppc: > Fix lxv/stxv MSR facility check)", but the indexed forms were missed. > > Cc: qemu-stable@nongnu.org > Fixes: 70426b5bb7 ("target/ppc: moved stxvx and lxvx from legacy to decodtree") > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > target/ppc/translate/vsx-impl.c.inc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc > index 40a87ddc4a..a869f30e86 100644 > --- a/target/ppc/translate/vsx-impl.c.inc > +++ b/target/ppc/translate/vsx-impl.c.inc > @@ -2244,7 +2244,7 @@ static bool do_lstxv_PLS_D(DisasContext *ctx, arg_PLS_D *a, > > static bool do_lstxv_X(DisasContext *ctx, arg_X *a, bool store, bool paired) > { > - if (paired || a->rt >= 32) { > + if (paired || a->rt < 32) { > REQUIRE_VSX(ctx); > } else { > REQUIRE_VECTOR(ctx); Reviewed-by: Claudio Fontana <cfontana@suse.de> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/ppc: Fix lxvx/stxvx facility check 2024-09-11 16:19 ` Claudio Fontana @ 2024-09-18 15:11 ` Claudio Fontana 2024-09-19 11:36 ` Claudio Fontana 0 siblings, 1 reply; 6+ messages in thread From: Claudio Fontana @ 2024-09-18 15:11 UTC (permalink / raw) To: Fabiano Rosas, qemu-devel, Ilya Leoshkevich Cc: qemu-stable, Nicholas Piggin, Daniel Henrique Barboza Adding Ilya FYI. Ciao, Claudio On 9/11/24 18:19, Claudio Fontana wrote: > On 9/11/24 16:16, Fabiano Rosas wrote: >> The XT check for the lxvx/stxvx instructions is currently >> inverted. This was introduced during the move to decodetree. >> >> From the ISA: >> Chapter 7. Vector-Scalar Extension Facility >> Load VSX Vector Indexed X-form >> >> lxvx XT,RA,RB >> if TX=0 & MSR.VSX=0 then VSX_Unavailable() >> if TX=1 & MSR.VEC=0 then Vector_Unavailable() >> ... >> Let XT be the value 32×TX + T. >> >> The code currently does the opposite: >> >> if (paired || a->rt >= 32) { >> REQUIRE_VSX(ctx); >> } else { >> REQUIRE_VECTOR(ctx); >> } >> >> This was already fixed for lxv/stxv at commit "2cc0e449d1 (target/ppc: >> Fix lxv/stxv MSR facility check)", but the indexed forms were missed. >> >> Cc: qemu-stable@nongnu.org >> Fixes: 70426b5bb7 ("target/ppc: moved stxvx and lxvx from legacy to decodtree") >> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> --- >> target/ppc/translate/vsx-impl.c.inc | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc >> index 40a87ddc4a..a869f30e86 100644 >> --- a/target/ppc/translate/vsx-impl.c.inc >> +++ b/target/ppc/translate/vsx-impl.c.inc >> @@ -2244,7 +2244,7 @@ static bool do_lstxv_PLS_D(DisasContext *ctx, arg_PLS_D *a, >> >> static bool do_lstxv_X(DisasContext *ctx, arg_X *a, bool store, bool paired) >> { >> - if (paired || a->rt >= 32) { >> + if (paired || a->rt < 32) { >> REQUIRE_VSX(ctx); >> } else { >> REQUIRE_VECTOR(ctx); > > Reviewed-by: Claudio Fontana <cfontana@suse.de> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/ppc: Fix lxvx/stxvx facility check 2024-09-18 15:11 ` Claudio Fontana @ 2024-09-19 11:36 ` Claudio Fontana 2024-09-20 9:24 ` Ilya Leoshkevich 0 siblings, 1 reply; 6+ messages in thread From: Claudio Fontana @ 2024-09-19 11:36 UTC (permalink / raw) To: Fabiano Rosas, qemu-devel, Ilya Leoshkevich, Richard Henderson Cc: qemu-stable, Nicholas Piggin, Daniel Henrique Barboza ping, adding Richard. We will need to include this downstream because of the breakage its lack causes. It is already reviewed by me, but some TCG maintainer indicating it will be included in some queue would help, Thanks, Claudio On 9/18/24 17:11, Claudio Fontana wrote: > Adding Ilya FYI. > > Ciao, > > Claudio > > On 9/11/24 18:19, Claudio Fontana wrote: >> On 9/11/24 16:16, Fabiano Rosas wrote: >>> The XT check for the lxvx/stxvx instructions is currently >>> inverted. This was introduced during the move to decodetree. >>> >>> From the ISA: >>> Chapter 7. Vector-Scalar Extension Facility >>> Load VSX Vector Indexed X-form >>> >>> lxvx XT,RA,RB >>> if TX=0 & MSR.VSX=0 then VSX_Unavailable() >>> if TX=1 & MSR.VEC=0 then Vector_Unavailable() >>> ... >>> Let XT be the value 32×TX + T. >>> >>> The code currently does the opposite: >>> >>> if (paired || a->rt >= 32) { >>> REQUIRE_VSX(ctx); >>> } else { >>> REQUIRE_VECTOR(ctx); >>> } >>> >>> This was already fixed for lxv/stxv at commit "2cc0e449d1 (target/ppc: >>> Fix lxv/stxv MSR facility check)", but the indexed forms were missed. >>> >>> Cc: qemu-stable@nongnu.org >>> Fixes: 70426b5bb7 ("target/ppc: moved stxvx and lxvx from legacy to decodtree") >>> Signed-off-by: Fabiano Rosas <farosas@suse.de> >>> --- >>> target/ppc/translate/vsx-impl.c.inc | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc >>> index 40a87ddc4a..a869f30e86 100644 >>> --- a/target/ppc/translate/vsx-impl.c.inc >>> +++ b/target/ppc/translate/vsx-impl.c.inc >>> @@ -2244,7 +2244,7 @@ static bool do_lstxv_PLS_D(DisasContext *ctx, arg_PLS_D *a, >>> >>> static bool do_lstxv_X(DisasContext *ctx, arg_X *a, bool store, bool paired) >>> { >>> - if (paired || a->rt >= 32) { >>> + if (paired || a->rt < 32) { >>> REQUIRE_VSX(ctx); >>> } else { >>> REQUIRE_VECTOR(ctx); >> >> Reviewed-by: Claudio Fontana <cfontana@suse.de> >> > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/ppc: Fix lxvx/stxvx facility check 2024-09-19 11:36 ` Claudio Fontana @ 2024-09-20 9:24 ` Ilya Leoshkevich 2024-09-22 4:56 ` Richard Henderson 0 siblings, 1 reply; 6+ messages in thread From: Ilya Leoshkevich @ 2024-09-20 9:24 UTC (permalink / raw) To: Claudio Fontana, Fabiano Rosas, qemu-devel, Richard Henderson Cc: qemu-stable, Nicholas Piggin, Daniel Henrique Barboza On Thu, 2024-09-19 at 13:36 +0200, Claudio Fontana wrote: > ping, adding Richard. > > We will need to include this downstream because of the breakage its > lack causes. > It is already reviewed by me, but some TCG maintainer indicating it > will be included in some queue would help, > > Thanks, > > Claudio > > On 9/18/24 17:11, Claudio Fontana wrote: > > Adding Ilya FYI. > > > > Ciao, > > > > Claudio > > > > On 9/11/24 18:19, Claudio Fontana wrote: > > > On 9/11/24 16:16, Fabiano Rosas wrote: > > > > The XT check for the lxvx/stxvx instructions is currently > > > > inverted. This was introduced during the move to decodetree. > > > > > > > > From the ISA: > > > > Chapter 7. Vector-Scalar Extension Facility > > > > Load VSX Vector Indexed X-form > > > > > > > > lxvx XT,RA,RB > > > > if TX=0 & MSR.VSX=0 then VSX_Unavailable() > > > > if TX=1 & MSR.VEC=0 then Vector_Unavailable() > > > > ... > > > > Let XT be the value 32×TX + T. > > > > > > > > The code currently does the opposite: > > > > > > > > if (paired || a->rt >= 32) { > > > > REQUIRE_VSX(ctx); > > > > } else { > > > > REQUIRE_VECTOR(ctx); > > > > } > > > > > > > > This was already fixed for lxv/stxv at commit "2cc0e449d1 > > > > (target/ppc: > > > > Fix lxv/stxv MSR facility check)", but the indexed forms were > > > > missed. > > > > > > > > Cc: qemu-stable@nongnu.org > > > > Fixes: 70426b5bb7 ("target/ppc: moved stxvx and lxvx from > > > > legacy to decodtree") > > > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > > > > --- > > > > target/ppc/translate/vsx-impl.c.inc | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/target/ppc/translate/vsx-impl.c.inc > > > > b/target/ppc/translate/vsx-impl.c.inc > > > > index 40a87ddc4a..a869f30e86 100644 > > > > --- a/target/ppc/translate/vsx-impl.c.inc > > > > +++ b/target/ppc/translate/vsx-impl.c.inc > > > > @@ -2244,7 +2244,7 @@ static bool do_lstxv_PLS_D(DisasContext > > > > *ctx, arg_PLS_D *a, > > > > > > > > static bool do_lstxv_X(DisasContext *ctx, arg_X *a, bool > > > > store, bool paired) > > > > { > > > > - if (paired || a->rt >= 32) { > > > > + if (paired || a->rt < 32) { > > > > REQUIRE_VSX(ctx); > > > > } else { > > > > REQUIRE_VECTOR(ctx); > > > > > > Reviewed-by: Claudio Fontana <cfontana@suse.de> FWIW Acked-by: Ilya Leoshkevich <iii@linux.ibm.com> But I'm not a maintainer, I guess Richard will need to pick it up. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/ppc: Fix lxvx/stxvx facility check 2024-09-20 9:24 ` Ilya Leoshkevich @ 2024-09-22 4:56 ` Richard Henderson 0 siblings, 0 replies; 6+ messages in thread From: Richard Henderson @ 2024-09-22 4:56 UTC (permalink / raw) To: Ilya Leoshkevich, Claudio Fontana, Fabiano Rosas, qemu-devel Cc: qemu-stable, Nicholas Piggin, Daniel Henrique Barboza On 9/20/24 11:24, Ilya Leoshkevich wrote: > On Thu, 2024-09-19 at 13:36 +0200, Claudio Fontana wrote: >> ping, adding Richard. >> >> We will need to include this downstream because of the breakage its >> lack causes. >> It is already reviewed by me, but some TCG maintainer indicating it >> will be included in some queue would help, >> >> Thanks, >> >> Claudio >> >> On 9/18/24 17:11, Claudio Fontana wrote: >>> Adding Ilya FYI. >>> >>> Ciao, >>> >>> Claudio >>> >>> On 9/11/24 18:19, Claudio Fontana wrote: >>>> On 9/11/24 16:16, Fabiano Rosas wrote: >>>>> The XT check for the lxvx/stxvx instructions is currently >>>>> inverted. This was introduced during the move to decodetree. >>>>> >>>>> From the ISA: >>>>> Chapter 7. Vector-Scalar Extension Facility >>>>> Load VSX Vector Indexed X-form >>>>> >>>>> lxvx XT,RA,RB >>>>> if TX=0 & MSR.VSX=0 then VSX_Unavailable() >>>>> if TX=1 & MSR.VEC=0 then Vector_Unavailable() >>>>> ... >>>>> Let XT be the value 32×TX + T. >>>>> >>>>> The code currently does the opposite: >>>>> >>>>> if (paired || a->rt >= 32) { >>>>> REQUIRE_VSX(ctx); >>>>> } else { >>>>> REQUIRE_VECTOR(ctx); >>>>> } >>>>> >>>>> This was already fixed for lxv/stxv at commit "2cc0e449d1 >>>>> (target/ppc: >>>>> Fix lxv/stxv MSR facility check)", but the indexed forms were >>>>> missed. >>>>> >>>>> Cc: qemu-stable@nongnu.org >>>>> Fixes: 70426b5bb7 ("target/ppc: moved stxvx and lxvx from >>>>> legacy to decodtree") >>>>> Signed-off-by: Fabiano Rosas <farosas@suse.de> >>>>> --- >>>>> target/ppc/translate/vsx-impl.c.inc | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/target/ppc/translate/vsx-impl.c.inc >>>>> b/target/ppc/translate/vsx-impl.c.inc >>>>> index 40a87ddc4a..a869f30e86 100644 >>>>> --- a/target/ppc/translate/vsx-impl.c.inc >>>>> +++ b/target/ppc/translate/vsx-impl.c.inc >>>>> @@ -2244,7 +2244,7 @@ static bool do_lstxv_PLS_D(DisasContext >>>>> *ctx, arg_PLS_D *a, >>>>> >>>>> static bool do_lstxv_X(DisasContext *ctx, arg_X *a, bool >>>>> store, bool paired) >>>>> { >>>>> - if (paired || a->rt >= 32) { >>>>> + if (paired || a->rt < 32) { >>>>> REQUIRE_VSX(ctx); >>>>> } else { >>>>> REQUIRE_VECTOR(ctx); >>>> >>>> Reviewed-by: Claudio Fontana <cfontana@suse.de> > > FWIW > > Acked-by: Ilya Leoshkevich <iii@linux.ibm.com> > > But I'm not a maintainer, I guess Richard will need to pick it up. Queued, thanks. r~ ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-22 4:57 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-11 14:16 [PATCH] target/ppc: Fix lxvx/stxvx facility check Fabiano Rosas 2024-09-11 16:19 ` Claudio Fontana 2024-09-18 15:11 ` Claudio Fontana 2024-09-19 11:36 ` Claudio Fontana 2024-09-20 9:24 ` Ilya Leoshkevich 2024-09-22 4:56 ` Richard Henderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).