Re: [PATCH 28/41] hw/intc/arm_gicv3_redist: Factor out "update hpplpi for all LPIs" logic

[Richard Henderson]

On 4/8/22 07:15, Peter Maydell wrote:
> Factor out the common part of gicv3_redist_update_lpi_only() into
> a new function update_for_all_lpis(), which does a full rescan
> of an LPI Pending table and sets the specified PendingIrq struct
> with the highest priority pending enabled LPI it finds.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/intc/arm_gicv3_redist.c | 66 ++++++++++++++++++++++++++------------
>   1 file changed, 46 insertions(+), 20 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
> index 571e0fa8309..2379389d14e 100644
> --- a/hw/intc/arm_gicv3_redist.c
> +++ b/hw/intc/arm_gicv3_redist.c
> @@ -103,6 +103,48 @@ static void update_for_one_lpi(GICv3CPUState *cs, int irq,
>       }
>   }
>   
> +/**
> + * update_for_all_lpis: Fully scan LPI tables and find best pending LPI
> + *
> + * @cs: GICv3CPUState
> + * @ptbase: physical address of LPI Pending table
> + * @ctbase: physical address of LPI Configuration table
> + * @ptsizebits: size of tables, specified as number of interrupt ID bits minus 
> 1
> + * @ds: true if priority value should not be shifted
> + * @hpp: points to pending information to set
> + *
> + * Recalculate the highest priority pending enabled LPI from scratch,
> + * and set @hpp accordingly.
> + *
> + * We scan the LPI pending table @ptbase; for each pending LPI, we read the
> + * corresponding entry in the LPI configuration table @ctbase to extract
> + * the priority and enabled information.
> + *
> + * We take @ptsizebits in the form idbits-1 because this is the way that
> + * LPI table sizes are architecturally specified in GICR_PROPBASER.IDBits
> + * and in the VMAPP command's VPT_size field.
> + */
> +static void update_for_all_lpis(GICv3CPUState *cs, uint64_t ptbase,
> +                                uint64_t ctbase, unsigned ptsizebits,
> +                                bool ds, PendingIrq *hpp)
> +{
> +    AddressSpace *as = &cs->gic->dma_as;
> +    uint8_t pend;
> +    uint32_t pendt_size = (1ULL << (ptsizebits + 1));

Code movement, so ok, but no need for ull.

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

r~