On 11/15/23 12:15, Jiaxin Wu wrote: > This patch is to complete 170d4ce8, sync the change to BaseXApicLib. > > Checking the max cpuid leaf is not enough to figure whenever > CPUID_V2_EXTENDED_TOPOLOGY is supported. Intel SDM says: > > Software must detect the presence of CPUID leaf 1FH by verifying > (a) the highest leaf index supported by CPUID is >= 1FH, and > (b) CPUID.1FH:EBX[15:0] reports a non-zero value. > > The same is true for CPUID leaf 0BH. > > This patch adds the EBX check to GetProcessorLocation2ByApicId(). The > patch also fixes the existing check in GetProcessorLocationByApicId() to > be in line with the spec by looking at bits 15:0. The comments are > updated with a quote from the Intel SDM. > > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Rahul Kumar <rahul1.ku...@intel.com> > Cc: Gerd Hoffmann <kra...@redhat.com> > Cc: Star Zeng <star.z...@intel.com> > Signed-off-by: Gerd Hoffmann <kra...@redhat.com> > Signed-off-by: Jiaxin Wu <jiaxin...@intel.com> > --- > UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 21 ++++++++++++++++----- > 1 file changed, 16 insertions(+), 5 deletions(-) > > diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c > b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c > index d56c6275cc..efb9d71ca1 100644 > --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c > +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c > @@ -1053,15 +1053,16 @@ GetProcessorLocationByApicId ( > &ExtendedTopologyEbx.Uint32, > &ExtendedTopologyEcx.Uint32, > NULL > ); > // > - // If CPUID.(EAX=0BH, ECX=0H):EBX returns zero and maximum input value > for > - // basic CPUID information is greater than 0BH, then CPUID.0BH leaf is > not > - // supported on that processor. > + // Quoting Intel SDM: > + // 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. > // > - if (ExtendedTopologyEbx.Uint32 != 0) { > + if (ExtendedTopologyEbx.Bits.LogicalProcessors != 0) { > TopologyLeafSupported = TRUE; > > // > // Sub-leaf index 0 (ECX= 0 as input) provides enumeration parameters > to extract > // the SMT sub-field of x2APIC ID. > @@ -1183,10 +1184,11 @@ GetProcessorLocation2ByApicId ( > OUT UINT32 *Core OPTIONAL, > OUT UINT32 *Thread OPTIONAL > ) > { > CPUID_EXTENDED_TOPOLOGY_EAX ExtendedTopologyEax; > + CPUID_EXTENDED_TOPOLOGY_EBX ExtendedTopologyEbx; > CPUID_EXTENDED_TOPOLOGY_ECX ExtendedTopologyEcx; > UINT32 MaxStandardCpuIdIndex; > UINT32 Index; > UINTN LevelType; > UINT32 > Bits[CPUID_V2_EXTENDED_TOPOLOGY_LEVEL_TYPE_DIE + 2]; > @@ -1195,14 +1197,23 @@ GetProcessorLocation2ByApicId ( > for (LevelType = 0; LevelType < ARRAY_SIZE (Bits); LevelType++) { > Bits[LevelType] = 0; > } > > // > - // Get max index of CPUID > + // Quoting Intel SDM: > + // Software must detect the presence of CPUID leaf 1FH by verifying > + // (a) the highest leaf index supported by CPUID is >= 1FH, and (b) > + // CPUID.1FH:EBX[15:0] reports a non-zero value. > // > AsmCpuid (CPUID_SIGNATURE, &MaxStandardCpuIdIndex, NULL, NULL, NULL); > if (MaxStandardCpuIdIndex < CPUID_V2_EXTENDED_TOPOLOGY) { > + ExtendedTopologyEbx.Bits.LogicalProcessors = 0; > + } else { > + AsmCpuidEx (CPUID_V2_EXTENDED_TOPOLOGY, 0, NULL, > &ExtendedTopologyEbx.Uint32, NULL, NULL); > + } > + > + if (ExtendedTopologyEbx.Bits.LogicalProcessors == 0) { > if (Die != NULL) { > *Die = 0; > } > > if (Tile != NULL) {
This synchronizes the GetProcessorLocationByApicId() and GetProcessorLocation2ByApicId() functions in BaseXApicLib with those in BaseXApicX2ApicLib. Reviewed-by: Laszlo Ersek <ler...@redhat.com> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111278): https://edk2.groups.io/g/devel/message/111278 Mute This Topic: https://groups.io/mt/102602851/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-