Re: [PATCH v3 2/7] MdeModulePkg: VariableSmmRuntimeDxe: Added request unblock memory interface

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: Kun Qin <kun.q@outlook.com>
> Sent: Saturday, February 27, 2021 6:52 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Liming Gao <gaoliming@byosoft.com.cn>
> Subject: [PATCH v3 2/7] MdeModulePkg: VariableSmmRuntimeDxe: Added
> request unblock memory interface
> 
> This changes added usage of MmUnblockMemoryLib to explicitly request
> runtime cache regions(and its indicators) to be accessible from MM
> environment when PcdEnableVariableRuntimeCache is enabled. It will bring
> in compatibility with architectures that supports full memory blockage inside
> MM.
> 
> 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>
> 
> Signed-off-by: Kun Qin <kun.q@outlook.com>
> ---
> 
> Notes:
>     v3:
>     - Removed Dxe prefix to match interface change. [Jiewen]
> 
>     v2:
>     - Newly added in v2.
> 
> 
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
> c   | 42 ++++++++++++++++++++
>  MdeModulePkg/MdeModulePkg.dsc                                        |  1 +
> 
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.i
> nf |  1 +
>  3 files changed, 44 insertions(+)
> 
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> index c47e614d81f4..a166d284dfe4 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> @@ -35,6 +35,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #include
> <Library/DebugLib.h>  #include <Library/UefiLib.h>  #include
> <Library/BaseLib.h>
> +#include <Library/MmUnblockMemoryLib.h>
> 
>  #include <Guid/EventGroup.h>
>  #include <Guid/SmmVariableCommon.h>
> @@ -165,6 +166,7 @@ InitVariableCache (
>    )
>  {
>    VARIABLE_STORE_HEADER   *VariableCacheStorePtr;
> +  EFI_STATUS              Status;
> 
>    if (TotalVariableCacheSize == NULL) {
>      return EFI_INVALID_PARAMETER;
> @@ -186,6 +188,18 @@ InitVariableCache (
>    if (*VariableCacheBuffer == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> +
> +  //
> +  // Request to unblock the newly allocated cache region to be
> + accessible from inside MM  //  Status = MmUnblockMemoryRequest (
> +            (EFI_PHYSICAL_ADDRESS) (UINTN) *VariableCacheBuffer,
> +            EFI_SIZE_TO_PAGES (*TotalVariableCacheSize)
> +            );
> +  if (Status != EFI_UNSUPPORTED && EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
>    VariableCacheStorePtr = *VariableCacheBuffer;
>    SetMem32 ((VOID *) VariableCacheStorePtr, *TotalVariableCacheSize,
> (UINT32) 0xFFFFFFFF);
> 
> @@ -1536,6 +1550,34 @@ SendRuntimeVariableCacheContextToSmm (
>    SmmRuntimeVarCacheContext->ReadLock =
> &mVariableRuntimeCacheReadLock;
>    SmmRuntimeVarCacheContext->HobFlushComplete =
> &mHobFlushComplete;
> 
> +  //
> +  // Request to unblock this region to be accessible from inside MM
> + environment  // These fields "should" be all on the same page, but just to
> be on the safe side...
> +  //
> +  Status = MmUnblockMemoryRequest (
> +            (EFI_PHYSICAL_ADDRESS) ALIGN_VALUE ((UINTN)
> SmmRuntimeVarCacheContext->PendingUpdate - EFI_PAGE_SIZE + 1,
> EFI_PAGE_SIZE),
> +            EFI_SIZE_TO_PAGES (sizeof(mVariableRuntimeCachePendingUpdate))
> +            );
> +  if (Status != EFI_UNSUPPORTED && EFI_ERROR (Status)) {
> +    goto Done;
> +  }
> +
> +  Status = MmUnblockMemoryRequest (
> +            (EFI_PHYSICAL_ADDRESS) ALIGN_VALUE ((UINTN)
> SmmRuntimeVarCacheContext->ReadLock - EFI_PAGE_SIZE + 1,
> EFI_PAGE_SIZE),
> +            EFI_SIZE_TO_PAGES (sizeof(mVariableRuntimeCacheReadLock))
> +            );
> +  if (Status != EFI_UNSUPPORTED && EFI_ERROR (Status)) {
> +    goto Done;
> +  }
> +
> +  Status = MmUnblockMemoryRequest (
> +            (EFI_PHYSICAL_ADDRESS) ALIGN_VALUE ((UINTN)
> SmmRuntimeVarCacheContext->HobFlushComplete - EFI_PAGE_SIZE + 1,
> EFI_PAGE_SIZE),
> +            EFI_SIZE_TO_PAGES (sizeof(mHobFlushComplete))
> +            );
> +  if (Status != EFI_UNSUPPORTED && EFI_ERROR (Status)) {
> +    goto Done;
> +  }
> +


Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


>    //
>    // Send data to SMM.
>    //
> diff --git a/MdeModulePkg/MdeModulePkg.dsc
> b/MdeModulePkg/MdeModulePkg.dsc index 7ca4a1bb3080..9272da89a998
> 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -100,6 +100,7 @@ [LibraryClasses]
>    SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> 
> DisplayUpdateProgressLib|MdeModulePkg/Library/DisplayUpdateProgressLi
> bGraphics/DisplayUpdateProgressLibGraphics.inf
> 
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Va
> riablePolicyHelperLib.inf
> +
> +
> MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblo
> ckMemoryLi
> + bNull.inf
> 
>  [LibraryClasses.EBC.PEIM]
>    IoLib|MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.inf
> index b6dbc839e023..a0d8b2267e92 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.inf
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.i
> +++ nf
> @@ -60,6 +60,7 @@ [LibraryClasses]
>    TpmMeasurementLib
>    SafeIntLib
>    PcdLib
> +  MmUnblockMemoryLib
> 
>  [Protocols]
>    gEfiVariableWriteArchProtocolGuid             ## PRODUCES
> --
> 2.30.0.windows.1