I cannot remember if CPUID.0B and CPUID.1F return the same value for package ID.

And I am not sure about the benefit if we get the package id from location2.

Thanks,
Ray
________________________________
From: Wu, Jiaxin <jiaxin.wu@intel.com>
Sent: Wednesday, November 15, 2023 7:15 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Zeng, Star <star.zeng@intel.com>
Subject: [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Use processor extended information

This patch changes to use the processor extended information,
which can reflect the value from CPUID leaf 1FH.

The reasons are listed as below:
1. The processor package ID retrieved from CPUID 0x0Bh may be
not correct/accurate if CPU has the module & die info, it depends
on the CPUID implementation. See SDM statement:

EAX Bits 04 - 00: Number of bits to shift right on x2APIC ID to
get a unique topology ID of the next level type.
ECX Bits 15 - 08: level type

Level type field has the following encoding:
0: Invalid.
1: SMT.
2: Core.
3-255: Reserved

So, if level type returned from ECX Bits 15 - 08 is 2 (Core),
then it's not clear about the next level. It can be Module or
Die or Package. SDM doesn't has explanation for the next level
of Core. If so, the value will be decided by implementation.
The value can be package info for compatibility consideration,
but it's not standardized.

2. According SDM declaration, "CPUID leaf 1FH is a preferred
superset to leaf 0BH. Intel recommends first checking for the
existence of CPUID leaf 1FH before using leaf 0BH." This is
perfect match the existing GetProcessorLocation2ByApicId()
implementation.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 25d058c5b9..c61562c867 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -177,11 +177,11 @@ IsPackageFirstThread (
   IN UINTN  CpuIndex
   )
 {
   UINT32  PackageIndex;

-  PackageIndex =  gSmmCpuPrivate->ProcessorInfo[CpuIndex].Location.Package;
+  PackageIndex =  gSmmCpuPrivate->ProcessorInfo[CpuIndex].ExtendedInformation.Location2.Package;

   ASSERT (mPackageFirstThreadIndex != NULL);

   //
   // Set the value of mPackageFirstThreadIndex[PackageIndex].
@@ -1834,12 +1834,12 @@ InitPackageFirstThreadIndexInfo (

   //
   // Count the number of package, set to max PackageId + 1
   //
   for (Index = 0; Index < mNumberOfCpus; Index++) {
-    if (PackageId < gSmmCpuPrivate->ProcessorInfo[Index].Location.Package) {
-      PackageId = gSmmCpuPrivate->ProcessorInfo[Index].Location.Package;
+    if (PackageId < gSmmCpuPrivate->ProcessorInfo[Index].ExtendedInformation.Location2.Package) {
+      PackageId = gSmmCpuPrivate->ProcessorInfo[Index].ExtendedInformation.Location2.Package;
     }
   }

   PackageCount = PackageId + 1;

--
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111297): https://edk2.groups.io/g/devel/message/111297
Mute This Topic: https://groups.io/mt/102602853/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-