From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::144; helo=mail-it1-x144.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x144.google.com (mail-it1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 16F90211CC3CF for ; Fri, 1 Mar 2019 03:31:07 -0800 (PST) Received: by mail-it1-x144.google.com with SMTP id l15so20778956iti.4 for ; Fri, 01 Mar 2019 03:31:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=kJP+GgKdOLwkxPunEwEduzNK2gRVGfoxZq5EgfB1I2E=; b=Hbap/Ti+1XCBd9kbpY0cCqaKymAP2wnMhb2Msw2x2PZzMFkcWjeroc/PWizl52qXUa vLR080mvA1RvwBzGxr1YckNze82caYu2F/5tT8qcRepMi86cw5+EKQkcPbB8VCY2YHi6 ioNSobgeH1nfHLeZJOz2b2RC9vAtSKhUzrLRlv9gntF0oyafg6dlsuJjPJYptYxk4ExW I3dogHhCHHVavL3qN17BnVIk3lTN9QmnRnBfSIVak7cnVcElx+Ytv2g3ekBc2O+y5l8p kOcWTuTntHbUGahX2472mVXb9m3ZSnLyup6NhkN+RYPMnafNDakfLif1r9OhTUwQb/9F tIxQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=kJP+GgKdOLwkxPunEwEduzNK2gRVGfoxZq5EgfB1I2E=; b=AmNRUzKEFybX0TJGXotBWWhF/zPHo3SSJgQl8CUHsv21fFSNTL3Er26MoN7eFooMZv wVCfyj2jWvjd9ZAnwC0b61op91OAFV0SAUBGS49X9UD7BVp9/wbKizZAqg4elpeke+88 P8UZLZhxZgPlPWHl/ZgS1Q5pa/DDbZoHRRhU4lZ4OlLk50pMqNhgeDUR9ymLUsMxEutA rgEM9iDVso/zrsjYJK5uNQbMtwSyIemIq4/SsHbglpD4lwV0xmj5TL6apMPFsMjOSDNf 2QdK40sw5Vl57/A+JLV3mbWBS+955xG7KI7dha0fISxVqQSh5sbEZ7JXfteq5THo8L+X 3TDQ== X-Gm-Message-State: APjAAAWv1T7l8s/GV1Pg6teDDgUBxYOpXat8I/X6nCfmiS8dpkPZiZ2D 4hkq2VFXrl+plFu3wN50qBPXqMg9SWaJeH+AJpSehA== X-Google-Smtp-Source: AHgI3IbOfCxRCoqhwCRsV/PtJzRrPRkSPd30yXBiDHOy14k6WsSVhXzyrf2LGf2Ro3R0iSAM9jecHGXunITlV5TpIHY= X-Received: by 2002:a24:1947:: with SMTP id b68mr2474554itb.121.1551439866752; Fri, 01 Mar 2019 03:31:06 -0800 (PST) MIME-Version: 1.0 References: <1551438858-16928-1-git-send-email-jagadeesh.ujja@arm.com> In-Reply-To: <1551438858-16928-1-git-send-email-jagadeesh.ujja@arm.com> From: Ard Biesheuvel Date: Fri, 1 Mar 2019 12:30:55 +0100 Message-ID: To: Jagadeesh Ujja Cc: "edk2-devel@lists.01.org" , "Gao, Liming" , "Zhang, Chao B" , Leif Lindholm , "Zeng, Star" , "Yao, Jiewen" , "Kinney, Michael D" Subject: Re: [PATCH v2] MdePkg/Library: Install dummy variable arch protocol X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Mar 2019 11:31:08 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 1 Mar 2019 at 12:14, Jagadeesh Ujja wrote: > > In a system implementing the variable store in MM, there are no variable > arch protocol and variable write arch protocol installed into the > DXE_SMM protocol database. On such systems, it is not required to > locate these protocols by the DXE runtime variable drivers because > it can be assumed that these protocols are already installed in the MM > context. But then such an implementation will deviate from the existing > traditional MM based variable driver implementation. > > So in order to maintain consistency with the traditional MM variable > driver implementation, allow platforms to install dummy versions of > these protocols into the DXE protocol database but these protocol will > not be consumed by non-secure variable service runtime driver. > > The Platform which uses StandaloneMM based secure variable storage > have to include this library as below. > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf { > > NULL|MdePkg/Library/VariableMmDependency/VariableMmDependency.inf > } > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jagadeesh Ujja > --- > Changes since v1: > - This is a next version of patch > =E2=80=9CMdeModulePkg/VariableSmmRuntimeDxe: Refactor locating Variabl= e Arch Protocol=E2=80=9D. > [https://lists.01.org/pipermail/edk2-devel/2019-February/036885.html] > - Addressed the comments from Ard Biesheuvel and Zeng Star > - Can this library be placed in MdePkg rather then the StandaloneMmPkg? > This does not belong in MdePkg. What is wrong with keeping it in StandaloneMmPkg? > MdePkg/Library/VariableMmDependency/VariableMmDependency.c | 85 ++++++= ++++++++++++++ > MdePkg/Library/VariableMmDependency/VariableMmDependency.inf | 48 ++++++= +++++ > 2 files changed, 133 insertions(+) > > diff --git a/MdePkg/Library/VariableMmDependency/VariableMmDependency.c b= /MdePkg/Library/VariableMmDependency/VariableMmDependency.c > new file mode 100644 > index 0000000..6e5117e > --- /dev/null > +++ b/MdePkg/Library/VariableMmDependency/VariableMmDependency.c > @@ -0,0 +1,85 @@ > +/** @file > + Runtime DXE part corresponding to StanaloneMM variable module. > + > +This module installs dummy variable arch protocol and dummy variable wri= te arch protocol > +to StandaloneMM runtime variable service. > + I think 'dummy' is a misnomer here. They are NULL protocols in the sense that only their presence is signifcant, and the protocol does not have an implementation. But this is true for traditional MM as well. > +Copyright (c) 2019, ARM Ltd. All rights reserved. > + > +This program and the accompanying materials > +are licensed and made available under the terms and conditions of the BS= D License > +which accompanies this distribution. The full text of the license may b= e found at > +http://opensource.org/licenses/bsd-license.php. > + > +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMP= LIED. > + > +**/ > + > +#include > +#include > + > +/** > + Notify the system that the SMM variable driver is ready. > +**/ > +VOID > +VariableNotifySmmReady ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + EFI_HANDLE Handle; > + > + Handle =3D NULL; > + Status =3D gBS->InstallProtocolInterface ( > + &Handle, > + &gEfiSmmVariableProtocolGuid, > + EFI_NATIVE_INTERFACE, > + NULL > + ); > + ASSERT_EFI_ERROR (Status); > +} > + > +/** > + Notify the system that the SMM variable write driver is ready. > +**/ > +VOID > +VariableNotifySmmWriteReady ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + EFI_HANDLE Handle; > + > + Handle =3D NULL; > + Status =3D gBS->InstallProtocolInterface ( > + &Handle, > + &gSmmVariableWriteGuid, > + EFI_NATIVE_INTERFACE, > + NULL > + ); > + ASSERT_EFI_ERROR (Status); > +} > + > +/** > + The constructor function calls and installs dummy variable arch protoc= ol and > + dummy variable write arch protocol to StandaloneMM runtime variable se= rvice > + > + @param ImageHandle The firmware allocated handle for the EFI image. > + @param SystemTable A pointer to the Management mode System Table. > + > + @retval EFI_SUCCESS The constructor always returns EFI_SUCCESS. > + > +**/ > +EFI_STATUS > +EFIAPI > +VariableMmDependencyLibConstructor ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + VariableNotifySmmReady(); > + VariableNotifySmmWriteReady(); > + return EFI_SUCCESS; > +} > + Can we replace all of this with a single call to InstallMultipleProtocolInterfaces() please? > diff --git a/MdePkg/Library/VariableMmDependency/VariableMmDependency.inf= b/MdePkg/Library/VariableMmDependency/VariableMmDependency.inf > new file mode 100644 > index 0000000..09fd200 > --- /dev/null > +++ b/MdePkg/Library/VariableMmDependency/VariableMmDependency.inf > @@ -0,0 +1,48 @@ > +## @file > +# Runtime DXE part corresponding to StanaloneMM variable module. > +# > +# This module installs dummy variable arch protocol and dummy variable = write arch protocol > +# to StandaloneMM runtime variable service. > +# > +# Copyright (c) 2019, ARM Ltd. All rights reserved. > +# This program and the accompanying materials > +# are licensed and made available under the terms and conditions of the = BSD License > +# which accompanies this distribution. The full text of the license may = be found at > +# http://opensource.org/licenses/bsd-license.php > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR I= MPLIED. > +# > +## > + > +[Defines] > + INF_VERSION =3D 0x0001001A > + BASE_NAME =3D VariableMmDependency > + FILE_GUID =3D 64BC4129-778E-4867-BA07-13999A4DEC3= F > + MODULE_TYPE =3D DXE_RUNTIME_DRIVER Better use DXE_DRIVER here > + VERSION_STRING =3D 1.0 > + PI_SPECIFICATION_VERSION =3D 0x00010032 You can drop this > + LIBRARY_CLASS =3D NULL > + CONSTRUCTOR =3D VariableMmDependencyLibConstructor > + > +# > +# The following information is for reference only and not required by th= e build tools. > +# > +# VALID_ARCHITECTURES =3D AARCH64 > +# > +# > + > +[Sources] > + VariableMmDependency.c > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + > +[Protocols] > + gEfiSmmVariableProtocolGuid > + > +[Guids] > + gSmmVariableWriteGuid > + This looks slightly inconsistent, but it actually matches what MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf does. So perhaps you could add the same annotation here as well: [Protocols] gEfiSmmVariableProtocolGuid ## PRODUCES [Guids] gSmmVariableWriteGuid ## PRODUCES ## GUID # Install protocol > +[Depex] > + TRUE > -- > 2.7.4 >