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] -=-=-=-=-=-=-=-=-=-=-=-
