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.web10.33458.1608514042457246426 for ; Sun, 20 Dec 2020 17:27:23 -0800 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 ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Mon, 21 Dec 2020 09:27:16 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: , , "'Masahisa Kojima'" , Cc: "'Kun Qin'" , "'Jian J Wang'" , "'Hao A Wu'" , "'Ard Biesheuvel'" , "'Sami Mujawar'" , "'Jiewen Yao'" , "'Supreeth Venkatesh'" , "'Bret Barkelew'" References: <20201216141919.23262-1-masahisa.kojima@linaro.org> <20201216141919.23262-2-masahisa.kojima@linaro.org> <16515BFEBC173A6F.9537@groups.io> In-Reply-To: <16515BFEBC173A6F.9537@groups.io> Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0g5Zue5aSNOiBbUEFUQ0ggMS8xXSBNZGVNb2R1bGVQa2cvVmFyQ2hlY2tQb2xpY3lMaWI6IGltcGxlbWVudCBzdGFuZGFsb25lIE1NIHZlcnNpb24=?= Date: Mon, 21 Dec 2020 09:27:17 +0800 Message-ID: <012d01d6d738$6b5ac6e0$421054a0$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQIsTiDj8DCaOFji41OQNzs0OfaOJAJMIgpRAjTce0OpMcwwsA== Content-Type: text/plain; charset="gb2312" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Masahisa: One minor comment, new added VarCheckPolicyLibStandaloneMm.inf is requir= ed to be listed in MdeModulePkg.dsc for build test.=20 Thanks Liming > -----=D3=CA=BC=FE=D4=AD=BC=FE----- > =B7=A2=BC=FE=C8=CB: bounce+27952+69077+4905953+8761045@groups.io > =B4=FA=B1=ED gaoliming > =B7=A2=CB=CD=CA=B1=BC=E4: 2020=C4=EA12=D4=C217=C8=D5 9:15 > =CA=D5=BC=FE=C8=CB: 'Masahisa Kojima' ; > devel@edk2.groups.io; michael.d.kinney@intel.com > =B3=AD=CB=CD: 'Kun Qin' ; 'Jian J Wang' ; > 'Hao A Wu' ; 'Ard Biesheuvel' > ; 'Sami Mujawar' ; > 'Jiewen Yao' ; 'Supreeth Venkatesh' > ; 'Bret Barkelew' > > =D6=F7=CC=E2: [edk2-devel] =BB=D8=B8=B4: [PATCH 1/1] MdeModulePkg/VarChe= ckPolicyLib: > implement standalone MM version >=20 > Masahisa: > The patch is good. Reviewed-by: Liming Gao >=20 > Now, Mike proposes to create stable tag branch to include the critical bug > fix. I think this one is also the critical fix to be cherry-pick to the > stable tag branch. >=20 > Thanks > Liming > > -----=D3=CA=BC=FE=D4=AD=BC=FE----- > > =B7=A2=BC=FE=C8=CB: Masahisa Kojima > > =B7=A2=CB=CD=CA=B1=BC=E4: 2020=C4=EA12=D4=C216=C8=D5 22:19 > > =CA=D5=BC=FE=C8=CB: devel@edk2.groups.io > > =B3=AD=CB=CD: Kun Qin ; Masahisa Kojima > > ; Jian J Wang ; Hao > A > > Wu ; Liming Gao ; Ard > > Biesheuvel ; Sami Mujawar > > ; Jiewen Yao ; Supreeth > > Venkatesh ; Bret Barkelew > > > > =D6=F7=CC=E2: [PATCH 1/1] MdeModulePkg/VarCheckPolicyLib: implement > standalone > > MM version > > > > This commit adds the VarCheckPolicyLib that will be able to > > execute in the context of standalone MM. > > > > Signed-off-by: Masahisa Kojima > > Co-authored-by: Kun Qin > > Cc: Jian J Wang > > Cc: Hao A Wu > > Cc: Liming Gao > > Cc: Ard Biesheuvel > > Cc: Sami Mujawar > > Cc: Jiewen Yao > > Cc: Supreeth Venkatesh > > Cc: Bret Barkelew > > --- > > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > > | 5 +- > > MdeModulePkg/Library/VarCheckPolicyLib/{VarCheckPolicyLib.inf =3D> > > VarCheckPolicyLibStandaloneMm.inf} | 23 +++++---- > > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.h > > | 42 ++++++++++++++++ > > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c > > | 14 +++--- > > > > > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneMm > > .c | 50 ++++++++++++++++++++ > > > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibTraditional.c > > | 50 ++++++++++++++++++++ > > 6 files changed, 165 insertions(+), 19 deletions(-) > > > > diff --git a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > > index 077bcc8990ca..9af436d25f81 100644 > > --- a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > > +++ b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > > @@ -13,11 +13,13 @@ [Defines] > > MODULE_TYPE =3D DXE_RUNTIME_DRIVER > > VERSION_STRING =3D 1.0 > > LIBRARY_CLASS =3D NULL|DXE_RUNTIME_DRIVER > > DXE_SMM_DRIVER > > - CONSTRUCTOR =3D VarCheckPolicyLibConstructor > > + CONSTRUCTOR =3D > > VarCheckPolicyLibTraditionalConstructor > > > > > > [Sources] > > VarCheckPolicyLib.c > > + VarCheckPolicyLibTraditional.c > > + VarCheckPolicyLib.h > > > > > > [Packages] > > @@ -29,7 +31,6 @@ [LibraryClasses] > > BaseLib > > DebugLib > > BaseMemoryLib > > - DxeServicesLib > > MemoryAllocationLib > > VarCheckLib > > VariablePolicyLib > > diff --git a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > > > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneM > > m.inf > > similarity index 51% > > copy from MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > > copy to > > > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneMm > > .inf > > index 077bcc8990ca..ab427f189a3d 100644 > > --- a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > > +++ > > > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneM > > m.inf > > @@ -1,35 +1,41 @@ > > -## @file VarCheckPolicyLib.inf > > +## @file VarCheckPolicyLibStandaloneMm.inf > > # This is an instance of a VarCheck lib that leverages the business logic > > behind > > # the VariablePolicy code to make its decisions. > > # > > -# Copyright (c) Microsoft Corporation. > > +## > > +# Copyright (c) Microsoft Corporation. All rights reserved. > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > +# > > ## > > > > [Defines] > > INF_VERSION =3D 0x00010005 > > - BASE_NAME =3D VarCheckPolicyLib > > - FILE_GUID =3D > > 9C28A48F-C884-4B1F-8B95-DEF125448023 > > - MODULE_TYPE =3D DXE_RUNTIME_DRIVER > > + BASE_NAME =3D > > VarCheckPolicyLibStandaloneMm > > + FILE_GUID =3D > > 44B09E3D-5EDA-4673-ABCF-C8AE4560C8EC > > + MODULE_TYPE =3D MM_STANDALONE > > + PI_SPECIFICATION_VERSION =3D 0x00010032 > > VERSION_STRING =3D 1.0 > > - LIBRARY_CLASS =3D NULL|DXE_RUNTIME_DRIVER > > DXE_SMM_DRIVER > > - CONSTRUCTOR =3D VarCheckPolicyLibConstructor > > + LIBRARY_CLASS =3D NULL|MM_STANDALONE > > + CONSTRUCTOR =3D > > VarCheckPolicyLibStandaloneConstructor > > > > > > [Sources] > > VarCheckPolicyLib.c > > + VarCheckPolicyLibStandaloneMm.c > > + VarCheckPolicyLib.h > > > > > > [Packages] > > MdePkg/MdePkg.dec > > MdeModulePkg/MdeModulePkg.dec > > + StandaloneMmPkg/StandaloneMmPkg.dec > > > > > > [LibraryClasses] > > BaseLib > > DebugLib > > BaseMemoryLib > > - DxeServicesLib > > + MemLib > > MemoryAllocationLib > > VarCheckLib > > VariablePolicyLib > > @@ -37,6 +43,5 @@ [LibraryClasses] > > SafeIntLib > > MmServicesTableLib > > > > - > > [Guids] > > gVarCheckPolicyLibMmiHandlerGuid ## CONSUME ## Used to > > register for MM Communication events. > > diff --git a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.= h > > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.h > > new file mode 100644 > > index 000000000000..2226c8a19fec > > --- /dev/null > > +++ b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.h > > @@ -0,0 +1,42 @@ > > +/** @file -- VarCheckPolicyLib.h > > +This internal header file defines the common interface of constructor for > > +VarCheckPolicyLib. > > + > > +Copyright (c) Microsoft Corporation. All rights reserved. > > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#ifndef _VAR_CHECK_POLICY_LIB_H_ > > +#define _VAR_CHECK_POLICY_LIB_H_ > > + > > +/** > > + Common constructor function of VarCheckPolicyLib to register VarChe= ck > > handler > > + and SW MMI handlers. > > + > > + @retval EFI_SUCCESS The constructor executed correctly. > > + > > +**/ > > +EFI_STATUS > > +EFIAPI > > +VarCheckPolicyLibCommonConstructor ( > > + VOID > > + ); > > + > > +/** > > + This function is wrapper function to validate the buffer. > > + > > + @param Buffer The buffer start address to be checked. > > + @param Length The buffer length to be checked. > > + > > + @retval TRUE This buffer is valid per processor architecture and n= ot > > overlap with SMRAM/MMRAM. > > + @retval FALSE This buffer is not valid per processor architecture o= r > > overlap with SMRAM/MMRAM. > > +**/ > > +BOOLEAN > > +EFIAPI > > +VarCheckPolicyIsBufferOutsideValid ( > > + IN EFI_PHYSICAL_ADDRESS Buffer, > > + IN UINT64 Length > > + ); > > + > > +#endif // _VAR_CHECK_POLICY_LIB_H_ > > diff --git a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.= c > > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c > > index 257aa9591303..14e1904e96d3 100644 > > --- a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c > > +++ b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c > > @@ -12,7 +12,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #include > > #include > > #include > > -#include > > #include > > #include > > > > @@ -23,6 +22,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > > > #include > > > > +#include "VarCheckPolicyLib.h" > > + > > //=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D > > // As a VarCheck library, we're linked into the VariableServices > > // and may not be able to call them indirectly. To get around this, > > @@ -102,7 +103,8 @@ VarCheckPolicyLibMmiHandler ( > > // Make sure that the buffer does not overlap SMM. > > // This should be covered by the SmiManage infrastructure, but just to > be > > safe... > > InternalCommBufferSize =3D *CommBufferSize; > > - if (InternalCommBufferSize > > > VAR_CHECK_POLICY_MM_COMM_BUFFER_SIZE > > || !SmmIsBufferOutsideSmmValid((UINTN)CommBuffer, > > (UINT64)InternalCommBufferSize)) { > > + if (InternalCommBufferSize > > > VAR_CHECK_POLICY_MM_COMM_BUFFER_SIZE || > > + !VarCheckPolicyIsBufferOutsideValid((UINTN)CommBuffer, > > (UINT64)InternalCommBufferSize)) { > > DEBUG ((DEBUG_ERROR, "%a - Invalid CommBuffer supplied! > > 0x%016lX[0x%016lX]\n", __FUNCTION__, CommBuffer, > > InternalCommBufferSize)); > > return EFI_INVALID_PARAMETER; > > } > > @@ -305,17 +307,13 @@ VarCheckPolicyLibMmiHandler ( > > Constructor function of VarCheckPolicyLib to register VarCheck handler > > and > > SW MMI handlers. > > > > - @param[in] ImageHandle The firmware allocated handle for the EFI > > image. > > - @param[in] SystemTable A pointer to the EFI System Table. > > - > > @retval EFI_SUCCESS The constructor executed correctly. > > > > **/ > > EFI_STATUS > > EFIAPI > > -VarCheckPolicyLibConstructor ( > > - IN EFI_HANDLE ImageHandle, > > - IN EFI_SYSTEM_TABLE *SystemTable > > +VarCheckPolicyLibCommonConstructor ( > > + VOID > > ) > > { > > EFI_STATUS Status; > > diff --git > > > a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneM > > m.c > > > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneM > > m.c > > new file mode 100644 > > index 000000000000..b283ced9d4e3 > > --- /dev/null > > +++ > > > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneM > > m.c > > @@ -0,0 +1,50 @@ > > +/** @file -- VarCheckPolicyLibStandaloneMm.c > > +This is an instance of a VarCheck lib constructor for Standalone MM. > > + > > +Copyright (c) Microsoft Corporation. All rights reserved. > > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#include > > + > > +#include "VarCheckPolicyLib.h" > > + > > +/** > > + Standalone MM constructor function of VarCheckPolicyLib to invoke > > common > > + constructor routine. > > + > > + @param[in] ImageHandle The firmware allocated handle for the EFI > > image. > > + @param[in] SystemTable A pointer to the EFI System Table. > > + > > + @retval EFI_SUCCESS The constructor executed correctly. > > + > > +**/ > > +EFI_STATUS > > +EFIAPI > > +VarCheckPolicyLibStandaloneConstructor ( > > + IN EFI_HANDLE ImageHandle, > > + IN EFI_MM_SYSTEM_TABLE *SystemTable > > + ) > > +{ > > + return VarCheckPolicyLibCommonConstructor (); > > +} > > + > > +/** > > + This function is wrapper function to validate the buffer. > > + > > + @param Buffer The buffer start address to be checked. > > + @param Length The buffer length to be checked. > > + > > + @retval TRUE This buffer is valid per processor architectureand no= t > > overlap with MMRAM. > > + @retval FALSE This buffer is not valid per processor architecture o= r > > overlap with MMRAM. > > +**/ > > +BOOLEAN > > +EFIAPI > > +VarCheckPolicyIsBufferOutsideValid ( > > + IN EFI_PHYSICAL_ADDRESS Buffer, > > + IN UINT64 Length > > + ) > > +{ > > + return MmIsBufferOutsideMmValid (Buffer, Length); > > +} > > diff --git > > > a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibTraditional.c > > > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibTraditional.c > > new file mode 100644 > > index 000000000000..f404aaaa470c > > --- /dev/null > > +++ > > > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibTraditional.c > > @@ -0,0 +1,50 @@ > > +/** @file -- VarCheckPolicyLibTraditional.c > > +This is an instance of a VarCheck lib constructor for traditional SMM= . > > + > > +Copyright (c) Microsoft Corporation. All rights reserved. > > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#include > > + > > +#include "VarCheckPolicyLib.h" > > + > > +/** > > + Traditional constructor function of VarCheckPolicyLib to invoke common > > + constructor routine. > > + > > + @param[in] ImageHandle The firmware allocated handle for the EFI > > image. > > + @param[in] SystemTable A pointer to the EFI System Table. > > + > > + @retval EFI_SUCCESS The constructor executed correctly. > > + > > +**/ > > +EFI_STATUS > > +EFIAPI > > +VarCheckPolicyLibTraditionalConstructor ( > > + IN EFI_HANDLE ImageHandle, > > + IN EFI_SYSTEM_TABLE *SystemTable > > + ) > > +{ > > + return VarCheckPolicyLibCommonConstructor (); > > +} > > + > > +/** > > + This function is wrapper function to validate the buffer. > > + > > + @param Buffer The buffer start address to be checked. > > + @param Length The buffer length to be checked. > > + > > + @retval TRUE This buffer is valid per processor architecture and n= ot > > overlap with SMRAM. > > + @retval FALSE This buffer is not valid per processor architecture o= r > > overlap with SMRAM. > > +**/ > > +BOOLEAN > > +EFIAPI > > +VarCheckPolicyIsBufferOutsideValid ( > > + IN EFI_PHYSICAL_ADDRESS Buffer, > > + IN UINT64 Length > > + ) > > +{ > > + return SmmIsBufferOutsideSmmValid (Buffer, Length); > > +} > > -- > > 2.17.1 >=20 >=20 >=20 >=20 >=20 >=20 >=20