Min, You cannot use C global variable in PEIM. Can you add a new field in _CPU_MP_DATA?
> -----Original Message----- > From: Xu, Min M <min.m...@intel.com> > Sent: Friday, April 29, 2022 11:03 AM > To: devel@edk2.groups.io > Cc: Xu, Min M <min.m...@intel.com>; Brijesh Singh <brijesh.si...@amd.com>; > Aktas, Erdem <erdemak...@google.com>; > James Bottomley <j...@linux.ibm.com>; Yao, Jiewen <jiewen....@intel.com>; Tom > Lendacky <thomas.lenda...@amd.com>; > Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com> > Subject: [PATCH 1/1] UefiCpuPkg: Save PcdConfidentialComputingGuestAttr in > mCcGuestAttr > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3918 > > There is an issue reported when booting multiple vCPU guests (regular > and SEV). This issues consist of EfiAcquireLock() / EfiReleaseLock() > ASSERTS and TPL level ASSERTS that occur during ExitBootServices when > the APs are being parked by RelocateApLoop(). The root cause is that > PCD accesses use locking which is not SMP safe. > > To fix this issue PCD usage can be reduced to getting the > PcdConfidentialComputingGuestAttr value at init (MpInitLibInitialize) > and caching it in a STATIC variable (mCcGuestAttr). > > Cc: Brijesh Singh <brijesh.si...@amd.com> > Cc: Erdem Aktas <erdemak...@google.com> > Cc: James Bottomley <j...@linux.ibm.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Tom Lendacky <thomas.lenda...@amd.com> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Signed-off-by: Min Xu <min.m...@intel.com> > --- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index 91c7afaeb2ad..fb9247f58427 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -16,6 +16,7 @@ > #include <ConfidentialComputingGuestAttr.h> > > EFI_GUID mCpuInitMpLibHobGuid = CPU_INIT_MP_LIB_HOB_GUID; > +UINT64 mCcGuestAttr = 0; > > /** > The function will check if BSP Execute Disable is enabled. > @@ -1805,7 +1806,9 @@ MpInitLibInitialize ( > UINTN BackupBufferAddr; > UINTN ApIdtBase; > > - if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) { > + mCcGuestAttr = PcdGet64 (PcdConfidentialComputingGuestAttr); > + > + if (CC_GUEST_IS_TDX (mCcGuestAttr)) { > return EFI_SUCCESS; > } > > @@ -2079,7 +2082,7 @@ MpInitLibGetProcessorInfo ( > CPU_INFO_IN_HOB *CpuInfoInHob; > UINTN OriginalProcessorNumber; > > - if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) { > + if (CC_GUEST_IS_TDX (mCcGuestAttr)) { > return TdxMpInitLibGetProcessorInfo (ProcessorNumber, > ProcessorInfoBuffer, HealthData); > } > > @@ -2177,7 +2180,7 @@ SwitchBSPWorker ( > BOOLEAN OldInterruptState; > BOOLEAN OldTimerInterruptState; > > - if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) { > + if (CC_GUEST_IS_TDX (mCcGuestAttr)) { > return EFI_UNSUPPORTED; > } > > @@ -2321,7 +2324,7 @@ EnableDisableApWorker ( > CPU_MP_DATA *CpuMpData; > UINTN CallerNumber; > > - if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) { > + if (CC_GUEST_IS_TDX (mCcGuestAttr)) { > return EFI_UNSUPPORTED; > } > > @@ -2385,7 +2388,7 @@ MpInitLibWhoAmI ( > return EFI_INVALID_PARAMETER; > } > > - if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) { > + if (CC_GUEST_IS_TDX (mCcGuestAttr)) { > *ProcessorNumber = 0; > return EFI_SUCCESS; > } > @@ -2432,7 +2435,7 @@ MpInitLibGetNumberOfProcessors ( > return EFI_INVALID_PARAMETER; > } > > - if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) { > + if (CC_GUEST_IS_TDX (mCcGuestAttr)) { > return TdxMpInitLibGetNumberOfProcessors (NumberOfProcessors, > NumberOfEnabledProcessors); > } > > @@ -2534,7 +2537,7 @@ StartupAllCPUsWorker ( > return EFI_INVALID_PARAMETER; > } > > - if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) { > + if (CC_GUEST_IS_TDX (mCcGuestAttr)) { > // > // For Td guest ExcludeBsp must be FALSE. Otherwise it will return in > above checks. > // > @@ -2692,7 +2695,7 @@ StartupThisAPWorker ( > // > // In Td guest, startup of AP is not supported in current stage. > // > - if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) { > + if (CC_GUEST_IS_TDX (mCcGuestAttr)) { > return EFI_UNSUPPORTED; > } > > -- > 2.29.2.windows.2 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#89382): https://edk2.groups.io/g/devel/message/89382 Mute This Topic: https://groups.io/mt/90768905/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
