Hi Julien,
Thank you for your review.
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.
Best regards,
Leonid
