* [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired @ 2017-04-18 2:16 Jeff Fan 2017-04-18 7:23 ` Wu, Hao A 2017-04-24 11:41 ` Laszlo Ersek 0 siblings, 2 replies; 5+ messages in thread From: Jeff Fan @ 2017-04-18 2:16 UTC (permalink / raw) To: edk2-devel; +Cc: Hao Wu, Feng Tian, Michael Kinney SMM BSP's *busy* state should be acquired. We could use AcquireSpinLock() instead of AcquireSpinLockOrFail(). Cc: Hao Wu <hao.a.wu@intel.com> Cc: Feng Tian <feng.tian@intel.com> Cc: Michael Kinney <michael.d.kinney@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jeff Fan <jeff.fan@intel.com> --- UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c index a1d16b4..e03f1e0 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c @@ -407,7 +407,7 @@ BSPHandler ( // // The BUSY lock is initialized to Acquired state // - AcquireSpinLockOrFail (mSmmMpSyncData->CpuData[CpuIndex].Busy); + AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy); // // Perform the pre tasks -- 2.9.3.windows.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired 2017-04-18 2:16 [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired Jeff Fan @ 2017-04-18 7:23 ` Wu, Hao A 2017-04-24 11:41 ` Laszlo Ersek 1 sibling, 0 replies; 5+ messages in thread From: Wu, Hao A @ 2017-04-18 7:23 UTC (permalink / raw) To: Fan, Jeff, edk2-devel@lists.01.org; +Cc: Tian, Feng, Kinney, Michael D Reviewed-by: Hao Wu <hao.a.wu@intel.com> Best Regards, Hao Wu > -----Original Message----- > From: Fan, Jeff > Sent: Tuesday, April 18, 2017 10:16 AM > To: edk2-devel@lists.01.org > Cc: Wu, Hao A; Tian, Feng; Kinney, Michael D > Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired > > SMM BSP's *busy* state should be acquired. We could use AcquireSpinLock() > instead of AcquireSpinLockOrFail(). > > Cc: Hao Wu <hao.a.wu@intel.com> > Cc: Feng Tian <feng.tian@intel.com> > Cc: Michael Kinney <michael.d.kinney@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jeff Fan <jeff.fan@intel.com> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > index a1d16b4..e03f1e0 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > @@ -407,7 +407,7 @@ BSPHandler ( > // > // The BUSY lock is initialized to Acquired state > // > - AcquireSpinLockOrFail (mSmmMpSyncData->CpuData[CpuIndex].Busy); > + AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy); > > // > // Perform the pre tasks > -- > 2.9.3.windows.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired 2017-04-18 2:16 [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired Jeff Fan 2017-04-18 7:23 ` Wu, Hao A @ 2017-04-24 11:41 ` Laszlo Ersek 2017-04-25 0:54 ` Fan, Jeff 1 sibling, 1 reply; 5+ messages in thread From: Laszlo Ersek @ 2017-04-24 11:41 UTC (permalink / raw) To: Jeff Fan, edk2-devel; +Cc: Hao Wu, Michael Kinney, Feng Tian Hi Jeff, On 04/18/17 04:16, Jeff Fan wrote: > SMM BSP's *busy* state should be acquired. We could use AcquireSpinLock() > instead of AcquireSpinLockOrFail(). > > Cc: Hao Wu <hao.a.wu@intel.com> > Cc: Feng Tian <feng.tian@intel.com> > Cc: Michael Kinney <michael.d.kinney@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jeff Fan <jeff.fan@intel.com> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > index a1d16b4..e03f1e0 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > @@ -407,7 +407,7 @@ BSPHandler ( > // > // The BUSY lock is initialized to Acquired state > // > - AcquireSpinLockOrFail (mSmmMpSyncData->CpuData[CpuIndex].Busy); > + AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy); > > // > // Perform the pre tasks > what symptoms did you experience without the fix? Thanks Laszlo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired 2017-04-24 11:41 ` Laszlo Ersek @ 2017-04-25 0:54 ` Fan, Jeff 2017-04-25 15:09 ` Laszlo Ersek 0 siblings, 1 reply; 5+ messages in thread From: Fan, Jeff @ 2017-04-25 0:54 UTC (permalink / raw) To: Laszlo Ersek, edk2-devel@lists.01.org Cc: Wu, Hao A, Kinney, Michael D, Tian, Feng Laszlo, There is no any real issue we encountered. Some static code check tool reported AcquireSpinLockOrFai() return value was not been checked. Then I found we may ignore some issue if AcquireSpinLockOrFai() return FALSE (even it will not be happened). Using AcquireSpinLock() is due to the following code are using AcquireSpinLock() to check AP's BUSY state also. Thanks! Jeff -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com] Sent: Monday, April 24, 2017 7:41 PM To: Fan, Jeff; edk2-devel@lists.01.org Cc: Wu, Hao A; Kinney, Michael D; Tian, Feng Subject: Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired Hi Jeff, On 04/18/17 04:16, Jeff Fan wrote: > SMM BSP's *busy* state should be acquired. We could use > AcquireSpinLock() instead of AcquireSpinLockOrFail(). > > Cc: Hao Wu <hao.a.wu@intel.com> > Cc: Feng Tian <feng.tian@intel.com> > Cc: Michael Kinney <michael.d.kinney@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jeff Fan <jeff.fan@intel.com> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > index a1d16b4..e03f1e0 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > @@ -407,7 +407,7 @@ BSPHandler ( > // > // The BUSY lock is initialized to Acquired state > // > - AcquireSpinLockOrFail (mSmmMpSyncData->CpuData[CpuIndex].Busy); > + AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy); > > // > // Perform the pre tasks > what symptoms did you experience without the fix? Thanks Laszlo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired 2017-04-25 0:54 ` Fan, Jeff @ 2017-04-25 15:09 ` Laszlo Ersek 0 siblings, 0 replies; 5+ messages in thread From: Laszlo Ersek @ 2017-04-25 15:09 UTC (permalink / raw) To: Fan, Jeff, edk2-devel@lists.01.org Cc: Wu, Hao A, Kinney, Michael D, Tian, Feng On 04/25/17 02:54, Fan, Jeff wrote: > Laszlo, > > There is no any real issue we encountered. > > Some static code check tool reported AcquireSpinLockOrFai() return value was not been checked. > Then I found we may ignore some issue if AcquireSpinLockOrFai() return FALSE (even it will not be happened). > > Using AcquireSpinLock() is due to the following code are using AcquireSpinLock() to check AP's BUSY state also. Thanks for the explanation! Laszlo > > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Monday, April 24, 2017 7:41 PM > To: Fan, Jeff; edk2-devel@lists.01.org > Cc: Wu, Hao A; Kinney, Michael D; Tian, Feng > Subject: Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired > > Hi Jeff, > > On 04/18/17 04:16, Jeff Fan wrote: >> SMM BSP's *busy* state should be acquired. We could use >> AcquireSpinLock() instead of AcquireSpinLockOrFail(). >> >> Cc: Hao Wu <hao.a.wu@intel.com> >> Cc: Feng Tian <feng.tian@intel.com> >> Cc: Michael Kinney <michael.d.kinney@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Jeff Fan <jeff.fan@intel.com> >> --- >> UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c >> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c >> index a1d16b4..e03f1e0 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c >> @@ -407,7 +407,7 @@ BSPHandler ( >> // >> // The BUSY lock is initialized to Acquired state >> // >> - AcquireSpinLockOrFail (mSmmMpSyncData->CpuData[CpuIndex].Busy); >> + AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy); >> >> // >> // Perform the pre tasks >> > > what symptoms did you experience without the fix? > > Thanks > Laszlo > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-04-25 15:09 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-18 2:16 [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired Jeff Fan 2017-04-18 7:23 ` Wu, Hao A 2017-04-24 11:41 ` Laszlo Ersek 2017-04-25 0:54 ` Fan, Jeff 2017-04-25 15:09 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox