* [edk2-devel] [PATCH v2 1/1] UefiCpuPkg/BaseXApicX2ApicLib: fix CPUID_V2_EXTENDED_TOPOLOGY detection
@ 2023-10-17 11:28 Gerd Hoffmann
2023-10-17 11:42 ` Laszlo Ersek
2023-10-25 10:09 ` Laszlo Ersek
0 siblings, 2 replies; 4+ messages in thread
From: Gerd Hoffmann @ 2023-10-17 11:28 UTC (permalink / raw)
To: devel
Cc: Eric Dong, Ray Ni, Oliver Steffen, Rahul Kumar,
László Érsek, Gerd Hoffmann
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.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2241388
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
.../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 21 ++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
index aa4eb11181f6..c0a847583330 100644
--- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
@@ -1294,11 +1294,12 @@ GetProcessorLocationByApicId (
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;
//
@@ -1424,6 +1425,7 @@ GetProcessorLocation2ByApicId (
)
{
CPUID_EXTENDED_TOPOLOGY_EAX ExtendedTopologyEax;
+ CPUID_EXTENDED_TOPOLOGY_EBX ExtendedTopologyEbx;
CPUID_EXTENDED_TOPOLOGY_ECX ExtendedTopologyEcx;
UINT32 MaxStandardCpuIdIndex;
UINT32 Index;
@@ -1436,10 +1438,19 @@ GetProcessorLocation2ByApicId (
}
//
- // 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;
}
--
2.41.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109672): https://edk2.groups.io/g/devel/message/109672
Mute This Topic: https://groups.io/mt/102015439/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [edk2-devel] [PATCH v2 1/1] UefiCpuPkg/BaseXApicX2ApicLib: fix CPUID_V2_EXTENDED_TOPOLOGY detection
2023-10-17 11:28 [edk2-devel] [PATCH v2 1/1] UefiCpuPkg/BaseXApicX2ApicLib: fix CPUID_V2_EXTENDED_TOPOLOGY detection Gerd Hoffmann
@ 2023-10-17 11:42 ` Laszlo Ersek
2023-10-24 12:30 ` Laszlo Ersek
2023-10-25 10:09 ` Laszlo Ersek
1 sibling, 1 reply; 4+ messages in thread
From: Laszlo Ersek @ 2023-10-17 11:42 UTC (permalink / raw)
To: Gerd Hoffmann, devel; +Cc: Eric Dong, Ray Ni, Oliver Steffen, Rahul Kumar
On 10/17/23 13:28, Gerd Hoffmann wrote:
> 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.
>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2241388
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 21 ++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> index aa4eb11181f6..c0a847583330 100644
> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> @@ -1294,11 +1294,12 @@ GetProcessorLocationByApicId (
> 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;
>
> //
> @@ -1424,6 +1425,7 @@ GetProcessorLocation2ByApicId (
> )
> {
> CPUID_EXTENDED_TOPOLOGY_EAX ExtendedTopologyEax;
> + CPUID_EXTENDED_TOPOLOGY_EBX ExtendedTopologyEbx;
> CPUID_EXTENDED_TOPOLOGY_ECX ExtendedTopologyEcx;
> UINT32 MaxStandardCpuIdIndex;
> UINT32 Index;
> @@ -1436,10 +1438,19 @@ GetProcessorLocation2ByApicId (
> }
>
> //
> - // 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;
> }
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109674): https://edk2.groups.io/g/devel/message/109674
Mute This Topic: https://groups.io/mt/102015439/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [edk2-devel] [PATCH v2 1/1] UefiCpuPkg/BaseXApicX2ApicLib: fix CPUID_V2_EXTENDED_TOPOLOGY detection
2023-10-17 11:42 ` Laszlo Ersek
@ 2023-10-24 12:30 ` Laszlo Ersek
0 siblings, 0 replies; 4+ messages in thread
From: Laszlo Ersek @ 2023-10-24 12:30 UTC (permalink / raw)
To: Gerd Hoffmann, devel; +Cc: Eric Dong, Ray Ni, Oliver Steffen, Rahul Kumar
Eric, Ray, Rahul -- can you ACK this patch please? I intend to merge it
in one or two days.
Thanks
Laszlo
On 10/17/23 13:42, Laszlo Ersek wrote:
> On 10/17/23 13:28, Gerd Hoffmann wrote:
>> 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.
>>
>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2241388
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>> .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 21 ++++++++++++++-----
>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
>> index aa4eb11181f6..c0a847583330 100644
>> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
>> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
>> @@ -1294,11 +1294,12 @@ GetProcessorLocationByApicId (
>> 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;
>>
>> //
>> @@ -1424,6 +1425,7 @@ GetProcessorLocation2ByApicId (
>> )
>> {
>> CPUID_EXTENDED_TOPOLOGY_EAX ExtendedTopologyEax;
>> + CPUID_EXTENDED_TOPOLOGY_EBX ExtendedTopologyEbx;
>> CPUID_EXTENDED_TOPOLOGY_ECX ExtendedTopologyEcx;
>> UINT32 MaxStandardCpuIdIndex;
>> UINT32 Index;
>> @@ -1436,10 +1438,19 @@ GetProcessorLocation2ByApicId (
>> }
>>
>> //
>> - // 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;
>> }
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> Thanks!
> Laszlo
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110000): https://edk2.groups.io/g/devel/message/110000
Mute This Topic: https://groups.io/mt/102015439/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/1] UefiCpuPkg/BaseXApicX2ApicLib: fix CPUID_V2_EXTENDED_TOPOLOGY detection
2023-10-17 11:28 [edk2-devel] [PATCH v2 1/1] UefiCpuPkg/BaseXApicX2ApicLib: fix CPUID_V2_EXTENDED_TOPOLOGY detection Gerd Hoffmann
2023-10-17 11:42 ` Laszlo Ersek
@ 2023-10-25 10:09 ` Laszlo Ersek
1 sibling, 0 replies; 4+ messages in thread
From: Laszlo Ersek @ 2023-10-25 10:09 UTC (permalink / raw)
To: devel, kraxel; +Cc: Eric Dong, Ray Ni, Oliver Steffen, Rahul Kumar
On 10/17/23 13:28, Gerd Hoffmann wrote:
> 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.
>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2241388
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 21 ++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> index aa4eb11181f6..c0a847583330 100644
> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> @@ -1294,11 +1294,12 @@ GetProcessorLocationByApicId (
> 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;
>
> //
> @@ -1424,6 +1425,7 @@ GetProcessorLocation2ByApicId (
> )
> {
> CPUID_EXTENDED_TOPOLOGY_EAX ExtendedTopologyEax;
> + CPUID_EXTENDED_TOPOLOGY_EBX ExtendedTopologyEbx;
> CPUID_EXTENDED_TOPOLOGY_ECX ExtendedTopologyEcx;
> UINT32 MaxStandardCpuIdIndex;
> UINT32 Index;
> @@ -1436,10 +1438,19 @@ GetProcessorLocation2ByApicId (
> }
>
> //
> - // 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;
> }
Commit 170d4ce8e90abb1eff03852940a69c9d17f8afe5 (via #4954).
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110029): https://edk2.groups.io/g/devel/message/110029
Mute This Topic: https://groups.io/mt/102015439/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-25 10:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-17 11:28 [edk2-devel] [PATCH v2 1/1] UefiCpuPkg/BaseXApicX2ApicLib: fix CPUID_V2_EXTENDED_TOPOLOGY detection Gerd Hoffmann
2023-10-17 11:42 ` Laszlo Ersek
2023-10-24 12:30 ` Laszlo Ersek
2023-10-25 10:09 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox