1. Patchew
  2. QEMU
  3. View series

[PATCH] target/i386: allow MMX instructions with CR4.OSFXSR=0

Paolo Bonzini posted 1 patch 3 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20221130141917.29667-1-pbonzini@redhat.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>
target/i386/tcg/decode-new.c.inc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] target/i386: allow MMX instructions with CR4.OSFXSR=0
Posted by Paolo Bonzini 3 years, 2 months ago 
MMX state is saved/restored by FSAVE/FRSTOR so the instructions are
not illegal opcodes even if CR4.OSFXSR=0.  Make sure that validate_vex
takes into account the prefix and only checks HF_OSFXSR_MASK in the
presence of an SSE instruction.

Fixes: 20581aadec5e ("target/i386: validate VEX prefixes via the instructions' exception classes", 2022-10-18)
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1350
Reported-by: Helge Konetzka (@hejko on gitlab.com)
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/decode-new.c.inc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
index e4878b967f0e..80c579164ff2 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -1488,7 +1488,8 @@ static bool validate_vex(DisasContext *s, X86DecodedInsn *decode)
             if (!(s->flags & HF_AVX_EN_MASK)) {
                 goto illegal;
             }
-        } else {
+        } else if (e->special != X86_SPECIAL_MMX ||
+                   (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ | PREFIX_DATA))) {
             if (!(s->flags & HF_OSFXSR_MASK)) {
                 goto illegal;
             }
-- 
2.38.1
Re: [PATCH] target/i386: allow MMX instructions with CR4.OSFXSR=0
Posted by Thomas Huth 3 years, 2 months ago 
On 30/11/2022 15.19, Paolo Bonzini wrote:
> MMX state is saved/restored by FSAVE/FRSTOR so the instructions are
> not illegal opcodes even if CR4.OSFXSR=0.  Make sure that validate_vex
> takes into account the prefix and only checks HF_OSFXSR_MASK in the
> presence of an SSE instruction.
> 
> Fixes: 20581aadec5e ("target/i386: validate VEX prefixes via the instructions' exception classes", 2022-10-18)
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1350
> Reported-by: Helge Konetzka (@hejko on gitlab.com)
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/tcg/decode-new.c.inc | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
> index e4878b967f0e..80c579164ff2 100644
> --- a/target/i386/tcg/decode-new.c.inc
> +++ b/target/i386/tcg/decode-new.c.inc
> @@ -1488,7 +1488,8 @@ static bool validate_vex(DisasContext *s, X86DecodedInsn *decode)
>               if (!(s->flags & HF_AVX_EN_MASK)) {
>                   goto illegal;
>               }
> -        } else {
> +        } else if (e->special != X86_SPECIAL_MMX ||
> +                   (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ | PREFIX_DATA))) {
>               if (!(s->flags & HF_OSFXSR_MASK)) {
>                   goto illegal;
>               }

This fixes the MicroSnow image, indeed, thus:

Tested-by: Thomas Huth <thuth@redhat.com>

Should this patch still be included in QEMU 7.2 ? ... since it fixes a 
regression?
Re: [PATCH] target/i386: allow MMX instructions with CR4.OSFXSR=0
Posted by Richard Henderson 3 years, 2 months ago 
On 11/30/22 22:43, Thomas Huth wrote:
> On 30/11/2022 15.19, Paolo Bonzini wrote:
>> MMX state is saved/restored by FSAVE/FRSTOR so the instructions are
>> not illegal opcodes even if CR4.OSFXSR=0.  Make sure that validate_vex
>> takes into account the prefix and only checks HF_OSFXSR_MASK in the
>> presence of an SSE instruction.
>>
>> Fixes: 20581aadec5e ("target/i386: validate VEX prefixes via the instructions' exception 
>> classes", 2022-10-18)
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1350
>> Reported-by: Helge Konetzka (@hejko on gitlab.com)
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   target/i386/tcg/decode-new.c.inc | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
>> index e4878b967f0e..80c579164ff2 100644
>> --- a/target/i386/tcg/decode-new.c.inc
>> +++ b/target/i386/tcg/decode-new.c.inc
>> @@ -1488,7 +1488,8 @@ static bool validate_vex(DisasContext *s, X86DecodedInsn *decode)
>>               if (!(s->flags & HF_AVX_EN_MASK)) {
>>                   goto illegal;
>>               }
>> -        } else {
>> +        } else if (e->special != X86_SPECIAL_MMX ||
>> +                   (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ | PREFIX_DATA))) {
>>               if (!(s->flags & HF_OSFXSR_MASK)) {
>>                   goto illegal;
>>               }
> 
> This fixes the MicroSnow image, indeed, thus:
> 
> Tested-by: Thomas Huth <thuth@redhat.com>
> 
> Should this patch still be included in QEMU 7.2 ? ... since it fixes a regression?

Yes.


r~
Re: [PATCH] target/i386: allow MMX instructions with CR4.OSFXSR=0
Posted by Richard Henderson 3 years, 2 months ago 
On 11/30/22 06:19, Paolo Bonzini wrote:
> MMX state is saved/restored by FSAVE/FRSTOR so the instructions are
> not illegal opcodes even if CR4.OSFXSR=0.  Make sure that validate_vex
> takes into account the prefix and only checks HF_OSFXSR_MASK in the
> presence of an SSE instruction.
> 
> Fixes: 20581aadec5e ("target/i386: validate VEX prefixes via the instructions' exception classes", 2022-10-18)
> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1350
> Reported-by: Helge Konetzka (@hejko on gitlab.com)
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   target/i386/tcg/decode-new.c.inc | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.