From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web09.9015.1627835802223487181 for ; Sun, 01 Aug 2021 09:36:42 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=G8t9YReT; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id 7D85C610A2 for ; Sun, 1 Aug 2021 16:36:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1627835801; bh=yN0ssYV3aYPCNoe9SF16dsnDHxCtvG/u9eD1/m9ZibQ=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=G8t9YReTtH6jPiUPeSQfB5IU1XGjuKOCjyeso+9DInQii1SQnSLaUTDSb6plhOYDt KNcG+DGubP8cOc3d9k7c2lxenBDW7KjiWF6Ju9VO6wzF89XtCKMwjJ0qPScCFCBMt3 RnrPwTpLhjB3vzOk03fZgqfxa/srvWeptVyYU5xDYSDd595TteZ/gOHiZ6W0KNrUga LClkiZnJue+iQfavEawjoBiLHxDCHSYI7PgJxcN3aP1VBL7ZVHjB2Zlr3xTgicefMa UAS+rn0gjxg0QSavlNpSVqOMxeoJkL+uSoS9YQMxv/BEXv3A2rVn50wiZuRhft+X7k +InxuAR87fKsA== Received: by mail-oo1-f48.google.com with SMTP id s21-20020a4ae5550000b02902667598672bso3828785oot.12 for ; Sun, 01 Aug 2021 09:36:41 -0700 (PDT) X-Gm-Message-State: AOAM531m6NpBqJwJFM3s80ceCgxRuATIjAJwSJcOfdd1udEcc4UTaby4 AAiDgVLAcnBjEYdBHwicxORuvJdS6ZNjTRGipIQ= X-Google-Smtp-Source: ABdhPJzVnNDPD8qdg1abxc4GCLznd456lrfX8AWt6sOiSqhkKoYg0GoFUj2ZlmV6LZvfQRtrmlkV7hT0MDlAJM0CzoE= X-Received: by 2002:a4a:414e:: with SMTP id x75mr8111913ooa.13.1627835800806; Sun, 01 Aug 2021 09:36:40 -0700 (PDT) MIME-Version: 1.0 References: <20210730173510.24235-1-sayanta.pattanayak@arm.com> In-Reply-To: <20210730173510.24235-1-sayanta.pattanayak@arm.com> From: "Ard Biesheuvel" Date: Sun, 1 Aug 2021 18:36:29 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree To: Sayanta Pattanayak Cc: edk2-devel-groups-io , Ard Biesheuvel , Sami Mujawar , Achin Gupta Content-Type: text/plain; charset="UTF-8" On Fri, 30 Jul 2021 at 19:35, Sayanta Pattanayak 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 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 > #include > > +#include > #include > #include > #include > @@ -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 >