UefiCpuPkg/MpInitLib: fix feature test for Extended Topology CPUID leaf

According to the Intel SDM (325462-060US / September 2016),

> INPUT EAX = 0BH: Returns Extended Topology Information
>
> [...] Software must detect the presence of CPUID leaf 0BH by verifying
> (a) the highest leaf index supported by CPUID is >= 0BH, and
> (b) CPUID.0BH:EBX[15:0] reports a non-zero value. [...]

The "GetApicId" sections in the Ia32 and X64 "MpFuncs.nasm" files do not
perform check (b).

This causes an actual bug in the following OVMF setup:

- Intel W3550 host processor <http://ark.intel.com/products/39720/>,

- the QEMU/KVM guest's VCPU model is set to "host", that is, "the CPU
  visible to the guest should be exactly the same as the host CPU".

Under "GetApicId", check (a) passes: the CPUID level of the W3550 is
exactly 11 decimal. However, leaf 11 itself is not supported, therefore
EDX is set to zero:

> If a value entered for CPUID.EAX is less than or equal to the maximum
> input value and the leaf is not supported on that processor then 0 is
> returned in all the registers.

Because we don't check (b), the "GetProcessorNumber" section of the code
is reached with an initial APIC ID of 0 in EDX on all of the APs. Given
that "GetProcessorNumber" searches the
"MP_CPU_EXCHANGE_INFO.CpuInfo[*].InitialApicId" fields for a match, all
APs enter ApWakeupFunction() with an identical "NumApsExecuting"
parameter. This results in unpredictable guest behavior (crashes, reboots,
hangs etc).

Reorganize the "GetApicId" section and add the missing check in both
assembly files.

Cc: Jeff Fan <jeff.fan@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jeff Fan <jeff.fan@intel.com>

diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 64e51d87ae2462155ec4e34a2c4d69bb37345381..9067f7807098201c539192147eacd1cb72fdda4b 100644 (file)
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -154,21 +154,24 @@ GetApicId:
     mov        eax, 0\r
     cpuid\r
     cmp        eax, 0bh\r
-    jnb        X2Apic\r
+    jb         NoX2Apic             ; CPUID level below CPUID_EXTENDED_TOPOLOGY\r
+\r
+    mov        eax, 0bh\r
+    xor        ecx, ecx\r
+    cpuid\r
+    test       ebx, 0ffffh\r
+    jz         NoX2Apic             ; CPUID.0BH:EBX[15:0] is zero\r
+\r
+    ; Processor is x2APIC capable; 32-bit x2APIC ID is already in EDX\r
+    jmp        GetProcessorNumber\r
+\r
+NoX2Apic:\r
     ; Processor is not x2APIC capable, so get 8-bit APIC ID\r
     mov        eax, 1\r
     cpuid\r
     shr        ebx, 24\r
     mov        edx, ebx\r
-    jmp        GetProcessorNumber\r
 \r
-X2Apic:\r
-    ; Processor is x2APIC capable, so get 32-bit x2APIC ID\r
-    mov        eax, 0bh\r
-    xor        ecx, ecx\r
-    cpuid                   \r
-    ; edx save x2APIC ID\r
-    \r
 GetProcessorNumber:\r
     ;\r
     ; Get processor number for this AP\r

diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index aaabb50c54684493e1b5f9c2b4e98225f2c19efe..e7e7d8086dd01e989ab6114c983b9abd37d04513 100644 (file)
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -158,21 +158,24 @@ GetApicId:
     mov        eax, 0\r
     cpuid\r
     cmp        eax, 0bh\r
-    jnb        X2Apic\r
+    jb         NoX2Apic             ; CPUID level below CPUID_EXTENDED_TOPOLOGY\r
+\r
+    mov        eax, 0bh\r
+    xor        ecx, ecx\r
+    cpuid\r
+    test       ebx, 0ffffh\r
+    jz         NoX2Apic             ; CPUID.0BH:EBX[15:0] is zero\r
+\r
+    ; Processor is x2APIC capable; 32-bit x2APIC ID is already in EDX\r
+    jmp        GetProcessorNumber\r
+\r
+NoX2Apic:\r
     ; Processor is not x2APIC capable, so get 8-bit APIC ID\r
     mov        eax, 1\r
     cpuid\r
     shr        ebx, 24\r
     mov        edx, ebx\r
-    jmp        GetProcessorNumber\r
 \r
-X2Apic:\r
-    ; Processor is x2APIC capable, so get 32-bit x2APIC ID\r
-    mov        eax, 0bh\r
-    xor        ecx, ecx\r
-    cpuid                   \r
-    ; edx save x2APIC ID\r
-    \r
 GetProcessorNumber:\r
     ;\r
     ; Get processor number for this AP\r