This patch is to replace mIsBsp by mBspApicId.
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/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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100063): https://edk2.groups.io/g/devel/message/100063
Mute This Topic: https://groups.io/mt/96932000/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-On Mon, Feb 13, 2023 at 04:44:13PM +0800, Jiaxin Wu wrote:
> This patch is to replace mIsBsp by mBspApicId.
... and mIsBsp becomes the local variable IsBsp ...
> EFIAPI
> SmmInitHandler (
> VOID
> )
> {
> - UINT32 ApicId;
> - UINTN Index;
> + UINT32 ApicId;
> + UINTN Index;
> + BOOLEAN IsBsp;
... which allows running SmmInitHandler in parallel. Which is the
motivation for the change. The commit message should explain that.
The code changes are fine.
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100082): https://edk2.groups.io/g/devel/message/100082
Mute This Topic: https://groups.io/mt/96932000/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
It's hard to say the motivation for this change if we leave the patch series. Standing in a single patch, it's optional change. That's the reason I didn't want to separate this. It's hard to say the benefits of too small granularity patches. But it's fine to me also, I can explain more background why need this.
Thanks,
Jiaxin
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Monday, February 13, 2023 8:34 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>
> Cc: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
> Subject: Re: [PATCH v6 2/6] UefiCpuPkg/PiSmmCpuDxeSmm: Replace
> mIsBsp by mBspApicId
>
> On Mon, Feb 13, 2023 at 04:44:13PM +0800, Jiaxin Wu wrote:
> > This patch is to replace mIsBsp by mBspApicId.
>
> ... and mIsBsp becomes the local variable IsBsp ...
>
> > EFIAPI
> > SmmInitHandler (
> > VOID
> > )
> > {
> > - UINT32 ApicId;
> > - UINTN Index;
> > + UINT32 ApicId;
> > + UINTN Index;
> > + BOOLEAN IsBsp;
>
> ... which allows running SmmInitHandler in parallel. Which is the
> motivation for the change. The commit message should explain that.
>
> The code changes are fine.
>
> take care,
> Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100152): https://edk2.groups.io/g/devel/message/100152
Mute This Topic: https://groups.io/mt/96932000/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Wu, Jiaxin <jiaxin.wu@intel.com>
> Sent: Monday, February 13, 2023 4:44 PM
> 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 v6 2/6] UefiCpuPkg/PiSmmCpuDxeSmm: Replace mIsBsp by
> mBspApicId
>
> This patch is to replace mIsBsp by mBspApicId.
>
> 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/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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100069): https://edk2.groups.io/g/devel/message/100069
Mute This Topic: https://groups.io/mt/96932000/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.