[PATCH] xen/events: Fix Global and Domain VIRQ tracking

[PATCH] xen/events: Fix Global and Domain VIRQ tracking

[Jason Andryuk]

VIRQs come in 3 flavors, per-VPU, per-domain, and global.  The existing
tracking of VIRQs is handled by per-cpu variables virq_to_irq.

The issue is that bind_virq_to_irq() sets the per_cpu virq_to_irq at
registration time - typically CPU 0.  Later, the interrupt can migrate,
and info->cpu is updated.  When calling unbind_from_irq(), the per-cpu
virq_to_irq is cleared for a different cpu.  If bind_virq_to_irq() is
called again with CPU 0, the stale irq is returned.

Change the virq_to_irq tracking to use CPU 0 for per-domain and global
VIRQs.  As there can be at most one of each, there is no need for
per-vcpu tracking.  Also, per-domain and global VIRQs need to be
registered on CPU 0 and can later move, so this matches the expectation.

Fixes: e46cdb66c8fc ("xen: event channels")
Cc: stable@vger.kernel.org
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
Fixes is the introduction of the virq_to_irq per-cpu array.

This was found with the out-of-tree argo driver during suspend/resume.
On suspend, the per-domain VIRQ_ARGO is unbound.  On resume, the driver
attempts to bind VIRQ_ARGO.  The stale irq is returned, but the
WARN_ON(info == NULL || info->type != IRQT_VIRQ) in bind_virq_to_irq()
triggers for NULL info.  The bind fails and execution continues with the
driver trying to clean up by unbinding.  This eventually faults over the
NULL info.
---
 drivers/xen/events/events_base.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 41309d38f78c..a27e4d7f061e 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -159,7 +159,19 @@ static DEFINE_MUTEX(irq_mapping_update_lock);
 
 static LIST_HEAD(xen_irq_list_head);
 
-/* IRQ <-> VIRQ mapping. */
+static bool is_per_vcpu_virq(int virq) {
+	switch (virq) {
+	case VIRQ_TIMER:
+	case VIRQ_DEBUG:
+	case VIRQ_XENOPROF:
+	case VIRQ_XENPMU:
+		return true;
+	default:
+		return false;
+	}
+}
+
+/* IRQ <-> VIRQ mapping.  Global/Domain virqs are tracked in cpu 0.  */
 static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0 ... NR_VIRQS-1] = -1};
 
 /* IRQ <-> IPI mapping */
@@ -974,6 +986,9 @@ static void __unbind_from_irq(struct irq_info *info, unsigned int irq)
 
 		switch (info->type) {
 		case IRQT_VIRQ:
+			if (!is_per_vcpu_virq(virq_from_irq(info)))
+				cpu = 0;
+
 			per_cpu(virq_to_irq, cpu)[virq_from_irq(info)] = -1;
 			break;
 		case IRQT_IPI:
-- 
2.50.1

Re: [PATCH] xen/events: Fix Global and Domain VIRQ tracking

[Andrew Cooper]

On 12/08/2025 8:00 pm, Jason Andryuk wrote:
> VIRQs come in 3 flavors, per-VPU, per-domain, and global.  The existing
> tracking of VIRQs is handled by per-cpu variables virq_to_irq.
>
> The issue is that bind_virq_to_irq() sets the per_cpu virq_to_irq at
> registration time - typically CPU 0.  Later, the interrupt can migrate,
> and info->cpu is updated.  When calling unbind_from_irq(), the per-cpu
> virq_to_irq is cleared for a different cpu.  If bind_virq_to_irq() is
> called again with CPU 0, the stale irq is returned.
>
> Change the virq_to_irq tracking to use CPU 0 for per-domain and global
> VIRQs.  As there can be at most one of each, there is no need for
> per-vcpu tracking.  Also, per-domain and global VIRQs need to be
> registered on CPU 0 and can later move, so this matches the expectation.
>
> Fixes: e46cdb66c8fc ("xen: event channels")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> Fixes is the introduction of the virq_to_irq per-cpu array.
>
> This was found with the out-of-tree argo driver during suspend/resume.
> On suspend, the per-domain VIRQ_ARGO is unbound.  On resume, the driver
> attempts to bind VIRQ_ARGO.  The stale irq is returned, but the
> WARN_ON(info == NULL || info->type != IRQT_VIRQ) in bind_virq_to_irq()
> triggers for NULL info.  The bind fails and execution continues with the
> driver trying to clean up by unbinding.  This eventually faults over the
> NULL info.

I don't think the Fixes: tag is entirely appropriate.

per-domain VIRQs were created (unexpectedly) by the merge of ARGO into
Xen.  It was during some unrelated cleanup that this was noticed and
bugfixed into working.  i.e. the ARGO VIRQ is the singular weird one here.

In Xen we did accept that per-domain VIRQs now exist; they had for
several releases before we realised.

~Andrew

> ---
>  drivers/xen/events/events_base.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index 41309d38f78c..a27e4d7f061e 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -159,7 +159,19 @@ static DEFINE_MUTEX(irq_mapping_update_lock);
>  
>  static LIST_HEAD(xen_irq_list_head);
>  
> -/* IRQ <-> VIRQ mapping. */
> +static bool is_per_vcpu_virq(int virq) {
> +	switch (virq) {
> +	case VIRQ_TIMER:
> +	case VIRQ_DEBUG:
> +	case VIRQ_XENOPROF:
> +	case VIRQ_XENPMU:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +/* IRQ <-> VIRQ mapping.  Global/Domain virqs are tracked in cpu 0.  */
>  static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0 ... NR_VIRQS-1] = -1};
>  
>  /* IRQ <-> IPI mapping */
> @@ -974,6 +986,9 @@ static void __unbind_from_irq(struct irq_info *info, unsigned int irq)
>  
>  		switch (info->type) {
>  		case IRQT_VIRQ:
> +			if (!is_per_vcpu_virq(virq_from_irq(info)))
> +				cpu = 0;
> +
>  			per_cpu(virq_to_irq, cpu)[virq_from_irq(info)] = -1;
>  			break;
>  		case IRQT_IPI:

Re: [PATCH] xen/events: Fix Global and Domain VIRQ tracking

[Jason Andryuk]

On 2025-08-12 15:10, Andrew Cooper wrote:
> On 12/08/2025 8:00 pm, Jason Andryuk wrote:
>> VIRQs come in 3 flavors, per-VPU, per-domain, and global.  The existing
>> tracking of VIRQs is handled by per-cpu variables virq_to_irq.
>>
>> The issue is that bind_virq_to_irq() sets the per_cpu virq_to_irq at
>> registration time - typically CPU 0.  Later, the interrupt can migrate,
>> and info->cpu is updated.  When calling unbind_from_irq(), the per-cpu
>> virq_to_irq is cleared for a different cpu.  If bind_virq_to_irq() is
>> called again with CPU 0, the stale irq is returned.
>>
>> Change the virq_to_irq tracking to use CPU 0 for per-domain and global
>> VIRQs.  As there can be at most one of each, there is no need for
>> per-vcpu tracking.  Also, per-domain and global VIRQs need to be
>> registered on CPU 0 and can later move, so this matches the expectation.
>>
>> Fixes: e46cdb66c8fc ("xen: event channels")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>> ---
>> Fixes is the introduction of the virq_to_irq per-cpu array.
>>
>> This was found with the out-of-tree argo driver during suspend/resume.
>> On suspend, the per-domain VIRQ_ARGO is unbound.  On resume, the driver
>> attempts to bind VIRQ_ARGO.  The stale irq is returned, but the
>> WARN_ON(info == NULL || info->type != IRQT_VIRQ) in bind_virq_to_irq()
>> triggers for NULL info.  The bind fails and execution continues with the
>> driver trying to clean up by unbinding.  This eventually faults over the
>> NULL info.
> 
> I don't think the Fixes: tag is entirely appropriate.
> 
> per-domain VIRQs were created (unexpectedly) by the merge of ARGO into
> Xen.  It was during some unrelated cleanup that this was noticed and
> bugfixed into working.  i.e. the ARGO VIRQ is the singular weird one here.
> 
> In Xen we did accept that per-domain VIRQs now exist; they had for
> several releases before we realised.

AFAICT, global VIRQs have the same issue - I think they just aren't 
unbound and rebound.  I just happened to trigger this with the 
per-domain ARGO VIRQ.

I double checked, and the cpu is updated like so:

set_affinity_irq()
   rebind_irq_to_cpu()
     bind_evtchn_to_cpu()
       cpu_evtchn[chn] = cpu;

unbind_from_irq()
     case IRQT_VIRQ:
         per_cpu(virq_to_irq, cpu_from_evtchn(evtchn))
                 [index_from_irq(irq)] = -1;

cpu_from_evtchn()
     return cpu_evtchn[evtchn];

So global VIRQs were mis-tracked even in the Fixes commit.

Regards,
Jason

>> ---
>>   drivers/xen/events/events_base.c | 17 ++++++++++++++++-
>>   1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
>> index 41309d38f78c..a27e4d7f061e 100644
>> --- a/drivers/xen/events/events_base.c
>> +++ b/drivers/xen/events/events_base.c
>> @@ -159,7 +159,19 @@ static DEFINE_MUTEX(irq_mapping_update_lock);
>>   
>>   static LIST_HEAD(xen_irq_list_head);
>>   
>> -/* IRQ <-> VIRQ mapping. */
>> +static bool is_per_vcpu_virq(int virq) {
>> +	switch (virq) {
>> +	case VIRQ_TIMER:
>> +	case VIRQ_DEBUG:
>> +	case VIRQ_XENOPROF:
>> +	case VIRQ_XENPMU:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +}
>> +
>> +/* IRQ <-> VIRQ mapping.  Global/Domain virqs are tracked in cpu 0.  */
>>   static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0 ... NR_VIRQS-1] = -1};
>>   
>>   /* IRQ <-> IPI mapping */
>> @@ -974,6 +986,9 @@ static void __unbind_from_irq(struct irq_info *info, unsigned int irq)
>>   
>>   		switch (info->type) {
>>   		case IRQT_VIRQ:
>> +			if (!is_per_vcpu_virq(virq_from_irq(info)))
>> +				cpu = 0;
>> +
>>   			per_cpu(virq_to_irq, cpu)[virq_from_irq(info)] = -1;
>>   			break;
>>   		case IRQT_IPI:
> 

Re: [PATCH] xen/events: Fix Global and Domain VIRQ tracking

[Jason Andryuk]

On 2025-08-12 15:00, Jason Andryuk wrote:
> VIRQs come in 3 flavors, per-VPU, per-domain, and global.  The existing
> tracking of VIRQs is handled by per-cpu variables virq_to_irq.
> 
> The issue is that bind_virq_to_irq() sets the per_cpu virq_to_irq at
> registration time - typically CPU 0.  Later, the interrupt can migrate,
> and info->cpu is updated.  When calling unbind_from_irq(), the per-cpu
> virq_to_irq is cleared for a different cpu.  If bind_virq_to_irq() is
> called again with CPU 0, the stale irq is returned.
> 
> Change the virq_to_irq tracking to use CPU 0 for per-domain and global
> VIRQs.  As there can be at most one of each, there is no need for
> per-vcpu tracking.  Also, per-domain and global VIRQs need to be
> registered on CPU 0 and can later move, so this matches the expectation.
> 
> Fixes: e46cdb66c8fc ("xen: event channels")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> Fixes is the introduction of the virq_to_irq per-cpu array.
> 
> This was found with the out-of-tree argo driver during suspend/resume.
> On suspend, the per-domain VIRQ_ARGO is unbound.  On resume, the driver
> attempts to bind VIRQ_ARGO.  The stale irq is returned, but the
> WARN_ON(info == NULL || info->type != IRQT_VIRQ) in bind_virq_to_irq()
> triggers for NULL info.  The bind fails and execution continues with the
> driver trying to clean up by unbinding.  This eventually faults over the
> NULL info.
> ---
>   drivers/xen/events/events_base.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index 41309d38f78c..a27e4d7f061e 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -159,7 +159,19 @@ static DEFINE_MUTEX(irq_mapping_update_lock);
>   
>   static LIST_HEAD(xen_irq_list_head);
>   
> -/* IRQ <-> VIRQ mapping. */
> +static bool is_per_vcpu_virq(int virq) {
> +	switch (virq) {
> +	case VIRQ_TIMER:
> +	case VIRQ_DEBUG:
> +	case VIRQ_XENOPROF:
> +	case VIRQ_XENPMU:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +/* IRQ <-> VIRQ mapping.  Global/Domain virqs are tracked in cpu 0.  */
>   static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0 ... NR_VIRQS-1] = -1};
>   
>   /* IRQ <-> IPI mapping */
> @@ -974,6 +986,9 @@ static void __unbind_from_irq(struct irq_info *info, unsigned int irq)
>   
>   		switch (info->type) {
>   		case IRQT_VIRQ:
> +			if (!is_per_vcpu_virq(virq_from_irq(info)))
> +				cpu = 0;
> +
>   			per_cpu(virq_to_irq, cpu)[virq_from_irq(info)] = -1;
>   			break;
>   		case IRQT_IPI:

Thinking about it a little more, bind_virq_to_irq() should ensure cpu == 
0 for per-domain and global VIRQs to ensure the property holds.  Also 
virq_to_irq accesses should go through wrappers to ensure all accesses 
are handled consistently.

I'll send a v2.

Regards,
Jason

Re: [PATCH] xen/events: Fix Global and Domain VIRQ tracking

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 4048 bytes --]

On 13.08.25 17:03, Jason Andryuk wrote:
> On 2025-08-12 15:00, Jason Andryuk wrote:
>> VIRQs come in 3 flavors, per-VPU, per-domain, and global.  The existing
>> tracking of VIRQs is handled by per-cpu variables virq_to_irq.
>>
>> The issue is that bind_virq_to_irq() sets the per_cpu virq_to_irq at
>> registration time - typically CPU 0.  Later, the interrupt can migrate,
>> and info->cpu is updated.  When calling unbind_from_irq(), the per-cpu
>> virq_to_irq is cleared for a different cpu.  If bind_virq_to_irq() is

This is what needs to be fixed. At migration the per_cpu virq_to_irq of the
source and the target cpu need to be updated to reflect that migration.

>> called again with CPU 0, the stale irq is returned.
>>
>> Change the virq_to_irq tracking to use CPU 0 for per-domain and global
>> VIRQs.  As there can be at most one of each, there is no need for
>> per-vcpu tracking.  Also, per-domain and global VIRQs need to be
>> registered on CPU 0 and can later move, so this matches the expectation.
>>
>> Fixes: e46cdb66c8fc ("xen: event channels")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>> ---
>> Fixes is the introduction of the virq_to_irq per-cpu array.
>>
>> This was found with the out-of-tree argo driver during suspend/resume.
>> On suspend, the per-domain VIRQ_ARGO is unbound.  On resume, the driver
>> attempts to bind VIRQ_ARGO.  The stale irq is returned, but the
>> WARN_ON(info == NULL || info->type != IRQT_VIRQ) in bind_virq_to_irq()
>> triggers for NULL info.  The bind fails and execution continues with the
>> driver trying to clean up by unbinding.  This eventually faults over the
>> NULL info.
>> ---
>>   drivers/xen/events/events_base.c | 17 ++++++++++++++++-
>>   1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
>> index 41309d38f78c..a27e4d7f061e 100644
>> --- a/drivers/xen/events/events_base.c
>> +++ b/drivers/xen/events/events_base.c
>> @@ -159,7 +159,19 @@ static DEFINE_MUTEX(irq_mapping_update_lock);
>>   static LIST_HEAD(xen_irq_list_head);
>> -/* IRQ <-> VIRQ mapping. */
>> +static bool is_per_vcpu_virq(int virq) {
>> +    switch (virq) {
>> +    case VIRQ_TIMER:
>> +    case VIRQ_DEBUG:
>> +    case VIRQ_XENOPROF:
>> +    case VIRQ_XENPMU:
>> +        return true;
>> +    default:
>> +        return false;
>> +    }
>> +}
>> +
>> +/* IRQ <-> VIRQ mapping.  Global/Domain virqs are tracked in cpu 0.  */
>>   static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0 ... NR_VIRQS-1] = -1};
>>   /* IRQ <-> IPI mapping */
>> @@ -974,6 +986,9 @@ static void __unbind_from_irq(struct irq_info *info, 
>> unsigned int irq)
>>           switch (info->type) {
>>           case IRQT_VIRQ:
>> +            if (!is_per_vcpu_virq(virq_from_irq(info)))
>> +                cpu = 0;
>> +
>>               per_cpu(virq_to_irq, cpu)[virq_from_irq(info)] = -1;
>>               break;
>>           case IRQT_IPI:
> 
> Thinking about it a little more, bind_virq_to_irq() should ensure cpu == 0 for 
> per-domain and global VIRQs to ensure the property holds.  Also virq_to_irq 

In Xen's evtchn_bind_virq() there is:

     if ( type != VIRQ_VCPU && vcpu != 0 )
         return -EINVAL;

Making sure in Linux that there is never a violation of that restriction would
require to always have an up-to-date table of all possible VIRQs and their
type, which I'd like to avoid.

I think it is the user of the VIRQ who is responsible to ensure cpu 0 is passed
to bind_virq_to_irq(), as this user knows that such a restriction applies to
the VIRQ in question (at least he should know that).

Special handling for really used VIRQs in the kernel can have some special
handling, of course, as they are known already and should be used correctly.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] xen/events: Fix Global and Domain VIRQ tracking

[Jason Andryuk]

On 2025-08-14 03:05, Jürgen Groß wrote:
> On 13.08.25 17:03, Jason Andryuk wrote:
>> On 2025-08-12 15:00, Jason Andryuk wrote:
>>> VIRQs come in 3 flavors, per-VPU, per-domain, and global.  The existing
>>> tracking of VIRQs is handled by per-cpu variables virq_to_irq.
>>>
>>> The issue is that bind_virq_to_irq() sets the per_cpu virq_to_irq at
>>> registration time - typically CPU 0.  Later, the interrupt can migrate,
>>> and info->cpu is updated.  When calling unbind_from_irq(), the per-cpu
>>> virq_to_irq is cleared for a different cpu.  If bind_virq_to_irq() is
> 
> This is what needs to be fixed. At migration the per_cpu virq_to_irq of the
> source and the target cpu need to be updated to reflect that migration.

I considered this, and even implemented it, before changing my approach. 
  My concern was that the single VIRQ is now in one of the N per_cpu 
virq_to_irq arrays.  A second attempt to register on CPU 0 will probably 
find -1 and continue and issue the hypercall.

It looks like Xen tracks virq on the bind_virq vcpu, so 
per-domain/global stays on vcpu0.  Binding again would return -EEXISTS. 
find_virq() would not match the virq if it was re-bound to a different vcpu.

If we don't care about handling duplicate registration, then updating 
the virq_to_irq tables should be is fine.

>>> called again with CPU 0, the stale irq is returned.
>>>
>>> Change the virq_to_irq tracking to use CPU 0 for per-domain and global
>>> VIRQs.  As there can be at most one of each, there is no need for
>>> per-vcpu tracking.  Also, per-domain and global VIRQs need to be
>>> registered on CPU 0 and can later move, so this matches the expectation.
>>>
>>> Fixes: e46cdb66c8fc ("xen: event channels")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>>> ---
>>> Fixes is the introduction of the virq_to_irq per-cpu array.
>>>
>>> This was found with the out-of-tree argo driver during suspend/resume.
>>> On suspend, the per-domain VIRQ_ARGO is unbound.  On resume, the driver
>>> attempts to bind VIRQ_ARGO.  The stale irq is returned, but the
>>> WARN_ON(info == NULL || info->type != IRQT_VIRQ) in bind_virq_to_irq()
>>> triggers for NULL info.  The bind fails and execution continues with the
>>> driver trying to clean up by unbinding.  This eventually faults over the
>>> NULL info.
>>> ---
>>>   drivers/xen/events/events_base.c | 17 ++++++++++++++++-
>>>   1 file changed, 16 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/ 
>>> events_base.c
>>> index 41309d38f78c..a27e4d7f061e 100644
>>> --- a/drivers/xen/events/events_base.c
>>> +++ b/drivers/xen/events/events_base.c
>>> @@ -159,7 +159,19 @@ static DEFINE_MUTEX(irq_mapping_update_lock);
>>>   static LIST_HEAD(xen_irq_list_head);
>>> -/* IRQ <-> VIRQ mapping. */
>>> +static bool is_per_vcpu_virq(int virq) {
>>> +    switch (virq) {
>>> +    case VIRQ_TIMER:
>>> +    case VIRQ_DEBUG:
>>> +    case VIRQ_XENOPROF:
>>> +    case VIRQ_XENPMU:
>>> +        return true;
>>> +    default:
>>> +        return false;
>>> +    }
>>> +}
>>> +
>>> +/* IRQ <-> VIRQ mapping.  Global/Domain virqs are tracked in cpu 0.  */
>>>   static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0 ... 
>>> NR_VIRQS-1] = -1};
>>>   /* IRQ <-> IPI mapping */
>>> @@ -974,6 +986,9 @@ static void __unbind_from_irq(struct irq_info 
>>> *info, unsigned int irq)
>>>           switch (info->type) {
>>>           case IRQT_VIRQ:
>>> +            if (!is_per_vcpu_virq(virq_from_irq(info)))
>>> +                cpu = 0;
>>> +
>>>               per_cpu(virq_to_irq, cpu)[virq_from_irq(info)] = -1;
>>>               break;
>>>           case IRQT_IPI:
>>
>> Thinking about it a little more, bind_virq_to_irq() should ensure cpu 
>> == 0 for per-domain and global VIRQs to ensure the property holds.  
>> Also virq_to_irq 
> 
> In Xen's evtchn_bind_virq() there is:
> 
>      if ( type != VIRQ_VCPU && vcpu != 0 )
>          return -EINVAL;
> 
> Making sure in Linux that there is never a violation of that restriction 
> would
> require to always have an up-to-date table of all possible VIRQs and their
> type, which I'd like to avoid.

Yes, I agree with this.

> I think it is the user of the VIRQ who is responsible to ensure cpu 0 is 
> passed
> to bind_virq_to_irq(), as this user knows that such a restriction 
> applies to
> the VIRQ in question (at least he should know that).
> 
> Special handling for really used VIRQs in the kernel can have some special
> handling, of course, as they are known already and should be used 
> correctly.

Thanks,
Jason

Re: [PATCH] xen/events: Fix Global and Domain VIRQ tracking

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 1694 bytes --]

On 14.08.25 23:04, Jason Andryuk wrote:
> On 2025-08-14 03:05, Jürgen Groß wrote:
>> On 13.08.25 17:03, Jason Andryuk wrote:
>>> On 2025-08-12 15:00, Jason Andryuk wrote:
>>>> VIRQs come in 3 flavors, per-VPU, per-domain, and global.  The existing
>>>> tracking of VIRQs is handled by per-cpu variables virq_to_irq.
>>>>
>>>> The issue is that bind_virq_to_irq() sets the per_cpu virq_to_irq at
>>>> registration time - typically CPU 0.  Later, the interrupt can migrate,
>>>> and info->cpu is updated.  When calling unbind_from_irq(), the per-cpu
>>>> virq_to_irq is cleared for a different cpu.  If bind_virq_to_irq() is
>>
>> This is what needs to be fixed. At migration the per_cpu virq_to_irq of the
>> source and the target cpu need to be updated to reflect that migration.
> 
> I considered this, and even implemented it, before changing my approach.  My 
> concern was that the single VIRQ is now in one of the N per_cpu virq_to_irq 
> arrays.  A second attempt to register on CPU 0 will probably find -1 and 
> continue and issue the hypercall.

The hypervisor would reject the attempt, right? So in the end no problem.

> It looks like Xen tracks virq on the bind_virq vcpu, so per-domain/global stays 
> on vcpu0.  Binding again would return -EEXISTS. find_virq() would not match the 
> virq if it was re-bound to a different vcpu.

We probably would want to modify find_virq() and bind_virq_to_irq() to not
result in a BUG() if a non-percpu virq is bound to another cpu. This could
be done by passing the percpu flag to find_virq() and let find_virq() return
e.g. -EEXIST if a non-percpu virq is found to be bound to another cpu.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]