Hi Michael, On 1/23/21 3:02 AM, Laszlo Ersek wrote: > +Tom, comments below > > On 01/22/21 18:10, Michael D Kinney wrote: >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3182 >> >> Fix the order of operations in ApWakeupFunction() when PcdCpuApLoopMode >> is set to HLT mode that uses INIT-SIPI-SIPI to wake APs. In this mode, >> volatile state is restored and saved each time a INIT-SIPI-SIPI is sent >> to an AP to request a function to be executed on the AP. When the >> function is completed the volatile state of the AP is saved. However, >> the counters NumApsExecuting and FinishedCount are updated before >> the volatile state is saved. This allows for a race condition window >> for the BSP that is waiting on these counters to request a new >> INIT-SIPI-SIPI before all the APs have completely saved their volatile >> state. The fix is to save the AP volatile state before updating the >> NumApsExecuting and FinishedCount counters. >> >> Cc: Eric Dong <eric.d...@intel.com> >> Cc: Ray Ni <ray...@intel.com> >> Cc: Laszlo Ersek <ler...@redhat.com> >> Cc: Rahul Kumar <rahul1.ku...@intel.com> >> Signed-off-by: Michael D Kinney <michael.d.kin...@intel.com> >> --- >> UefiCpuPkg/Library/MpInitLib/MpLib.c | 31 ++++++++++++++++------------ >> 1 file changed, 18 insertions(+), 13 deletions(-) ...
> When we squash steps #1 through #3, we get *precisely* the posted patch. > > A further improvement from the patch is that the relative order between > incrementing FinishedCount, and decrementing NumApsExecuting, is now > consistent between the SEV-ES-enabled and SEV-ES-disabled cases. > Regardless of SEV-ES, we now incrementing FinishedCount first, and > decrement NumApsExecuting second. Without the patch, the relative order > between these two operations gets inverted upon negating the SEV-ES > enablement. > > I would prefer this patch to be broken up into the three steps that I > reconstructed above (steps #1 and #2 being no-op refactorings), but I > don't insist. I agree with Laszlo, it was easier to understand your changes looking at his explanation. Eventually adding Star's assert() on top. Regards, Phil. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#70769): https://edk2.groups.io/g/devel/message/70769 Mute This Topic: https://groups.io/mt/80035510/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
