Re: [Patch 1/3] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls PeiService.

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Can you just change the AcquireSpinLock() behavior to remove the Timeout PCD consumption?

I haven't seen a real case that the timed acquisition of spin lock is needed.


Thanks/Ray

> -----Original Message-----
> From: Dong, Eric <eric.dong@intel.com>
> Sent: Thursday, December 20, 2018 9:23 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: RE: [edk2] [Patch 1/3] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid
> AP calls PeiService.
> 
> 
> Agreed, Maybe it's time to add a new API like
> AcquireSpinLockWithoutTimeOut?
> 
> Thanks,
> Eric
> > -----Original Message-----
> > From: Yao, Jiewen
> > Sent: Thursday, December 20, 2018 9:19 AM
> > To: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> > Subject: RE: [edk2] [Patch 1/3] UefiCpuPkg/RegisterCpuFeaturesLib:
> > Avoid AP calls PeiService.
> >
> > Hi
> > If we think below code is generic, can we have an API for that?
> >
> > +      //
> > +      // Wait for the AP to release the MSR spin lock.
> > +      //
> > +      while (!AcquireSpinLockOrFail (&CpuFlags->ConsoleLogLock)) {
> > +        CpuPause ();
> > +      }
> >
> >
> >
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> > > Of Eric Dong
> > > Sent: Thursday, December 20, 2018 9:16 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> > > Subject: [edk2] [Patch 1/3] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid
> > > AP calls PeiService.
> > >
> > > In AcquireSpinLock function, it calls GetPerformanceCounter which
> > > final calls PeiService service. This patch avoid to call
> > > AcquireSpinLock function.
> > >
> > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1411
> > >
> > > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Eric Dong <eric.dong@intel.com>
> > > ---
> > >  UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c |
> > > 7
> > > ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git
> > > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > > index 624ddee055..a64326239f 100644
> > > ---
> > > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.
> > > +++ c
> > > @@ -832,7 +832,12 @@ ProgramProcessorRegister (
> > >      RegisterTableEntry = &RegisterTableEntryHead[Index];
> > >
> > >      DEBUG_CODE_BEGIN ();
> > > -      AcquireSpinLock (&CpuFlags->ConsoleLogLock);
> > > +      //
> > > +      // Wait for the AP to release the MSR spin lock.
> > > +      //
> > > +      while (!AcquireSpinLockOrFail (&CpuFlags->ConsoleLogLock)) {
> > > +        CpuPause ();
> > > +      }
> > >        ThreadIndex = ApLocation->Package * CpuStatus->MaxCoreCount *
> > > CpuStatus->MaxThreadCount +
> > >                ApLocation->Core * CpuStatus->MaxThreadCount +
> > >                ApLocation->Thread;
> > > --
> > > 2.15.0.windows.1
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel