* [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).