Re: [edk2-devel] [PATCH 14/18] UefiCpuPkg: Remove code to set register table

["Ni, Ray" <ray.ni@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 16918 bytes --]

Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
________________________________
From: Tan, Dun <dun.tan@intel.com>
Sent: Friday, May 10, 2024 18:08
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [PATCH 14/18] UefiCpuPkg: Remove code to set register table

Remove code to set register table in CpuS3.c.
In previous commit, PcdCpuFeaturesInitOnS3Resume
has been set to TRUE. So that CpuFeaturesPei PEIM
will initialize the CPU registers and perform CPU
features initialization.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 423 ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 1 file changed, 423 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 7ac6b62676..9520451d92 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -91,425 +91,6 @@ UINT8  mApHltLoopCodeTemplate[] = {
   0xEB, 0xFC              // jmp $-2
 };

-/**
-  Increment semaphore by 1.
-
-  @param      Sem            IN:  32-bit unsigned integer
-
-**/
-VOID
-S3ReleaseSemaphore (
-  IN OUT  volatile UINT32  *Sem
-  )
-{
-  InterlockedIncrement (Sem);
-}
-
-/**
-  Decrement the semaphore by 1 if it is not zero.
-
-  Performs an atomic decrement operation for semaphore.
-  The compare exchange operation must be performed using
-  MP safe mechanisms.
-
-  @param      Sem            IN:  32-bit unsigned integer
-
-**/
-VOID
-S3WaitForSemaphore (
-  IN OUT  volatile UINT32  *Sem
-  )
-{
-  UINT32  Value;
-
-  do {
-    Value = *Sem;
-  } while (Value == 0 ||
-           InterlockedCompareExchange32 (
-             Sem,
-             Value,
-             Value - 1
-             ) != Value);
-}
-
-/**
-  Read / write CR value.
-
-  @param[in]      CrIndex         The CR index which need to read/write.
-  @param[in]      Read            Read or write. TRUE is read.
-  @param[in,out]  CrValue         CR value.
-
-  @retval    EFI_SUCCESS means read/write success, else return EFI_UNSUPPORTED.
-**/
-UINTN
-ReadWriteCr (
-  IN     UINT32   CrIndex,
-  IN     BOOLEAN  Read,
-  IN OUT UINTN    *CrValue
-  )
-{
-  switch (CrIndex) {
-    case 0:
-      if (Read) {
-        *CrValue = AsmReadCr0 ();
-      } else {
-        AsmWriteCr0 (*CrValue);
-      }
-
-      break;
-    case 2:
-      if (Read) {
-        *CrValue = AsmReadCr2 ();
-      } else {
-        AsmWriteCr2 (*CrValue);
-      }
-
-      break;
-    case 3:
-      if (Read) {
-        *CrValue = AsmReadCr3 ();
-      } else {
-        AsmWriteCr3 (*CrValue);
-      }
-
-      break;
-    case 4:
-      if (Read) {
-        *CrValue = AsmReadCr4 ();
-      } else {
-        AsmWriteCr4 (*CrValue);
-      }
-
-      break;
-    default:
-      return EFI_UNSUPPORTED;
-  }
-
-  return EFI_SUCCESS;
-}
-
-/**
-  Initialize the CPU registers from a register table.
-
-  @param[in]  RegisterTable         The register table for this AP.
-  @param[in]  ApLocation            AP location info for this ap.
-  @param[in]  CpuStatus             CPU status info for this CPU.
-  @param[in]  CpuFlags              Flags data structure used when program the register.
-
-  @note This service could be called by BSP/APs.
-**/
-VOID
-ProgramProcessorRegister (
-  IN CPU_REGISTER_TABLE          *RegisterTable,
-  IN EFI_CPU_PHYSICAL_LOCATION   *ApLocation,
-  IN CPU_STATUS_INFORMATION      *CpuStatus,
-  IN PROGRAM_CPU_REGISTER_FLAGS  *CpuFlags
-  )
-{
-  CPU_REGISTER_TABLE_ENTRY  *RegisterTableEntry;
-  UINTN                     Index;
-  UINTN                     Value;
-  CPU_REGISTER_TABLE_ENTRY  *RegisterTableEntryHead;
-  volatile UINT32           *SemaphorePtr;
-  UINT32                    FirstThread;
-  UINT32                    CurrentThread;
-  UINT32                    CurrentCore;
-  UINTN                     ProcessorIndex;
-  UINT32                    *ThreadCountPerPackage;
-  UINT8                     *ThreadCountPerCore;
-  EFI_STATUS                Status;
-  UINT64                    CurrentValue;
-
-  //
-  // Traverse Register Table of this logical processor
-  //
-  RegisterTableEntryHead = (CPU_REGISTER_TABLE_ENTRY *)(UINTN)RegisterTable->RegisterTableEntry;
-
-  for (Index = 0; Index < RegisterTable->TableLength; Index++) {
-    RegisterTableEntry = &RegisterTableEntryHead[Index];
-
-    //
-    // Check the type of specified register
-    //
-    switch (RegisterTableEntry->RegisterType) {
-      //
-      // The specified register is Control Register
-      //
-      case ControlRegister:
-        Status = ReadWriteCr (RegisterTableEntry->Index, TRUE, &Value);
-        if (EFI_ERROR (Status)) {
-          break;
-        }
-
-        if (RegisterTableEntry->TestThenWrite) {
-          CurrentValue = BitFieldRead64 (
-                           Value,
-                           RegisterTableEntry->ValidBitStart,
-                           RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1
-                           );
-          if (CurrentValue == RegisterTableEntry->Value) {
-            break;
-          }
-        }
-
-        Value = (UINTN)BitFieldWrite64 (
-                         Value,
-                         RegisterTableEntry->ValidBitStart,
-                         RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
-                         RegisterTableEntry->Value
-                         );
-        ReadWriteCr (RegisterTableEntry->Index, FALSE, &Value);
-        break;
-      //
-      // The specified register is Model Specific Register
-      //
-      case Msr:
-        if (RegisterTableEntry->TestThenWrite) {
-          Value = (UINTN)AsmReadMsr64 (RegisterTableEntry->Index);
-          if (RegisterTableEntry->ValidBitLength >= 64) {
-            if (Value == RegisterTableEntry->Value) {
-              break;
-            }
-          } else {
-            CurrentValue = BitFieldRead64 (
-                             Value,
-                             RegisterTableEntry->ValidBitStart,
-                             RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1
-                             );
-            if (CurrentValue == RegisterTableEntry->Value) {
-              break;
-            }
-          }
-        }
-
-        //
-        // If this function is called to restore register setting after INIT signal,
-        // there is no need to restore MSRs in register table.
-        //
-        if (RegisterTableEntry->ValidBitLength >= 64) {
-          //
-          // If length is not less than 64 bits, then directly write without reading
-          //
-          AsmWriteMsr64 (
-            RegisterTableEntry->Index,
-            RegisterTableEntry->Value
-            );
-        } else {
-          //
-          // Set the bit section according to bit start and length
-          //
-          AsmMsrBitFieldWrite64 (
-            RegisterTableEntry->Index,
-            RegisterTableEntry->ValidBitStart,
-            RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
-            RegisterTableEntry->Value
-            );
-        }
-
-        break;
-      //
-      // MemoryMapped operations
-      //
-      case MemoryMapped:
-        AcquireSpinLock (&CpuFlags->MemoryMappedLock);
-        MmioBitFieldWrite32 (
-          (UINTN)(RegisterTableEntry->Index | LShiftU64 (RegisterTableEntry->HighIndex, 32)),
-          RegisterTableEntry->ValidBitStart,
-          RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
-          (UINT32)RegisterTableEntry->Value
-          );
-        ReleaseSpinLock (&CpuFlags->MemoryMappedLock);
-        break;
-      //
-      // Enable or disable cache
-      //
-      case CacheControl:
-        //
-        // If value of the entry is 0, then disable cache.  Otherwise, enable cache.
-        //
-        if (RegisterTableEntry->Value == 0) {
-          AsmDisableCache ();
-        } else {
-          AsmEnableCache ();
-        }
-
-        break;
-
-      case Semaphore:
-        // Semaphore works logic like below:
-        //
-        //  V(x) = LibReleaseSemaphore (Semaphore[FirstThread + x]);
-        //  P(x) = LibWaitForSemaphore (Semaphore[FirstThread + x]);
-        //
-        //  All threads (T0...Tn) waits in P() line and continues running
-        //  together.
-        //
-        //
-        //  T0             T1            ...           Tn
-        //
-        //  V(0...n)       V(0...n)      ...           V(0...n)
-        //  n * P(0)       n * P(1)      ...           n * P(n)
-        //
-        ASSERT (
-          (ApLocation != NULL) &&
-          (CpuStatus->ThreadCountPerPackage != 0) &&
-          (CpuStatus->ThreadCountPerCore != 0) &&
-          (CpuFlags->CoreSemaphoreCount != NULL) &&
-          (CpuFlags->PackageSemaphoreCount != NULL)
-          );
-        switch (RegisterTableEntry->Value) {
-          case CoreDepType:
-            SemaphorePtr       = CpuFlags->CoreSemaphoreCount;
-            ThreadCountPerCore = (UINT8 *)(UINTN)CpuStatus->ThreadCountPerCore;
-
-            CurrentCore = ApLocation->Package * CpuStatus->MaxCoreCount + ApLocation->Core;
-            //
-            // Get Offset info for the first thread in the core which current thread belongs to.
-            //
-            FirstThread   = CurrentCore * CpuStatus->MaxThreadCount;
-            CurrentThread = FirstThread + ApLocation->Thread;
-
-            //
-            // Different cores may have different valid threads in them. If driver maintail clearly
-            // thread index in different cores, the logic will be much complicated.
-            // Here driver just simply records the max thread number in all cores and use it as expect
-            // thread number for all cores.
-            // In below two steps logic, first current thread will Release semaphore for each thread
-            // in current core. Maybe some threads are not valid in this core, but driver don't
-            // care. Second, driver will let current thread wait semaphore for all valid threads in
-            // current core. Because only the valid threads will do release semaphore for this
-            // thread, driver here only need to wait the valid thread count.
-            //
-
-            //
-            // First Notify ALL THREADs in current Core that this thread is ready.
-            //
-            for (ProcessorIndex = 0; ProcessorIndex < CpuStatus->MaxThreadCount; ProcessorIndex++) {
-              S3ReleaseSemaphore (&SemaphorePtr[FirstThread + ProcessorIndex]);
-            }
-
-            //
-            // Second, check whether all VALID THREADs (not all threads) in current core are ready.
-            //
-            for (ProcessorIndex = 0; ProcessorIndex < ThreadCountPerCore[CurrentCore]; ProcessorIndex++) {
-              S3WaitForSemaphore (&SemaphorePtr[CurrentThread]);
-            }
-
-            break;
-
-          case PackageDepType:
-            SemaphorePtr          = CpuFlags->PackageSemaphoreCount;
-            ThreadCountPerPackage = (UINT32 *)(UINTN)CpuStatus->ThreadCountPerPackage;
-            //
-            // Get Offset info for the first thread in the package which current thread belongs to.
-            //
-            FirstThread = ApLocation->Package * CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount;
-            //
-            // Get the possible threads count for current package.
-            //
-            CurrentThread = FirstThread + CpuStatus->MaxThreadCount * ApLocation->Core + ApLocation->Thread;
-
-            //
-            // Different packages may have different valid threads in them. If driver maintail clearly
-            // thread index in different packages, the logic will be much complicated.
-            // Here driver just simply records the max thread number in all packages and use it as expect
-            // thread number for all packages.
-            // In below two steps logic, first current thread will Release semaphore for each thread
-            // in current package. Maybe some threads are not valid in this package, but driver don't
-            // care. Second, driver will let current thread wait semaphore for all valid threads in
-            // current package. Because only the valid threads will do release semaphore for this
-            // thread, driver here only need to wait the valid thread count.
-            //
-
-            //
-            // First Notify ALL THREADS in current package that this thread is ready.
-            //
-            for (ProcessorIndex = 0; ProcessorIndex < CpuStatus->MaxThreadCount * CpuStatus->MaxCoreCount; ProcessorIndex++) {
-              S3ReleaseSemaphore (&SemaphorePtr[FirstThread + ProcessorIndex]);
-            }
-
-            //
-            // Second, check whether VALID THREADS (not all threads) in current package are ready.
-            //
-            for (ProcessorIndex = 0; ProcessorIndex < ThreadCountPerPackage[ApLocation->Package]; ProcessorIndex++) {
-              S3WaitForSemaphore (&SemaphorePtr[CurrentThread]);
-            }
-
-            break;
-
-          default:
-            break;
-        }
-
-        break;
-
-      default:
-        break;
-    }
-  }
-}
-
-/**
-
-  Set Processor register for one AP.
-
-  @param     PreSmmRegisterTable     Use pre Smm register table or register table.
-
-**/
-VOID
-SetRegister (
-  IN BOOLEAN  PreSmmRegisterTable
-  )
-{
-  CPU_FEATURE_INIT_DATA  *FeatureInitData;
-  CPU_REGISTER_TABLE     *RegisterTable;
-  CPU_REGISTER_TABLE     *RegisterTables;
-  UINT32                 InitApicId;
-  UINTN                  ProcIndex;
-  UINTN                  Index;
-
-  FeatureInitData = &mAcpiCpuData.CpuFeatureInitData;
-
-  if (PreSmmRegisterTable) {
-    RegisterTables = (CPU_REGISTER_TABLE *)(UINTN)FeatureInitData->PreSmmInitRegisterTable;
-  } else {
-    RegisterTables = (CPU_REGISTER_TABLE *)(UINTN)FeatureInitData->RegisterTable;
-  }
-
-  if (RegisterTables == NULL) {
-    return;
-  }
-
-  InitApicId    = GetInitialApicId ();
-  RegisterTable = NULL;
-  ProcIndex     = (UINTN)-1;
-  for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) {
-    if (RegisterTables[Index].InitialApicId == InitApicId) {
-      RegisterTable = &RegisterTables[Index];
-      ProcIndex     = Index;
-      break;
-    }
-  }
-
-  ASSERT (RegisterTable != NULL);
-
-  if (FeatureInitData->ApLocation != 0) {
-    ProgramProcessorRegister (
-      RegisterTable,
-      (EFI_CPU_PHYSICAL_LOCATION *)(UINTN)FeatureInitData->ApLocation + ProcIndex,
-      &FeatureInitData->CpuStatus,
-      &mCpuFlags
-      );
-  } else {
-    ProgramProcessorRegister (
-      RegisterTable,
-      NULL,
-      &FeatureInitData->CpuStatus,
-      &mCpuFlags
-      );
-  }
-}
-
 /**
   The function is invoked before SMBASE relocation in S3 path to restores CPU status.

@@ -524,8 +105,6 @@ InitializeCpuBeforeRebase (
   IN BOOLEAN  IsBsp
   )
 {
-  SetRegister (TRUE);
-
   ProgramVirtualWireMode ();
   if (!IsBsp) {
     DisableLvtInterrupts ();
@@ -563,8 +142,6 @@ InitializeCpuAfterRebase (
   UINTN  TopOfStack;
   UINT8  Stack[128];

-  SetRegister (FALSE);
-
   if (mSmmS3ResumeState->MpService2Ppi == 0) {
     if (IsBsp) {
       while (mNumberToFinish > 0) {
--
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118845): https://edk2.groups.io/g/devel/message/118845
Mute This Topic: https://groups.io/mt/106018140/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 35784 bytes --]