commit bfc341a65cfb ("x86: Make the hypercall PHYSDEVOP_alloc_irq_vector
hypercall dummy.") modified map_domain_pirq() to return 0 when an irq or
pirq is already mapped, when it previously returned -EINVAL.  This
occured when moving map_domain_pirq()'s call from
PHYSDEVOP_alloc_irq_vector into PHYSDEVOP_apic_write.

However, this means other callers cannot detect when a pirq or irq is
already mapped.  Since success is returned but the pirq is not
connected, it will never fire.

Modify map_domain_pirq() to return -EEXIST.  -EINVAL is already returned
elsewhere in map_domain_pirq(), so -EEXIST allows identifying this case.
With that, squash -EEXIST in ioapic_guest_write() so the behavior does not
change.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
I'm not sure whether or not this warrants a Fixes.  Nothing in tree
today is broken, so I did not add one.

 xen/arch/x86/io_apic.c | 2 ++
 xen/arch/x86/irq.c     | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 46c2a43dac..2b928fc236 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2315,6 +2315,8 @@ int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val)
         ret = map_domain_pirq(hardware_domain, pirq, irq,
                               MAP_PIRQ_TYPE_GSI, NULL);
         write_unlock(&hardware_domain->event_lock);
+        if ( ret == -EEXIST )
+            ret = 0;
         if ( ret < 0 )
             return ret;
     }
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 739fc04bd1..938cca6203 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -2210,7 +2210,7 @@ int map_domain_pirq(
         dprintk(XENLOG_G_WARNING,
                 "dom%d: pirq %d or irq %d already mapped (%d,%d)\n",
                 d->domain_id, pirq, irq, old_pirq, old_irq);
-        return 0;
+        return -EEXIST;
     }
 
     ret = xsm_map_domain_irq(XSM_HOOK, d, irq, data);
-- 
2.54.0

On 19.05.2026 02:15, Jason Andryuk wrote:
> commit bfc341a65cfb ("x86: Make the hypercall PHYSDEVOP_alloc_irq_vector
> hypercall dummy.") modified map_domain_pirq() to return 0 when an irq or
> pirq is already mapped, when it previously returned -EINVAL.  This
> occured when moving map_domain_pirq()'s call from
> PHYSDEVOP_alloc_irq_vector into PHYSDEVOP_apic_write.
> 
> However, this means other callers cannot detect when a pirq or irq is
> already mapped.  Since success is returned but the pirq is not
> connected, it will never fire.
> 
> Modify map_domain_pirq() to return -EEXIST.  -EINVAL is already returned
> elsewhere in map_domain_pirq(), so -EEXIST allows identifying this case.
> With that, squash -EEXIST in ioapic_guest_write() so the behavior does not
> change.

What about the function's uses from allocate_and_map_[gm]si_pirq()? Don't they
then also need to cope with getting back -EEXIST to keep externally visible
behavior unaltered? Else those cases want discussing in the description, I
suppose.

> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> I'm not sure whether or not this warrants a Fixes.  Nothing in tree
> today is broken, so I did not add one.

In which case this then also isn't 4.22 material, I guess?

> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -2315,6 +2315,8 @@ int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val)
>          ret = map_domain_pirq(hardware_domain, pirq, irq,
>                                MAP_PIRQ_TYPE_GSI, NULL);
>          write_unlock(&hardware_domain->event_lock);
> +        if ( ret == -EEXIST )
> +            ret = 0;
>          if ( ret < 0 )
>              return ret;

Slightly shorter

        if ( ret < 0 && ret != -EEXIST )
            return ret;

?

Jan

On 2026-05-19 03:21, Jan Beulich wrote:
> On 19.05.2026 02:15, Jason Andryuk wrote:
>> commit bfc341a65cfb ("x86: Make the hypercall PHYSDEVOP_alloc_irq_vector
>> hypercall dummy.") modified map_domain_pirq() to return 0 when an irq or
>> pirq is already mapped, when it previously returned -EINVAL.  This
>> occured when moving map_domain_pirq()'s call from
>> PHYSDEVOP_alloc_irq_vector into PHYSDEVOP_apic_write.
>>
>> However, this means other callers cannot detect when a pirq or irq is
>> already mapped.  Since success is returned but the pirq is not
>> connected, it will never fire.
>>
>> Modify map_domain_pirq() to return -EEXIST.  -EINVAL is already returned
>> elsewhere in map_domain_pirq(), so -EEXIST allows identifying this case.
>> With that, squash -EEXIST in ioapic_guest_write() so the behavior does not
>> change.
> 
> What about the function's uses from allocate_and_map_[gm]si_pirq()? Don't they
> then also need to cope with getting back -EEXIST to keep externally visible
> behavior unaltered? Else those cases want discussing in the description, I
> suppose.

For my use, I want allocate_and_map_gsi_pirq() to return -EEXIST.  I'll 
have to look at everything again and update the description.

>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>> ---
>> I'm not sure whether or not this warrants a Fixes.  Nothing in tree
>> today is broken, so I did not add one.
> 
> In which case this then also isn't 4.22 material, I guess?

Probably not.

>> --- a/xen/arch/x86/io_apic.c
>> +++ b/xen/arch/x86/io_apic.c
>> @@ -2315,6 +2315,8 @@ int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val)
>>           ret = map_domain_pirq(hardware_domain, pirq, irq,
>>                                 MAP_PIRQ_TYPE_GSI, NULL);
>>           write_unlock(&hardware_domain->event_lock);
>> +        if ( ret == -EEXIST )
>> +            ret = 0;
>>           if ( ret < 0 )
>>               return ret;
> 
> Slightly shorter
> 
>          if ( ret < 0 && ret != -EEXIST )
>              return ret;
> 
> ?

Well, I intentionally set ret = 0, so -EEXIST wasn't seen later in the 
function.  Though double checking now, there aren't any further uses of 
ret, so your suggestion is okay.  I just felt better overriding ret 
instead of leaving it.

Thanks,
Jason