[PATCH 01/18] target/i386/tcg: fix check for invalid VSIB instruction

Paolo Bonzini posted 18 patches 6 days, 14 hours ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Zhao Liu <zhao1.liu@intel.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>
[PATCH 01/18] target/i386/tcg: fix check for invalid VSIB instruction
Posted by Paolo Bonzini 6 days, 14 hours ago
VSIB instructions (VEX class 12) must not have an address prefix.
Checking s->aflag == MO_16 is not enough because in 64-bit mode
the address prefix changes aflag to MO_32.  Add a specific check
bit instead.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/decode-new.h     |  3 +++
 target/i386/tcg/decode-new.c.inc | 27 +++++++++++++--------------
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/target/i386/tcg/decode-new.h b/target/i386/tcg/decode-new.h
index 7f23d373ea7..38882b5c6ab 100644
--- a/target/i386/tcg/decode-new.h
+++ b/target/i386/tcg/decode-new.h
@@ -181,6 +181,9 @@ typedef enum X86InsnCheck {
     /* Vendor-specific checks for Intel/AMD differences */
     X86_CHECK_i64_amd = 2048,
     X86_CHECK_o64_intel = 4096,
+
+    /* No 0x67 prefix allowed */
+    X86_CHECK_no_adr = 8192,
 } X86InsnCheck;
 
 typedef enum X86InsnSpecial {
diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
index 0f8c5d16938..0b85b0f6513 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -623,10 +623,10 @@ static const X86OpEntry opcodes_0F38_00toEF[240] = {
     [0x46] = X86_OP_ENTRY3(VPSRAV,      V,x,  H,x,       W,x,  vex6 chk(W0) cpuid(AVX2) p_66),
     [0x47] = X86_OP_ENTRY3(VPSLLV,      V,x,  H,x,       W,x,  vex6 cpuid(AVX2) p_66),
 
-    [0x90] = X86_OP_ENTRY3(VPGATHERD, V,x,  H,x,  M,d,  vex12 cpuid(AVX2) p_66), /* vpgatherdd/q */
-    [0x91] = X86_OP_ENTRY3(VPGATHERQ, V,x,  H,x,  M,q,  vex12 cpuid(AVX2) p_66), /* vpgatherqd/q */
-    [0x92] = X86_OP_ENTRY3(VPGATHERD, V,x,  H,x,  M,d,  vex12 cpuid(AVX2) p_66), /* vgatherdps/d */
-    [0x93] = X86_OP_ENTRY3(VPGATHERQ, V,x,  H,x,  M,q,  vex12 cpuid(AVX2) p_66), /* vgatherqps/d */
+    [0x90] = X86_OP_ENTRY3(VPGATHERD, V,x,  H,x,  M,d,  vex12 chk(no_adr) cpuid(AVX2) p_66), /* vpgatherdd/q */
+    [0x91] = X86_OP_ENTRY3(VPGATHERQ, V,x,  H,x,  M,q,  vex12 chk(no_adr) cpuid(AVX2) p_66), /* vpgatherqd/q */
+    [0x92] = X86_OP_ENTRY3(VPGATHERD, V,x,  H,x,  M,d,  vex12 chk(no_adr) cpuid(AVX2) p_66), /* vgatherdps/d */
+    [0x93] = X86_OP_ENTRY3(VPGATHERQ, V,x,  H,x,  M,q,  vex12 chk(no_adr) cpuid(AVX2) p_66), /* vgatherqps/d */
 
     /* Should be exception type 2 but they do not have legacy SSE equivalents? */
     [0x96] = X86_OP_ENTRY3(VFMADDSUB132Px, V,x,  H,x, W,x,  vex6 cpuid(FMA) p_66),
@@ -2435,8 +2435,8 @@ static bool validate_vex(DisasContext *s, X86DecodedInsn *decode)
         break;
     case 12:
         /* Must have a VSIB byte and no address prefix.  */
-        assert(s->has_modrm);
-        if ((s->modrm & 7) != 4 || s->aflag == MO_16) {
+        assert(s->has_modrm && (decode->e.check & X86_CHECK_no_adr));
+        if ((s->modrm & 7) != 4) {
             goto illegal;
         }
 
@@ -2740,15 +2740,14 @@ static void disas_insn(DisasContext *s, CPUState *cpu)
                 goto illegal_op;
             }
         }
-        if (decode.e.check & X86_CHECK_prot_or_vm86) {
-            if (!PE(s)) {
-                goto illegal_op;
-            }
+        if ((decode.e.check & X86_CHECK_prot_or_vm86) && !PE(s)) {
+            goto illegal_op;
         }
-        if (decode.e.check & X86_CHECK_no_vm86) {
-            if (VM86(s)) {
-                goto illegal_op;
-            }
+        if ((decode.e.check & X86_CHECK_no_vm86) && VM86(s)) {
+            goto illegal_op;
+        }
+        if ((decode.e.check & X86_CHECK_no_adr) && (s->prefix & PREFIX_ADR)) {
+            goto illegal_op;
         }
     }
 
-- 
2.52.0
Re: [PATCH 01/18] target/i386/tcg: fix check for invalid VSIB instruction
Posted by Richard Henderson 5 days, 11 hours ago
On 12/10/25 07:16, Paolo Bonzini wrote:
> VSIB instructions (VEX class 12) must not have an address prefix.
> Checking s->aflag == MO_16 is not enough because in 64-bit mode
> the address prefix changes aflag to MO_32.  Add a specific check
> bit instead.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/tcg/decode-new.h     |  3 +++
>   target/i386/tcg/decode-new.c.inc | 27 +++++++++++++--------------
>   2 files changed, 16 insertions(+), 14 deletions(-)

Where do you see this?  I think this is wrong.

In particular,

Table 2-27. Type 12 Class Exception Conditions
- If address size attribute is 16 bit.

and

2.3.12 Vector SIB (VSIB) Memory Addressing
In 16-bit protected mode, VSIB memory addressing is permitted if address size attribute is 
overridden to 32 bits.

Therefore, in 16-bit mode, one *must* use the address prefix.



r~
Re: [PATCH 01/18] target/i386/tcg: fix check for invalid VSIB instruction
Posted by Paolo Bonzini 5 days, 7 hours ago
On Thu, Dec 11, 2025 at 4:47 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 12/10/25 07:16, Paolo Bonzini wrote:
> > VSIB instructions (VEX class 12) must not have an address prefix.
> > Checking s->aflag == MO_16 is not enough because in 64-bit mode
> > the address prefix changes aflag to MO_32.  Add a specific check
> > bit instead.
> >
> > Cc: qemu-stable@nongnu.org
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >   target/i386/tcg/decode-new.h     |  3 +++
> >   target/i386/tcg/decode-new.c.inc | 27 +++++++++++++--------------
> >   2 files changed, 16 insertions(+), 14 deletions(-)
>
> Where do you see this?  I think this is wrong.

Yes, I was confused by the comment and by QEMU's incorrect decoding logic:

        if (CODE32(s) && !VM86(s)) {

which should be changed to

       if (PE(s) && !VM86(s)) {

And by the way, this also means that we need either separate helpers
for 32- and 64-bit addresses, or a mask argument.

Paolo

> In particular,
>
> Table 2-27. Type 12 Class Exception Conditions
> - If address size attribute is 16 bit.
>
> and
>
> 2.3.12 Vector SIB (VSIB) Memory Addressing
> In 16-bit protected mode, VSIB memory addressing is permitted if address size attribute is
> overridden to 32 bits.
>
> Therefore, in 16-bit mode, one *must* use the address prefix.
>
>
>
> r~
>
Re: [PATCH 01/18] target/i386/tcg: fix check for invalid VSIB instruction
Posted by Richard Henderson 5 days, 5 hours ago
On 12/11/25 14:28, Paolo Bonzini wrote:
> On Thu, Dec 11, 2025 at 4:47 PM Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 12/10/25 07:16, Paolo Bonzini wrote:
>>> VSIB instructions (VEX class 12) must not have an address prefix.
>>> Checking s->aflag == MO_16 is not enough because in 64-bit mode
>>> the address prefix changes aflag to MO_32.  Add a specific check
>>> bit instead.
>>>
>>> Cc: qemu-stable@nongnu.org
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>    target/i386/tcg/decode-new.h     |  3 +++
>>>    target/i386/tcg/decode-new.c.inc | 27 +++++++++++++--------------
>>>    2 files changed, 16 insertions(+), 14 deletions(-)
>>
>> Where do you see this?  I think this is wrong.
> 
> Yes, I was confused by the comment and by QEMU's incorrect decoding logic:
> 
>          if (CODE32(s) && !VM86(s)) {
> 
> which should be changed to
> 
>         if (PE(s) && !VM86(s)) {

I can't find the language for that.  Can you point me at it?

> And by the way, this also means that we need either separate helpers
> for 32- and 64-bit addresses, or a mask argument.

Of course.


r~

Re: [PATCH 01/18] target/i386/tcg: fix check for invalid VSIB instruction
Posted by Paolo Bonzini 5 days, 1 hour ago
Il gio 11 dic 2025, 23:22 Richard Henderson <richard.henderson@linaro.org>
ha scritto:

> > Yes, I was confused by the comment and by QEMU's incorrect decoding
> logic:
> >
> >          if (CODE32(s) && !VM86(s)) {
> >
> > which should be changed to
> >
> >         if (PE(s) && !VM86(s)) {
>
> I can't find the language for that.  Can you point me at it?
>

It's the exception condition tables. They all mention that you get #UD for
the VEX prefix in real or vm86 mode.

Several BMI instructions also have language like "This instruction is not
supported in real mode and virtual-8086 mode".

Paolo


> > And by the way, this also means that we need either separate helpers
> > for 32- and 64-bit addresses, or a mask argument.
>
> Of course.
>
>
> r~
>
>
Re: [PATCH 01/18] target/i386/tcg: fix check for invalid VSIB instruction
Posted by Richard Henderson 4 days, 12 hours ago
On 12/11/25 20:06, Paolo Bonzini wrote:
> 
> 
> Il gio 11 dic 2025, 23:22 Richard Henderson <richard.henderson@linaro.org 
> <mailto:richard.henderson@linaro.org>> ha scritto:
> 
>      > Yes, I was confused by the comment and by QEMU's incorrect decoding logic:
>      >
>      >          if (CODE32(s) && !VM86(s)) {
>      >
>      > which should be changed to
>      >
>      >         if (PE(s) && !VM86(s)) {
> 
>     I can't find the language for that.  Can you point me at it?
> 
> 
> It's the exception condition tables. They all mention that you get #UD for the VEX prefix 
> in real or vm86 mode.

Ah right, found it.  Thanks.

> Several BMI instructions also have language like "This instruction is not supported in 
> real mode and virtual-8086 mode".

Amusingly, some of them dropped the "not" in that sentence -- see ADCX.


r~