| Message ID | 20240911141651.6914-1-farosas@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/ppc: Fix lxvx/stxvx facility check | expand |
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>
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> >
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> >> > >
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.
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~
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);
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(-)