public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "gaoliming" <gaoliming@byosoft.com.cn>
To: "'Masahisa Kojima'" <masahisa.kojima@linaro.org>,
	<devel@edk2.groups.io>, <michael.d.kinney@intel.com>
Cc: "'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>
Subject: 回复: [PATCH 1/1] MdeModulePkg/VarCheckPolicyLib: implement standalone MM version
Date: Thu, 17 Dec 2020 09:14:41 +0800	[thread overview]
Message-ID: <002001d6d411$ff6a2fd0$fe3e8f70$@byosoft.com.cn> (raw)
In-Reply-To: <20201216141919.23262-2-masahisa.kojima@linaro.org>

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




  reply	other threads:[~2020-12-17  1:14 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-16 14:19 [PATCH 0/1] MdeModulePkg/VarCheckPolicyLib: implement standalone MM version Masahisa Kojima
2020-12-16 14:19 ` [PATCH 1/1] " Masahisa Kojima
2020-12-17  1:14   ` gaoliming [this message]
     [not found]   ` <16515BFEBC173A6F.9537@groups.io>
2020-12-21  1:27     ` 回复: [edk2-devel] 回复: " gaoliming
2020-12-21  6:09       ` Masahisa Kojima

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='002001d6d411$ff6a2fd0$fe3e8f70$@byosoft.com.cn' \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox