V1 changes:
> Current code logic can't confirm CpuS3DataDxe driver start before
> CpuFeaturesDxe driver. So the assumption in CpuFeaturesDxe not valid.
> Add implementation for AllocateAcpiCpuData function to remove this
> assumption.

V2 changes:
> Because CpuS3Data memory will be copy to smram at SmmReadToLock point,
> so the memory type no need to be ACPI NVS type, also the address not
> limit to below 4G.
> This change remove the limit of ACPI NVS memory type and below 4G.

V3 changes:
> Remove function definition in header file.
> Add STATIC in function implementation.

Pass OS boot and resume from S3 test.

Bugz: https://bugzilla.tianocore.org/show_bug.cgi?id=959

Reported-by: Marvin Häuser <Marvin.Haeuser@outlook.com>
Suggested-by: Fan Jeff <vanjeff_919@hotmail.com>
Cc: Marvin Häuser <Marvin.Haeuser@outlook.com>
Cc: Fan Jeff <vanjeff_919@hotmail.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
---
 .../DxeRegisterCpuFeaturesLib.c                    |  67 -----------
 .../PeiRegisterCpuFeaturesLib.c                    | 131 ---------------------
 .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  20 ----
 .../RegisterCpuFeaturesLib.c                       |  92 +++++++++++++++
 4 files changed, 92 insertions(+), 218 deletions(-)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
index 902a339529..1f34a3f489 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
@@ -197,70 +197,3 @@ GetNumberOfProcessor (
   ASSERT_EFI_ERROR (Status);
 }
 
-/**
-  Allocates ACPI NVS memory to save ACPI_CPU_DATA.
-
-  @return  Pointer to allocated ACPI_CPU_DATA.
-**/
-ACPI_CPU_DATA *
-AllocateAcpiCpuData (
-  VOID
-  )
-{
-  //
-  // CpuS3DataDxe will do it.
-  //
-  ASSERT (FALSE);
-  return NULL;
-}
-
-/**
-  Enlarges CPU register table for each processor.
-
-  @param[in, out]  RegisterTable   Pointer processor's CPU register table
-**/
-VOID
-EnlargeRegisterTable (
-  IN OUT CPU_REGISTER_TABLE            *RegisterTable
-  )
-{
-  EFI_STATUS            Status;
-  EFI_PHYSICAL_ADDRESS  Address;
-  UINTN                 AllocatePages;
-
-  Address = BASE_4GB - 1;
-  AllocatePages = RegisterTable->AllocatedSize / EFI_PAGE_SIZE;
-  Status  = gBS->AllocatePages (
-                   AllocateMaxAddress,
-                   EfiACPIMemoryNVS,
-                   AllocatePages + 1,
-                   &Address
-                   );
-  ASSERT_EFI_ERROR (Status);
-
-  //
-  // If there are records existing in the register table, then copy its contents
-  // to new region and free the old one.
-  //
-  if (RegisterTable->AllocatedSize > 0) {
-    CopyMem (
-      (VOID *) (UINTN) Address,
-      (VOID *) (UINTN) RegisterTable->RegisterTableEntry,
-      RegisterTable->AllocatedSize
-      );
-    //
-    // RegisterTableEntry is allocated by gBS->AllocatePages() service.
-    // So, gBS->FreePages() service is used to free it.
-    //
-    gBS->FreePages (
-      RegisterTable->RegisterTableEntry,
-      AllocatePages
-      );
-  }
-
-  //
-  // Adjust the allocated size and register table base address.
-  //
-  RegisterTable->AllocatedSize     += EFI_PAGE_SIZE;
-  RegisterTable->RegisterTableEntry = Address;
-}
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c
index 6804eddf65..82fe268812 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c
@@ -257,134 +257,3 @@ GetNumberOfProcessor (
                          );
   ASSERT_EFI_ERROR (Status);
 }
-
-/**
-  Allocates ACPI NVS memory to save ACPI_CPU_DATA.
-
-  @return  Pointer to allocated ACPI_CPU_DATA.
-**/
-ACPI_CPU_DATA *
-AllocateAcpiCpuData (
-  VOID
-  )
-{
-  EFI_STATUS                           Status;
-  EFI_PEI_MP_SERVICES_PPI              *CpuMpPpi;
-  UINTN                                NumberOfCpus;
-  UINTN                                NumberOfEnabledProcessors;
-  ACPI_CPU_DATA                        *AcpiCpuData;
-  EFI_PHYSICAL_ADDRESS                 Address;
-  UINTN                                TableSize;
-  CPU_REGISTER_TABLE                   *RegisterTable;
-  UINTN                                Index;
-  EFI_PROCESSOR_INFORMATION            ProcessorInfoBuffer;
-
-  Status = PeiServicesAllocatePages (
-             EfiACPIMemoryNVS,
-             EFI_SIZE_TO_PAGES (sizeof (ACPI_CPU_DATA)),
-             &Address
-             );
-  ASSERT_EFI_ERROR (Status);
-  AcpiCpuData = (ACPI_CPU_DATA *) (UINTN) Address;
-  ASSERT (AcpiCpuData != NULL);
-
-  //
-  // Get MP Services Protocol
-  //
-  Status = PeiServicesLocatePpi (
-             &gEfiPeiMpServicesPpiGuid,
-             0,
-             NULL,
-             (VOID **)&CpuMpPpi
-             );
-  ASSERT_EFI_ERROR (Status);
-
-  //
-  // Get the number of CPUs
-  //
-  Status = CpuMpPpi->GetNumberOfProcessors (
-                         GetPeiServicesTablePointer (),
-                         CpuMpPpi,
-                         &NumberOfCpus,
-                         &NumberOfEnabledProcessors
-                         );
-  ASSERT_EFI_ERROR (Status);
-  AcpiCpuData->NumberOfCpus = (UINT32)NumberOfCpus;
-
-  //
-  // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for all CPUs
-  //
-  TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
-  Status = PeiServicesAllocatePages (
-             EfiACPIMemoryNVS,
-             EFI_SIZE_TO_PAGES (TableSize),
-             &Address
-             );
-  ASSERT_EFI_ERROR (Status);
-  RegisterTable = (CPU_REGISTER_TABLE *) (UINTN) Address;
-
-  for (Index = 0; Index < NumberOfCpus; Index++) {
-    Status = CpuMpPpi->GetProcessorInfo (
-                         GetPeiServicesTablePointer (),
-                         CpuMpPpi,
-                         Index,
-                         &ProcessorInfoBuffer
-                         );
-    ASSERT_EFI_ERROR (Status);
-
-    RegisterTable[Index].InitialApicId      = (UINT32)ProcessorInfoBuffer.ProcessorId;
-    RegisterTable[Index].TableLength        = 0;
-    RegisterTable[Index].AllocatedSize      = 0;
-    RegisterTable[Index].RegisterTableEntry = 0;
-
-    RegisterTable[NumberOfCpus + Index].InitialApicId      = (UINT32)ProcessorInfoBuffer.ProcessorId;
-    RegisterTable[NumberOfCpus + Index].TableLength        = 0;
-    RegisterTable[NumberOfCpus + Index].AllocatedSize      = 0;
-    RegisterTable[NumberOfCpus + Index].RegisterTableEntry = 0;
-  }
-  AcpiCpuData->RegisterTable           = (EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTable;
-  AcpiCpuData->PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)(RegisterTable + NumberOfCpus);
-
-  return AcpiCpuData;
-}
-
-/**
-  Enlarges CPU register table for each processor.
-
-  @param[in, out]  RegisterTable   Pointer processor's CPU register table
-**/
-VOID
-EnlargeRegisterTable (
-  IN OUT CPU_REGISTER_TABLE            *RegisterTable
-  )
-{
-  EFI_STATUS                           Status;
-  EFI_PHYSICAL_ADDRESS                 Address;
-  UINTN                                AllocatePages;
-
-  AllocatePages = RegisterTable->AllocatedSize / EFI_PAGE_SIZE;
-  Status = PeiServicesAllocatePages (
-             EfiACPIMemoryNVS,
-             AllocatePages + 1,
-             &Address
-             );
-  ASSERT_EFI_ERROR (Status);
-
-  //
-  // If there are records existing in the register table, then copy its contents
-  // to new region and free the old one.
-  //
-  if (RegisterTable->AllocatedSize > 0) {
-    CopyMem (
-      (VOID *) (UINTN) Address,
-      (VOID *) (UINTN) RegisterTable->RegisterTableEntry,
-      RegisterTable->AllocatedSize
-      );
-  }
-
-  //
-  // Adjust the allocated size and register table base address.
-  //
-  RegisterTable->AllocatedSize += EFI_PAGE_SIZE;
-  RegisterTable->RegisterTableEntry = Address;
-}
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
index 69b412172a..edd266934f 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
@@ -87,26 +87,6 @@ GetCpuFeaturesData (
   VOID
   );
 
-/**
-  Enlarges CPU register table for each processor.
-
-  @param[in, out]  RegisterTable   Pointer processor's CPU register table
-**/
-VOID
-EnlargeRegisterTable (
-  IN OUT CPU_REGISTER_TABLE            *RegisterTable
-  );
-
-/**
-  Allocates ACPI NVS memory to save ACPI_CPU_DATA.
-
-  @return  Pointer to allocated ACPI_CPU_DATA.
-**/
-ACPI_CPU_DATA *
-AllocateAcpiCpuData (
-  VOID
-  );
-
 /**
   Worker function to return processor index.
 
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
index dd6a82be7a..4143ee4bb1 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -488,6 +488,98 @@ RegisterCpuFeature (
   return RETURN_SUCCESS;
 }
 
+/**
+  Allocates boot service data to save ACPI_CPU_DATA.
+
+  @return  Pointer to allocated ACPI_CPU_DATA.
+**/
+STATIC
+ACPI_CPU_DATA *
+AllocateAcpiCpuData (
+  VOID
+  )
+{
+  EFI_STATUS                           Status;
+  UINTN                                NumberOfCpus;
+  UINTN                                NumberOfEnabledProcessors;
+  ACPI_CPU_DATA                        *AcpiCpuData;
+  UINTN                                TableSize;
+  CPU_REGISTER_TABLE                   *RegisterTable;
+  UINTN                                Index;
+  EFI_PROCESSOR_INFORMATION            ProcessorInfoBuffer;
+
+  AcpiCpuData  = AllocatePages (EFI_SIZE_TO_PAGES (sizeof (ACPI_CPU_DATA)));
+  ASSERT (AcpiCpuData != NULL);
+
+  GetNumberOfProcessor (&NumberOfCpus, &NumberOfEnabledProcessors);
+  AcpiCpuData->NumberOfCpus = (UINT32)NumberOfCpus;
+
+  //
+  // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for all CPUs
+  //  
+  TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
+  RegisterTable  = AllocatePages (EFI_SIZE_TO_PAGES (TableSize));
+  ASSERT (RegisterTable != NULL);
+
+  for (Index = 0; Index < NumberOfCpus; Index++) {
+    Status = GetProcessorInformation (Index, &ProcessorInfoBuffer);
+    ASSERT_EFI_ERROR (Status);
+
+    RegisterTable[Index].InitialApicId      = (UINT32)ProcessorInfoBuffer.ProcessorId;
+    RegisterTable[Index].TableLength        = 0;
+    RegisterTable[Index].AllocatedSize      = 0;
+    RegisterTable[Index].RegisterTableEntry = 0;
+
+    RegisterTable[NumberOfCpus + Index].InitialApicId      = (UINT32)ProcessorInfoBuffer.ProcessorId;
+    RegisterTable[NumberOfCpus + Index].TableLength        = 0;
+    RegisterTable[NumberOfCpus + Index].AllocatedSize      = 0;
+    RegisterTable[NumberOfCpus + Index].RegisterTableEntry = 0;
+  }
+  AcpiCpuData->RegisterTable           = (EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTable;
+  AcpiCpuData->PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)(RegisterTable + NumberOfCpus);
+
+  return AcpiCpuData;
+}
+
+/**
+  Enlarges CPU register table for each processor.
+
+  @param[in, out]  RegisterTable   Pointer processor's CPU register table
+**/
+STATIC
+VOID
+EnlargeRegisterTable (
+  IN OUT CPU_REGISTER_TABLE            *RegisterTable
+  )
+{
+  EFI_PHYSICAL_ADDRESS  Address;
+  UINTN                 UsedPages;
+
+  UsedPages = RegisterTable->AllocatedSize / EFI_PAGE_SIZE;
+  Address  = (UINTN)AllocatePages (UsedPages + 1);
+  ASSERT (Address != 0);
+
+  //
+  // If there are records existing in the register table, then copy its contents
+  // to new region and free the old one.
+  //
+  if (RegisterTable->AllocatedSize > 0) {
+    CopyMem (
+      (VOID *) (UINTN) Address,
+      (VOID *) (UINTN) RegisterTable->RegisterTableEntry,
+      RegisterTable->AllocatedSize
+      );
+
+    FreePages ((VOID *)(UINTN)RegisterTable->RegisterTableEntry, UsedPages);
+  }
+
+  //
+  // Adjust the allocated size and register table base address.
+  //
+  RegisterTable->AllocatedSize     += EFI_PAGE_SIZE;
+  RegisterTable->RegisterTableEntry = Address;
+}
+
 /**
   Add an entry in specified register table.
 
-- 
2.15.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel