[PATCH v2] target/i386: ignore VEX.L when emitting VROUNDSS, VROUNDSD, VMOVSS and VMOVSD

Andrey Polivoda posted 1 patch 2 weeks, 6 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260906045836.1077269-1-apolivodaa433@gmail.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>
target/i386/tcg/decode-new.c.inc | 12 ++++++++++--
target/i386/tcg/emit.c.inc       |  2 --
2 files changed, 10 insertions(+), 4 deletions(-)
[PATCH v2] target/i386: ignore VEX.L when emitting VROUNDSS, VROUNDSD, VMOVSS and VMOVSD
Posted by Andrey Polivoda 2 weeks, 6 days ago
According to both Volume 2 of Intel 64 and IA-32 Architectures Software
Developer's Manual and Volume 4 of AMD64 Architecture Programmer's Manual,
VEX.L is a "don't care" bit for `VROUNDSS`, `VROUNDSD`, `VMOVSS`, `VMOVSD`.

Currently, QEMU handles VEX.L with these instructions differently:
- In the VMOVSS and VMOVSD case, VEX.L allows to select the register size
  (XMM or YMM), leading to differences between QEMU and the real hardware.
- In `gen_VROUNDSS()` and `gen_VROUNDSD()`, there is an assert which checks
  that `s->vex_l` is not set. When either instruction is encountered with VEX.L
  bit set, the QEMU process crashes with an assertion failure.

These behaviors deviate from real hardware, and in VROUNDSS/VROUNDSD case,
it also allows unprivileged guest userspace to crash the QEMU process itself.

This patch fixes this by removing the incorrect asserts and ensuring
that instructions with a `X86_SIZE_ss/sd` operand are always executed
with 128-bit size to match the behavior of the real Intel and AMD hardware.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Fixes: 790684776861 ("target/i386: reimplement 0x0f 0x3a, add AVX")
Signed-off-by: Andrey Polivoda <apolivodaa433@gmail.com>
---
Hardware tested:
Intel Core i3-6100
Intel Xeon Platinum 8370C (GitHub Codespaces 2-core instance)
AMD EPYC 7763 (GitHub Codespaces 4-core instance)

 target/i386/tcg/decode-new.c.inc | 12 ++++++++++--
 target/i386/tcg/emit.c.inc       |  2 --
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
index 459452b04e..f069f3d739 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -2203,8 +2203,16 @@ static bool decode_op_size(DisasContext *s, X86OpEntry *e, X86OpSize size, MemOp
         }
         /* fall through */
     case X86_SIZE_ps: /* SSE/AVX packed single precision */
-    case X86_SIZE_pd: /* SSE/AVX packed double precision */
-        *ot = s->vex_l ? MO_256 : MO_128;
+    case X86_SIZE_pd: { /* SSE/AVX packed double precision */
+            /*
+             * Force 128-bit size for some VEX.LIG scalar instructions
+             * (VROUNDSS, VROUNDSD, VMOVSS, VMOVSD)
+             */
+            bool is_scalar = (e->s0 == X86_SIZE_ss || e->s0 == X86_SIZE_sd ||
+                              e->s1 == X86_SIZE_ss || e->s1 == X86_SIZE_sd ||
+                              e->s2 == X86_SIZE_ss || e->s2 == X86_SIZE_sd);
+            *ot = (s->vex_l && !is_scalar) ? MO_256 : MO_128;
+        }
         return true;
 
     case X86_SIZE_xh: /* SSE/AVX packed half register */
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index c83ab80940..7e0d439f6d 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -4642,14 +4642,12 @@ static void gen_VPHMINPOSUW(DisasContext *s, X86DecodedInsn *decode)
 static void gen_VROUNDSD(DisasContext *s, X86DecodedInsn *decode)
 {
     TCGv_i32 imm = tcg_constant8u_i32(decode->immediate);
-    assert(!s->vex_l);
     gen_helper_roundsd_xmm(tcg_env, OP_PTR0, OP_PTR1, OP_PTR2, imm);
 }
 
 static void gen_VROUNDSS(DisasContext *s, X86DecodedInsn *decode)
 {
     TCGv_i32 imm = tcg_constant8u_i32(decode->immediate);
-    assert(!s->vex_l);
     gen_helper_roundss_xmm(tcg_env, OP_PTR0, OP_PTR1, OP_PTR2, imm);
 }
 
-- 
2.53.0