* Re: [edk2-devel] [PATCH] UefiCpuPkg/CpuFeature: Introduce FirstX to indicate 1st unit in parent scope.
[not found] <15AE6662809251E0.3248@groups.io>
@ 2019-07-17 6:59 ` Ni, Ray
0 siblings, 0 replies; only message in thread
From: Ni, Ray @ 2019-07-17 6:59 UTC (permalink / raw)
To: Kinney, Michael D; +Cc: Dong, Eric, Zeng, Star, Ni, Ray, devel@edk2.groups.io
Mike,
Per last discussion in TianoCore Community Design Meeting
https://edk2.groups.io/g/announce/topic/tianocore_community_design/32438738,
"FirstX" is the only proposed option and you would like to see whether a better name
can be used.
Here I listed some candidates:
1. REGISTER_CPU_FEATURE_FIRST_X FirstX
2. REGISTER_CPU_FEATURE_LOGICAL_FIRST LogicalFirst
3. REGISTER_CPU_FEATURE_BSP Bsp
4. REGISTER_CPU_FEATURE_PROCESSOR_0 Processor0
5. REGISTER_CPU_FEATURE_VIRTUAL_FIRST VirtualFirst
Which one do you think is more proper? Or do you have any suggestions?
Thanks,
Ray
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Friday, July 5, 2019 11:51 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [edk2-devel] [PATCH] UefiCpuPkg/CpuFeature: Introduce FirstX to
> indicate 1st unit in parent scope.
>
> 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 thead/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 FistX to indicate the CPU's location in its parent
> scope.
> FirstX.Package is set for all APs/threads under first package; FirstX.Core is set
> for all APs/threads under first core of each package; FirstX.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 | 19 +++++
> .../CpuFeaturesInitialize.c | 74 +++++++++++++++++++
> 2 files changed, 93 insertions(+)
>
> diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> index 6f964027be..7c1e122ed7 100644
> --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> @@ -78,6 +78,20 @@
> #define CPU_FEATURE_END MAX_UINT32
> /// @}
>
> +///
> +/// The bit field to indicate whether the CPU is the first in its parent scope.
> +///
> +typedef struct {
> + UINT32 Thread : 1;
> + UINT32 Core : 1;
> + UINT32 Module : 1;
> + UINT32 Tile : 1;
> + UINT32 Die : 1;
> + UINT32 Package : 1;
> + UINT32 Reserved : 26;
> +} REGISTER_CPU_FEATURE_FIRST_X;
> +
> +
> ///
> /// CPU Information passed into the SupportFunc and InitializeFunc of the
> /// RegisterCpuFeature() library function. This structure contains information
> @@ -88,6 +102,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_X FirstX;
> ///
> /// 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 aff7ad600c..c7858bc3a8 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -110,6 +110,9 @@ CpuInitDataInitialize (
> EFI_CPU_PHYSICAL_LOCATION *Location;
> BOOLEAN *CoresVisited;
> UINTN Index;
> + UINT32 PackageIndex;
> + UINT32 CoreIndex;
> + UINT32 FirstIndex;
> ACPI_CPU_DATA *AcpiCpuData;
> CPU_STATUS_INFORMATION *CpuStatus;
> UINT32 *ValidCoreCountPerPackage;
> @@ -239,6 +242,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.FirstX
> + //
> +
> + //
> + // Set FirstX.Package for each thread belonging to the first package.
> + //
> + FirstIndex = MAX_UINT32;
> + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> + Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> + FirstIndex = MIN (Location->Package, FirstIndex); } for
> + (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> + Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> + if (Location->Package == FirstIndex) {
> + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.FirstX.Package
> = 1;
> + }
> + }
> +
> + //
> + // Set FirstX.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.FirstX.Die = 1;
> + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.FirstX.Tile = 1;
> + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.FirstX.Module =
> + 1; }
> +
> + for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount;
> PackageIndex++) {
> + //
> + // Set FirstX.Core for each thread belonging to the first core of each
> package.
> + //
> + FirstIndex = MAX_UINT32;
> + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> + Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> + if (Location->Package == PackageIndex) {
> + FirstIndex = MIN (Location->Core, FirstIndex);
> + }
> + }
> +
> + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> + Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> + if (Location->Package == PackageIndex && Location->Core == FirstIndex)
> {
> + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.FirstX.Core =
> 1;
> + }
> + }
> + }
> +
> + for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount;
> PackageIndex++) {
> + for (CoreIndex = 0; CoreIndex < CpuStatus->MaxCoreCount;
> CoreIndex++) {
> + //
> + // Set FirstX.Thread for the first thread of each core.
> + //
> + FirstIndex = MAX_UINT32;
> + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> + Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> + if (Location->Package == PackageIndex && Location->Core ==
> CoreIndex) {
> + FirstIndex = MIN (Location->Thread, FirstIndex);
> + }
> + }
> +
> + for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> + Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> + if (Location->Package == PackageIndex && Location->Core ==
> CoreIndex && Location->Thread == FirstIndex) {
> + CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.FirstX.Thread = 1;
> + }
> + }
> + }
> + }
> }
>
> /**
> --
> 2.21.0.windows.1
>
>
>
^ permalink raw reply [flat|nested] only message in thread