On 03/01/21 20:03, Kun Qin wrote:
> Hi Laszlo,
> 
> The library is intended to allow MM modules to access requested areas that 
> are outside of MMRAM. The idea behind this library is to create an MM model 
> that will block access to all non-MMRAM regions except the requested areas 
> for isolation/protection purpose. To resolve your confusion, the agents that 
> are responsible for unblocking memory are the ones that sets up these regions 
> that need to be accessed by MM modules.
> 
> For example:
> 
>   1.  To enable runtime cache feature for variable service, Variable MM 
> module needs to access the allocated runtime buffer, which is set up in 
> VariableSmmRuntimeDxe;
>   2.  For TPM ACPI table to communicate to physical presence handler, the 
> corresponding NVS regions has to be accessible from inside MM, which is set 
> up when assigning NVS region in DXE environment;
> 
> So the library is not necessarily restricted to DXE_RUNTIME drivers, but 
> could be applicable to DXE drivers and even PEI modules as well.

Very nice, so please include this in the commit message.

> 
> Thanks for the suggestion, I will replace the “EFI_” error code with 
> “RETURN_” error codes and add more statements in commit message to make the 
> purpose of this library more explanatory.

Thanks!
Laszlo

> 
> Regards,
> Kun
> 
> From: Laszlo Ersek<mailto:ler...@redhat.com>
> Sent: Monday, March 1, 2021 08:40
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; 
> ku...@outlook.com<mailto:ku...@outlook.com>
> Cc: Michael D Kinney<mailto:michael.d.kin...@intel.com>; Liming 
> Gao<mailto:gaolim...@byosoft.com.cn>; Zhiguang 
> Liu<mailto:zhiguang....@intel.com>; Hao A Wu<mailto:hao.a...@intel.com>; 
> Jiewen Yao<mailto:jiewen....@intel.com>
> Subject: Re: [edk2-devel] [PATCH v3 1/7] MdePkg: MmUnblockMemoryLib: Added 
> definition and null instance
> 
> On 02/26/21 23:51, Kun Qin wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3168
>>
>> This interface definition provides an abstraction layer for applicable
>> drivers to request certain memory blocks to be mapped/unblocked for
>> accessibility inside MM environment.
>>
>> Cc: Michael D Kinney <michael.d.kin...@intel.com>
>> Cc: Liming Gao <gaolim...@byosoft.com.cn>
>> Cc: Zhiguang Liu <zhiguang....@intel.com>
>> Cc: Hao A Wu <hao.a...@intel.com>
>> Cc: Jiewen Yao <jiewen....@intel.com>
>>
>> Signed-off-by: Kun Qin <ku...@outlook.com>
>> ---
>>
>> Notes:
>>     v3:
>>     - Move interface to MdePkg [Hao, Liming, Jiewen]
>>     - Remove Dxe prefix [Jiewen]
>>
>>     v2:
>>     - Resend with practical usage. No change [Hao]
>>
>>  MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c   | 40 
>> ++++++++++++++++++++
>>  MdePkg/Include/Library/MmUnblockMemoryLib.h                  | 40 
>> ++++++++++++++++++++
>>  MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf | 29 
>> ++++++++++++++
>>  MdePkg/MdePkg.dec                                            |  5 +++
>>  MdePkg/MdePkg.dsc                                            |  1 +
>>  5 files changed, 115 insertions(+)
>>
>> diff --git a/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c 
>> b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c
>> new file mode 100644
>> index 000000000000..ed9a45587b64
>> --- /dev/null
>> +++ b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c
>> @@ -0,0 +1,40 @@
>> +/** @file
>> +  Null instance of MM Unblock Page Library.
>> +
>> +  This library provides an abstraction layer of requesting certain page 
>> access to be unblocked
>> +  by MM environment if applicable.
>> +
>> +  Copyright (c) Microsoft Corporation.
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#include <Uefi.h>
>> +
>> +/**
>> +  This API provides a way to unblock certain data pages to be accessible 
>> inside MM environment.
>> +
>> +  @param  UnblockAddress          The address of buffer caller requests to 
>> unblock, the address
>> +                                  has to be page aligned.
>> +  @param  NumberOfPages           The number of pages requested to be 
>> unblocked from MM
>> +                                  environment.
>> +
>> +  @return EFI_SUCCESS             The request goes through successfully.
>> +  @return EFI_NOT_AVAILABLE_YET   The requested functionality is not 
>> produced yet.
>> +  @return EFI_UNSUPPORTED         The requested functionality is not 
>> supported on current platform.
>> +  @return EFI_SECURITY_VIOLATION  The requested address failed to pass 
>> security check for
>> +                                  unblocking.
>> +  @return EFI_INVALID_PARAMETER   Input address either NULL pointer or not 
>> page aligned.
>> +  @return EFI_ACCESS_DENIED       The request is rejected due to system has 
>> passed certain boot
>> +                                  phase.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +MmUnblockMemoryRequest (
>> +  IN EFI_PHYSICAL_ADDRESS   UnblockAddress,
>> +  IN UINT64                 NumberOfPages
>> +  )
>> +{
>> +  return EFI_UNSUPPORTED;
>> +}
>> diff --git a/MdePkg/Include/Library/MmUnblockMemoryLib.h 
>> b/MdePkg/Include/Library/MmUnblockMemoryLib.h
>> new file mode 100644
>> index 000000000000..adff8ddc8063
>> --- /dev/null
>> +++ b/MdePkg/Include/Library/MmUnblockMemoryLib.h
>> @@ -0,0 +1,40 @@
>> +/** @file
>> +  MM Unblock Memory Library Interface.
>> +
>> +  This library provides an abstraction layer of requesting certain page 
>> access to be unblocked
>> +  by MM environment if applicable.
>> +
>> +  Copyright (c) Microsoft Corporation.
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#ifndef MM_UNBLOCK_MEMORY_LIB_H_
>> +#define MM_UNBLOCK_MEMORY_LIB_H_
>> +
>> +/**
>> +  This API provides a way to unblock certain data pages to be accessible 
>> inside MM environment.
>> +
>> +  @param  UnblockAddress          The address of buffer caller requests to 
>> unblock, the address
>> +                                  has to be page aligned.
>> +  @param  NumberOfPages           The number of pages requested to be 
>> unblocked from MM
>> +                                  environment.
>> +
>> +  @return EFI_SUCCESS             The request goes through successfully.
>> +  @return EFI_NOT_AVAILABLE_YET   The requested functionality is not 
>> produced yet.
>> +  @return EFI_UNSUPPORTED         The requested functionality is not 
>> supported on current platform.
>> +  @return EFI_SECURITY_VIOLATION  The requested address failed to pass 
>> security check for
>> +                                  unblocking.
>> +  @return EFI_INVALID_PARAMETER   Input address either NULL pointer or not 
>> page aligned.
>> +  @return EFI_ACCESS_DENIED       The request is rejected due to system has 
>> passed certain boot
>> +                                  phase.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +MmUnblockMemoryRequest (
>> +  IN EFI_PHYSICAL_ADDRESS   UnblockAddress,
>> +  IN UINT64                 NumberOfPages
>> +);
>> +
>> +#endif // MM_UNBLOCK_MEMORY_LIB_H_
>> diff --git a/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf 
>> b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
>> new file mode 100644
>> index 000000000000..4c1f3d1c8e87
>> --- /dev/null
>> +++ b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
>> @@ -0,0 +1,29 @@
>> +## @file
>> +#  Null instance of MM Unblock Page Library.
>> +#
>> +#  This library provides an abstraction layer of requesting certain page 
>> access to be unblocked
>> +#  by MM environment if applicable.
>> +#
>> +#  Copyright (c) Microsoft Corporation.
>> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +#
>> +#
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION                    = 0x0001001B
>> +  BASE_NAME                      = MmUnblockMemoryLibNull
>> +  FILE_GUID                      = 9E890F68-5C95-4C31-95DD-59E6286F85EA
>> +  MODULE_TYPE                    = BASE
>> +  VERSION_STRING                 = 1.0
>> +  LIBRARY_CLASS                  = MmUnblockMemoryLib
> 
> (1) I think the applicability of this library class is not spelled out
> clearly enough. The commit message says "applicable drivers", and I have
> no idea what that means. The facts that MODULE_TYPE is BASE here, and
> LIBRARY_CLASS is not restricted to any particular module types, do not
> help either.
> 
> (2) Assuming "MODULE_TYPE = BASE" is correct, then the EFI_ prefixes in
> the library class (and Null instance) look wrong. BASE libraries should
> use RETURN_ prefixes, to my understanding. (I.e., replace EFI_STATUS,
> EFI_SUCCESS etc with RETURN_STATUS, RETURN_SUCESS ...)
> 
> So I would suggest either restricting this API to runtime DXE drivers,
> or using RETURN_... macros.
> 
> Either way, it should be clarified what agents are responsible for
> "unblocking" memory areas for MM. Do you have something like "runtime
> DXE drivers that submit MM Communicate requests" in mind?
> 
> Thanks
> Laszlo
> 
> 
>> +
>> +#
>> +#  VALID_ARCHITECTURES           = IA32 X64
>> +#
>> +
>> +[Sources]
>> +  MmUnblockMemoryLibNull.c
>> +
>> +[Packages]
>> +  MdePkg/MdePkg.dec
>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
>> index 3928db65d188..32a9e009c813 100644
>> --- a/MdePkg/MdePkg.dec
>> +++ b/MdePkg/MdePkg.dec
>> @@ -257,6 +257,11 @@ [LibraryClasses]
>>    #
>>    UnitTestHostBaseLib|Test/UnitTest/Include/Library/UnitTestHostBaseLib.h
>>
>> +  ##  @libraryclass  This library provides an interface for DXE drivers to 
>> request MM environment
>> +  #   to map/unblock a memory region for accessibility inside MM.
>> +  #
>> +  MmUnblockMemoryLib|Include/Library/MmUnblockMemoryLib.h
>> +
>>  [LibraryClasses.IA32, LibraryClasses.X64]
>>    ##  @libraryclass  Abstracts both S/W SMI generation and detection.
>>    ##
>> diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
>> index ce009086815f..79629e3f93ba 100644
>> --- a/MdePkg/MdePkg.dsc
>> +++ b/MdePkg/MdePkg.dsc
>> @@ -168,6 +168,7 @@ [Components.IA32, Components.X64]
>>    MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf
>>    MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.inf
>>    MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
>> +  MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
>>
>>  [Components.EBC]
>>    MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>>
> 
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72328): https://edk2.groups.io/g/devel/message/72328
Mute This Topic: https://groups.io/mt/80939990/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to