From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.22679.1684849149017923241 for ; Tue, 23 May 2023 06:39:09 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=s/OuEhk2; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 877C36319F for ; Tue, 23 May 2023 13:39:08 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id EBBF1C433D2 for ; Tue, 23 May 2023 13:39:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1684849148; bh=EvD6eE0nnJOycSUII59GZyR6LWp8y6uqiFe2K2wJSRg=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=s/OuEhk2rC9HMIsT/wLriGIPbfdOWozaWJZjijXTOi4Fe7QVIT3SR2UA1nyr5HLMq bF7yZn7g6JhpFlf2A3f/KYXmJscZbxrLbhWuefKSuUpae0jwI5/Kxllu+o7sbJ33px U8LB387gBW0/l3y06KM5CnMbNi02k3nDLwaWrTV4kPO+YGKhfLOzHqUF8Bd3o4iM+f h1uav8mkEgum4O4qfMKlRAc8bJ8Dytc6OxKlTz+mSD+1UIkrun0GGN0JcpFlr+jqOX Q3wXg1EMRwcnMgD4+1lbTJSOe0oYUDl0gEPuKcUmJ5WPmETxJ62jBiRVZvGadqBhR1 Nj+PG4r4L5b1Q== Received: by mail-lf1-f51.google.com with SMTP id 2adb3069b0e04-4f3b9c88af8so3635664e87.2 for ; Tue, 23 May 2023 06:39:07 -0700 (PDT) X-Gm-Message-State: AC+VfDx83VA+uy+qGlUduqSWuC35VAlJyIvc5vesnwP9r1KRWll1o8n/ 6GR69RJObAe/HR1fRnr8Brvqs0Cyswzou0H724c= X-Google-Smtp-Source: ACHHUZ68vdbjecB+AojLvm7Hem7JVR8jrmN2vMxEho7R3n8UgQhQLxwnRi8FSbnAhTlR8wZt1BIpmTtnWHR0XI6+mCM= X-Received: by 2002:ac2:4f8a:0:b0:4ec:8853:136 with SMTP id z10-20020ac24f8a000000b004ec88530136mr4039316lfs.12.1684849145950; Tue, 23 May 2023 06:39:05 -0700 (PDT) MIME-Version: 1.0 References: <20230523130421.10804-1-sami.mujawar@arm.com> <20230523130421.10804-13-sami.mujawar@arm.com> In-Reply-To: <20230523130421.10804-13-sami.mujawar@arm.com> From: "Ard Biesheuvel" Date: Tue, 23 May 2023 15:38:54 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v1 12/12] ArmPkg: Fix ArmGicAcknowledgeInterrupt () for GICv3 To: Sami Mujawar Cc: devel@edk2.groups.io, ardb+tianocore@kernel.org, quic_llindhol@quicinc.com, neil.jones@blaize.com, pedro.falcato@gmail.com, pierre.gondois@arm.com, Matteo.Carlini@arm.com, Akanksha.Jain2@arm.com, Ben.Adderson@arm.com, Sibel.Allinson@arm.com, nd@arm.com Content-Type: text/plain; charset="UTF-8" On Tue, 23 May 2023 at 15:04, Sami Mujawar 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 Reviewed-by: Ard Biesheuvel > --- > 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)' >