public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif@nuviainc.com>
To: Masahisa Kojima <masahisa.kojima@linaro.org>
Cc: devel@edk2.groups.io, Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Graeme Gregory <graeme@nuviainc.com>,
	Radoslaw Biernacki <rad@semihalf.com>,
	Shashi Mallela <shashi.mallela@linaro.org>
Subject: Re: [PATCH edk2-platforms v2 2/4] SbsaQemu: add MM based UEFI secure boot support
Date: Mon, 1 Mar 2021 17:22:16 +0000	[thread overview]
Message-ID: <20210301172216.GU1664@vanye> (raw)
In-Reply-To: <20210301051952.29091-3-masahisa.kojima@linaro.org>

On Mon, Mar 01, 2021 at 14:19:50 +0900, Masahisa Kojima wrote:
> This implements support for UEFI secure boot on SbsaQemu 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.
> 
> Secure variable storage is located at 0x01000000 in secure NOR Flash.
> 
> Non-secure shared memory between UEFI and standalone MM
> is allocated at the top of DRAM.
> DRAM size of SbsaQemu varies depends on the QEMU parameter,
> the non-secure shared memory base address is passed from
> trusted-firmware through the device tree "/reserved-memory" node.
> 
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
>  Platform/Qemu/SbsaQemu/SbsaQemu.dsc           | 43 +++++++---
>  .../Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc    | 39 +++++++++
>  Platform/Qemu/SbsaQemu/SbsaQemu.fdf           | 82 +++++++++++++++++--
>  .../Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf    |  7 +-
>  .../Library/SbsaQemuLib/SbsaQemuLib.inf       |  2 +
>  .../Library/SbsaQemuLib/SbsaQemuMem.c         | 37 ++++++++-
>  6 files changed, 190 insertions(+), 20 deletions(-)
> 
> diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> index c1f8a4696560..a75116ee70fc 100644
> --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> @@ -28,6 +28,8 @@ [Defines]
>  
>    DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
>  
> +  DEFINE SECURE_BOOT_ENABLE      = FALSE
> +
>  #
>  # Network definition
>  #
> @@ -152,12 +154,10 @@ [LibraryClasses.common]
>    # Secure Boot dependencies
>    #
>    TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> -  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
>  
>    # re-use the UserPhysicalPresent() dummy implementation from the ovmf tree
>    PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>  
> -  VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>    VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
>    VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
>  
> @@ -171,6 +171,7 @@ [LibraryClasses.common]
>    ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
>  
>    TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> +

This blank line is added for no apparent reason.

>    NorFlashPlatformLib|Silicon/Qemu/SbsaQemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.inf
>  
>    CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
> @@ -300,6 +301,8 @@ [PcdsFeatureFlag.common]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
>  
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE
> +
>  [PcdsFixedAtBuild.common]
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|1000000
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|1000000
> @@ -551,6 +554,9 @@ [PcdsDynamicDefault.common]
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisAssetTag|L"AT0000"
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisSKU|L"SK0000"
>  
> +  gArmTokenSpaceGuid.PcdMmBufferBase|0x10000000000
> +  gArmTokenSpaceGuid.PcdMmBufferSize|0x00200000
> +
>  ################################################################################
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> @@ -604,7 +610,6 @@ [Components.common]
>    ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>    ArmPkg/Drivers/CpuPei/CpuPei.inf
>  
> -
>    MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
>      <LibraryClasses>
>        NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
> @@ -628,24 +633,40 @@ [Components.common]
>    #
>    ArmPkg/Drivers/CpuDxe/CpuDxe.inf
>    MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> -  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
> -    <LibraryClasses>
> -      NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
> -      # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
> -      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> -  }
>    MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
>      <LibraryClasses>
> +!if $(SECURE_BOOT_ENABLE) == TRUE
>        NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> +!endif
>    }
> -  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
>    MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> -  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>    MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
>    MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
>    EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
>    EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
>  
> +  #
> +  # Variable services
> +  #
> +!if $(SECURE_BOOT_ENABLE) == FALSE
> +  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> +  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
> +    <LibraryClasses>
> +      NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
> +      AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
> +      VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> +      # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
> +      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf

Would this diff be neater if this if statement moved up to the
original location of the
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
stanza?

> +  }
> +!else
> +  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf {
> +    <LibraryClasses>
> +      NULL|StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
> +  }
> +  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
> +  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> +!endif
> +
>    MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
>    MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
>    MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
> diff --git a/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc b/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
> index 87f5ee351eaa..b80379acd1ad 100644
> --- a/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
> +++ b/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
> @@ -77,6 +77,18 @@ [LibraryClasses.common.MM_STANDALONE]
>    HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
>    MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
>    MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
> +  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> +  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> +  NorFlashPlatformLib|Silicon/Qemu/SbsaQemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.inf
> +  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
> +  PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
> +  SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> +  TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> +  VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> +  SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> +  ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
>  
>  ################################################################################
>  #
> @@ -94,6 +106,20 @@ [PcdsFixedAtBuild]
>  
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x2
>  
> +  gArmTokenSpaceGuid.PcdFdBaseAddress|0x01000000
> +  gArmTokenSpaceGuid.PcdFdSize|0x000C0000
> +
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> +  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> +
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x01000000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x01040000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00040000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x01080000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
> +
>  ###################################################################################################
>  #
>  # Components Section - list of the modules and components that will be processed by compilation
> @@ -118,6 +144,19 @@ [Components.common]
>    #
>    StandaloneMmPkg/Core/StandaloneMmCore.inf
>    StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
> +  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
> +      # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
> +      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +      VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
> +      VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
> +  }
>  
>  ###################################################################################################
>  #
> diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
> index b61ae1891233..a46a47063ccc 100644
> --- a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
> +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
> @@ -21,10 +21,10 @@
>  
>  [FD.SBSA_FLASH0]
>  BaseAddress   = 0x00000000
> -Size          = 0x00400000
> +Size          = 0x01100000
>  ErasePolarity = 1
>  BlockSize     = 0x00001000
> -NumBlocks     = 0x400
> +NumBlocks     = 0x1100
>  
>  ################################################################################
>  #
> @@ -50,6 +50,66 @@ [FD.SBSA_FLASH0]
>  0x00008000|0x00300000
>  FILE = Platform/Qemu/Sbsa/fip.bin
>  
> +!if $(SECURE_BOOT_ENABLE)
> +## Place for Secure Variables.
> +# Must be aligned to Flash Block size 0x40000
> +0x01000000|0x00040000
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> +#NV_VARIABLE_STORE
> +DATA = {
> +  ## This is the EFI_FIRMWARE_VOLUME_HEADER
> +  # ZeroVector []
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  # FileSystemGuid: gEfiSystemNvDataFvGuid         =
> +  #   { 0xFFF12B8D, 0x7696, 0x4C8B,
> +  #     { 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50 }}
> +  0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
> +  0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
> +  # FvLength: 0xC0000
> +  0x00, 0x00, 0x0C, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  # Signature "_FVH"       # Attributes
> +  0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00,
> +  # HeaderLength # CheckSum # ExtHeaderOffset #Reserved #Revision
> +  0x48, 0x00, 0x28, 0x09, 0x00, 0x00, 0x00, 0x02,
> +  # Blockmap[0]: 0x3 Blocks * 0x40000 Bytes / Block
> +  0x3, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
> +  # Blockmap[1]: End
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  ## This is the VARIABLE_STORE_HEADER
> +  # It is compatible with SECURE_BOOT_ENABLE == FALSE as well.
> +  # Signature: gEfiAuthenticatedVariableGuid =
> +  #   { 0xaaf32c78, 0x947b, 0x439a,
> +  #     { 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92 }}
> +  0x78, 0x2c, 0xf3, 0xaa, 0x7b, 0x94, 0x9a, 0x43,
> +  0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92,
> +  # Size: 0x40000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) -
> +  #         0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x3ffb8
> +  # This can speed up the Variable Dispatch a bit.
> +  0xB8, 0xFF, 0x03, 0x00,
> +  # FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
> +  0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +}
> +
> +0x01040000|0x00040000
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
> +#NV_FTW_WORKING
> +DATA = {
> +  # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature = gEdkiiWorkingBlockSignatureGuid         =
> +  #  { 0x9e58292b, 0x7c68, 0x497d, { 0xa0, 0xce, 0x65,  0x0, 0xfd, 0x9f, 0x1b, 0x95 }}
> +  0x2b, 0x29, 0x58, 0x9e, 0x68, 0x7c, 0x7d, 0x49,
> +  0xa0, 0xce, 0x65,  0x0, 0xfd, 0x9f, 0x1b, 0x95,
> +  # Crc:UINT32            #WorkingBlockValid:1, WorkingBlockInvalid:1, Reserved
> +  0x5b, 0xe7, 0xc6, 0x86, 0xFE, 0xFF, 0xFF, 0xFF,
> +  # WriteQueueSize: UINT64
> +  0xE0, 0xFF, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00
> +}
> +
> +0x01080000|0x00040000
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
> +#NV_FTW_SPARE
> +!endif
> +
>  ################################################################################
>  #
>  # FD Section for FLASH1
> @@ -169,15 +229,25 @@ [FV.FvMain]
>    INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>    INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
>    INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> -  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> -  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> -  INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
>    INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
>    INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
>    INF EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
>    INF EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
>    INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>  
> +  #
> +  # Variable services
> +  #
> +!if $(SECURE_BOOT_ENABLE) == FALSE
> +  INF ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> +  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> +  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> +!else
> +  INF ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> +  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
> +  INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> +!endif
> +
>    #
>    # Multiple Console IO support
>    #
> @@ -189,7 +259,6 @@ [FV.FvMain]
>  
>    INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
>    INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
> -  INF ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
>    INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>  
>    #
> @@ -294,6 +363,7 @@ [FV.FVMAIN_COMPACT]
>    INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>    INF ArmPkg/Drivers/CpuPei/CpuPei.inf
>    INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
> +

Another spuriously added blank line.

>    INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>  
>    # IDE/AHCI Support
> diff --git a/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf b/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf
> index a1acefcfb0a7..dbe1555c68f2 100644
> --- a/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf
> +++ b/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf
> @@ -19,8 +19,8 @@
>  ################################################################################
>  
>  [FD.STANDALONE_MM]
> -BaseAddress   = 0x20001000|gArmTokenSpaceGuid.PcdFdBaseAddress
> -Size          = 0x00e00000|gArmTokenSpaceGuid.PcdFdSize # The size in bytes of the device (14MiB).
> +BaseAddress   = 0x20002000
> +Size          = 0x00e00000
>  ErasePolarity = 1
>  
>  BlockSize     = 0x00001000
> @@ -49,6 +49,9 @@ [FV.FVMAIN_COMPACT]
>  READ_LOCK_STATUS   = TRUE
>  
>    INF StandaloneMmPkg/Core/StandaloneMmCore.inf
> +  INF ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> +  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
> +  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
>    INF StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
>  
>  ################################################################################
> diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf
> index c067a80cc715..1d7f12202ecc 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf
> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf
> @@ -40,6 +40,8 @@ [Pcd]
>    gArmTokenSpaceGuid.PcdSystemMemoryBase
>    gArmTokenSpaceGuid.PcdSystemMemorySize
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
> +  gArmTokenSpaceGuid.PcdMmBufferBase
> +  gArmTokenSpaceGuid.PcdMmBufferSize
>  
>  [FixedPcd]
>    gArmTokenSpaceGuid.PcdFdBaseAddress
> diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c
> index 8c2eb0b6a028..fa164ff455f5 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c
> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c
> @@ -25,15 +25,20 @@ SbsaQemuLibConstructor (
>  {
>    VOID          *DeviceTreeBase;
>    INT32         Node, Prev;
> -  UINT64        NewBase, CurBase;
> +  UINT64        NewBase, CurBase, NsBufBase;
>    UINT64        NewSize, CurSize;
> +  UINT32        NsBufSize;
>    CONST CHAR8   *Type;
>    INT32         Len;
>    CONST UINT64  *RegProp;
>    RETURN_STATUS PcdStatus;
> +  INT32         ParentOffset;
> +  INT32         Offset;
>  
>    NewBase = 0;
>    NewSize = 0;
> +  NsBufBase = 0;
> +  NsBufSize = 0;
>  
>    DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
>    ASSERT (DeviceTreeBase != NULL);
> @@ -73,9 +78,39 @@ SbsaQemuLibConstructor (
>      }
>    }
>  
> +  // StandaloneMM non-secure shared buffer is allocated at the top of
> +  // the system memory by trusted-firmware using "/reserved-memory" node.
> +  ParentOffset = fdt_path_offset(DeviceTreeBase, "/reserved-memory");
> +  if (ParentOffset < 0) {
> +    DEBUG ((DEBUG_ERROR, "%a: reserved-memory node not found\n",
> +      __FUNCTION__));
> +  }
> +  Offset = fdt_subnode_offset(DeviceTreeBase, ParentOffset, "ns-buf-spm-mm");
> +  if (Offset < 0) {
> +    DEBUG ((DEBUG_ERROR, "%a: ns-buf-spm-mm node not found\n",
> +      __FUNCTION__));
> +  }
> +  // Get the 'reg' property of this node. 8 byte quantities for base address
> +  // and 4 byte quantities for size.
> +  RegProp = fdt_getprop (DeviceTreeBase, Offset, "reg", &Len);
> +  if (RegProp != 0 && Len == (sizeof (UINT64) + sizeof(UINT32))) {
> +    NsBufBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
> +    NsBufSize = fdt32_to_cpu (ReadUnaligned32 ((UINT32 *)(RegProp + 1)));
> +
> +    DEBUG ((DEBUG_INFO, "%a: ns buf @ 0x%lx - 0x%lx\n",
> +      __FUNCTION__, NsBufBase, NsBufBase + NsBufSize - 1));
> +  } else {
> +    DEBUG ((DEBUG_ERROR, "%a: Failed to parse FDT reserved-memory node Len %d\n",
> +      __FUNCTION__, Len));
> +  }

Could the above device-tree parsing be moved to a helper function in
Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/ ?

(Yes, I should also move the memory node parsing there, but it wasn't
quite worth creating the library just for that before.)

Best Regards,

Leif

> +
> +  NewSize -= NsBufSize;
> +
>    // Make sure the start of DRAM matches our expectation
>    ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) == NewBase);
>    PcdStatus = PcdSet64S (PcdSystemMemorySize, NewSize);
> +  PcdStatus = PcdSet64S (PcdMmBufferBase, NsBufBase);
> +  PcdStatus = PcdSet64S (PcdMmBufferSize, (UINT64)NsBufSize);
>    ASSERT_RETURN_ERROR (PcdStatus);
>  
>    return RETURN_SUCCESS;
> -- 
> 2.17.1
> 

  reply	other threads:[~2021-03-01 17:22 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-01  5:19 [PATCH edk2-platforms v2 0/4] add MM based UEFI secure boot on SbsaQemu Masahisa Kojima
2021-03-01  5:19 ` [PATCH edk2-platforms v2 1/4] SbsaQemu: Build infrastructure for StandaloneMm image Masahisa Kojima
2021-03-01 17:05   ` Leif Lindholm
2021-03-02 12:45     ` Masahisa Kojima
2021-03-02 14:13       ` Leif Lindholm
2021-03-03  0:27         ` Masahisa Kojima
2021-03-01  5:19 ` [PATCH edk2-platforms v2 2/4] SbsaQemu: add MM based UEFI secure boot support Masahisa Kojima
2021-03-01 17:22   ` Leif Lindholm [this message]
2021-03-03  6:35     ` Masahisa Kojima
2021-03-01  5:19 ` [PATCH edk2-platforms v2 3/4] SbsaQemu: add standalone MM build instruction Masahisa Kojima
2021-03-01 17:23   ` Leif Lindholm
2021-03-01  5:19 ` [PATCH edk2-platforms v2 4/4] SbsaQemu: fix typo Masahisa Kojima
2021-03-01 17:24   ` Leif Lindholm

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=20210301172216.GU1664@vanye \
    --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