[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 03/10] xen/arm: vgic: implement helper functions for virq checks


  • To: Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Leonid Komarianskyi <Leonid_Komarianskyi@xxxxxxxx>
  • Date: Sun, 24 Aug 2025 18:08:49 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=4imJ6dvURYjQYR/dZwnaWRCobQTX2Deyj3iYav320gM=; b=eFHkLwtt+v6toZZiHBoQM6Go2U1BgGPbNW694gMuRiQ0rX1zinZ5EQl9NKrHNqGclrWTsGu5StTrPVVGZ5RIicu7DfZQxcsj4mjZMSLm2EYfyiAWJqOIqoBgx9oMn22u4XiaxIlLbGRzJzgSTnPwxh8FUavR+Sw8FKHqO1+hDKrGVnHbo15TCT/y9YcXTHDha7JbrYHmtVf3ZXtYLfM+CKQwM/ni+jvTcHZ7RhIbUZmHT0K7Hc8SSAnWzd+encpR30D7gxUVHlQNBCAYWqEBiLEZbBcPhctzUK8ufmw0zQOlwr4HNZWWHS1hccouFDus+vfsEMxFXSyLeC/Oe3zS2g==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=F+vWnWWjjxsBhXXVf1odbtL27Aaqp9/h5nSoFrkEn/3d7rh0HpgIMEmjKqEJV3h6f3KFCgKrgE8lM8+hWR66YozR1Ab8T9smqKsyO4fM3qCw7JkDuOHpia821Bi0Z14O40auJgtWHN1D04KUCmYtSU3QEfYfML1XvD9BLYEl5b9/1X8iuNZp0l3cHBKVKgsBZddcg6slCqhk8DnKg4XcRrNc5ieaQOs32wbDdKKvTV+RXHIcx7xlWcbzyHXsxwPdCGCi3E/G585QAHQpg/bOI6QWZlSdIPytuTTvEpazsLrEDeh8OfJ3SWxLPEFfuo++jc0f9cipS/OL8sW/S8JPUA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=epam.com;
  • Delivery-date: Sun, 24 Aug 2025 18:09:07 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHcB5d76MRh/Swq2EGu624C+G/Fb7RwRBaAgAHxAoA=
  • Thread-topic: [PATCH v2 03/10] xen/arm: vgic: implement helper functions for virq checks

Hello Oleksandr,

On 23.08.25 15:29, Oleksandr Tyshchenko wrote:
> [You don't often get email from olekstysh@xxxxxxxxx. 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_komarianskyi@xxxxxxxx>
>>
>> ---
>> 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.

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.