Re: [PATCH 2/2 v5] StMMRpmb: Add support for building StandaloneMm image for OP-TEE


PierreGondois
 

Hello Ilias,

I would have some (inlined) remarks about your patch,

Regards,

Pierre

On 3/2/21 3:35 PM, Pierre Gondois wrote:


------------------------------------------------------------------------
*From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Ilias Apalodimas via groups.io <ilias.apalodimas@...>
*Sent:* Friday, February 12, 2021 5:34 PM
*To:* devel@edk2.groups.io <devel@edk2.groups.io>; Sami Mujawar <Sami.Mujawar@...>
*Cc:* ardb+tianocore@... <ardb+tianocore@...>; sughosh.ganu@... <sughosh.ganu@...>; leif@... <leif@...>; Ilias Apalodimas <ilias.apalodimas@...>
*Subject:* [edk2-devel] [PATCH 2/2 v5] StMMRpmb: Add support for building StandaloneMm image for OP-TEE
With some recent changes in OP-TEE [1] and U-Boot [2] we can compile StMM
and launch it from an OP-TEE secure partition which is mimicking SPM.

There's a number of advantages in this approach. In Arm world SPM,
currently used for dispatching StMM, and SPD used for OP-TEE, are
mutually exclusive. Since there's no application in OP-TEE for managing
EFI variables, this means that one can have a secure OS or secure
variable storage.

By re-using StMM we have EDK2s approved application controlling
variable storage and the ability to run a secure world OS. This also
allows various firmware implementations to adopt EDK2 way of storing
variables (including the FTW implementation), as long as OP-TEE is
available on that given platform (or any other secure OS that can launch
StMM and has a supplicant for handling the RPMB partition).
Another advantage is that OP-TEE has the ability to access an eMMC RPMB
partition to store those variables. This requires a normal world
supplicant, which is implemented in U-Boot currently. The supplicant
picks up the encrypted buffer from OP-TEE and wires it to the eMMC
driver(s). Similar functionality can be added in EDK2 by porting the
supplicant and adapt it to using the native eMMC drivers.

There's is one drawback in using OP-TEE. The current SPM calls need to run
to completion. This contradicts the current OP-TEE RPC call requirements,
used to access the RPMB storage. Thats leads to two different SMC calls for
entering secure world to access StMM.

So let's add support for a platform that compiles StMM and an RPMB
driver that communicates with OP-TEE to read/write the variables.
For anyone interested in testing this there's repo that builds all the
sources and works on QEMU [3].

[1] https://github.com/OP-TEE/optee_os/pull/3973 <https://github.com/OP-TEE/optee_os/pull/3973>
[2] http://u-boot.10912.n7.nabble.com/PATCH-0-7-v4-EFI-variable-support-via-OP-TEE-td412499.html <http://u-boot.10912.n7.nabble.com/PATCH-0-7-v4-EFI-variable-support-via-OP-TEE-td412499.html>
[3] https://git.linaro.org/people/ilias.apalodimas/efi_optee_variables.git/ <https://git.linaro.org/people/ilias.apalodimas/efi_optee_variables.git/>

Reviewed-by: Sami Mujawar <sami.mujawar@...>
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@...>
---
 Platform/StMMRpmb/PlatformStandaloneMm.dsc | 165 +++++++++++++++++++++
 Platform/StMMRpmb/PlatformStandaloneMm.fdf | 111 ++++++++++++++
 2 files changed, 276 insertions(+)
 create mode 100644 Platform/StMMRpmb/PlatformStandaloneMm.dsc
 create mode 100644 Platform/StMMRpmb/PlatformStandaloneMm.fdf

diff --git a/Platform/StMMRpmb/PlatformStandaloneMm.dsc b/Platform/StMMRpmb/PlatformStandaloneMm.dsc
new file mode 100644
index 000000000000..1f1ddf968c1f
--- /dev/null
+++ b/Platform/StMMRpmb/PlatformStandaloneMm.dsc
@@ -0,0 +1,165 @@
+#

+#  Copyright (c) 2018, ARM Limited. All rights reserved.

+#  Copyright (c) 2020, Linaro Ltd. All rights reserved.

+#

+#  SPDX-License-Identifier: BSD-2-Clause-Patent

+#

+

+################################################################################

+#

+# Defines Section - statements that will be processed to create a Makefile.

+#

+################################################################################

+[Defines]

+  PLATFORM_NAME                  = MmStandaloneRpmb

+  PLATFORM_GUID                  = A27A486E-D7B9-4D70-9F37-FED9ABE041A2

+  PLATFORM_VERSION               = 1.0

+  DSC_SPECIFICATION              = 0x00010011
I think we are at the revision 0x0001001C, cf https://edk2-docs.gitbook.io/edk-ii-dsc-specification/3_edk_ii_dsc_file_format/35_-defines-_section


+  OUTPUT_DIRECTORY               = Build/$(PLATFORM_NAME)

+  SUPPORTED_ARCHITECTURES        = AARCH64

+  BUILD_TARGETS                  = DEBUG|RELEASE|NOOPT

+  SKUID_IDENTIFIER               = DEFAULT

+  FLASH_DEFINITION               = Platform/StMMRpmb/PlatformStandaloneMm.fdf

+  DEFINE DEBUG_MESSAGE           = TRUE

+

+  # LzmaF86

+  DEFINE COMPRESSION_TOOL_GUID   = D42AE6BD-1352-4bfb-909A-CA72A6EAE889

+

+################################################################################

+#

+# Library Class section - list of all Library Classes needed by this Platform.

+#

+################################################################################

+[LibraryClasses]

+  ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf

+  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf

+  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf

+ SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf

+ VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf

+ BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf

+ DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf

+ ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf

+  FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf

+ HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf

+ IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf

+ MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf

+ MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.inf

+  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf

+ PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf

+  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf

+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf

+ ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf

+

+  #

+  # Entry point

+  #

+ StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf

+ StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf

+

+ StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf

+ CacheMaintenanceLib|MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.inf

+ PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf

+  RngLib|MdePkg/Library/BaseRngLibNull/BaseRngLibNull.inf

+

+ SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf

+ DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
It seems in the previous patch you are using some 'DEBUG ()' and 'ASSERT () statements. Wouldn't using BaseDebugLibNull and BaseSerialPortLibNull make them useless for DEBUG and RELEASE build ?

+

+  #

+  # It is not possible to prevent the ARM compiler for generic intrinsic functions.

+  # This library provides the intrinsic functions generate by a given compiler.

+  # NULL means link this library into all ARM images.

+  #

+ NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf

+

+[LibraryClasses.common.MM_STANDALONE]

+ HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf

+ MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf

+ MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf

+

+ IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.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

+################################################################################

+#

+# Pcd Section - list of all EDK II PCD Entries defined by this Platform

+#

+################################################################################
Since this comment is for the PCD section, I think it would be better to remove the empty line after the comment and add one at the top of this comment.
+

+[PcdsFeatureFlag.common]

+  gArmTokenSpaceGuid.PcdFfaEnable|TRUE

+

+[PcdsFixedAtBuild]

+ gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x800000CF

+  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff

+ gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x0f

+

+ gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x2

+  # Secure Storage

+ gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE

+ gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000

+ gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800

+

+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00004000

+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00004000

+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00004000

+ gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x00004000

+

+[PcdsPatchableInModule]

+  # Allocated memory for EDK2 uppers layers

+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0x0

+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0x0

+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0x0

+

+###################################################################################################

+#

+# Components Section - list of the modules and components that will be processed by compilation

+#                      tools and the EDK II tools to generate PE32/PE32+/Coff image files.

+#

+# Note: The EDK II DSC file is not used to specify how compiled binary images get placed

+#       into firmware volume images. This section is just a list of modules to compile from

+#       source into UEFI-compliant binaries.

+#       It is the FDF file that contains information on combining binary files into firmware

+#       volume images, whose concept is beyond UEFI and is described in PI specification.

+#       Binary modules do not need to be listed in this section, as they should be

+#       specified in the FDF file. For example: Shell binary (Shell_Full.efi), FAT binary (Fat.efi),

+#       Logo (Logo.bmp), and etc.

+#       There may also be modules listed in this section that are not required in the FDF file,

+#       When a module listed here is excluded from FDF file, then UEFI-compliant binary will be

+#       generated for it, but the binary will not be put into any firmware volume.

+#

+###################################################################################################

+[Components.common]

+  #

+  # Standalone MM components

+  #

+  Drivers/OpTeeRpmb/OpTeeRpmbFv.inf

+  StandaloneMmPkg/Core/StandaloneMmCore.inf

+ StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf

+ MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf {

+    <LibraryClasses>

+      NULL|Drivers/OpTeeRpmb/FixupPcd.inf

+  }

+ MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf {

+    <LibraryClasses>

+ AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf

+ BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf

+ DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf

+ VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf

+ NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf

+      NULL|Drivers/OpTeeRpmb/FixupPcd.inf

+  }

+

+###################################################################################################

+#

+# BuildOptions Section - Define the module specific tool chain flags that should be used asOn 3/2/21 4:43 PM, Pierre wrote:

+#                        the default flags for a module. These flags are appended to any

+#                        standard flags that are defined by the build process. They can be

+#                        applied for any modules or only those modules with the specific

+#                        module style (EDK or EDKII) specified in [Components] section.

+#

+###################################################################################################

+[BuildOptions.AARCH64]

+GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000 -march=armv8-a+nofp

+GCC:*_*_*_CC_FLAGS = -mstrict-align

diff --git a/Platform/StMMRpmb/PlatformStandaloneMm.fdf b/Platform/StMMRpmb/PlatformStandaloneMm.fdf
new file mode 100644
index 000000000000..febc6d0d959b
--- /dev/null
+++ b/Platform/StMMRpmb/PlatformStandaloneMm.fdf
@@ -0,0 +1,111 @@
+#

+#  Copyright (c) 2018, ARM Limited. All rights reserved.

+#  Copyright (c) 2020, Linaro Ltd. All rights reserved.

+#

+#  SPDX-License-Identifier: BSD-2-Clause-Patent

+#

+

+################################################################################

+#

+# FD Section

+# The [FD] Section is made up of the definition statements and a

+# description of what goes into  the Flash Device Image. Each FD section

+# defines one flash "device" image.  A flash device image may be one of

+# the following: Removable media bootable image (like a boot floppy

+# image,) an Option ROM image (that would be "flashed" into an add-in

+# card,) a System "Flash"  image (that would be burned into a system's

+# flash) or an Update ("Capsule") image that will be used to update and

+# existing system flash.

+#

+################################################################################

+

+[FD.BL32_AP_MM]

+BaseAddress   = 0x1000 # any address apart from 0x0

+Size          = 0x00300000

+ErasePolarity = 1

+

+BlockSize     = 0x00001000

+NumBlocks     = 0x0300

+

+################################################################################

+#

+# Following are lists of FD Region layout which correspond to the locations of different

+# images within the flash device.

+#

+# Regions must be defined in ascending order and may not overlap.

+#

+# A Layout Region start with a eight digit hex offset (leading "0x" required) followed by

+# the pipe "|" character, followed by the size of the region, also in hex with the leading

+# "0x" characters. Like:

+# Offset|Size

+# PcdOffsetCName|PcdSizeCName

+# RegionType <FV, DATA, or FILE>

+#

+################################################################################

+

+0x00000000|0x00280000

+FV = FVMAIN_COMPACT

+

+[FV.FVMAIN_COMPACT]

+FvAlignment        = 8

+ERASE_POLARITY     = 1

+MEMORY_MAPPED      = TRUE

+STICKY_WRITE       = TRUE

+LOCK_CAP           = TRUE

+LOCK_STATUS        = TRUE

+WRITE_DISABLED_CAP = TRUE

+WRITE_ENABLED_CAP  = TRUE

+WRITE_STATUS       = TRUE

+WRITE_LOCK_CAP     = TRUE

+WRITE_LOCK_STATUS  = TRUE

+READ_DISABLED_CAP  = TRUE

+READ_ENABLED_CAP   = TRUE

+READ_STATUS        = TRUE

+READ_LOCK_CAP      = TRUE

+READ_LOCK_STATUS   = TRUE

+

+  INF StandaloneMmPkg/Core/StandaloneMmCore.inf

+  INF Drivers/OpTeeRpmb/OpTeeRpmbFv.inf

+  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf

+  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf

+  INF StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf

+################################################################################

+#

+# Rules are use with the [FV] section's module INF type to define

+# how an FFS file is created for a given INF file. The following Rule are the default

+# rules for the different module type. User can add the customized rules to define the

+# content of the FFS file.

+#

+################################################################################

+

+

+############################################################################

+# Example of a DXE_DRIVER FFS file with a Checksum encapsulation section   #

+############################################################################

+#

+#[Rule.Common.DXE_DRIVER]

+#  FILE DRIVER = $(NAMED_GUID) {

+#    DXE_DEPEX    DXE_DEPEX               Optional $(INF_OUTPUT)/$(MODULE_NAME).depex

+#    COMPRESS PI_STD {

+#      GUIDED {

+#        PE32     PE32 $(INF_OUTPUT)/$(MODULE_NAME).efi

+#        UI       STRING="$(MODULE_NAME)" Optional

+#        VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)

+#      }

+#    }

+#  }

+#

+############################################################################

+

+[Rule.Common.MM_CORE_STANDALONE]

+  FILE SEC = $(NAMED_GUID) FIXED {

+    PE32  PE32 Align = Auto $(INF_OUTPUT)/$(MODULE_NAME).efi

+  }

+

+[Rule.Common.MM_STANDALONE]

+  FILE MM_STANDALONE = $(NAMED_GUID) {

+    SMM_DEPEX SMM_DEPEX Optional $(INF_OUTPUT)/$(MODULE_NAME).depex

+    PE32      PE32 $(INF_OUTPUT)/$(MODULE_NAME).efi

+    UI        STRING="$(MODULE_NAME)" Optional

+    VERSION   STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)

+  }

--
2.30.0



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71652): https://edk2.groups.io/g/devel/message/71652 <https://edk2.groups.io/g/devel/message/71652>
Mute This Topic: https://groups.io/mt/80588995/1821310 <https://groups.io/mt/80588995/1821310>
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub <https://edk2.groups.io/g/devel/unsub> [pierre.gondois@...]
-=-=-=-=-=-=

Join devel@edk2.groups.io to automatically receive all group messages.