[PATCH 1/2] target/arm: GICv5 cpuif: gicr_cdia_read() ppibit should be 64 bits

Peter Maydell posted 2 patches 1 month, 3 weeks ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>
[PATCH 1/2] target/arm: GICv5 cpuif: gicr_cdia_read() ppibit should be 64 bits
Posted by Peter Maydell 1 month, 3 weeks ago
In gicr_cdia_read() we turn a PPI interrupt ID into a register
index and a bit mask with a 1 for the bit we want to change:

        ppireg = id / 64;
        ppibit = 1ULL << (id % 64);

However, we used the wrong type for ppibit, making it a uint32_t.  If
'id' is too large we'll shift off the end, so we won't ever update
the state of PPIs with indexes above 31.

This didn't have any visible effects because the currently allocated
architected PPIs are indexes 0..31, so you'd only see this if for
some reason a guest was manually marking as pending a PPI in 32..63.

Fix the type of ppibit to the intended 64 bit width.

Fixes: 3f79212abae89 ("target/arm: GICv5 cpuif: Implement GICR CDIA command")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/tcg/gicv5-cpuif.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/arm/tcg/gicv5-cpuif.c b/target/arm/tcg/gicv5-cpuif.c
index 1cdd4103d0e..8e2a08fdb6f 100644
--- a/target/arm/tcg/gicv5-cpuif.c
+++ b/target/arm/tcg/gicv5-cpuif.c
@@ -633,7 +633,8 @@ static uint64_t gicr_cdia_read(CPUARMState *env, const ARMCPRegInfo *ri)
     switch (type) {
     case GICV5_PPI:
     {
-        uint32_t ppireg, ppibit;
+        uint32_t ppireg;
+        uint64_t ppibit;
 
         assert(id < GICV5_NUM_PPIS);
         ppireg = id / 64;
-- 
2.43.0
Re: [PATCH 1/2] target/arm: GICv5 cpuif: gicr_cdia_read() ppibit should be 64 bits
Posted by Richard Henderson 1 month, 3 weeks ago
On 7/14/26 02:18, Peter Maydell wrote:
> In gicr_cdia_read() we turn a PPI interrupt ID into a register
> index and a bit mask with a 1 for the bit we want to change:
> 
>          ppireg = id / 64;
>          ppibit = 1ULL << (id % 64);
> 
> However, we used the wrong type for ppibit, making it a uint32_t.  If
> 'id' is too large we'll shift off the end, so we won't ever update
> the state of PPIs with indexes above 31.
> 
> This didn't have any visible effects because the currently allocated
> architected PPIs are indexes 0..31, so you'd only see this if for
> some reason a guest was manually marking as pending a PPI in 32..63.
> 
> Fix the type of ppibit to the intended 64 bit width.
> 
> Fixes: 3f79212abae89 ("target/arm: GICv5 cpuif: Implement GICR CDIA command")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/tcg/gicv5-cpuif.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Re: [PATCH 1/2] target/arm: GICv5 cpuif: gicr_cdia_read() ppibit should be 64 bits
Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago
On 14/7/26 11:18, Peter Maydell wrote:
> In gicr_cdia_read() we turn a PPI interrupt ID into a register
> index and a bit mask with a 1 for the bit we want to change:
> 
>          ppireg = id / 64;
>          ppibit = 1ULL << (id % 64);
> 
> However, we used the wrong type for ppibit, making it a uint32_t.  If
> 'id' is too large we'll shift off the end, so we won't ever update
> the state of PPIs with indexes above 31.
> 
> This didn't have any visible effects because the currently allocated
> architected PPIs are indexes 0..31, so you'd only see this if for
> some reason a guest was manually marking as pending a PPI in 32..63.
> 
> Fix the type of ppibit to the intended 64 bit width.
> 
> Fixes: 3f79212abae89 ("target/arm: GICv5 cpuif: Implement GICR CDIA command")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/tcg/gicv5-cpuif.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>