From: "Laszlo Ersek" <lersek@redhat.com>
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: "Eric Dong" <eric.dong@intel.com>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>,
"Rahul Kumar" <rahul1.kumar@intel.com>,
"Ray Ni" <ray.ni@intel.com>, "Star Zeng" <star.zeng@intel.com>
Subject: [PATCH v2 1/4] UefiCpuPkg/CpuFeature: Don't assume CpuS3DataDxe alloc RegisterTable
Date: Tue, 19 Jan 2021 16:54:37 +0100 [thread overview]
Message-ID: <20210119155440.2262-2-lersek@redhat.com> (raw)
In-Reply-To: <20210119155440.2262-1-lersek@redhat.com>
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
next prev parent reply other threads:[~2021-01-19 15:54 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2021-01-19 15:54 ` [PATCH v2 2/4] UefiCpuPkg/AcpiCpuData: update comments on register table fields Laszlo Ersek
2021-01-19 15:54 ` [PATCH v2 3/4] UefiCpuPkg/CpuS3DataDxe: do not allocate useless register tables Laszlo Ersek
2021-01-19 15:54 ` [PATCH v2 4/4] OvmfPkg/CpuS3DataDxe: " 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
2021-01-20 18:46 ` [edk2-devel] " Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210119155440.2262-2-lersek@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox