Hi Oleksandr,
Thank you for your review.
On 28.08.25 16:19, Oleksandr Tyshchenko wrote:
>
>
> On 27.08.25 21:24, Leonid Komarianskyi wrote:
>
> Hello Leonid
>
>> 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. These
>> changes
>> ensure the proper calculation of the maximum number of SPIs and eSPIs
>> available
>> to Dom0 and DomUs in dom0less setups.
>>
>> When eSPIs are supported by the hardware and CONFIG_GICV3_ESPI is
>> enabled, the
>> maximum number of eSPI interrupts is calculated using the ESPI_BASE_INTID
>> offset (4096) and is limited to 1024, with 32 IRQs subtracted. To ensure
>> compatibility with non-Dom0 domains, this adjustment is applied by the
>> toolstack during domain creation, while for Dom0 or DomUs in Dom0, it is
>> handled directly during VGIC initialization. If eSPIs are not
>> supported, the
>> calculation defaults to using the standard SPI range, with a maximum
>> value of
>> 992 interrupt lines, as it works currently.
>>
>> Signed-off-by: Leonid Komarianskyi <leonid_komarians...@epam.com>
>>
>> ---
>> Changes in V4:
>> - consolidated the eSPI and SPI logic into a new inline function,
>> vgic_def_nr_spis. Without eSPI support (either due to config being
>> disabled or hardware not supporting it), it will return the regular
>> SPI
>> range, as it works currently. There are no functional changes compared
>> with the previous patch version
>> - removed VGIC_DEF_MAX_SPI macro, to reduce the number of ifdefs
>
> very good, thanks, only NIT below
>
> Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>
>
>
>
>>
>> Changes in V3:
>> - renamed macro VGIC_DEF_NR_ESPIS to more appropriate VGIC_DEF_MAX_SPI
>> - added eSPI initialization for dom0less setups
>> - fixed comment with mentions about dom0less builds
>> - fixed formatting for lines with more than 80 symbols
>> - updated commit message
>>
>> Changes in V2:
>> - no changes
>> ---
>> xen/arch/arm/dom0less-build.c | 2 +-
>> xen/arch/arm/domain_build.c | 2 +-
>> xen/arch/arm/include/asm/vgic.h | 21 +++++++++++++++++++++
>> 3 files changed, 23 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 fb4cea73eb..11f9d216eb 100644
>> --- a/xen/arch/arm/include/asm/vgic.h
>> +++ b/xen/arch/arm/include/asm/vgic.h
>> @@ -355,6 +355,27 @@ 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
>> + * returns the maximum eSPI INTID, supported by HW GIC,
>> subtracted by 32.
>
> NIT: s/returns/return ...
>
I will fix this in V5.
>
>> + * For non-Dom0 domains, the toolstack or arch_create_domUs function
>> + * applies the same adjustment to cover local IRQs (please, see
>> comment
>> + * for macro that is used for regular SPIs - VGIC_DEF_NR_SPIS).
>
> ... I am not 100% sure, that I follow the wording "For non-Dom0
> domains, the toolstack or arch_create_domUs ...". Did you perhaps mean:
>
> For Dom0 and started at boot time DomUs ...?
>
> We will
Yes, I agree that 'For Dom0 and started at boot time DomUs, we will add
back...' sounds better :)
>> + * 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;
>> +}
>> +
>> extern bool vgic_is_valid_line(struct domain *d, unsigned int virq);
>> static inline bool vgic_is_spi(struct domain *d, unsigned int virq)
>
Best regards,
Leonid
