From: "Sami Mujawar" <sami.mujawar@arm.com>
To: Sayanta Pattanayak <sayanta.pattanayak@arm.com>, devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>, nd@arm.com
Subject: Re: [edk2-platforms][PATCH V1 3/3] Platform/Sgi: enable support for UEFI secure boot
Date: Tue, 25 May 2021 15:00:30 +0100 [thread overview]
Message-ID: <540101a6-dad9-0b69-8477-62ee2f369825@arm.com> (raw)
In-Reply-To: <20210524172300.28754-4-sayanta.pattanayak@arm.com>
Hi Sayanta,
Thank you for this patch.
Please find my response inline marked [SAMI].
Regards,
Sami Mujawar
On 24/05/2021 06:23 PM, Sayanta Pattanayak wrote:
> Enable the use of UEFI secure boot for Arm's Neoverse reference design
> platforms. The UEFI authenticated variable store uses NOR flash 2 which
> is accessible from Standalone MM context residing in a secure partition.
>
> Signed-off-by: Sayanta Pattanayak <sayanta.pattanayak@arm.com>
> ---
> Platform/ARM/SgiPkg/SgiPlatform.dsc.inc | 31 +++++++++++++++++++
> Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc | 32 ++++++++++++++++++++
> Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc | 15 +++++++++
> Platform/ARM/SgiPkg/PlatformStandaloneMm2.dsc | 15 +++++++++
> Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf | 5 +++
> Platform/ARM/SgiPkg/SgiPlatform.fdf | 9 +++++-
> 6 files changed, 106 insertions(+), 1 deletion(-)
>
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc b/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
> index 091de0c99c74..e4aee7a09acf 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
> @@ -6,6 +6,14 @@
>
> !include Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
>
> +[Defines]
> + # To allow the use of secure storage, set this to TRUE.
> + DEFINE SECURE_STORAGE_ENABLE = FALSE
> +
> + # To allow the use of UEFI secure boot, set this to TRUE.
> + # Secure boot requires secure storage to be enabled as well.
> + DEFINE SECURE_BOOT_ENABLE = FALSE
> +
> [BuildOptions]
> *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
>
> @@ -22,6 +30,9 @@
> NorFlashPlatformLib|Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.inf
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> + MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
> +!endif
>
> # Virtio Support
> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
> @@ -84,6 +95,7 @@
> [PcdsFeatureFlag.common]
> gArmSgiTokenSpaceGuid.PcdVirtioBlkSupported|TRUE
> gArmSgiTokenSpaceGuid.PcdVirtioNetSupported|TRUE
> + gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE
>
> [PcdsFixedAtBuild.common]
> gArmTokenSpaceGuid.PcdVFPEnabled|1
> @@ -230,7 +242,15 @@
> MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> + 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
> @@ -238,6 +258,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
> @@ -245,6 +268,7 @@
> BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> }
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> +!endif
>
> #
> # ACPI Support
> @@ -314,4 +338,11 @@
> #
> MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
>
> +!if $(SECURE_STORAGE_ENABLE) == TRUE
> + ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf {
> + <LibraryClasses>
> + NULL|StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
> + }
> +!else
> ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> +!endif
> diff --git a/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc b/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc
> index 3389ff676a91..6839ec35da8a 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc
> +++ b/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc
> @@ -59,6 +59,19 @@
> HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
> MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
> +!if $(SECURE_STORAGE_ENABLE) == TRUE
> + 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
> + RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
[SAMI] There is a recent patch series that adds ARMv8.5 FEAT_RNG support
to BaseRngLib
see
https://github.com/tianocore/edk2/commit/9301e5644cef5a5234f71b178373dd508cabb9ee.
Can this be used instead of BaseRngLibTimerLib? BaseRngLibTimerLib is
for non-production use so it would be good to avoid.
Indeed, this would require that Sgi platforms are ARMv8.5 or above. If
not, then can we conditionally use BaseRngLibTimerLib for platforms that
do not support FEAT_RNG.
[/SAMI]
> + PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
> + SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> + TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
> + VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> + SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> +!endif
>
> ################################################################################
> #
> @@ -75,6 +88,12 @@
>
> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x2
>
> +!if $(SECURE_STORAGE_ENABLE) == TRUE
> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> + gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
> +!endif
> +
> ###################################################################################################
> #
> # Components Section - list of the modules and components that will be processed by compilation
> @@ -101,6 +120,19 @@
>
> [Components.AARCH64]
> StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
> +!if $(SECURE_STORAGE_ENABLE) == TRUE
> + ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> + MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
> + MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf {
> + <LibraryClasses>
> + DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> + NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
> + NULL|MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneMm.inf
> + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
> + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
> + }
> +!endif
>
> ###################################################################################################
> #
> diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> index cdf8aaa88f03..2cb4895cfcff 100644
> --- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> +++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> @@ -39,3 +39,18 @@
> [PcdsFixedAtBuild]
> ## PL011 - Serial Terminal
> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x7FF70000
> +
> +!if $(SECURE_STORAGE_ENABLE) == TRUE
> + ##Secure NOR Flash 2
> + gArmSgiTokenSpaceGuid.PcdSmcCs2Base|0x10000000
> + gArmSgiTokenSpaceGuid.PcdSysPeriphBase|0x1C000000
> + gArmSgiTokenSpaceGuid.PcdSysPeriphSysRegBase|0x1C010000
> +
> + ##Secure Variable Storage in NOR Flash 2
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x10000000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00100000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x10100000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00100000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x10200000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00100000
> +!endif
> diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm2.dsc b/Platform/ARM/SgiPkg/PlatformStandaloneMm2.dsc
> index bb359a15cc0d..46c2ae3529d1 100644
> --- a/Platform/ARM/SgiPkg/PlatformStandaloneMm2.dsc
> +++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm2.dsc
> @@ -38,3 +38,18 @@
> [PcdsFixedAtBuild]
> ## PL011 - Serial Terminal
> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x0EF80000
> +
> +!if $(SECURE_STORAGE_ENABLE) == TRUE
> + ##Secure NOR Flash 2
> + gArmSgiTokenSpaceGuid.PcdSmcCs2Base|0x1054000000
> + gArmSgiTokenSpaceGuid.PcdSysPeriphBase|0x0C000000
> + gArmSgiTokenSpaceGuid.PcdSysPeriphSysRegBase|0x0C010000
> +
> + ##Secure Variable Storage in NOR Flash 2
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0x1054000000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00100000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0x1054100000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00100000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0x1054200000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00100000
> +!endif
> diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf b/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> index 5a0772cd8522..474c9c0ce764 100644
> --- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> +++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> @@ -49,6 +49,11 @@ READ_LOCK_CAP = TRUE
> READ_LOCK_STATUS = TRUE
>
> INF StandaloneMmPkg/Core/StandaloneMmCore.inf
> +!if $(SECURE_STORAGE_ENABLE) == TRUE
> + 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.fdf b/Platform/ARM/SgiPkg/SgiPlatform.fdf
> index e11d943d6efc..d94e4633e36c 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.fdf
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.fdf
> @@ -90,10 +90,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
> + INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
> +!else
> + INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> +!endif
>
> #
> # ACPI Support
next prev parent reply other threads:[~2021-05-25 14:00 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-24 17:22 [edk2-platforms][PATCH V1 0/3] Platform/Sgi: enable support for UEFI secure boot sayanta.pattanayak
2021-05-24 17:22 ` [edk2-platforms][PATCH V1 1/3] Platform/Sgi: refactor StandaloneMM platform description file Sayanta Pattanayak
2021-05-25 13:57 ` Sami Mujawar
2021-05-24 17:22 ` [edk2-platforms][PATCH V1 2/3] Platform/Sgi: add StandaloneMM usable NorFlashPlatformLib Sayanta Pattanayak
2021-05-25 13:57 ` Sami Mujawar
2021-05-24 17:23 ` [edk2-platforms][PATCH V1 3/3] Platform/Sgi: enable support for UEFI secure boot Sayanta Pattanayak
2021-05-25 14:00 ` Sami Mujawar [this message]
2021-05-26 18:15 ` Sayanta Pattanayak
2021-05-26 18:20 ` Sami Mujawar
2021-06-01 18:09 ` [edk2-devel] [edk2-platforms][PATCH V1 0/3] " Thomas Abraham
2021-06-02 18:18 ` Sami Mujawar
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=540101a6-dad9-0b69-8477-62ee2f369825@arm.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