diff mbox series

[PULL,04/49] target/i386: fix CPUID check for LFENCE and SFENCE

Message ID 20241031175214.214455-5-pbonzini@redhat.com
State New
Headers show
Series [PULL,01/49] stubs: avoid duplicate symbols in libqemuutil.a | expand

Commit Message

Paolo Bonzini Oct. 31, 2024, 5:51 p.m. UTC
LFENCE and SFENCE were introduced with the original SSE instruction set;
marking them incorrectly as cpuid(SSE2) causes failures for CPU models
that lack SSE2, for example pentium3.

Reported-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/decode-new.c.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Michael Tokarev Nov. 4, 2024, 6:17 p.m. UTC | #1
31.10.2024 20:51, Paolo Bonzini wrote:
> LFENCE and SFENCE were introduced with the original SSE instruction set;
> marking them incorrectly as cpuid(SSE2) causes failures for CPU models
> that lack SSE2, for example pentium3.
> 
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Tested-by: Guenter Roeck <linux@roeck-us.net>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/tcg/decode-new.c.inc | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
> index 1f193716468..48bf730cd3e 100644
> --- a/target/i386/tcg/decode-new.c.inc
> +++ b/target/i386/tcg/decode-new.c.inc
> @@ -345,9 +345,9 @@ static void decode_group15(DisasContext *s, CPUX86State *env, X86OpEntry *entry,
>           [1] = X86_OP_ENTRYw(RDxxBASE,   R,y, cpuid(FSGSBASE) chk(o64) p_f3),
>           [2] = X86_OP_ENTRYr(WRxxBASE,   R,y, cpuid(FSGSBASE) chk(o64) p_f3 zextT0),
>           [3] = X86_OP_ENTRYr(WRxxBASE,   R,y, cpuid(FSGSBASE) chk(o64) p_f3 zextT0),
> -        [5] = X86_OP_ENTRY0(LFENCE,          cpuid(SSE2) p_00),
> +        [5] = X86_OP_ENTRY0(LFENCE,          cpuid(SSE) p_00),
>           [6] = X86_OP_ENTRY0(MFENCE,          cpuid(SSE2) p_00),
> -        [7] = X86_OP_ENTRY0(SFENCE,          cpuid(SSE2) p_00),
> +        [7] = X86_OP_ENTRY0(SFENCE,          cpuid(SSE) p_00),
>       };

Should tests/tcg/i386/x86.csv be modified for LFENCE too?
(it already specifies SSE for SFENCE).

/mjt
Paolo Bonzini Nov. 4, 2024, 6:31 p.m. UTC | #2
On 11/4/24 19:17, Michael Tokarev wrote:
> 31.10.2024 20:51, Paolo Bonzini wrote:
>> LFENCE and SFENCE were introduced with the original SSE instruction set;
>> marking them incorrectly as cpuid(SSE2) causes failures for CPU models
>> that lack SSE2, for example pentium3.
>>
>> Reported-by: Guenter Roeck <linux@roeck-us.net>
>> Tested-by: Guenter Roeck <linux@roeck-us.net>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   target/i386/tcg/decode-new.c.inc | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/ 
>> decode-new.c.inc
>> index 1f193716468..48bf730cd3e 100644
>> --- a/target/i386/tcg/decode-new.c.inc
>> +++ b/target/i386/tcg/decode-new.c.inc
>> @@ -345,9 +345,9 @@ static void decode_group15(DisasContext *s, 
>> CPUX86State *env, X86OpEntry *entry,
>>           [1] = X86_OP_ENTRYw(RDxxBASE,   R,y, cpuid(FSGSBASE) 
>> chk(o64) p_f3),
>>           [2] = X86_OP_ENTRYr(WRxxBASE,   R,y, cpuid(FSGSBASE) 
>> chk(o64) p_f3 zextT0),
>>           [3] = X86_OP_ENTRYr(WRxxBASE,   R,y, cpuid(FSGSBASE) 
>> chk(o64) p_f3 zextT0),
>> -        [5] = X86_OP_ENTRY0(LFENCE,          cpuid(SSE2) p_00),
>> +        [5] = X86_OP_ENTRY0(LFENCE,          cpuid(SSE) p_00),
>>           [6] = X86_OP_ENTRY0(MFENCE,          cpuid(SSE2) p_00),
>> -        [7] = X86_OP_ENTRY0(SFENCE,          cpuid(SSE2) p_00),
>> +        [7] = X86_OP_ENTRY0(SFENCE,          cpuid(SSE) p_00),
>>       };
> 
> Should tests/tcg/i386/x86.csv be modified for LFENCE too?
> (it already specifies SSE for SFENCE).

Gah.  Looking at other sources, it seems that LFENCE is actually in SSE2 
together with MFENCE. :(  I'll send a patch to clean up.

Thanks for pointing out x86.csv!  That one is built from Intel sources 
and generally more authoritative that QEMU source code, so it's very 
good for code review.

Paolo
diff mbox series

Patch

diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
index 1f193716468..48bf730cd3e 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -345,9 +345,9 @@  static void decode_group15(DisasContext *s, CPUX86State *env, X86OpEntry *entry,
         [1] = X86_OP_ENTRYw(RDxxBASE,   R,y, cpuid(FSGSBASE) chk(o64) p_f3),
         [2] = X86_OP_ENTRYr(WRxxBASE,   R,y, cpuid(FSGSBASE) chk(o64) p_f3 zextT0),
         [3] = X86_OP_ENTRYr(WRxxBASE,   R,y, cpuid(FSGSBASE) chk(o64) p_f3 zextT0),
-        [5] = X86_OP_ENTRY0(LFENCE,          cpuid(SSE2) p_00),
+        [5] = X86_OP_ENTRY0(LFENCE,          cpuid(SSE) p_00),
         [6] = X86_OP_ENTRY0(MFENCE,          cpuid(SSE2) p_00),
-        [7] = X86_OP_ENTRY0(SFENCE,          cpuid(SSE2) p_00),
+        [7] = X86_OP_ENTRY0(SFENCE,          cpuid(SSE) p_00),
     };
 
     static const X86OpEntry group15_mem[8] = {