* [PATCH v2 1/4] UefiCpuPkg/CpuFeature: Don't assume CpuS3DataDxe alloc RegisterTable
2021-01-19 15:54 [PATCH v2 0/4] UefiCpuPkg, OvmfPkg: do not allocate useless register tables for S3 resume Laszlo Ersek
@ 2021-01-19 15:54 ` Laszlo Ersek
2021-01-19 15:54 ` [PATCH v2 2/4] UefiCpuPkg/AcpiCpuData: update comments on register table fields Laszlo Ersek
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2021-01-19 15:54 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Eric Dong, Philippe Mathieu-Daudé, Rahul Kumar, Ray Ni,
Star Zeng
From: Ray Ni <ray.ni@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@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3159
Signed-off-by: Ray Ni <ray.ni@intel.com>
[lersek@redhat.com: update CC list, add BZ reference, add my S-o-b]
[lersek@redhat.com: deal with RegisterTable and PreSmmInitRegisterTable
being zero independently of each other; replacing the ASSERT()]
Signed-off-by: Laszlo Ersek <lersek@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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 2/4] UefiCpuPkg/AcpiCpuData: update comments on register table fields
2021-01-19 15:54 [PATCH v2 0/4] UefiCpuPkg, OvmfPkg: do not allocate useless register tables for S3 resume Laszlo Ersek
2021-01-19 15:54 ` [PATCH v2 1/4] UefiCpuPkg/CpuFeature: Don't assume CpuS3DataDxe alloc RegisterTable Laszlo Ersek
@ 2021-01-19 15:54 ` Laszlo Ersek
2021-01-19 15:54 ` [PATCH v2 3/4] UefiCpuPkg/CpuS3DataDxe: do not allocate useless register tables Laszlo Ersek
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2021-01-19 15:54 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Eric Dong, Philippe Mathieu-Daudé, Rahul Kumar, Ray Ni,
Star Zeng
After commit e992cc3f4859 ("UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM
consumption in CpuS3.c", 2021-01-11), it is valid for a CPU S3 Data DXE
Driver to set "ACPI_CPU_DATA.PreSmmInitRegisterTable" and/or
"ACPI_CPU_DATA.RegisterTable" to 0, in case none of the CPUs needs a
register table of the corresponding kind, during S3 resume.
Document this fact in the "UefiCpuPkg/Include/AcpiCpuData.h" header file.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3159
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Notes:
v2:
- no changes, pick up feedback tags
UefiCpuPkg/Include/AcpiCpuData.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/UefiCpuPkg/Include/AcpiCpuData.h b/UefiCpuPkg/Include/AcpiCpuData.h
index b5a69ad80c88..62a01b2c6bac 100644
--- a/UefiCpuPkg/Include/AcpiCpuData.h
+++ b/UefiCpuPkg/Include/AcpiCpuData.h
@@ -178,6 +178,8 @@ typedef struct {
// If TableLength is > 0, then elements of RegisterTableEntry are used to
// initialize the CPU that matches InitialApicId, during an ACPI S3 resume,
// before SMBASE relocation is performed.
+ // If a register table is not required for any one of the CPUs, then
+ // PreSmmInitRegisterTable may be set to 0.
//
EFI_PHYSICAL_ADDRESS PreSmmInitRegisterTable;
//
@@ -187,6 +189,8 @@ typedef struct {
// If TableLength is > 0, then elements of RegisterTableEntry are used to
// initialize the CPU that matches InitialApicId, during an ACPI S3 resume,
// after SMBASE relocation is performed.
+ // If a register table is not required for any one of the CPUs, then
+ // RegisterTable may be set to 0.
//
EFI_PHYSICAL_ADDRESS RegisterTable;
//
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 3/4] UefiCpuPkg/CpuS3DataDxe: do not allocate useless register tables
2021-01-19 15:54 [PATCH v2 0/4] UefiCpuPkg, OvmfPkg: do not allocate useless register tables for S3 resume Laszlo Ersek
2021-01-19 15:54 ` [PATCH v2 1/4] UefiCpuPkg/CpuFeature: Don't assume CpuS3DataDxe alloc RegisterTable Laszlo Ersek
2021-01-19 15:54 ` [PATCH v2 2/4] UefiCpuPkg/AcpiCpuData: update comments on register table fields Laszlo Ersek
@ 2021-01-19 15:54 ` Laszlo Ersek
2021-01-19 15:54 ` [PATCH v2 4/4] OvmfPkg/CpuS3DataDxe: " Laszlo Ersek
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2021-01-19 15:54 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Eric Dong, Philippe Mathieu-Daudé, Rahul Kumar, Ray Ni,
Star Zeng
CpuS3DataDxe allocates the "RegisterTable" and "PreSmmInitRegisterTable"
arrays in ACPI_CPU_DATA just so every processor in the system can have its
own empty register table, matched by APIC ID. This has never been useful
in practice.
Given commit e992cc3f4859 ("UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM
consumption in CpuS3.c", 2021-01-11), simply leave both
"AcpiCpuData->RegisterTable" and "AcpiCpuData->PreSmmInitRegisterTable"
initialized to the zero address. This simplifies the driver, and saves
both normal RAM (boot services data type memory) and -- in PiSmmCpuDxeSmm
-- SMRAM.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3159
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
---
Notes:
v2:
- no changes, pick up feedback tags
v1:
- Tested by temporarily replacing OvmfPkgPkg/CpuS3DataDxe in the OVMF
IA32 and IA32X64 platforms with this driver -- this driver works OK in
OVMF as long as no CPUs are hot-plugged.
UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c | 32 --------------------
1 file changed, 32 deletions(-)
diff --git a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
index 2be335d91903..078af36cfb41 100644
--- a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
+++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
@@ -165,10 +165,6 @@ CpuS3DataInitialize (
UINTN NumberOfCpus;
UINTN NumberOfEnabledProcessors;
VOID *Stack;
- UINTN TableSize;
- CPU_REGISTER_TABLE *RegisterTable;
- UINTN Index;
- EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
UINTN GdtSize;
UINTN IdtSize;
VOID *Gdt;
@@ -255,34 +251,6 @@ CpuS3DataInitialize (
AcpiCpuData->PreSmmInitRegisterTable = OldAcpiCpuData->PreSmmInitRegisterTable;
AcpiCpuData->ApLocation = OldAcpiCpuData->ApLocation;
CopyMem (&AcpiCpuData->CpuStatus, &OldAcpiCpuData->CpuStatus, sizeof (CPU_STATUS_INFORMATION));
- } else {
- //
- // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for all CPUs
- //
- TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
- RegisterTable = (CPU_REGISTER_TABLE *)AllocateZeroPages (TableSize);
- ASSERT (RegisterTable != NULL);
-
- for (Index = 0; Index < NumberOfCpus; Index++) {
- Status = MpServices->GetProcessorInfo (
- MpServices,
- 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);
}
//
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 4/4] OvmfPkg/CpuS3DataDxe: do not allocate useless register tables
2021-01-19 15:54 [PATCH v2 0/4] UefiCpuPkg, OvmfPkg: do not allocate useless register tables for S3 resume Laszlo Ersek
` (2 preceding siblings ...)
2021-01-19 15:54 ` [PATCH v2 3/4] UefiCpuPkg/CpuS3DataDxe: do not allocate useless register tables Laszlo Ersek
@ 2021-01-19 15:54 ` Laszlo Ersek
2021-01-19 16:44 ` [edk2-devel] [PATCH v2 0/4] UefiCpuPkg, OvmfPkg: do not allocate useless register tables for S3 resume Laszlo Ersek
2021-01-20 9:28 ` Zeng, Star
5 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2021-01-19 15:54 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Philippe Mathieu-Daudé
CpuS3DataDxe allocates the "RegisterTable" and "PreSmmInitRegisterTable"
arrays in ACPI_CPU_DATA just so every processor in the system can have its
own empty register table, matched by APIC ID. This has never been useful
in practice.
Given commit e992cc3f4859 ("UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM
consumption in CpuS3.c", 2021-01-11), simply leave both
"AcpiCpuData->RegisterTable" and "AcpiCpuData->PreSmmInitRegisterTable"
initialized to the zero address. This simplifies the driver, and saves
both normal RAM (boot services data type memory) and -- in PiSmmCpuDxeSmm
-- SMRAM.
(This simplification backs out a good chunk of commit 1158fc8e2c7b
("OvmfPkg/CpuS3DataDxe: enable S3 resume after CPU hotplug", 2020-03-04).
But CpuS3DataDxe still differs between UefiCpuPkg and OvmfPkg, due to the
latter supporting CPU hotplug; thus, we can't remove OvmfPkg/CpuS3DataDxe
altogether.)
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3159
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
Notes:
v2:
- no changes, pick up feedback tags
OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf | 1 -
OvmfPkg/CpuS3DataDxe/CpuS3Data.c | 70 +-------------------
2 files changed, 1 insertion(+), 70 deletions(-)
diff --git a/OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf b/OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf
index ceae1d4078c7..228d5ae1b260 100644
--- a/OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf
+++ b/OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf
@@ -42,7 +42,6 @@ [LibraryClasses]
BaseLib
BaseMemoryLib
DebugLib
- IoLib
MemoryAllocationLib
MtrrLib
UefiBootServicesTableLib
diff --git a/OvmfPkg/CpuS3DataDxe/CpuS3Data.c b/OvmfPkg/CpuS3DataDxe/CpuS3Data.c
index bac7285aa2f3..5ffe1f3cd74e 100644
--- a/OvmfPkg/CpuS3DataDxe/CpuS3Data.c
+++ b/OvmfPkg/CpuS3DataDxe/CpuS3Data.c
@@ -23,7 +23,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/BaseLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
-#include <Library/IoLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/MtrrLib.h>
#include <Library/UefiBootServicesTableLib.h>
@@ -31,9 +30,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Protocol/MpService.h>
#include <Guid/EventGroup.h>
-#include <IndustryStandard/Q35MchIch9.h>
-#include <IndustryStandard/QemuCpuHotplug.h>
-
//
// Data structure used to allocate ACPI_CPU_DATA and its supporting structures
//
@@ -168,17 +164,12 @@ CpuS3DataInitialize (
EFI_MP_SERVICES_PROTOCOL *MpServices;
UINTN NumberOfCpus;
VOID *Stack;
- UINTN TableSize;
- CPU_REGISTER_TABLE *RegisterTable;
- UINTN Index;
- EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
UINTN GdtSize;
UINTN IdtSize;
VOID *Gdt;
VOID *Idt;
EFI_EVENT Event;
ACPI_CPU_DATA *OldAcpiCpuData;
- BOOLEAN FetchPossibleApicIds;
if (!PcdGetBool (PcdAcpiS3Enable)) {
return EFI_UNSUPPORTED;
@@ -193,13 +184,7 @@ CpuS3DataInitialize (
ASSERT (AcpiCpuDataEx != NULL);
AcpiCpuData = &AcpiCpuDataEx->AcpiCpuData;
- //
- // The "SMRAM at default SMBASE" feature guarantees that
- // QEMU_CPUHP_CMD_GET_ARCH_ID too is available.
- //
- FetchPossibleApicIds = PcdGetBool (PcdQ35SmramAtDefaultSmbase);
-
- if (FetchPossibleApicIds) {
+ if (PcdGetBool (PcdQ35SmramAtDefaultSmbase)) {
NumberOfCpus = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
} else {
UINTN NumberOfEnabledProcessors;
@@ -271,59 +256,6 @@ CpuS3DataInitialize (
AcpiCpuData->PreSmmInitRegisterTable = OldAcpiCpuData->PreSmmInitRegisterTable;
AcpiCpuData->ApLocation = OldAcpiCpuData->ApLocation;
CopyMem (&AcpiCpuData->CpuStatus, &OldAcpiCpuData->CpuStatus, sizeof (CPU_STATUS_INFORMATION));
- } else {
- //
- // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for all CPUs
- //
- TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
- RegisterTable = (CPU_REGISTER_TABLE *)AllocateZeroPages (TableSize);
- ASSERT (RegisterTable != NULL);
-
- if (FetchPossibleApicIds) {
- //
- // Write a valid selector so that other hotplug registers can be
- // accessed.
- //
- IoWrite32 (ICH9_CPU_HOTPLUG_BASE + QEMU_CPUHP_W_CPU_SEL, 0);
- //
- // We'll be fetching the APIC IDs.
- //
- IoWrite8 (ICH9_CPU_HOTPLUG_BASE + QEMU_CPUHP_W_CMD,
- QEMU_CPUHP_CMD_GET_ARCH_ID);
- }
- for (Index = 0; Index < NumberOfCpus; Index++) {
- UINT32 InitialApicId;
-
- if (FetchPossibleApicIds) {
- IoWrite32 (ICH9_CPU_HOTPLUG_BASE + QEMU_CPUHP_W_CPU_SEL,
- (UINT32)Index);
- InitialApicId = IoRead32 (
- ICH9_CPU_HOTPLUG_BASE + QEMU_CPUHP_RW_CMD_DATA);
- } else {
- Status = MpServices->GetProcessorInfo (
- MpServices,
- Index,
- &ProcessorInfoBuffer
- );
- ASSERT_EFI_ERROR (Status);
- InitialApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
- }
-
- DEBUG ((DEBUG_VERBOSE, "%a: Index=%05Lu ApicId=0x%08x\n", __FUNCTION__,
- (UINT64)Index, InitialApicId));
-
- RegisterTable[Index].InitialApicId = InitialApicId;
- RegisterTable[Index].TableLength = 0;
- RegisterTable[Index].AllocatedSize = 0;
- RegisterTable[Index].RegisterTableEntry = 0;
-
- RegisterTable[NumberOfCpus + Index].InitialApicId = InitialApicId;
- 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);
}
//
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [edk2-devel] [PATCH v2 0/4] UefiCpuPkg, OvmfPkg: do not allocate useless register tables for S3 resume
2021-01-19 15:54 [PATCH v2 0/4] UefiCpuPkg, OvmfPkg: do not allocate useless register tables for S3 resume Laszlo Ersek
` (3 preceding siblings ...)
2021-01-19 15:54 ` [PATCH v2 4/4] OvmfPkg/CpuS3DataDxe: " Laszlo Ersek
@ 2021-01-19 16:44 ` Laszlo Ersek
2021-01-20 9:28 ` Zeng, Star
5 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2021-01-19 16:44 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Eric Dong, Jordan Justen,
Philippe Mathieu-Daudé, Rahul Kumar, Ray Ni, Star Zeng
On 01/19/21 16:54, Laszlo Ersek wrote:
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3159
> Repo: https://pagure.io/lersek/edk2.git
> Branch: remove-cpu-reg-table-alloc-3159-v2
>
> Updates in v2:
>
> - v1 patches have not received any updates, I've only picked up the
> feedback tags.
I didn't regenerate the CC's for the v1-originated patches, so the CC
list still uses Ard's @arm.com email address. I don't think that's a
problem for this specific posting (and I expect to merge this series
once Eric or Star approves patch v2 #1).
Thanks
Laszlo
>
> - Patch v2 #1 -- for RegisterCpuFeaturesLib -- is new; authored by Ray
> and updated slightly by myself. Star and/or Eric should please approve
> this patch.
>
> v1 was posted at:
>
> [edk2-devel] [PATCH 0/3] UefiCpuPkg, OvmfPkg: do not allocate useless
> register tables for S3 resume
>
> Message-Id: <20210112191934.12459-1-lersek@redhat.com>
> https://edk2.groups.io/g/devel/message/70167
> https://www.redhat.com/archives/edk2-devel-archive/2021-January/msg00652.html
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
>
> Thanks
> Laszlo
>
> Laszlo Ersek (3):
> UefiCpuPkg/AcpiCpuData: update comments on register table fields
> UefiCpuPkg/CpuS3DataDxe: do not allocate useless register tables
> OvmfPkg/CpuS3DataDxe: do not allocate useless register tables
>
> Ray Ni (1):
> UefiCpuPkg/CpuFeature: Don't assume CpuS3DataDxe alloc RegisterTable
>
> OvmfPkg/CpuS3DataDxe/CpuS3Data.c | 70 +----------------
> OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf | 1 -
> UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c | 32 --------
> UefiCpuPkg/Include/AcpiCpuData.h | 4 +
> UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c | 80 +++++++++++---------
> 5 files changed, 48 insertions(+), 139 deletions(-)
>
>
> base-commit: 83facfd184021874f95a6a34b2e47e0ebb34a762
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 0/4] UefiCpuPkg, OvmfPkg: do not allocate useless register tables for S3 resume
2021-01-19 15:54 [PATCH v2 0/4] UefiCpuPkg, OvmfPkg: do not allocate useless register tables for S3 resume Laszlo Ersek
` (4 preceding siblings ...)
2021-01-19 16:44 ` [edk2-devel] [PATCH v2 0/4] UefiCpuPkg, OvmfPkg: do not allocate useless register tables for S3 resume Laszlo Ersek
@ 2021-01-20 9:28 ` Zeng, Star
2021-01-20 18:46 ` [edk2-devel] " Laszlo Ersek
5 siblings, 1 reply; 8+ messages in thread
From: Zeng, Star @ 2021-01-20 9:28 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Ard Biesheuvel, Dong, Eric, Justen, Jordan L,
Philippe Mathieu-Daudé, Kumar, Rahul1, Ni, Ray, Zeng, Star
Series Reviewed-by: Star Zeng <star.zeng@intel.com>
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, January 19, 2021 11:55 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; Dong, Eric
> <eric.dong@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Philippe Mathieu-Daudé <philmd@redhat.com>; Kumar, Rahul1
> <rahul1.kumar@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: [PATCH v2 0/4] UefiCpuPkg, OvmfPkg: do not allocate useless
> register tables for S3 resume
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3159
> Repo: https://pagure.io/lersek/edk2.git
> Branch: remove-cpu-reg-table-alloc-3159-v2
>
> Updates in v2:
>
> - v1 patches have not received any updates, I've only picked up the
> feedback tags.
>
> - Patch v2 #1 -- for RegisterCpuFeaturesLib -- is new; authored by Ray
> and updated slightly by myself. Star and/or Eric should please approve
> this patch.
>
> v1 was posted at:
>
> [edk2-devel] [PATCH 0/3] UefiCpuPkg, OvmfPkg: do not allocate useless
> register tables for S3 resume
>
> Message-Id: <20210112191934.12459-1-lersek@redhat.com>
> https://edk2.groups.io/g/devel/message/70167
> https://www.redhat.com/archives/edk2-devel-archive/2021-
> January/msg00652.html
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
>
> Thanks
> Laszlo
>
> Laszlo Ersek (3):
> UefiCpuPkg/AcpiCpuData: update comments on register table fields
> UefiCpuPkg/CpuS3DataDxe: do not allocate useless register tables
> OvmfPkg/CpuS3DataDxe: do not allocate useless register tables
>
> Ray Ni (1):
> UefiCpuPkg/CpuFeature: Don't assume CpuS3DataDxe alloc RegisterTable
>
> OvmfPkg/CpuS3DataDxe/CpuS3Data.c | 70 +---------------
> -
> OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf | 1 -
> UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c | 32 --------
> UefiCpuPkg/Include/AcpiCpuData.h | 4 +
> UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c | 80
> +++++++++++---------
> 5 files changed, 48 insertions(+), 139 deletions(-)
>
>
> base-commit: 83facfd184021874f95a6a34b2e47e0ebb34a762
> --
> 2.19.1.3.g30247aa5d201
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [edk2-devel] [PATCH v2 0/4] UefiCpuPkg, OvmfPkg: do not allocate useless register tables for S3 resume
2021-01-20 9:28 ` Zeng, Star
@ 2021-01-20 18:46 ` Laszlo Ersek
0 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2021-01-20 18:46 UTC (permalink / raw)
To: devel, star.zeng
Cc: Ard Biesheuvel, Dong, Eric, Justen, Jordan L,
Philippe Mathieu-Daudé, Kumar, Rahul1, Ni, Ray
On 01/20/21 10:28, Zeng, Star wrote:
> Series Reviewed-by: Star Zeng <star.zeng@intel.com>
Series merged as commit range e843a21e23ea..339371ef78eb, via
<https://github.com/tianocore/edk2/pull/1373>.
Thanks!
Laszlo
>
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Tuesday, January 19, 2021 11:55 PM
>> To: edk2-devel-groups-io <devel@edk2.groups.io>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; Dong, Eric
>> <eric.dong@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
>> Philippe Mathieu-Daudé <philmd@redhat.com>; Kumar, Rahul1
>> <rahul1.kumar@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
>> <star.zeng@intel.com>
>> Subject: [PATCH v2 0/4] UefiCpuPkg, OvmfPkg: do not allocate useless
>> register tables for S3 resume
>>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3159
>> Repo: https://pagure.io/lersek/edk2.git
>> Branch: remove-cpu-reg-table-alloc-3159-v2
>>
>> Updates in v2:
>>
>> - v1 patches have not received any updates, I've only picked up the
>> feedback tags.
>>
>> - Patch v2 #1 -- for RegisterCpuFeaturesLib -- is new; authored by Ray
>> and updated slightly by myself. Star and/or Eric should please approve
>> this patch.
>>
>> v1 was posted at:
>>
>> [edk2-devel] [PATCH 0/3] UefiCpuPkg, OvmfPkg: do not allocate useless
>> register tables for S3 resume
>>
>> Message-Id: <20210112191934.12459-1-lersek@redhat.com>
>> https://edk2.groups.io/g/devel/message/70167
>> https://www.redhat.com/archives/edk2-devel-archive/2021-
>> January/msg00652.html
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>>
>> Thanks
>> Laszlo
>>
>> Laszlo Ersek (3):
>> UefiCpuPkg/AcpiCpuData: update comments on register table fields
>> UefiCpuPkg/CpuS3DataDxe: do not allocate useless register tables
>> OvmfPkg/CpuS3DataDxe: do not allocate useless register tables
>>
>> Ray Ni (1):
>> UefiCpuPkg/CpuFeature: Don't assume CpuS3DataDxe alloc RegisterTable
>>
>> OvmfPkg/CpuS3DataDxe/CpuS3Data.c | 70 +---------------
>> -
>> OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf | 1 -
>> UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c | 32 --------
>> UefiCpuPkg/Include/AcpiCpuData.h | 4 +
>> UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c | 80
>> +++++++++++---------
>> 5 files changed, 48 insertions(+), 139 deletions(-)
>>
>>
>> base-commit: 83facfd184021874f95a6a34b2e47e0ebb34a762
>> --
>> 2.19.1.3.g30247aa5d201
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread