[edk2-devel] [edk2-platforms][PATCH V1 3/3] Platform/Sgi: enable support for UEFI secure boot

Sami Mujawar sami.mujawar at arm.com
Tue May 25 14:00:30 UTC 2021


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 at 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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75581): https://edk2.groups.io/g/devel/message/75581
Mute This Topic: https://groups.io/mt/83062022/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list