Hi Pedro,

Thank you for the feedback.

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On Tue, May 23, 2023 at 2:04 PM Sami Mujawar <sami.mujawar@arm.com> wrote:

The IAR register of the Gic CPU interface is 32 bit, while
the value returned by ArmGicV2AcknowledgeInterrupt() is
UINTN. Therefore, typecast the return value to UINTN before
returning.

Since IAR is 32-bit, doesn't it make sense to return UINT32 here? Is
this for compatibility with GICv3 or...?

[SAMI] IAR in GICv3 is 32-bit as well. I think this is done to keep compatibility with HARDWARE_INTERRUPT_SOURCE which is UINTN.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
 ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c
index f403bec367b5254c248e620e56471904e520f9f2..80115b243afabd5e4faad88089af738b19ce4cd1 100644
--- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c
+++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c
@@ -16,7 +16,7 @@ ArmGicV2AcknowledgeInterrupt (
   )
 {
   // Read the Interrupt Acknowledge Register
-  return MmioRead32 (GicInterruptInterfaceBase + ARM_GIC_ICCIAR);
+  return (UINTN)MmioRead32 (GicInterruptInterfaceBase + ARM_GIC_ICCIAR);
 }

You don't need to cast from UINT32 to UINTN IMO. UINTN is, as far as I
know, always going to be at least 32-bits, so casting makes little
sense here, in my view, as UINTN >= UINT32.