public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Sami Mujawar" <sami.mujawar@arm.com>
To: <devel@edk2.groups.io>
Cc: Sami Mujawar <sami.mujawar@arm.com>, <ardb+tianocore@kernel.org>,
	<leif@nuviainc.com>, <Matteo.Carlini@arm.com>,
	<Ben.Adderson@arm.com>, <nd@arm.com>
Subject: [PATCH v1 1/1] ArmPkg: Fix uninitialised variable in ArmMmuStandaloneMmLib
Date: Wed, 24 Feb 2021 19:37:56 +0000	[thread overview]
Message-ID: <20210224193756.24132-1-sami.mujawar@arm.com> (raw)

The following patches added support for StandaloneMM using FF-A:
9da5ee116a28 ArmPkg: Allow FF-A calls to set memory region's attributes
0e43e02b9bd8 ArmPkg: Allow FF-A calls to get memory region's attributes

However, the error handling logic for the Get/Set Memory attributes
introduced an issue wherein a status variable could be used without
initialisation. This issue is reported by CLANG compiler and is not
seen with GCC.

The Get/Set Memory attributes operation is atomic and therefore an
FFA_INTERRUPT or FFA_SUCCESS response is not expected in response
to FFA_MSG_SEND_DIRECT_REQ. So the remaining cases that could occur
are:
 - the target sends FFA_MSG_SEND_DIRECT_RESP with a success or
   failure code.
 or
 - FFA_MSG_SEND_DIRECT_REQ transmission failure.

Therefore, reorder the error handling conditions such that the
uninitialised variable issue is fixed.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
The changes can be seen at:
https://github.com/samimujawar/edk2/tree/1657_stmm_ffa_fix_unused_var_v1

 ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c | 92 ++++++++++----------
 1 file changed, 45 insertions(+), 47 deletions(-)

diff --git a/ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c b/ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c
index a30369af9c91fb8045dfec7a68e2bd072706d101..73b63ca396e5395bdf2112709b0aa2ab871a2a07 100644
--- a/ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c
+++ b/ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c
@@ -57,36 +57,35 @@ GetMemoryPermissions (
     // for other Direct Request calls which are not atomic
     // We therefore check only for Direct Response by the
     // callee.
-    if (GetMemoryPermissionsSvcArgs.Arg0 !=
+    if (GetMemoryPermissionsSvcArgs.Arg0 ==
         ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64) {
-      // If Arg0 is not a Direct Response, that means we
-      // have an FF-A error. We need to check Arg2 for the
-      // FF-A error code.
-      Ret = GetMemoryPermissionsSvcArgs.Arg2;
-      switch (Ret) {
-      case ARM_FFA_SPM_RET_INVALID_PARAMETERS:
-
-        return EFI_INVALID_PARAMETER;
-
-      case ARM_FFA_SPM_RET_DENIED:
-        return EFI_NOT_READY;
-
-      case ARM_FFA_SPM_RET_NOT_SUPPORTED:
-        return EFI_UNSUPPORTED;
-
-      case ARM_FFA_SPM_RET_BUSY:
-        return EFI_NOT_READY;
-
-      case ARM_FFA_SPM_RET_ABORTED:
-        return EFI_ABORTED;
-      }
-    } else if (GetMemoryPermissionsSvcArgs.Arg0 ==
-               ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64) {
       // A Direct Response means FF-A success
       // Now check the payload for errors
       // The callee sends back the return value
       // in Arg3
       Ret = GetMemoryPermissionsSvcArgs.Arg3;
+    } else {
+      // If Arg0 is not a Direct Response, that means we
+      // have an FF-A error. We need to check Arg2 for the
+      // FF-A error code.
+      Ret = GetMemoryPermissionsSvcArgs.Arg2;
+      switch (Ret) {
+      case ARM_FFA_SPM_RET_INVALID_PARAMETERS:
+
+        return EFI_INVALID_PARAMETER;
+
+      case ARM_FFA_SPM_RET_DENIED:
+        return EFI_NOT_READY;
+
+      case ARM_FFA_SPM_RET_NOT_SUPPORTED:
+        return EFI_UNSUPPORTED;
+
+      case ARM_FFA_SPM_RET_BUSY:
+        return EFI_NOT_READY;
+
+      case ARM_FFA_SPM_RET_ABORTED:
+        return EFI_ABORTED;
+      }
     }
   } else {
     Ret = GetMemoryPermissionsSvcArgs.Arg0;
@@ -150,35 +149,34 @@ RequestMemoryPermissionChange (
     // for other Direct Request calls which are not atomic
     // We therefore check only for Direct Response by the
     // callee.
-    if (ChangeMemoryPermissionsSvcArgs.Arg0 !=
+    if (ChangeMemoryPermissionsSvcArgs.Arg0 ==
         ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64) {
-      // If Arg0 is not a Direct Response, that means we
-      // have an FF-A error. We need to check Arg2 for the
-      // FF-A error code.
-      Ret = ChangeMemoryPermissionsSvcArgs.Arg2;
-      switch (Ret) {
-      case ARM_FFA_SPM_RET_INVALID_PARAMETERS:
-        return EFI_INVALID_PARAMETER;
-
-      case ARM_FFA_SPM_RET_DENIED:
-        return EFI_NOT_READY;
-
-      case ARM_FFA_SPM_RET_NOT_SUPPORTED:
-        return EFI_UNSUPPORTED;
-
-      case ARM_FFA_SPM_RET_BUSY:
-        return EFI_NOT_READY;
-
-      case ARM_FFA_SPM_RET_ABORTED:
-        return EFI_ABORTED;
-      }
-    } else if (ChangeMemoryPermissionsSvcArgs.Arg0 ==
-               ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64) {
       // A Direct Response means FF-A success
       // Now check the payload for errors
       // The callee sends back the return value
       // in Arg3
       Ret = ChangeMemoryPermissionsSvcArgs.Arg3;
+    } else {
+      // If Arg0 is not a Direct Response, that means we
+      // have an FF-A error. We need to check Arg2 for the
+      // FF-A error code.
+      Ret = ChangeMemoryPermissionsSvcArgs.Arg2;
+      switch (Ret) {
+      case ARM_FFA_SPM_RET_INVALID_PARAMETERS:
+        return EFI_INVALID_PARAMETER;
+
+      case ARM_FFA_SPM_RET_DENIED:
+        return EFI_NOT_READY;
+
+      case ARM_FFA_SPM_RET_NOT_SUPPORTED:
+        return EFI_UNSUPPORTED;
+
+      case ARM_FFA_SPM_RET_BUSY:
+        return EFI_NOT_READY;
+
+      case ARM_FFA_SPM_RET_ABORTED:
+        return EFI_ABORTED;
+      }
     }
   } else {
     Ret = ChangeMemoryPermissionsSvcArgs.Arg0;
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


             reply	other threads:[~2021-02-24 19:38 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-24 19:37 Sami Mujawar [this message]
2021-02-25 11:37 ` [PATCH v1 1/1] ArmPkg: Fix uninitialised variable in ArmMmuStandaloneMmLib Leif Lindholm
2021-02-25 12:50   ` Sami Mujawar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210224193756.24132-1-sami.mujawar@arm.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox