* [edk2-devel] [Patch V3] UefiCpuPkg/PiSmmCpuDxeSmm: SmmCpuRendezvous ensure all Aps in Present. @ 2023-12-15 9:29 Yuanhao Xie 2023-12-19 4:01 ` Ni, Ray 0 siblings, 1 reply; 3+ messages in thread From: Yuanhao Xie @ 2023-12-15 9:29 UTC (permalink / raw) To: devel; +Cc: xieyuanh, Zhihao Li, Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu SMM read save state requires AP must be present. This patch aim to avoid the AP not ready for save state check. Furthermore, SmmWaitForApArrival has two callers: SmmCpuRendezvous and BSPHandler, the behavior of the two callers can be consistent by add the while loop to check the present of the APs to SmmCpuRendezvous. Signed-off-by: Zhihao Li <zhihao.li@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Jiaxin Wu <jiaxin.wu@intel.com> --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c | 27 +++++++++++++++++++++++++++ UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 13 +++++++++++++ 2 files changed, 40 insertions(+) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c index c0485b0519..3e98955319 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c @@ -416,8 +416,15 @@ SmmCpuRendezvous ( IN BOOLEAN BlockingMode ) { + UINTN Index; + UINTN PresentCount; + UINT32 BlockedCount; + UINT32 DisabledCount; EFI_STATUS Status; + BlockedCount = 0; + DisabledCount = 0; + // // Return success immediately if all CPUs are already synchronized. // @@ -436,6 +443,26 @@ SmmCpuRendezvous ( // There are some APs outside SMM, Wait for all avaiable APs to arrive. // SmmWaitForApArrival (); + + // + // Make sure all APs have their Present flag set + // + while (TRUE) { + PresentCount = 0; + for (Index = 0; Index < mMaxNumberOfCpus; Index++) { + if (*(mSmmMpSyncData->CpuData[Index].Present)) { + PresentCount++; + } + } + + GetSmmDelayedBlockedDisabledCount (NULL, &BlockedCount, &DisabledCount); + if (PresentCount == mMaxNumberOfCpus - BlockedCount - DisabledCount ) { + break; + } + + CpuPause (); + } + Status = mSmmMpSyncData->AllApArrivedWithException ? EFI_SUCCESS : EFI_TIMEOUT; } else { // diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h index f18345881b..e3538957fb 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h @@ -1552,6 +1552,19 @@ SmmWaitForApArrival ( VOID ); +/** + Returns the Number of SMM Delayed & Blocked & Disabled Thread Count. + @param[in,out] DelayedCount The Number of SMM Delayed Thread Count. + @param[in,out] BlockedCount The Number of SMM Blocked Thread Count. + @param[in,out] DisabledCount The Number of SMM Disabled Thread Count. +**/ +VOID +GetSmmDelayedBlockedDisabledCount ( + IN OUT UINT32 *DelayedCount, + IN OUT UINT32 *BlockedCount, + IN OUT UINT32 *DisabledCount + ); + /** Write unprotect read-only pages if Cr0.Bits.WP is 1. -- 2.39.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112589): https://edk2.groups.io/g/devel/message/112589 Mute This Topic: https://groups.io/mt/103187773/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [Patch V3] UefiCpuPkg/PiSmmCpuDxeSmm: SmmCpuRendezvous ensure all Aps in Present. 2023-12-15 9:29 [edk2-devel] [Patch V3] UefiCpuPkg/PiSmmCpuDxeSmm: SmmCpuRendezvous ensure all Aps in Present Yuanhao Xie @ 2023-12-19 4:01 ` Ni, Ray 2023-12-19 5:12 ` Yuanhao Xie 0 siblings, 1 reply; 3+ messages in thread From: Ni, Ray @ 2023-12-19 4:01 UTC (permalink / raw) To: Xie, Yuanhao, devel@edk2.groups.io Cc: Li, Zhihao, Kumar, Rahul R, Gerd Hoffmann, Wu, Jiaxin Yuanhao, Zhihao, Can you kindly explain a bit more about this sentence in commit message " SMM read save state requires AP must be present."? Thanks, Ray > -----Original Message----- > From: Xie, Yuanhao <yuanhao.xie@intel.com> > Sent: Friday, December 15, 2023 5:30 PM > To: devel@edk2.groups.io > Cc: Xie, Yuanhao <yuanhao.xie@intel.com>; Li, Zhihao <zhihao.li@intel.com>; > Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd > Hoffmann <kraxel@redhat.com>; Wu, Jiaxin <jiaxin.wu@intel.com> > Subject: [Patch V3] UefiCpuPkg/PiSmmCpuDxeSmm: SmmCpuRendezvous > ensure all Aps in Present. > > SMM read save state requires AP must be present. > This patch aim to avoid the AP not ready for save state check. > Furthermore, SmmWaitForApArrival has two callers: SmmCpuRendezvous and > BSPHandler, the behavior of the two callers can be consistent by add > the while loop to check the present of the APs to SmmCpuRendezvous. > > Signed-off-by: Zhihao Li <zhihao.li@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Rahul Kumar <rahul1.kumar@intel.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Jiaxin Wu <jiaxin.wu@intel.com> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c | 27 > +++++++++++++++++++++++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 13 +++++++++++++ > 2 files changed, 40 insertions(+) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c > index c0485b0519..3e98955319 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c > @@ -416,8 +416,15 @@ SmmCpuRendezvous ( > IN BOOLEAN BlockingMode > ) > { > + UINTN Index; > + UINTN PresentCount; > + UINT32 BlockedCount; > + UINT32 DisabledCount; > EFI_STATUS Status; > > + BlockedCount = 0; > + DisabledCount = 0; > + > // > // Return success immediately if all CPUs are already synchronized. > // > @@ -436,6 +443,26 @@ SmmCpuRendezvous ( > // There are some APs outside SMM, Wait for all avaiable APs to arrive. > // > SmmWaitForApArrival (); > + > + // > + // Make sure all APs have their Present flag set > + // > + while (TRUE) { > + PresentCount = 0; > + for (Index = 0; Index < mMaxNumberOfCpus; Index++) { > + if (*(mSmmMpSyncData->CpuData[Index].Present)) { > + PresentCount++; > + } > + } > + > + GetSmmDelayedBlockedDisabledCount (NULL, &BlockedCount, > &DisabledCount); > + if (PresentCount == mMaxNumberOfCpus - BlockedCount - > DisabledCount ) { > + break; > + } > + > + CpuPause (); > + } > + > Status = mSmmMpSyncData->AllApArrivedWithException ? > EFI_SUCCESS : EFI_TIMEOUT; > } else { > // > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > index f18345881b..e3538957fb 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > @@ -1552,6 +1552,19 @@ SmmWaitForApArrival ( > VOID > ); > > +/** > + Returns the Number of SMM Delayed & Blocked & Disabled Thread Count. > + @param[in,out] DelayedCount The Number of SMM Delayed Thread > Count. > + @param[in,out] BlockedCount The Number of SMM Blocked Thread > Count. > + @param[in,out] DisabledCount The Number of SMM Disabled Thread > Count. > +**/ > +VOID > +GetSmmDelayedBlockedDisabledCount ( > + IN OUT UINT32 *DelayedCount, > + IN OUT UINT32 *BlockedCount, > + IN OUT UINT32 *DisabledCount > + ); > + > /** > Write unprotect read-only pages if Cr0.Bits.WP is 1. > > -- > 2.39.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112683): https://edk2.groups.io/g/devel/message/112683 Mute This Topic: https://groups.io/mt/103187773/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [Patch V3] UefiCpuPkg/PiSmmCpuDxeSmm: SmmCpuRendezvous ensure all Aps in Present. 2023-12-19 4:01 ` Ni, Ray @ 2023-12-19 5:12 ` Yuanhao Xie 0 siblings, 0 replies; 3+ messages in thread From: Yuanhao Xie @ 2023-12-19 5:12 UTC (permalink / raw) To: Ni, Ray Cc: Li, Zhihao, Kumar, Rahul R, Gerd Hoffmann, Wu, Jiaxin, devel@edk2.groups.io Hi Ray, It means that: The "SmmCpuRendezvous" function has a bug in that it summons all APs into SMM, but there is a time gap before they are set as "present." During this window, if using the "setVariable" operation, it can cause issues because "setVariable" requires all APs to be in SMM and marked as "present." Therefore, this patch adds extra logic to "SmmCpuRendezvous" to check if the APs are already "present" before proceeding. Thanks, Yuanhao -----Original Message----- From: Ni, Ray <ray.ni@intel.com> Sent: Tuesday, December 19, 2023 12:01 PM To: Xie, Yuanhao <yuanhao.xie@intel.com>; devel@edk2.groups.io Cc: Li, Zhihao <zhihao.li@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Wu, Jiaxin <jiaxin.wu@intel.com> Subject: RE: [Patch V3] UefiCpuPkg/PiSmmCpuDxeSmm: SmmCpuRendezvous ensure all Aps in Present. Yuanhao, Zhihao, Can you kindly explain a bit more about this sentence in commit message " SMM read save state requires AP must be present."? Thanks, Ray > -----Original Message----- > From: Xie, Yuanhao <yuanhao.xie@intel.com> > Sent: Friday, December 15, 2023 5:30 PM > To: devel@edk2.groups.io > Cc: Xie, Yuanhao <yuanhao.xie@intel.com>; Li, Zhihao > <zhihao.li@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R > <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Wu, > Jiaxin <jiaxin.wu@intel.com> > Subject: [Patch V3] UefiCpuPkg/PiSmmCpuDxeSmm: SmmCpuRendezvous ensure > all Aps in Present. > > SMM read save state requires AP must be present. > This patch aim to avoid the AP not ready for save state check. > Furthermore, SmmWaitForApArrival has two callers: SmmCpuRendezvous and > BSPHandler, the behavior of the two callers can be consistent by add > the while loop to check the present of the APs to SmmCpuRendezvous. > > Signed-off-by: Zhihao Li <zhihao.li@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Rahul Kumar <rahul1.kumar@intel.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Jiaxin Wu <jiaxin.wu@intel.com> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c | 27 > +++++++++++++++++++++++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 13 +++++++++++++ > 2 files changed, 40 insertions(+) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c > index c0485b0519..3e98955319 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c > @@ -416,8 +416,15 @@ SmmCpuRendezvous ( > IN BOOLEAN BlockingMode > ) > { > + UINTN Index; > + UINTN PresentCount; > + UINT32 BlockedCount; > + UINT32 DisabledCount; > EFI_STATUS Status; > > + BlockedCount = 0; > + DisabledCount = 0; > + > // > // Return success immediately if all CPUs are already synchronized. > // > @@ -436,6 +443,26 @@ SmmCpuRendezvous ( > // There are some APs outside SMM, Wait for all avaiable APs to arrive. > // > SmmWaitForApArrival (); > + > + // > + // Make sure all APs have their Present flag set > + // > + while (TRUE) { > + PresentCount = 0; > + for (Index = 0; Index < mMaxNumberOfCpus; Index++) { > + if (*(mSmmMpSyncData->CpuData[Index].Present)) { > + PresentCount++; > + } > + } > + > + GetSmmDelayedBlockedDisabledCount (NULL, &BlockedCount, > &DisabledCount); > + if (PresentCount == mMaxNumberOfCpus - BlockedCount - > DisabledCount ) { > + break; > + } > + > + CpuPause (); > + } > + > Status = mSmmMpSyncData->AllApArrivedWithException ? > EFI_SUCCESS : EFI_TIMEOUT; > } else { > // > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > index f18345881b..e3538957fb 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > @@ -1552,6 +1552,19 @@ SmmWaitForApArrival ( > VOID > ); > > +/** > + Returns the Number of SMM Delayed & Blocked & Disabled Thread Count. > + @param[in,out] DelayedCount The Number of SMM Delayed Thread > Count. > + @param[in,out] BlockedCount The Number of SMM Blocked Thread > Count. > + @param[in,out] DisabledCount The Number of SMM Disabled Thread > Count. > +**/ > +VOID > +GetSmmDelayedBlockedDisabledCount ( > + IN OUT UINT32 *DelayedCount, > + IN OUT UINT32 *BlockedCount, > + IN OUT UINT32 *DisabledCount > + ); > + > /** > Write unprotect read-only pages if Cr0.Bits.WP is 1. > > -- > 2.39.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112689): https://edk2.groups.io/g/devel/message/112689 Mute This Topic: https://groups.io/mt/103187773/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-12-19 5:12 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-12-15 9:29 [edk2-devel] [Patch V3] UefiCpuPkg/PiSmmCpuDxeSmm: SmmCpuRendezvous ensure all Aps in Present Yuanhao Xie 2023-12-19 4:01 ` Ni, Ray 2023-12-19 5:12 ` Yuanhao Xie
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox