Re: [PATCH 32/41] hw/intc/arm_gicv3_redist: Implement gicv3_redist_vlpi_pending()

[Richard Henderson]

On 4/8/22 07:15, Peter Maydell wrote:
> Implement the function gicv3_redist_vlpi_pending(), which was
> previously left as a stub.  This is the function that is called by
> the CPU interface when it changes the state of a vLPI.  It's similar
> to gicv3_redist_process_vlpi(), but we know that the vCPU is
> definitely resident on the redistributor and the irq is in range, so
> it is a bit simpler.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/intc/arm_gicv3_redist.c | 23 +++++++++++++++++++++--
>   1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
> index be36978b45c..eadf5e8265e 100644
> --- a/hw/intc/arm_gicv3_redist.c
> +++ b/hw/intc/arm_gicv3_redist.c
> @@ -1009,9 +1009,28 @@ void gicv3_redist_movall_lpis(GICv3CPUState *src, 
> GICv3CPUState *dest)
>   void gicv3_redist_vlpi_pending(GICv3CPUState *cs, int irq, int level)
>   {
>       /*
> -     * The redistributor handling for changing the pending state
> -     * of a vLPI will be added in a subsequent commit.
> +     * Change the pending state of the specified vLPI.
> +     * Unlike gicv3_redist_process_vlpi(), we know here that the
> +     * vCPU is definitely resident on this redistributor, and that
> +     * the irq is in range.
>        */
> +    uint64_t vptbase, ctbase;
> +
> +    vptbase = FIELD_EX64(cs->gicr_vpendbaser, GICR_VPENDBASER, PHYADDR) << 16;
> +
> +    if (set_pending_table_bit(cs, vptbase, irq, level)) {
> +        if (level) {
> +            /* Check whether this vLPI is now the best */
> +            ctbase = cs->gicr_vpropbaser & R_GICR_VPROPBASER_PHYADDR_MASK;
> +            update_for_one_lpi(cs, irq, ctbase, true, &cs->hppvlpi);
> +            gicv3_cpuif_virt_irq_fiq_update(cs);
> +        } else {
> +            /* Only need to recalculate if this was previously the best vLPI */
> +            if (irq == cs->hppvlpi.irq) {
> +                gicv3_redist_update_vlpi(cs);
> +            }
> +        }
> +    }

I'll note that looks a lot like the previous patch, modulo "resident".
Perhaps this could be better factored?

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


r~