diff mbox series

CPUID Patch for IDT Winchip

Message ID CAP8WD_bRgyVC0ku138-s3HV9FvfRQiB=p7itrOr5nOGh+ShuEQ@mail.gmail.com
State New
Headers show
Series CPUID Patch for IDT Winchip | expand

Commit Message

tedheadster May 19, 2019, 4:10 p.m. UTC 
I have confirmed that the IDT Winchip 2 does not expressly set %ecx
after a call to cpuid() with %eax = 1, and this causes incorrect
reporting of cpu capabilities. The %ecx register should return 0x0
(and likely %ebx should too) on this hardware. This patch proposes a
fix.

The symptoms of this arose on a Winchip 2 system when systemd called
cpuid() with %eax = 1 and it incorrectly reported rdrand() support
(bit 30 set in %ecx). A call to the supposedly supported rdrand()
function triggered an illegal instruction exception, causing the
system to panic early in booting.

The IDT Winchip documentation is silent on what happens to %ecx and
%ebx after such a call (see below). It appears to leave whatever is in
%ecx untouched instead of zeroing it. This patch defensively zeroes
out %ebx:%ecx:%edx before such a call in cpuid.h.

I should be able to test the earlier Winchip C6 model in the future. I
wlll also check older AMD, Cyrix, and VIA processors.

Thanks to Mike Gilbert for suggesting the solution. Thank you also to
Segher Boessenkool for helping get the inline assembly syntax correct.

Here is the assembly code fragment the patch generates with comments:

   0xb7e21498 <+88>:    xor    %edi,%edi <--- zeroes %edi for use below
   0xb7e2149a <+90>:    mov    %edi,%eax
   0xb7e2149c <+92>:    cpuid
   0xb7e2149e <+94>:    test   %eax,%eax
   0xb7e214a0 <+96>:    je     0xb7e214c0 <rdrand+128>
   0xb7e214a2 <+98>:    mov    %edi,%ebx <--- zeroes %ebx
   0xb7e214a4 <+100>:   mov    %edi,%ecx <--- zeroes %ecx
   0xb7e214a6 <+102>:   mov    %edi,%edx <--- zeroes %edx
   0xb7e214a8 <+104>:   mov    $0x1,%eax <--- call cpuid with %eax = 1
   0xb7e214ad <+109>:   cpuid
   0xb7e214af <+111>:   shr    $0x1e

References:

http://datasheets.chipdb.org/IDT/x86/WinChip2/wc2ds.pdf
http://datasheets.chipdb.org/IDT/x86/C6/c6_data_sheet.pdf

- Matthew Whitehead

 }

Comments

Mike Gilbert May 20, 2019, 3:10 p.m. UTC | #1 
On Sun, May 19, 2019 at 12:10 PM tedheadster <tedheadster@gmail.com> wrote:
>
> I have confirmed that the IDT Winchip 2 does not expressly set %ecx
> after a call to cpuid() with %eax = 1, and this causes incorrect
> reporting of cpu capabilities. The %ecx register should return 0x0
> (and likely %ebx should too) on this hardware. This patch proposes a
> fix.
>
> The symptoms of this arose on a Winchip 2 system when systemd called
> cpuid() with %eax = 1 and it incorrectly reported rdrand() support
> (bit 30 set in %ecx). A call to the supposedly supported rdrand()
> function triggered an illegal instruction exception, causing the
> system to panic early in booting.
>
> The IDT Winchip documentation is silent on what happens to %ecx and
> %ebx after such a call (see below). It appears to leave whatever is in
> %ecx untouched instead of zeroing it. This patch defensively zeroes
> out %ebx:%ecx:%edx before such a call in cpuid.h.
>
> I should be able to test the earlier Winchip C6 model in the future. I
> wlll also check older AMD, Cyrix, and VIA processors.
>
> Thanks to Mike Gilbert for suggesting the solution. Thank you also to
> Segher Boessenkool for helping get the inline assembly syntax correct.

Sorry for the lack of feedback; your mails were getting filed into my
systemd-devel label and I only review that every few days.

I think it would be simpler/better to clear the registers within the
__cpuid macro itself. Something like this I think:

diff --git a/gcc/config/i386/cpuid.h b/gcc/config/i386/cpuid.h
index 8ddd425c8b7..ae05bfc6114 100644
--- a/gcc/config/i386/cpuid.h
+++ b/gcc/config/i386/cpuid.h
@@ -188,7 +188,10 @@
 #define signature_VORTEX_edx   0x36387865

 #define __cpuid(level, a, b, c, d)                     \
-  __asm__ ("cpuid\n\t"                                 \
+  __asm__ ("xorl\t%1, %1\n\t"                           \
+          "xorl\t%2, %2\n\t"                           \
+          "xorl\t%3, %3\n\t"                           \
+          "cpuid\n\t"                                  \
           : "=a" (a), "=b" (b), "=c" (c), "=d" (d)     \
           : "0" (level))
diff mbox series

Patch

diff --git a/gcc/config/i386/cpuid.h b/gcc/config/i386/cpuid.h
index b3b0f91..7f6d726 100644
--- a/gcc/config/i386/cpuid.h
+++ b/gcc/config/i386/cpuid.h
@@ -251,6 +251,15 @@  __get_cpuid (unsigned int __leaf,
   if (__maxlevel == 0 || __maxlevel < __leaf)
     return 0;

+  /* At least one confirmed cpu (Winchip 2) does not set %ecx correctly for
+    cpuid() %eax = 1, and leaves garbage in it (usually containing the results
+    of a previous call to cpuid() %eax = 0, which puts 'auls' in %ecx from
+    'CentaurHauls' in %ebx:%edx:%ecx, the vendor identification string).
+    Forcibly zero the three registers before calling cpuid() as a
precaution. */
+
+  *__ebx = *__ecx = *__edx = 0;
+  asm volatile("" : "+b" (*__ebx), "+c" (*__ecx), "+d" (*__edx));
+
   __cpuid (__leaf, *__eax, *__ebx, *__ecx, *__edx);
   return 1;