From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.1858.1614616813508412756 for ; Mon, 01 Mar 2021 08:40:13 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=YGGffW8Q; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1614616812; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QoUXyorMixbOjIPm5rn99Rri6WTHm0o0yQ3s/LIcGYo=; b=YGGffW8QDIMXiKCpwfWPmGoycor1TiAYyxOw/LBSca+FDvjAWm3XEFwQJLpxbnyVk73yJ5 umkzNLNd3Rg9a91kTuf/cKxxpt0sFdnbww0e1DAtwgH/CE+kY4/DYGfnJZIu7lYZSTHwGw 4aFbwt2M9io3BR0/lgAbJ22rcZInEog= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-30-CVvgvLByNiq5jihqvzPqzg-1; Mon, 01 Mar 2021 11:40:05 -0500 X-MC-Unique: CVvgvLByNiq5jihqvzPqzg-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E673F1007B31; Mon, 1 Mar 2021 16:40:03 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-254.ams2.redhat.com [10.36.114.254]) by smtp.corp.redhat.com (Postfix) with ESMTP id 217236A8EF; Mon, 1 Mar 2021 16:40:00 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v3 1/7] MdePkg: MmUnblockMemoryLib: Added definition and null instance To: devel@edk2.groups.io, kun.q@outlook.com Cc: Michael D Kinney , Liming Gao , Zhiguang Liu , Hao A Wu , Jiewen Yao References: <20210226225158.1378-1-kun.q@outlook.com> From: "Laszlo Ersek" Message-ID: Date: Mon, 1 Mar 2021 17:40:00 +0100 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 > Cc: Liming Gao > Cc: Zhiguang Liu > Cc: Hao A Wu > Cc: Jiewen Yao > > Signed-off-by: Kun Qin > --- > > 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 > + > +/** > + 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 >