[v1] Introduce eSPI support

[PATCH 04/10] xen/arm/irq: allow assignment/releasing of eSPI interrupts

Posted by Leonid Komarianskyi 3 months, 1 week ago

The current checks don't allow us to assign or release interrupts
with INTID greater than 1024. This patch adds an additional condition
to check whether the IRQ number is in the eSPI range and allows it to
be assigned to Xen and domains if it is.

Signed-off-by: Leonid Komarianskyi <leonid_komarianskyi@epam.com>
---
 xen/arch/arm/irq.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 3f68257fde..8c47eeb7c3 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -444,8 +444,8 @@ err:
 
 bool is_assignable_irq(unsigned int irq)
 {
-    /* For now, we can only route SPIs to the guest */
-    return (irq >= NR_LOCAL_IRQS) && (irq < gic_number_lines());
+    /* For now, we can only route SPIs and eSPIs to the guest */
+    return (((irq >= NR_LOCAL_IRQS) && (irq < gic_number_lines())) || is_espi(irq));
 }
 
 /*
@@ -589,8 +589,8 @@ int release_guest_irq(struct domain *d, unsigned int virq)
     unsigned long flags;
     int ret;
 
-    /* Only SPIs are supported */
-    if ( virq < NR_LOCAL_IRQS || virq >= vgic_num_irqs(d) )
+    /* Only SPIs and eSPIs are supported */
+    if ( (virq < NR_LOCAL_IRQS || virq >= vgic_num_irqs(d)) && !is_espi(virq) )
         return -EINVAL;
 
     desc = vgic_get_hw_irq_desc(d, NULL, virq);
-- 
2.34.1

Re: [PATCH 04/10] xen/arm/irq: allow assignment/releasing of eSPI interrupts

Posted by Julien Grall 3 months ago

Hi,

On 24/07/2025 15:57, Leonid Komarianskyi wrote:
> The current checks don't allow us to assign or release interrupts
> with INTID greater than 1024. This patch adds an additional condition
> to check whether the IRQ number is in the eSPI range and allows it to
> be assigned to Xen and domains if it is.
 > > Signed-off-by: Leonid Komarianskyi <leonid_komarianskyi@epam.com>
> ---
>   xen/arch/arm/irq.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 3f68257fde..8c47eeb7c3 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -444,8 +444,8 @@ err:
>   
>   bool is_assignable_irq(unsigned int irq)
>   {
> -    /* For now, we can only route SPIs to the guest */
> -    return (irq >= NR_LOCAL_IRQS) && (irq < gic_number_lines());
> +    /* For now, we can only route SPIs and eSPIs to the guest */
> +    return (((irq >= NR_LOCAL_IRQS) && (irq < gic_number_lines())) || is_espi(irq));
>   }

is_assignable_irq() is called by route_irq_to_guest() which first check 
'virq >= vgic_num_irqs()'. AFAICT, if we apply only up to this patch, 
'virq' would still require to < 1024. So ...


>   be
>   /*
> @@ -589,8 +589,8 @@ int release_guest_irq(struct domain *d, unsigned int virq)
>       unsigned long flags;
>       int ret;
>   
> -    /* Only SPIs are supported */
> -    if ( virq < NR_LOCAL_IRQS || virq >= vgic_num_irqs(d) )
> +    /* Only SPIs and eSPIs are supported */
> +    if ( (virq < NR_LOCAL_IRQS || virq >= vgic_num_irqs(d)) && !is_espi(virq) )

... I don't quite understand why this (yet?) need a change. Can you clarify?

>           return -EINVAL;
>   
>       desc = vgic_get_hw_irq_desc(d, NULL, virq);

Cheers,

-- 
Julien Grall

Re: [PATCH 04/10] xen/arm/irq: allow assignment/releasing of eSPI interrupts

Posted by Leonid Komarianskyi 3 months ago

Hi Julien,

On 29.07.25 16:56, Julien Grall wrote:
> Hi,
> 
> On 24/07/2025 15:57, Leonid Komarianskyi wrote:
>> The current checks don't allow us to assign or release interrupts
>> with INTID greater than 1024. This patch adds an additional condition
>> to check whether the IRQ number is in the eSPI range and allows it to
>> be assigned to Xen and domains if it is.
>  > > Signed-off-by: Leonid Komarianskyi <leonid_komarianskyi@epam.com>
>> ---
>>   xen/arch/arm/irq.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
>> index 3f68257fde..8c47eeb7c3 100644
>> --- a/xen/arch/arm/irq.c
>> +++ b/xen/arch/arm/irq.c
>> @@ -444,8 +444,8 @@ err:
>>   bool is_assignable_irq(unsigned int irq)
>>   {
>> -    /* For now, we can only route SPIs to the guest */
>> -    return (irq >= NR_LOCAL_IRQS) && (irq < gic_number_lines());
>> +    /* For now, we can only route SPIs and eSPIs to the guest */
>> +    return (((irq >= NR_LOCAL_IRQS) && (irq < gic_number_lines())) || 
>> is_espi(irq));
>>   }
> 
> is_assignable_irq() is called by route_irq_to_guest() which first check 
> 'virq >= vgic_num_irqs()'. AFAICT, if we apply only up to this patch, 
> 'virq' would still require to < 1024. So ...

That is my mistake when splitting the changes into separate patches. I 
will at least reorder the patches to make the changes more logical, or, 
based on your comments for the 6th patch in the series, I will try to 
refactor it with a helper function instead.

>>   be
>>   /*
>> @@ -589,8 +589,8 @@ int release_guest_irq(struct domain *d, unsigned 
>> int virq)
>>       unsigned long flags;
>>       int ret;
>> -    /* Only SPIs are supported */
>> -    if ( virq < NR_LOCAL_IRQS || virq >= vgic_num_irqs(d) )
>> +    /* Only SPIs and eSPIs are supported */
>> +    if ( (virq < NR_LOCAL_IRQS || virq >= vgic_num_irqs(d)) && ! 
>> is_espi(virq) )
> 
> ... I don't quite understand why this (yet?) need a change. Can you 
> clarify? ...

This is necessary to allow releasing eSPI IRQs when destroying the 
domain. And yes, this is the same mistake on my part - the patches are 
in the wrong order, and I will refactor them to fix that and make 
changes more logical and straightforward.

Thank you for your comments.

Cheers,
Leonid