* [PATCH v8 0/6] Simplify SMM Relocation Process
@ 2023-02-16 2:46 Wu, Jiaxin
2023-02-16 2:46 ` [PATCH v8 1/6] UefiCpuPkg/PiSmmCpuDxeSmm: Fix invalid InitializeMpSyncData call Wu, Jiaxin
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: Wu, Jiaxin @ 2023-02-16 2:46 UTC (permalink / raw)
To: devel
Cc: Eric Dong, Ray Ni, Zeng Star, Laszlo Ersek, Gerd Hoffmann,
Rahul Kumar
Existing SMBASE Relocation is in the PiSmmCpuDxeSmm driver, which
will relocate the SMBASE of each processor by setting the SMBASE
field in the saved state map (at offset 7EF8h) to a new value.
The RSM instruction reloads the internal SMBASE register with the
value in SMBASE field when each time it exits SMM. All subsequent
SMI requests will use the new SMBASE to find the starting address
for the SMI handler (at SMBASE + 8000h).
Due to the default SMBASE for all x86 processors is 0x30000, the
APs' 1st SMI for rebase has to be executed one by one to avoid
the CPUs over-writing each other's SMM Save State Area (see
existing SmmRelocateBases() function), which means the next AP has
to wait for the previous AP to finish its 1st SMI, then it can call
into its 1st SMI for rebase via Smi Ipi command, thus leading the
existing SMBASE Relocation has to be running in series. Besides, it
needs very complex code to handle the AP exit semaphore
(mRebased[Index]), which will hook return address of SMM Save State
so that semaphore code can be executed immediately after AP exits
SMM for SMBASE relocation (see existing SemaphoreHook() function).
This series is to add the new SMM Base HOB for any PEI module to do
the SmBase relocation ahead of PiSmmCpuDxeSmm driver and store the
relocated SmBase address in array for each Processors. When the
SMBASE relocation happens in a PEI module, the PEI module shall
produce the SMM_BASE_HOB in HOB database which tells the
PiSmmCpuDxeSmm driver (runs at a later phase) about the new SMBASE
for each CPU thread. PiSmmCpuDxeSmm driver installs the SMI handler
at the SMM_BASE_HOB.SmBase[Index]+0x8000 for CPU thread Index. When
the HOB doesn't exist, PiSmmCpuDxeSmm driver shall relocate and
program the new SMBASE itself (keep existing SMBASE Relocation way).
With SMM Base Hob support, PiSmmCpuDxeSmm does not need the RSM
instruction to do the SMBASE Relocation. SMBASE Register for each
processors have already been programmed and all SMBASE address have
recorded in SMM Base Hob. So the same default SMBASE Address
(0x30000) will not be used, thus the CPUs over-writing each other's
SMM Save State Area will not happen in PiSmmCpuDxeSmm driver. This
way makes the first SMI init can be executed in parallel and save
boot time on multi-core system. Besides, Semaphore Hook code logic
is also not required, which will greatly simplify the SMBASE
Relocation flow.
Note:
This is the new way that firmware can program the SMBASE
independently of the RSM instruction. The PEI code performing
this logic will not be open sourced, similarly to other things
that are kept binary-only in the FSP. Due to the register
difference in different vender, and it has not been documented
in the Intel SDM yet, we need a new binary-only interface for
SMM Base HOB.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
Jiaxin Wu (6):
UefiCpuPkg/PiSmmCpuDxeSmm: Fix invalid InitializeMpSyncData call
UefiCpuPkg/PiSmmCpuDxeSmm: Replace mIsBsp by mBspApicId check
UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB Data
UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM Base Hob for SmBase info
UefiCpuPkg/SmmCpuFeaturesLib: Skip SMBASE configuration
OvmfPkg/SmmCpuFeaturesLib: Check SmBase relocation supported or not
.../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 10 +-
.../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 6 +-
UefiCpuPkg/Include/Guid/SmmBaseHob.h | 75 ++++++++
.../Library/SmmCpuFeaturesLib/CpuFeaturesLib.h | 2 +
.../SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c | 25 ++-
.../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 6 +-
.../SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf | 3 +-
UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c | 3 +-
.../StandaloneMmCpuFeaturesLib.inf | 6 +-
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 31 +++-
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 25 ++-
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 193 ++++++++++++++++-----
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 26 ++-
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 3 +-
UefiCpuPkg/UefiCpuPkg.dec | 5 +-
15 files changed, 356 insertions(+), 63 deletions(-)
create mode 100644 UefiCpuPkg/Include/Guid/SmmBaseHob.h
--
2.16.2.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v8 1/6] UefiCpuPkg/PiSmmCpuDxeSmm: Fix invalid InitializeMpSyncData call
2023-02-16 2:46 [PATCH v8 0/6] Simplify SMM Relocation Process Wu, Jiaxin
@ 2023-02-16 2:46 ` Wu, Jiaxin
2023-02-16 2:46 ` [PATCH v8 2/6] UefiCpuPkg/PiSmmCpuDxeSmm: Replace mIsBsp by mBspApicId check Wu, Jiaxin
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Wu, Jiaxin @ 2023-02-16 2:46 UTC (permalink / raw)
To: devel
Cc: Eric Dong, Ray Ni, Zeng Star, Laszlo Ersek, Gerd Hoffmann,
Rahul Kumar
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4338
No need call InitializeMpSyncData during normal boot SMI init,
because mSmmMpSyncData is NULL at that time. mSmmMpSyncData is
allocated in InitializeMpServiceData, which is invoked after
normal boot SMI init (SmmRelocateBases).
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ray Ni <ray.ni@Intel.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 655175a2c6..2ac655d032 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -1,9 +1,9 @@
/** @file
Agent Module to load other modules to deploy SMM Entry Vector for X86 CPU.
-Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -369,13 +369,11 @@ SmmInitHandler (
if (!mSmmS3Flag) {
//
// Check XD and BTS features on each processor on normal boot
//
CheckFeatureSupported ();
- }
-
- if (mIsBsp) {
+ } else if (mIsBsp) {
//
// BSP rebase is already done above.
// Initialize private data during S3 resume
//
InitializeMpSyncData ();
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v8 2/6] UefiCpuPkg/PiSmmCpuDxeSmm: Replace mIsBsp by mBspApicId check
2023-02-16 2:46 [PATCH v8 0/6] Simplify SMM Relocation Process Wu, Jiaxin
2023-02-16 2:46 ` [PATCH v8 1/6] UefiCpuPkg/PiSmmCpuDxeSmm: Fix invalid InitializeMpSyncData call Wu, Jiaxin
@ 2023-02-16 2:46 ` Wu, Jiaxin
2023-02-16 2:46 ` [PATCH v8 3/6] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB Data Wu, Jiaxin
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Wu, Jiaxin @ 2023-02-16 2:46 UTC (permalink / raw)
To: devel
Cc: Eric Dong, Ray Ni, Zeng Star, Laszlo Ersek, Gerd Hoffmann,
Rahul Kumar
This patch is to replace mIsBsp by mBspApicId check.
mIsBsp becomes the local variable (IsBsp), then it can be
checked dynamically in the function. Instead, we define the
mBspApicId, which is to record the BSP ApicId used for
compare in SmmInitHandler. With this change, SmmInitHandler
can be run in parallel during SMM init.
Note:
This patch is the per-prepared work by refining the
SmmInitHandler, then, we can do the next step to
combine 2 SMIs (gcSmmInitTemplate & gcSmiHandlerTemplate)
into one (gcSmiHandlerTemplate), the new SMI handler
will call the SmmInitHandler in parallel to do the init.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 2ac655d032..6e795d1756 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -57,11 +57,10 @@ SMM_CPU_PRIVATE_DATA *gSmmCpuPrivate = &mSmmCpuPrivateData;
//
// SMM Relocation variables
//
volatile BOOLEAN *mRebased;
-volatile BOOLEAN mIsBsp;
///
/// Handle for the SMM CPU Protocol
///
EFI_HANDLE mSmmCpuHandle = NULL;
@@ -83,10 +82,12 @@ EDKII_SMM_MEMORY_ATTRIBUTE_PROTOCOL mSmmMemoryAttribute = {
EdkiiSmmClearMemoryAttributes
};
EFI_CPU_INTERRUPT_HANDLER mExternalVectorTable[EXCEPTION_VECTOR_NUMBER];
+UINT32 mBspApicId = 0;
+
//
// SMM stack information
//
UINTN mSmmStackArrayBase;
UINTN mSmmStackArrayEnd;
@@ -341,39 +342,42 @@ VOID
EFIAPI
SmmInitHandler (
VOID
)
{
- UINT32 ApicId;
- UINTN Index;
+ UINT32 ApicId;
+ UINTN Index;
+ BOOLEAN IsBsp;
//
// Update SMM IDT entries' code segment and load IDT
//
AsmWriteIdtr (&gcSmiIdtr);
ApicId = GetApicId ();
+ IsBsp = (BOOLEAN)(mBspApicId == ApicId);
+
ASSERT (mNumberOfCpus <= mMaxNumberOfCpus);
for (Index = 0; Index < mNumberOfCpus; Index++) {
if (ApicId == (UINT32)gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId) {
//
// Initialize SMM specific features on the currently executing CPU
//
SmmCpuFeaturesInitializeProcessor (
Index,
- mIsBsp,
+ IsBsp,
gSmmCpuPrivate->ProcessorInfo,
&mCpuHotPlugData
);
if (!mSmmS3Flag) {
//
// Check XD and BTS features on each processor on normal boot
//
CheckFeatureSupported ();
- } else if (mIsBsp) {
+ } else if (IsBsp) {
//
// BSP rebase is already done above.
// Initialize private data during S3 resume
//
InitializeMpSyncData ();
@@ -405,11 +409,10 @@ SmmRelocateBases (
{
UINT8 BakBuf[BACK_BUF_SIZE];
SMRAM_SAVE_STATE_MAP BakBuf2;
SMRAM_SAVE_STATE_MAP *CpuStatePtr;
UINT8 *U8Ptr;
- UINT32 ApicId;
UINTN Index;
UINTN BspIndex;
//
// Make sure the reserved size is large enough for procedure SmmInitTemplate.
@@ -446,21 +449,20 @@ SmmRelocateBases (
CopyMem (U8Ptr, gcSmmInitTemplate, gcSmmInitSize);
//
// Retrieve the local APIC ID of current processor
//
- ApicId = GetApicId ();
+ mBspApicId = GetApicId ();
//
// Relocate SM bases for all APs
// This is APs' 1st SMI - rebase will be done here, and APs' default SMI handler will be overridden by gcSmmInitTemplate
//
- mIsBsp = FALSE;
BspIndex = (UINTN)-1;
for (Index = 0; Index < mNumberOfCpus; Index++) {
mRebased[Index] = FALSE;
- if (ApicId != (UINT32)gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId) {
+ if (mBspApicId != (UINT32)gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId) {
SendSmiIpi ((UINT32)gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId);
//
// Wait for this AP to finish its 1st SMI
//
while (!mRebased[Index]) {
@@ -475,12 +477,11 @@ SmmRelocateBases (
//
// Relocate BSP's SMM base
//
ASSERT (BspIndex != (UINTN)-1);
- mIsBsp = TRUE;
- SendSmiIpi (ApicId);
+ SendSmiIpi (mBspApicId);
//
// Wait for the BSP to finish its 1st SMI
//
while (!mRebased[BspIndex]) {
}
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v8 3/6] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB Data
2023-02-16 2:46 [PATCH v8 0/6] Simplify SMM Relocation Process Wu, Jiaxin
2023-02-16 2:46 ` [PATCH v8 1/6] UefiCpuPkg/PiSmmCpuDxeSmm: Fix invalid InitializeMpSyncData call Wu, Jiaxin
2023-02-16 2:46 ` [PATCH v8 2/6] UefiCpuPkg/PiSmmCpuDxeSmm: Replace mIsBsp by mBspApicId check Wu, Jiaxin
@ 2023-02-16 2:46 ` Wu, Jiaxin
2023-02-16 4:59 ` Ni, Ray
2023-02-16 2:46 ` [PATCH v8 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM Base Hob for SmBase info Wu, Jiaxin
` (2 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Wu, Jiaxin @ 2023-02-16 2:46 UTC (permalink / raw)
To: devel
Cc: Eric Dong, Ray Ni, Zeng Star, Laszlo Ersek, Gerd Hoffmann,
Rahul Kumar
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4337
The default SMBASE for the x86 processor is 0x30000. When
SMI happens, CPU runs the SMI handler at SMBASE+0x8000.
Also, the SMM save state area is within SMBASE+0x10000.
One of the SMM initialization from CPU perspective is to relocate
and program the new SMBASE (in TSEG range) for each CPU thread. When
the SMBASE relocation happens in a PEI module, the PEI module shall
produce the SMM_BASE_HOB in HOB database which tells the
PiSmmCpuDxeSmm driver (runs at a later phase) about the new SMBASE
for each CPU thread. PiSmmCpuDxeSmm driver installs the SMI handler
at the SMM_BASE_HOB.SmBase[Index]+0x8000 for CPU thread Index. When
the HOB doesn't exist, PiSmmCpuDxeSmm driver shall relocate and
program the new SMBASE itself.
This patch adds the SMM Base HOB for any PEI module to do
the SmBase relocation ahead of PiSmmCpuDxeSmm driver and
store the relocated SmBase address in array for reach
Processors.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
UefiCpuPkg/Include/Guid/SmmBaseHob.h | 75 ++++++++++++++++++++++++++++++++++++
UefiCpuPkg/UefiCpuPkg.dec | 5 ++-
2 files changed, 79 insertions(+), 1 deletion(-)
create mode 100644 UefiCpuPkg/Include/Guid/SmmBaseHob.h
diff --git a/UefiCpuPkg/Include/Guid/SmmBaseHob.h b/UefiCpuPkg/Include/Guid/SmmBaseHob.h
new file mode 100644
index 0000000000..9434276991
--- /dev/null
+++ b/UefiCpuPkg/Include/Guid/SmmBaseHob.h
@@ -0,0 +1,75 @@
+/** @file
+ The Smm Base HOB is used to store the information of:
+ * The relocated SmBase address in array for each Processors.
+
+ The default Smbase for the x86 processor is 0x30000. When SMI happens, CPU
+ runs the SMI handler at Smbase+0x8000. Also, the SMM save state area is within
+ Smbase+0x10000. Since it's the start address to store the CPU save state and
+ code for the SMI entry point, those info are tiled within an SMRAM allocated
+ or reserved buffer. This tile size shall be enough to cover 3 parts:
+ 1. CPU SMRAM Save State Map starts at Smbase + 0xfc00
+ 2. Extra CPU specific context start starts at Smbase + 0xfb00
+ 3. SMI entry point starts at Smbase + 0x8000.
+ Besides, This size should be rounded up to nearest power of 2. The Smm Base HOB
+ producer should be responsible for reserving enough size.
+
+ One of the SMM initialization from CPU perspective is to relocate and program
+ the new Smbase (in TSEG range) for each CPU thread. When the Smbase relocation
+ happens in a PEI module, the PEI module shall produce the SMM_BASE_HOB in HOB
+ database which tells the PiSmmCpuDxeSmm driver (which runs at a later phase)
+ about the new Smbase for each CPU thread. PiSmmCpuDxeSmm driver installs the
+ SMI handler at the SMM_BASE_HOB.Smbase[Index]+0x8000 for CPU thread Index.
+ When the HOB doesn't exist, PiSmmCpuDxeSmm driver shall relocate and program
+ the new Smbase itself.
+
+ Note:
+ 1. Smbase relocation process needs to program the vender specific hardware
+ interface to set Smbase, it should be in the thread scope. It's doable to
+ program the hardware interface using DXE MP service protocol in PiSmmCpuDxeSmm
+ entry point. But, considering the standalone MM environment where the CpuMm
+ driver runs in a isolated environment and it cannot invoke any DXE or PEI MP
+ service, we recommend to put the hardware interface programming in a separate
+ PEI module instead of in the PiSmmCpuDxeSmm driver.
+
+ 2. There is the hard requirement that SMI Entry Size <= 0x1000, data Size <=
+ 0x1000 in PiSmmCpuDxeSmm. So, this require the allocated or reserved buffer in
+ SMRAM should be >= 0x2000.
+
+ Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef SMM_BASE_HOB_H_
+#define SMM_BASE_HOB_H_
+
+#define SMM_BASE_HOB_DATA_GUID \
+ { \
+ 0xc2217ba7, 0x03bb, 0x4f63, {0xa6, 0x47, 0x7c, 0x25, 0xc5, 0xfc, 0x9d, 0x73} \
+ }
+
+#pragma pack(1)
+typedef struct {
+ ///
+ /// CpuIndex tells which CPU range this specific HOB instance described.
+ /// If CpuIndex is set to 0, it indicats the HOB describes the CPU from 0 to
+ /// NumberOfCpus - 1. The HOB list may contains multiple this HOB instances.
+ /// Each HOB instances describe the information for CPU from CpuIndex to
+ /// CpuIndex + NumberOfCpus - 1. The instance order in the HOB list is random
+ /// so consumer can not assume the CpuIndex of first instance is 0.
+ ///
+ UINT32 CpuIndex;
+ ///
+ /// Describes the Number of all max supported processors.
+ ///
+ UINT32 NumberOfProcessors;
+ ///
+ /// Pointer to SmBase address for each Processors.
+ ///
+ UINT64 SmBase[];
+} SMM_BASE_HOB_DATA;
+#pragma pack()
+
+extern EFI_GUID gSmmBaseHobGuid;
+
+#endif
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index cff239d528..7003a2ba77 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -1,9 +1,9 @@
## @file UefiCpuPkg.dec
# This Package provides UEFI compatible CPU modules and libraries.
#
-# Copyright (c) 2007 - 2022, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2023, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -76,10 +76,13 @@
gEdkiiCpuFeaturesInitDoneGuid = { 0xc77c3a41, 0x61ab, 0x4143, { 0x98, 0x3e, 0x33, 0x39, 0x28, 0x6, 0x28, 0xe5 }}
## Include/Guid/MicrocodePatchHob.h
gEdkiiMicrocodePatchHobGuid = { 0xd178f11d, 0x8716, 0x418e, { 0xa1, 0x31, 0x96, 0x7d, 0x2a, 0xc4, 0x28, 0x43 }}
+ ## Include/Guid/SmmBaseHob.h
+ gSmmBaseHobGuid = { 0xc2217ba7, 0x03bb, 0x4f63, {0xa6, 0x47, 0x7c, 0x25, 0xc5, 0xfc, 0x9d, 0x73 }}
+
[Protocols]
## Include/Protocol/SmmCpuService.h
gEfiSmmCpuServiceProtocolGuid = { 0x1d202cab, 0xc8ab, 0x4d5c, { 0x94, 0xf7, 0x3c, 0xfc, 0xc0, 0xd3, 0xd3, 0x35 }}
gEdkiiSmmCpuRendezvousProtocolGuid = { 0xaa00d50b, 0x4911, 0x428f, { 0xb9, 0x1a, 0xa5, 0x9d, 0xdb, 0x13, 0xe2, 0x4c }}
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v8 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM Base Hob for SmBase info
2023-02-16 2:46 [PATCH v8 0/6] Simplify SMM Relocation Process Wu, Jiaxin
` (2 preceding siblings ...)
2023-02-16 2:46 ` [PATCH v8 3/6] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB Data Wu, Jiaxin
@ 2023-02-16 2:46 ` Wu, Jiaxin
2023-02-16 5:01 ` Ni, Ray
2023-02-16 2:46 ` [PATCH v8 5/6] UefiCpuPkg/SmmCpuFeaturesLib: Skip SMBASE configuration Wu, Jiaxin
2023-02-16 2:46 ` [PATCH v8 6/6] OvmfPkg/SmmCpuFeaturesLib: Check SmBase relocation supported or not Wu, Jiaxin
5 siblings, 1 reply; 10+ messages in thread
From: Wu, Jiaxin @ 2023-02-16 2:46 UTC (permalink / raw)
To: devel
Cc: Eric Dong, Ray Ni, Zeng Star, Laszlo Ersek, Gerd Hoffmann,
Rahul Kumar
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4337
Existing SMBASE Relocation is in the PiSmmCpuDxeSmm driver, which
will relocate the SMBASE of each processor by setting the SMBASE
field in the saved state map (at offset 7EF8h) to a new value.
The RSM instruction reloads the internal SMBASE register with the
value in SMBASE field when each time it exits SMM. All subsequent
SMI requests will use the new SMBASE to find the starting address
for the SMI handler (at SMBASE + 8000h).
Due to the default SMBASE for all x86 processors is 0x30000, the
APs' 1st SMI for rebase has to be executed one by one to avoid
the CPUs over-writing each other's SMM Save State Area (see
existing SmmRelocateBases() function), which means the next AP has
to wait for the previous AP to finish its 1st SMI, then it can call
into its 1st SMI for rebase via Smi Ipi command, thus leading the
existing SMBASE Relocation has to be running in series. Besides, it
needs very complex code to handle the AP exit semaphore
(mRebased[Index]), which will hook return address of SMM Save State
so that semaphore code can be executed immediately after AP exits
SMM for SMBASE relocation (see existing SemaphoreHook() function).
With SMM Base Hob support, PiSmmCpuDxeSmm does not need the RSM
instruction to do the SMBASE Relocation. SMBASE Register for each
processors have already been programmed and all SMBASE address have
recorded in SMM Base Hob. So the same default SMBASE Address
(0x30000) will not be used, thus the CPUs over-writing each other's
SMM Save State Area will not happen in PiSmmCpuDxeSmm driver. This
way makes the first SMI init can be executed in parallel and save
boot time on multi-core system. Besides, Semaphore Hook code logic
is also not required, which will greatly simplify the SMBASE
Relocation flow.
Mainly changes as below:
* Assume the biggest possibility of tile size is 8k.
* Combine 2 SMIs (gcSmmInitTemplate & gcSmiHandlerTemplate) into one
(gcSmiHandlerTemplate), the new SMI handler needs to run to 2 paths:
one to SmmCpuFeaturesInitializeProcessor(), the other to SMM Core
Entry Point.
* Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) for first
SMI init before normal SMI sources happen.
* Call SmmCpuFeaturesInitializeProcessor() in parallel.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 31 ++++-
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 25 +++-
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 166 ++++++++++++++++++++++-----
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 26 ++++-
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 3 +-
5 files changed, 214 insertions(+), 37 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index fb4a44eab6..d408b3f9f7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -1,9 +1,9 @@
/** @file
Code for Processor S3 restoration
-Copyright (c) 2006 - 2022, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include "PiSmmCpuDxeSmm.h"
@@ -822,13 +822,38 @@ SmmRestoreCpu (
//
InitializeCpuBeforeRebase ();
}
//
- // Restore SMBASE for BSP and all APs
+ // Make sure the gSmmBaseHobGuid existence status is the same between normal and S3 boot.
//
- SmmRelocateBases ();
+ ASSERT (mSmmRelocated == (BOOLEAN)(GetFirstGuidHob (&gSmmBaseHobGuid) != NULL));
+ if (mSmmRelocated != (BOOLEAN)(GetFirstGuidHob (&gSmmBaseHobGuid) != NULL)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "gSmmBaseHobGuid %a produced in normal boot but %a in S3 boot!",
+ mSmmRelocated ? "is" : "is not",
+ mSmmRelocated ? "is not" : "is"
+ ));
+ CpuDeadLoop ();
+ }
+
+ //
+ // Check whether Smm Relocation is done or not.
+ // If not, will do the SmmBases Relocation here!!!
+ //
+ if (!mSmmRelocated) {
+ //
+ // Restore SMBASE for BSP and all APs
+ //
+ SmmRelocateBases ();
+ } else {
+ //
+ // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute first SMI init.
+ //
+ ExecuteFirstSmiInit ();
+ }
//
// Skip initialization if mAcpiCpuData is not valid
//
if (mAcpiCpuData.NumberOfCpus > 0) {
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index a0967eb69c..baf827cf9d 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1,9 +1,9 @@
/** @file
SMM MP service implementation
-Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -1721,17 +1721,40 @@ SmiRendezvous (
UINTN Index;
UINTN Cr2;
ASSERT (CpuIndex < mMaxNumberOfCpus);
+ if (mSmmRelocated) {
+ ASSERT (mSmmInitialized != NULL);
+ }
+
//
// Save Cr2 because Page Fault exception in SMM may override its value,
// when using on-demand paging for above 4G memory.
//
Cr2 = 0;
SaveCr2 (&Cr2);
+ if (mSmmRelocated && !mSmmInitialized[CpuIndex]) {
+ //
+ // Perform SmmInitHandler for CpuIndex
+ //
+ SmmInitHandler ();
+
+ //
+ // Restore Cr2
+ //
+ RestoreCr2 (Cr2);
+
+ //
+ // Mark the first SMI init for CpuIndex has been done so as to avoid the reentry.
+ //
+ mSmmInitialized[CpuIndex] = TRUE;
+
+ return;
+ }
+
//
// Call the user register Startup function first.
//
if (mSmmMpSyncData->StartupProcedure != NULL) {
mSmmMpSyncData->StartupProcedure (mSmmMpSyncData->StartupProcArgs);
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 6e795d1756..d7a51d7e80 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -82,10 +82,12 @@ EDKII_SMM_MEMORY_ATTRIBUTE_PROTOCOL mSmmMemoryAttribute = {
EdkiiSmmClearMemoryAttributes
};
EFI_CPU_INTERRUPT_HANDLER mExternalVectorTable[EXCEPTION_VECTOR_NUMBER];
+BOOLEAN mSmmRelocated = FALSE;
+volatile BOOLEAN *mSmmInitialized = NULL;
UINT32 mBspApicId = 0;
//
// SMM stack information
//
@@ -381,22 +383,69 @@ SmmInitHandler (
// Initialize private data during S3 resume
//
InitializeMpSyncData ();
}
- //
- // Hook return after RSM to set SMM re-based flag
- //
- SemaphoreHook (Index, &mRebased[Index]);
+ if (!mSmmRelocated) {
+ //
+ // Hook return after RSM to set SMM re-based flag
+ //
+ SemaphoreHook (Index, &mRebased[Index]);
+ }
return;
}
}
ASSERT (FALSE);
}
+/**
+ Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute first SMI init.
+
+**/
+VOID
+ExecuteFirstSmiInit (
+ VOID
+ )
+{
+ UINTN Index;
+
+ if (mSmmInitialized == NULL) {
+ mSmmInitialized = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * mMaxNumberOfCpus);
+ }
+
+ ASSERT (mSmmInitialized != NULL);
+ if (mSmmInitialized == NULL) {
+ return;
+ }
+
+ //
+ // Reset the mSmmInitialized to false.
+ //
+ ZeroMem ((VOID *)mSmmInitialized, sizeof (BOOLEAN) * mMaxNumberOfCpus);
+
+ //
+ // Get the BSP ApicId.
+ //
+ mBspApicId = GetApicId ();
+
+ //
+ // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) for SMM init
+ //
+ SendSmiIpi (mBspApicId);
+ SendSmiIpiAllExcludingSelf ();
+
+ //
+ // Wait for all processors to finish its 1st SMI
+ //
+ for (Index = 0; Index < mNumberOfCpus; Index++) {
+ while (!(BOOLEAN)mSmmInitialized[Index]) {
+ }
+ }
+}
+
/**
Relocate SmmBases for each processor.
Execute on first boot and all S3 resumes
@@ -560,10 +609,15 @@ PiCpuSmmEntry (
UINT32 RegEcx;
UINT32 RegEdx;
UINTN FamilyId;
UINTN ModelId;
UINT32 Cr3;
+ EFI_HOB_GUID_TYPE *GuidHob;
+ SMM_BASE_HOB_DATA *SmmBaseHobData;
+
+ GuidHob = NULL;
+ SmmBaseHobData = NULL;
//
// Initialize address fixup
//
PiSmmCpuSmmInitFixupAddress ();
@@ -788,30 +842,58 @@ PiCpuSmmEntry (
// context must be reduced.
//
ASSERT (TileSize <= (SMRAM_SAVE_STATE_MAP_OFFSET + sizeof (SMRAM_SAVE_STATE_MAP) - SMM_HANDLER_OFFSET));
//
- // Allocate buffer for all of the tiles.
- //
- // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
- // Volume 3C, Section 34.11 SMBASE Relocation
- // For Pentium and Intel486 processors, the SMBASE values must be
- // aligned on a 32-KByte boundary or the processor will enter shutdown
- // state during the execution of a RSM instruction.
- //
- // Intel486 processors: FamilyId is 4
- // Pentium processors : FamilyId is 5
+ // Retrive the allocated SmmBase from gSmmBaseHobGuid. If found,
+ // means the SmBase relocation has been done.
//
- BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize * (mMaxNumberOfCpus - 1));
- if ((FamilyId == 4) || (FamilyId == 5)) {
- Buffer = AllocateAlignedCodePages (BufferPages, SIZE_32KB);
+ GuidHob = GetFirstGuidHob (&gSmmBaseHobGuid);
+ if (GuidHob != NULL) {
+ //
+ // Check whether the Required TileSize is enough.
+ //
+ if (TileSize > SIZE_8KB) {
+ DEBUG ((DEBUG_ERROR, "The Range of Smbase in SMRAM is not enough -- Required TileSize = 0x%08x, Actual TileSize = 0x%08x\n", TileSize, SIZE_8KB));
+ CpuDeadLoop ();
+ return RETURN_BUFFER_TOO_SMALL;
+ }
+
+ SmmBaseHobData = GET_GUID_HOB_DATA (GuidHob);
+
+ //
+ // Assume single instance of HOB produced, expect the HOB.NumberOfProcessors equals to the mMaxNumberOfCpus.
+ //
+ ASSERT (SmmBaseHobData->NumberOfProcessors == (UINT32)mMaxNumberOfCpus && SmmBaseHobData->CpuIndex == 0);
+ mSmmRelocated = TRUE;
} else {
- Buffer = AllocateAlignedCodePages (BufferPages, SIZE_4KB);
- }
+ //
+ // When the HOB doesn't exist, allocate new SMBASE itself.
+ //
+ DEBUG ((DEBUG_INFO, "PiCpuSmmEntry: gSmmBaseHobGuid not found!\n"));
+ //
+ // Allocate buffer for all of the tiles.
+ //
+ // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ // Volume 3C, Section 34.11 SMBASE Relocation
+ // For Pentium and Intel486 processors, the SMBASE values must be
+ // aligned on a 32-KByte boundary or the processor will enter shutdown
+ // state during the execution of a RSM instruction.
+ //
+ // Intel486 processors: FamilyId is 4
+ // Pentium processors : FamilyId is 5
+ //
+ BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize * (mMaxNumberOfCpus - 1));
+ if ((FamilyId == 4) || (FamilyId == 5)) {
+ Buffer = AllocateAlignedCodePages (BufferPages, SIZE_32KB);
+ } else {
+ Buffer = AllocateAlignedCodePages (BufferPages, SIZE_4KB);
+ }
- ASSERT (Buffer != NULL);
- DEBUG ((DEBUG_INFO, "SMRAM SaveState Buffer (0x%08x, 0x%08x)\n", Buffer, EFI_PAGES_TO_SIZE (BufferPages)));
+ ASSERT (Buffer != NULL);
+ DEBUG ((DEBUG_INFO, "New Allcoated SMRAM SaveState Buffer (0x%08x, 0x%08x)\n", Buffer, EFI_PAGES_TO_SIZE (BufferPages)));
+ }
//
// Allocate buffer for pointers to array in SMM_CPU_PRIVATE_DATA.
//
gSmmCpuPrivate->ProcessorInfo = (EFI_PROCESSOR_INFORMATION *)AllocatePool (sizeof (EFI_PROCESSOR_INFORMATION) * mMaxNumberOfCpus);
@@ -842,11 +924,12 @@ PiCpuSmmEntry (
// Retrieve APIC ID of each enabled processor from the MP Services protocol.
// Also compute the SMBASE address, CPU Save State address, and CPU Save state
// size for each CPU in the platform
//
for (Index = 0; Index < mMaxNumberOfCpus; Index++) {
- mCpuHotPlugData.SmBase[Index] = (UINTN)Buffer + Index * TileSize - SMM_HANDLER_OFFSET;
+ mCpuHotPlugData.SmBase[Index] = mSmmRelocated ? (UINTN)SmmBaseHobData->SmBase[Index] : (UINTN)Buffer + Index * TileSize - SMM_HANDLER_OFFSET;
+
gSmmCpuPrivate->CpuSaveStateSize[Index] = sizeof (SMRAM_SAVE_STATE_MAP);
gSmmCpuPrivate->CpuSaveState[Index] = (VOID *)(mCpuHotPlugData.SmBase[Index] + SMRAM_SAVE_STATE_MAP_OFFSET);
gSmmCpuPrivate->Operation[Index] = SmmCpuNone;
if (Index < mNumberOfCpus) {
@@ -955,21 +1038,27 @@ PiCpuSmmEntry (
// Initialize IDT
//
InitializeSmmIdt ();
//
- // Relocate SMM Base addresses to the ones allocated from SMRAM
+ // Check whether Smm Relocation is done or not.
+ // If not, will do the SmmBases Relocation here!!!
//
- mRebased = (BOOLEAN *)AllocateZeroPool (sizeof (BOOLEAN) * mMaxNumberOfCpus);
- ASSERT (mRebased != NULL);
- SmmRelocateBases ();
+ if (!mSmmRelocated) {
+ //
+ // Relocate SMM Base addresses to the ones allocated from SMRAM
+ //
+ mRebased = (BOOLEAN *)AllocateZeroPool (sizeof (BOOLEAN) * mMaxNumberOfCpus);
+ ASSERT (mRebased != NULL);
+ SmmRelocateBases ();
- //
- // Call hook for BSP to perform extra actions in normal mode after all
- // SMM base addresses have been relocated on all CPUs
- //
- SmmCpuFeaturesSmmRelocationComplete ();
+ //
+ // Call hook for BSP to perform extra actions in normal mode after all
+ // SMM base addresses have been relocated on all CPUs
+ //
+ SmmCpuFeaturesSmmRelocationComplete ();
+ }
DEBUG ((DEBUG_INFO, "mXdSupported - 0x%x\n", mXdSupported));
//
// SMM Time initialization
@@ -996,10 +1085,25 @@ PiCpuSmmEntry (
);
}
}
}
+ //
+ // For relocated SMBASE, some MSRs & CSRs are still required to be configured in SMM Mode for SMM Initialization.
+ // Those MSRs & CSRs must be configured before normal SMI sources happen.
+ // So, here is to issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute first SMI init.
+ //
+ if (mSmmRelocated) {
+ ExecuteFirstSmiInit ();
+
+ //
+ // Call hook for BSP to perform extra actions in normal mode after all
+ // SMM base addresses have been relocated on all CPUs
+ //
+ SmmCpuFeaturesSmmRelocationComplete ();
+ }
+
//
// Fill in SMM Reserved Regions
//
gSmmCpuPrivate->SmmReservedSmramRegion[0].SmramReservedStart = 0;
gSmmCpuPrivate->SmmReservedSmramRegion[0].SmramReservedSize = 0;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 5f0a38e400..c3731f174b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -1,9 +1,9 @@
/** @file
Agent Module to load other modules to deploy SMM Entry Vector for X86 CPU.
-Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -23,10 +23,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Protocol/MmMp.h>
#include <Guid/AcpiS3Context.h>
#include <Guid/MemoryAttributesTable.h>
#include <Guid/PiSmmMemoryAttributesTable.h>
+#include <Guid/SmmBaseHob.h>
#include <Library/BaseLib.h>
#include <Library/IoLib.h>
#include <Library/TimerLib.h>
#include <Library/SynchronizationLib.h>
@@ -346,10 +347,29 @@ SmmWriteSaveState (
IN EFI_SMM_SAVE_STATE_REGISTER Register,
IN UINTN CpuIndex,
IN CONST VOID *Buffer
);
+/**
+ C function for SMI handler. To change all processor's SMMBase Register.
+
+**/
+VOID
+EFIAPI
+SmmInitHandler (
+ VOID
+ );
+
+/**
+ Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute first SMI init.
+
+**/
+VOID
+ExecuteFirstSmiInit (
+ VOID
+ );
+
/**
Read a CPU Save State register on the target processor.
This function abstracts the differences that whether the CPU Save State register is in the
IA32 CPU Save State Map or X64 CPU Save State Map.
@@ -400,10 +420,14 @@ WriteSaveStateRegister (
IN EFI_SMM_SAVE_STATE_REGISTER Register,
IN UINTN Width,
IN CONST VOID *Buffer
);
+extern BOOLEAN mSmmRelocated;
+extern volatile BOOLEAN *mSmmInitialized;
+extern UINT32 mBspApicId;
+
extern CONST UINT8 gcSmmInitTemplate[];
extern CONST UINT16 gcSmmInitSize;
X86_ASSEMBLY_PATCH_LABEL gPatchSmmCr0;
extern UINT32 mSmmCr0;
X86_ASSEMBLY_PATCH_LABEL gPatchSmmCr3;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index b4b327f60c..9bfa8c1a76 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -2,11 +2,11 @@
# CPU SMM driver.
#
# This SMM driver performs SMM initialization, deploy SMM Entry Vector,
# provides CPU specific services in SMM.
#
-# Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -112,10 +112,11 @@
[Guids]
gEfiAcpiVariableGuid ## SOMETIMES_CONSUMES ## HOB # it is used for S3 boot.
gEdkiiPiSmmMemoryAttributesTableGuid ## CONSUMES ## SystemTable
gEfiMemoryAttributesTableGuid ## CONSUMES ## SystemTable
+ gSmmBaseHobGuid ## CONSUMES
[FeaturePcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmDebug ## CONSUMES
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmBlockStartupThisAp ## CONSUMES
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection ## CONSUMES
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v8 5/6] UefiCpuPkg/SmmCpuFeaturesLib: Skip SMBASE configuration
2023-02-16 2:46 [PATCH v8 0/6] Simplify SMM Relocation Process Wu, Jiaxin
` (3 preceding siblings ...)
2023-02-16 2:46 ` [PATCH v8 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM Base Hob for SmBase info Wu, Jiaxin
@ 2023-02-16 2:46 ` Wu, Jiaxin
2023-02-16 2:46 ` [PATCH v8 6/6] OvmfPkg/SmmCpuFeaturesLib: Check SmBase relocation supported or not Wu, Jiaxin
5 siblings, 0 replies; 10+ messages in thread
From: Wu, Jiaxin @ 2023-02-16 2:46 UTC (permalink / raw)
To: devel
Cc: Eric Dong, Ray Ni, Zeng Star, Laszlo Ersek, Gerd Hoffmann,
Rahul Kumar
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4337
This patch is to avoid configure SMBASE if SmBase relocation has been
done. If gSmmBaseHobGuid found, means SmBase info has been relocated
and recorded in the SmBase array. No need to do the relocation in
SmmCpuFeaturesInitializeProcessor().
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
---
.../Library/SmmCpuFeaturesLib/CpuFeaturesLib.h | 2 ++
.../SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c | 25 ++++++++++++++++++----
.../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 6 +++++-
.../SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf | 3 ++-
UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c | 3 +--
.../StandaloneMmCpuFeaturesLib.inf | 6 +++++-
6 files changed, 36 insertions(+), 9 deletions(-)
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
index fd3e902547..c2e4fbe96b 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
@@ -7,15 +7,17 @@
**/
#ifndef CPU_FEATURES_LIB_H_
#define CPU_FEATURES_LIB_H_
+#include <Guid/SmmBaseHob.h>
#include <Library/SmmCpuFeaturesLib.h>
#include <Library/BaseLib.h>
#include <Library/PcdLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
/**
Performs library initialization.
This initialization function contains common functionality shared betwen all
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
index d5eaaa7a99..1a2c706fa1 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
@@ -1,9 +1,9 @@
/** @file
Implementation shared across all library instances.
-Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.<BR>
Copyright (c) Microsoft Corporation.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -36,10 +36,16 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
// Set default value to assume IA-32 Architectural MSRs are used
//
UINT32 mSmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE;
UINT32 mSmrrPhysMaskMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK;
+//
+// Indicate SmBase for each Processors has been relocated or not. If TRUE,
+// means no need to do the relocation in SmmCpuFeaturesInitializeProcessor().
+//
+BOOLEAN mSmmCpuFeaturesSmmRelocated;
+
//
// Set default value to assume MTRRs need to be configured on each SMI
//
BOOLEAN mNeedConfigureMtrrs = TRUE;
@@ -142,10 +148,16 @@ CpuFeaturesLibInitialization (
//
// Allocate array for state of SMRR enable on all CPUs
//
mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * GetCpuMaxLogicalProcessorNumber ());
ASSERT (mSmrrEnabled != NULL);
+
+ //
+ // If gSmmBaseHobGuid found, means SmBase info has been relocated and recorded
+ // in the SmBase array.
+ //
+ mSmmCpuFeaturesSmmRelocated = (BOOLEAN)(GetFirstGuidHob (&gSmmBaseHobGuid) != NULL);
}
/**
Called during the very first SMI into System Management Mode to initialize
CPU features, including SMBASE, for the currently executing CPU. Since this
@@ -185,14 +197,19 @@ SmmCpuFeaturesInitializeProcessor (
UINT32 RegEdx;
UINTN FamilyId;
UINTN ModelId;
//
- // Configure SMBASE.
+ // No need to configure SMBASE if SmBase relocation has been done.
//
- CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
- CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
+ if (!mSmmCpuFeaturesSmmRelocated) {
+ //
+ // Configure SMBASE.
+ //
+ CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
+ CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
+ }
//
// Intel(R) 64 and IA-32 Architectures Software Developer's Manual
// Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
//
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 9ac7dde78f..46ae2bf85e 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -1,9 +1,9 @@
## @file
# The CPU specific programming for PiSmmCpuDxeSmm module.
#
-# Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
[Defines]
@@ -31,10 +31,14 @@
[LibraryClasses]
BaseLib
PcdLib
MemoryAllocationLib
DebugLib
+ HobLib
+
+[Guids]
+ gSmmBaseHobGuid ## CONSUMES
[Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## SOMETIMES_CONSUMES
[FeaturePcd]
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
index 86d367e0a0..51322ff189 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
@@ -1,10 +1,10 @@
## @file
# The CPU specific programming for PiSmmCpuDxeSmm module when STM support
# is included.
#
-# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
[Defines]
@@ -62,10 +62,11 @@
[Guids]
gMsegSmramGuid ## SOMETIMES_CONSUMES ## HOB
gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ## SystemTable
gEfiAcpi10TableGuid ## SOMETIMES_CONSUMES ## SystemTable
+ gSmmBaseHobGuid ## CONSUMES
[Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## SOMETIMES_CONSUMES
gUefiCpuPkgTokenSpaceGuid.PcdCpuMsegSize ## SOMETIMES_CONSUMES
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStmExceptionStackSize ## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
index 3cf162ada0..6cb1c515c0 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
@@ -1,16 +1,15 @@
/** @file
SMM STM support functions
- Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include <PiMm.h>
#include <Library/BaseMemoryLib.h>
-#include <Library/HobLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/SmmServicesTableLib.h>
#include <Library/TpmMeasurementLib.h>
#include <Register/Intel/Cpuid.h>
#include <Register/Intel/ArchitecturalMsr.h>
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
index b1f60a5505..c836939d33 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
@@ -1,9 +1,9 @@
## @file
# Standalone MM CPU specific programming.
#
-# Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
# Copyright (c) Microsoft Corporation.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -32,10 +32,14 @@
[LibraryClasses]
BaseLib
DebugLib
MemoryAllocationLib
PcdLib
+ HobLib
+
+[Guids]
+ gSmmBaseHobGuid ## CONSUMES
[FixedPcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## SOMETIMES_CONSUMES
[FeaturePcd]
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v8 6/6] OvmfPkg/SmmCpuFeaturesLib: Check SmBase relocation supported or not
2023-02-16 2:46 [PATCH v8 0/6] Simplify SMM Relocation Process Wu, Jiaxin
` (4 preceding siblings ...)
2023-02-16 2:46 ` [PATCH v8 5/6] UefiCpuPkg/SmmCpuFeaturesLib: Skip SMBASE configuration Wu, Jiaxin
@ 2023-02-16 2:46 ` Wu, Jiaxin
5 siblings, 0 replies; 10+ messages in thread
From: Wu, Jiaxin @ 2023-02-16 2:46 UTC (permalink / raw)
To: devel
Cc: Eric Dong, Ray Ni, Zeng Star, Laszlo Ersek, Gerd Hoffmann,
Rahul Kumar
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4337
This patch is to check SmBase relocation supported or not.
If gSmmBaseHobGuid found, means SmBase info has been relocated
and recorded in the SmBase array. ASSERT it's not supported in OVMF.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
---
OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 10 +++++++++-
OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 6 +++++-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 6693666d04..a1dd10c9f2 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -1,9 +1,9 @@
/** @file
The CPU specific programming for PiSmmCpuDxeSmm module.
- Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include <IndustryStandard/Q35MchIch9.h>
@@ -15,14 +15,16 @@
#include <Library/PcdLib.h>
#include <Library/SafeIntLib.h>
#include <Library/SmmCpuFeaturesLib.h>
#include <Library/SmmServicesTableLib.h>
#include <Library/UefiBootServicesTableLib.h>
+#include <Library/HobLib.h>
#include <Pcd/CpuHotEjectData.h>
#include <PiSmm.h>
#include <Register/Intel/SmramSaveStateMap.h>
#include <Register/QemuSmramSaveStateMap.h>
+#include <Guid/SmmBaseHob.h>
//
// EFER register LMA bit
//
#define LMA BIT10
@@ -41,10 +43,16 @@ EFIAPI
SmmCpuFeaturesLibConstructor (
IN EFI_HANDLE ImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
)
{
+ //
+ // If gSmmBaseHobGuid found, means SmBase info has been relocated and recorded
+ // in the SmBase array. ASSERT it's not supported in OVMF.
+ //
+ ASSERT (GetFirstGuidHob (&gSmmBaseHobGuid) == NULL);
+
//
// No need to program SMRRs on our virtual platform.
//
return EFI_SUCCESS;
}
diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 8a426a4c10..2697a90525 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -1,9 +1,9 @@
## @file
# The CPU specific programming for PiSmmCpuDxeSmm module.
#
-# Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -33,10 +33,14 @@
MemoryAllocationLib
PcdLib
SafeIntLib
SmmServicesTableLib
UefiBootServicesTableLib
+ HobLib
+
+[Guids]
+ gSmmBaseHobGuid ## CONSUMES
[Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress
gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v8 3/6] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB Data
2023-02-16 2:46 ` [PATCH v8 3/6] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB Data Wu, Jiaxin
@ 2023-02-16 4:59 ` Ni, Ray
2023-02-16 5:11 ` Wu, Jiaxin
0 siblings, 1 reply; 10+ messages in thread
From: Ni, Ray @ 2023-02-16 4:59 UTC (permalink / raw)
To: Wu, Jiaxin, devel@edk2.groups.io
Cc: Dong, Eric, Zeng, Star, Laszlo Ersek, Gerd Hoffmann,
Kumar, Rahul R
> -----Original Message-----
> From: Wu, Jiaxin <jiaxin.wu@intel.com>
> Sent: Thursday, February 16, 2023 10:47 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
> Subject: [PATCH v8 3/6] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB
> Data
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4337
>
> The default SMBASE for the x86 processor is 0x30000. When
> SMI happens, CPU runs the SMI handler at SMBASE+0x8000.
> Also, the SMM save state area is within SMBASE+0x10000.
>
> One of the SMM initialization from CPU perspective is to relocate
> and program the new SMBASE (in TSEG range) for each CPU thread. When
> the SMBASE relocation happens in a PEI module, the PEI module shall
> produce the SMM_BASE_HOB in HOB database which tells the
> PiSmmCpuDxeSmm driver (runs at a later phase) about the new SMBASE
> for each CPU thread. PiSmmCpuDxeSmm driver installs the SMI handler
> at the SMM_BASE_HOB.SmBase[Index]+0x8000 for CPU thread Index. When
> the HOB doesn't exist, PiSmmCpuDxeSmm driver shall relocate and
> program the new SMBASE itself.
>
> This patch adds the SMM Base HOB for any PEI module to do
> the SmBase relocation ahead of PiSmmCpuDxeSmm driver and
> store the relocated SmBase address in array for reach
> Processors.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zeng Star <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
> UefiCpuPkg/Include/Guid/SmmBaseHob.h | 75
> ++++++++++++++++++++++++++++++++++++
> UefiCpuPkg/UefiCpuPkg.dec | 5 ++-
> 2 files changed, 79 insertions(+), 1 deletion(-)
> create mode 100644 UefiCpuPkg/Include/Guid/SmmBaseHob.h
>
> diff --git a/UefiCpuPkg/Include/Guid/SmmBaseHob.h
> b/UefiCpuPkg/Include/Guid/SmmBaseHob.h
> new file mode 100644
> index 0000000000..9434276991
> --- /dev/null
> +++ b/UefiCpuPkg/Include/Guid/SmmBaseHob.h
> @@ -0,0 +1,75 @@
> +/** @file
> + The Smm Base HOB is used to store the information of:
> + * The relocated SmBase address in array for each Processors.
1. Processors -> processor.
> + CPU runs the SMI handler at Smbase+0x8000. Also, the SMM save state area is
2. better align the term to use processor.
MP protocol also uses the term "processor"
> +
> + Note:
> + 1. Smbase relocation process needs to program the vender specific
> hardware
> + interface to set Smbase, it should be in the thread scope. It's doable to
3. "it should be in the thread scope" -> "it might be in the thread scope".
The HOB itself doesn't have "thread scope" requirement.
With the "thread scope" possibility, following statement looks good to me.
Just change "should be" to "might be".
> +#pragma pack(1)
> +typedef struct {
> + ///
> + /// CpuIndex tells which CPU range this specific HOB instance described.
> + /// If CpuIndex is set to 0, it indicats the HOB describes the CPU from 0 to
> + /// NumberOfCpus - 1. The HOB list may contains multiple this HOB
> instances.
> + /// Each HOB instances describe the information for CPU from CpuIndex to
> + /// CpuIndex + NumberOfCpus - 1. The instance order in the HOB list is
> random
> + /// so consumer can not assume the CpuIndex of first instance is 0.
> + ///
> + UINT32 CpuIndex;
4. "ProcessorIndex". Please also update the comments to use "processor".
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM Base Hob for SmBase info
2023-02-16 2:46 ` [PATCH v8 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM Base Hob for SmBase info Wu, Jiaxin
@ 2023-02-16 5:01 ` Ni, Ray
0 siblings, 0 replies; 10+ messages in thread
From: Ni, Ray @ 2023-02-16 5:01 UTC (permalink / raw)
To: Wu, Jiaxin, devel@edk2.groups.io
Cc: Dong, Eric, Zeng, Star, Laszlo Ersek, Gerd Hoffmann,
Kumar, Rahul R
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Wu, Jiaxin <jiaxin.wu@intel.com>
> Sent: Thursday, February 16, 2023 10:47 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
> Subject: [PATCH v8 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM
> Base Hob for SmBase info
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4337
>
> Existing SMBASE Relocation is in the PiSmmCpuDxeSmm driver, which
> will relocate the SMBASE of each processor by setting the SMBASE
> field in the saved state map (at offset 7EF8h) to a new value.
> The RSM instruction reloads the internal SMBASE register with the
> value in SMBASE field when each time it exits SMM. All subsequent
> SMI requests will use the new SMBASE to find the starting address
> for the SMI handler (at SMBASE + 8000h).
>
> Due to the default SMBASE for all x86 processors is 0x30000, the
> APs' 1st SMI for rebase has to be executed one by one to avoid
> the CPUs over-writing each other's SMM Save State Area (see
> existing SmmRelocateBases() function), which means the next AP has
> to wait for the previous AP to finish its 1st SMI, then it can call
> into its 1st SMI for rebase via Smi Ipi command, thus leading the
> existing SMBASE Relocation has to be running in series. Besides, it
> needs very complex code to handle the AP exit semaphore
> (mRebased[Index]), which will hook return address of SMM Save State
> so that semaphore code can be executed immediately after AP exits
> SMM for SMBASE relocation (see existing SemaphoreHook() function).
>
> With SMM Base Hob support, PiSmmCpuDxeSmm does not need the RSM
> instruction to do the SMBASE Relocation. SMBASE Register for each
> processors have already been programmed and all SMBASE address have
> recorded in SMM Base Hob. So the same default SMBASE Address
> (0x30000) will not be used, thus the CPUs over-writing each other's
> SMM Save State Area will not happen in PiSmmCpuDxeSmm driver. This
> way makes the first SMI init can be executed in parallel and save
> boot time on multi-core system. Besides, Semaphore Hook code logic
> is also not required, which will greatly simplify the SMBASE
> Relocation flow.
>
> Mainly changes as below:
> * Assume the biggest possibility of tile size is 8k.
> * Combine 2 SMIs (gcSmmInitTemplate & gcSmiHandlerTemplate) into one
> (gcSmiHandlerTemplate), the new SMI handler needs to run to 2 paths:
> one to SmmCpuFeaturesInitializeProcessor(), the other to SMM Core
> Entry Point.
> * Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) for first
> SMI init before normal SMI sources happen.
> * Call SmmCpuFeaturesInitializeProcessor() in parallel.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zeng Star <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 31 ++++-
> UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 25 +++-
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 166
> ++++++++++++++++++++++-----
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 26 ++++-
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 3 +-
> 5 files changed, 214 insertions(+), 37 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> index fb4a44eab6..d408b3f9f7 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> @@ -1,9 +1,9 @@
> /** @file
> Code for Processor S3 restoration
>
> -Copyright (c) 2006 - 2022, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
>
> #include "PiSmmCpuDxeSmm.h"
> @@ -822,13 +822,38 @@ SmmRestoreCpu (
> //
> InitializeCpuBeforeRebase ();
> }
>
> //
> - // Restore SMBASE for BSP and all APs
> + // Make sure the gSmmBaseHobGuid existence status is the same
> between normal and S3 boot.
> //
> - SmmRelocateBases ();
> + ASSERT (mSmmRelocated == (BOOLEAN)(GetFirstGuidHob
> (&gSmmBaseHobGuid) != NULL));
> + if (mSmmRelocated != (BOOLEAN)(GetFirstGuidHob
> (&gSmmBaseHobGuid) != NULL)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "gSmmBaseHobGuid %a produced in normal boot but %a in S3 boot!",
> + mSmmRelocated ? "is" : "is not",
> + mSmmRelocated ? "is not" : "is"
> + ));
> + CpuDeadLoop ();
> + }
> +
> + //
> + // Check whether Smm Relocation is done or not.
> + // If not, will do the SmmBases Relocation here!!!
> + //
> + if (!mSmmRelocated) {
> + //
> + // Restore SMBASE for BSP and all APs
> + //
> + SmmRelocateBases ();
> + } else {
> + //
> + // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute
> first SMI init.
> + //
> + ExecuteFirstSmiInit ();
> + }
>
> //
> // Skip initialization if mAcpiCpuData is not valid
> //
> if (mAcpiCpuData.NumberOfCpus > 0) {
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index a0967eb69c..baf827cf9d 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -1,9 +1,9 @@
> /** @file
> SMM MP service implementation
>
> -Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
> Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -1721,17 +1721,40 @@ SmiRendezvous (
> UINTN Index;
> UINTN Cr2;
>
> ASSERT (CpuIndex < mMaxNumberOfCpus);
>
> + if (mSmmRelocated) {
> + ASSERT (mSmmInitialized != NULL);
> + }
> +
> //
> // Save Cr2 because Page Fault exception in SMM may override its value,
> // when using on-demand paging for above 4G memory.
> //
> Cr2 = 0;
> SaveCr2 (&Cr2);
>
> + if (mSmmRelocated && !mSmmInitialized[CpuIndex]) {
> + //
> + // Perform SmmInitHandler for CpuIndex
> + //
> + SmmInitHandler ();
> +
> + //
> + // Restore Cr2
> + //
> + RestoreCr2 (Cr2);
> +
> + //
> + // Mark the first SMI init for CpuIndex has been done so as to avoid the
> reentry.
> + //
> + mSmmInitialized[CpuIndex] = TRUE;
> +
> + return;
> + }
> +
> //
> // Call the user register Startup function first.
> //
> if (mSmmMpSyncData->StartupProcedure != NULL) {
> mSmmMpSyncData->StartupProcedure (mSmmMpSyncData-
> >StartupProcArgs);
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index 6e795d1756..d7a51d7e80 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -82,10 +82,12 @@ EDKII_SMM_MEMORY_ATTRIBUTE_PROTOCOL
> mSmmMemoryAttribute = {
> EdkiiSmmClearMemoryAttributes
> };
>
> EFI_CPU_INTERRUPT_HANDLER
> mExternalVectorTable[EXCEPTION_VECTOR_NUMBER];
>
> +BOOLEAN mSmmRelocated = FALSE;
> +volatile BOOLEAN *mSmmInitialized = NULL;
> UINT32 mBspApicId = 0;
>
> //
> // SMM stack information
> //
> @@ -381,22 +383,69 @@ SmmInitHandler (
> // Initialize private data during S3 resume
> //
> InitializeMpSyncData ();
> }
>
> - //
> - // Hook return after RSM to set SMM re-based flag
> - //
> - SemaphoreHook (Index, &mRebased[Index]);
> + if (!mSmmRelocated) {
> + //
> + // Hook return after RSM to set SMM re-based flag
> + //
> + SemaphoreHook (Index, &mRebased[Index]);
> + }
>
> return;
> }
> }
>
> ASSERT (FALSE);
> }
>
> +/**
> + Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute first
> SMI init.
> +
> +**/
> +VOID
> +ExecuteFirstSmiInit (
> + VOID
> + )
> +{
> + UINTN Index;
> +
> + if (mSmmInitialized == NULL) {
> + mSmmInitialized = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) *
> mMaxNumberOfCpus);
> + }
> +
> + ASSERT (mSmmInitialized != NULL);
> + if (mSmmInitialized == NULL) {
> + return;
> + }
> +
> + //
> + // Reset the mSmmInitialized to false.
> + //
> + ZeroMem ((VOID *)mSmmInitialized, sizeof (BOOLEAN) *
> mMaxNumberOfCpus);
> +
> + //
> + // Get the BSP ApicId.
> + //
> + mBspApicId = GetApicId ();
> +
> + //
> + // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) for SMM init
> + //
> + SendSmiIpi (mBspApicId);
> + SendSmiIpiAllExcludingSelf ();
> +
> + //
> + // Wait for all processors to finish its 1st SMI
> + //
> + for (Index = 0; Index < mNumberOfCpus; Index++) {
> + while (!(BOOLEAN)mSmmInitialized[Index]) {
> + }
> + }
> +}
> +
> /**
> Relocate SmmBases for each processor.
>
> Execute on first boot and all S3 resumes
>
> @@ -560,10 +609,15 @@ PiCpuSmmEntry (
> UINT32 RegEcx;
> UINT32 RegEdx;
> UINTN FamilyId;
> UINTN ModelId;
> UINT32 Cr3;
> + EFI_HOB_GUID_TYPE *GuidHob;
> + SMM_BASE_HOB_DATA *SmmBaseHobData;
> +
> + GuidHob = NULL;
> + SmmBaseHobData = NULL;
>
> //
> // Initialize address fixup
> //
> PiSmmCpuSmmInitFixupAddress ();
> @@ -788,30 +842,58 @@ PiCpuSmmEntry (
> // context must be reduced.
> //
> ASSERT (TileSize <= (SMRAM_SAVE_STATE_MAP_OFFSET + sizeof
> (SMRAM_SAVE_STATE_MAP) - SMM_HANDLER_OFFSET));
>
> //
> - // Allocate buffer for all of the tiles.
> - //
> - // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
> - // Volume 3C, Section 34.11 SMBASE Relocation
> - // For Pentium and Intel486 processors, the SMBASE values must be
> - // aligned on a 32-KByte boundary or the processor will enter shutdown
> - // state during the execution of a RSM instruction.
> - //
> - // Intel486 processors: FamilyId is 4
> - // Pentium processors : FamilyId is 5
> + // Retrive the allocated SmmBase from gSmmBaseHobGuid. If found,
> + // means the SmBase relocation has been done.
> //
> - BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize *
> (mMaxNumberOfCpus - 1));
> - if ((FamilyId == 4) || (FamilyId == 5)) {
> - Buffer = AllocateAlignedCodePages (BufferPages, SIZE_32KB);
> + GuidHob = GetFirstGuidHob (&gSmmBaseHobGuid);
> + if (GuidHob != NULL) {
> + //
> + // Check whether the Required TileSize is enough.
> + //
> + if (TileSize > SIZE_8KB) {
> + DEBUG ((DEBUG_ERROR, "The Range of Smbase in SMRAM is not
> enough -- Required TileSize = 0x%08x, Actual TileSize = 0x%08x\n", TileSize,
> SIZE_8KB));
> + CpuDeadLoop ();
> + return RETURN_BUFFER_TOO_SMALL;
> + }
> +
> + SmmBaseHobData = GET_GUID_HOB_DATA (GuidHob);
> +
> + //
> + // Assume single instance of HOB produced, expect the
> HOB.NumberOfProcessors equals to the mMaxNumberOfCpus.
> + //
> + ASSERT (SmmBaseHobData->NumberOfProcessors ==
> (UINT32)mMaxNumberOfCpus && SmmBaseHobData->CpuIndex == 0);
> + mSmmRelocated = TRUE;
> } else {
> - Buffer = AllocateAlignedCodePages (BufferPages, SIZE_4KB);
> - }
> + //
> + // When the HOB doesn't exist, allocate new SMBASE itself.
> + //
> + DEBUG ((DEBUG_INFO, "PiCpuSmmEntry: gSmmBaseHobGuid not
> found!\n"));
> + //
> + // Allocate buffer for all of the tiles.
> + //
> + // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
> + // Volume 3C, Section 34.11 SMBASE Relocation
> + // For Pentium and Intel486 processors, the SMBASE values must be
> + // aligned on a 32-KByte boundary or the processor will enter shutdown
> + // state during the execution of a RSM instruction.
> + //
> + // Intel486 processors: FamilyId is 4
> + // Pentium processors : FamilyId is 5
> + //
> + BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize *
> (mMaxNumberOfCpus - 1));
> + if ((FamilyId == 4) || (FamilyId == 5)) {
> + Buffer = AllocateAlignedCodePages (BufferPages, SIZE_32KB);
> + } else {
> + Buffer = AllocateAlignedCodePages (BufferPages, SIZE_4KB);
> + }
>
> - ASSERT (Buffer != NULL);
> - DEBUG ((DEBUG_INFO, "SMRAM SaveState Buffer (0x%08x, 0x%08x)\n",
> Buffer, EFI_PAGES_TO_SIZE (BufferPages)));
> + ASSERT (Buffer != NULL);
> + DEBUG ((DEBUG_INFO, "New Allcoated SMRAM SaveState Buffer
> (0x%08x, 0x%08x)\n", Buffer, EFI_PAGES_TO_SIZE (BufferPages)));
> + }
>
> //
> // Allocate buffer for pointers to array in SMM_CPU_PRIVATE_DATA.
> //
> gSmmCpuPrivate->ProcessorInfo = (EFI_PROCESSOR_INFORMATION
> *)AllocatePool (sizeof (EFI_PROCESSOR_INFORMATION) *
> mMaxNumberOfCpus);
> @@ -842,11 +924,12 @@ PiCpuSmmEntry (
> // Retrieve APIC ID of each enabled processor from the MP Services
> protocol.
> // Also compute the SMBASE address, CPU Save State address, and CPU
> Save state
> // size for each CPU in the platform
> //
> for (Index = 0; Index < mMaxNumberOfCpus; Index++) {
> - mCpuHotPlugData.SmBase[Index] = (UINTN)Buffer + Index * TileSize
> - SMM_HANDLER_OFFSET;
> + mCpuHotPlugData.SmBase[Index] = mSmmRelocated ?
> (UINTN)SmmBaseHobData->SmBase[Index] : (UINTN)Buffer + Index *
> TileSize - SMM_HANDLER_OFFSET;
> +
> gSmmCpuPrivate->CpuSaveStateSize[Index] = sizeof
> (SMRAM_SAVE_STATE_MAP);
> gSmmCpuPrivate->CpuSaveState[Index] = (VOID
> *)(mCpuHotPlugData.SmBase[Index] +
> SMRAM_SAVE_STATE_MAP_OFFSET);
> gSmmCpuPrivate->Operation[Index] = SmmCpuNone;
>
> if (Index < mNumberOfCpus) {
> @@ -955,21 +1038,27 @@ PiCpuSmmEntry (
> // Initialize IDT
> //
> InitializeSmmIdt ();
>
> //
> - // Relocate SMM Base addresses to the ones allocated from SMRAM
> + // Check whether Smm Relocation is done or not.
> + // If not, will do the SmmBases Relocation here!!!
> //
> - mRebased = (BOOLEAN *)AllocateZeroPool (sizeof (BOOLEAN) *
> mMaxNumberOfCpus);
> - ASSERT (mRebased != NULL);
> - SmmRelocateBases ();
> + if (!mSmmRelocated) {
> + //
> + // Relocate SMM Base addresses to the ones allocated from SMRAM
> + //
> + mRebased = (BOOLEAN *)AllocateZeroPool (sizeof (BOOLEAN) *
> mMaxNumberOfCpus);
> + ASSERT (mRebased != NULL);
> + SmmRelocateBases ();
>
> - //
> - // Call hook for BSP to perform extra actions in normal mode after all
> - // SMM base addresses have been relocated on all CPUs
> - //
> - SmmCpuFeaturesSmmRelocationComplete ();
> + //
> + // Call hook for BSP to perform extra actions in normal mode after all
> + // SMM base addresses have been relocated on all CPUs
> + //
> + SmmCpuFeaturesSmmRelocationComplete ();
> + }
>
> DEBUG ((DEBUG_INFO, "mXdSupported - 0x%x\n", mXdSupported));
>
> //
> // SMM Time initialization
> @@ -996,10 +1085,25 @@ PiCpuSmmEntry (
> );
> }
> }
> }
>
> + //
> + // For relocated SMBASE, some MSRs & CSRs are still required to be
> configured in SMM Mode for SMM Initialization.
> + // Those MSRs & CSRs must be configured before normal SMI sources
> happen.
> + // So, here is to issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to
> execute first SMI init.
> + //
> + if (mSmmRelocated) {
> + ExecuteFirstSmiInit ();
> +
> + //
> + // Call hook for BSP to perform extra actions in normal mode after all
> + // SMM base addresses have been relocated on all CPUs
> + //
> + SmmCpuFeaturesSmmRelocationComplete ();
> + }
> +
> //
> // Fill in SMM Reserved Regions
> //
> gSmmCpuPrivate->SmmReservedSmramRegion[0].SmramReservedStart =
> 0;
> gSmmCpuPrivate->SmmReservedSmramRegion[0].SmramReservedSize =
> 0;
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index 5f0a38e400..c3731f174b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -1,9 +1,9 @@
> /** @file
> Agent Module to load other modules to deploy SMM Entry Vector for X86
> CPU.
>
> -Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
> Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -23,10 +23,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Protocol/MmMp.h>
>
> #include <Guid/AcpiS3Context.h>
> #include <Guid/MemoryAttributesTable.h>
> #include <Guid/PiSmmMemoryAttributesTable.h>
> +#include <Guid/SmmBaseHob.h>
>
> #include <Library/BaseLib.h>
> #include <Library/IoLib.h>
> #include <Library/TimerLib.h>
> #include <Library/SynchronizationLib.h>
> @@ -346,10 +347,29 @@ SmmWriteSaveState (
> IN EFI_SMM_SAVE_STATE_REGISTER Register,
> IN UINTN CpuIndex,
> IN CONST VOID *Buffer
> );
>
> +/**
> + C function for SMI handler. To change all processor's SMMBase Register.
> +
> +**/
> +VOID
> +EFIAPI
> +SmmInitHandler (
> + VOID
> + );
> +
> +/**
> + Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute first
> SMI init.
> +
> +**/
> +VOID
> +ExecuteFirstSmiInit (
> + VOID
> + );
> +
> /**
> Read a CPU Save State register on the target processor.
>
> This function abstracts the differences that whether the CPU Save State
> register is in the
> IA32 CPU Save State Map or X64 CPU Save State Map.
> @@ -400,10 +420,14 @@ WriteSaveStateRegister (
> IN EFI_SMM_SAVE_STATE_REGISTER Register,
> IN UINTN Width,
> IN CONST VOID *Buffer
> );
>
> +extern BOOLEAN mSmmRelocated;
> +extern volatile BOOLEAN *mSmmInitialized;
> +extern UINT32 mBspApicId;
> +
> extern CONST UINT8 gcSmmInitTemplate[];
> extern CONST UINT16 gcSmmInitSize;
> X86_ASSEMBLY_PATCH_LABEL gPatchSmmCr0;
> extern UINT32 mSmmCr0;
> X86_ASSEMBLY_PATCH_LABEL gPatchSmmCr3;
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> index b4b327f60c..9bfa8c1a76 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> @@ -2,11 +2,11 @@
> # CPU SMM driver.
> #
> # This SMM driver performs SMM initialization, deploy SMM Entry Vector,
> # provides CPU specific services in SMM.
> #
> -# Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
> # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> @@ -112,10 +112,11 @@
>
> [Guids]
> gEfiAcpiVariableGuid ## SOMETIMES_CONSUMES ## HOB # it is
> used for S3 boot.
> gEdkiiPiSmmMemoryAttributesTableGuid ## CONSUMES ## SystemTable
> gEfiMemoryAttributesTableGuid ## CONSUMES ## SystemTable
> + gSmmBaseHobGuid ## CONSUMES
>
> [FeaturePcd]
> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmDebug ##
> CONSUMES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmBlockStartupThisAp ##
> CONSUMES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection ##
> CONSUMES
> --
> 2.16.2.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 3/6] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB Data
2023-02-16 4:59 ` Ni, Ray
@ 2023-02-16 5:11 ` Wu, Jiaxin
0 siblings, 0 replies; 10+ messages in thread
From: Wu, Jiaxin @ 2023-02-16 5:11 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io
Cc: Dong, Eric, Zeng, Star, Laszlo Ersek, Gerd Hoffmann,
Kumar, Rahul R
Thanks Ray, I will update all you mentioned.
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Thursday, February 16, 2023 12:59 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>;
> Laszlo Ersek <lersek@redhat.com>; Gerd Hoffmann <kraxel@redhat.com>;
> Kumar, Rahul R <rahul.r.kumar@intel.com>
> Subject: RE: [PATCH v8 3/6] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB
> Data
>
>
>
> > -----Original Message-----
> > From: Wu, Jiaxin <jiaxin.wu@intel.com>
> > Sent: Thursday, February 16, 2023 10:47 AM
> > To: devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> > <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>; Gerd Hoffmann
> > <kraxel@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
> > Subject: [PATCH v8 3/6] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB
> > Data
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4337
> >
> > The default SMBASE for the x86 processor is 0x30000. When
> > SMI happens, CPU runs the SMI handler at SMBASE+0x8000.
> > Also, the SMM save state area is within SMBASE+0x10000.
> >
> > One of the SMM initialization from CPU perspective is to relocate
> > and program the new SMBASE (in TSEG range) for each CPU thread. When
> > the SMBASE relocation happens in a PEI module, the PEI module shall
> > produce the SMM_BASE_HOB in HOB database which tells the
> > PiSmmCpuDxeSmm driver (runs at a later phase) about the new SMBASE
> > for each CPU thread. PiSmmCpuDxeSmm driver installs the SMI handler
> > at the SMM_BASE_HOB.SmBase[Index]+0x8000 for CPU thread Index. When
> > the HOB doesn't exist, PiSmmCpuDxeSmm driver shall relocate and
> > program the new SMBASE itself.
> >
> > This patch adds the SMM Base HOB for any PEI module to do
> > the SmBase relocation ahead of PiSmmCpuDxeSmm driver and
> > store the relocated SmBase address in array for reach
> > Processors.
> >
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Zeng Star <star.zeng@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Rahul Kumar <rahul1.kumar@intel.com>
> > Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> > ---
> > UefiCpuPkg/Include/Guid/SmmBaseHob.h | 75
> > ++++++++++++++++++++++++++++++++++++
> > UefiCpuPkg/UefiCpuPkg.dec | 5 ++-
> > 2 files changed, 79 insertions(+), 1 deletion(-)
> > create mode 100644 UefiCpuPkg/Include/Guid/SmmBaseHob.h
> >
> > diff --git a/UefiCpuPkg/Include/Guid/SmmBaseHob.h
> > b/UefiCpuPkg/Include/Guid/SmmBaseHob.h
> > new file mode 100644
> > index 0000000000..9434276991
> > --- /dev/null
> > +++ b/UefiCpuPkg/Include/Guid/SmmBaseHob.h
> > @@ -0,0 +1,75 @@
> > +/** @file
> > + The Smm Base HOB is used to store the information of:
> > + * The relocated SmBase address in array for each Processors.
>
> 1. Processors -> processor.
>
> > + CPU runs the SMI handler at Smbase+0x8000. Also, the SMM save state
> area is
>
> 2. better align the term to use processor.
> MP protocol also uses the term "processor"
>
> > +
> > + Note:
> > + 1. Smbase relocation process needs to program the vender specific
> > hardware
> > + interface to set Smbase, it should be in the thread scope. It's doable to
>
> 3. "it should be in the thread scope" -> "it might be in the thread scope".
> The HOB itself doesn't have "thread scope" requirement.
> With the "thread scope" possibility, following statement looks good to me.
> Just change "should be" to "might be".
>
> > +#pragma pack(1)
> > +typedef struct {
> > + ///
> > + /// CpuIndex tells which CPU range this specific HOB instance described.
> > + /// If CpuIndex is set to 0, it indicats the HOB describes the CPU from 0 to
> > + /// NumberOfCpus - 1. The HOB list may contains multiple this HOB
> > instances.
> > + /// Each HOB instances describe the information for CPU from CpuIndex to
> > + /// CpuIndex + NumberOfCpus - 1. The instance order in the HOB list is
> > random
> > + /// so consumer can not assume the CpuIndex of first instance is 0.
> > + ///
> > + UINT32 CpuIndex;
>
> 4. "ProcessorIndex". Please also update the comments to use "processor".
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-02-16 5:11 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-16 2:46 [PATCH v8 0/6] Simplify SMM Relocation Process Wu, Jiaxin
2023-02-16 2:46 ` [PATCH v8 1/6] UefiCpuPkg/PiSmmCpuDxeSmm: Fix invalid InitializeMpSyncData call Wu, Jiaxin
2023-02-16 2:46 ` [PATCH v8 2/6] UefiCpuPkg/PiSmmCpuDxeSmm: Replace mIsBsp by mBspApicId check Wu, Jiaxin
2023-02-16 2:46 ` [PATCH v8 3/6] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB Data Wu, Jiaxin
2023-02-16 4:59 ` Ni, Ray
2023-02-16 5:11 ` Wu, Jiaxin
2023-02-16 2:46 ` [PATCH v8 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM Base Hob for SmBase info Wu, Jiaxin
2023-02-16 5:01 ` Ni, Ray
2023-02-16 2:46 ` [PATCH v8 5/6] UefiCpuPkg/SmmCpuFeaturesLib: Skip SMBASE configuration Wu, Jiaxin
2023-02-16 2:46 ` [PATCH v8 6/6] OvmfPkg/SmmCpuFeaturesLib: Check SmBase relocation supported or not Wu, Jiaxin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox