From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM04-MW2-obe.outbound.protection.outlook.com (NAM04-MW2-obe.outbound.protection.outlook.com [40.92.46.39]) by mx.groups.io with SMTP id smtpd.web11.1135.1614799894492758963 for ; Wed, 03 Mar 2021 11:31:34 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@outlook.com header.s=selector1 header.b=GbbiSEK0; spf=pass (domain: outlook.com, ip: 40.92.46.39, mailfrom: kun.q@outlook.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=YfLL+Y844LwVz3xoJwcXTR5d++CO5W6uFDKTR3Hbn3xIEMt+yRUIkBn5N8wYV07FuZ+DA5+OocizjEwELgUmI1N0f5meVMqKijV7e0c/mh3L7dTiN92k7AnarRC2V6Wx5lhCgWildhqga6Pdq7LwCRRW8v0srrFC+MSmDzBA8uHcVm7uCFKziAyDgZu2h3XXwiNJ3qYbxXcgz9aeXbxQbj43pvNboTUVkpj4mgr1Wlw1oYsS8EQQvw84DNUQWDFhiM/upsC0DFlqXOHggPhMVxprD4PgSyot7n4NyyN2120Sr2Mn8RyeepOn7DQClTHuyzGZpucuMUxP5BRtz3/gzA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=yrsDO55hUCOajNPGtH+AzSA/ihoZHoqmNYCqa9g0k+A=; b=VF64ieG9uDCB98D6I8xJQkfTWhiwM0nSoGe8xNsna4q6TdrIZ+oK7f0lgif++ljRPWqrrrbJfAeLLfOXph23sjVMYDcmq9RK0iqYtKWFsc+4bDMWi5kIqF530cGcjIYQpRG2qde3SbEfZgF76ghaAC1vsHhXn5RqScgJ47NHgNR3cvd26TpE7y0mYnZbJo7tJfNmZf61BB/cdu4E67uBVf+ph4lVFdGl4lGS13xrqc5DBereDMBcgDWLtXHozQTKxEOap+bhS0IyhLkctj3FN09pOaQRaUOYQaCONzqkP+DlfHrWjlbXq314wb5WVxjz1bcKvw1DHKIfpdZt4Ipe/g== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=yrsDO55hUCOajNPGtH+AzSA/ihoZHoqmNYCqa9g0k+A=; b=GbbiSEK0QSTf7HMIP95mf0GWllfkOz0arHeAMeOc81GZ97DoY2TLUbL6nR2Wx/SV6+JbC1oR30ZpwxmVBJL81LF/3d6CqLJfv7CHa7dD1ggLfmUQipZUmjsL/sMRVuKciJI/aLccpe0to2mxdbppYDHDcI8c8xSKkMQMCHwEkpqlW/pNVh4/3Z4pTKa80nAOd/b1jMpbxawbK1E+7TBk2M0gaHgD8J3zHuoadyq1UzEOLp4eJWSSRJAIlv8roHoGLj35hcCqtnpKQue+thg+RdsXvhenOal0D8hg4Ha8z0UMogbOVuI/oBhi9wGhCho2ylFOIIQClCMT4NR3Fc5mpA== Received: from BN8NAM04FT031.eop-NAM04.prod.protection.outlook.com (2a01:111:e400:7e85::47) by BN8NAM04HT191.eop-NAM04.prod.protection.outlook.com (2a01:111:e400:7e85::260) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3890.20; Wed, 3 Mar 2021 19:31:32 +0000 Received: from MWHPR06MB3102.namprd06.prod.outlook.com (2a01:111:e400:7e85::45) by BN8NAM04FT031.mail.protection.outlook.com (2a01:111:e400:7e85::371) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3890.20 via Frontend Transport; Wed, 3 Mar 2021 19:31:32 +0000 Received: from MWHPR06MB3102.namprd06.prod.outlook.com ([fe80::d4ee:1260:6f53:3f7b]) by MWHPR06MB3102.namprd06.prod.outlook.com ([fe80::d4ee:1260:6f53:3f7b%7]) with mapi id 15.20.3890.028; Wed, 3 Mar 2021 19:31:32 +0000 From: "Kun Qin" To: Laszlo Ersek , "devel@edk2.groups.io" CC: Michael D Kinney , Liming Gao , Zhiguang Liu , Hao A Wu , Jiewen Yao Subject: Re: [PATCH v4 1/7] MdePkg: MmUnblockMemoryLib: Added definition and null instance Thread-Topic: [PATCH v4 1/7] MdePkg: MmUnblockMemoryLib: Added definition and null instance Thread-Index: AQHXD59Q6eS3xEdf60ul4KbaCFMj1qpyZfsAgABBZlo= Date: Wed, 3 Mar 2021 19:31:32 +0000 Message-ID: References: <20210302200438.1901-1-kun.q@outlook.com> ,<5e239d7d-9639-42ab-26ea-4bd2273d556c@redhat.com> In-Reply-To: <5e239d7d-9639-42ab-26ea-4bd2273d556c@redhat.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-incomingtopheadermarker: OriginalChecksum:BBFFEB62C314EC654EC6417FBC185AD4748C2753D5AB42973A17A78F231B46C2;UpperCasedChecksum:FD8989588A4EF1B1FF8DACA66D64724C2B94E0C7FE8FFCB2795E8654A074FAC3;SizeAsReceived:7180;Count:44 x-tmn: [hL4ddYBST/zt6xSid3ozeuAUm15RQWZs] x-ms-publictraffictype: Email x-incomingheadercount: 44 x-eopattributedmessage: 0 x-ms-office365-filtering-correlation-id: 1494333b-cdba-4866-2444-08d8de7af399 x-ms-traffictypediagnostic: BN8NAM04HT191: x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Vhk/UDtkAaLlIv7yxQJXZTU3Y/KUzt7PDZB+ufhOrcgNAizootOLmZDYuOQ7O+6/zCiQTwS/UKBgC7NPc/p+0zpUQFDNg+ZaF6HQuj20NFzbcjONMZIdH9JAynlpnTU+YQ41kywSFptrs27E+46jIWUW9GA4xb7yjhNgSeiwQchb93Yq4mwZ0wAcwTEB25nvm4Vz4PeCTgF9Jhh37P1ZOT9lgNF1tCuInRBI/fu9VYtga2iu/wfIsG5XiQ0LaoBI/YzXtt8JLtstStuxAUmzP6yyvf1/6BaX+BHXO/RHBIndbCAeO2t4viSeYh3HWE7Po7y3+ta0UzoCh7WqXKPIPhWS9BPQHKBzwn6CfO4pAFDN4V3hp67Xk2jTdVDx/pv//FHHc87ueZbO3Y6q2YyaWJ744xrz03YIx/nXLmgDdZiyA+h20vUp/w184fO0Gt0g x-ms-exchange-antispam-messagedata: w4uA9X+w7FlYpNq2VyJbPUwsTU/NGDEnUwVlaHoHqHvhBWODvuoP/uA4edoKixQpO/gC2MeJzYENjSrJyfZtwALa5m795htjYYYIE1OcHB09VEY0s6hQ2/rMV3hmRm8ToTIT32FCSOwG9tyUhfI+lA== x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-AuthSource: BN8NAM04FT031.eop-NAM04.prod.protection.outlook.com X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-CrossTenant-Network-Message-Id: 1494333b-cdba-4866-2444-08d8de7af399 X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Mar 2021 19:31:32.6791 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Internet X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8NAM04HT191 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_MWHPR06MB31022D6BFDC936309036C847F3989MWHPR06MB3102namp_" --_000_MWHPR06MB31022D6BFDC936309036C847F3989MWHPR06MB3102namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks for finding these. I missed the return data type completely and agre= e that @retval should be used. Will update the patch in v5. Regards, Kun From: Laszlo Ersek Sent: Wednesday, March 3, 2021 07:34 To: Kun Qin; devel@edk2.groups.io Cc: Michael D Kinney; Liming Gao; Zhiguang Liu; Hao= A Wu; Jiewen Yao Subject: Re: [PATCH v4 1/7] MdePkg: MmUnblockMemoryLib: Added definition an= d null instance On 03/02/21 21:04, Kun Qin wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3168 > > This interface provides an abstration layer to allow MM modules to access > requested areas that are outside of MMRAM. On MM model that blocks all > non-MMRAM accesses, areas requested through this API will be mapped or > unblocked for accessibility inside MM environment. > > For MM modules that need to access regions outside of MMRAMs, the agents > that set up these regions are responsible for invoking this API in order > for these memory areas to be accessible from inside MM. > > Example usages: > 1. To enable runtime cache feature for variable service, Variable MM > module will need to access the allocated runtime buffer. Thus the agent > sets up these buffers, VariableSmmRuntimeDxe, will need to invoke this > API to make these regions accessible by Variable MM. > 2. For TPM ACPI table to communicate to physical presence handler, the > corresponding NVS region has to be accessible from inside MM. Once the > NVS region are assigned, it needs to be unblocked thourgh this API. > > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Zhiguang Liu > Cc: Hao A Wu > Cc: Jiewen Yao > Cc: Laszlo Ersek > > Signed-off-by: Kun Qin > --- > > Notes: > v4: > - Added more commit message [Laszlo] The commit message looks great, thanks for that. However, I notice the only "RETURN_xxx" macro in the patch is the "RETURN_UNSUPPORTED" return code, in the Null library instance. My point was that the *interface* should be downgraded to RETURN_xxx (that is, the lib class header -- the EFI_STATUS return type should be RETURN_STATUS; the documented @retval codes should all be RETURN_xxx, and so on). An example here is the PcdLib class. It uses the RETURN_STATUS return type, and documents the RETURN_INVALID_PARAMETER retval in some spots. And then edk2 provides several lib instances: - MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf - MdePkg/Library/PeiPcdLib/PeiPcdLib.inf - MdePkg/Library/DxePcdLib/DxePcdLib.inf The first of these is "BASE" (with no particular module type restriction), the 2nd is "PEIM" (with PEIM PEI_CORE SEC restrictions), and the 3rd is "DXE_DRIVER" (restricted to all the remaining module types). Every instance uses RETURN_xxx just fine. ... In case I'm wrong, please correct me, of course. Yet another comment: I'm just noticing that "@return" is used with actual constants. That's wrong, "@retval" needs to be used for constants; "@return" is only suitable if you provide a natural language description without a constant. Example: @retval TRUE It is currently raining. @retval FALSE It is currently not raining. vs. @return Whether it's currently raining or not. Thanks! Laszlo > - Added UNI file [Hao] > > 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 | 44 ++++++= ++++++++++++++ > MdePkg/Include/Library/MmUnblockMemoryLib.h | 44 ++++++= ++++++++++++++ > MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf | 34 ++++++= +++++++++ > MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.uni | 21 ++++++= ++++ > MdePkg/MdePkg.dec | 5 +++ > MdePkg/MdePkg.dsc | 1 + > 6 files changed, 149 insertions(+) > > diff --git a/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c b= /MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c > new file mode 100644 > index 000000000000..abdce41f10d1 > --- /dev/null > +++ b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c > @@ -0,0 +1,44 @@ > +/** @file > + Null instance of MM Unblock Page Library. > + > + This library provides an interface to request non-MMRAM pages to be ma= pped/unblocked > + from inside MM environment. > + > + For MM modules that need to access regions outside of MMRAMs, the agen= ts that set up > + these regions are responsible for invoking this API in order for these= memory areas > + to be accessed from inside MM. > + > + Copyright (c) Microsoft Corporation. > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > + > +/** > + 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 un= blocked from MM > + environment. > + > + @return EFI_SUCCESS The request goes through successfully. > + @return EFI_NOT_AVAILABLE_YET The requested functionality is not pro= duced yet. > + @return EFI_UNSUPPORTED The requested functionality is not sup= ported on current platform. > + @return EFI_SECURITY_VIOLATION The requested address failed to pass s= ecurity check for > + unblocking. > + @return EFI_INVALID_PARAMETER Input address either NULL pointer or n= ot 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 RETURN_UNSUPPORTED; > +} > diff --git a/MdePkg/Include/Library/MmUnblockMemoryLib.h b/MdePkg/Include= /Library/MmUnblockMemoryLib.h > new file mode 100644 > index 000000000000..980afe9a5fd3 > --- /dev/null > +++ b/MdePkg/Include/Library/MmUnblockMemoryLib.h > @@ -0,0 +1,44 @@ > +/** @file > + MM Unblock Memory Library Interface. > + > + This library provides an interface to request non-MMRAM pages to be ma= pped/unblocked > + from inside MM environment. > + > + For MM modules that need to access regions outside of MMRAMs, the agen= ts that set up > + these regions are responsible for invoking this API in order for these= memory areas > + to be accessed from inside MM. > + > + 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 un= blocked from MM > + environment. > + > + @return EFI_SUCCESS The request goes through successfully. > + @return EFI_NOT_AVAILABLE_YET The requested functionality is not pro= duced yet. > + @return EFI_UNSUPPORTED The requested functionality is not sup= ported on current platform. > + @return EFI_SECURITY_VIOLATION The requested address failed to pass s= ecurity check for > + unblocking. > + @return EFI_INVALID_PARAMETER Input address either NULL pointer or n= ot 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..8ecb767ff7bd > --- /dev/null > +++ b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf > @@ -0,0 +1,34 @@ > +## @file > +# Null instance of MM Unblock Page Library. > +# > +# This library provides an interface to request non-MMRAM pages to be m= apped/unblocked > +# from inside MM environment. > +# > +# For MM modules that need to access regions outside of MMRAMs, the age= nts that set up > +# these regions are responsible for invoking this API in order for thes= e memory areas > +# to be accessed from inside MM. > +# > +# Copyright (c) Microsoft Corporation. > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +# > +## > + > +[Defines] > + INF_VERSION =3D 0x0001001B > + BASE_NAME =3D MmUnblockMemoryLibNull > + MODULE_UNI_FILE =3D MmUnblockMemoryLibNull.uni > + FILE_GUID =3D 9E890F68-5C95-4C31-95DD-59E6286F85E= A > + MODULE_TYPE =3D BASE > + VERSION_STRING =3D 1.0 > + LIBRARY_CLASS =3D MmUnblockMemoryLib > + > +# > +# VALID_ARCHITECTURES =3D IA32 X64 > +# > + > +[Sources] > + MmUnblockMemoryLibNull.c > + > +[Packages] > + MdePkg/MdePkg.dec > diff --git a/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.uni= b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.uni > new file mode 100644 > index 000000000000..d7f2709a3dce > --- /dev/null > +++ b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.uni > @@ -0,0 +1,21 @@ > +// /** @file > +// Null instance of MM Unblock Page Library. > +// > +// This library provides an interface to request non-MMRAM pages to be m= apped/unblocked > +// from inside MM environment. > +// > +// For MM modules that need to access regions outside of MMRAMs, the age= nts that set up > +// these regions are responsible for invoking this API in order for thes= e memory areas > +// to be accessed from inside MM. > +// > +// Copyright (c) Microsoft Corporation. > +// SPDX-License-Identifier: BSD-2-Clause-Patent > +// > +// **/ > + > + > +#string STR_MODULE_ABSTRACT #language en-US "Null instance o= f MM Unblock Page Library." > + > +#string STR_MODULE_DESCRIPTION #language en-US "This library pr= ovides an interface to request non-MMRAM pages to be mapped/unblocked from = inside MM environment.\n" > + "For MM modules = that need to access regions outside of MMRAMs, the agents that set up these= regions are responsible for invoking this API in order for these memory ar= eas to be accessed from inside MM." > + > 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 > --_000_MWHPR06MB31022D6BFDC936309036C847F3989MWHPR06MB3102namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

Thanks for finding these. I missed the return data t= ype completely and agree that @retval should be used. Will update the patch= in v5.

 

Regards,

Kun

 

From: Laszlo Ersek
Sent: Wednesday, March 3, 2021 07:34
To: Kun Qin; devel@edk2.groups.io
Cc: Michael D Kinney; Liming Gao; Zhiguang Liu;= Hao A Wu; Jiewen Yao
Subject: Re: [PATCH v4 1/7] MdePkg: MmUnblockMemoryLib: Added defini= tion and null instance

 

On 03/02/21 21:04, Ku= n Qin wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3168
>
> This interface provides an abstration layer to allow MM modules to acc= ess
> requested areas that are outside of MMRAM. On MM model that blocks all=
> non-MMRAM accesses, areas requested through this API will be mapped or=
> unblocked for accessibility inside MM environment.
>
> For MM modules that need to access regions outside of MMRAMs, the agen= ts
> that set up these regions are responsible for invoking this API in ord= er
> for these memory areas to be accessible from inside MM.
>
> Example usages:
> 1. To enable runtime cache feature for variable service, Variable MM > module will need to access the allocated runtime buffer. Thus the agen= t
> sets up these buffers, VariableSmmRuntimeDxe, will need to invoke this=
> API to make these regions accessible by Variable MM.
> 2. For TPM ACPI table to communicate to physical presence handler, the=
> corresponding NVS region has to be accessible from inside MM. Once the=
> NVS region are assigned, it needs to be unblocked thourgh this API. >
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
>
> Signed-off-by: Kun Qin <kun.q@outlook.com>
> ---
>
> Notes:
>     v4:
>     - Added more commit message [Laszlo]

The commit message looks great, thanks for that.

However, I notice the only "RETURN_xxx" macro in the patch is the=
"RETURN_UNSUPPORTED" return code, in the Null library instance.
My point was that the *interface* should be downgraded to RETURN_xxx
(that is, the lib class header -- the EFI_STATUS return type should be
RETURN_STATUS; the documented @retval codes should all be RETURN_xxx,
and so on).

An example here is the PcdLib class. It uses the RETURN_STATUS return
type, and documents the RETURN_INVALID_PARAMETER retval in some spots.
And then edk2 provides several lib instances:

- MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
- MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
- MdePkg/Library/DxePcdLib/DxePcdLib.inf

The first of these is "BASE" (with no particular module type
restriction), the 2nd is "PEIM" (with PEIM PEI_CORE SEC restricti= ons),
and the 3rd is "DXE_DRIVER" (restricted to all the remaining modu= le
types). Every instance uses RETURN_xxx just fine.

... In case I'm wrong, please correct me, of course.


Yet another comment: I'm just noticing that "@return" is used wit= h
actual constants. That's wrong, "@retval" needs to be used for constants; "@return" is only suitable if you provide a natural la= nguage
description without a constant. Example:

  @retval TRUE   It is currently raining.
  @retval FALSE  It is currently not raining.

vs.

  @return  Whether it's currently raining or not.

Thanks!
Laszlo

>     - Added UNI file [Hao]
>    
>     v3:
>     - Move interface to MdePkg [Hao, Liming, Jiewe= n]
>     - Remove Dxe prefix [Jiewen]
>    
>     v2:
>     - Resend with practical usage. No change [Hao]=
>
>  MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c =   | 44 ++++++++++++++++++++
>  MdePkg/Include/Library/MmUnblockMemoryLib.h   &nb= sp;            =   | 44 ++++++++++++++++++++
>  MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf | 3= 4 +++++++++++++++
>  MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.uni | 2= 1 ++++++++++
>  MdePkg/MdePkg.dec        = ;            &n= bsp;            = ;           |  5 +++=
>  MdePkg/MdePkg.dsc        = ;            &n= bsp;            = ;           |  1 + >  6 files changed, 149 insertions(+)
>
> diff --git a/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.= c b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c
> new file mode 100644
> index 000000000000..abdce41f10d1
> --- /dev/null
> +++ b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c
> @@ -0,0 +1,44 @@
> +/** @file
> +  Null instance of MM Unblock Page Library.
> +
> +  This library provides an interface to request non-MMRAM pages = to be mapped/unblocked
> +  from inside MM environment.
> +
> +  For MM modules that need to access regions outside of MMRAMs, = the agents that set up
> +  these regions are responsible for invoking this API in order f= or these memory areas
> +  to be accessed from inside MM.
> +
> +  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 ac= cessible inside MM environment.
> +
> +  @param  UnblockAddress      = ;    The address of buffer caller requests to unblock, the a= ddress
> +           &nb= sp;            =           has to be page align= ed.
> +  @param  NumberOfPages      =      The number of pages requested to be unblocked from= MM
> +           &nb= sp;            =           environment.
> +
> +  @return EFI_SUCCESS       &= nbsp;     The request goes through successfully.
> +  @return EFI_NOT_AVAILABLE_YET   The requested functi= onality is not produced yet.
> +  @return EFI_UNSUPPORTED      &nb= sp;  The requested functionality is not supported on current platform.=
> +  @return EFI_SECURITY_VIOLATION  The requested address fai= led to pass security check for
> +           &nb= sp;            =           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
> +           &nb= sp;            =           phase.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MmUnblockMemoryRequest (
> +  IN EFI_PHYSICAL_ADDRESS   UnblockAddress,
> +  IN UINT64         = ;        NumberOfPages
> +  )
> +{
> +  return RETURN_UNSUPPORTED;
> +}
> diff --git a/MdePkg/Include/Library/MmUnblockMemoryLib.h b/MdePkg/Incl= ude/Library/MmUnblockMemoryLib.h
> new file mode 100644
> index 000000000000..980afe9a5fd3
> --- /dev/null
> +++ b/MdePkg/Include/Library/MmUnblockMemoryLib.h
> @@ -0,0 +1,44 @@
> +/** @file
> +  MM Unblock Memory Library Interface.
> +
> +  This library provides an interface to request non-MMRAM pages = to be mapped/unblocked
> +  from inside MM environment.
> +
> +  For MM modules that need to access regions outside of MMRAMs, = the agents that set up
> +  these regions are responsible for invoking this API in order f= or these memory areas
> +  to be accessed from inside MM.
> +
> +  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 ac= cessible inside MM environment.
> +
> +  @param  UnblockAddress      = ;    The address of buffer caller requests to unblock, the a= ddress
> +           &nb= sp;            =           has to be page align= ed.
> +  @param  NumberOfPages      =      The number of pages requested to be unblocked from= MM
> +           &nb= sp;            =           environment.
> +
> +  @return EFI_SUCCESS       &= nbsp;     The request goes through successfully.
> +  @return EFI_NOT_AVAILABLE_YET   The requested functi= onality is not produced yet.
> +  @return EFI_UNSUPPORTED      &nb= sp;  The requested functionality is not supported on current platform.=
> +  @return EFI_SECURITY_VIOLATION  The requested address fai= led to pass security check for
> +           &nb= sp;            =           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
> +           &nb= sp;            =           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..8ecb767ff7bd
> --- /dev/null
> +++ b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
> @@ -0,0 +1,34 @@
> +## @file
> +#  Null instance of MM Unblock Page Library.
> +#
> +#  This library provides an interface to request non-MMRAM pages= to be mapped/unblocked
> +#  from inside MM environment.
> +#
> +#  For MM modules that need to access regions outside of MMRAMs,= the agents that set up
> +#  these regions are responsible for invoking this API in order = for these memory areas
> +#  to be accessed from inside MM.
> +#
> +#  Copyright (c) Microsoft Corporation.
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION        &nb= sp;           =3D 0x00010= 01B
> +  BASE_NAME         = ;             = =3D MmUnblockMemoryLibNull
> +  MODULE_UNI_FILE        = ;        =3D MmUnblockMemoryLibNull.uni<= br> > +  FILE_GUID         = ;             = =3D 9E890F68-5C95-4C31-95DD-59E6286F85EA
> +  MODULE_TYPE        &nb= sp;           =3D BASE > +  VERSION_STRING        =          =3D 1.0
> +  LIBRARY_CLASS        &= nbsp;         =3D MmUnblockMemoryLi= b
> +
> +#
> +#  VALID_ARCHITECTURES       =     =3D IA32 X64
> +#
> +
> +[Sources]
> +  MmUnblockMemoryLibNull.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> diff --git a/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.= uni b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.uni
> new file mode 100644
> index 000000000000..d7f2709a3dce
> --- /dev/null
> +++ b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.uni
> @@ -0,0 +1,21 @@
> +// /** @file
> +// Null instance of MM Unblock Page Library.
> +//
> +// This library provides an interface to request non-MMRAM pages to b= e mapped/unblocked
> +// from inside MM environment.
> +//
> +// For MM modules that need to access regions outside of MMRAMs, the = agents that set up
> +// these regions are responsible for invoking this API in order for t= hese memory areas
> +// to be accessed from inside MM.
> +//
> +// Copyright (c) Microsoft Corporation.
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +// **/
> +
> +
> +#string STR_MODULE_ABSTRACT       =       #language en-US "Null instance of MM Un= block Page Library."
> +
> +#string STR_MODULE_DESCRIPTION      &nb= sp;   #language en-US "This library provides an interface to= request non-MMRAM pages to be mapped/unblocked from inside MM environment.= \n"
> +           &nb= sp;            =             &nb= sp;            =        "For MM modules that need to acce= ss regions outside of MMRAMs, the agents that set up these regions are resp= onsible for invoking this API in order for these memory areas to be accesse= d from inside MM."
> +
> 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/Un= itTestHostBaseLib.h
> 
> +  ##  @libraryclass  This library provides an interfac= e 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/SmiHandlerPr= ofileLibNull.inf
>    MdePkg/Library/MmServicesTableLib/MmServicesTableLib= .inf
> +  MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf > 
>  [Components.EBC]
>    MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic= .inf
>

 

--_000_MWHPR06MB31022D6BFDC936309036C847F3989MWHPR06MB3102namp_--