ArmGicIsInterruptEnabled returns incorrect value


Robbie King
 

Hello folks,

I am running the SBSA Conformance Suite of tests on a new platform, and have come across an issue with the routine
which tests if an interrupt is enabled on an ARM processor. The routine in question is:

ArmPkg/Drivers/ArmGic/ArmGicLib.c:ArmGicIsInterruptEnabled()

The issue appears to have been introduced by the following commit:

41fb5d4634c17c042e0a3b2be0e8db85d2a083ad : "ArmPkg/ArmGic: Use the GIC Redistributor instead of GIC Distributor for GICv3"

https://github.com/tianocore/edk2/commit/41fb5d4634c17c042e0a3b2be0e8db85d2a083ad#diff-530eb470563a5e128777a19c1fdae116ed435b6b447459f679ad370008a50b67

The changes to ArmGicIsInterruptEnabled() introduced the error where the Boolean result is assigned to "Interrupts", but then
the bit position check is performed again (against the computed Boolean result instead of the interrupt mask) during the return statement.

I have posted the fix I'm running with here, would appreciate any guidance on how to go about upstreaming this.

ArmGicIsInterruptEnabled returns incorrect value by robbieking64 * Pull Request #1 * robbieking64/edk2 * GitHub<https://github.com/robbieking64/edk2/pull/1/files>

Thanks!
Robbie King


Sami Mujawar
 

Hi Robbie,

Thank you for reporting this issue and for the fix.
Is it possible to submit this patch to the EDK2 mailing list at devel@edk2.groups.io copying the maintainers, please?

More information on how to contribute to EDK2 can be found at:
https://github.com/tianocore/tianocore.github.io/wiki/How-To-Contribute
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process

Also, you could remove the additional brackets enclosing the call to MmioRead32().

Regards,

Sami Mujawar


´╗┐On 30/06/2022, 23:05, "discuss@edk2.groups.io on behalf of Robbie King via groups.io" <discuss@edk2.groups.io on behalf of robbiek@...> wrote:

Hello folks,

I am running the SBSA Conformance Suite of tests on a new platform, and have come across an issue with the routine
which tests if an interrupt is enabled on an ARM processor. The routine in question is:

ArmPkg/Drivers/ArmGic/ArmGicLib.c:ArmGicIsInterruptEnabled()

The issue appears to have been introduced by the following commit:

41fb5d4634c17c042e0a3b2be0e8db85d2a083ad : "ArmPkg/ArmGic: Use the GIC Redistributor instead of GIC Distributor for GICv3"

https://github.com/tianocore/edk2/commit/41fb5d4634c17c042e0a3b2be0e8db85d2a083ad#diff-530eb470563a5e128777a19c1fdae116ed435b6b447459f679ad370008a50b67

The changes to ArmGicIsInterruptEnabled() introduced the error where the Boolean result is assigned to "Interrupts", but then
the bit position check is performed again (against the computed Boolean result instead of the interrupt mask) during the return statement.

I have posted the fix I'm running with here, would appreciate any guidance on how to go about upstreaming this.

ArmGicIsInterruptEnabled returns incorrect value by robbieking64 * Pull Request #1 * robbieking64/edk2 * GitHub<https://github.com/robbieking64/edk2/pull/1/files>

Thanks!
Robbie King






IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.