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

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

Hi Laszlo,

Thanks for your patch and regression test. I will include your change when I push the patches.

Thanks,
Eric

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, July 12, 2019 1:54 AM
> To: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [Patch v5 2/2] UefiCpuPkg/PiSmmCpuDxeSmm:
> Enable MM MP Protocol
> 
> Hello Eric,
> 
> On 07/10/19 09:56, Dong, Eric wrote:
> > V5 changes:
> > 1. some small enhancement.
> >
> > v4 changes:
> > 1. Use link list to save the token info.
> >
> > v3 changes:
> > 1. Fix Token clean up too early caused CheckProcedure return error.
> >
> > 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:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1937
> >
> > 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        | 570
> ++++++++++++++++++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   |  18 +
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h   | 193 ++++++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |   3 +
> >  UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.c            | 344 +++++++++++
> >  UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.h            | 286 ++++++++++
> >  6 files changed, 1391 insertions(+), 23 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..f09e2738c3 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > @@ -140,7 +140,7 @@ ReleaseAllAPs (
> >
> >    BspIndex = mSmmMpSyncData->BspIndex;
> >    for (Index = mMaxNumberOfCpus; Index-- > 0;) {
> > -    if (Index != BspIndex && *(mSmmMpSyncData-
> >CpuData[Index].Present)) {
> > +    if (IsPresentAp (Index)) {
> >        ReleaseSemaphore (mSmmMpSyncData->CpuData[Index].Run);
> >      }
> >    }
> 
> version 5 again fails to build for me, with the following error message:
> 
> > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c: In function 'ReleaseAllAPs':
> > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c:139:37: error: variable
> > 'BspIndex' set but not used [-Werror=unused-but-set-variable]
> 
> With the following incremental patch:
> 
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > index f09e2738c30d..ef16997547b8 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > @@ -136,9 +136,7 @@ ReleaseAllAPs (
> >    )
> >  {
> >    UINTN                             Index;
> > -  UINTN                             BspIndex;
> >
> > -  BspIndex = mSmmMpSyncData->BspIndex;
> >    for (Index = mMaxNumberOfCpus; Index-- > 0;) {
> >      if (IsPresentAp (Index)) {
> >        ReleaseSemaphore (mSmmMpSyncData->CpuData[Index].Run);
> 
> the build completes fine (using GCC48).
> 
> If you change nothing on the series other than squashing the above fix, you
> can add my:
> 
> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
> 
> to both patches in the series.
> 
> -*-
> 
> Important: please do not push the series until the 5-level paging commits are
> reverted, and reapplied (with Mike's R-b on the MdePkg patch).
> 
> Thanks!
> Laszlo