回复: [edk2-devel] 回复: [PATCH 1/1] MdeModulePkg/VarCheckPolicyLib: implement standalone MM version

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

Masahisa:
  One minor comment, new added VarCheckPolicyLibStandaloneMm.inf is required
to be listed in MdeModulePkg.dsc for build test. 

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+69077+4905953+8761045@groups.io
> <bounce+27952+69077+4905953+8761045@groups.io> 代表 gaoliming
> 发送时间: 2020年12月17日 9:15
> 收件人: 'Masahisa Kojima' <masahisa.kojima@linaro.org>;
> devel@edk2.groups.io; michael.d.kinney@intel.com
> 抄送: 'Kun Qin' <kun.q@outlook.com>; 'Jian J Wang'
<jian.j.wang@intel.com>;
> 'Hao A Wu' <hao.a.wu@intel.com>; 'Ard Biesheuvel'
> <ard.biesheuvel@arm.com>; 'Sami Mujawar' <sami.mujawar@arm.com>;
> 'Jiewen Yao' <jiewen.yao@intel.com>; 'Supreeth Venkatesh'
> <supreeth.venkatesh@arm.com>; 'Bret Barkelew'
> <Bret.Barkelew@microsoft.com>
> 主题: [edk2-devel] 回复: [PATCH 1/1] MdeModulePkg/VarCheckPolicyLib:
> implement standalone MM version
> 
> Masahisa:
>   The patch is good. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> 
>   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.
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Masahisa Kojima <masahisa.kojima@linaro.org>
> > 发送时间: 2020年12月16日 22:19
> > 收件人: devel@edk2.groups.io
> > 抄送: Kun Qin <kun.q@outlook.com>; Masahisa Kojima
> > <masahisa.kojima@linaro.org>; Jian J Wang <jian.j.wang@intel.com>; Hao
> A
> > Wu <hao.a.wu@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Ard
> > Biesheuvel <ard.biesheuvel@arm.com>; Sami Mujawar
> > <sami.mujawar@arm.com>; Jiewen Yao <jiewen.yao@intel.com>; Supreeth
> > Venkatesh <supreeth.venkatesh@arm.com>; Bret Barkelew
> > <Bret.Barkelew@microsoft.com>
> > 主题: [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 <masahisa.kojima@linaro.org>
> > Co-authored-by: Kun Qin <kun.q@outlook.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > Cc: Sami Mujawar <sami.mujawar@arm.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > ---
> >  MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf
> > |  5 +-
> >  MdeModulePkg/Library/VarCheckPolicyLib/{VarCheckPolicyLib.inf =>
> > 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                    = DXE_RUNTIME_DRIVER
> >    VERSION_STRING                 = 1.0
> >    LIBRARY_CLASS                  = NULL|DXE_RUNTIME_DRIVER
> > DXE_SMM_DRIVER
> > -  CONSTRUCTOR                    = VarCheckPolicyLibConstructor
> > +  CONSTRUCTOR                    =
> > 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                    = 0x00010005
> > -  BASE_NAME                      = VarCheckPolicyLib
> > -  FILE_GUID                      =
> > 9C28A48F-C884-4B1F-8B95-DEF125448023
> > -  MODULE_TYPE                    = DXE_RUNTIME_DRIVER
> > +  BASE_NAME                      =
> > VarCheckPolicyLibStandaloneMm
> > +  FILE_GUID                      =
> > 44B09E3D-5EDA-4673-ABCF-C8AE4560C8EC
> > +  MODULE_TYPE                    = MM_STANDALONE
> > +  PI_SPECIFICATION_VERSION       = 0x00010032
> >    VERSION_STRING                 = 1.0
> > -  LIBRARY_CLASS                  = NULL|DXE_RUNTIME_DRIVER
> > DXE_SMM_DRIVER
> > -  CONSTRUCTOR                    = VarCheckPolicyLibConstructor
> > +  LIBRARY_CLASS                  = NULL|MM_STANDALONE
> > +  CONSTRUCTOR                    =
> > 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 VarCheck
> > 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 not
> > overlap with SMRAM/MMRAM.
> > +  @retval FALSE This buffer is not valid per processor architecture or
> > 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 <Library/DebugLib.h>
> >  #include <Library/SafeIntLib.h>
> >  #include <Library/MmServicesTableLib.h>
> > -#include <Library/SmmMemLib.h>
> >  #include <Library/BaseMemoryLib.h>
> >  #include <Library/MemoryAllocationLib.h>
> >
> > @@ -23,6 +22,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  #include <Guid/VarCheckPolicyMmi.h>
> >
> > +#include "VarCheckPolicyLib.h"
> > +
> >  //================================================
> >  // 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 = *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 <Library/StandaloneMmMemLib.h>
> > +
> > +#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 not
> > overlap with MMRAM.
> > +  @retval FALSE This buffer is not valid per processor architecture or
> > 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 <Library/SmmMemLib.h>
> > +
> > +#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 not
> > overlap with SMRAM.
> > +  @retval FALSE This buffer is not valid per processor architecture or
> > overlap with SMRAM.
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +VarCheckPolicyIsBufferOutsideValid (
> > +  IN EFI_PHYSICAL_ADDRESS  Buffer,
> > +  IN UINT64                Length
> > +  )
> > +{
> > +  return SmmIsBufferOutsideSmmValid (Buffer, Length);
> > +}
> > --
> > 2.17.1
> 
> 
> 
> 
> 
> 
>