diff mbox series

[03/41] hw/intc/arm_gicv3: Insist that redist region capacity matches CPU count

Message ID 20220408141550.1271295-4-peter.maydell@linaro.org
State Superseded
Headers show
Series arm: Implement GICv4 | expand

Commit Message

Peter Maydell April 8, 2022, 2:15 p.m. UTC
Boards using the GICv3 need to configure it with both the total
number of CPUs and also the sizes of all the memory regions which
contain redistributors (one redistributor per CPU).  At the moment
the GICv3 checks that the number of CPUs specified is not too many to
fit in the defined redistributor regions, but in fact the code
assumes that the two match exactly.  For instance when we set the
GICR_TYPER.Last bit on the final redistributor in each region, we
assume that we don't need to consider the possibility of a region
being only half full of redistributors or even completely empty.  We
also assume in gicv3_redist_read() and gicv3_redist_write() that we
can calculate the CPU index from the offset within the MemoryRegion
and that this will always be in range.

Fortunately all the board code sets the redistributor region sizes to
exactly match the CPU count, so this isn't a visible bug.  We could
in theory make the GIC code handle non-full redistributor regions, or
have it automatically reduce the provided region sizes to match the
CPU count, but the simplest thing is just to strengthen the error
check and insist that the CPU count and redistributor region size
settings match exactly, since all the board code does that anyway.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/arm_gicv3_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Richard Henderson April 8, 2022, 11:20 p.m. UTC | #1
On 4/8/22 07:15, Peter Maydell wrote:
> Boards using the GICv3 need to configure it with both the total
> number of CPUs and also the sizes of all the memory regions which
> contain redistributors (one redistributor per CPU).  At the moment
> the GICv3 checks that the number of CPUs specified is not too many to
> fit in the defined redistributor regions, but in fact the code
> assumes that the two match exactly.  For instance when we set the
> GICR_TYPER.Last bit on the final redistributor in each region, we
> assume that we don't need to consider the possibility of a region
> being only half full of redistributors or even completely empty.  We
> also assume in gicv3_redist_read() and gicv3_redist_write() that we
> can calculate the CPU index from the offset within the MemoryRegion
> and that this will always be in range.
> 
> Fortunately all the board code sets the redistributor region sizes to
> exactly match the CPU count, so this isn't a visible bug.  We could
> in theory make the GIC code handle non-full redistributor regions, or
> have it automatically reduce the provided region sizes to match the
> CPU count, but the simplest thing is just to strengthen the error
> check and insist that the CPU count and redistributor region size
> settings match exactly, since all the board code does that anyway.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/intc/arm_gicv3_common.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

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

r~
diff mbox series

Patch

diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 90204be25b6..c797c82786b 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -354,9 +354,9 @@  static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < s->nb_redist_regions; i++) {
         rdist_capacity += s->redist_region_count[i];
     }
-    if (rdist_capacity < s->num_cpu) {
+    if (rdist_capacity != s->num_cpu) {
         error_setg(errp, "Capacity of the redist regions(%d) "
-                   "is less than number of vcpus(%d)",
+                   "does not match the number of vcpus(%d)",
                    rdist_capacity, s->num_cpu);
         return;
     }