public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree
@ 2021-07-30 17:35 Sayanta Pattanayak
  2021-07-30 19:34 ` [EXTERNAL] [edk2-devel] " Bret Barkelew
  2021-08-01 16:36 ` Ard Biesheuvel
  0 siblings, 2 replies; 7+ messages in thread
From: Sayanta Pattanayak @ 2021-07-30 17:35 UTC (permalink / raw)
  To: devel; +Cc: Ard Biesheuvel, Sami Mujawar, Achin Gupta

Introduce support to populate StMM boot data via DTS parsing. 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>
---
 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [EXTERNAL] [edk2-devel] [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree
  2021-07-30 17:35 [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree Sayanta Pattanayak
@ 2021-07-30 19:34 ` Bret Barkelew
  2021-07-31  0:44   ` Andrew Fish
  2021-08-01 16:36 ` Ard Biesheuvel
  1 sibling, 1 reply; 7+ messages in thread
From: Bret Barkelew @ 2021-07-30 19:34 UTC (permalink / raw)
  To: devel@edk2.groups.io, sayanta.pattanayak@arm.com
  Cc: Ard Biesheuvel, Sami Mujawar, Achin Gupta

[-- Attachment #1: Type: text/plain, Size: 14739 bytes --]

I don’t think this is a good dependency. StandaloneMmCore shouldn’t require anything from EmbeddedPkg, and if it does there’s probably some more generalization necessary.

Further, we can’t require the MmCore (which should be considered generic at the level of PEI Core or DXE Core) include some external library like Libfdt without significant justification.

- Bret

From: Sayanta Pattanayak via groups.io<mailto:sayanta.pattanayak=arm.com@groups.io>
Sent: Friday, July 30, 2021 10:36 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Ard Biesheuvel<mailto:ardb+tianocore@kernel.org>; Sami Mujawar<mailto:sami.mujawar@arm.com>; Achin Gupta<mailto:achin.gupta@arm.comw>
Subject: [EXTERNAL] [edk2-devel] [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree

Introduce support to populate StMM boot data via DTS parsing. 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 = <..>;fc
  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>
---
 Link to github branch with this patch -
 https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FSayantaP-arm%2Fedk2%2Ftree%2Fstmm-dts&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786132182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=gJw%2BH8Dj9lih1DNrIzbNPToJn9nH%2BHqpvpSr2JJrS4I%3D&amp;reserved=0

 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



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78449): https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F78449&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786132182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=0DbkyBJe0prgx2vSCVVsrI0GoVbCipcfmbZp83LREjo%3D&amp;reserved=0
Mute This Topic: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F84555304%2F1852292&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786132182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=1u3cr0MmJmjrkxtGgfOe1mb8%2F7vUXaoyUr5%2FmC1Uq%2F8%3D&amp;reserved=0
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786142135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=Yr%2BSNqWHllfxHkFqBhCyP2TE4IGRLFLm%2FCCWexjB0AM%3D&amp;reserved=0 [bret.barkelew@microsoft.com]
-=-=-=-=-=-=



[-- Attachment #2: Type: text/html, Size: 24891 bytes --]

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [EXTERNAL] [edk2-devel] [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree
  2021-07-30 19:34 ` [EXTERNAL] [edk2-devel] " Bret Barkelew
@ 2021-07-31  0:44   ` Andrew Fish
  2021-07-31  1:44     ` Yao, Jiewen
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Fish @ 2021-07-31  0:44 UTC (permalink / raw)
  To: edk2-devel-groups-io, bret.barkelew
  Cc: sayanta.pattanayak@arm.com, Ard Biesheuvel, Sami Mujawar

[-- Attachment #1: Type: text/plain, Size: 17638 bytes --]



> On Jul 30, 2021, at 12:34 PM, Bret Barkelew via groups.io <http://groups.io/> <bret.barkelew=microsoft.com@groups.io <mailto:bret.barkelew=microsoft.com@groups.io>> wrote:
> 
> I don’t think this is a good dependency. StandaloneMmCore shouldn’t require anything from EmbeddedPkg, and if it does there’s probably some more generalization necessary.
>
> Further, we can’t require the MmCore (which should be considered generic at the level of PEI Core or DXE Core) include some external library like Libfdt without significant justification.
>

I think we should understand if this is being driven by some kind of standard? Maybe the Flat Device Tree lib should be part of the MdePkg? Or maybe MdeModulePkg?

Thanks,

Andrew Fish

> - Bret 
>
> From: Sayanta Pattanayak via groups.io <mailto:sayanta.pattanayak=arm.com@groups.io>
> Sent: Friday, July 30, 2021 10:36 AM
> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> Cc: Ard Biesheuvel <mailto:ardb+tianocore@kernel.org>; Sami Mujawar <mailto:sami.mujawar@arm.com>; Achin Gupta <mailto:achin.gupta@arm.comw>
> Subject: [EXTERNAL] [edk2-devel] [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree
>
> Introduce support to populate StMM boot data via DTS parsing. 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 = <..>;fc
>   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 <mailto:sayanta.pattanayak@arm.com>>
> ---
>  Link to github branch with this patch -
>  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FSayantaP-arm%2Fedk2%2Ftree%2Fstmm-dts&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786132182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=gJw%2BH8Dj9lih1DNrIzbNPToJn9nH%2BHqpvpSr2JJrS4I%3D&amp;reserved=0 <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FSayantaP-arm%2Fedk2%2Ftree%2Fstmm-dts&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786132182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=gJw%2BH8Dj9lih1DNrIzbNPToJn9nH%2BHqpvpSr2JJrS4I%3D&amp;reserved=0>
> 
>  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
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io <http://groups.io/> Links: You receive all messages sent to this group.
> View/Reply Online (#78449): https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F78449&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786132182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=0DbkyBJe0prgx2vSCVVsrI0GoVbCipcfmbZp83LREjo%3D&amp;reserved=0 <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F78449&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786132182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=0DbkyBJe0prgx2vSCVVsrI0GoVbCipcfmbZp83LREjo%3D&amp;reserved=0>
> Mute This Topic: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F84555304%2F1852292&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786132182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=1u3cr0MmJmjrkxtGgfOe1mb8%2F7vUXaoyUr5%2FmC1Uq%2F8%3D&amp;reserved=0 <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F84555304%2F1852292&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786132182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=1u3cr0MmJmjrkxtGgfOe1mb8%2F7vUXaoyUr5%2FmC1Uq%2F8%3D&amp;reserved=0>
> Group Owner: devel+owner@edk2.groups.io <mailto:devel+owner@edk2.groups.io>
> Unsubscribe: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786142135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=Yr%2BSNqWHllfxHkFqBhCyP2TE4IGRLFLm%2FCCWexjB0AM%3D&amp;reserved=0 <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786142135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=Yr%2BSNqWHllfxHkFqBhCyP2TE4IGRLFLm%2FCCWexjB0AM%3D&amp;reserved=0> [bret.barkelew@microsoft.com <mailto:bret.barkelew@microsoft.com>]
> -=-=-=-=-=-=
> 
> 
>
> 


[-- Attachment #2: Type: text/html, Size: 30637 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [EXTERNAL] [edk2-devel] [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree
  2021-07-31  0:44   ` Andrew Fish
@ 2021-07-31  1:44     ` Yao, Jiewen
  0 siblings, 0 replies; 7+ messages in thread
From: Yao, Jiewen @ 2021-07-31  1:44 UTC (permalink / raw)
  To: devel@edk2.groups.io, afish@apple.com,
	bret.barkelew@microsoft.com
  Cc: sayanta.pattanayak@arm.com, Ard Biesheuvel, Sami Mujawar

[-- Attachment #1: Type: text/plain, Size: 15940 bytes --]

I agree with Bret. Adding EmbeddedPkg dependency is not a good idea.

Another option: Is there any possibility to move the function to EmbeddedPkg and just add a simple function call in StanaloneMmPkg ?

Thank you
Yao Jiewen

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Andrew Fish via groups.io
Sent: Saturday, July 31, 2021 8:45 AM
To: edk2-devel-groups-io <devel@edk2.groups.io>; bret.barkelew@microsoft.com
Cc: sayanta.pattanayak@arm.com; Ard Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar <sami.mujawar@arm.com>
Subject: Re: [EXTERNAL] [edk2-devel] [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree




On Jul 30, 2021, at 12:34 PM, Bret Barkelew via groups.io<http://groups.io/> <bret.barkelew=microsoft.com@groups.io<mailto:bret.barkelew=microsoft.com@groups.io>> wrote:

I don’t think this is a good dependency. StandaloneMmCore shouldn’t require anything from EmbeddedPkg, and if it does there’s probably some more generalization necessary.

Further, we can’t require the MmCore (which should be considered generic at the level of PEI Core or DXE Core) include some external library like Libfdt without significant justification.


I think we should understand if this is being driven by some kind of standard? Maybe the Flat Device Tree lib should be part of the MdePkg? Or maybe MdeModulePkg?

Thanks,

Andrew Fish


- Bret

From: Sayanta Pattanayak via groups.io<mailto:sayanta.pattanayak=arm.com@groups.io>
Sent: Friday, July 30, 2021 10:36 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Ard Biesheuvel<mailto:ardb+tianocore@kernel.org>; Sami Mujawar<mailto:sami.mujawar@arm.com>; Achin Gupta<mailto:achin.gupta@arm.comw>
Subject: [EXTERNAL] [edk2-devel] [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree

Introduce support to populate StMM boot data via DTS parsing. 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 = <..>;fc
  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<mailto:sayanta.pattanayak@arm.com>>
---
 Link to github branch with this patch -
 https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FSayantaP-arm%2Fedk2%2Ftree%2Fstmm-dts&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786132182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=gJw%2BH8Dj9lih1DNrIzbNPToJn9nH%2BHqpvpSr2JJrS4I%3D&amp;reserved=0

 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



-=-=-=-=-=-=
Groups.io<http://groups.io/> Links: You receive all messages sent to this group.
View/Reply Online (#78449): https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F78449&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786132182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=0DbkyBJe0prgx2vSCVVsrI0GoVbCipcfmbZp83LREjo%3D&amp;reserved=0
Mute This Topic: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F84555304%2F1852292&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786132182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=1u3cr0MmJmjrkxtGgfOe1mb8%2F7vUXaoyUr5%2FmC1Uq%2F8%3D&amp;reserved=0
Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io>
Unsubscribe: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cb365266c83cb433d1e0108d953806c83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632633786142135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=Yr%2BSNqWHllfxHkFqBhCyP2TE4IGRLFLm%2FCCWexjB0AM%3D&amp;reserved=0 [bret.barkelew@microsoft.com<mailto:bret.barkelew@microsoft.com>]
-=-=-=-=-=-=






[-- Attachment #2: Type: text/html, Size: 29092 bytes --]

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree
  2021-07-30 17:35 [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree Sayanta Pattanayak
  2021-07-30 19:34 ` [EXTERNAL] [edk2-devel] " Bret Barkelew
@ 2021-08-01 16:36 ` Ard Biesheuvel
  2021-08-01 16:38   ` Ard Biesheuvel
  1 sibling, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2021-08-01 16:36 UTC (permalink / raw)
  To: Sayanta Pattanayak
  Cc: edk2-devel-groups-io, Ard Biesheuvel, Sami Mujawar, Achin Gupta

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
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree
  2021-08-01 16:36 ` Ard Biesheuvel
@ 2021-08-01 16:38   ` Ard Biesheuvel
  2021-08-03 17:30     ` Sayanta Pattanayak
  0 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2021-08-01 16:38 UTC (permalink / raw)
  To: Sayanta Pattanayak
  Cc: edk2-devel-groups-io, Sami Mujawar, Achin Gupta, Bret Barkelew,
	Jiewen Yao, Andrew Fish

(correct Achin's email address, cc other replyers)

On Sun, 1 Aug 2021 at 18:36, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> 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 marshal 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
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree
  2021-08-01 16:38   ` Ard Biesheuvel
@ 2021-08-03 17:30     ` Sayanta Pattanayak
  0 siblings, 0 replies; 7+ messages in thread
From: Sayanta Pattanayak @ 2021-08-03 17:30 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel-groups-io, Sami Mujawar, Achin Gupta, Bret Barkelew,
	Jiewen Yao, Andrew Fish

Hi,

Please find my response inline.

Regards,
Sayanta

>-----Original Message-----
>From: Ard Biesheuvel <ardb@kernel.org>
>Sent: Sunday, August 1, 2021 10:08 PM
>To: Sayanta Pattanayak <Sayanta.Pattanayak@arm.com>
>Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Sami Mujawar
><Sami.Mujawar@arm.com>; Achin Gupta <Achin.Gupta@arm.com>; Bret
>Barkelew <Bret.Barkelew@microsoft.com>; Jiewen Yao
><jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>
>Subject: Re: [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to
>populate StMM boot data from device tree
>
>(correct Achin's email address, cc other replyers)
>
>On Sun, 1 Aug 2021 at 18:36, Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> 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 marshal this data into the appropriate format
>> when necessary.
>>

I may not have presented this patch properly.
The key objective of this patch is that in a FF-A Secure partition manager which has StandaloneMM as S-EL0 partition, the StMM boot data could be passed by partition manager through DT and StMM prepares the boot data by parsing the DT.
In existing solution, secure partition manager has StMM specific logic to fetch the StMM boot data and pass it on to StMM through sharedbuffer. Having FF-A manifest for secure partition, DT in this case, will allow Secure partition manager to not have StMM or any other secure partition specific logic to consolidate boot data and pass on.
In the context of this patch, secure partition manager(from EL3) passes the manifest or DT address as booting argument to StMM. StMM will just have the logic to parse the DT and prepare boot data structure, so the manifest is actually part of partition manager codebase.
StMM as secure partition can be used across various types of Secure partition manager, so some amount of uniformity is needed to follow same DT properties for StMM across all kind of Secure partition managers.

>> > 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/StandaloneMmCo
>reEntryPoint.inf       |   3 +
>> >
>> >
>StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalo
>n
>> > eMmCoreEntryPoint.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/StandaloneMmHo
>bLib.inf
>> >    IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> >
>> >
>MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMm
>MemLib
>> > .inf
>> > +  FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>> >
>MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemor
>yAllocationLib/StandaloneMmCoreMemoryAllocationLib.inf
>> >
>MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stand
>aloneMmServicesTableLib.inf
>> >    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>> > diff --git
>> >
>a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
>Cor
>> > eEntryPoint.inf
>> >
>b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
>Cor
>> > eEntryPoint.inf index 4fa426f58ef4..0a2e519dd664 100644
>> > ---
>> >
>a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
>Cor
>> > eEntryPoint.inf
>> > +++
>b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneM
>> > +++ mCoreEntryPoint.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/Standa
>l
>> > oneMmCoreEntryPoint.c
>> >
>b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standa
>l
>> > oneMmCoreEntryPoint.c index 6c50f470aa35..cc09d75dac36 100644
>> > ---
>> >
>a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standa
>l
>> > oneMmCoreEntryPoint.c
>> > +++
>b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Sta
>> > +++ ndaloneMmCoreEntryPoint.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
>> >
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-08-03 17:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-30 17:35 [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree Sayanta Pattanayak
2021-07-30 19:34 ` [EXTERNAL] [edk2-devel] " Bret Barkelew
2021-07-31  0:44   ` Andrew Fish
2021-07-31  1:44     ` Yao, Jiewen
2021-08-01 16:36 ` Ard Biesheuvel
2021-08-01 16:38   ` Ard Biesheuvel
2021-08-03 17:30     ` Sayanta Pattanayak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox