1. Patchew
  2. QEMU
  3. View series

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

Andrey Polivoda posted 1 patch 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260905014629.991586-1-apolivodaa433@gmail.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>
target/i386/tcg/decode-new.c.inc | 8 ++++++--
target/i386/tcg/emit.c.inc       | 2 --
2 files changed, 6 insertions(+), 4 deletions(-)
[PATCH] target/i386: ignore VEX.L when emitting VROUNDSS and VROUNDSD
Posted by Andrey Polivoda 3 weeks 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` and `VROUNDSD`.

Currently, QEMU has an assertion in `gen_VROUNDSS()` and `gen_VROUNDSD()`,
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.

Not only does this behavior deviate from real hardware, but it also
allows unprivileged guest userspace to crash the QEMU process itself.

This patch fixes this bug by removing the incorrect assertions and ensuring
that `VROUNDSS` and `VROUNDSD` 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 | 8 ++++++--
 target/i386/tcg/emit.c.inc       | 2 --
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
index 459452b04e..14abc898fb 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -2203,8 +2203,12 @@ 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 */
+            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
Re: [PATCH] target/i386: ignore VEX.L when emitting VROUNDSS and VROUNDSD
Posted by Paolo Bonzini 3 weeks ago 
Il sab 5 set 2026, 03:46 Andrey Polivoda <apolivodaa433@gmail.com> ha
scritto:

> 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` and `VROUNDSD`.
>
> Currently, QEMU has an assertion in `gen_VROUNDSS()` and `gen_VROUNDSD()`,
> 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.
>
> Not only does this behavior deviate from real hardware, but it also
> allows unprivileged guest userspace to crash the QEMU process itself.
>

Good catch - though, note that TCG is not considered a security boundary.


>      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 */
> +            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;
> +        }
>

Oh, this also applies to VMOVSS, VSQRTSS, etc. but that's actually correct
- they're all VEX.LIG. It's worth listing these instructions above the
declaration of is_scalar.

Paolo

         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
>
>
Re: [PATCH] target/i386: ignore VEX.L when emitting VROUNDSS and VROUNDSD
Posted by Andrey Polivoda 2 weeks, 6 days ago 
>>
>>      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 */
>> +            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;
>> +        }
>
>
> Oh, this also applies to VMOVSS, VSQRTSS, etc. but that's actually correct - they're all VEX.LIG. It's worth listing these instructions above the declaration of is_scalar.
>
> Paolo

Checked which instructions are actually affected by this patch:
```
$ grep -Rn ',\s*s[sd]' | grep OP | grep -E ',\s*(x|ps|pd)'
tcg/decode-new.c.inc:999:    [0x0a] = X86_OP_ENTRY4(VROUNDSS,   V,x,
H,x, W,ss, vex3 cpuid(SSE41) p_66),
tcg/decode-new.c.inc:1000:    [0x0b] = X86_OP_ENTRY4(VROUNDSD,   V,x,
H,x, W,sd, vex3 cpuid(SSE41) p_66),
tcg/decode-new.c.inc:1056:        X86_OP_ENTRY3(VMOVSS_ld,  V,x,  H,x,
      M,ss, vex5),
tcg/decode-new.c.inc:1057:        X86_OP_ENTRY3(VMOVSD_ld,  V,x,  H,x,
      M,sd, vex5),
tcg/decode-new.c.inc:1079:        X86_OP_ENTRY3(VMOVSS_st,  M,ss,
None,None, V,x, vex5),
tcg/decode-new.c.inc:1080:        X86_OP_ENTRY3(VMOVLPx_st, M,sd,
None,None, V,x, vex5), /* MOVSD */
tcg/decode-new.c.inc:1163:        X86_OP_ENTRY3(VMOVSS_st,  M,ss,
None,None, V,x, vex4 cpuid(SSE4A)), /* MOVNTSS */
tcg/decode-new.c.inc:1164:        X86_OP_ENTRY3(VMOVLPx_st, M,sd,
None,None, V,x, vex4 cpuid(SSE4A)), /* MOVNTSD */
$
```
After running some tests, it looks like the behavior of (V)ROUNDSS,
(V)ROUNDSD, (V)MOVSS and (V)MOVSD is actually modified by this patch.
(MOVNTSS and MOVNTSD do not generate different TCG ops with and
without the patch applied. However, the VEX prefix
is actually illegal for these two AMD-specific instructions (tested on
AMD EPYC 7763; material for a different patch, I guess))

Also, VSQRTSS and VSQRTSD are actually raising an invalid opcode
exception when VEX.L is set
That's because:
1. `vex_special` field of VSQRTSS/VSQRTSD opcode entries is set to
`X86_VEX_REPScalar`
2. For entries with `vex_special == X86_VEX_REPScalar`,
`validate_vex()` raises the #UD
    if `(s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) != 0` and `s->vex_l` is set
Intel Core i3-6100 and AMD EPYC 7763 do not raise #UD when this
happens. (looks like another patch material)

I will send a v2 with a comment listing (V)ROUNDSS, (V)ROUNDSD,
(V)MOVSS, (V)MOVSD as the instructions affected by this change
Re: [PATCH] target/i386: ignore VEX.L when emitting VROUNDSS and VROUNDSD
Posted by Paolo Bonzini 2 weeks, 5 days ago 
On Sun, Sep 6, 2026 at 5:44 AM Andrey Polivoda <apolivodaa433@gmail.com> wrote:
> After running some tests, it looks like the behavior of (V)ROUNDSS,
> (V)ROUNDSD, (V)MOVSS and (V)MOVSD is actually modified by this patch.
> (MOVNTSS and MOVNTSD do not generate different TCG ops with and
> without the patch applied. However, the VEX prefix
> is actually illegal for these two AMD-specific instructions (tested on
> AMD EPYC 7763; material for a different patch, I guess))

Thanks for checking that, I had no idea!

> Also, VSQRTSS and VSQRTSD are actually raising an invalid opcode
> exception when VEX.L is set
> That's because:
> 1. `vex_special` field of VSQRTSS/VSQRTSD opcode entries is set to
> `X86_VEX_REPScalar`
> 2. For entries with `vex_special == X86_VEX_REPScalar`,
> `validate_vex()` raises the #UD
>     if `(s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) != 0` and `s->vex_l` is set
> Intel Core i3-6100 and AMD EPYC 7763 do not raise #UD when this
> happens. (looks like another patch material)

Yeah, please check if this can be done in a single step. Maybe with a
new vex_special X86_VEX_REPScalar_Unary or X86_VEX_REPScalar_LIG.

Paolo

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.