On Tue, 23 May 2023 at 15:04, Sami Mujawar <sami.mujawar@arm.com> wrote:
>
> 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 8f3315d76f6f2b28a551d73400938430ff3e23c7..7f4bb248fc7225bf63f0aea720486092b30ced10 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;
> }
>
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105194): https://edk2.groups.io/g/devel/message/105194
Mute This Topic: https://groups.io/mt/99086465/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-