Re: [edk2-devel] [Patch v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Enable MM MP Protocol.

["Dong, Eric" <eric.dong@intel.com>]

Hi Laszlo,


> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, July 2, 2019 10:16 PM
> To: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [Patch v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm:
> Enable MM MP Protocol.
> 
> Hi Eric,
> 
> I cannot apply this patch for testing, because it is corrupt. Git complains about
> it (and it's justified):
> 
> On 07/02/19 09:37, Dong, Eric wrote:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1937
> >
> > v2 changes:
> > 1. Remove some duplicated global variables.
> > 2. Enhance token design to support multiple task trig for different
> > APs at the same time.
> >
> > V1 changes:
> > Add MM Mp Protocol in PiSmmCpuDxeSmm driver.
> >
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Signed-off-by: Eric Dong <eric.dong@intel.com>
> > ---
> >  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c        | 555
> ++++++++++++++++++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   |  11 +
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h   | 160 +++++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |   3 +
> >  UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.c            | 372 +++++++++++++
> >  UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.h            | 286 ++++++++++
> >  6 files changed, 1370 insertions(+), 17 deletions(-)  create mode
> > 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.c  create mode 100644
> > UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.h
> >
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > index 64fb4d6344..76bcee7133 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > @@ -22,6 +22,7 @@ UINTN                                       mSemaphoreSize;
> >  SPIN_LOCK                                   *mPFLock = NULL;
> >  SMM_CPU_SYNC_MODE                           mCpuSmmSyncMode;
> >  BOOLEAN                                     mMachineCheckSupported = FALSE;
> > +SPIN_LOCK                                   **mApTokenLock;
> >
> 
> Please note the empty line above. In the patch, the line consists of a single
> space character (before the line terminator), expressing that the line is
> preseved from the context. So this is a *valid* patch line.
> 
> Now compare:
> 
> >  /**
> >    Performs an atomic compare exchange operation to get semaphore.
> > @@ -146,6 +147,45 @@ ReleaseAllAPs (
> >    }
> >  }
> >
> > +/**
> > +  Wheck whether task has been finished by all APs.
> > +
> > +  @param       BlockMode   Whether did it in block mode or non-block
> mode.
> > +
> > +  @retval      TRUE        Task has been finished by all APs.
> > +  @retval      FALSE       Task not has been finished by all APs.
> > +
> > +**/
> > +BOOLEAN
> > +IsTaskFinishInAllAPs (
> > +  IN BOOLEAN                        BlockMode
> > +  )
> > +{
> > +  UINTN                             Index;
> > +
> > +  for (Index = mMaxNumberOfCpus; Index-- > 0;) {
> > +    //
> > +    // Ignore BSP and APs which not call in SMM.
> > +    //
> > +    if ((Index == gSmmCpuPrivate-
> >SmmCoreEntryContext.CurrentlyExecutingCpu) || (!*(mSmmMpSyncData-
> >CpuData[Index].Present))) {
> > +      continue;
> > +    }
> > +
> > +    if (BlockMode) {
> > +      AcquireSpinLock(mSmmMpSyncData->CpuData[Index].Busy);
> >
> 
> The above line is malformed. In the patch, the line has *zero* characters
> before the line terminator. That's invalid: the first character should be a
> space (preserved context), plus sign (new line) or minus sign (line being
> removed).
> 

I test v2 patch in my local and don't meet the error. 
I check the original git patch, it already adds the empty line. I manually remove this empty line and send V3 patch again. 
Can you help to check the new patch?

Thanks,
Eric

> I don't know how this patch was generated, but it doesn't conform to the
> expected format. Please both repost the series and push it to a topic branch
> in your repo.
> 
> I had no problems applying the first patch in the series; what's more, in patch
> #2, the changes for the other files apply just fine.
> 
> Thanks
> Laszlo