[1/3] hw/intc/arm_gicv3: Add cast to match the documentation

Commit Message

Alexandra Diupina Oct. 1, 2024, 9:51 a.m. UTC

The result of 1 << regbit with regbit==31 has a 1 in the 32nd bit.
When cast to uint64_t (for further bitwise OR), the 32 most
significant bits will be filled with 1s. However, the documentation
states that the upper 32 bits of ICH_AP[0/1]R<n>_EL2 are reserved.

Add an explicit cast to match the documentation.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: d2c0c6aab6 ("hw/intc/arm_gicv3: Handle icv_nmiar1_read() for icc_nmiar1_read()")
Signed-off-by: Alexandra Diupina <[e-mail]>
---
 hw/intc/arm_gicv3_cpuif.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Peter Maydell Oct. 4, 2024, 6:16 p.m. UTC | #1

On Tue, 1 Oct 2024 at 10:51, Alexandra Diupina <[e-mail]> wrote:
>
> The result of 1 << regbit with regbit==31 has a 1 in the 32nd bit.
> When cast to uint64_t (for further bitwise OR), the 32 most
> significant bits will be filled with 1s. However, the documentation
> states that the upper 32 bits of ICH_AP[0/1]R<n>_EL2 are reserved.
>
> Add an explicit cast to match the documentation.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: d2c0c6aab6 ("hw/intc/arm_gicv3: Handle icv_nmiar1_read() for icc_nmiar1_read()")
> Signed-off-by: Alexandra Diupina <[e-mail]>

Yep, these look definitely like fixes for wrong code.
I've applied these to target-arm.next (with a cc to stable).

PS: for future multi-patch submissions would you mind
including a cover letter (of the form generated by
'git format-patch --cover-letter')? Our automated tools
expect the cover letter, so patchsets are a bit easier
to process if there is one. (Single patches don't need
a cover letter.)

thanks
-- PMM

Patch

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index bdb13b00e9..ebad7aaea1 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -781,7 +781,7 @@  static void icv_activate_irq(GICv3CPUState *cs, int idx, int grp)
     if (nmi) {
         cs->ich_apr[grp][regno] |= ICV_AP1R_EL1_NMI;
     } else {
-        cs->ich_apr[grp][regno] |= (1 << regbit);
+        cs->ich_apr[grp][regno] |= (1U << regbit);
     }
 }