From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@ml01.01.org>
Cc: Jeff Fan <jeff.fan@intel.com>
Subject: [PATCH 2/4] UefiCpuPkg/MpInitLib: fix feature test for Extended Topology CPUID leaf
Date: Tue, 22 Nov 2016 21:26:17 +0100 [thread overview]
Message-ID: <20161122202619.12594-3-lersek@redhat.com> (raw)
In-Reply-To: <20161122202619.12594-1-lersek@redhat.com>
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>
---
UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 21 +++++++++++---------
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 21 +++++++++++---------
2 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 64e51d87ae24..9067f7807098 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -154,21 +154,24 @@ GetApicId:
mov eax, 0
cpuid
cmp eax, 0bh
- jnb X2Apic
+ jb NoX2Apic ; CPUID level below CPUID_EXTENDED_TOPOLOGY
+
+ mov eax, 0bh
+ xor ecx, ecx
+ cpuid
+ test ebx, 0ffffh
+ jz NoX2Apic ; CPUID.0BH:EBX[15:0] is zero
+
+ ; Processor is x2APIC capable; 32-bit x2APIC ID is already in EDX
+ jmp GetProcessorNumber
+
+NoX2Apic:
; Processor is not x2APIC capable, so get 8-bit APIC ID
mov eax, 1
cpuid
shr ebx, 24
mov edx, ebx
- jmp GetProcessorNumber
-X2Apic:
- ; Processor is x2APIC capable, so get 32-bit x2APIC ID
- mov eax, 0bh
- xor ecx, ecx
- cpuid
- ; edx save x2APIC ID
-
GetProcessorNumber:
;
; Get processor number for this AP
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index aaabb50c5468..e7e7d8086dd0 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -158,21 +158,24 @@ GetApicId:
mov eax, 0
cpuid
cmp eax, 0bh
- jnb X2Apic
+ jb NoX2Apic ; CPUID level below CPUID_EXTENDED_TOPOLOGY
+
+ mov eax, 0bh
+ xor ecx, ecx
+ cpuid
+ test ebx, 0ffffh
+ jz NoX2Apic ; CPUID.0BH:EBX[15:0] is zero
+
+ ; Processor is x2APIC capable; 32-bit x2APIC ID is already in EDX
+ jmp GetProcessorNumber
+
+NoX2Apic:
; Processor is not x2APIC capable, so get 8-bit APIC ID
mov eax, 1
cpuid
shr ebx, 24
mov edx, ebx
- jmp GetProcessorNumber
-X2Apic:
- ; Processor is x2APIC capable, so get 32-bit x2APIC ID
- mov eax, 0bh
- xor ecx, ecx
- cpuid
- ; edx save x2APIC ID
-
GetProcessorNumber:
;
; Get processor number for this AP
--
2.9.2
next prev parent reply other threads:[~2016-11-22 20:26 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-22 20:26 [PATCH 0/4] UefiCpuPkg, OvmfPkg: multiprocessing fixes and improvements Laszlo Ersek
2016-11-22 20:26 ` [PATCH 1/4] UefiCpuPkg/LocalApicLib: fix feature test for Extended Topology CPUID leaf Laszlo Ersek
2016-11-23 5:10 ` Fan, Jeff
2016-11-22 20:26 ` Laszlo Ersek [this message]
2016-11-23 5:10 ` [PATCH 2/4] UefiCpuPkg/MpInitLib: " Fan, Jeff
2016-11-22 20:26 ` [PATCH 3/4] UefiCpuPkg/MpInitLib: allow platforms to provide a known CPU count upfront Laszlo Ersek
2016-11-23 5:36 ` Fan, Jeff
2016-11-23 16:04 ` Laszlo Ersek
2016-11-24 1:18 ` Fan, Jeff
2016-11-24 9:36 ` Laszlo Ersek
2016-11-24 13:49 ` Fan, Jeff
2016-11-24 18:32 ` Igor Mammedov
2016-11-24 21:20 ` Laszlo Ersek
2016-11-22 20:26 ` [PATCH 4/4] OvmfPkg/PlatformPei: set PcdCpuKnownLogicalProcessorNumber for MpInitLib Laszlo Ersek
2016-11-23 20:43 ` [PATCH 0/4] UefiCpuPkg, OvmfPkg: multiprocessing fixes and improvements Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161122202619.12594-3-lersek@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox