From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web08.25829.1650166065094893983 for ; Sat, 16 Apr 2022 20:27:46 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([101.224.116.119]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Sun, 17 Apr 2022 11:27:41 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 101.224.116.119 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: , , "'Michael Kubacki'" , "'Ard Biesheuvel'" , "'Leif Lindholm'" , "'Sean Brogan'" Cc: "'Wang, Jian J'" , "'Fu, Siyuan'" , "'Ni, Ray'" , "'Kinney, Michael D'" , "'Yao, Jiewen'" References: <20220411070631.1266-1-zhihao.li@intel.com> <011201d84e08$9e123d50$da36b7f0$@byosoft.com.cn> <001601d84ed6$0f46dda0$2dd498e0$@byosoft.com.cn> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIIHYxIDEvMV0gTWRlTW9kdWxlUGtnOiBVc2UgU21tV2FpdEZvckFsbFByb2Nlc3NvcigpIGluIFZhcmlhYmxlU21tIGRyaXZlci4=?= Date: Sun, 17 Apr 2022 11:27:42 +0800 Message-ID: <000201d8520b$190f7170$4b2e5450$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQFtCTG1UWK9uDQ6I7Bl2eh6XmYNfQHtGwlnASmenxABcFvVOQHgvN8trZbEfoA= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Zhihao: I see three platforms in edk2-platforms to consume VariableStandaloneMm m= odule. So, I think this change will impact them. Can you confirm this chang= e with those platform owners?=20 Platform\ARM\SgiPkg\PlatformStandaloneMm.fdf Platform\Socionext\DeveloperBox\DeveloperBoxMm.fdf Platform\StandaloneMm\PlatformStandaloneMmPkg\PlatformStandaloneMmRpmb.fdf Thanks Liming > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io = =E4=BB=A3=E8=A1=A8 Li, Zhihao > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2022=E5=B9=B44=E6=9C=8815=E6=97=A5 = 17:12 > =E6=94=B6=E4=BB=B6=E4=BA=BA: Gao, Liming ; deve= l@edk2.groups.io; > 'Michael Kubacki' ; 'Ard Biesheuvel' > ; 'Leif Lindholm' ; 'Sean > Brogan' > =E6=8A=84=E9=80=81: Wang, Jian J ; Fu, Siyuan > ; Ni, Ray ; Kinney, Michael D > ; Yao, Jiewen > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use > SmmWaitForAllProcessor() in VariableSmm driver. >=20 > 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. >=20 > 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? >=20 > 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? >=20 > > -----Original Message----- > > From: gaoliming > > Sent: Wednesday, April 13, 2022 9:30 AM > > To: Li, Zhihao ; devel@edk2.groups.io; 'Michael > Kubacki' > > ; 'Ard Biesheuvel' > > ; 'Leif Lindholm' ; 'Sean > > Brogan' > > Cc: Wang, Jian J ; Fu, Siyuan ; > Ni, > > Ray ; Kinney, Michael D ; > Yao, > > Jiewen > > Subject: =E5=9B=9E=E5=A4=8D: [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 > > > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > > =E5=8F=91=E4=BB=B6=E4=BA=BA: Li, Zhihao > > > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2022=E5=B9=B44=E6=9C=8813=E6=97= =A5 2:14 > > > =E6=94=B6=E4=BB=B6=E4=BA=BA: Gao, Liming ; > devel@edk2.groups.io > > > =E6=8A=84=E9=80=81: Wang, Jian J ; Fu, Siyuan > > > ; Ni, Ray ; Kinney, Michael D > > > > > > =E4=B8=BB=E9=A2=98: 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=E2=80=99= 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 > > > Sent: Tuesday, April 12, 2022 9:00 AM > > > To: devel@edk2.groups.io; Li, Zhihao > > > Cc: Wang, Jian J ; Fu, Siyuan > > > ; Ni, Ray ; Kinney, Michael D > > > > > > Subject: =E5=9B=9E=E5=A4=8D: [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 > > > > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > > > =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io =E4=BB=A3=E8=A1=A8 Li, > Zhihao > > > > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2022=E5=B9=B44=E6=9C=8811=E6= =97=A5 15:07 > > > > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io > > > > =E6=8A=84=E9=80=81: Jian J Wang ; Liming Gao > > > > ; Siyuan Fu ; Ni Ray > > > > ; Michael D Kinney > > > > =E4=B8=BB=E9=A2=98: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use > > > > SmmWaitForAllProcessor() in VariableSmm driver. > > > > > > > > REF=EF=BC=9A https://bugzilla.tianocore.org/show_bug.cgi?id=3D3854 > > > > > > > > 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 > > > > Cc: Liming Gao > > > > Cc: Siyuan Fu > > > > Cc: Ni Ray > > > > Cc: Michael D Kinney > > > > > > > > Signed-off-by: Zhihao Li > > > > --- > > > > 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 it= s > > > > own knowledge. > > > > > > > > > > > > > > > > -Copyright (c) 2010 - 2019, Intel Corporation. All rights > > > > reserved.
> > > > > > > > +Copyright (c) 2010 - 2022, Intel Corporation. All rights > > > > +reserved.
> > > > > > > > Copyright (c) 2018, Linaro, Ltd. All rights reserved.
> > > > > > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > > > > > > > > > > > @@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > > > > > > > #include > > > > > > > > #include > > > > > > > > +#include > > > > > > > > > > > > > > > > #include > > > > > > > > #include "Variable.h" > > > > > > > > @@ -656,6 +657,13 @@ SmmVariableHandler ( > > > > goto EXIT; > > > > > > > > } > > > > > > > > > > > > > > > > + if ((SmmVariableHeader->Attributes & > > > > EFI_VARIABLE_NON_VOLATILE) !=3D 0) { > > > > > > > > + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) { > > > > > > > > + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all > > > > + AP > > > > check in SMM!\n")); > > > > > > > > + goto EXIT; > > > > > > > > + } > > > > > > > > + } > > > > > > > > + > > > > > > > > Status =3D 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.
> > > > > > > > + 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.
> > > > > > > > +# Copyright (c) 2007 - 2022, Intel Corporation. All rights > > > > +reserved.
> > > > > > > > # Copyright (c) 2016, Linaro Ltd. All rights reserved.
> > > > > > > > # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development > > > > LP
> > > > > > > > # Copyright (c) 2017, AMD Incorporated. All rights reserved.
> > > > > > > > @@ -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.
> > > > > > > > -# Copyright (c) 2007 - 2021, Intel Corporation. All rights > > > > reserved.
> > > > > > > > +# Copyright (c) 2007 - 2022, Intel Corporation. All rights > > > > +reserved.
> > > > > > > > # 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.
> > > > > > > > +# Copyright (c) 2010 - 2022, Intel Corporation. All rights > > > > +reserved.
> > > > > > > > # 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.
> > > > > > > > +# Copyright (c) 2010 - 2022, Intel Corporation. All rights > > > > +reserved.
> > > > > > > > # Copyright (c) 2018, Linaro, Ltd. All rights reserved.
> > > > > > > > # 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >=20 >=20 >=20 >=20 >=20