From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [PATCH edk2-platforms 3/3] Platform/ARM/SgiPkg: add MM based UEFI secure boot support
Date: Mon, 4 Mar 2019 16:29:52 +0100 [thread overview]
Message-ID: <CAKv+Gu8osM2+M6QKyDdKWJQ=LUUa3+rfraLpWpMCu=+DS4KWZg@mail.gmail.com> (raw)
In-Reply-To: <1551694313-23594-4-git-send-email-jagadeesh.ujja@arm.com>
On Mon, 4 Mar 2019 at 11:12, Jagadeesh Ujja <jagadeesh.ujja@arm.com> 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 <jagadeesh.ujja@arm.com>
> ---
> 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 {
> + <LibraryClasses>
> + 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 {
> <LibraryClasses>
> 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 {
> + <LibraryClasses>
> + 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
>
prev parent reply other threads:[~2019-03-04 15:30 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-04 10:11 [PATCH edk2-platforms 0/3] Platform/ARM/SgiPkg: Implement StandaloneMm based secure boot Jagadeesh Ujja
2019-03-04 10:11 ` [PATCH edk2-platforms 1/3] Platform/ARM/Sgi: define nor2 flash controller memory map Jagadeesh Ujja
2019-03-04 15:23 ` Ard Biesheuvel
2019-03-04 10:11 ` [PATCH edk2-platforms 2/3] Platform/ARM/Sgi: allow MM_STANDALONE modules to use NorFlashPlatformLib Jagadeesh Ujja
2019-03-04 15:22 ` Ard Biesheuvel
2019-03-04 10:11 ` [PATCH edk2-platforms 3/3] Platform/ARM/SgiPkg: add MM based UEFI secure boot support Jagadeesh Ujja
2019-03-04 15:29 ` Ard Biesheuvel [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKv+Gu8osM2+M6QKyDdKWJQ=LUUa3+rfraLpWpMCu=+DS4KWZg@mail.gmail.com' \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox