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::d43; helo=mail-io1-xd43.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io1-xd43.google.com (mail-io1-xd43.google.com [IPv6:2607:f8b0:4864:20::d43]) (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 BD619211D35F0 for ; Mon, 4 Mar 2019 07:30:05 -0800 (PST) Received: by mail-io1-xd43.google.com with SMTP id y13so4235076iop.11 for ; Mon, 04 Mar 2019 07:30:05 -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; bh=xnHBNuXV5auzkX+EFilfFw3Vd2gUNrDyTVDBwhrEkrA=; b=AuQKSQ0HAu3PO9Qi2sr7aU4K7Go8H8PoMquWFy5ei5H0ipbtdzCfguwQAPszaWDExP xHpMrpyxv8KZLMnCout3TW87cNmzylGhDnoLEZRUU1nND5zHOw5hcicaN8I+jG7s+9hT srOpQ/KkyIw+qFjvk0abHD/RBLlwTOBUfz27Q5ALnKfkc34OhNNxcvvPFB+oOVbm4SuC h0LoS+vgPIDUlv0Vx+At6pwr05vs+zBOt56QTgFxH+JC1etuwjrSfx9xgeFhYTT6vOHV fVaSKZxQIDLXbK4eczEZdV2MEXemOhAnXrrkP8I7nK2PymjnJ3w0lbPIM5pEs2t6NLzc bB/g== 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; bh=xnHBNuXV5auzkX+EFilfFw3Vd2gUNrDyTVDBwhrEkrA=; b=hY2PsPMainpVuAD/uVP0lfWHqODWHoLhCIm+eACkvjCs/ZH9u85dfPIU/8gz5MKW8Y EDM/FJprIAgRaoBUT9V0ukSgXPPQjQb2EcuBgBH+WSpQIdUA73HeMndpZtG8p8Vkbrw5 hSy4lOxSWt/3Og31CiG4yqU5hbPvO5DYteGGhNd5H6ALGG/ha44916sOFjHjF7zfB9Zr XA21yO3z2rS8yd0fA1hsvAKCQGXHN6wKzLJBgC4Rsf8uXV1RiLy8cRYjWYO9Fk261Ufx 4cp7CoPLHTW5C3HD7NjSc+bSm/BCuFCIjT98D+t52PS5Vqk91xZ/1046Gazg/9TwA1e5 2c5A== X-Gm-Message-State: APjAAAUg80mrGHAFN5cmINLsXuAnlR0TF8RdJ5zSXBHJqIhVt09K3fd1 BG/JdnGxDJBRIcyTi7sQXRSCG4Bf0g63UqVhvIVh1w== X-Google-Smtp-Source: APXvYqzySKpyMjItv2xeZWfG2szCw9IFXEfXsfMjepCGL5D5DhshdACpzSDauF1+tgQaEqqm9JKJzcd6r53Pl+4zpto= X-Received: by 2002:a6b:7b02:: with SMTP id l2mr5858717iop.60.1551713404143; Mon, 04 Mar 2019 07:30:04 -0800 (PST) MIME-Version: 1.0 References: <1551694313-23594-1-git-send-email-jagadeesh.ujja@arm.com> <1551694313-23594-4-git-send-email-jagadeesh.ujja@arm.com> In-Reply-To: <1551694313-23594-4-git-send-email-jagadeesh.ujja@arm.com> From: Ard Biesheuvel Date: Mon, 4 Mar 2019 16:29:52 +0100 Message-ID: To: Jagadeesh Ujja Cc: "edk2-devel@lists.01.org" , Leif Lindholm Subject: Re: [PATCH edk2-platforms 3/3] Platform/ARM/SgiPkg: add MM based UEFI secure boot support 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: Mon, 04 Mar 2019 15:30:07 -0000 Content-Type: text/plain; charset="UTF-8" On Mon, 4 Mar 2019 at 11:12, Jagadeesh Ujja wrote: > > This implements support for UEFI secure boot on SGI platforms using > the standalone MM framework. This moves all of the software handling > of the UEFI authenticated variable store into the standalone MM > context residing in a secure partition. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jagadeesh Ujja > --- > Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc | 33 +++++++++++++++++++- > Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf | 5 +++ > Platform/ARM/SgiPkg/SgiPlatform.dsc | 18 ++++++++++- > Platform/ARM/SgiPkg/SgiPlatform.fdf | 9 +++++- > 4 files changed, 62 insertions(+), 3 deletions(-) > > diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc > index 65dd6ac..889be2f 100644 > --- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc > +++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc > @@ -75,7 +75,17 @@ > HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf > MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf > MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf > - > +!if $(MM_SECURE_STORAGE_ENABLE) == TRUE Does it make sense to make this conditional? Would you build PlatformStandaloneMm.dsc if MM_SECURE_STORAGE_ENABLE is FALSE? > + AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf > + BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > + IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf > + NorFlashPlatformLib|Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf > + OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf > + PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf > + SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > + TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf > + VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf > +!endif > ################################################################################ > # > # Pcd Section - list of all EDK II PCD Entries defined by this Platform > @@ -93,6 +103,21 @@ > gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x7FF70000 > gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200 > > +!if $(MM_SECURE_STORAGE_ENABLE) == TRUE Same here > + #Secure Storage > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 > + gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 > + > + ## NV Storage - 1MB*3 in NOR2 Flash > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x10400000 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00100000 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x10500000 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00100000 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x10600000 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00100000 > +!endif > + > ################################################################################################### > # > # Components Section - list of the modules and components that will be processed by compilation > @@ -118,6 +143,12 @@ > StandaloneMmPkg/Core/StandaloneMmCore.inf > > [Components.AARCH64] > +!if $(MM_SECURE_STORAGE_ENABLE) == TRUE Same here > + ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf > + MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf > + MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > +!endif > + > StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf > > ################################################################################################### > diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf b/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf > index 810460c..04aa5cd 100644 > --- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf > +++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf > @@ -55,6 +55,11 @@ READ_LOCK_CAP = TRUE > READ_LOCK_STATUS = TRUE > > INF StandaloneMmPkg/Core/StandaloneMmCore.inf > +!if $(MM_SECURE_STORAGE_ENABLE) == TRUE and here > + INF ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf > + INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf > + INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > +!endif > INF StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf > > ################################################################################ > diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc > index bdb4ecb..cba2e1f 100644 > --- a/Platform/ARM/SgiPkg/SgiPlatform.dsc > +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc > @@ -260,7 +260,15 @@ > MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf > MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf > MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf > +!if $(SECURE_BOOT_ENABLE) == TRUE Please declare this symbol in the [Defines] section using 'DEFINE' > + MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf { > + > + NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf > + } > + SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf > +!else > MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf > +!endif > OvmfPkg/VirtioBlkDxe/VirtioBlk.inf > > MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf > @@ -268,6 +276,9 @@ > MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf > MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf > MdeModulePkg/Universal/SerialDxe/SerialDxe.inf > +!if $(SECURE_STORAGE_ENABLE) == TRUE > + MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf > +!else > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { > > NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf > @@ -275,6 +286,7 @@ > BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf > } > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > +!endif > > # > # ACPI Support > @@ -344,4 +356,8 @@ > # > MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf > > - ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > + ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf { > + > + NULL|StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf > + } > + ArmPkg/Drivers/ApeiDxe/Apei.inf This driver does not exist > diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf b/Platform/ARM/SgiPkg/SgiPlatform.fdf > index 7916a52..a438940 100644 > --- a/Platform/ARM/SgiPkg/SgiPlatform.fdf > +++ b/Platform/ARM/SgiPkg/SgiPlatform.fdf > @@ -96,10 +96,17 @@ READ_LOCK_STATUS = TRUE > INF EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf > INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf > INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf > - INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf > INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf > +!if $(SECURE_BOOT_ENABLE) == TRUE > + INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf > +!endif > +!if $(SECURE_STORAGE_ENABLE) == TRUE Why would you support secure boot without secure storage? Can you create a single configuration item for this? > + INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf > +!else > + INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > +!endif > > # > # ACPI Support > -- > 2.7.4 >