Re: [edk2-devel] [PATCH 17/18] UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c

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

[-- Attachment #1: Type: text/plain, Size: 14627 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 17/18] UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c

Remove GetAcpiCpuData() in CpuS3.c. The mAcpiCpuData
is not needed in S3 boot anymore.

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          | 243 +--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c |   6 ++----
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  24 ------------------------
 3 files changed, 3 insertions(+), 270 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index e84bc14de0..78ecf4efc6 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -9,22 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include "PiSmmCpuDxeSmm.h"
 #include <PiPei.h>

-//
-// Flags used when program the register.
-//
-typedef struct {
-  volatile UINTN     MemoryMappedLock;              // Spinlock used to program mmio
-  volatile UINT32    *CoreSemaphoreCount;           // Semaphore container used to program
-                                                    // core level semaphore.
-  volatile UINT32    *PackageSemaphoreCount;        // Semaphore container used to program
-                                                    // package level semaphore.
-} PROGRAM_CPU_REGISTER_FLAGS;
-
-#define LEGACY_REGION_SIZE  (2 * 0x1000)
-#define LEGACY_REGION_BASE  (0xA0000 - LEGACY_REGION_SIZE)
-
-ACPI_CPU_DATA  mAcpiCpuData;
-BOOLEAN        mRestoreSmmConfigurationInS3 = FALSE;
+BOOLEAN  mRestoreSmmConfigurationInS3 = FALSE;

 //
 // S3 boot flag
@@ -266,232 +251,6 @@ InitSmmS3ResumeState (
   }
 }

-/**
-  Copy register table from non-SMRAM into SMRAM.
-
-  @param[in] DestinationRegisterTableList  Points to destination register table.
-  @param[in] SourceRegisterTableList       Points to source register table.
-  @param[in] NumberOfCpus                  Number of CPUs.
-
-**/
-VOID
-CopyRegisterTable (
-  IN CPU_REGISTER_TABLE  *DestinationRegisterTableList,
-  IN CPU_REGISTER_TABLE  *SourceRegisterTableList,
-  IN UINT32              NumberOfCpus
-  )
-{
-  UINTN                     Index;
-  CPU_REGISTER_TABLE_ENTRY  *RegisterTableEntry;
-
-  CopyMem (DestinationRegisterTableList, SourceRegisterTableList, NumberOfCpus * sizeof (CPU_REGISTER_TABLE));
-  for (Index = 0; Index < NumberOfCpus; Index++) {
-    if (DestinationRegisterTableList[Index].TableLength != 0) {
-      DestinationRegisterTableList[Index].AllocatedSize = DestinationRegisterTableList[Index].TableLength * sizeof (CPU_REGISTER_TABLE_ENTRY);
-      RegisterTableEntry                                = AllocateCopyPool (
-                                                            DestinationRegisterTableList[Index].AllocatedSize,
-                                                            (VOID *)(UINTN)SourceRegisterTableList[Index].RegisterTableEntry
-                                                            );
-      ASSERT (RegisterTableEntry != NULL);
-      DestinationRegisterTableList[Index].RegisterTableEntry = (EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTableEntry;
-    }
-  }
-}
-
-/**
-  Check whether the register table is empty or not.
-
-  @param[in] RegisterTable  Point to the register table.
-  @param[in] NumberOfCpus   Number of CPUs.
-
-  @retval TRUE              The register table is empty.
-  @retval FALSE             The register table is not empty.
-**/
-BOOLEAN
-IsRegisterTableEmpty (
-  IN CPU_REGISTER_TABLE  *RegisterTable,
-  IN UINT32              NumberOfCpus
-  )
-{
-  UINTN  Index;
-
-  if (RegisterTable != NULL) {
-    for (Index = 0; Index < NumberOfCpus; Index++) {
-      if (RegisterTable[Index].TableLength != 0) {
-        return FALSE;
-      }
-    }
-  }
-
-  return TRUE;
-}
-
-/**
-  Copy the data used to initialize processor register into SMRAM.
-
-  @param[in,out]  CpuFeatureInitDataDst   Pointer to the destination CPU_FEATURE_INIT_DATA structure.
-  @param[in]      CpuFeatureInitDataSrc   Pointer to the source CPU_FEATURE_INIT_DATA structure.
-
-**/
-VOID
-CopyCpuFeatureInitDatatoSmram (
-  IN OUT CPU_FEATURE_INIT_DATA  *CpuFeatureInitDataDst,
-  IN     CPU_FEATURE_INIT_DATA  *CpuFeatureInitDataSrc
-  )
-{
-  CPU_STATUS_INFORMATION  *CpuStatus;
-
-  if (!IsRegisterTableEmpty ((CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->PreSmmInitRegisterTable, mAcpiCpuData.NumberOfCpus)) {
-    CpuFeatureInitDataDst->PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (mAcpiCpuData.NumberOfCpus * sizeof (CPU_REGISTER_TABLE));
-    ASSERT (CpuFeatureInitDataDst->PreSmmInitRegisterTable != 0);
-
-    CopyRegisterTable (
-      (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataDst->PreSmmInitRegisterTable,
-      (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->PreSmmInitRegisterTable,
-      mAcpiCpuData.NumberOfCpus
-      );
-  }
-
-  if (!IsRegisterTableEmpty ((CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->RegisterTable, mAcpiCpuData.NumberOfCpus)) {
-    CpuFeatureInitDataDst->RegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (mAcpiCpuData.NumberOfCpus * sizeof (CPU_REGISTER_TABLE));
-    ASSERT (CpuFeatureInitDataDst->RegisterTable != 0);
-
-    CopyRegisterTable (
-      (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataDst->RegisterTable,
-      (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->RegisterTable,
-      mAcpiCpuData.NumberOfCpus
-      );
-  }
-
-  CpuStatus = &CpuFeatureInitDataDst->CpuStatus;
-  CopyMem (CpuStatus, &CpuFeatureInitDataSrc->CpuStatus, sizeof (CPU_STATUS_INFORMATION));
-
-  if (CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerPackage != 0) {
-    CpuStatus->ThreadCountPerPackage = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool (
-                                                                      sizeof (UINT32) * CpuStatus->PackageCount,
-                                                                      (UINT32 *)(UINTN)CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerPackage
-                                                                      );
-    ASSERT (CpuStatus->ThreadCountPerPackage != 0);
-  }
-
-  if (CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerCore != 0) {
-    CpuStatus->ThreadCountPerCore = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool (
-                                                                   sizeof (UINT8) * (CpuStatus->PackageCount * CpuStatus->MaxCoreCount),
-                                                                   (UINT32 *)(UINTN)CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerCore
-                                                                   );
-    ASSERT (CpuStatus->ThreadCountPerCore != 0);
-  }
-
-  if (CpuFeatureInitDataSrc->ApLocation != 0) {
-    CpuFeatureInitDataDst->ApLocation = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool (
-                                                                       mAcpiCpuData.NumberOfCpus * sizeof (EFI_CPU_PHYSICAL_LOCATION),
-                                                                       (EFI_CPU_PHYSICAL_LOCATION *)(UINTN)CpuFeatureInitDataSrc->ApLocation
-                                                                       );
-    ASSERT (CpuFeatureInitDataDst->ApLocation != 0);
-  }
-}
-
-/**
-  Get ACPI CPU data.
-
-**/
-VOID
-GetAcpiCpuData (
-  VOID
-  )
-{
-  ACPI_CPU_DATA           *AcpiCpuData;
-  IA32_DESCRIPTOR         *Gdtr;
-  IA32_DESCRIPTOR         *Idtr;
-  VOID                    *GdtForAp;
-  VOID                    *IdtForAp;
-  VOID                    *MachineCheckHandlerForAp;
-  CPU_STATUS_INFORMATION  *CpuStatus;
-
-  if (!mAcpiS3Enable) {
-    return;
-  }
-
-  //
-  // Prevent use of mAcpiCpuData by initialize NumberOfCpus to 0
-  //
-  mAcpiCpuData.NumberOfCpus = 0;
-
-  //
-  // If PcdCpuS3DataAddress was never set, then do not copy CPU S3 Data into SMRAM
-  //
-  AcpiCpuData = (ACPI_CPU_DATA *)(UINTN)PcdGet64 (PcdCpuS3DataAddress);
-  if (AcpiCpuData == 0) {
-    return;
-  }
-
-  //
-  // For a native platform, copy the CPU S3 data into SMRAM for use on CPU S3 Resume.
-  //
-  CopyMem (&mAcpiCpuData, AcpiCpuData, sizeof (mAcpiCpuData));
-
-  mAcpiCpuData.MtrrTable = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (sizeof (MTRR_SETTINGS));
-  ASSERT (mAcpiCpuData.MtrrTable != 0);
-
-  CopyMem ((VOID *)(UINTN)mAcpiCpuData.MtrrTable, (VOID *)(UINTN)AcpiCpuData->MtrrTable, sizeof (MTRR_SETTINGS));
-
-  mAcpiCpuData.GdtrProfile = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (sizeof (IA32_DESCRIPTOR));
-  ASSERT (mAcpiCpuData.GdtrProfile != 0);
-
-  CopyMem ((VOID *)(UINTN)mAcpiCpuData.GdtrProfile, (VOID *)(UINTN)AcpiCpuData->GdtrProfile, sizeof (IA32_DESCRIPTOR));
-
-  mAcpiCpuData.IdtrProfile = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (sizeof (IA32_DESCRIPTOR));
-  ASSERT (mAcpiCpuData.IdtrProfile != 0);
-
-  CopyMem ((VOID *)(UINTN)mAcpiCpuData.IdtrProfile, (VOID *)(UINTN)AcpiCpuData->IdtrProfile, sizeof (IA32_DESCRIPTOR));
-
-  //
-  // Copy AP's GDT, IDT and Machine Check handler into SMRAM.
-  //
-  Gdtr = (IA32_DESCRIPTOR *)(UINTN)mAcpiCpuData.GdtrProfile;
-  Idtr = (IA32_DESCRIPTOR *)(UINTN)mAcpiCpuData.IdtrProfile;
-
-  GdtForAp = AllocatePool ((Gdtr->Limit + 1) + (Idtr->Limit + 1) + mAcpiCpuData.ApMachineCheckHandlerSize);
-  ASSERT (GdtForAp != NULL);
-  IdtForAp                 = (VOID *)((UINTN)GdtForAp + (Gdtr->Limit + 1));
-  MachineCheckHandlerForAp = (VOID *)((UINTN)IdtForAp + (Idtr->Limit + 1));
-
-  CopyMem (GdtForAp, (VOID *)Gdtr->Base, Gdtr->Limit + 1);
-  CopyMem (IdtForAp, (VOID *)Idtr->Base, Idtr->Limit + 1);
-  CopyMem (MachineCheckHandlerForAp, (VOID *)(UINTN)mAcpiCpuData.ApMachineCheckHandlerBase, mAcpiCpuData.ApMachineCheckHandlerSize);
-
-  Gdtr->Base                             = (UINTN)GdtForAp;
-  Idtr->Base                             = (UINTN)IdtForAp;
-  mAcpiCpuData.ApMachineCheckHandlerBase = (EFI_PHYSICAL_ADDRESS)(UINTN)MachineCheckHandlerForAp;
-
-  ZeroMem (&mAcpiCpuData.CpuFeatureInitData, sizeof (CPU_FEATURE_INIT_DATA));
-
-  if (!PcdGetBool (PcdCpuFeaturesInitOnS3Resume)) {
-    //
-    // If the CPU features will not be initialized by CpuFeaturesPei module during
-    // next ACPI S3 resume, copy the CPU features initialization data into SMRAM,
-    // which will be consumed in SmmRestoreCpu during next S3 resume.
-    //
-    CopyCpuFeatureInitDatatoSmram (&mAcpiCpuData.CpuFeatureInitData, &AcpiCpuData->CpuFeatureInitData);
-
-    CpuStatus = &mAcpiCpuData.CpuFeatureInitData.CpuStatus;
-
-    mCpuFlags.CoreSemaphoreCount = AllocateZeroPool (
-                                     sizeof (UINT32) * CpuStatus->PackageCount *
-                                     CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount
-                                     );
-    ASSERT (mCpuFlags.CoreSemaphoreCount != NULL);
-
-    mCpuFlags.PackageSemaphoreCount = AllocateZeroPool (
-                                        sizeof (UINT32) * CpuStatus->PackageCount *
-                                        CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount
-                                        );
-    ASSERT (mCpuFlags.PackageSemaphoreCount != NULL);
-
-    InitializeSpinLock ((SPIN_LOCK *)&mCpuFlags.MemoryMappedLock);
-  }
-}
-
 /**
   Get ACPI S3 enable flag.

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index e400bee8d5..f5e113d99f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -435,8 +435,8 @@ ExecuteFirstSmiInit (
 /**
   SMM Ready To Lock event notification handler.

-  The CPU S3 data is copied to SMRAM for security and mSmmReadyToLock is set to
-  perform additional lock actions that must be performed from SMM on the next SMI.
+  mSmmReadyToLock is set to perform additional lock actions that must be
+  performed from SMM on the next SMI.

   @param[in] Protocol   Points to the protocol's unique identifier.
   @param[in] Interface  Points to the interface instance.
@@ -452,8 +452,6 @@ SmmReadyToLockEventNotify (
   IN EFI_HANDLE      Handle
   )
 {
-  GetAcpiCpuData ();
-
   //
   // Cache a copy of UEFI memory map before we start profiling feature.
   //
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index f42910ddf1..af0fda4da1 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -1048,15 +1048,6 @@ InitSmmS3ResumeState (
   IN UINT32  Cr3
   );

-/**
-  Get ACPI CPU data.
-
-**/
-VOID
-GetAcpiCpuData (
-  VOID
-  );
-
 /**
   Restore SMM Configuration in S3 boot path.

@@ -1075,21 +1066,6 @@ GetAcpiS3EnableFlag (
   VOID
   );

-/**
-  Transfer AP to safe hlt-loop after it finished restore CPU features on S3 patch.
-
-  @param[in] ApHltLoopCode          The address of the safe hlt-loop function.
-  @param[in] TopOfStack             A pointer to the new stack to use for the ApHltLoopCode.
-  @param[in] NumberToFinishAddress  Address of Semaphore of APs finish count.
-
-**/
-VOID
-TransferApToSafeState (
-  IN UINTN  ApHltLoopCode,
-  IN UINTN  TopOfStack,
-  IN UINTN  NumberToFinishAddress
-  );
-
 /**
   Set ShadowStack memory.

--
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118848): https://edk2.groups.io/g/devel/message/118848
Mute This Topic: https://groups.io/mt/106018144/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: 27303 bytes --]