[edk2-devel] [PATCH v2 1/4] UefiCpuPkg/CpuFeature: Don't assume CpuS3DataDxe alloc RegisterTable

[Laszlo Ersek]

From: Ray Ni <ray.ni at intel.com>

There are lots of fields in ACPI_CPU_DATA structure while only
followings are accessed by CpuFeature infra:
* NumberOfCpus
* PreSmmInitRegisterTable // pointer
* RegisterTable  // pointer
* CpuStatus
* ApLocation  // pointer

So it's possible that an implementation of CpuS3DataDxe doesn't
allocate memory for PreSmmInitRegisterTable/RegisterTable/ApLocation.

This patch handles the case when CpuS3DataDxe doesn't allocate
memory for PreSmmInitRegisterTable/RegisterTable.

Cc: Eric Dong <eric.dong at intel.com>
Cc: Philippe Mathieu-Daudé <philmd at redhat.com>
Cc: Rahul Kumar <rahul1.kumar at intel.com>
Cc: Ray Ni <ray.ni at intel.com>
Cc: Star Zeng <star.zeng at intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3159
Signed-off-by: Ray Ni <ray.ni at intel.com>
[lersek at redhat.com: update CC list, add BZ reference, add my S-o-b]
[lersek at redhat.com: deal with RegisterTable and PreSmmInitRegisterTable
 being zero independently of each other; replacing the ASSERT()]
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v2:
    
    - new patch, cherry-picked from
      <https://github.com/niruiyu/edk2/commit/7091aa50b9d8>
    
    - Update as follows: do not assume that (RegisterTable == 0) implies
      (PreSmmInitRegisterTable == 0). The ACPI_CPU_DATA documentation update
      in the next patch permits each field to be zero in separation, and
      updating Ray's patch for accommodating that is not difficult. Some
      normal RAM may be wasted if exactly one of the "RegisterTable" and
      "PreSmmInitRegisterTable" fields is zero (the memory is allocated with
      the PEI or DXE instance of MemoryAllocationLib).
    
    - this patch is easiest to review with "git show -b -W"

 UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c | 80 +++++++++++---------
 1 file changed, 43 insertions(+), 37 deletions(-)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
index 4063d45760ad..7bb92404027f 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -1,7 +1,7 @@
 /** @file
   CPU Register Table Library functions.
 
-  Copyright (c) 2017 - 2020, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -937,45 +937,51 @@ GetAcpiCpuData (
   EFI_PROCESSOR_INFORMATION            ProcessorInfoBuffer;
 
   AcpiCpuData = (ACPI_CPU_DATA *) (UINTN) PcdGet64 (PcdCpuS3DataAddress);
-  if (AcpiCpuData != NULL) {
-    return AcpiCpuData;
-  }
-
-  AcpiCpuData  = AllocatePages (EFI_SIZE_TO_PAGES (sizeof (ACPI_CPU_DATA)));
-  ASSERT (AcpiCpuData != NULL);
-
-  //
-  // Set PcdCpuS3DataAddress to the base address of the ACPI_CPU_DATA structure
-  //
-  Status = PcdSet64S (PcdCpuS3DataAddress, (UINT64)(UINTN)AcpiCpuData);
-  ASSERT_EFI_ERROR (Status);
-
-  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);
+  if (AcpiCpuData == NULL) {
+    AcpiCpuData  = AllocatePages (EFI_SIZE_TO_PAGES (sizeof (ACPI_CPU_DATA)));
+    ASSERT (AcpiCpuData != NULL);
+    ZeroMem (AcpiCpuData, sizeof (ACPI_CPU_DATA));
+
+    //
+    // Set PcdCpuS3DataAddress to the base address of the ACPI_CPU_DATA structure
+    //
+    Status = PcdSet64S (PcdCpuS3DataAddress, (UINT64)(UINTN)AcpiCpuData);
     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;
+    GetNumberOfProcessor (&NumberOfCpus, &NumberOfEnabledProcessors);
+    AcpiCpuData->NumberOfCpus = (UINT32)NumberOfCpus;
+  }
+
+  if (AcpiCpuData->RegisterTable == 0 ||
+      AcpiCpuData->PreSmmInitRegisterTable == 0) {
+    //
+    // 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;
+    }
+    if (AcpiCpuData->RegisterTable == 0) {
+      AcpiCpuData->RegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTable;
+    }
+    if (AcpiCpuData->PreSmmInitRegisterTable == 0) {
+      AcpiCpuData->PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)(RegisterTable + NumberOfCpus);
+    }
   }
-  AcpiCpuData->RegisterTable           = (EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTable;
-  AcpiCpuData->PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)(RegisterTable + NumberOfCpus);
 
   return AcpiCpuData;
 }
-- 
2.19.1.3.g30247aa5d201




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70549): https://edk2.groups.io/g/devel/message/70549
Mute This Topic: https://groups.io/mt/79954424/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-