[edk2-devel] [PATCH v1 09/12] ArmPkg/StandaloneMmMmuLib: Add option to use FF-A calls to get memory region's permissions

[Sami Mujawar]

Hi Sughosh,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

-----Original Message-----
From: Sughosh Ganu <sughosh.ganu at linaro.org> 
Sent: 21 October 2020 12:33 PM
To: devel at edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel at arm.com>; Sami Mujawar <Sami.Mujawar at arm.com>; Jiewen Yao <jiewen.yao at intel.com>; Achin Gupta <Achin.Gupta at arm.com>
Subject: [PATCH v1 09/12] ArmPkg/StandaloneMmMmuLib: Add option to use FF-A calls to get memory region's permissions

[SAMI] Please abbreviate the subject line if possible. [/SAMI]

From: Achin Gupta <achin.gupta at arm.com>

Allow getting memory region's permissions using either of the Firmware
Framework(FF-A) ABI transport or through the earlier used SVC calls.

Signed-off-by: Achin Gupta <achin.gupta at arm.com>
Co-developed-by: Sughosh Ganu <sughosh.ganu at linaro.org>
---
 ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf       |  3 +++
 ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c | 28 +++++++++++++++-----
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
index 85973687f5..a29dd800b5 100644
--- a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
+++ b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
@@ -23,6 +23,9 @@
   ArmPkg/ArmPkg.dec
   MdePkg/MdePkg.dec
 
+[Pcd]
+  gArmTokenSpaceGuid.PcdFfaEnable
+
 [LibraryClasses]
   ArmLib
   CacheMaintenanceLib
diff --git a/ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c b/ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c
index 362b1a0f8a..ab13602556 100644
--- a/ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c
+++ b/ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c
@@ -16,6 +16,7 @@
 #include <Library/ArmSvcLib.h>
 #include <Library/BaseLib.h>
 #include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
 
 STATIC
 EFI_STATUS
@@ -25,19 +26,32 @@ GetMemoryPermissions (
   )
 {
   ARM_SVC_ARGS  GetMemoryPermissionsSvcArgs = {0};
-
-  GetMemoryPermissionsSvcArgs.Arg0 = ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64;
-  GetMemoryPermissionsSvcArgs.Arg1 = BaseAddress;
-  GetMemoryPermissionsSvcArgs.Arg2 = 0;
-  GetMemoryPermissionsSvcArgs.Arg3 = 0;
+  BOOLEAN FfaEnabled;
+
+  FfaEnabled = FeaturePcdGet (PcdFfaEnable);
+  if (FfaEnabled) {
+    GetMemoryPermissionsSvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64;
+    GetMemoryPermissionsSvcArgs.Arg1 = 0x3;
+    GetMemoryPermissionsSvcArgs.Arg2 = 0;
+    GetMemoryPermissionsSvcArgs.Arg3 = ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64;
+    GetMemoryPermissionsSvcArgs.Arg4 = BaseAddress;
+  } else {
+    GetMemoryPermissionsSvcArgs.Arg0 = ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64;
+    GetMemoryPermissionsSvcArgs.Arg1 = BaseAddress;
+    GetMemoryPermissionsSvcArgs.Arg2 = 0;
+    GetMemoryPermissionsSvcArgs.Arg3 = 0;
+  }
 
   ArmCallSvc (&GetMemoryPermissionsSvcArgs);
-  if (GetMemoryPermissionsSvcArgs.Arg0 == ARM_SVC_SPM_RET_INVALID_PARAMS) {
+  if (GetMemoryPermissionsSvcArgs.Arg0 == ARM_SVC_SPM_RET_INVALID_PARAMS ||
+      GetMemoryPermissionsSvcArgs.Arg3 == ARM_SVC_SPM_RET_INVALID_PARAMS) {

[SAMI] Other error codes can also be returned and must be checked. 
See FFA Spec, section 10.2 and also https://trustedfirmware-a.readthedocs.io/en/latest/components/secure-partition-manager-mm.html#mm-sp-memory-attributes-get-aarch64

For FFA the error codes would be returned in Arg0 & Arg3. For SPM based on the MM interface the error code would be in Arg0.
It might be better to check if FfaEnabled is enabled here and handle the errors appropriately. Also, it could be combined with the following code that returns the Memory attributes.
[/SAMI]

     *MemoryAttributes = 0;
     return EFI_INVALID_PARAMETER;
   }
 
-  *MemoryAttributes = GetMemoryPermissionsSvcArgs.Arg0;
+  *MemoryAttributes = FfaEnabled ?
+    GetMemoryPermissionsSvcArgs.Arg3 : GetMemoryPermissionsSvcArgs.Arg0;
+
   return EFI_SUCCESS;
 }
 
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67802): https://edk2.groups.io/g/devel/message/67802
Mute This Topic: https://groups.io/mt/77703657/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-