public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] UefiCpuPkg/CpuFeature: Introduce FirstX to indicate 1st unit in parent scope.
@ 2019-07-05  3:51 Ni, Ray
  2019-07-09  0:49 ` Dong, Eric
  0 siblings, 1 reply; 2+ messages in thread
From: Ni, Ray @ 2019-07-05  3:51 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Star Zeng, Michael D Kinney

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 related	[flat|nested] 2+ messages in thread

* Re: [PATCH] UefiCpuPkg/CpuFeature: Introduce FirstX to indicate 1st unit in parent scope.
  2019-07-05  3:51 [PATCH] UefiCpuPkg/CpuFeature: Introduce FirstX to indicate 1st unit in parent scope Ni, Ray
@ 2019-07-09  0:49 ` Dong, Eric
  0 siblings, 0 replies; 2+ messages in thread
From: Dong, Eric @ 2019-07-09  0:49 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io; +Cc: Zeng, Star, Kinney, Michael D

Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: 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: [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] 2+ messages in thread

end of thread, other threads:[~2019-07-09  0:49 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-05  3:51 [PATCH] UefiCpuPkg/CpuFeature: Introduce FirstX to indicate 1st unit in parent scope Ni, Ray
2019-07-09  0:49 ` Dong, Eric

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox