From ded1d5414b5a0161de8fcf234b7fb200fb59fb2c Mon Sep 17 00:00:00 2001
From: Sami Mujawar <sami.mujawar@arm.com>
Date: Fri, 19 May 2023 11:33:39 +0100
Subject: [PATCH] ArmPkg: Fix ArmGicAcknowledgeInterrupt () for GICv3

The ArmGicAcknowledgeInterrupt () returns the value returned by the
Interrupt Acknowledge Register and the InterruptID separately in an out
parameter.

The function documents the following: 'InterruptId is returned
separately from the register value because in the GICv2 the register
value contains the CpuId and InterruptId while in the GICv3 the register
value is only the InterruptId.'

This function skips setting the InterruptId in the out parameter for
GICv3. Although the return value from the function is the InterruptId
for GICv3, this breaks the function usage model as the caller expects
the InterruptId in the out parameter for the function.  e.g. The caller
may end up using the InterruptID which could potentially be an
uninitialised variable value.

Therefore, set the InterruptID in the function out parameter for GICv3
as well.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmPkg/Drivers/ArmGic/ArmGicLib.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
index 8f3315d76f..7f4bb248fc 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
@@ -176,19 +176,17 @@ ArmGicAcknowledgeInterrupt (
   )
 {
   UINTN                  Value;
+  UINTN                  IntId;
   ARM_GIC_ARCH_REVISION  Revision;
 
+  ASSERT (InterruptId != NULL);
   Revision = ArmGicGetSupportedArchRevision ();
   if (Revision == ARM_GIC_ARCH_REVISION_2) {
     Value = ArmGicV2AcknowledgeInterrupt (GicInterruptInterfaceBase);
-    // InterruptId is required for the caller to know if a valid or spurious
-    // interrupt has been read
-    ASSERT (InterruptId != NULL);
-    if (InterruptId != NULL) {
-      *InterruptId = Value & ARM_GIC_ICCIAR_ACKINTID;
-    }
+    IntId = Value & ARM_GIC_ICCIAR_ACKINTID;
   } else if (Revision == ARM_GIC_ARCH_REVISION_3) {
     Value = ArmGicV3AcknowledgeInterrupt ();
+    IntId = Value;
   } else {
     ASSERT_EFI_ERROR (EFI_UNSUPPORTED);
     // Report Spurious interrupt which is what the above controllers would
@@ -196,6 +194,12 @@ ArmGicAcknowledgeInterrupt (
     Value = 1023;
   }
 
+  if (InterruptId != NULL) {
+    // InterruptId is required for the caller to know if a valid or spurious
+    // interrupt has been read
+    *InterruptId = IntId;
+  }
+
   return Value;
 }