diff mbox series

target/ppc: Fix lxv/stxv MSR facility check

Message ID 20240213083933.718881-1-npiggin@gmail.com (mailing list archive)
State New, archived
Headers show
Series target/ppc: Fix lxv/stxv MSR facility check | expand

Commit Message

Nicholas Piggin Feb. 13, 2024, 8:39 a.m. UTC
The move to decodetree flipped the inequality test for the VEC / VSX
MSR facility check.

This caused application crashes under Linux, where these facility
unavailable interrupts are used for lazy-switching of VEC/VSX register
sets. Getting the incorrect interrupt would result in wrong registers
being loaded, potentially overwriting live values and/or exposing
stale ones.

Cc: qemu-stable@nongnu.org
Reported-by: Joel Stanley <joel@jms.id.au>
Fixes: 70426b5bb738 ("target/ppc: moved stxvx and lxvx from legacy to decodtree")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1769
Tested-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/translate/vsx-impl.c.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Harsh Prateek Bora Feb. 13, 2024, 9:18 a.m. UTC | #1
On 2/13/24 14:09, Nicholas Piggin wrote:
> The move to decodetree flipped the inequality test for the VEC / VSX
> MSR facility check.
> 
> This caused application crashes under Linux, where these facility
> unavailable interrupts are used for lazy-switching of VEC/VSX register
> sets. Getting the incorrect interrupt would result in wrong registers
> being loaded, potentially overwriting live values and/or exposing
> stale ones.
> 
> Cc: qemu-stable@nongnu.org
> Reported-by: Joel Stanley <joel@jms.id.au>
> Fixes: 70426b5bb738 ("target/ppc: moved stxvx and lxvx from legacy to decodtree")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1769
> Tested-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   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 6db87ab336..0266f09119 100644
> --- a/target/ppc/translate/vsx-impl.c.inc
> +++ b/target/ppc/translate/vsx-impl.c.inc
> @@ -2268,7 +2268,7 @@ static bool do_lstxv(DisasContext *ctx, int ra, TCGv displ,
>   
>   static bool do_lstxv_D(DisasContext *ctx, arg_D *a, bool store, bool paired)
>   {
> -    if (paired || a->rt >= 32) {
> +    if (paired || a->rt < 32) {

Thanks for catching this,

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

>           REQUIRE_VSX(ctx);
>       } else {
>           REQUIRE_VECTOR(ctx);
Cédric Le Goater Feb. 14, 2024, 7:35 a.m. UTC | #2
On 2/13/24 09:39, Nicholas Piggin wrote:
> The move to decodetree flipped the inequality test for the VEC / VSX
> MSR facility check.
> 
> This caused application crashes under Linux, where these facility
> unavailable interrupts are used for lazy-switching of VEC/VSX register
> sets. Getting the incorrect interrupt would result in wrong registers
> being loaded, potentially overwriting live values and/or exposing
> stale ones.
> 
> Cc: qemu-stable@nongnu.org
> Reported-by: Joel Stanley <joel@jms.id.au>
> Fixes: 70426b5bb738 ("target/ppc: moved stxvx and lxvx from legacy to decodtree")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1769
> Tested-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>
Tested-by: Cédric Le Goater <clg@kaod.org>

with a RHEL9 image.

Thanks,

C.


> ---
>   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 6db87ab336..0266f09119 100644
> --- a/target/ppc/translate/vsx-impl.c.inc
> +++ b/target/ppc/translate/vsx-impl.c.inc
> @@ -2268,7 +2268,7 @@ static bool do_lstxv(DisasContext *ctx, int ra, TCGv displ,
>   
>   static bool do_lstxv_D(DisasContext *ctx, arg_D *a, bool store, bool paired)
>   {
> -    if (paired || a->rt >= 32) {
> +    if (paired || a->rt < 32) {
>           REQUIRE_VSX(ctx);
>       } else {
>           REQUIRE_VECTOR(ctx);
diff mbox series

Patch

diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
index 6db87ab336..0266f09119 100644
--- a/target/ppc/translate/vsx-impl.c.inc
+++ b/target/ppc/translate/vsx-impl.c.inc
@@ -2268,7 +2268,7 @@  static bool do_lstxv(DisasContext *ctx, int ra, TCGv displ,
 
 static bool do_lstxv_D(DisasContext *ctx, arg_D *a, bool store, bool paired)
 {
-    if (paired || a->rt >= 32) {
+    if (paired || a->rt < 32) {
         REQUIRE_VSX(ctx);
     } else {
         REQUIRE_VECTOR(ctx);