Re: [PATCH v5 09/12] xen/arm: domain_build/dom0less-build: adjust domains config to support eSPIs

[Julien Grall <julien@xxxxxxx>]

On 02/09/2025 18:15, Leonid Komarianskyi wrote:
> Hi Julien,
Hi Leonid,

> On 02.09.25 19:42, Julien Grall wrote:
> > Hi Leonid,
> >
> > On 29/08/2025 17:06, Leonid Komarianskyi wrote:
> > > The Dom0 and DomUs logic for the dom0less configuration in
> > > create_dom0() and
> > > arch_create_domUs() has been updated to account for extended SPIs when
> > > supported by the hardware and enabled with CONFIG_GICV3_ESPI.
> > Style: We don't commonly use past tense to describe the new behavior.
> I will update it in V6.
>
> > >    xen/arch/arm/dom0less-build.c   |  2 +-
> > >    xen/arch/arm/domain_build.c     |  2 +-
> > >    xen/arch/arm/include/asm/vgic.h | 19 +++++++++++++++++++
> > >    3 files changed, 21 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-
> > > build.c
> > > index 69b9ea22ce..02d5559102 100644
> > > --- a/xen/arch/arm/dom0less-build.c
> > > +++ b/xen/arch/arm/dom0less-build.c
> > > @@ -285,7 +285,7 @@ void __init arch_create_domUs(struct
> > > dt_device_node *node,
> > >        {
> > >            int vpl011_virq = GUEST_VPL011_SPI;
> > > -        d_cfg->arch.nr_spis = VGIC_DEF_NR_SPIS;
> > > +        d_cfg->arch.nr_spis = vgic_def_nr_spis();
> > >            /*
> > >             * The VPL011 virq is GUEST_VPL011_SPI, unless direct-map is
> > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > > index d91a71acfd..39eea0be00 100644
> > > --- a/xen/arch/arm/domain_build.c
> > > +++ b/xen/arch/arm/domain_build.c
> > > @@ -2054,7 +2054,7 @@ void __init create_dom0(void)
> > >        /* The vGIC for DOM0 is exactly emulating the hardware GIC */
> > >        dom0_cfg.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
> > > -    dom0_cfg.arch.nr_spis = VGIC_DEF_NR_SPIS;
> > > +    dom0_cfg.arch.nr_spis = vgic_def_nr_spis();
> > >        dom0_cfg.arch.tee_type = tee_get_type();
> > >        dom0_cfg.max_vcpus = dom0_max_vcpus();
> > > diff --git a/xen/arch/arm/include/asm/vgic.h b/xen/arch/arm/include/
> > > asm/vgic.h
> > > index 912d5b7694..3aa22114ba 100644
> > > --- a/xen/arch/arm/include/asm/vgic.h
> > > +++ b/xen/arch/arm/include/asm/vgic.h
> > > @@ -347,6 +347,25 @@ extern void
> > > vgic_check_inflight_irqs_pending(struct vcpu *v,
> > >    /* Default number of vGIC SPIs. 32 are substracted to cover local
> > > IRQs. */
> > >    #define VGIC_DEF_NR_SPIS (min(gic_number_lines(), VGIC_MAX_IRQS) - 32)>
> > > +static inline unsigned int vgic_def_nr_spis(void)
> > > +{
> > > +#ifdef CONFIG_GICV3_ESPI
> > > +    /*
> > > +     * Check if the hardware supports extended SPIs (even if the
> > > appropriate
> > > +     * config is set). If not, the common SPI range will be used.
> > > Otherwise
> > > +     * return the maximum eSPI INTID, supported by HW GIC, subtracted
> > > by 32.
> > > +     * For Dom0 and started at boot time DomUs we will add back this
> > > value
> > > +     * during VGIC initialization. This ensures consistent handling
> > > for Dom0
> > > +     * and other domains. For the regular SPI range interrupts in
> > > this case,
> > > +     * the maximum value of VGIC_DEF_NR_SPIS will be used.
> > > +     */
> > > +    if ( gic_number_espis() > 0 )
> > > +        return ESPI_BASE_INTID + min(gic_number_espis(), 1024U) - 32;
> > > +#endif
> > > +
> > > +    return VGIC_DEF_NR_SPIS;
> > This is the only user of VGIC_DEF_NR_SPIS. Therefore, I would prefer if
> > we remove the define. This will avoid any confusion between the helper
> > and the define.
> >
> > Cheers,
> Actually, we need this macro in the previous patch in the series:
> [08/12] xen/arm: vgic: add resource management for extended SPIs
> (domain_vgic_init):
>
>       if ( nr_spis + 32 >= ESPI_BASE_INTID )
>       {
>           d->arch.vgic.nr_espis = min(nr_spis - ESPI_BASE_INTID + 32, 1024U);
>           /* Verify if GIC HW can handle provided INTID */
>           if ( d->arch.vgic.nr_espis > gic_number_espis() )
>               return -EINVAL;
>           /*
>            * Set the maximum available number for regular
>            * SPI to pass the next check
>            */
>           nr_spis = VGIC_DEF_NR_SPIS;
>       }
>
> I have seen your comment regarding reworking the checks, but in any
> case, I still need this macro to assign the value to nr_spis, so I would
> prefer to leave it.
Ah! I didn't spot the other use. Then let's keep it for now.

Cheers,

--
Julien Grall