Hi Volodymyr, Thank you for your review comment.
On 28.08.25 02:01, Volodymyr Babchuk wrote: > Hi Leonid, > > Leonid Komarianskyi<leonid_komarians...@epam.com> writes: > >> This change introduces resource management in the VGIC to handle >> extended SPIs introduced in GICv3.1. The pending_irqs and >> allocated_irqs arrays are resized to support the required >> number of eSPIs, based on what is supported by the hardware and >> requested by the guest. A new field, ext_shared_irqs, is added >> to the VGIC structure to store information about eSPIs, similar >> to how shared_irqs is used for regular SPIs. >> >> Since the eSPI range starts at INTID 4096 and INTIDs between 1025 >> and 4095 are reserved, helper macros are introduced to simplify the >> transformation of indices and to enable easier access to eSPI-specific >> resources. These changes prepare the VGIC for processing eSPIs as >> required by future functionality. >> >> The initialization and deinitialization paths for vgic have been updated >> to allocate and free these resources appropriately. Additionally, >> updated handling of INTIDs greater than 1024, passed from the toolstack >> during domain creation, and verification logic ensures only valid SPI or >> eSPI INTIDs are used. >> >> The existing SPI behavior remains unaffected when guests do not request >> eSPIs, GIC hardware does not support them, or the CONFIG_GICV3_ESPI >> option is disabled. >> >> Signed-off-by: Leonid Komarianskyi<leonid_komarians...@epam.com> >> >> --- >> Changes in V4: >> - added has_espi field to simplify determining whether a domain is able >> to operate with eSPI > I don't think that this is a good idea. You already have an invariant that > tells if domain has eSPIs: d->nr_espis != 0. If you introduce a new > field, you now have to keep these two values coherent or deal with possible > cases > like d->nr_espis == 0 && d->has_espi == true > > Also, this new field is not used anywhere, so why adding it in the first > place? I just wanted to simplify the checks in the next patch: https://patchew.org/Xen/cover.1756317702.git.leonid._5fkomarians...@epam.com/6b312e1997da5abdf592f66d16067f4330431ded.1756317702.git.leonid._5fkomarians...@epam.com/ e.g.: + if ( !v->domain->arch.vgic.has_espi ) + goto read_reserved; But yes, I agree that it looks redundant. Would it be okay if I drop this change in V5 and modify the checks in the next patch to something like this? + if ( v->domain->arch.vgic.nr_espis == 0 ) + goto read_reserved; Best regards, Leonid
