From: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1584
The flow of CPU feature initialization logic is:
1. BSP calls GetConfigDataFunc() for each thread/AP;
2. Each thread/AP calls SupportFunc() to detect its own capability;
3. BSP calls InitializeFunc() for each thread/AP.
There is a design gap in step #3. For a package scope feature that only
requires one thread of each package does the initialization operation,
what InitializeFunc() currently does is to do the initialization
operation only CPU physical location Core# is 0.
But in certain platform, Core#0 might be disabled in hardware level
which results the certain package scope feature isn't initialized at
all.
The patch adds a new field Fist to indicate the CPU's location in
its parent scope.
First.Package is set for all APs/threads under first package;
First.Core is set for all APs/threads under first core of each
package;
First.Thread is set for the AP/thread of each core.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
---
.../Include/Library/RegisterCpuFeaturesLib.h | 36 +++++++++
.../CpuFeaturesInitialize.c | 74 +++++++++++++++++++
2 files changed, 110 insertions(+)
diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
index d075606cdb..7114c8ce89 100644
--- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
+++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
@@ -78,6 +78,37 @@
#define CPU_FEATURE_END MAX_UINT32
/// @}
+///
+/// The bit field to indicate whether the processor is the first in its parent scope.
+///
+typedef struct {
+ //
+ // Set to 1 when current processor is the first thread in the core it resides in.
+ //
+ UINT32 Thread : 1;
+ //
+ // Set to 1 when current processor is a thread of the first core in the module it resides in.
+ //
+ UINT32 Core : 1;
+ //
+ // Set to 1 when current processor is a thread of the first module in the tile it resides in.
+ //
+ UINT32 Module : 1;
+ //
+ // Set to 1 when current processor is a thread of the first tile in the die it resides in.
+ //
+ UINT32 Tile : 1;
+ //
+ // Set to 1 when current processor is a thread of the first die in the package it resides in.
+ //
+ UINT32 Die : 1;
+ //
+ // Set to 1 when current processor is a thread of the first package in the system.
+ //
+ UINT32 Package : 1;
+ UINT32 Reserved : 26;
+} REGISTER_CPU_FEATURE_FIRST_PROCESSOR;
+
///
/// CPU Information passed into the SupportFunc and InitializeFunc of the
/// RegisterCpuFeature() library function. This structure contains information
@@ -88,6 +119,11 @@ typedef struct {
/// The package that the CPU resides
///
EFI_PROCESSOR_INFORMATION ProcessorInfo;
+
+ ///
+ /// The bit flag indicating whether the CPU is the first Thread/Core/Module/Tile/Die/Package in its parent scope.
+ ///
+ REGISTER_CPU_FEATURE_FIRST_PROCESSOR First;
///
/// The Display Family of the CPU computed from CPUID leaf CPUID_VERSION_INFO
///
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 0a4fcff033..23076fd453 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -105,6 +105,9 @@ CpuInitDataInitialize (
EFI_CPU_PHYSICAL_LOCATION *Location;
BOOLEAN *CoresVisited;
UINTN Index;
+ UINT32 PackageIndex;
+ UINT32 CoreIndex;
+ UINT32 First;
ACPI_CPU_DATA *AcpiCpuData;
CPU_STATUS_INFORMATION *CpuStatus;
UINT32 *ValidCoreCountPerPackage;
@@ -234,6 +237,77 @@ CpuInitDataInitialize (
ASSERT (CpuFeaturesData->CpuFlags.CoreSemaphoreCount != NULL);
CpuFeaturesData->CpuFlags.PackageSemaphoreCount = AllocateZeroPool (sizeof (UINT32) * CpuStatus->PackageCount * CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount);
ASSERT (CpuFeaturesData->CpuFlags.PackageSemaphoreCount != NULL);
+
+ //
+ // Initialize CpuFeaturesData->InitOrder[].CpuInfo.First
+ //
+
+ //
+ // Set First.Package for each thread belonging to the first package.
+ //
+ First = MAX_UINT32;
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+ First = MIN (Location->Package, First);
+ }
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+ if (Location->Package == First) {
+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Package = 1;
+ }
+ }
+
+ //
+ // Set First.Die/Tile/Module for each thread assuming:
+ // single Die under each package, single Tile under each Die, single Module under each Tile
+ //
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Die = 1;
+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Tile = 1;
+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Module = 1;
+ }
+
+ for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount; PackageIndex++) {
+ //
+ // Set First.Core for each thread in the first core of each package.
+ //
+ First = MAX_UINT32;
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+ if (Location->Package == PackageIndex) {
+ First = MIN (Location->Core, First);
+ }
+ }
+
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+ if (Location->Package == PackageIndex && Location->Core == First) {
+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Core = 1;
+ }
+ }
+ }
+
+ for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount; PackageIndex++) {
+ for (CoreIndex = 0; CoreIndex < CpuStatus->MaxCoreCount; CoreIndex++) {
+ //
+ // Set First.Thread for the first thread of each core.
+ //
+ First = MAX_UINT32;
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+ if (Location->Package == PackageIndex && Location->Core == CoreIndex) {
+ First = MIN (Location->Thread, First);
+ }
+ }
+
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+ if (Location->Package == PackageIndex && Location->Core == CoreIndex && Location->Thread == First) {
+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Thread = 1;
+ }
+ }
+ }
+ }
}
/**
--
2.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#51302): https://edk2.groups.io/g/devel/message/51302
Mute This Topic: https://groups.io/mt/61962261/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Some feedback added.
> -----Original Message-----
> From: Ray Ni [mailto:niruiyu@users.noreply.github.com]
> Sent: Tuesday, November 26, 2019 2:16 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate
> 1st unit.
>
> From: Ray Ni <ray.ni@intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1584
>
> The flow of CPU feature initialization logic is:
> 1. BSP calls GetConfigDataFunc() for each thread/AP;
> 2. Each thread/AP calls SupportFunc() to detect its own capability;
> 3. BSP calls InitializeFunc() for each thread/AP.
>
> There is a design gap in step #3. For a package scope feature that only
> requires one thread of each package does the initialization operation,
> what InitializeFunc() currently does is to do the initialization
> operation only CPU physical location Core# is 0.
> But in certain platform, Core#0 might be disabled in hardware level
> which results the certain package scope feature isn't initialized at
> all.
Need some patches to update individual InitializeFunc() for features.
These patches can be a separated patch series.
>
> The patch adds a new field Fist to indicate the CPU's location in
"Firt" should be "First".
> its parent scope.
> First.Package is set for all APs/threads under first package;
> First.Core is set for all APs/threads under first core of each
> package;
> First.Thread is set for the AP/thread of each core.
>
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
> .../Include/Library/RegisterCpuFeaturesLib.h | 36 +++++++++
> .../CpuFeaturesInitialize.c | 74 +++++++++++++++++++
> 2 files changed, 110 insertions(+)
>
> diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> index d075606cdb..7114c8ce89 100644
> --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> @@ -78,6 +78,37 @@
> #define CPU_FEATURE_END MAX_UINT32
> /// @}
>
> +///
> +/// The bit field to indicate whether the processor is the first in its parent
> scope.
> +///
> +typedef struct {
> + //
> + // Set to 1 when current processor is the first thread in the core it resides
> in.
> + //
> + UINT32 Thread : 1;
> + //
> + // Set to 1 when current processor is a thread of the first core in the
> module it resides in.
> + //
> + UINT32 Core : 1;
> + //
> + // Set to 1 when current processor is a thread of the first module in the tile
> it resides in.
> + //
> + UINT32 Module : 1;
> + //
> + // Set to 1 when current processor is a thread of the first tile in the die it
> resides in.
> + //
> + UINT32 Tile : 1;
> + //
> + // Set to 1 when current processor is a thread of the first die in the package
> it resides in.
> + //
> + UINT32 Die : 1;
> + //
> + // Set to 1 when current processor is a thread of the first package in the
> system.
> + //
> + UINT32 Package : 1;
> + UINT32 Reserved : 26;
> +} REGISTER_CPU_FEATURE_FIRST_PROCESSOR;
> +
> ///
> /// CPU Information passed into the SupportFunc and InitializeFunc of the
> /// RegisterCpuFeature() library function. This structure contains
> information
> @@ -88,6 +119,11 @@ typedef struct {
> /// The package that the CPU resides
> ///
> EFI_PROCESSOR_INFORMATION ProcessorInfo;
> +
> + ///
> + /// The bit flag indicating whether the CPU is the first
> Thread/Core/Module/Tile/Die/Package in its parent scope.
> + ///
> + REGISTER_CPU_FEATURE_FIRST_PROCESSOR First;
> ///
> /// The Display Family of the CPU computed from CPUID leaf
> CPUID_VERSION_INFO
> ///
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> index 0a4fcff033..23076fd453 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -105,6 +105,9 @@ CpuInitDataInitialize (
> EFI_CPU_PHYSICAL_LOCATION *Location;
> BOOLEAN *CoresVisited;
> UINTN Index;
> + UINT32 PackageIndex;
> + UINT32 CoreIndex;
> + UINT32 First;
> ACPI_CPU_DATA *AcpiCpuData;
> CPU_STATUS_INFORMATION *CpuStatus;
> UINT32 *ValidCoreCountPerPackage;
> @@ -234,6 +237,77 @@ CpuInitDataInitialize (
> ASSERT (CpuFeaturesData->CpuFlags.CoreSemaphoreCount != NULL);
> CpuFeaturesData->CpuFlags.PackageSemaphoreCount = AllocateZeroPool
> (sizeof (UINT32) * CpuStatus->PackageCount * CpuStatus->MaxCoreCount *
> CpuStatus->MaxThreadCount);
> ASSERT (CpuFeaturesData->CpuFlags.PackageSemaphoreCount != NULL);
> +
> + //
> + // Initialize CpuFeaturesData->InitOrder[].CpuInfo.First
> + //
> +
> + //
> + // Set First.Package for each thread belonging to the first package.
> + //
> + First = MAX_UINT32;
> + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> + Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> + First = MIN (Location->Package, First);
> + }
> + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> + Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> + if (Location->Package == First) {
> + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Package =
> 1;
> + }
> + }
> +
> + //
> + // Set First.Die/Tile/Module for each thread assuming:
> + // single Die under each package, single Tile under each Die, single Module
> under each Tile
This assumption needs to be addressed in this or a separated patch series.
> + //
> + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Die = 1;
> + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Tile = 1;
> + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Module =
> 1;
> + }
> +
> + for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount;
> PackageIndex++) {
> + //
> + // Set First.Core for each thread in the first core of each package.
> + //
> + First = MAX_UINT32;
> + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> + Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> + if (Location->Package == PackageIndex) {
Here the code is assuming Location->Package starts from 0 and consecutive.
> + First = MIN (Location->Core, First);
> + }
> + }
> +
> + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> + Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> + if (Location->Package == PackageIndex && Location->Core == First) {
> + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Core = 1;
> + }
> + }
> + }
> +
> + for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount;
> PackageIndex++) {
> + for (CoreIndex = 0; CoreIndex < CpuStatus->MaxCoreCount;
> CoreIndex++) {
> + //
> + // Set First.Thread for the first thread of each core.
> + //
> + First = MAX_UINT32;
> + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> + Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> + if (Location->Package == PackageIndex && Location->Core ==
> CoreIndex) {
Here the code is assuming Location->Package and Location->Core start from 0 and consecutive.
We could not have this assumption, this patch is to resolve this assumption.
Thanks,
Star
> + First = MIN (Location->Thread, First);
> + }
> + }
> +
> + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> + Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> + if (Location->Package == PackageIndex && Location->Core ==
> CoreIndex && Location->Thread == First) {
> + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Thread
> = 1;
> + }
> + }
> + }
> + }
> }
>
> /**
> --
> 2.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52565): https://edk2.groups.io/g/devel/message/52565
Mute This Topic: https://groups.io/mt/61962261/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
>
> Need some patches to update individual InitializeFunc() for features.
> These patches can be a separated patch series.
>
Yes.
> >
> > The patch adds a new field Fist to indicate the CPU's location in
>
> "Firt" should be "First".
Will fix the typo in next version of patch or pushing.
> > + //
> > + // Set First.Die/Tile/Module for each thread assuming:
> > + // single Die under each package, single Tile under each Die, single
> Module
> > under each Tile
>
> This assumption needs to be addressed in this or a separated patch series.
The assumption will be fixed after the below changes are merged to trunk.
https://github.com/tianocore/edk2-staging/tree/cpu/6-level
> > + for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount;
> > PackageIndex++) {
> > + //
> > + // Set First.Core for each thread in the first core of each package.
> > + //
> > + First = MAX_UINT32;
> > + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> > ProcessorNumber++) {
> > + Location = &CpuFeaturesData-
> > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> > + if (Location->Package == PackageIndex) {
>
> Here the code is assuming Location->Package starts from 0 and consecutive.
CpuStatus->PackageCount is assigned in CpuInitDataInitialize():
> CpuStatus->PackageCount = Package + 1;
> CpuStatus->MaxCoreCount = Core + 1;
> CpuStatus->MaxThreadCount = Thread + 1;
So PackageCount actually is the value of max package ID + 1.
With that, the code change isn't assuming Location->Package starts from 0 and consecutive.
>
> Here the code is assuming Location->Package and Location->Core start from
> 0 and consecutive.
> We could not have this assumption, this patch is to resolve this assumption.
Similarly, The code change above isn't assuming Location->Core starts from 0 and consecutive.
>
>
> Thanks,
> Star
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52642): https://edk2.groups.io/g/devel/message/52642
Mute This Topic: https://groups.io/mt/61962261/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Got the point.
With the typo fixed, Reviewed-by: Star Zeng <star.zeng@intel.com>
> -----Original Message-----
> From: Ni, Ray
> Sent: Thursday, January 2, 2020 11:15 AM
> To: Zeng, Star <star.zeng@intel.com>; Ray Ni
> <niruiyu@users.noreply.github.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate
> 1st unit.
>
> >
> > Need some patches to update individual InitializeFunc() for features.
> > These patches can be a separated patch series.
> >
> Yes.
>
> > >
> > > The patch adds a new field Fist to indicate the CPU's location in
> >
> > "Firt" should be "First".
> Will fix the typo in next version of patch or pushing.
>
> > > + //
> > > + // Set First.Die/Tile/Module for each thread assuming:
> > > + // single Die under each package, single Tile under each Die, single
> > Module
> > > under each Tile
> >
> > This assumption needs to be addressed in this or a separated patch series.
>
> The assumption will be fixed after the below changes are merged to trunk.
> https://github.com/tianocore/edk2-staging/tree/cpu/6-level
>
>
> > > + for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount;
> > > PackageIndex++) {
> > > + //
> > > + // Set First.Core for each thread in the first core of each package.
> > > + //
> > > + First = MAX_UINT32;
> > > + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> > > ProcessorNumber++) {
> > > + Location = &CpuFeaturesData-
> > > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> > > + if (Location->Package == PackageIndex) {
> >
> > Here the code is assuming Location->Package starts from 0 and consecutive.
>
> CpuStatus->PackageCount is assigned in CpuInitDataInitialize():
> > CpuStatus->PackageCount = Package + 1;
> > CpuStatus->MaxCoreCount = Core + 1;
> > CpuStatus->MaxThreadCount = Thread + 1;
> So PackageCount actually is the value of max package ID + 1.
> With that, the code change isn't assuming Location->Package starts from 0 and
> consecutive.
>
> >
> > Here the code is assuming Location->Package and Location->Core start from
> > 0 and consecutive.
> > We could not have this assumption, this patch is to resolve this assumption.
> Similarly, The code change above isn't assuming Location->Core starts from 0
> and consecutive.
>
> >
> >
> > Thanks,
> > Star
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#53641): https://edk2.groups.io/g/devel/message/53641
Mute This Topic: https://groups.io/mt/61962261/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Reviewed-by: Eric Dong <eric.dong@intel.com>
-----Original Message-----
From: Ray Ni <niruiyu@users.noreply.github.com>
Sent: Tuesday, November 26, 2019 2:16 PM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.
From: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1584
The flow of CPU feature initialization logic is:
1. BSP calls GetConfigDataFunc() for each thread/AP;
2. Each thread/AP calls SupportFunc() to detect its own capability;
3. BSP calls InitializeFunc() for each thread/AP.
There is a design gap in step #3. For a package scope feature that only
requires one thread of each package does the initialization operation,
what InitializeFunc() currently does is to do the initialization
operation only CPU physical location Core# is 0.
But in certain platform, Core#0 might be disabled in hardware level
which results the certain package scope feature isn't initialized at
all.
The patch adds a new field Fist to indicate the CPU's location in
its parent scope.
First.Package is set for all APs/threads under first package;
First.Core is set for all APs/threads under first core of each
package;
First.Thread is set for the AP/thread of each core.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
---
.../Include/Library/RegisterCpuFeaturesLib.h | 36 +++++++++
.../CpuFeaturesInitialize.c | 74 +++++++++++++++++++
2 files changed, 110 insertions(+)
diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
index d075606cdb..7114c8ce89 100644
--- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
+++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
@@ -78,6 +78,37 @@
#define CPU_FEATURE_END MAX_UINT32
/// @}
+///
+/// The bit field to indicate whether the processor is the first in its parent scope.
+///
+typedef struct {
+ //
+ // Set to 1 when current processor is the first thread in the core it resides in.
+ //
+ UINT32 Thread : 1;
+ //
+ // Set to 1 when current processor is a thread of the first core in the module it resides in.
+ //
+ UINT32 Core : 1;
+ //
+ // Set to 1 when current processor is a thread of the first module in the tile it resides in.
+ //
+ UINT32 Module : 1;
+ //
+ // Set to 1 when current processor is a thread of the first tile in the die it resides in.
+ //
+ UINT32 Tile : 1;
+ //
+ // Set to 1 when current processor is a thread of the first die in the package it resides in.
+ //
+ UINT32 Die : 1;
+ //
+ // Set to 1 when current processor is a thread of the first package in the system.
+ //
+ UINT32 Package : 1;
+ UINT32 Reserved : 26;
+} REGISTER_CPU_FEATURE_FIRST_PROCESSOR;
+
///
/// CPU Information passed into the SupportFunc and InitializeFunc of the
/// RegisterCpuFeature() library function. This structure contains information
@@ -88,6 +119,11 @@ typedef struct {
/// The package that the CPU resides
///
EFI_PROCESSOR_INFORMATION ProcessorInfo;
+
+ ///
+ /// The bit flag indicating whether the CPU is the first Thread/Core/Module/Tile/Die/Package in its parent scope.
+ ///
+ REGISTER_CPU_FEATURE_FIRST_PROCESSOR First;
///
/// The Display Family of the CPU computed from CPUID leaf CPUID_VERSION_INFO
///
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 0a4fcff033..23076fd453 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -105,6 +105,9 @@ CpuInitDataInitialize (
EFI_CPU_PHYSICAL_LOCATION *Location;
BOOLEAN *CoresVisited;
UINTN Index;
+ UINT32 PackageIndex;
+ UINT32 CoreIndex;
+ UINT32 First;
ACPI_CPU_DATA *AcpiCpuData;
CPU_STATUS_INFORMATION *CpuStatus;
UINT32 *ValidCoreCountPerPackage;
@@ -234,6 +237,77 @@ CpuInitDataInitialize (
ASSERT (CpuFeaturesData->CpuFlags.CoreSemaphoreCount != NULL);
CpuFeaturesData->CpuFlags.PackageSemaphoreCount = AllocateZeroPool (sizeof (UINT32) * CpuStatus->PackageCount * CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount);
ASSERT (CpuFeaturesData->CpuFlags.PackageSemaphoreCount != NULL);
+
+ //
+ // Initialize CpuFeaturesData->InitOrder[].CpuInfo.First
+ //
+
+ //
+ // Set First.Package for each thread belonging to the first package.
+ //
+ First = MAX_UINT32;
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+ First = MIN (Location->Package, First);
+ }
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+ if (Location->Package == First) {
+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Package = 1;
+ }
+ }
+
+ //
+ // Set First.Die/Tile/Module for each thread assuming:
+ // single Die under each package, single Tile under each Die, single Module under each Tile
+ //
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Die = 1;
+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Tile = 1;
+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Module = 1;
+ }
+
+ for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount; PackageIndex++) {
+ //
+ // Set First.Core for each thread in the first core of each package.
+ //
+ First = MAX_UINT32;
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+ if (Location->Package == PackageIndex) {
+ First = MIN (Location->Core, First);
+ }
+ }
+
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+ if (Location->Package == PackageIndex && Location->Core == First) {
+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Core = 1;
+ }
+ }
+ }
+
+ for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount; PackageIndex++) {
+ for (CoreIndex = 0; CoreIndex < CpuStatus->MaxCoreCount; CoreIndex++) {
+ //
+ // Set First.Thread for the first thread of each core.
+ //
+ First = MAX_UINT32;
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+ if (Location->Package == PackageIndex && Location->Core == CoreIndex) {
+ First = MIN (Location->Thread, First);
+ }
+ }
+
+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+ Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+ if (Location->Package == PackageIndex && Location->Core == CoreIndex && Location->Thread == First) {
+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Thread = 1;
+ }
+ }
+ }
+ }
}
/**
--
2.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#54256): https://edk2.groups.io/g/devel/message/54256
Mute This Topic: https://groups.io/mt/61962261/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.