[edk2-devel] [edk2-platforms][PATCH V1 09/20] StandaloneMmPkg: parse SP manifest and populate new boot information
Chris Fernald
chris.fernald at outlook.com
Thu Jul 13 16:48:24 UTC 2023
Achin/Nishant, could you explain the motivation behind falling back to
device tree for some of the secure partition information? It seems like
we have this large abstraction framework using FF-A and it seems a bit
odd to have the secure partition have to directly read device tree for
some of this information when everything else is query-able from the
framework itself.
Thanks,
Chris Fernald
On 7/13/2023 8:24 AM, Girish Mahadevan via groups.io wrote:
> I had one comment , in-line.
>
> Thanks
> Girish
>
> On 7/11/2023 8:36 AM, Nishant Sharma via groups.io wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> From: Achin Gupta <achin.gupta at arm.com>
>>
>> This patch discovers the SP manifest in DT format passed by the SPMC. It
>> then parses it to obtain the boot information required to initialise the
>> SP.
>>
>> Signed-off-by: Achin Gupta <achin.gupta at arm.com>
>> Signed-off-by: Sayanta Pattanayak <sayanta.pattanayak at arm.com>
>> Signed-off-by: Nishant Sharma <nishant.sharma at arm.com>
>> ---
>> StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h |
>> 2 +-
>> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
>> | 389 +++++++++++++++++++-
>> 2 files changed, 381 insertions(+), 10 deletions(-)
>>
>> diff --git
>> a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
>> b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
>> index c965192c702e..90d67a2f25b5 100644
>> --- a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
>> +++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
>> @@ -2,7 +2,7 @@
>> Entry point to the Standalone MM Foundation when initialized
>> during the SEC
>> phase on ARM platforms
>>
>> -Copyright (c) 2017 - 2021, Arm Ltd. All rights reserved.<BR>
>> +Copyright (c) 2017 - 2023, Arm Ltd. All rights reserved.<BR>
>> SPDX-License-Identifier: BSD-2-Clause-Patent
>>
>> **/
>> diff --git
>> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
>> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
>>
>> index 9f6af55c86c4..505786aff07c 100644
>> ---
>> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
>> +++
>> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
>> @@ -38,6 +38,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>>
>> #define BOOT_PAYLOAD_VERSION 1
>>
>> +#define FFA_PAGE_4K 0
>> +#define FFA_PAGE_16K 1
>> +#define FFA_PAGE_64K 2
>> +
>> PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT CpuDriverEntryPoint = NULL;
>>
>> /**
>> @@ -106,6 +110,7 @@ GetAndPrintBootinformation (
>> }
>>
>> return PayloadBootInfo;
>> +}
>>
>> /**
>> An StMM SP implements partial support for FF-A v1.0. The FF-A
>> ABIs are used to
>> @@ -266,6 +271,308 @@ DelegatedEventLoop (
>> }
>> }
>>
>> +STATIC
>> +BOOLEAN
>> +CheckDescription (
>> + IN VOID * DtbAddress,
>> + IN INT32 Offset,
>> + OUT CHAR8 * Description,
>> + OUT UINT32 Size
>> + )
>> +{
>> + CONST CHAR8 * Property;
>> + INT32 LenP;
>> +
>> + Property = fdt_getprop (DtbAddress, Offset, "description", &LenP);
>> + if (Property == NULL) {
>> + return FALSE;
>> + }
>> +
>> + return CompareMem (Description, Property, MIN(Size, (UINT32)LenP))
>> == 0;
>> +
>> +}
>> +
>> +STATIC
>> +EFI_STATUS
>> +ReadProperty32 (
>> + IN VOID * DtbAddress,
>> + IN INT32 Offset,
>> + IN CHAR8 * Property,
>> + OUT UINT32 * Value
>> + )
>> +{
>> + CONST UINT32 * Property32;
>> +
>> + Property32 = fdt_getprop (DtbAddress, Offset, Property, NULL);
>> + if (Property32 == NULL) {
>> + DEBUG ((
>> + DEBUG_ERROR,
>> + "%s: Missing in FF-A boot information manifest\n",
>> + Property
>> + ));
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> + *Value = fdt32_to_cpu (*Property32);
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> +STATIC
>> +EFI_STATUS
>> +ReadProperty64 (
>> + IN VOID * DtbAddress,
>> + IN INT32 Offset,
>> + IN CHAR8 * Property,
>> + OUT UINT64 * Value
>> + )
>> +{
>> + CONST UINT64 * Property64;
>> +
>> + Property64 = fdt_getprop (DtbAddress, Offset, Property, NULL);
>> + if (Property64 == NULL) {
>> + DEBUG ((
>> + DEBUG_ERROR,
>> + "%s: Missing in FF-A boot information manifest\n",
>> + Property
>> + ));
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> + *Value = fdt64_to_cpu (*Property64);
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> +STATIC
>> +BOOLEAN
>> +ReadRegionInfo (
>> + IN VOID *DtbAddress,
>> + IN INT32 Node,
>> + IN CHAR8 *Region,
>> + IN UINTN RegionStrSize,
>> + IN UINT32 PageSize,
>> + OUT UINT64 *Address,
>> + OUT UINT64 *Size
>> + )
>> +{
>> + BOOLEAN FoundBuffer;
>> + INTN Status = 0;
>> +
>> + FoundBuffer = CheckDescription (
>> + DtbAddress,
>> + Node,
>> + Region,
>> + RegionStrSize
>> + );
>> + if (!FoundBuffer) {
>> + return FALSE;
>> + }
>> +
>> + DEBUG ((DEBUG_INFO, "Found Node: %a\n", Region));
>> + Status = ReadProperty64 (
>> + DtbAddress,
>> + Node,
>> + "base-address",
>> + Address
>> + );
>> + if (Status != EFI_SUCCESS) {
>> + DEBUG ((DEBUG_ERROR, "base-address missing in DTB"));
>> + return FALSE;
>> + }
>> + DEBUG ((
>> + DEBUG_INFO,
>> + "base = 0x%llx\n",
>> + *Address
>> + ));
>> +
>> + Status = ReadProperty32 (
>> + DtbAddress,
>> + Node,
>> + "pages-count",
>> + (UINT32*)Size
>> + );
>> + if (Status != EFI_SUCCESS) {
>> + DEBUG ((DEBUG_ERROR, "pages-count missing in DTB"));
>> + return FALSE;
>> + }
>> +
>> + DEBUG ((DEBUG_ERROR, "pages-count: 0x%lx\n", *Size));
>> +
>> + *Size = *Size * PageSize;
>> + DEBUG ((
>> + DEBUG_INFO,
>> + "Size = 0x%llx\n",
>> + *Size
>> + ));
>> +
>> + return TRUE;
>> +}
>> +
>> +/**
>> +
>> + Populates FF-A boot information structure.
>> +
>> + This function receives the address of a DTB from which boot
>> information defind
>> + by FF-A and required to initialize the standalone environment is
>> extracted.
>> +
>> + @param [in, out] StmmBootInfo Pointer to a pre-allocated boot
>> info structure to be
>> + populated.
>> + @param [in] DtbAddress Address of the Device tree from
>> where boot
>> + information will be fetched.
>> +**/
>> +STATIC
>> +EFI_STATUS
>> +PopulateBootinformation (
>> + IN OUT EFI_STMM_BOOT_INFO *StmmBootInfo,
>> + IN VOID *DtbAddress
>> +)
>> +{
>> + INTN Status;
>> + INT32 Offset;
>> + INT32 Node;
>> + BOOLEAN FoundNsCommBuffer = FALSE;
>> + BOOLEAN FoundSharedBuffer = FALSE;
>> + BOOLEAN FoundHeap = FALSE;
>> + UINT32 PageSize;
>> +
>> + Offset = fdt_node_offset_by_compatible (DtbAddress, -1,
>> "arm,ffa-manifest-1.0");
>> + DEBUG ((DEBUG_INFO, "Offset = %d \n", Offset));
>> + if (Offset < 0) {
>> + DEBUG ((DEBUG_ERROR, "Missing FF-A boot information in
>> manifest\n"));
>> + return EFI_NOT_FOUND;
>> + }
>> +
>> + Status = ReadProperty64 (
>> + DtbAddress,
>> + Offset,
>> + "load-address",
>> + &StmmBootInfo->SpMemBase
>> + );
>> + if (Status != EFI_SUCCESS) {
>> + return Status;
>> + }
>> + DEBUG ((DEBUG_INFO, "sp mem base = 0x%llx\n",
>> StmmBootInfo->SpMemBase));
>> +
>> + Status = ReadProperty64 (
>> + DtbAddress,
>> + Offset,
>> + "image-size",
>> + &StmmBootInfo->SpMemSize
>> + );
>> + if (Status != EFI_SUCCESS) {
>> + return Status;
>> + }
>> + DEBUG ((DEBUG_INFO, "sp mem size = 0x%llx\n",
>> StmmBootInfo->SpMemSize));
>> +
>> + Status = ReadProperty32 (DtbAddress, Offset, "xlat-granule",
>> &PageSize);
>> + if (Status != EFI_SUCCESS) {
>> + return Status;
>> + }
>> +
>> + /* EFI_PAGE_SIZE is 4KB */
>> + switch (PageSize) {
>> + case FFA_PAGE_4K:
>> + PageSize = EFI_PAGE_SIZE;
>> + break;
>> +
>> + case FFA_PAGE_16K:
>> + PageSize = 4 * EFI_PAGE_SIZE;
>> + break;
>> +
>> + case FFA_PAGE_64K:
>> + PageSize = 16 * EFI_PAGE_SIZE;
>> + break;
>> +
>> + default:
>> + DEBUG ((DEBUG_ERROR, "Invalid page type = %lu\n", PageSize));
>> + return EFI_INVALID_PARAMETER;
>> + break;
>> + };
>> +
>> + DEBUG ((DEBUG_INFO, "Page Size = 0x%lx\n", PageSize));
>> +
>> + Offset = fdt_subnode_offset_namelen (
>> + DtbAddress,
>> + Offset,
>> + "memory-regions",
>> + sizeof("memory-regions") - 1
>> + );
>> + if (Offset < 1) {
>> + DEBUG ((
>> + DEBUG_ERROR,
>> + "%s: Missing in FF-A boot information manifest\n",
>> + "memory-regions"
>> + ));
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> + for (
>> + Node = fdt_first_subnode (DtbAddress, Offset);
>> + Node > 0;
>> + Node = fdt_next_subnode (DtbAddress, Node)) {
>> + if (!FoundNsCommBuffer) {
>> + FoundNsCommBuffer = ReadRegionInfo (
>> + DtbAddress,
>> + Node,
>> + "ns-comm",
>> + sizeof ("ns-comm") - 1,
>> + PageSize,
>> + &StmmBootInfo->SpNsCommBufBase,
>> + &StmmBootInfo->SpNsCommBufSize
>> + );
>> + }
>> +
>> + if (!FoundHeap) {
>> + FoundHeap = ReadRegionInfo (
>> + DtbAddress,
>> + Node,
>> + "heap",
>> + sizeof ("heap") - 1,
>> + PageSize,
>> + &StmmBootInfo->SpHeapBase,
>> + &StmmBootInfo->SpHeapSize
>> + );
>> + }
>> +
>> + if (!FoundSharedBuffer) {
>> + FoundSharedBuffer = ReadRegionInfo (
>> + DtbAddress,
>> + Node,
>> + "shared-buff",
>> + sizeof ("shared-buff") - 1,
>> + PageSize,
>> + &StmmBootInfo->SpSharedBufBase,
>> + &StmmBootInfo->SpSharedBufSize
>> + );
>> + }
>> + }
>> +
>> + if (!FoundNsCommBuffer) {
>> + DEBUG ((DEBUG_ERROR, "Failed to find ns-comm buffer info\n"));
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> + if (!FoundHeap) {
>> + DEBUG ((DEBUG_ERROR, "Failed to find heap buffer info\n"));
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> + if (!FoundSharedBuffer) {
>> + DEBUG ((DEBUG_ERROR, "Failed to find shared buffer info\n"));
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> + // Populate CPU information under the assumption made in the FF-A
>> spec that
>> + // this is a uniprocessor SP that is capable of migration. So, it
>> is fine if
>> + // it sees 0 as both its physical and linear cpu id
>> + StmmBootInfo->CpuInfo.Mpidr = 0;
>> + StmmBootInfo->CpuInfo.LinearId = 0;
>> + StmmBootInfo->CpuInfo.Flags = 0;
>> +
>> + return EFI_SUCCESS;
>> +}
>
> [GM]
> Outside of the mandatory properties, I expect that there could be some
> custom fields in the dtb manifest. Can you add a means for a vendor to
> hook in parsing properties that only apply to them. An OEM Library ?
>> +
>> /**
>> Query the SPM version, check compatibility and return success if
>> compatible.
>>
>> @@ -343,6 +650,49 @@ InitArmSvcArgs (
>> }
>> }
>>
>> +
>> +STATIC
>> +EFI_STATUS
>> +GetSpManifest (
>> + IN OUT UINT64 **SpManifestAddr,
>> + IN VOID *BootInfoAddr
>> + )
>> +{
>> + EFI_FFA_BOOT_INFO_HEADER *FfaBootInfo;
>> + EFI_FFA_BOOT_INFO_DESC *FfaBootInfoDesc;
>> +
>> + // Paranoid check to avoid an inadvertent NULL pointer dereference.
>> + if (BootInfoAddr == NULL) {
>> + DEBUG ((DEBUG_ERROR, "FF-A Boot information is NULL\n"));
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> + // Check boot information magic number.
>> + FfaBootInfo = (EFI_FFA_BOOT_INFO_HEADER *) BootInfoAddr;
>> + if (FfaBootInfo->Magic != FFA_INIT_DESC_SIGNATURE) {
>> + DEBUG ((
>> + DEBUG_ERROR, "FfaBootInfo Magic no. is invalid 0x%ux\n",
>> + FfaBootInfo->Magic
>> + ));
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> +
>> + FfaBootInfoDesc =
>> + (EFI_FFA_BOOT_INFO_DESC *)((UINT8 *)BootInfoAddr +
>> + FfaBootInfo->OffsetBootInfoDesc);
>> +
>> + if (FfaBootInfoDesc->Type ==
>> + (FFA_BOOT_INFO_TYPE(FFA_BOOT_INFO_TYPE_STD) |
>> + FFA_BOOT_INFO_TYPE_ID(FFA_BOOT_INFO_TYPE_ID_FDT))) {
>> + *SpManifestAddr = (UINT64 *) FfaBootInfoDesc->Content;
>> + return EFI_SUCCESS;
>> + }
>> +
>> + DEBUG ((DEBUG_ERROR, "SP manifest not found \n"));
>> + return EFI_NOT_FOUND;
>> +}
>> +
>> /**
>> The entry point of Standalone MM Foundation.
>>
>> @@ -363,6 +713,7 @@ ModuleEntryPoint (
>> {
>> PE_COFF_LOADER_IMAGE_CONTEXT ImageContext;
>> EFI_SECURE_PARTITION_BOOT_INFO *PayloadBootInfo;
>> + EFI_STMM_BOOT_INFO StmmBootInfo = {0};
>> ARM_SVC_ARGS InitMmFoundationSvcArgs;
>> EFI_STATUS Status;
>> INT32 Ret;
>> @@ -372,6 +723,8 @@ ModuleEntryPoint (
>> VOID *TeData;
>> UINTN TeDataSize;
>> EFI_PHYSICAL_ADDRESS ImageBase;
>> + UINT64 *DtbAddress;
>> + EFI_FIRMWARE_VOLUME_HEADER *BfvAddress;
>> BOOLEAN UseOnlyFfaAbis = FALSE;
>>
>> if (FixedPcdGet32 (PcdFfaEnable) != 0) {
>> @@ -384,18 +737,36 @@ ModuleEntryPoint (
>> goto finish;
>> }
>>
>> - PayloadBootInfo = GetAndPrintBootinformation (SharedBufAddress);
>> - if (PayloadBootInfo == NULL) {
>> - Status = EFI_UNSUPPORTED;
>> - goto finish;
>> + // If only FF-A is used, the DTB address is passed in the Boot
>> information
>> + // structure. Else, the Boot info is copied from Sharedbuffer.
>> + if (UseOnlyFfaAbis) {
>> + Status = GetSpManifest (&DtbAddress, SharedBufAddress);
>> + if (Status != EFI_SUCCESS) {
>> + goto finish;
>> + }
>> +
>> + // Extract boot information from the DTB
>> + Status = PopulateBootinformation (&StmmBootInfo, (VOID *)
>> DtbAddress);
>> + if (Status != EFI_SUCCESS) {
>> + goto finish;
>> + }
>> +
>> + // Stash the base address of the boot firmware volume
>> + BfvAddress = (EFI_FIRMWARE_VOLUME_HEADER *) StmmBootInfo.SpMemBase;
>> + } else {
>> + PayloadBootInfo = GetAndPrintBootinformation (SharedBufAddress);
>> + if (PayloadBootInfo == NULL) {
>> + Status = EFI_UNSUPPORTED;
>> + goto finish;
>> + }
>> +
>> + // Stash the base address of the boot firmware volume
>> + BfvAddress = (EFI_FIRMWARE_VOLUME_HEADER *)
>> PayloadBootInfo->SpImageBase;
>> }
>>
>> +
>> // Locate PE/COFF File information for the Standalone MM core module
>> - Status = LocateStandaloneMmCorePeCoffData (
>> - (EFI_FIRMWARE_VOLUME_HEADER
>> *)(UINTN)PayloadBootInfo->SpImageBase,
>> - &TeData,
>> - &TeDataSize
>> - );
>> + Status = LocateStandaloneMmCorePeCoffData (BfvAddress, &TeData,
>> &TeDataSize);
>>
>> if (EFI_ERROR (Status)) {
>> goto finish;
>> --
>> 2.34.1
>>
>>
>>
>> -=-=-=-=-=-=
>> Groups.io Links: You receive all messages sent to this group.
>> View/Reply Online (#106801):
>> https://edk2.groups.io/g/devel/message/106801
>> Mute This Topic: https://groups.io/mt/100079881/6098446
>> Group Owner: devel+owner at edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub
>> [gmahadevan at nvidia.com]
>> -=-=-=-=-=-=
>>
>>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106914): https://edk2.groups.io/g/devel/message/106914
Mute This Topic: https://groups.io/mt/100079881/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