Re: [PATCH 22/41] hw/intc/arm_gicv3: Implement GICv4's new redistributor frame

[Richard Henderson]

On 4/8/22 07:15, Peter Maydell wrote:
> diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
> index 7c75dd6f072..9f1fe09a78e 100644
> --- a/hw/intc/arm_gicv3_redist.c
> +++ b/hw/intc/arm_gicv3_redist.c
> @@ -442,8 +442,8 @@ MemTxResult gicv3_redist_read(void *opaque, hwaddr offset, 
> uint64_t *data,
>        * in the memory map); if so then the GIC has multiple MemoryRegions
>        * for the redistributors.
>        */
> -    cpuidx = region->cpuidx + offset / GICV3_REDIST_SIZE;
> -    offset %= GICV3_REDIST_SIZE;
> +    cpuidx = region->cpuidx + offset / gicv3_redist_size(s);
> +    offset %= gicv3_redist_size(s);
>   
>       cs = &s->cpu[cpuidx];
>   
> @@ -501,8 +501,8 @@ MemTxResult gicv3_redist_write(void *opaque, hwaddr offset, uint64_t data,
>        * in the memory map); if so then the GIC has multiple MemoryRegions
>        * for the redistributors.
>        */
> -    cpuidx = region->cpuidx + offset / GICV3_REDIST_SIZE;
> -    offset %= GICV3_REDIST_SIZE;
> +    cpuidx = region->cpuidx + offset / gicv3_redist_size(s);
> +    offset %= gicv3_redist_size(s);

In these two cases, it would be better to call the function only once.

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


r~