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

["gaoliming" <gaoliming@byosoft.com.cn>]

Zhihao:
  I see three platforms in edk2-platforms to consume VariableStandaloneMm module. So, I think this change will impact them. Can you confirm this change with those platform owners? 

Platform\ARM\SgiPkg\PlatformStandaloneMm.fdf
Platform\Socionext\DeveloperBox\DeveloperBoxMm.fdf
Platform\StandaloneMm\PlatformStandaloneMmPkg\PlatformStandaloneMmRpmb.fdf

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Li, Zhihao
> 发送时间: 2022年4月15日 17:12
> 收件人: Gao, Liming <gaoliming@byosoft.com.cn>; 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>
> 抄送: 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>
> 主题: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> SmmWaitForAllProcessor() in VariableSmm driver.
> 
> 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
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> >
> >
> 
> 
> 
> 
>