Re: [PATCH v7 05/12] xen/arm: gicv3: implement handling of GICv3.1 eSPI

[Julien Grall <julien@xxxxxxx>]

Hi Leonid,

On 04/09/2025 21:01, Leonid Komarianskyi wrote:
> +#ifdef CONFIG_GICV3_ESPI
> +unsigned int gic_number_espis(void)
> +{
> +    return gic_hw_ops->info->nr_espi;
> +}
> +
> +static void __init gicv3_dist_espi_common_init(uint32_t type)
> +{
> +    unsigned int espi_nr, i;
> +
> +    espi_nr = min(1024U, GICD_TYPER_ESPIS_NUM(type));
> +    gicv3_info.nr_espi = espi_nr;
> +    /* The GIC HW doesn't support eSPI, so we can leave from here */
> +    if ( gicv3_info.nr_espi == 0 )
> +        return;
> +
> +    printk("GICv3: %d eSPI lines\n", gicv3_info.nr_espi);
Style: nr_espi is unsigned. So it should be %u. Can be fixed on commit 
if there is nothing else major to change.
> +
> +    /* The configuration for eSPIs is similar to that for regular SPIs */
> +    for ( i = 0; i < espi_nr; i += 16 )
> +        writel_relaxed(0, GICD + GICD_ICFGRnE + (i / 16) * 4);
> +
> +    for ( i = 0; i < espi_nr; i += 4 )
> +        writel_relaxed(GIC_PRI_IRQ_ALL,
> +                       GICD + GICD_IPRIORITYRnE + (i / 4) * 4);
> +
> +    for ( i = 0; i < espi_nr; i += 32 )
> +    {
> +        writel_relaxed(GENMASK(31, 0), GICD + GICD_ICENABLERnE + (i / 32) * 4);
Sorry I only spotted now.

The goal of gicv3_dist_espi_common_init() is to make sure the GIC is in 
a sane state for Xen (the previous OS or firmware may have left some 
state). Now the firmware may decide to use eSPI but not Xen (e.g. 
because CONFIG_ESPI=n).
This would means we may have eSPI interrupts that may be enabled and 
pending. So as soon as we re-enable the GIC we may receive interrupts we 
can't handle. So I think we may need to initialize the eSPI part of the 
distributor unconditionally. What do you think?

This could be handled as a follow-up but within the timeframe of Xen 
4.21. So for this patch:
Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall