Re: [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree

["Ard Biesheuvel" <ardb@kernel.org>]

On Fri, 30 Jul 2021 at 19:35, Sayanta Pattanayak
<sayanta.pattanayak@arm.com> wrote:
>
> Introduce support to populate StMM boot data via DTS parsing.

Why? Don't we have FF-A manifests for this? I would expect the secure
partition manager to marshall this data into the appropriate format
when necessary.

> The DTB is
> passed as a boot argument by a binary of higer exception level.
> Previously it was achieved by placing the boot data structure in a
> shared buffer and the address of this shared buffer was passed by the
> binary of higher exception level. Now either of the option can be used
> for populating StMM boot info.
>
> StMM boot information structure binding in device tree can be of following
> prototype. Property values are not mentioned here.
>
> bootarg {
>   compatible = "bootargs";
>   h_type  = <..>;
>   h_version = <..>;
>   h_size    = <..>;
>   h_attr    = <..>;
>   sp_mem_base         = <..>;
>   sp_mem_limit        = <..>;
>   sp_image_base       = <..>;
>   sp_stack_base       = <..>;
>   sp_heap_base        = <..>;
>   sp_ns_comm_buf_base = <..>;
>   sp_shared_buf_base  = <..>;
>   sp_image_size       = <..>;
>   sp_pcpu_stack_size  = <..>;
>   sp_heap_size        = <..>;
>   sp_ns_comm_buf_size = <..>;
>   sp_shared_buf_size  = <..>;
>   num_sp_mem_regions  = <..>;
>   num_cpus            = <..>;
> };
>
> Addition of DTS supoort involves a dependency on FdtLib from EmbeddedPkg.
>
> Signed-off-by: Sayanta Pattanayak <sayanta.pattanayak@arm.com>

I don't think we should apply this change. DT is not part of the
original SPM or current FF-A spec, right? So please fix this in the
S-EL1 component instead.


> ---
>  Link to github branch with this patch -
>  https://github.com/SayantaP-arm/edk2/tree/stmm-dts
>
>  StandaloneMmPkg/StandaloneMmPkg.dsc                                                     |   1 +
>  StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf       |   3 +
>  StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c | 153 ++++++++++++++++++--
>  3 files changed, 143 insertions(+), 14 deletions(-)
>
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc b/StandaloneMmPkg/StandaloneMmPkg.dsc
> index 0c45df95e2dd..e3a3a6ee3ba1 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dsc
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
> @@ -49,6 +49,7 @@
>    HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
>    IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>    MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
> +  FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>    MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.inf
>    MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> index 4fa426f58ef4..0a2e519dd664 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> @@ -30,6 +30,7 @@
>    X64/StandaloneMmCoreEntryPoint.c
>
>  [Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
>    StandaloneMmPkg/StandaloneMmPkg.dec
> @@ -40,10 +41,12 @@
>  [LibraryClasses]
>    BaseLib
>    DebugLib
> +  FdtLib
>
>  [LibraryClasses.AARCH64]
>    StandaloneMmMmuLib
>    ArmSvcLib
> +  FdtLib
>
>  [Guids]
>    gMpInformationHobGuid
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> index 6c50f470aa35..cc09d75dac36 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> @@ -16,6 +16,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Guid/MmramMemoryReserve.h>
>  #include <Guid/MpInformation.h>
>
> +#include <libfdt.h>
>  #include <Library/ArmMmuLib.h>
>  #include <Library/ArmSvcLib.h>
>  #include <Library/DebugLib.h>
> @@ -45,33 +46,31 @@ STATIC CONST UINT32 mSpmMinorVerFfa = SPM_MINOR_VERSION_FFA;
>  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT      CpuDriverEntryPoint = NULL;
>
>  /**
> -  Retrieve a pointer to and print the boot information passed by privileged
> -  secure firmware.
> +  Prints boot information.
>
> -  @param  [in] SharedBufAddress   The pointer memory shared with privileged
> -                                  firmware.
> +  This function prints the boot information, which is passed by privileged
> +  secure firmware through shared buffer or other mechanism.
>
> +  @param  [in] PayloadBootInfo   Pointer to StandaloneMM Boot Info structure.
>  **/
> -EFI_SECURE_PARTITION_BOOT_INFO *
> -GetAndPrintBootinformation (
> -  IN VOID                      *SharedBufAddress
> +VOID
> +PrintBootinformation (
> +  IN EFI_SECURE_PARTITION_BOOT_INFO *PayloadBootInfo
>  )
>  {
> -  EFI_SECURE_PARTITION_BOOT_INFO *PayloadBootInfo;
>    EFI_SECURE_PARTITION_CPU_INFO  *PayloadCpuInfo;
>    UINTN                          Index;
>
> -  PayloadBootInfo = (EFI_SECURE_PARTITION_BOOT_INFO *) SharedBufAddress;
>
>    if (PayloadBootInfo == NULL) {
>      DEBUG ((DEBUG_ERROR, "PayloadBootInfo NULL\n"));
> -    return NULL;
> +    return;
>    }
>
>    if (PayloadBootInfo->Header.Version != BOOT_PAYLOAD_VERSION) {
>      DEBUG ((DEBUG_ERROR, "Boot Information Version Mismatch. Current=0x%x, Expected=0x%x.\n",
>              PayloadBootInfo->Header.Version, BOOT_PAYLOAD_VERSION));
> -    return NULL;
> +    return;
>    }
>
>    DEBUG ((DEBUG_INFO, "NumSpMemRegions - 0x%x\n", PayloadBootInfo->NumSpMemRegions));
> @@ -96,7 +95,7 @@ GetAndPrintBootinformation (
>
>    if (PayloadCpuInfo == NULL) {
>      DEBUG ((DEBUG_ERROR, "PayloadCpuInfo NULL\n"));
> -    return NULL;
> +    return;
>    }
>
>    for (Index = 0; Index < PayloadBootInfo->NumCpus; Index++) {
> @@ -105,7 +104,7 @@ GetAndPrintBootinformation (
>      DEBUG ((DEBUG_INFO, "Flags           - 0x%x\n", PayloadCpuInfo[Index].Flags));
>    }
>
> -  return PayloadBootInfo;
> +  return;
>  }
>
>  /**
> @@ -194,6 +193,119 @@ DelegatedEventLoop (
>    }
>  }
>
> +/**
> +  Populates StandAloneMM boot information structure.
> +
> +  This function receives dtb Address, where StMM Boot information specific
> +  properties will be looked out to form the booting structure of type
> +  EFI_SECURE_PARTITION_BOOT_INFO. At first, the properties for StandAloneMM
> +  ConfigSize and  Memory limit will be checked out. Boot information will
> +  be stored at address (Memory Limit - ConfigSize). Thereafter all boot
> +  information specific properties will be parsed and corresponding values
> +  will be obtained.
> +
> +  @param  [out] BootInfo   Pointer, where Boot Info structure will be populated.
> +  @param  [in] DtbAddress  Address of the Device tree from where Boot
> +                           information will be fetched.
> +**/
> +VOID
> +PopulateBootinformation (
> +  OUT EFI_SECURE_PARTITION_BOOT_INFO **BootInfo,
> +  IN VOID   *DtbAddress
> +)
> +{
> +  INT32           Offset;
> +  CONST UINT32    *Property;
> +  CONST UINT64    *Property64;
> +  UINT32          ConfigSize;
> +  UINT64          SpMemLimit;
> +  EFI_SECURE_PARTITION_BOOT_INFO *PayloadBootInfo;
> +
> +  Offset = fdt_node_offset_by_compatible (DtbAddress, -1, "config-size");
> +  if (Offset < 0) {
> +    DEBUG ((DEBUG_WARN, "Total Config Size is not  defined\n"));
> +  } else {
> +    Property = fdt_getprop (DtbAddress, Offset, "size", NULL);
> +    if (Property) {
> +      ConfigSize = fdt32_to_cpu (*Property);
> +      DEBUG ((DEBUG_INFO, "stmm dtb config-size  = 0x%x \n", ConfigSize));
> +    }
> +  }
> +
> +  Offset = fdt_node_offset_by_compatible (DtbAddress, -1, "bootargs");
> +  if (Offset >= 0) {
> +    Property64 =  fdt_getprop (DtbAddress, Offset, "sp_mem_limit", NULL);
> +    SpMemLimit = fdt64_to_cpu (*Property64);
> +  }
> +
> +  if (SpMemLimit && ConfigSize)
> +    PayloadBootInfo =
> +      (EFI_SECURE_PARTITION_BOOT_INFO *)(SpMemLimit - ConfigSize);
> +
> +  if (PayloadBootInfo) {
> +    PayloadBootInfo->SpMemLimit = SpMemLimit;
> +
> +    Property =  fdt_getprop (DtbAddress, Offset, "h_type", NULL);
> +    PayloadBootInfo->Header.Type = (UINT8) fdt32_to_cpu(*Property);
> +
> +    Property =  fdt_getprop (DtbAddress, Offset, "h_version", NULL);
> +    PayloadBootInfo->Header.Version = (UINT8) fdt32_to_cpu(*Property);
> +
> +    Property =  fdt_getprop (DtbAddress, Offset, "h_size", NULL);
> +    PayloadBootInfo->Header.Size = (UINT8) fdt32_to_cpu(*Property);
> +
> +    Property =  fdt_getprop (DtbAddress, Offset, "h_attr", NULL);
> +    PayloadBootInfo->Header.Attr = fdt32_to_cpu(*Property);
> +
> +    Property64 =  fdt_getprop (DtbAddress, Offset, "sp_mem_base", NULL);
> +    PayloadBootInfo->SpMemBase = fdt64_to_cpu(*Property64);
> +
> +    Property64 =  fdt_getprop (DtbAddress, Offset, "sp_image_base", NULL);
> +    PayloadBootInfo->SpImageBase = fdt64_to_cpu(*Property64);
> +
> +    Property64 =  fdt_getprop (DtbAddress, Offset, "sp_stack_base", NULL);
> +    PayloadBootInfo->SpStackBase = fdt64_to_cpu(*Property64);
> +
> +    Property64 =  fdt_getprop (DtbAddress, Offset, "sp_heap_base", NULL);
> +    PayloadBootInfo->SpHeapBase = fdt64_to_cpu(*Property64);
> +
> +    Property64 =  fdt_getprop (DtbAddress, Offset, "sp_ns_comm_buf_base", NULL);
> +    PayloadBootInfo->SpNsCommBufBase = fdt64_to_cpu(*Property64);
> +
> +    Property64 =  fdt_getprop (DtbAddress, Offset, "sp_shared_buf_base", NULL);
> +    PayloadBootInfo->SpSharedBufBase = fdt64_to_cpu(*Property64);
> +
> +    Property64 =  fdt_getprop (DtbAddress, Offset, "sp_image_size", NULL);
> +    PayloadBootInfo->SpImageSize = fdt64_to_cpu(*Property64);
> +
> +    Property64 =  fdt_getprop (DtbAddress, Offset, "sp_pcpu_stack_size", NULL);
> +    PayloadBootInfo->SpPcpuStackSize = fdt64_to_cpu(*Property64);
> +
> +    Property64 =  fdt_getprop (DtbAddress, Offset, "sp_heap_size", NULL);
> +    PayloadBootInfo->SpHeapSize = fdt64_to_cpu(*Property64);
> +
> +    Property64 =  fdt_getprop (DtbAddress, Offset, "sp_ns_comm_buf_size", NULL);
> +    PayloadBootInfo->SpNsCommBufSize = fdt64_to_cpu(*Property64);
> +
> +    Property64 =  fdt_getprop (DtbAddress, Offset, "sp_shared_buf_size", NULL);
> +    PayloadBootInfo->SpPcpuSharedBufSize = fdt64_to_cpu(*Property64);
> +
> +    Property =  fdt_getprop (DtbAddress, Offset, "num_sp_mem_regions", NULL);
> +    PayloadBootInfo->NumSpMemRegions = fdt32_to_cpu(*Property);
> +
> +    Property =  fdt_getprop (DtbAddress, Offset, "num_cpus", NULL);
> +    PayloadBootInfo->NumCpus = fdt32_to_cpu(*Property);
> +
> +    PayloadBootInfo->CpuInfo =
> +      (EFI_SECURE_PARTITION_CPU_INFO *)((UINT64)PayloadBootInfo +
> +                                        sizeof(EFI_SECURE_PARTITION_BOOT_INFO));
> +  }
> +
> +  *BootInfo = PayloadBootInfo;
> +
> +  return;
> +}
> +
>  /**
>    Query the SPM version, check compatibility and return success if compatible.
>
> @@ -313,6 +425,7 @@ _ModuleEntryPoint (
>    VOID                                    *TeData;
>    UINTN                                   TeDataSize;
>    EFI_PHYSICAL_ADDRESS                    ImageBase;
> +  VOID                                    *DtbAddress;
>
>    // Get Secure Partition Manager Version Information
>    Status = GetSpmVersion ();
> @@ -320,12 +433,24 @@ _ModuleEntryPoint (
>      goto finish;
>    }
>
> -  PayloadBootInfo = GetAndPrintBootinformation (SharedBufAddress);
> +  // In cookie1 the DTB address is passed. With reference to DTB, Boot
> +  // info structure can be populated.
> +  // If cookie1 doesn't have any value, then Boot info is copied from
> +  // Sharedbuffer.
> +  if (cookie1) {
> +    DtbAddress = (void *)cookie1;
> +    PopulateBootinformation (&PayloadBootInfo, DtbAddress);
> +  } else {
> +    PayloadBootInfo = (EFI_SECURE_PARTITION_BOOT_INFO *)SharedBufAddress;
> +  }
> +
>    if (PayloadBootInfo == NULL) {
>      Status = EFI_UNSUPPORTED;
>      goto finish;
>    }
>
> +  PrintBootinformation (PayloadBootInfo);
> +
>    // Locate PE/COFF File information for the Standalone MM core module
>    Status = LocateStandaloneMmCorePeCoffData (
>               (EFI_FIRMWARE_VOLUME_HEADER *) PayloadBootInfo->SpImageBase,
> --
> 2.17.1
>