public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH v2 0/2] StandaloneMmPkg: Make StandaloneMmCpu platform
@ 2023-09-14 23:10 Tuan Phan
  2023-09-14 23:10 ` [edk2-devel] [PATCH v2 1/2] StandaloneMmPkg: Make StandaloneMmCpu driver platform independent Tuan Phan
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Tuan Phan @ 2023-09-14 23:10 UTC (permalink / raw)
  To: devel
  Cc: ardb+tianocore, sami.mujawar, ray.ni, huangming, sunilvl, yong.li,
	Tuan Phan

This series makes StandaloneMmCpu platform independent so that
other platforms besides ARM/AARCH64 can use it without creating
new driver.

There are two parts in this series:
1. Remove ARM/AARCH64 code from StandaloneMmCpu.
2. Update ARM/AARCH64 entry point library code.

Tuan Phan (2):
  StandaloneMmPkg: Make StandaloneMmCpu driver platform independent
  StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver

 .../Drivers/StandaloneMmCpu/EventHandle.c     | 25 +++--------
 .../Drivers/StandaloneMmCpu/StandaloneMmCpu.c | 42 +++++++++---------
 .../StandaloneMmCpu/StandaloneMmCpu.inf       |  9 ++--
 .../Library/Arm/StandaloneMmCoreEntryPoint.h  | 17 ++------
 .../StandaloneMmCpu.h                         | 22 +++++++---
 .../Arm/CreateHobList.c                       | 43 ++++++++++---------
 .../Arm/StandaloneMmCoreEntryPoint.c          | 15 ++++++-
 .../StandaloneMmCoreEntryPoint.inf            |  2 +-
 StandaloneMmPkg/StandaloneMmPkg.dec           |  2 +-
 9 files changed, 86 insertions(+), 91 deletions(-)
 rename StandaloneMmPkg/{Drivers/StandaloneMmCpu => Include}/StandaloneMmCpu.h (82%)

-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108660): https://edk2.groups.io/g/devel/message/108660
Mute This Topic: https://groups.io/mt/101369645/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* [edk2-devel] [PATCH v2 1/2] StandaloneMmPkg: Make StandaloneMmCpu driver platform independent
  2023-09-14 23:10 [edk2-devel] [PATCH v2 0/2] StandaloneMmPkg: Make StandaloneMmCpu platform Tuan Phan
@ 2023-09-14 23:10 ` Tuan Phan
  2023-09-27 10:10   ` levi.yun
  2023-09-28 16:12   ` Sami Mujawar
  2023-09-14 23:10 ` [edk2-devel] [PATCH v2 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver Tuan Phan
  2023-09-26 16:38 ` [edk2-devel] [PATCH v2 0/2] StandaloneMmPkg: Make StandaloneMmCpu platform Tuan Phan
  2 siblings, 2 replies; 10+ messages in thread
From: Tuan Phan @ 2023-09-14 23:10 UTC (permalink / raw)
  To: devel
  Cc: ardb+tianocore, sami.mujawar, ray.ni, huangming, sunilvl, yong.li,
	Tuan Phan

StandaloneMmCpu now can supports more platforms like RISC-V besides
ARM/AARCH64.

Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
---
 .../Drivers/StandaloneMmCpu/EventHandle.c     | 25 +++--------
 .../Drivers/StandaloneMmCpu/StandaloneMmCpu.c | 42 +++++++++----------
 .../StandaloneMmCpu/StandaloneMmCpu.inf       |  9 ++--
 .../StandaloneMmCpu.h                         | 22 +++++++---
 StandaloneMmPkg/StandaloneMmPkg.dec           |  2 +-
 5 files changed, 46 insertions(+), 54 deletions(-)
 rename StandaloneMmPkg/{Drivers/StandaloneMmCpu => Include}/StandaloneMmCpu.h (82%)

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
index 818e147f874c..dc11d4375a02 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
@@ -3,6 +3,7 @@
   Copyright (c) 2016 HP Development Company, L.P.
   Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
   Copyright (c) 2021, Linaro Limited
+  Copyright (c) 2023, Ventana Micro System Inc. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -11,8 +12,6 @@
 #include <Base.h>
 #include <Pi/PiMmCis.h>
 
-#include <Library/ArmSvcLib.h>
-#include <Library/ArmLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 #include <Library/HobLib.h>
@@ -22,10 +21,7 @@
 #include <Guid/ZeroGuid.h>
 #include <Guid/MmramMemoryReserve.h>
 
-#include <IndustryStandard/ArmFfaSvc.h>
-#include <IndustryStandard/ArmStdSmc.h>
-
-#include "StandaloneMmCpu.h"
+#include <StandaloneMmCpu.h>
 
 EFI_STATUS
 EFIAPI
@@ -108,7 +104,7 @@ CheckBufferAddr (
 }
 
 /**
-  The PI Standalone MM entry point for the TF-A CPU driver.
+  The PI Standalone MM entry point for the CPU driver.
 
   @param  [in] EventId            The event Id.
   @param  [in] CpuNumber          The CPU number.
@@ -121,7 +117,7 @@ CheckBufferAddr (
   @retval   EFI_UNSUPPORTED         Operation not supported.
 **/
 EFI_STATUS
-PiMmStandaloneArmTfCpuDriverEntry (
+PiMmStandaloneMmCpuDriverEntry (
   IN UINTN  EventId,
   IN UINTN  CpuNumber,
   IN UINTN  NsCommBufferAddr
@@ -135,17 +131,6 @@ PiMmStandaloneArmTfCpuDriverEntry (
   DEBUG ((DEBUG_INFO, "Received event - 0x%x on cpu %d\n", EventId, CpuNumber));
 
   Status = EFI_SUCCESS;
-  //
-  // ARM TF passes SMC FID of the MM_COMMUNICATE interface as the Event ID upon
-  // receipt of a synchronous MM request. Use the Event ID to distinguish
-  // between synchronous and asynchronous events.
-  //
-  if ((ARM_SMC_ID_MM_COMMUNICATE != EventId) &&
-      (ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ != EventId))
-  {
-    DEBUG ((DEBUG_ERROR, "UnRecognized Event - 0x%x\n", EventId));
-    return EFI_INVALID_PARAMETER;
-  }
 
   // Perform parameter validation of NsCommBufferAddr
   if (NsCommBufferAddr == (UINTN)NULL) {
@@ -177,7 +162,7 @@ PiMmStandaloneArmTfCpuDriverEntry (
   }
 
   // X1 contains the VA of the normal world memory accessible from
-  // S-EL0
+  // secure world.
   CopyMem (GuidedEventContext, (CONST VOID *)NsCommBufferAddr, NsCommBufferSize);
 
   // Stash the pointer to the allocated Event Context for this CPU
diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
index 3d1dc6181bd9..c5ec1a5a80c5 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
@@ -3,6 +3,7 @@
   Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
   Copyright (c) 2016 HP Development Company, L.P.
   Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
+  Copyright (c) 2023, Ventana Micro System Inc. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -10,10 +11,7 @@
 
 #include <Base.h>
 #include <Pi/PiMmCis.h>
-#include <Library/Arm/StandaloneMmCoreEntryPoint.h>
 #include <Library/DebugLib.h>
-#include <Library/ArmSvcLib.h>
-#include <Library/ArmLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/HobLib.h>
 
@@ -22,7 +20,7 @@
 #include <Guid/ZeroGuid.h>
 #include <Guid/MmramMemoryReserve.h>
 
-#include "StandaloneMmCpu.h"
+#include <StandaloneMmCpu.h>
 
 // GUID to identify HOB with whereabouts of communication buffer with Normal
 // World
@@ -31,7 +29,7 @@ extern EFI_GUID  gEfiStandaloneMmNonSecureBufferGuid;
 // GUID to identify HOB where the entry point of this CPU driver will be
 // populated to allow the entry point driver to invoke it upon receipt of an
 // event
-extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;
+extern EFI_GUID  gEfiMmCpuDriverEpDescriptorGuid;
 
 //
 // Private copy of the MM system table for future use
@@ -96,17 +94,17 @@ StandaloneMmCpuInitialize (
   IN EFI_MM_SYSTEM_TABLE  *SystemTable   // not actual systemtable
   )
 {
-  ARM_TF_CPU_DRIVER_EP_DESCRIPTOR  *CpuDriverEntryPointDesc;
-  EFI_CONFIGURATION_TABLE          *ConfigurationTable;
-  MP_INFORMATION_HOB_DATA          *MpInformationHobData;
-  EFI_MMRAM_DESCRIPTOR             *NsCommBufMmramRange;
-  EFI_STATUS                       Status;
-  EFI_HANDLE                       DispatchHandle;
-  UINT32                           MpInfoSize;
-  UINTN                            Index;
-  UINTN                            ArraySize;
-  VOID                             *HobStart;
-  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK   *MmramRangesHob;
+  MM_CPU_DRIVER_EP_DESCRIPTOR     *CpuDriverEntryPointDesc;
+  EFI_CONFIGURATION_TABLE         *ConfigurationTable;
+  MP_INFORMATION_HOB_DATA         *MpInformationHobData;
+  EFI_MMRAM_DESCRIPTOR            *NsCommBufMmramRange;
+  EFI_STATUS                      Status;
+  EFI_HANDLE                      DispatchHandle;
+  UINT32                          MpInfoSize;
+  UINTN                           Index;
+  UINTN                           ArraySize;
+  VOID                            *HobStart;
+  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK  *MmramRangesHob;
 
   ASSERT (SystemTable != NULL);
   mMmst = SystemTable;
@@ -133,7 +131,7 @@ StandaloneMmCpuInitialize (
   }
 
   // Retrieve the Hoblist from the MMST to extract the details of the NS
-  // communication buffer that has been reserved by S-EL1/EL3
+  // communication buffer that has been reserved for StandaloneMmPkg
   ConfigurationTable = mMmst->MmConfigurationTable;
   for (Index = 0; Index < mMmst->NumberOfTableEntries; Index++) {
     if (CompareGuid (&gEfiHobListGuid, &(ConfigurationTable[Index].VendorGuid))) {
@@ -154,11 +152,11 @@ StandaloneMmCpuInitialize (
   //
   Status = GetGuidedHobData (
              HobStart,
-             &gEfiArmTfCpuDriverEpDescriptorGuid,
+             &gEfiMmCpuDriverEpDescriptorGuid,
              (VOID **)&CpuDriverEntryPointDesc
              );
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "ArmTfCpuDriverEpDesc HOB data extraction failed - 0x%x\n", Status));
+    DEBUG ((DEBUG_ERROR, "MmCpuDriverEpDesc HOB data extraction failed - 0x%x\n", Status));
     return Status;
   }
 
@@ -166,10 +164,10 @@ StandaloneMmCpuInitialize (
   DEBUG ((
     DEBUG_INFO,
     "Sharing Cpu Driver EP *0x%lx = 0x%lx\n",
-    (UINTN)CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,
-    (UINTN)PiMmStandaloneArmTfCpuDriverEntry
+    (UINTN)CpuDriverEntryPointDesc->MmCpuDriverEpPtr,
+    (UINTN)PiMmStandaloneMmCpuDriverEntry
     ));
-  *(CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr) = PiMmStandaloneArmTfCpuDriverEntry;
+  *(CpuDriverEntryPointDesc->MmCpuDriverEpPtr) = PiMmStandaloneMmCpuDriverEntry;
 
   // Find the descriptor that contains the whereabouts of the buffer for
   // communication with the Normal world.
diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
index 1fcb17d89d36..486ccbac1b7c 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
@@ -1,9 +1,10 @@
 ## @file
-#  Standalone MM CPU driver for ARM Standard Platforms
+#  Standalone MM CPU driver
 #
 #  Copyright (c) 2009, Apple Inc. All rights reserved.<BR>
 #  Copyright (c) 2016 HP Development Company, L.P.
 #  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.
+#  Copyright (c) 2023, Ventana Micro System Inc. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
@@ -19,18 +20,14 @@
 
 [Sources]
   StandaloneMmCpu.c
-  StandaloneMmCpu.h
   EventHandle.c
 
 [Packages]
-  ArmPkg/ArmPkg.dec
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
   StandaloneMmPkg/StandaloneMmPkg.dec
 
 [LibraryClasses]
-  ArmLib
-  ArmSvcLib
   BaseMemoryLib
   DebugLib
   HobLib
@@ -46,7 +43,7 @@
   gZeroGuid
   gMpInformationHobGuid
   gEfiStandaloneMmNonSecureBufferGuid
-  gEfiArmTfCpuDriverEpDescriptorGuid
+  gEfiMmCpuDriverEpDescriptorGuid
 
 [Depex]
   TRUE
diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h b/StandaloneMmPkg/Include/StandaloneMmCpu.h
similarity index 82%
rename from StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h
rename to StandaloneMmPkg/Include/StandaloneMmCpu.h
index 3d4698ec1957..1dce7c132ec2 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h
+++ b/StandaloneMmPkg/Include/StandaloneMmCpu.h
@@ -7,14 +7,26 @@
 
 **/
 
-#ifndef _ARM_TF_CPU_DRIVER_H_
-#define _ARM_TF_CPU_DRIVER_H_
+#ifndef MM_CPU_DRIVER_H_
+#define MM_CPU_DRIVER_H_
 
 #include <Protocol/MmCommunication2.h>
 #include <Protocol/MmConfiguration.h>
 #include <Protocol/MmCpu.h>
 #include <Guid/MpInformation.h>
 
+typedef
+EFI_STATUS
+(*PI_MM_CPU_DRIVER_ENTRYPOINT) (
+  IN UINTN  EventId,
+  IN UINTN  CpuNumber,
+  IN UINTN  NsCommBufferAddr
+  );
+
+typedef struct {
+  PI_MM_CPU_DRIVER_ENTRYPOINT    *MmCpuDriverEpPtr;
+} MM_CPU_DRIVER_EP_DESCRIPTOR;
+
 //
 // CPU driver initialization specific declarations
 //
@@ -35,7 +47,7 @@ extern MP_INFORMATION_HOB_DATA        *mMpInformationHobData;
 extern EFI_MM_CONFIGURATION_PROTOCOL  mMmConfig;
 
 /**
-  The PI Standalone MM entry point for the TF-A CPU driver.
+  The PI Standalone MM entry point for the CPU driver.
 
   @param  [in] EventId            The event Id.
   @param  [in] CpuNumber          The CPU number.
@@ -48,7 +60,7 @@ extern EFI_MM_CONFIGURATION_PROTOCOL  mMmConfig;
   @retval   EFI_UNSUPPORTED         Operation not supported.
 **/
 EFI_STATUS
-PiMmStandaloneArmTfCpuDriverEntry (
+PiMmStandaloneMmCpuDriverEntry (
   IN UINTN  EventId,
   IN UINTN  CpuNumber,
   IN UINTN  NsCommBufferAddr
@@ -79,4 +91,4 @@ PiMmCpuTpFwRootMmiHandler (
   IN OUT UINTN       *CommBufferSize  OPTIONAL
   );
 
-#endif
+#endif /* MM_CPU_DRIVER_H_ */
diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec b/StandaloneMmPkg/StandaloneMmPkg.dec
index 46784d94e421..f7cb9e8e2687 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dec
+++ b/StandaloneMmPkg/StandaloneMmPkg.dec
@@ -46,5 +46,5 @@
   gEfiMmPeiMmramMemoryReserveGuid          = { 0x0703f912, 0xbf8d, 0x4e2a, { 0xbe, 0x07, 0xab, 0x27, 0x25, 0x25, 0xc5, 0x92 }}
 
   gEfiStandaloneMmNonSecureBufferGuid      = { 0xf00497e3, 0xbfa2, 0x41a1, { 0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }}
-  gEfiArmTfCpuDriverEpDescriptorGuid       = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }}
+  gEfiMmCpuDriverEpDescriptorGuid          = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }}
 
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108661): https://edk2.groups.io/g/devel/message/108661
Mute This Topic: https://groups.io/mt/101369646/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* [edk2-devel] [PATCH v2 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver
  2023-09-14 23:10 [edk2-devel] [PATCH v2 0/2] StandaloneMmPkg: Make StandaloneMmCpu platform Tuan Phan
  2023-09-14 23:10 ` [edk2-devel] [PATCH v2 1/2] StandaloneMmPkg: Make StandaloneMmCpu driver platform independent Tuan Phan
@ 2023-09-14 23:10 ` Tuan Phan
  2023-09-27 10:10   ` levi.yun
  2023-09-28 16:16   ` Sami Mujawar
  2023-09-26 16:38 ` [edk2-devel] [PATCH v2 0/2] StandaloneMmPkg: Make StandaloneMmCpu platform Tuan Phan
  2 siblings, 2 replies; 10+ messages in thread
From: Tuan Phan @ 2023-09-14 23:10 UTC (permalink / raw)
  To: devel
  Cc: ardb+tianocore, sami.mujawar, ray.ni, huangming, sunilvl, yong.li,
	Tuan Phan

Update entry point library for Arm to use the new platform independent
StandaloneMmCpu driver.

Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
---
 .../Library/Arm/StandaloneMmCoreEntryPoint.h  | 17 ++------
 .../Arm/CreateHobList.c                       | 43 ++++++++++---------
 .../Arm/StandaloneMmCoreEntryPoint.c          | 15 ++++++-
 .../StandaloneMmCoreEntryPoint.inf            |  2 +-
 4 files changed, 40 insertions(+), 37 deletions(-)

diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
index 41bf0f132b4f..dbb81610ff8e 100644
--- a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
+++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
@@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #ifndef __STANDALONEMMCORE_ENTRY_POINT_H__
 #define __STANDALONEMMCORE_ENTRY_POINT_H__
 
+#include <StandaloneMmCpu.h>
 #include <Library/PeCoffLib.h>
 #include <Library/FvLib.h>
 
@@ -47,18 +48,6 @@ typedef struct {
   EFI_SECURE_PARTITION_CPU_INFO    *CpuInfo;
 } EFI_SECURE_PARTITION_BOOT_INFO;
 
-typedef
-EFI_STATUS
-(*PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT) (
-  IN UINTN  EventId,
-  IN UINTN  CpuNumber,
-  IN UINTN  NsCommBufferAddr
-  );
-
-typedef struct {
-  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT    *ArmTfCpuDriverEpPtr;
-} ARM_TF_CPU_DRIVER_EP_DESCRIPTOR;
-
 typedef RETURN_STATUS (*REGION_PERMISSION_UPDATE_FUNC) (
   IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
   IN  UINT64                Length
@@ -145,8 +134,8 @@ LocateStandaloneMmCorePeCoffData (
 VOID *
 EFIAPI
 CreateHobListFromBootInfo (
-  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  *CpuDriverEntryPoint,
-  IN       EFI_SECURE_PARTITION_BOOT_INFO      *PayloadBootInfo
+  IN  OUT  PI_MM_CPU_DRIVER_ENTRYPOINT     *CpuDriverEntryPoint,
+  IN       EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo
   );
 
 /**
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
index 2ac2d354f06a..80ed532352af 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
@@ -13,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Guid/MmramMemoryReserve.h>
 #include <Guid/MpInformation.h>
 
+#include <StandaloneMmCpu.h>
 #include <Library/Arm/StandaloneMmCoreEntryPoint.h>
 #include <Library/ArmMmuLib.h>
 #include <Library/ArmSvcLib.h>
@@ -39,7 +40,7 @@ extern EFI_GUID  gEfiStandaloneMmNonSecureBufferGuid;
 // GUID to identify HOB where the entry point of the CPU driver will be
 // populated to allow this entry point driver to invoke it upon receipt of an
 // event
-extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;
+extern EFI_GUID  gEfiMmCpuDriverEpDescriptorGuid;
 
 /**
   Use the boot information passed by privileged firmware to populate a HOB list
@@ -52,22 +53,22 @@ extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;
 **/
 VOID *
 CreateHobListFromBootInfo (
-  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  *CpuDriverEntryPoint,
-  IN       EFI_SECURE_PARTITION_BOOT_INFO      *PayloadBootInfo
+  IN  OUT  PI_MM_CPU_DRIVER_ENTRYPOINT     *CpuDriverEntryPoint,
+  IN       EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo
   )
 {
-  EFI_HOB_HANDOFF_INFO_TABLE       *HobStart;
-  EFI_RESOURCE_ATTRIBUTE_TYPE      Attributes;
-  UINT32                           Index;
-  UINT32                           BufferSize;
-  UINT32                           Flags;
-  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK   *MmramRangesHob;
-  EFI_MMRAM_DESCRIPTOR             *MmramRanges;
-  EFI_MMRAM_DESCRIPTOR             *NsCommBufMmramRange;
-  MP_INFORMATION_HOB_DATA          *MpInformationHobData;
-  EFI_PROCESSOR_INFORMATION        *ProcInfoBuffer;
-  EFI_SECURE_PARTITION_CPU_INFO    *CpuInfo;
-  ARM_TF_CPU_DRIVER_EP_DESCRIPTOR  *CpuDriverEntryPointDesc;
+  EFI_HOB_HANDOFF_INFO_TABLE      *HobStart;
+  EFI_RESOURCE_ATTRIBUTE_TYPE     Attributes;
+  UINT32                          Index;
+  UINT32                          BufferSize;
+  UINT32                          Flags;
+  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK  *MmramRangesHob;
+  EFI_MMRAM_DESCRIPTOR            *MmramRanges;
+  EFI_MMRAM_DESCRIPTOR            *NsCommBufMmramRange;
+  MP_INFORMATION_HOB_DATA         *MpInformationHobData;
+  EFI_PROCESSOR_INFORMATION       *ProcInfoBuffer;
+  EFI_SECURE_PARTITION_CPU_INFO   *CpuInfo;
+  MM_CPU_DRIVER_EP_DESCRIPTOR     *CpuDriverEntryPointDesc;
 
   // Create a hoblist with a PHIT and EOH
   HobStart = HobConstructor (
@@ -144,13 +145,13 @@ CreateHobListFromBootInfo (
 
   // Create a Guided HOB to enable the ARM TF CPU driver to share its entry
   // point and populate it with the address of the shared buffer
-  CpuDriverEntryPointDesc = (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR *)BuildGuidHob (
-                                                                 &gEfiArmTfCpuDriverEpDescriptorGuid,
-                                                                 sizeof (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR)
-                                                                 );
+  CpuDriverEntryPointDesc = (MM_CPU_DRIVER_EP_DESCRIPTOR *)BuildGuidHob (
+                                                             &gEfiMmCpuDriverEpDescriptorGuid,
+                                                             sizeof (MM_CPU_DRIVER_EP_DESCRIPTOR)
+                                                             );
 
-  *CpuDriverEntryPoint                         = NULL;
-  CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr = CpuDriverEntryPoint;
+  *CpuDriverEntryPoint                      = NULL;
+  CpuDriverEntryPointDesc->MmCpuDriverEpPtr = CpuDriverEntryPoint;
 
   // Find the size of the GUIDed HOB with SRAM ranges
   BufferSize  = sizeof (EFI_MMRAM_HOB_DESCRIPTOR_BLOCK);
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
index 96de10405af8..900e0252ef9f 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
@@ -15,6 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Guid/MmramMemoryReserve.h>
 #include <Guid/MpInformation.h>
 
+#include <StandaloneMmCpu.h>
 #include <Library/ArmSvcLib.h>
 #include <Library/DebugLib.h>
 #include <Library/HobLib.h>
@@ -41,7 +42,7 @@ STATIC CONST UINT32  mSpmMinorVerFfa = SPM_MINOR_VERSION_FFA;
 
 #define BOOT_PAYLOAD_VERSION  1
 
-PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;
+PI_MM_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;
 
 /**
   Retrieve a pointer to and print the boot information passed by privileged
@@ -140,6 +141,18 @@ DelegatedEventLoop (
     DEBUG ((DEBUG_INFO, "X6 :  0x%x\n", (UINT32)EventCompleteSvcArgs->Arg6));
     DEBUG ((DEBUG_INFO, "X7 :  0x%x\n", (UINT32)EventCompleteSvcArgs->Arg7));
 
+    //
+    // ARM TF passes SMC FID of the MM_COMMUNICATE interface as the Event ID upon
+    // receipt of a synchronous MM request. Use the Event ID to distinguish
+    // between synchronous and asynchronous events.
+    //
+    if ((ARM_SMC_ID_MM_COMMUNICATE != (UINT32)EventCompleteSvcArgs->Arg0) &&
+        (ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ != (UINT32)EventCompleteSvcArgs->Arg0))
+    {
+      DEBUG ((DEBUG_ERROR, "UnRecognized Event - 0x%x\n", (UINT32)EventCompleteSvcArgs->Arg0));
+      continue;
+    }
+
     FfaEnabled = FeaturePcdGet (PcdFfaEnable);
     if (FfaEnabled) {
       Status = CpuDriverEntryPoint (
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
index 75cfb98c0e75..d41d7630b614 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
@@ -49,7 +49,7 @@
   gMpInformationHobGuid
   gEfiMmPeiMmramMemoryReserveGuid
   gEfiStandaloneMmNonSecureBufferGuid
-  gEfiArmTfCpuDriverEpDescriptorGuid
+  gEfiMmCpuDriverEpDescriptorGuid
 
 [FeaturePcd.ARM, FeaturePcd.AARCH64]
   gArmTokenSpaceGuid.PcdFfaEnable
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108662): https://edk2.groups.io/g/devel/message/108662
Mute This Topic: https://groups.io/mt/101369647/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [PATCH v2 0/2] StandaloneMmPkg: Make StandaloneMmCpu platform
  2023-09-14 23:10 [edk2-devel] [PATCH v2 0/2] StandaloneMmPkg: Make StandaloneMmCpu platform Tuan Phan
  2023-09-14 23:10 ` [edk2-devel] [PATCH v2 1/2] StandaloneMmPkg: Make StandaloneMmCpu driver platform independent Tuan Phan
  2023-09-14 23:10 ` [edk2-devel] [PATCH v2 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver Tuan Phan
@ 2023-09-26 16:38 ` Tuan Phan
  2 siblings, 0 replies; 10+ messages in thread
From: Tuan Phan @ 2023-09-26 16:38 UTC (permalink / raw)
  To: devel, Yeo Reum Yun, Sami Mujawar
  Cc: ardb+tianocore, ray.ni, huangming, sunilvl, yong.li

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

Hi Sami/Yeo
Do you have any comments on this series?

Regards,

On Thu, Sep 14, 2023 at 4:10 PM Tuan Phan <tphan@ventanamicro.com> wrote:

> This series makes StandaloneMmCpu platform independent so that
> other platforms besides ARM/AARCH64 can use it without creating
> new driver.
>
> There are two parts in this series:
> 1. Remove ARM/AARCH64 code from StandaloneMmCpu.
> 2. Update ARM/AARCH64 entry point library code.
>
> Tuan Phan (2):
>   StandaloneMmPkg: Make StandaloneMmCpu driver platform independent
>   StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver
>
>  .../Drivers/StandaloneMmCpu/EventHandle.c     | 25 +++--------
>  .../Drivers/StandaloneMmCpu/StandaloneMmCpu.c | 42 +++++++++---------
>  .../StandaloneMmCpu/StandaloneMmCpu.inf       |  9 ++--
>  .../Library/Arm/StandaloneMmCoreEntryPoint.h  | 17 ++------
>  .../StandaloneMmCpu.h                         | 22 +++++++---
>  .../Arm/CreateHobList.c                       | 43 ++++++++++---------
>  .../Arm/StandaloneMmCoreEntryPoint.c          | 15 ++++++-
>  .../StandaloneMmCoreEntryPoint.inf            |  2 +-
>  StandaloneMmPkg/StandaloneMmPkg.dec           |  2 +-
>  9 files changed, 86 insertions(+), 91 deletions(-)
>  rename StandaloneMmPkg/{Drivers/StandaloneMmCpu =>
> Include}/StandaloneMmCpu.h (82%)
>
> --
> 2.25.1
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109073): https://edk2.groups.io/g/devel/message/109073
Mute This Topic: https://groups.io/mt/101369645/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

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

* Re: [edk2-devel] [PATCH v2 1/2] StandaloneMmPkg: Make StandaloneMmCpu driver platform independent
  2023-09-14 23:10 ` [edk2-devel] [PATCH v2 1/2] StandaloneMmPkg: Make StandaloneMmCpu driver platform independent Tuan Phan
@ 2023-09-27 10:10   ` levi.yun
  2023-09-28 16:12   ` Sami Mujawar
  1 sibling, 0 replies; 10+ messages in thread
From: levi.yun @ 2023-09-27 10:10 UTC (permalink / raw)
  To: devel@edk2.groups.io, tphan@ventanamicro.com
  Cc: ardb+tianocore@kernel.org, Sami Mujawar, ray.ni@intel.com,
	huangming@linux.alibaba.com, sunilvl@ventanamicro.com,
	yong.li@intel.com

Look good to me.

Thanks!

Reviewed-by: levi.yun <yeoreum.yun@arm.com>

________________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Tuan Phan via groups.io <tphan=ventanamicro.com@groups.io>
Sent: 15 September 2023 00:10
To: devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org; Sami Mujawar; ray.ni@intel.com; huangming@linux.alibaba.com; sunilvl@ventanamicro.com; yong.li@intel.com; Tuan Phan
Subject: [edk2-devel] [PATCH v2 1/2] StandaloneMmPkg: Make StandaloneMmCpu driver platform independent

StandaloneMmCpu now can supports more platforms like RISC-V besides
ARM/AARCH64.

Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
---
 .../Drivers/StandaloneMmCpu/EventHandle.c     | 25 +++--------
 .../Drivers/StandaloneMmCpu/StandaloneMmCpu.c | 42 +++++++++----------
 .../StandaloneMmCpu/StandaloneMmCpu.inf       |  9 ++--
 .../StandaloneMmCpu.h                         | 22 +++++++---
 StandaloneMmPkg/StandaloneMmPkg.dec           |  2 +-
 5 files changed, 46 insertions(+), 54 deletions(-)
 rename StandaloneMmPkg/{Drivers/StandaloneMmCpu => Include}/StandaloneMmCpu.h (82%)

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
index 818e147f874c..dc11d4375a02 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
@@ -3,6 +3,7 @@
   Copyright (c) 2016 HP Development Company, L.P.

   Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.

   Copyright (c) 2021, Linaro Limited

+  Copyright (c) 2023, Ventana Micro System Inc. All rights reserved.



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



@@ -11,8 +12,6 @@
 #include <Base.h>

 #include <Pi/PiMmCis.h>



-#include <Library/ArmSvcLib.h>

-#include <Library/ArmLib.h>

 #include <Library/BaseMemoryLib.h>

 #include <Library/DebugLib.h>

 #include <Library/HobLib.h>

@@ -22,10 +21,7 @@
 #include <Guid/ZeroGuid.h>

 #include <Guid/MmramMemoryReserve.h>



-#include <IndustryStandard/ArmFfaSvc.h>

-#include <IndustryStandard/ArmStdSmc.h>

-

-#include "StandaloneMmCpu.h"

+#include <StandaloneMmCpu.h>



 EFI_STATUS

 EFIAPI

@@ -108,7 +104,7 @@ CheckBufferAddr (
 }



 /**

-  The PI Standalone MM entry point for the TF-A CPU driver.

+  The PI Standalone MM entry point for the CPU driver.



   @param  [in] EventId            The event Id.

   @param  [in] CpuNumber          The CPU number.

@@ -121,7 +117,7 @@ CheckBufferAddr (
   @retval   EFI_UNSUPPORTED         Operation not supported.

 **/

 EFI_STATUS

-PiMmStandaloneArmTfCpuDriverEntry (

+PiMmStandaloneMmCpuDriverEntry (

   IN UINTN  EventId,

   IN UINTN  CpuNumber,

   IN UINTN  NsCommBufferAddr

@@ -135,17 +131,6 @@ PiMmStandaloneArmTfCpuDriverEntry (
   DEBUG ((DEBUG_INFO, "Received event - 0x%x on cpu %d\n", EventId, CpuNumber));



   Status = EFI_SUCCESS;

-  //

-  // ARM TF passes SMC FID of the MM_COMMUNICATE interface as the Event ID upon

-  // receipt of a synchronous MM request. Use the Event ID to distinguish

-  // between synchronous and asynchronous events.

-  //

-  if ((ARM_SMC_ID_MM_COMMUNICATE != EventId) &&

-      (ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ != EventId))

-  {

-    DEBUG ((DEBUG_ERROR, "UnRecognized Event - 0x%x\n", EventId));

-    return EFI_INVALID_PARAMETER;

-  }



   // Perform parameter validation of NsCommBufferAddr

   if (NsCommBufferAddr == (UINTN)NULL) {

@@ -177,7 +162,7 @@ PiMmStandaloneArmTfCpuDriverEntry (
   }



   // X1 contains the VA of the normal world memory accessible from

-  // S-EL0

+  // secure world.

   CopyMem (GuidedEventContext, (CONST VOID *)NsCommBufferAddr, NsCommBufferSize);



   // Stash the pointer to the allocated Event Context for this CPU

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
index 3d1dc6181bd9..c5ec1a5a80c5 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
@@ -3,6 +3,7 @@
   Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>

   Copyright (c) 2016 HP Development Company, L.P.

   Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.

+  Copyright (c) 2023, Ventana Micro System Inc. All rights reserved.



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



@@ -10,10 +11,7 @@


 #include <Base.h>

 #include <Pi/PiMmCis.h>

-#include <Library/Arm/StandaloneMmCoreEntryPoint.h>

 #include <Library/DebugLib.h>

-#include <Library/ArmSvcLib.h>

-#include <Library/ArmLib.h>

 #include <Library/BaseMemoryLib.h>

 #include <Library/HobLib.h>



@@ -22,7 +20,7 @@
 #include <Guid/ZeroGuid.h>

 #include <Guid/MmramMemoryReserve.h>



-#include "StandaloneMmCpu.h"

+#include <StandaloneMmCpu.h>



 // GUID to identify HOB with whereabouts of communication buffer with Normal

 // World

@@ -31,7 +29,7 @@ extern EFI_GUID  gEfiStandaloneMmNonSecureBufferGuid;
 // GUID to identify HOB where the entry point of this CPU driver will be

 // populated to allow the entry point driver to invoke it upon receipt of an

 // event

-extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;

+extern EFI_GUID  gEfiMmCpuDriverEpDescriptorGuid;



 //

 // Private copy of the MM system table for future use

@@ -96,17 +94,17 @@ StandaloneMmCpuInitialize (
   IN EFI_MM_SYSTEM_TABLE  *SystemTable   // not actual systemtable

   )

 {

-  ARM_TF_CPU_DRIVER_EP_DESCRIPTOR  *CpuDriverEntryPointDesc;

-  EFI_CONFIGURATION_TABLE          *ConfigurationTable;

-  MP_INFORMATION_HOB_DATA          *MpInformationHobData;

-  EFI_MMRAM_DESCRIPTOR             *NsCommBufMmramRange;

-  EFI_STATUS                       Status;

-  EFI_HANDLE                       DispatchHandle;

-  UINT32                           MpInfoSize;

-  UINTN                            Index;

-  UINTN                            ArraySize;

-  VOID                             *HobStart;

-  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK   *MmramRangesHob;

+  MM_CPU_DRIVER_EP_DESCRIPTOR     *CpuDriverEntryPointDesc;

+  EFI_CONFIGURATION_TABLE         *ConfigurationTable;

+  MP_INFORMATION_HOB_DATA         *MpInformationHobData;

+  EFI_MMRAM_DESCRIPTOR            *NsCommBufMmramRange;

+  EFI_STATUS                      Status;

+  EFI_HANDLE                      DispatchHandle;

+  UINT32                          MpInfoSize;

+  UINTN                           Index;

+  UINTN                           ArraySize;

+  VOID                            *HobStart;

+  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK  *MmramRangesHob;



   ASSERT (SystemTable != NULL);

   mMmst = SystemTable;

@@ -133,7 +131,7 @@ StandaloneMmCpuInitialize (
   }



   // Retrieve the Hoblist from the MMST to extract the details of the NS

-  // communication buffer that has been reserved by S-EL1/EL3

+  // communication buffer that has been reserved for StandaloneMmPkg

   ConfigurationTable = mMmst->MmConfigurationTable;

   for (Index = 0; Index < mMmst->NumberOfTableEntries; Index++) {

     if (CompareGuid (&gEfiHobListGuid, &(ConfigurationTable[Index].VendorGuid))) {

@@ -154,11 +152,11 @@ StandaloneMmCpuInitialize (
   //

   Status = GetGuidedHobData (

              HobStart,

-             &gEfiArmTfCpuDriverEpDescriptorGuid,

+             &gEfiMmCpuDriverEpDescriptorGuid,

              (VOID **)&CpuDriverEntryPointDesc

              );

   if (EFI_ERROR (Status)) {

-    DEBUG ((DEBUG_ERROR, "ArmTfCpuDriverEpDesc HOB data extraction failed - 0x%x\n", Status));

+    DEBUG ((DEBUG_ERROR, "MmCpuDriverEpDesc HOB data extraction failed - 0x%x\n", Status));

     return Status;

   }



@@ -166,10 +164,10 @@ StandaloneMmCpuInitialize (
   DEBUG ((

     DEBUG_INFO,

     "Sharing Cpu Driver EP *0x%lx = 0x%lx\n",

-    (UINTN)CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,

-    (UINTN)PiMmStandaloneArmTfCpuDriverEntry

+    (UINTN)CpuDriverEntryPointDesc->MmCpuDriverEpPtr,

+    (UINTN)PiMmStandaloneMmCpuDriverEntry

     ));

-  *(CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr) = PiMmStandaloneArmTfCpuDriverEntry;

+  *(CpuDriverEntryPointDesc->MmCpuDriverEpPtr) = PiMmStandaloneMmCpuDriverEntry;



   // Find the descriptor that contains the whereabouts of the buffer for

   // communication with the Normal world.

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
index 1fcb17d89d36..486ccbac1b7c 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
@@ -1,9 +1,10 @@
 ## @file

-#  Standalone MM CPU driver for ARM Standard Platforms

+#  Standalone MM CPU driver

 #

 #  Copyright (c) 2009, Apple Inc. All rights reserved.<BR>

 #  Copyright (c) 2016 HP Development Company, L.P.

 #  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.

+#  Copyright (c) 2023, Ventana Micro System Inc. All rights reserved.

 #

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

 ##

@@ -19,18 +20,14 @@


 [Sources]

   StandaloneMmCpu.c

-  StandaloneMmCpu.h

   EventHandle.c



 [Packages]

-  ArmPkg/ArmPkg.dec

   MdePkg/MdePkg.dec

   MdeModulePkg/MdeModulePkg.dec

   StandaloneMmPkg/StandaloneMmPkg.dec



 [LibraryClasses]

-  ArmLib

-  ArmSvcLib

   BaseMemoryLib

   DebugLib

   HobLib

@@ -46,7 +43,7 @@
   gZeroGuid

   gMpInformationHobGuid

   gEfiStandaloneMmNonSecureBufferGuid

-  gEfiArmTfCpuDriverEpDescriptorGuid

+  gEfiMmCpuDriverEpDescriptorGuid



 [Depex]

   TRUE

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h b/StandaloneMmPkg/Include/StandaloneMmCpu.h
similarity index 82%
rename from StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h
rename to StandaloneMmPkg/Include/StandaloneMmCpu.h
index 3d4698ec1957..1dce7c132ec2 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h
+++ b/StandaloneMmPkg/Include/StandaloneMmCpu.h
@@ -7,14 +7,26 @@


 **/



-#ifndef _ARM_TF_CPU_DRIVER_H_

-#define _ARM_TF_CPU_DRIVER_H_

+#ifndef MM_CPU_DRIVER_H_

+#define MM_CPU_DRIVER_H_



 #include <Protocol/MmCommunication2.h>

 #include <Protocol/MmConfiguration.h>

 #include <Protocol/MmCpu.h>

 #include <Guid/MpInformation.h>



+typedef

+EFI_STATUS

+(*PI_MM_CPU_DRIVER_ENTRYPOINT) (

+  IN UINTN  EventId,

+  IN UINTN  CpuNumber,

+  IN UINTN  NsCommBufferAddr

+  );

+

+typedef struct {

+  PI_MM_CPU_DRIVER_ENTRYPOINT    *MmCpuDriverEpPtr;

+} MM_CPU_DRIVER_EP_DESCRIPTOR;

+

 //

 // CPU driver initialization specific declarations

 //

@@ -35,7 +47,7 @@ extern MP_INFORMATION_HOB_DATA        *mMpInformationHobData;
 extern EFI_MM_CONFIGURATION_PROTOCOL  mMmConfig;



 /**

-  The PI Standalone MM entry point for the TF-A CPU driver.

+  The PI Standalone MM entry point for the CPU driver.



   @param  [in] EventId            The event Id.

   @param  [in] CpuNumber          The CPU number.

@@ -48,7 +60,7 @@ extern EFI_MM_CONFIGURATION_PROTOCOL  mMmConfig;
   @retval   EFI_UNSUPPORTED         Operation not supported.

 **/

 EFI_STATUS

-PiMmStandaloneArmTfCpuDriverEntry (

+PiMmStandaloneMmCpuDriverEntry (

   IN UINTN  EventId,

   IN UINTN  CpuNumber,

   IN UINTN  NsCommBufferAddr

@@ -79,4 +91,4 @@ PiMmCpuTpFwRootMmiHandler (
   IN OUT UINTN       *CommBufferSize  OPTIONAL

   );



-#endif

+#endif /* MM_CPU_DRIVER_H_ */

diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec b/StandaloneMmPkg/StandaloneMmPkg.dec
index 46784d94e421..f7cb9e8e2687 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dec
+++ b/StandaloneMmPkg/StandaloneMmPkg.dec
@@ -46,5 +46,5 @@
   gEfiMmPeiMmramMemoryReserveGuid          = { 0x0703f912, 0xbf8d, 0x4e2a, { 0xbe, 0x07, 0xab, 0x27, 0x25, 0x25, 0xc5, 0x92 }}



   gEfiStandaloneMmNonSecureBufferGuid      = { 0xf00497e3, 0xbfa2, 0x41a1, { 0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }}

-  gEfiArmTfCpuDriverEpDescriptorGuid       = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }}

+  gEfiMmCpuDriverEpDescriptorGuid          = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }}



--
2.25.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108661): https://edk2.groups.io/g/devel/message/108661
Mute This Topic: https://groups.io/mt/101369646/7717249
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [yeoreum.yun@arm.com]
-=-=-=-=-=-=


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.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109116): https://edk2.groups.io/g/devel/message/109116
Mute This Topic: https://groups.io/mt/101369646/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [PATCH v2 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver
  2023-09-14 23:10 ` [edk2-devel] [PATCH v2 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver Tuan Phan
@ 2023-09-27 10:10   ` levi.yun
  2023-09-28 16:16   ` Sami Mujawar
  1 sibling, 0 replies; 10+ messages in thread
From: levi.yun @ 2023-09-27 10:10 UTC (permalink / raw)
  To: devel@edk2.groups.io, tphan@ventanamicro.com
  Cc: ardb+tianocore@kernel.org, Sami Mujawar, ray.ni@intel.com,
	huangming@linux.alibaba.com, sunilvl@ventanamicro.com,
	yong.li@intel.com

Look good to me.

Thanks!

Reviewed-by: levi.yun <yeoreum.yun@arm.com>

________________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Tuan Phan via groups.io <tphan=ventanamicro.com@groups.io>
Sent: 15 September 2023 00:10
To: devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org; Sami Mujawar; ray.ni@intel.com; huangming@linux.alibaba.com; sunilvl@ventanamicro.com; yong.li@intel.com; Tuan Phan
Subject: [edk2-devel] [PATCH v2 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver

Update entry point library for Arm to use the new platform independent
StandaloneMmCpu driver.

Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
---
 .../Library/Arm/StandaloneMmCoreEntryPoint.h  | 17 ++------
 .../Arm/CreateHobList.c                       | 43 ++++++++++---------
 .../Arm/StandaloneMmCoreEntryPoint.c          | 15 ++++++-
 .../StandaloneMmCoreEntryPoint.inf            |  2 +-
 4 files changed, 40 insertions(+), 37 deletions(-)

diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
index 41bf0f132b4f..dbb81610ff8e 100644
--- a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
+++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
@@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #ifndef __STANDALONEMMCORE_ENTRY_POINT_H__

 #define __STANDALONEMMCORE_ENTRY_POINT_H__



+#include <StandaloneMmCpu.h>

 #include <Library/PeCoffLib.h>

 #include <Library/FvLib.h>



@@ -47,18 +48,6 @@ typedef struct {
   EFI_SECURE_PARTITION_CPU_INFO    *CpuInfo;

 } EFI_SECURE_PARTITION_BOOT_INFO;



-typedef

-EFI_STATUS

-(*PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT) (

-  IN UINTN  EventId,

-  IN UINTN  CpuNumber,

-  IN UINTN  NsCommBufferAddr

-  );

-

-typedef struct {

-  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT    *ArmTfCpuDriverEpPtr;

-} ARM_TF_CPU_DRIVER_EP_DESCRIPTOR;

-

 typedef RETURN_STATUS (*REGION_PERMISSION_UPDATE_FUNC) (

   IN  EFI_PHYSICAL_ADDRESS  BaseAddress,

   IN  UINT64                Length

@@ -145,8 +134,8 @@ LocateStandaloneMmCorePeCoffData (
 VOID *

 EFIAPI

 CreateHobListFromBootInfo (

-  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  *CpuDriverEntryPoint,

-  IN       EFI_SECURE_PARTITION_BOOT_INFO      *PayloadBootInfo

+  IN  OUT  PI_MM_CPU_DRIVER_ENTRYPOINT     *CpuDriverEntryPoint,

+  IN       EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo

   );



 /**

diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
index 2ac2d354f06a..80ed532352af 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
@@ -13,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Guid/MmramMemoryReserve.h>

 #include <Guid/MpInformation.h>



+#include <StandaloneMmCpu.h>

 #include <Library/Arm/StandaloneMmCoreEntryPoint.h>

 #include <Library/ArmMmuLib.h>

 #include <Library/ArmSvcLib.h>

@@ -39,7 +40,7 @@ extern EFI_GUID  gEfiStandaloneMmNonSecureBufferGuid;
 // GUID to identify HOB where the entry point of the CPU driver will be

 // populated to allow this entry point driver to invoke it upon receipt of an

 // event

-extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;

+extern EFI_GUID  gEfiMmCpuDriverEpDescriptorGuid;



 /**

   Use the boot information passed by privileged firmware to populate a HOB list

@@ -52,22 +53,22 @@ extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;
 **/

 VOID *

 CreateHobListFromBootInfo (

-  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  *CpuDriverEntryPoint,

-  IN       EFI_SECURE_PARTITION_BOOT_INFO      *PayloadBootInfo

+  IN  OUT  PI_MM_CPU_DRIVER_ENTRYPOINT     *CpuDriverEntryPoint,

+  IN       EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo

   )

 {

-  EFI_HOB_HANDOFF_INFO_TABLE       *HobStart;

-  EFI_RESOURCE_ATTRIBUTE_TYPE      Attributes;

-  UINT32                           Index;

-  UINT32                           BufferSize;

-  UINT32                           Flags;

-  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK   *MmramRangesHob;

-  EFI_MMRAM_DESCRIPTOR             *MmramRanges;

-  EFI_MMRAM_DESCRIPTOR             *NsCommBufMmramRange;

-  MP_INFORMATION_HOB_DATA          *MpInformationHobData;

-  EFI_PROCESSOR_INFORMATION        *ProcInfoBuffer;

-  EFI_SECURE_PARTITION_CPU_INFO    *CpuInfo;

-  ARM_TF_CPU_DRIVER_EP_DESCRIPTOR  *CpuDriverEntryPointDesc;

+  EFI_HOB_HANDOFF_INFO_TABLE      *HobStart;

+  EFI_RESOURCE_ATTRIBUTE_TYPE     Attributes;

+  UINT32                          Index;

+  UINT32                          BufferSize;

+  UINT32                          Flags;

+  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK  *MmramRangesHob;

+  EFI_MMRAM_DESCRIPTOR            *MmramRanges;

+  EFI_MMRAM_DESCRIPTOR            *NsCommBufMmramRange;

+  MP_INFORMATION_HOB_DATA         *MpInformationHobData;

+  EFI_PROCESSOR_INFORMATION       *ProcInfoBuffer;

+  EFI_SECURE_PARTITION_CPU_INFO   *CpuInfo;

+  MM_CPU_DRIVER_EP_DESCRIPTOR     *CpuDriverEntryPointDesc;



   // Create a hoblist with a PHIT and EOH

   HobStart = HobConstructor (

@@ -144,13 +145,13 @@ CreateHobListFromBootInfo (


   // Create a Guided HOB to enable the ARM TF CPU driver to share its entry

   // point and populate it with the address of the shared buffer

-  CpuDriverEntryPointDesc = (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR *)BuildGuidHob (

-                                                                 &gEfiArmTfCpuDriverEpDescriptorGuid,

-                                                                 sizeof (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR)

-                                                                 );

+  CpuDriverEntryPointDesc = (MM_CPU_DRIVER_EP_DESCRIPTOR *)BuildGuidHob (

+                                                             &gEfiMmCpuDriverEpDescriptorGuid,

+                                                             sizeof (MM_CPU_DRIVER_EP_DESCRIPTOR)

+                                                             );



-  *CpuDriverEntryPoint                         = NULL;

-  CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr = CpuDriverEntryPoint;

+  *CpuDriverEntryPoint                      = NULL;

+  CpuDriverEntryPointDesc->MmCpuDriverEpPtr = CpuDriverEntryPoint;



   // Find the size of the GUIDed HOB with SRAM ranges

   BufferSize  = sizeof (EFI_MMRAM_HOB_DESCRIPTOR_BLOCK);

diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
index 96de10405af8..900e0252ef9f 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
@@ -15,6 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Guid/MmramMemoryReserve.h>

 #include <Guid/MpInformation.h>



+#include <StandaloneMmCpu.h>

 #include <Library/ArmSvcLib.h>

 #include <Library/DebugLib.h>

 #include <Library/HobLib.h>

@@ -41,7 +42,7 @@ STATIC CONST UINT32  mSpmMinorVerFfa = SPM_MINOR_VERSION_FFA;


 #define BOOT_PAYLOAD_VERSION  1



-PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;

+PI_MM_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;



 /**

   Retrieve a pointer to and print the boot information passed by privileged

@@ -140,6 +141,18 @@ DelegatedEventLoop (
     DEBUG ((DEBUG_INFO, "X6 :  0x%x\n", (UINT32)EventCompleteSvcArgs->Arg6));

     DEBUG ((DEBUG_INFO, "X7 :  0x%x\n", (UINT32)EventCompleteSvcArgs->Arg7));



+    //

+    // ARM TF passes SMC FID of the MM_COMMUNICATE interface as the Event ID upon

+    // receipt of a synchronous MM request. Use the Event ID to distinguish

+    // between synchronous and asynchronous events.

+    //

+    if ((ARM_SMC_ID_MM_COMMUNICATE != (UINT32)EventCompleteSvcArgs->Arg0) &&

+        (ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ != (UINT32)EventCompleteSvcArgs->Arg0))

+    {

+      DEBUG ((DEBUG_ERROR, "UnRecognized Event - 0x%x\n", (UINT32)EventCompleteSvcArgs->Arg0));

+      continue;

+    }

+

     FfaEnabled = FeaturePcdGet (PcdFfaEnable);

     if (FfaEnabled) {

       Status = CpuDriverEntryPoint (

diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
index 75cfb98c0e75..d41d7630b614 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
@@ -49,7 +49,7 @@
   gMpInformationHobGuid

   gEfiMmPeiMmramMemoryReserveGuid

   gEfiStandaloneMmNonSecureBufferGuid

-  gEfiArmTfCpuDriverEpDescriptorGuid

+  gEfiMmCpuDriverEpDescriptorGuid



 [FeaturePcd.ARM, FeaturePcd.AARCH64]

   gArmTokenSpaceGuid.PcdFfaEnable

--
2.25.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108662): https://edk2.groups.io/g/devel/message/108662
Mute This Topic: https://groups.io/mt/101369647/7717249
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [yeoreum.yun@arm.com]
-=-=-=-=-=-=


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.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109117): https://edk2.groups.io/g/devel/message/109117
Mute This Topic: https://groups.io/mt/101369647/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [PATCH v2 1/2] StandaloneMmPkg: Make StandaloneMmCpu driver platform independent
  2023-09-14 23:10 ` [edk2-devel] [PATCH v2 1/2] StandaloneMmPkg: Make StandaloneMmCpu driver platform independent Tuan Phan
  2023-09-27 10:10   ` levi.yun
@ 2023-09-28 16:12   ` Sami Mujawar
  1 sibling, 0 replies; 10+ messages in thread
From: Sami Mujawar @ 2023-09-28 16:12 UTC (permalink / raw)
  To: Tuan Phan, devel
  Cc: ardb+tianocore, ray.ni, huangming, sunilvl, yong.li, yeoreum.yun,
	nd@arm.com

Hi Tuan,

Thank you for this patch.

This change looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 15/09/2023 12:10 am, Tuan Phan wrote:
> StandaloneMmCpu now can supports more platforms like RISC-V besides
> ARM/AARCH64.
>
> Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
> ---
>   .../Drivers/StandaloneMmCpu/EventHandle.c     | 25 +++--------
>   .../Drivers/StandaloneMmCpu/StandaloneMmCpu.c | 42 +++++++++----------
>   .../StandaloneMmCpu/StandaloneMmCpu.inf       |  9 ++--
>   .../StandaloneMmCpu.h                         | 22 +++++++---
>   StandaloneMmPkg/StandaloneMmPkg.dec           |  2 +-
>   5 files changed, 46 insertions(+), 54 deletions(-)
>   rename StandaloneMmPkg/{Drivers/StandaloneMmCpu => Include}/StandaloneMmCpu.h (82%)
>
> diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
> index 818e147f874c..dc11d4375a02 100644
> --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
> +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
> @@ -3,6 +3,7 @@
>     Copyright (c) 2016 HP Development Company, L.P.
>
>     Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
>
>     Copyright (c) 2021, Linaro Limited
>
> +  Copyright (c) 2023, Ventana Micro System Inc. All rights reserved.
>
>   
>
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   
>
> @@ -11,8 +12,6 @@
>   #include <Base.h>
>
>   #include <Pi/PiMmCis.h>
>
>   
>
> -#include <Library/ArmSvcLib.h>
>
> -#include <Library/ArmLib.h>
>
>   #include <Library/BaseMemoryLib.h>
>
>   #include <Library/DebugLib.h>
>
>   #include <Library/HobLib.h>
>
> @@ -22,10 +21,7 @@
>   #include <Guid/ZeroGuid.h>
>
>   #include <Guid/MmramMemoryReserve.h>
>
>   
>
> -#include <IndustryStandard/ArmFfaSvc.h>
>
> -#include <IndustryStandard/ArmStdSmc.h>
>
> -
>
> -#include "StandaloneMmCpu.h"
>
> +#include <StandaloneMmCpu.h>
>
>   
>
>   EFI_STATUS
>
>   EFIAPI
>
> @@ -108,7 +104,7 @@ CheckBufferAddr (
>   }
>
>   
>
>   /**
>
> -  The PI Standalone MM entry point for the TF-A CPU driver.
>
> +  The PI Standalone MM entry point for the CPU driver.
>
>   
>
>     @param  [in] EventId            The event Id.
>
>     @param  [in] CpuNumber          The CPU number.
>
> @@ -121,7 +117,7 @@ CheckBufferAddr (
>     @retval   EFI_UNSUPPORTED         Operation not supported.
>
>   **/
>
>   EFI_STATUS
>
> -PiMmStandaloneArmTfCpuDriverEntry (
>
> +PiMmStandaloneMmCpuDriverEntry (
>
>     IN UINTN  EventId,
>
>     IN UINTN  CpuNumber,
>
>     IN UINTN  NsCommBufferAddr
>
> @@ -135,17 +131,6 @@ PiMmStandaloneArmTfCpuDriverEntry (
>     DEBUG ((DEBUG_INFO, "Received event - 0x%x on cpu %d\n", EventId, CpuNumber));
>
>   
>
>     Status = EFI_SUCCESS;
>
> -  //
>
> -  // ARM TF passes SMC FID of the MM_COMMUNICATE interface as the Event ID upon
>
> -  // receipt of a synchronous MM request. Use the Event ID to distinguish
>
> -  // between synchronous and asynchronous events.
>
> -  //
>
> -  if ((ARM_SMC_ID_MM_COMMUNICATE != EventId) &&
>
> -      (ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ != EventId))
>
> -  {
>
> -    DEBUG ((DEBUG_ERROR, "UnRecognized Event - 0x%x\n", EventId));
>
> -    return EFI_INVALID_PARAMETER;
>
> -  }
>
>   
>
>     // Perform parameter validation of NsCommBufferAddr
>
>     if (NsCommBufferAddr == (UINTN)NULL) {
>
> @@ -177,7 +162,7 @@ PiMmStandaloneArmTfCpuDriverEntry (
>     }
>
>   
>
>     // X1 contains the VA of the normal world memory accessible from
>
> -  // S-EL0
>
> +  // secure world.
>
>     CopyMem (GuidedEventContext, (CONST VOID *)NsCommBufferAddr, NsCommBufferSize);
>
>   
>
>     // Stash the pointer to the allocated Event Context for this CPU
>
> diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
> index 3d1dc6181bd9..c5ec1a5a80c5 100644
> --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
> +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
> @@ -3,6 +3,7 @@
>     Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
>
>     Copyright (c) 2016 HP Development Company, L.P.
>
>     Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
>
> +  Copyright (c) 2023, Ventana Micro System Inc. All rights reserved.
>
>   
>
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   
>
> @@ -10,10 +11,7 @@
>   
>
>   #include <Base.h>
>
>   #include <Pi/PiMmCis.h>
>
> -#include <Library/Arm/StandaloneMmCoreEntryPoint.h>
>
>   #include <Library/DebugLib.h>
>
> -#include <Library/ArmSvcLib.h>
>
> -#include <Library/ArmLib.h>
>
>   #include <Library/BaseMemoryLib.h>
>
>   #include <Library/HobLib.h>
>
>   
>
> @@ -22,7 +20,7 @@
>   #include <Guid/ZeroGuid.h>
>
>   #include <Guid/MmramMemoryReserve.h>
>
>   
>
> -#include "StandaloneMmCpu.h"
>
> +#include <StandaloneMmCpu.h>
>
>   
>
>   // GUID to identify HOB with whereabouts of communication buffer with Normal
>
>   // World
>
> @@ -31,7 +29,7 @@ extern EFI_GUID  gEfiStandaloneMmNonSecureBufferGuid;
>   // GUID to identify HOB where the entry point of this CPU driver will be
>
>   // populated to allow the entry point driver to invoke it upon receipt of an
>
>   // event
>
> -extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;
>
> +extern EFI_GUID  gEfiMmCpuDriverEpDescriptorGuid;
>
>   
>
>   //
>
>   // Private copy of the MM system table for future use
>
> @@ -96,17 +94,17 @@ StandaloneMmCpuInitialize (
>     IN EFI_MM_SYSTEM_TABLE  *SystemTable   // not actual systemtable
>
>     )
>
>   {
>
> -  ARM_TF_CPU_DRIVER_EP_DESCRIPTOR  *CpuDriverEntryPointDesc;
>
> -  EFI_CONFIGURATION_TABLE          *ConfigurationTable;
>
> -  MP_INFORMATION_HOB_DATA          *MpInformationHobData;
>
> -  EFI_MMRAM_DESCRIPTOR             *NsCommBufMmramRange;
>
> -  EFI_STATUS                       Status;
>
> -  EFI_HANDLE                       DispatchHandle;
>
> -  UINT32                           MpInfoSize;
>
> -  UINTN                            Index;
>
> -  UINTN                            ArraySize;
>
> -  VOID                             *HobStart;
>
> -  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK   *MmramRangesHob;
>
> +  MM_CPU_DRIVER_EP_DESCRIPTOR     *CpuDriverEntryPointDesc;
>
> +  EFI_CONFIGURATION_TABLE         *ConfigurationTable;
>
> +  MP_INFORMATION_HOB_DATA         *MpInformationHobData;
>
> +  EFI_MMRAM_DESCRIPTOR            *NsCommBufMmramRange;
>
> +  EFI_STATUS                      Status;
>
> +  EFI_HANDLE                      DispatchHandle;
>
> +  UINT32                          MpInfoSize;
>
> +  UINTN                           Index;
>
> +  UINTN                           ArraySize;
>
> +  VOID                            *HobStart;
>
> +  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK  *MmramRangesHob;
>
>   
>
>     ASSERT (SystemTable != NULL);
>
>     mMmst = SystemTable;
>
> @@ -133,7 +131,7 @@ StandaloneMmCpuInitialize (
>     }
>
>   
>
>     // Retrieve the Hoblist from the MMST to extract the details of the NS
>
> -  // communication buffer that has been reserved by S-EL1/EL3
>
> +  // communication buffer that has been reserved for StandaloneMmPkg
>
>     ConfigurationTable = mMmst->MmConfigurationTable;
>
>     for (Index = 0; Index < mMmst->NumberOfTableEntries; Index++) {
>
>       if (CompareGuid (&gEfiHobListGuid, &(ConfigurationTable[Index].VendorGuid))) {
>
> @@ -154,11 +152,11 @@ StandaloneMmCpuInitialize (
>     //
>
>     Status = GetGuidedHobData (
>
>                HobStart,
>
> -             &gEfiArmTfCpuDriverEpDescriptorGuid,
>
> +             &gEfiMmCpuDriverEpDescriptorGuid,
>
>                (VOID **)&CpuDriverEntryPointDesc
>
>                );
>
>     if (EFI_ERROR (Status)) {
>
> -    DEBUG ((DEBUG_ERROR, "ArmTfCpuDriverEpDesc HOB data extraction failed - 0x%x\n", Status));
>
> +    DEBUG ((DEBUG_ERROR, "MmCpuDriverEpDesc HOB data extraction failed - 0x%x\n", Status));
>
>       return Status;
>
>     }
>
>   
>
> @@ -166,10 +164,10 @@ StandaloneMmCpuInitialize (
>     DEBUG ((
>
>       DEBUG_INFO,
>
>       "Sharing Cpu Driver EP *0x%lx = 0x%lx\n",
>
> -    (UINTN)CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,
>
> -    (UINTN)PiMmStandaloneArmTfCpuDriverEntry
>
> +    (UINTN)CpuDriverEntryPointDesc->MmCpuDriverEpPtr,
>
> +    (UINTN)PiMmStandaloneMmCpuDriverEntry
>
>       ));
>
> -  *(CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr) = PiMmStandaloneArmTfCpuDriverEntry;
>
> +  *(CpuDriverEntryPointDesc->MmCpuDriverEpPtr) = PiMmStandaloneMmCpuDriverEntry;
>
>   
>
>     // Find the descriptor that contains the whereabouts of the buffer for
>
>     // communication with the Normal world.
>
> diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> index 1fcb17d89d36..486ccbac1b7c 100644
> --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> @@ -1,9 +1,10 @@
>   ## @file
>
> -#  Standalone MM CPU driver for ARM Standard Platforms
>
> +#  Standalone MM CPU driver
>
>   #
>
>   #  Copyright (c) 2009, Apple Inc. All rights reserved.<BR>
>
>   #  Copyright (c) 2016 HP Development Company, L.P.
>
>   #  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.
>
> +#  Copyright (c) 2023, Ventana Micro System Inc. All rights reserved.
>
>   #
>
>   #  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   ##
>
> @@ -19,18 +20,14 @@
>   
>
>   [Sources]
>
>     StandaloneMmCpu.c
>
> -  StandaloneMmCpu.h
>
>     EventHandle.c
>
>   
>
>   [Packages]
>
> -  ArmPkg/ArmPkg.dec
>
>     MdePkg/MdePkg.dec
>
>     MdeModulePkg/MdeModulePkg.dec
>
>     StandaloneMmPkg/StandaloneMmPkg.dec
>
>   
>
>   [LibraryClasses]
>
> -  ArmLib
>
> -  ArmSvcLib
>
>     BaseMemoryLib
>
>     DebugLib
>
>     HobLib
>
> @@ -46,7 +43,7 @@
>     gZeroGuid
>
>     gMpInformationHobGuid
>
>     gEfiStandaloneMmNonSecureBufferGuid
>
> -  gEfiArmTfCpuDriverEpDescriptorGuid
>
> +  gEfiMmCpuDriverEpDescriptorGuid
>
>   
>
>   [Depex]
>
>     TRUE
>
> diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h b/StandaloneMmPkg/Include/StandaloneMmCpu.h
> similarity index 82%
> rename from StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h
> rename to StandaloneMmPkg/Include/StandaloneMmCpu.h
> index 3d4698ec1957..1dce7c132ec2 100644
> --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h
> +++ b/StandaloneMmPkg/Include/StandaloneMmCpu.h
> @@ -7,14 +7,26 @@
>   
>
>   **/
>
>   
>
> -#ifndef _ARM_TF_CPU_DRIVER_H_
>
> -#define _ARM_TF_CPU_DRIVER_H_
>
> +#ifndef MM_CPU_DRIVER_H_
>
> +#define MM_CPU_DRIVER_H_
>
>   
>
>   #include <Protocol/MmCommunication2.h>
>
>   #include <Protocol/MmConfiguration.h>
>
>   #include <Protocol/MmCpu.h>
>
>   #include <Guid/MpInformation.h>
>
>   
>
> +typedef
>
> +EFI_STATUS
>
> +(*PI_MM_CPU_DRIVER_ENTRYPOINT) (
>
> +  IN UINTN  EventId,
>
> +  IN UINTN  CpuNumber,
>
> +  IN UINTN  NsCommBufferAddr
>
> +  );
>
> +
>
> +typedef struct {
>
> +  PI_MM_CPU_DRIVER_ENTRYPOINT    *MmCpuDriverEpPtr;
>
> +} MM_CPU_DRIVER_EP_DESCRIPTOR;
>
> +
>
>   //
>
>   // CPU driver initialization specific declarations
>
>   //
>
> @@ -35,7 +47,7 @@ extern MP_INFORMATION_HOB_DATA        *mMpInformationHobData;
>   extern EFI_MM_CONFIGURATION_PROTOCOL  mMmConfig;
>
>   
>
>   /**
>
> -  The PI Standalone MM entry point for the TF-A CPU driver.
>
> +  The PI Standalone MM entry point for the CPU driver.
>
>   
>
>     @param  [in] EventId            The event Id.
>
>     @param  [in] CpuNumber          The CPU number.
>
> @@ -48,7 +60,7 @@ extern EFI_MM_CONFIGURATION_PROTOCOL  mMmConfig;
>     @retval   EFI_UNSUPPORTED         Operation not supported.
>
>   **/
>
>   EFI_STATUS
>
> -PiMmStandaloneArmTfCpuDriverEntry (
>
> +PiMmStandaloneMmCpuDriverEntry (
>
>     IN UINTN  EventId,
>
>     IN UINTN  CpuNumber,
>
>     IN UINTN  NsCommBufferAddr
>
> @@ -79,4 +91,4 @@ PiMmCpuTpFwRootMmiHandler (
>     IN OUT UINTN       *CommBufferSize  OPTIONAL
>
>     );
>
>   
>
> -#endif
>
> +#endif /* MM_CPU_DRIVER_H_ */
>
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec b/StandaloneMmPkg/StandaloneMmPkg.dec
> index 46784d94e421..f7cb9e8e2687 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dec
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dec
> @@ -46,5 +46,5 @@
>     gEfiMmPeiMmramMemoryReserveGuid          = { 0x0703f912, 0xbf8d, 0x4e2a, { 0xbe, 0x07, 0xab, 0x27, 0x25, 0x25, 0xc5, 0x92 }}
>
>   
>
>     gEfiStandaloneMmNonSecureBufferGuid      = { 0xf00497e3, 0xbfa2, 0x41a1, { 0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }}
>
> -  gEfiArmTfCpuDriverEpDescriptorGuid       = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }}
>
> +  gEfiMmCpuDriverEpDescriptorGuid          = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }}
>
>   
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109164): https://edk2.groups.io/g/devel/message/109164
Mute This Topic: https://groups.io/mt/101369646/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [PATCH v2 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver
  2023-09-14 23:10 ` [edk2-devel] [PATCH v2 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver Tuan Phan
  2023-09-27 10:10   ` levi.yun
@ 2023-09-28 16:16   ` Sami Mujawar
  2023-09-28 18:16     ` Tuan Phan
       [not found]     ` <178922C843CC81E7.9230@groups.io>
  1 sibling, 2 replies; 10+ messages in thread
From: Sami Mujawar @ 2023-09-28 16:16 UTC (permalink / raw)
  To: Tuan Phan, devel
  Cc: ardb+tianocore, ray.ni, huangming, sunilvl, yong.li, nd@arm.com,
	yeoreum.yun

Hi Tuan,

Thank you for this patch.

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 15/09/2023 12:10 am, Tuan Phan wrote:
> Update entry point library for Arm to use the new platform independent

[SAMI] Should this be worded as architecture independent instead of 
platform independent?

Can you also check the subject line and commit message for patch 1/2, 
please?

[/SAMI]

> StandaloneMmCpu driver.
>
> Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
> ---
>   .../Library/Arm/StandaloneMmCoreEntryPoint.h  | 17 ++------
>   .../Arm/CreateHobList.c                       | 43 ++++++++++---------
>   .../Arm/StandaloneMmCoreEntryPoint.c          | 15 ++++++-
>   .../StandaloneMmCoreEntryPoint.inf            |  2 +-
>   4 files changed, 40 insertions(+), 37 deletions(-)
>
> diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> index 41bf0f132b4f..dbb81610ff8e 100644
> --- a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> +++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> @@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   #ifndef __STANDALONEMMCORE_ENTRY_POINT_H__
>
>   #define __STANDALONEMMCORE_ENTRY_POINT_H__
>
>   
>
> +#include <StandaloneMmCpu.h>
>
>   #include <Library/PeCoffLib.h>
>
>   #include <Library/FvLib.h>
>
>   
>
> @@ -47,18 +48,6 @@ typedef struct {
>     EFI_SECURE_PARTITION_CPU_INFO    *CpuInfo;
>
>   } EFI_SECURE_PARTITION_BOOT_INFO;
>
>   
>
> -typedef
>
> -EFI_STATUS
>
> -(*PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT) (
>
> -  IN UINTN  EventId,
>
> -  IN UINTN  CpuNumber,
>
> -  IN UINTN  NsCommBufferAddr
>
> -  );
>
> -
>
> -typedef struct {
>
> -  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT    *ArmTfCpuDriverEpPtr;
>
> -} ARM_TF_CPU_DRIVER_EP_DESCRIPTOR;
>
> -
>
>   typedef RETURN_STATUS (*REGION_PERMISSION_UPDATE_FUNC) (
>
>     IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
>
>     IN  UINT64                Length
>
> @@ -145,8 +134,8 @@ LocateStandaloneMmCorePeCoffData (
>   VOID *
>
>   EFIAPI
>
>   CreateHobListFromBootInfo (
>
> -  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  *CpuDriverEntryPoint,
>
> -  IN       EFI_SECURE_PARTITION_BOOT_INFO      *PayloadBootInfo
>
> +  IN  OUT  PI_MM_CPU_DRIVER_ENTRYPOINT     *CpuDriverEntryPoint,
>
> +  IN       EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo
>
>     );
>
>   
>
>   /**
>
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
> index 2ac2d354f06a..80ed532352af 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
> @@ -13,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   #include <Guid/MmramMemoryReserve.h>
>
>   #include <Guid/MpInformation.h>
>
>   
>
> +#include <StandaloneMmCpu.h>
>
>   #include <Library/Arm/StandaloneMmCoreEntryPoint.h>
>
>   #include <Library/ArmMmuLib.h>
>
>   #include <Library/ArmSvcLib.h>
>
> @@ -39,7 +40,7 @@ extern EFI_GUID  gEfiStandaloneMmNonSecureBufferGuid;
>   // GUID to identify HOB where the entry point of the CPU driver will be
>
>   // populated to allow this entry point driver to invoke it upon receipt of an
>
>   // event
>
> -extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;
>
> +extern EFI_GUID  gEfiMmCpuDriverEpDescriptorGuid;
>
>   
>
>   /**
>
>     Use the boot information passed by privileged firmware to populate a HOB list
>
> @@ -52,22 +53,22 @@ extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;
>   **/
>
>   VOID *
>
>   CreateHobListFromBootInfo (
>
> -  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  *CpuDriverEntryPoint,
>
> -  IN       EFI_SECURE_PARTITION_BOOT_INFO      *PayloadBootInfo
>
> +  IN  OUT  PI_MM_CPU_DRIVER_ENTRYPOINT     *CpuDriverEntryPoint,
>
> +  IN       EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo
>
>     )
>
>   {
>
> -  EFI_HOB_HANDOFF_INFO_TABLE       *HobStart;
>
> -  EFI_RESOURCE_ATTRIBUTE_TYPE      Attributes;
>
> -  UINT32                           Index;
>
> -  UINT32                           BufferSize;
>
> -  UINT32                           Flags;
>
> -  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK   *MmramRangesHob;
>
> -  EFI_MMRAM_DESCRIPTOR             *MmramRanges;
>
> -  EFI_MMRAM_DESCRIPTOR             *NsCommBufMmramRange;
>
> -  MP_INFORMATION_HOB_DATA          *MpInformationHobData;
>
> -  EFI_PROCESSOR_INFORMATION        *ProcInfoBuffer;
>
> -  EFI_SECURE_PARTITION_CPU_INFO    *CpuInfo;
>
> -  ARM_TF_CPU_DRIVER_EP_DESCRIPTOR  *CpuDriverEntryPointDesc;
>
> +  EFI_HOB_HANDOFF_INFO_TABLE      *HobStart;
>
> +  EFI_RESOURCE_ATTRIBUTE_TYPE     Attributes;
>
> +  UINT32                          Index;
>
> +  UINT32                          BufferSize;
>
> +  UINT32                          Flags;
>
> +  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK  *MmramRangesHob;
>
> +  EFI_MMRAM_DESCRIPTOR            *MmramRanges;
>
> +  EFI_MMRAM_DESCRIPTOR            *NsCommBufMmramRange;
>
> +  MP_INFORMATION_HOB_DATA         *MpInformationHobData;
>
> +  EFI_PROCESSOR_INFORMATION       *ProcInfoBuffer;
>
> +  EFI_SECURE_PARTITION_CPU_INFO   *CpuInfo;
>
> +  MM_CPU_DRIVER_EP_DESCRIPTOR     *CpuDriverEntryPointDesc;
>
>   
>
>     // Create a hoblist with a PHIT and EOH
>
>     HobStart = HobConstructor (
>
> @@ -144,13 +145,13 @@ CreateHobListFromBootInfo (
>   
>
>     // Create a Guided HOB to enable the ARM TF CPU driver to share its entry
>
>     // point and populate it with the address of the shared buffer
>
> -  CpuDriverEntryPointDesc = (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR *)BuildGuidHob (
>
> -                                                                 &gEfiArmTfCpuDriverEpDescriptorGuid,
>
> -                                                                 sizeof (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR)
>
> -                                                                 );
>
> +  CpuDriverEntryPointDesc = (MM_CPU_DRIVER_EP_DESCRIPTOR *)BuildGuidHob (
>
> +                                                             &gEfiMmCpuDriverEpDescriptorGuid,
>
> +                                                             sizeof (MM_CPU_DRIVER_EP_DESCRIPTOR)
>
> +                                                             );
>
>   
>
> -  *CpuDriverEntryPoint                         = NULL;
>
> -  CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr = CpuDriverEntryPoint;
>
> +  *CpuDriverEntryPoint                      = NULL;
>
> +  CpuDriverEntryPointDesc->MmCpuDriverEpPtr = CpuDriverEntryPoint;
>
>   
>
>     // Find the size of the GUIDed HOB with SRAM ranges
>
>     BufferSize  = sizeof (EFI_MMRAM_HOB_DESCRIPTOR_BLOCK);
>
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
> index 96de10405af8..900e0252ef9f 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
> @@ -15,6 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   #include <Guid/MmramMemoryReserve.h>
>
>   #include <Guid/MpInformation.h>
>
>   
>
> +#include <StandaloneMmCpu.h>
>
>   #include <Library/ArmSvcLib.h>
>
>   #include <Library/DebugLib.h>
>
>   #include <Library/HobLib.h>
>
> @@ -41,7 +42,7 @@ STATIC CONST UINT32  mSpmMinorVerFfa = SPM_MINOR_VERSION_FFA;
>   
>
>   #define BOOT_PAYLOAD_VERSION  1
>
>   
>
> -PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;
>
> +PI_MM_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;
>
>   
>
>   /**
>
>     Retrieve a pointer to and print the boot information passed by privileged
>
> @@ -140,6 +141,18 @@ DelegatedEventLoop (
>       DEBUG ((DEBUG_INFO, "X6 :  0x%x\n", (UINT32)EventCompleteSvcArgs->Arg6));
>
>       DEBUG ((DEBUG_INFO, "X7 :  0x%x\n", (UINT32)EventCompleteSvcArgs->Arg7));
>
>   
>
> +    //
>
> +    // ARM TF passes SMC FID of the MM_COMMUNICATE interface as the Event ID upon
>
> +    // receipt of a synchronous MM request. Use the Event ID to distinguish
>
> +    // between synchronous and asynchronous events.
>
> +    //
>
> +    if ((ARM_SMC_ID_MM_COMMUNICATE != (UINT32)EventCompleteSvcArgs->Arg0) &&
>
> +        (ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ != (UINT32)EventCompleteSvcArgs->Arg0))
>
> +    {
>
> +      DEBUG ((DEBUG_ERROR, "UnRecognized Event - 0x%x\n", (UINT32)EventCompleteSvcArgs->Arg0));
>
> +      continue;

[SAMI] I think an error needs to be returned instead of continuing 
otherwise this changes the original behaviour.

Status needs to be set to EFI_INVALID_PARAMETER here.

[/SAMI]

>
> +    }

[SAMI] The code from this point needs to be enclosed in an else block 
until before the switch statement.

That way the proper error code would be returned. Can you check, please?

[/SAMI]

>
> +
>
>       FfaEnabled = FeaturePcdGet (PcdFfaEnable);
>
>       if (FfaEnabled) {
>
>         Status = CpuDriverEntryPoint (
>
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> index 75cfb98c0e75..d41d7630b614 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> @@ -49,7 +49,7 @@
>     gMpInformationHobGuid
>
>     gEfiMmPeiMmramMemoryReserveGuid
>
>     gEfiStandaloneMmNonSecureBufferGuid
>
> -  gEfiArmTfCpuDriverEpDescriptorGuid
>
> +  gEfiMmCpuDriverEpDescriptorGuid
>
>   
>
>   [FeaturePcd.ARM, FeaturePcd.AARCH64]
>
>     gArmTokenSpaceGuid.PcdFfaEnable
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109166): https://edk2.groups.io/g/devel/message/109166
Mute This Topic: https://groups.io/mt/101369647/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [PATCH v2 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver
  2023-09-28 16:16   ` Sami Mujawar
@ 2023-09-28 18:16     ` Tuan Phan
       [not found]     ` <178922C843CC81E7.9230@groups.io>
  1 sibling, 0 replies; 10+ messages in thread
From: Tuan Phan @ 2023-09-28 18:16 UTC (permalink / raw)
  To: Sami Mujawar
  Cc: devel, ardb+tianocore, ray.ni, huangming, sunilvl, yong.li,
	nd@arm.com, yeoreum.yun

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

Hi Sami,
Please see my comments below.

On Thu, Sep 28, 2023 at 9:16 AM Sami Mujawar <sami.mujawar@arm.com> wrote:

> Hi Tuan,
>
> Thank you for this patch.
>
> Please see my response inline marked [SAMI].
>
> Regards,
>
> Sami Mujawar
>
> On 15/09/2023 12:10 am, Tuan Phan wrote:
> > Update entry point library for Arm to use the new platform independent
>
> [SAMI] Should this be worded as architecture independent instead of
> platform independent?
>
> Can you also check the subject line and commit message for patch 1/2,
> please?
>
[Tuan] Sure, that makes sense.

>
> [/SAMI]
>
> > StandaloneMmCpu driver.
> >
> > Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
> > ---
> >   .../Library/Arm/StandaloneMmCoreEntryPoint.h  | 17 ++------
> >   .../Arm/CreateHobList.c                       | 43 ++++++++++---------
> >   .../Arm/StandaloneMmCoreEntryPoint.c          | 15 ++++++-
> >   .../StandaloneMmCoreEntryPoint.inf            |  2 +-
> >   4 files changed, 40 insertions(+), 37 deletions(-)
> >
> > diff --git
> a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> > index 41bf0f132b4f..dbb81610ff8e 100644
> > --- a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> > +++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> > @@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >   #ifndef __STANDALONEMMCORE_ENTRY_POINT_H__
> >
> >   #define __STANDALONEMMCORE_ENTRY_POINT_H__
> >
> >
> >
> > +#include <StandaloneMmCpu.h>
> >
> >   #include <Library/PeCoffLib.h>
> >
> >   #include <Library/FvLib.h>
> >
> >
> >
> > @@ -47,18 +48,6 @@ typedef struct {
> >     EFI_SECURE_PARTITION_CPU_INFO    *CpuInfo;
> >
> >   } EFI_SECURE_PARTITION_BOOT_INFO;
> >
> >
> >
> > -typedef
> >
> > -EFI_STATUS
> >
> > -(*PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT) (
> >
> > -  IN UINTN  EventId,
> >
> > -  IN UINTN  CpuNumber,
> >
> > -  IN UINTN  NsCommBufferAddr
> >
> > -  );
> >
> > -
> >
> > -typedef struct {
> >
> > -  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT    *ArmTfCpuDriverEpPtr;
> >
> > -} ARM_TF_CPU_DRIVER_EP_DESCRIPTOR;
> >
> > -
> >
> >   typedef RETURN_STATUS (*REGION_PERMISSION_UPDATE_FUNC) (
> >
> >     IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
> >
> >     IN  UINT64                Length
> >
> > @@ -145,8 +134,8 @@ LocateStandaloneMmCorePeCoffData (
> >   VOID *
> >
> >   EFIAPI
> >
> >   CreateHobListFromBootInfo (
> >
> > -  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  *CpuDriverEntryPoint,
> >
> > -  IN       EFI_SECURE_PARTITION_BOOT_INFO      *PayloadBootInfo
> >
> > +  IN  OUT  PI_MM_CPU_DRIVER_ENTRYPOINT     *CpuDriverEntryPoint,
> >
> > +  IN       EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo
> >
> >     );
> >
> >
> >
> >   /**
> >
> > diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
> > index 2ac2d354f06a..80ed532352af 100644
> > ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
> > +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
> > @@ -13,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >   #include <Guid/MmramMemoryReserve.h>
> >
> >   #include <Guid/MpInformation.h>
> >
> >
> >
> > +#include <StandaloneMmCpu.h>
> >
> >   #include <Library/Arm/StandaloneMmCoreEntryPoint.h>
> >
> >   #include <Library/ArmMmuLib.h>
> >
> >   #include <Library/ArmSvcLib.h>
> >
> > @@ -39,7 +40,7 @@ extern EFI_GUID  gEfiStandaloneMmNonSecureBufferGuid;
> >   // GUID to identify HOB where the entry point of the CPU driver will be
> >
> >   // populated to allow this entry point driver to invoke it upon
> receipt of an
> >
> >   // event
> >
> > -extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;
> >
> > +extern EFI_GUID  gEfiMmCpuDriverEpDescriptorGuid;
> >
> >
> >
> >   /**
> >
> >     Use the boot information passed by privileged firmware to populate a
> HOB list
> >
> > @@ -52,22 +53,22 @@ extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;
> >   **/
> >
> >   VOID *
> >
> >   CreateHobListFromBootInfo (
> >
> > -  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  *CpuDriverEntryPoint,
> >
> > -  IN       EFI_SECURE_PARTITION_BOOT_INFO      *PayloadBootInfo
> >
> > +  IN  OUT  PI_MM_CPU_DRIVER_ENTRYPOINT     *CpuDriverEntryPoint,
> >
> > +  IN       EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo
> >
> >     )
> >
> >   {
> >
> > -  EFI_HOB_HANDOFF_INFO_TABLE       *HobStart;
> >
> > -  EFI_RESOURCE_ATTRIBUTE_TYPE      Attributes;
> >
> > -  UINT32                           Index;
> >
> > -  UINT32                           BufferSize;
> >
> > -  UINT32                           Flags;
> >
> > -  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK   *MmramRangesHob;
> >
> > -  EFI_MMRAM_DESCRIPTOR             *MmramRanges;
> >
> > -  EFI_MMRAM_DESCRIPTOR             *NsCommBufMmramRange;
> >
> > -  MP_INFORMATION_HOB_DATA          *MpInformationHobData;
> >
> > -  EFI_PROCESSOR_INFORMATION        *ProcInfoBuffer;
> >
> > -  EFI_SECURE_PARTITION_CPU_INFO    *CpuInfo;
> >
> > -  ARM_TF_CPU_DRIVER_EP_DESCRIPTOR  *CpuDriverEntryPointDesc;
> >
> > +  EFI_HOB_HANDOFF_INFO_TABLE      *HobStart;
> >
> > +  EFI_RESOURCE_ATTRIBUTE_TYPE     Attributes;
> >
> > +  UINT32                          Index;
> >
> > +  UINT32                          BufferSize;
> >
> > +  UINT32                          Flags;
> >
> > +  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK  *MmramRangesHob;
> >
> > +  EFI_MMRAM_DESCRIPTOR            *MmramRanges;
> >
> > +  EFI_MMRAM_DESCRIPTOR            *NsCommBufMmramRange;
> >
> > +  MP_INFORMATION_HOB_DATA         *MpInformationHobData;
> >
> > +  EFI_PROCESSOR_INFORMATION       *ProcInfoBuffer;
> >
> > +  EFI_SECURE_PARTITION_CPU_INFO   *CpuInfo;
> >
> > +  MM_CPU_DRIVER_EP_DESCRIPTOR     *CpuDriverEntryPointDesc;
> >
> >
> >
> >     // Create a hoblist with a PHIT and EOH
> >
> >     HobStart = HobConstructor (
> >
> > @@ -144,13 +145,13 @@ CreateHobListFromBootInfo (
> >
> >
> >     // Create a Guided HOB to enable the ARM TF CPU driver to share its
> entry
> >
> >     // point and populate it with the address of the shared buffer
> >
> > -  CpuDriverEntryPointDesc = (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR
> *)BuildGuidHob (
> >
> > -
>  &gEfiArmTfCpuDriverEpDescriptorGuid,
> >
> > -                                                                 sizeof
> (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR)
> >
> > -                                                                 );
> >
> > +  CpuDriverEntryPointDesc = (MM_CPU_DRIVER_EP_DESCRIPTOR *)BuildGuidHob
> (
> >
> > +
>  &gEfiMmCpuDriverEpDescriptorGuid,
> >
> > +                                                             sizeof
> (MM_CPU_DRIVER_EP_DESCRIPTOR)
> >
> > +                                                             );
> >
> >
> >
> > -  *CpuDriverEntryPoint                         = NULL;
> >
> > -  CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr = CpuDriverEntryPoint;
> >
> > +  *CpuDriverEntryPoint                      = NULL;
> >
> > +  CpuDriverEntryPointDesc->MmCpuDriverEpPtr = CpuDriverEntryPoint;
> >
> >
> >
> >     // Find the size of the GUIDed HOB with SRAM ranges
> >
> >     BufferSize  = sizeof (EFI_MMRAM_HOB_DESCRIPTOR_BLOCK);
> >
> > diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
> > index 96de10405af8..900e0252ef9f 100644
> > ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
> > +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
> > @@ -15,6 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >   #include <Guid/MmramMemoryReserve.h>
> >
> >   #include <Guid/MpInformation.h>
> >
> >
> >
> > +#include <StandaloneMmCpu.h>
> >
> >   #include <Library/ArmSvcLib.h>
> >
> >   #include <Library/DebugLib.h>
> >
> >   #include <Library/HobLib.h>
> >
> > @@ -41,7 +42,7 @@ STATIC CONST UINT32  mSpmMinorVerFfa =
> SPM_MINOR_VERSION_FFA;
> >
> >
> >   #define BOOT_PAYLOAD_VERSION  1
> >
> >
> >
> > -PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;
> >
> > +PI_MM_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;
> >
> >
> >
> >   /**
> >
> >     Retrieve a pointer to and print the boot information passed by
> privileged
> >
> > @@ -140,6 +141,18 @@ DelegatedEventLoop (
> >       DEBUG ((DEBUG_INFO, "X6 :  0x%x\n",
> (UINT32)EventCompleteSvcArgs->Arg6));
> >
> >       DEBUG ((DEBUG_INFO, "X7 :  0x%x\n",
> (UINT32)EventCompleteSvcArgs->Arg7));
> >
> >
> >
> > +    //
> >
> > +    // ARM TF passes SMC FID of the MM_COMMUNICATE interface as the
> Event ID upon
> >
> > +    // receipt of a synchronous MM request. Use the Event ID to
> distinguish
> >
> > +    // between synchronous and asynchronous events.
> >
> > +    //
> >
> > +    if ((ARM_SMC_ID_MM_COMMUNICATE !=
> (UINT32)EventCompleteSvcArgs->Arg0) &&
> >
> > +        (ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ !=
> (UINT32)EventCompleteSvcArgs->Arg0))
> >
> > +    {
> >
> > +      DEBUG ((DEBUG_ERROR, "UnRecognized Event - 0x%x\n",
> (UINT32)EventCompleteSvcArgs->Arg0));
> >
> > +      continue;
>
> [SAMI] I think an error needs to be returned instead of continuing
> otherwise this changes the original behaviour.
>
> Status needs to be set to EFI_INVALID_PARAMETER here.
>
> [/SAMI]


> >
> > +    }
>
> [SAMI] The code from this point needs to be enclosed in an else block
> until before the switch statement.
>
> That way the proper error code would be returned. Can you check, please?
>
> [/SAMI]
>
[Tuan] Thanks for the catch. Will fix it

>
> >
> > +
> >
> >       FfaEnabled = FeaturePcdGet (PcdFfaEnable);
> >
> >       if (FfaEnabled) {
> >
> >         Status = CpuDriverEntryPoint (
> >
> > diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> > index 75cfb98c0e75..d41d7630b614 100644
> > ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> > +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> > @@ -49,7 +49,7 @@
> >     gMpInformationHobGuid
> >
> >     gEfiMmPeiMmramMemoryReserveGuid
> >
> >     gEfiStandaloneMmNonSecureBufferGuid
> >
> > -  gEfiArmTfCpuDriverEpDescriptorGuid
> >
> > +  gEfiMmCpuDriverEpDescriptorGuid
> >
> >
> >
> >   [FeaturePcd.ARM, FeaturePcd.AARCH64]
> >
> >     gArmTokenSpaceGuid.PcdFfaEnable
> >
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109174): https://edk2.groups.io/g/devel/message/109174
Mute This Topic: https://groups.io/mt/101369647/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

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

* Re: [edk2-devel] [PATCH v2 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver
       [not found]     ` <178922C843CC81E7.9230@groups.io>
@ 2023-09-28 21:14       ` Tuan Phan
  0 siblings, 0 replies; 10+ messages in thread
From: Tuan Phan @ 2023-09-28 21:14 UTC (permalink / raw)
  To: devel, tphan
  Cc: Sami Mujawar, ardb+tianocore, ray.ni, huangming, sunilvl, yong.li,
	nd@arm.com, yeoreum.yun

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

Hi Sami,
I just sent the V3 series to address your comments.

Regards,

On Thu, Sep 28, 2023 at 11:16 AM Tuan Phan via groups.io <tphan=
ventanamicro.com@groups.io> wrote:

> Hi Sami,
> Please see my comments below.
>
> On Thu, Sep 28, 2023 at 9:16 AM Sami Mujawar <sami.mujawar@arm.com> wrote:
>
>> Hi Tuan,
>>
>> Thank you for this patch.
>>
>> Please see my response inline marked [SAMI].
>>
>> Regards,
>>
>> Sami Mujawar
>>
>> On 15/09/2023 12:10 am, Tuan Phan wrote:
>> > Update entry point library for Arm to use the new platform independent
>>
>> [SAMI] Should this be worded as architecture independent instead of
>> platform independent?
>>
>> Can you also check the subject line and commit message for patch 1/2,
>> please?
>>
> [Tuan] Sure, that makes sense.
>
>>
>> [/SAMI]
>>
>> > StandaloneMmCpu driver.
>> >
>> > Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
>> > ---
>> >   .../Library/Arm/StandaloneMmCoreEntryPoint.h  | 17 ++------
>> >   .../Arm/CreateHobList.c                       | 43 ++++++++++---------
>> >   .../Arm/StandaloneMmCoreEntryPoint.c          | 15 ++++++-
>> >   .../StandaloneMmCoreEntryPoint.inf            |  2 +-
>> >   4 files changed, 40 insertions(+), 37 deletions(-)
>> >
>> > diff --git
>> a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
>> b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
>> > index 41bf0f132b4f..dbb81610ff8e 100644
>> > --- a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
>> > +++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
>> > @@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>> >   #ifndef __STANDALONEMMCORE_ENTRY_POINT_H__
>> >
>> >   #define __STANDALONEMMCORE_ENTRY_POINT_H__
>> >
>> >
>> >
>> > +#include <StandaloneMmCpu.h>
>> >
>> >   #include <Library/PeCoffLib.h>
>> >
>> >   #include <Library/FvLib.h>
>> >
>> >
>> >
>> > @@ -47,18 +48,6 @@ typedef struct {
>> >     EFI_SECURE_PARTITION_CPU_INFO    *CpuInfo;
>> >
>> >   } EFI_SECURE_PARTITION_BOOT_INFO;
>> >
>> >
>> >
>> > -typedef
>> >
>> > -EFI_STATUS
>> >
>> > -(*PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT) (
>> >
>> > -  IN UINTN  EventId,
>> >
>> > -  IN UINTN  CpuNumber,
>> >
>> > -  IN UINTN  NsCommBufferAddr
>> >
>> > -  );
>> >
>> > -
>> >
>> > -typedef struct {
>> >
>> > -  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT    *ArmTfCpuDriverEpPtr;
>> >
>> > -} ARM_TF_CPU_DRIVER_EP_DESCRIPTOR;
>> >
>> > -
>> >
>> >   typedef RETURN_STATUS (*REGION_PERMISSION_UPDATE_FUNC) (
>> >
>> >     IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
>> >
>> >     IN  UINT64                Length
>> >
>> > @@ -145,8 +134,8 @@ LocateStandaloneMmCorePeCoffData (
>> >   VOID *
>> >
>> >   EFIAPI
>> >
>> >   CreateHobListFromBootInfo (
>> >
>> > -  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  *CpuDriverEntryPoint,
>> >
>> > -  IN       EFI_SECURE_PARTITION_BOOT_INFO      *PayloadBootInfo
>> >
>> > +  IN  OUT  PI_MM_CPU_DRIVER_ENTRYPOINT     *CpuDriverEntryPoint,
>> >
>> > +  IN       EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo
>> >
>> >     );
>> >
>> >
>> >
>> >   /**
>> >
>> > diff --git
>> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
>> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
>> > index 2ac2d354f06a..80ed532352af 100644
>> > ---
>> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
>> > +++
>> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
>> > @@ -13,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>> >   #include <Guid/MmramMemoryReserve.h>
>> >
>> >   #include <Guid/MpInformation.h>
>> >
>> >
>> >
>> > +#include <StandaloneMmCpu.h>
>> >
>> >   #include <Library/Arm/StandaloneMmCoreEntryPoint.h>
>> >
>> >   #include <Library/ArmMmuLib.h>
>> >
>> >   #include <Library/ArmSvcLib.h>
>> >
>> > @@ -39,7 +40,7 @@ extern EFI_GUID  gEfiStandaloneMmNonSecureBufferGuid;
>> >   // GUID to identify HOB where the entry point of the CPU driver will
>> be
>> >
>> >   // populated to allow this entry point driver to invoke it upon
>> receipt of an
>> >
>> >   // event
>> >
>> > -extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;
>> >
>> > +extern EFI_GUID  gEfiMmCpuDriverEpDescriptorGuid;
>> >
>> >
>> >
>> >   /**
>> >
>> >     Use the boot information passed by privileged firmware to populate
>> a HOB list
>> >
>> > @@ -52,22 +53,22 @@ extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;
>> >   **/
>> >
>> >   VOID *
>> >
>> >   CreateHobListFromBootInfo (
>> >
>> > -  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  *CpuDriverEntryPoint,
>> >
>> > -  IN       EFI_SECURE_PARTITION_BOOT_INFO      *PayloadBootInfo
>> >
>> > +  IN  OUT  PI_MM_CPU_DRIVER_ENTRYPOINT     *CpuDriverEntryPoint,
>> >
>> > +  IN       EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo
>> >
>> >     )
>> >
>> >   {
>> >
>> > -  EFI_HOB_HANDOFF_INFO_TABLE       *HobStart;
>> >
>> > -  EFI_RESOURCE_ATTRIBUTE_TYPE      Attributes;
>> >
>> > -  UINT32                           Index;
>> >
>> > -  UINT32                           BufferSize;
>> >
>> > -  UINT32                           Flags;
>> >
>> > -  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK   *MmramRangesHob;
>> >
>> > -  EFI_MMRAM_DESCRIPTOR             *MmramRanges;
>> >
>> > -  EFI_MMRAM_DESCRIPTOR             *NsCommBufMmramRange;
>> >
>> > -  MP_INFORMATION_HOB_DATA          *MpInformationHobData;
>> >
>> > -  EFI_PROCESSOR_INFORMATION        *ProcInfoBuffer;
>> >
>> > -  EFI_SECURE_PARTITION_CPU_INFO    *CpuInfo;
>> >
>> > -  ARM_TF_CPU_DRIVER_EP_DESCRIPTOR  *CpuDriverEntryPointDesc;
>> >
>> > +  EFI_HOB_HANDOFF_INFO_TABLE      *HobStart;
>> >
>> > +  EFI_RESOURCE_ATTRIBUTE_TYPE     Attributes;
>> >
>> > +  UINT32                          Index;
>> >
>> > +  UINT32                          BufferSize;
>> >
>> > +  UINT32                          Flags;
>> >
>> > +  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK  *MmramRangesHob;
>> >
>> > +  EFI_MMRAM_DESCRIPTOR            *MmramRanges;
>> >
>> > +  EFI_MMRAM_DESCRIPTOR            *NsCommBufMmramRange;
>> >
>> > +  MP_INFORMATION_HOB_DATA         *MpInformationHobData;
>> >
>> > +  EFI_PROCESSOR_INFORMATION       *ProcInfoBuffer;
>> >
>> > +  EFI_SECURE_PARTITION_CPU_INFO   *CpuInfo;
>> >
>> > +  MM_CPU_DRIVER_EP_DESCRIPTOR     *CpuDriverEntryPointDesc;
>> >
>> >
>> >
>> >     // Create a hoblist with a PHIT and EOH
>> >
>> >     HobStart = HobConstructor (
>> >
>> > @@ -144,13 +145,13 @@ CreateHobListFromBootInfo (
>> >
>> >
>> >     // Create a Guided HOB to enable the ARM TF CPU driver to share its
>> entry
>> >
>> >     // point and populate it with the address of the shared buffer
>> >
>> > -  CpuDriverEntryPointDesc = (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR
>> *)BuildGuidHob (
>> >
>> > -
>>  &gEfiArmTfCpuDriverEpDescriptorGuid,
>> >
>> > -
>>  sizeof (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR)
>> >
>> > -                                                                 );
>> >
>> > +  CpuDriverEntryPointDesc = (MM_CPU_DRIVER_EP_DESCRIPTOR
>> *)BuildGuidHob (
>> >
>> > +
>>  &gEfiMmCpuDriverEpDescriptorGuid,
>> >
>> > +                                                             sizeof
>> (MM_CPU_DRIVER_EP_DESCRIPTOR)
>> >
>> > +                                                             );
>> >
>> >
>> >
>> > -  *CpuDriverEntryPoint                         = NULL;
>> >
>> > -  CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr = CpuDriverEntryPoint;
>> >
>> > +  *CpuDriverEntryPoint                      = NULL;
>> >
>> > +  CpuDriverEntryPointDesc->MmCpuDriverEpPtr = CpuDriverEntryPoint;
>> >
>> >
>> >
>> >     // Find the size of the GUIDed HOB with SRAM ranges
>> >
>> >     BufferSize  = sizeof (EFI_MMRAM_HOB_DESCRIPTOR_BLOCK);
>> >
>> > diff --git
>> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
>> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
>> > index 96de10405af8..900e0252ef9f 100644
>> > ---
>> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
>> > +++
>> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
>> > @@ -15,6 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>> >   #include <Guid/MmramMemoryReserve.h>
>> >
>> >   #include <Guid/MpInformation.h>
>> >
>> >
>> >
>> > +#include <StandaloneMmCpu.h>
>> >
>> >   #include <Library/ArmSvcLib.h>
>> >
>> >   #include <Library/DebugLib.h>
>> >
>> >   #include <Library/HobLib.h>
>> >
>> > @@ -41,7 +42,7 @@ STATIC CONST UINT32  mSpmMinorVerFfa =
>> SPM_MINOR_VERSION_FFA;
>> >
>> >
>> >   #define BOOT_PAYLOAD_VERSION  1
>> >
>> >
>> >
>> > -PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;
>> >
>> > +PI_MM_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;
>> >
>> >
>> >
>> >   /**
>> >
>> >     Retrieve a pointer to and print the boot information passed by
>> privileged
>> >
>> > @@ -140,6 +141,18 @@ DelegatedEventLoop (
>> >       DEBUG ((DEBUG_INFO, "X6 :  0x%x\n",
>> (UINT32)EventCompleteSvcArgs->Arg6));
>> >
>> >       DEBUG ((DEBUG_INFO, "X7 :  0x%x\n",
>> (UINT32)EventCompleteSvcArgs->Arg7));
>> >
>> >
>> >
>> > +    //
>> >
>> > +    // ARM TF passes SMC FID of the MM_COMMUNICATE interface as the
>> Event ID upon
>> >
>> > +    // receipt of a synchronous MM request. Use the Event ID to
>> distinguish
>> >
>> > +    // between synchronous and asynchronous events.
>> >
>> > +    //
>> >
>> > +    if ((ARM_SMC_ID_MM_COMMUNICATE !=
>> (UINT32)EventCompleteSvcArgs->Arg0) &&
>> >
>> > +        (ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ !=
>> (UINT32)EventCompleteSvcArgs->Arg0))
>> >
>> > +    {
>> >
>> > +      DEBUG ((DEBUG_ERROR, "UnRecognized Event - 0x%x\n",
>> (UINT32)EventCompleteSvcArgs->Arg0));
>> >
>> > +      continue;
>>
>> [SAMI] I think an error needs to be returned instead of continuing
>> otherwise this changes the original behaviour.
>>
>> Status needs to be set to EFI_INVALID_PARAMETER here.
>>
>> [/SAMI]
>
>
>> >
>> > +    }
>>
>> [SAMI] The code from this point needs to be enclosed in an else block
>> until before the switch statement.
>>
>> That way the proper error code would be returned. Can you check, please?
>>
>> [/SAMI]
>>
> [Tuan] Thanks for the catch. Will fix it
>
>>
>> >
>> > +
>> >
>> >       FfaEnabled = FeaturePcdGet (PcdFfaEnable);
>> >
>> >       if (FfaEnabled) {
>> >
>> >         Status = CpuDriverEntryPoint (
>> >
>> > diff --git
>> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
>> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
>> > index 75cfb98c0e75..d41d7630b614 100644
>> > ---
>> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
>> > +++
>> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
>> > @@ -49,7 +49,7 @@
>> >     gMpInformationHobGuid
>> >
>> >     gEfiMmPeiMmramMemoryReserveGuid
>> >
>> >     gEfiStandaloneMmNonSecureBufferGuid
>> >
>> > -  gEfiArmTfCpuDriverEpDescriptorGuid
>> >
>> > +  gEfiMmCpuDriverEpDescriptorGuid
>> >
>> >
>> >
>> >   [FeaturePcd.ARM, FeaturePcd.AARCH64]
>> >
>> >     gArmTokenSpaceGuid.PcdFfaEnable
>> >
>>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109180): https://edk2.groups.io/g/devel/message/109180
Mute This Topic: https://groups.io/mt/101369647/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

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

end of thread, other threads:[~2023-09-28 21:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-14 23:10 [edk2-devel] [PATCH v2 0/2] StandaloneMmPkg: Make StandaloneMmCpu platform Tuan Phan
2023-09-14 23:10 ` [edk2-devel] [PATCH v2 1/2] StandaloneMmPkg: Make StandaloneMmCpu driver platform independent Tuan Phan
2023-09-27 10:10   ` levi.yun
2023-09-28 16:12   ` Sami Mujawar
2023-09-14 23:10 ` [edk2-devel] [PATCH v2 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver Tuan Phan
2023-09-27 10:10   ` levi.yun
2023-09-28 16:16   ` Sami Mujawar
2023-09-28 18:16     ` Tuan Phan
     [not found]     ` <178922C843CC81E7.9230@groups.io>
2023-09-28 21:14       ` Tuan Phan
2023-09-26 16:38 ` [edk2-devel] [PATCH v2 0/2] StandaloneMmPkg: Make StandaloneMmCpu platform Tuan Phan

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