Hello Oleksandr, On 23.08.25 15:29, Oleksandr Tyshchenko wrote: > [You don't often get email from olekst...@gmail.com. Learn why this is > important at https://aka.ms/LearnAboutSenderIdentification ] > > On 07.08.25 15:33, Leonid Komarianskyi wrote: > > Hello Leonid > > >> Introduced two new helper functions for vGIC: vgic_is_valid_irq and >> vgic_is_shared_irq. The functions are similar to the newly introduced >> gic_is_valid_irq and gic_is_shared_irq, but they verify whether a vIRQ >> is available for a specific domain, while GIC-specific functions >> validate INTIDs for the real GIC hardware. For example, the GIC may >> support all 992 SPI lines, but the domain may use only some part of them >> (e.g., 640), depending on the highest IRQ number defined in the domain >> configuration. Therefore, for vGIC-related code and checks, the >> appropriate functions should be used. Also, updated the appropriate >> checks to use these new helper functions. >> >> The purpose of introducing new helper functions for vGIC is essentially >> the same as for GIC: to avoid potential confusion with GIC-related >> checks and to consolidate similar code into separate functions, which >> can be more easily extended by additional conditions, e.g., when >> implementing extended SPI interrupts. >> >> Only the validation change in vgic_inject_irq may affect existing >> functionality, as it currently checks whether the vIRQ is less than or >> equal to vgic_num_irqs. Since IRQ indexes start from 0 (where 32 is the >> first SPI), the check should behave consistently with similar logic in >> other places and should check if the vIRQ number is less than >> vgic_num_irqs. The remaining changes, which replace open-coded checks >> with the use of these new helper functions, do not introduce any >> functional changes, as the helper functions follow the current vIRQ >> index verification logic. >> >> Signed-off-by: Leonid Komarianskyi <leonid_komarians...@epam.com> >> >> --- >> Changes in V2: >> - introduced this patch >> --- >> xen/arch/arm/gic.c | 3 +-- >> xen/arch/arm/include/asm/vgic.h | 7 +++++++ >> xen/arch/arm/irq.c | 4 ++-- >> xen/arch/arm/vgic.c | 10 ++++++++-- >> 4 files changed, 18 insertions(+), 6 deletions(-) >> >> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c >> index eb0346a898..47fccf21d8 100644 >> --- a/xen/arch/arm/gic.c >> +++ b/xen/arch/arm/gic.c >> @@ -133,8 +133,7 @@ int gic_route_irq_to_guest(struct domain *d, >> unsigned int virq, >> >> ASSERT(spin_is_locked(&desc->lock)); >> /* Caller has already checked that the IRQ is an SPI */ >> - ASSERT(virq >= 32); >> - ASSERT(virq < vgic_num_irqs(d)); >> + ASSERT(vgic_is_shared_irq(d, virq)); >> ASSERT(!is_lpi(virq)); >> >> ret = vgic_connect_hw_irq(d, NULL, virq, desc, true); >> diff --git a/xen/arch/arm/include/asm/vgic.h b/xen/arch/arm/include/ >> asm/vgic.h >> index 35c0c6a8b0..45201f4ca5 100644 >> --- a/xen/arch/arm/include/asm/vgic.h >> +++ b/xen/arch/arm/include/asm/vgic.h >> @@ -335,6 +335,13 @@ 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) >> >> +extern bool vgic_is_valid_irq(struct domain *d, unsigned int virq); >> + >> +static inline bool vgic_is_shared_irq(struct domain *d, unsigned int >> virq) >> +{ >> + return (virq >= NR_LOCAL_IRQS && vgic_is_valid_irq(d, virq)); >> +} >> + >> /* >> * Allocate a guest VIRQ >> * - spi == 0 => allocate a PPI. It will be the same on every vCPU >> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c >> index 12c70d02cc..50e57aaea7 100644 >> --- a/xen/arch/arm/irq.c >> +++ b/xen/arch/arm/irq.c >> @@ -442,7 +442,7 @@ int route_irq_to_guest(struct domain *d, unsigned >> int virq, >> unsigned long flags; >> int retval = 0; >> >> - if ( virq >= vgic_num_irqs(d) ) >> + if ( !vgic_is_valid_irq(d, virq) ) > > > This file is common for all VGIC implementations, so > route_irq_to_guest() is used with CONFIG_NEW_VGIC=y as well. > > If your series is built with CONFIG_NEW_VGIC=y (I know, that NEW_VGIC > does not support GICV3 HW) I have got the following error: > > > aarch64-poky-linux-ld: prelink.o: in function `route_irq_to_guest': > /usr/src/debug/xen/4.18.0+gitAUTOINC+ce58f56108-r0/git/xen/arch/arm/ > irq.c:445: > undefined reference to `vgic_is_valid_irq' > /usr/src/debug/xen/4.18.0+gitAUTOINC+ce58f56108-r0/git/xen/arch/arm/ > irq.c:445:(.text+0x5e2f8): > relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol > `vgic_is_valid_irq' > ... > > From the quick look, vgic_is_valid_irq() needs a stub (or NEW_VGIC's > counterpart). > > [snip]
Thank you for your review and for testing the patch series with different config options. I will add the vgic_is_valid_irq counterpart (after discussion, we decided to rename the function to vgic_is_valid_line) for NEW_VGIC and recheck the build with NEW_VGIC config. Best regards, Leonid.
