Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use SmmWaitForAllProcessor() in VariableSmm driver.

["Li, Zhihao" <zhihao.li@intel.com>]

I see the configuration in MdeModulePkg\MdeModulePkg.ci.yaml. I add SmmCpuRendezvousLib.h in
MdeModulePkg/Include/Library folder so that it doesn't need add UefiCpuPkg.dec in [Packages] and bypass the check.

For the second point, due to the patch pass the CI test, it also pass the PlatformCI_ArmVirtPkg. I don't realize that any problem in
Arm platform build.
Leif and Ard
Does the patch has any influence on arm platform build?

Liming:
If the solution is not acceptable, how about I create NULL version of SmmCpuRendezvousLib in MdeModulePkg and use it in MdeModulePkg.dsc? Is that a acceptable solution for you?

> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Wednesday, April 13, 2022 9:30 AM
> To: Li, Zhihao <zhihao.li@intel.com>; devel@edk2.groups.io; 'Michael Kubacki'
> <mikuback@linux.microsoft.com>; 'Ard Biesheuvel'
> <ardb+tianocore@kernel.org>; 'Leif Lindholm' <leif@nuviainc.com>; 'Sean
> Brogan' <sean.brogan@microsoft.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>
> Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> SmmWaitForAllProcessor() in VariableSmm driver.
> 
> Zhihao:
>   I remember CI has the check for the package dependency. If this patch passes
> CI, seemly this checker doesn't do. You can see DependencyCheck in
> MdeModulePkg\MdeModulePkg.ci.yaml.
>   And, this patch introduces new dependency in VariableStandaloneMm. It has
> been used in edk2 platform ARM platform. This change will break these platform
> build. Please notify the platform owners.
> 
> Sean and Michael:
>   This patch adds UefiCpuPkg library instance SmmCpuRendezvousLib into
> MdeModulePkg.dsc. But, CI can pass. Is this the expected behavior?
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Li, Zhihao <zhihao.li@intel.com>
> > 发送时间: 2022年4月13日 2:14
> > 收件人: Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> > 抄送: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan
> > <siyuan.fu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > 主题: RE: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > SmmWaitForAllProcessor() in VariableSmm driver.
> >
> > 1. Although SmmCpuRendezvousLib in UefiCpuPkg, add SmmRendezvousLib.h
> > into MdeModulePkg/Include/Library folder bypass the check.
> > 2. The SmmRendezvousLib is a standalone MM library and doesn’t have
> > any DXE service dependency. It can be used by SMM variable module and
> > MM variable module.
> >
> > As the patch following, it have passed the Edk2 CI test. The code can
> > run successfully in practice, but I'm not sure if this is acceptable
> > in terms of the standard.
> >
> > -----Original Message-----
> > From: gaoliming <gaoliming@byosoft.com.cn>
> > Sent: Tuesday, April 12, 2022 9:00 AM
> > To: devel@edk2.groups.io; Li, Zhihao <zhihao.li@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan
> > <siyuan.fu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > SmmWaitForAllProcessor() in VariableSmm driver.
> >
> > Zhihao:
> >   This patch breaks two things. One is to let MdeModulePkg depend on
> > UefiCpuPkg, another is to let VariableStandaloneMm depend on
> > UefiCpuPkg SmmCpuRendezvousLib. Please provide your proposal to
> > resolve these two dependency first.
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Li, Zhihao
> > > 发送时间: 2022年4月11日 15:07
> > > 收件人: devel@edk2.groups.io
> > > 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> > > <gaoliming@byosoft.com.cn>; Siyuan Fu <siyuan.fu@intel.com>; Ni Ray
> > > <ray.ni@intel.com>; Michael D Kinney <michael.d.kinney@intel.com>
> > > 主题: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > > SmmWaitForAllProcessor() in VariableSmm driver.
> > >
> > > REF： https://bugzilla.tianocore.org/show_bug.cgi?id=3854
> > >
> > > In UefiCpuPkg, there are a new Protocol with the new service
> > > SmmWaitForAllProcessor(), which can be used by SMI handler to
> > > optionally wait for other APs to complete SMM rendezvous in relaxed
> > > AP mode.
> > >
> > > This patch use the new service to let VariableSmm driver work
> > > normally in relaxed AP mode.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > > Cc: Ni Ray <ray.ni@intel.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > >
> > > Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> > > ---
> > >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > | 10 +++++++-
> > >  MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > | 27 ++++++++++++++++++++
> > >  MdeModulePkg/MdeModulePkg.dec
> > > |  5 +++-
> > >  MdeModulePkg/MdeModulePkg.dsc
> > > |  4 ++-
> > >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > |  3 ++-
> > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> > > |  3 ++-
> > >  6 files changed, 47 insertions(+), 5 deletions(-)
> > >
> > > diff --git
> > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > index 517cae7b00f8..52a9b0e6b202 100644
> > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > @@ -14,7 +14,7 @@
> > >    VariableServiceSetVariable(), VariableServiceQueryVariableInfo(),
> > > ReclaimForOS(),
> > >
> > >    SmmVariableGetStatistics() should also do validation based on its
> > > own knowledge.
> > >
> > >
> > >
> > > -Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > > reserved.<BR>
> > >
> > > +Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > > +reserved.<BR>
> > >
> > >  Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> > >
> > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >
> > >
> > > @@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >
> > >  #include <Library/MmServicesTableLib.h>
> > >
> > >  #include <Library/VariablePolicyLib.h>
> > >
> > > +#include <Library/SmmCpuRendezvousLib.h>
> > >
> > >
> > >
> > >  #include <Guid/SmmVariableCommon.h>
> > >
> > >  #include "Variable.h"
> > >
> > > @@ -656,6 +657,13 @@ SmmVariableHandler (
> > >          goto EXIT;
> > >
> > >        }
> > >
> > >
> > >
> > > +      if ((SmmVariableHeader->Attributes &
> > > EFI_VARIABLE_NON_VOLATILE) != 0) {
> > >
> > > +        if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> > >
> > > +          DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all
> > > + AP
> > > check in SMM!\n"));
> > >
> > > +          goto EXIT;
> > >
> > > +        }
> > >
> > > +      }
> > >
> > > +
> > >
> > >        Status = VariableServiceSetVariable (
> > >
> > >                   SmmVariableHeader->Name,
> > >
> > >                   &SmmVariableHeader->Guid,
> > >
> > > diff --git a/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > new file mode 100644
> > > index 000000000000..82e459e9106e
> > > --- /dev/null
> > > +++ b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > @@ -0,0 +1,27 @@
> > > +/** @file
> > >
> > > +  SMM CPU Rendezvous library header file.
> > >
> > > +
> > >
> > > +  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> > >
> > > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > +
> > >
> > > +**/
> > >
> > > +
> > >
> > > +#ifndef SMM_CPU_RENDEZVOUS_H_
> > >
> > > +#define SMM_CPU_RENDEZVOUS_H_
> > >
> > > +
> > >
> > > +/**
> > >
> > > +  This routine wait for all AP processors to arrive in SMM.
> > >
> > > +
> > >
> > > +  @param[in]  BlockingMode  Blocking mode or non-blocking mode.
> > >
> > > +
> > >
> > > +  @retval EFI_SUCCESS       All processors checked in to SMM.
> > >
> > > +  @retval EFI_TIMEOUT       Wait for all APs until timeout.
> > >
> > > +
> > >
> > > +**/
> > >
> > > +EFI_STATUS
> > >
> > > +EFIAPI
> > >
> > > +SmmWaitForAllProcessor (
> > >
> > > +  IN  BOOLEAN  BlockingMode
> > >
> > > +  );
> > >
> > > +
> > >
> > > +#endif
> > >
> > > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > > b/MdeModulePkg/MdeModulePkg.dec index
> > 463e889e9a68..06ada41b7344
> > > 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > @@ -4,7 +4,7 @@
> > >  # and libraries instances, which are used for those modules.
> > >
> > >  #
> > >
> > >  # Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
> > >
> > > -# Copyright (c) 2007 - 2021, Intel Corporation. All rights
> > > reserved.<BR>
> > >
> > > +# Copyright (c) 2007 - 2022, Intel Corporation. All rights
> > > +reserved.<BR>
> > >
> > >  # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> > >
> > >  # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development
> > > LP<BR>
> > >
> > >  # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> > >
> > > @@ -154,6 +154,9 @@
> > >    #
> > >
> > >    VariablePolicyHelperLib|Include/Library/VariablePolicyHelperLib.h
> > >
> > >
> > >
> > > +  ## @libraryclass  Provides function for SMM CPU Rendezvous Library.
> > >
> > > +  SmmCpuRendezvousLib|Include/Library/SmmCpuRendezvousLib.h
> > >
> > > +
> > >
> > >  [Guids]
> > >
> > >    ## MdeModule package token space guid
> > >
> > >    # Include/Guid/MdeModulePkgTokenSpace.h
> > >
> > > diff --git a/MdeModulePkg/MdeModulePkg.dsc
> > > b/MdeModulePkg/MdeModulePkg.dsc index
> > b1d83461865e..a15dd5d7b23d
> > > 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dsc
> > > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > > @@ -2,7 +2,7 @@
> > >  # EFI/PI Reference Module Package for All Architectures
> > >
> > >  #
> > >
> > >  # (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
> > >
> > > -# Copyright (c) 2007 - 2021, Intel Corporation. All rights
> > > reserved.<BR>
> > >
> > > +# Copyright (c) 2007 - 2022, Intel Corporation. All rights
> > > +reserved.<BR>
> > >
> > >  # Copyright (c) Microsoft Corporation.
> > >
> > >  #
> > >
> > >  #    SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > @@ -152,6 +152,7 @@
> > >
> > >
> > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTa
> > > bleLib.inf
> > >
> > >
> > >
> > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.i
> > > nf
> > >
> > >    SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf
> > >
> > > +
> > >
> > SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> > > uRendezvousLib.inf
> > >
> > >
> > >
> > >  [LibraryClasses.common.UEFI_DRIVER]
> > >
> > >    HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> > >
> > > @@ -172,6 +173,7 @@
> > >
> > >
> > MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stand
> > > aloneMmServicesTableLib.inf
> > >
> > >
> > >
> > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxStandalon
> > > eMmLib.inf
> > >
> > >
> > >
> > MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMm
> > > MemLib.inf
> > >
> > > +
> > >
> > SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> > > uRendezvousLib.inf
> > >
> > >
> > >
> > >  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > >
> > >    ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > >
> > > diff --git
> > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > index eaa97a01c6e5..0bebd92b1626 100644
> > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > @@ -18,7 +18,7 @@
> > >  #  may not be modified without authorization. If platform fails to
> > > protect these resources,
> > >
> > >  #  the authentication service provided in this driver will be
> > > broken, and the behavior is undefined.
> > >
> > >  #
> > >
> > > -# Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > > reserved.<BR>
> > >
> > > +# Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > > +reserved.<BR>
> > >
> > >  # Copyright (c) Microsoft Corporation.
> > >
> > >  # SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  #
> > >
> > > @@ -82,6 +82,7 @@
> > >    UefiBootServicesTableLib
> > >
> > >    VariablePolicyLib
> > >
> > >    VariablePolicyHelperLib
> > >
> > > +  SmmCpuRendezvousLib
> > >
> > >
> > >
> > >  [Protocols]
> > >
> > >    gEfiSmmFirmwareVolumeBlockProtocolGuid        ## CONSUMES
> > >
> > > diff --git
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > f
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > f
> > > index d8c4f77e7f1f..595baaf70164 100644
> > > ---
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > f
> > > +++
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > f
> > > @@ -18,7 +18,7 @@
> > >  #  may not be modified without authorization. If platform fails to
> > > protect these resources,
> > >
> > >  #  the authentication service provided in this driver will be
> > > broken, and the behavior is undefined.
> > >
> > >  #
> > >
> > > -# Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > > reserved.<BR>
> > >
> > > +# Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > > +reserved.<BR>
> > >
> > >  # Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> > >
> > >  # Copyright (c) Microsoft Corporation.
> > >
> > >  # SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > @@ -78,6 +78,7 @@
> > >    VarCheckLib
> > >
> > >    VariablePolicyLib
> > >
> > >    VariablePolicyHelperLib
> > >
> > > +  SmmCpuRendezvousLib
> > >
> > >
> > >
> > >  [Protocols]
> > >
> > >    gEfiSmmFirmwareVolumeBlockProtocolGuid        ## CONSUMES
> > >
> > > --
> > > 2.26.2.windows.1
> > >
> > >
> > >
> > > 
> > >
> >
> >
> 
>