public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@ml01.01.org>
Cc: Jeff Fan <jeff.fan@intel.com>
Subject: [PATCH 1/4] UefiCpuPkg/LocalApicLib: fix feature test for Extended Topology CPUID leaf
Date: Tue, 22 Nov 2016 21:26:16 +0100	[thread overview]
Message-ID: <20161122202619.12594-2-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 LocalApicLib instances in UefiCpuPkg 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".

In the GetInitialApicId() function, 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), we return 0 as initial APIC ID on the BSP and
on all of the APs as well.

Add the missing check.

Cc: Jeff Fan <jeff.fan@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c             | 7 +++++--
 UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 7 +++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
index 4064049807b7..f81bbb2252d9 100644
--- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
@@ -314,12 +314,15 @@ GetInitialApicId (
 
   //
   // If CPUID Leaf B is supported, 
+  // And CPUID.0BH:EBX[15:0] reports a non-zero value,
   // Then the initial 32-bit APIC ID = CPUID.0BH:EDX
   // Else the initial 8-bit APIC ID = CPUID.1:EBX[31:24]
   //
   if (MaxCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) {
-    AsmCpuidEx (CPUID_EXTENDED_TOPOLOGY, 0, NULL, NULL, NULL, &ApicId);
-    return ApicId;
+    AsmCpuidEx (CPUID_EXTENDED_TOPOLOGY, 0, NULL, &RegEbx, NULL, &ApicId);
+    if ((RegEbx & (BIT16 - 1)) != 0) {
+      return ApicId;
+    }
   }
 
   AsmCpuid (CPUID_VERSION_INFO, NULL, &RegEbx, NULL, NULL);
diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
index 9720d26e60e2..e690d2aa1445 100644
--- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
@@ -411,12 +411,15 @@ GetInitialApicId (
     AsmCpuid (CPUID_SIGNATURE, &MaxCpuIdIndex, NULL, NULL, NULL);
     //
     // If CPUID Leaf B is supported, 
+    // And CPUID.0BH:EBX[15:0] reports a non-zero value,
     // Then the initial 32-bit APIC ID = CPUID.0BH:EDX
     // Else the initial 8-bit APIC ID = CPUID.1:EBX[31:24]
     //
     if (MaxCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) {
-      AsmCpuidEx (CPUID_EXTENDED_TOPOLOGY, 0, NULL, NULL, NULL, &ApicId);
-      return ApicId;
+      AsmCpuidEx (CPUID_EXTENDED_TOPOLOGY, 0, NULL, &RegEbx, NULL, &ApicId);
+      if ((RegEbx & (BIT16 - 1)) != 0) {
+        return ApicId;
+      }
     }
     AsmCpuid (CPUID_VERSION_INFO, NULL, &RegEbx, NULL, NULL);
     return RegEbx >> 24;
-- 
2.9.2




  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 ` Laszlo Ersek [this message]
2016-11-23  5:10   ` [PATCH 1/4] UefiCpuPkg/LocalApicLib: fix feature test for Extended Topology CPUID leaf Fan, Jeff
2016-11-22 20:26 ` [PATCH 2/4] UefiCpuPkg/MpInitLib: " Laszlo Ersek
2016-11-23  5:10   ` 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-2-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