Re: map_domain_pirq(): pirq already mapped?

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, May 13, 2026 at 09:18:46PM -0400, Jason Andryuk wrote:
> Hi,
> 
> Early in map_domain_pirq(), we have this block:
> 
>     old_irq = domain_pirq_to_irq(d, pirq);
>     old_pirq = domain_irq_to_pirq(d, irq);
> 
>     if ( (old_irq > 0 && (old_irq != irq) ) ||
>          (old_pirq && (old_pirq != 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;
>     }
> 
> Why do we return 0 instead of -EEXIST?  Since the pirq is not updated, the
> caller doesn't know that pirq won't fire - only old_pirq.  For
> allocate_and_map_gsi_pirq(), the new pirq is still returned to the caller.
> I would expect old_pirq to be returned so the caller knows what to use.  Am
> I missing something?

Looking at bfc341a65cfb2 it seems like this might have been an attempt
to keep the previous logic in ioapic_guest_write() that didn't return
an error when attempting to add/move an in use IRQ, while switching
ioapic_guest_write() to use map_domain_pirq()?

The commit description is not very helpful sadly.  I think the mention
of "And this patch also makes broken NetBSD dom0 work again." is
relevant. AFAICT NetBSD will do PHYSDEVOP_apic_read -> modify RTE (ie:
set mask bit for example) -> PHYSDEVOP_apic_write.  However the
semantics of those hypercalls is not symmetric.  PHYSDEVOP_apic_read
will return the vector used by Xen in the RTE, while
PHYSDEVOP_apic_write expects the vector field of the RTE to contain
the pIRQ.  I think this is why map_domain_pirq() was adjusted in such
a weird way, to ignore requests with bogus pIRQs and still succeed, so
that PHYSDEVOP_apic_write would also succeed.  Ideally the interface
should have been adjusted so that read/modify/write cycles using
PHYSDEVOP_apic_{read,write} would work as expected (iow:
PHYSDEVOP_apic_read should have returned the pIRQ in the vector
field).

In the context of GSIs, I think we aim for Xen to always identity map
them (so IRQ == pIRQ), but there might be (or might have been)
hypercalls that could allow you to create non-identity mappings
between GSIs and pIRQs.

Explicitly looking at allocate_and_map_gsi_pirq() do you know what
causes the domain_irq_to_pirq() in allocate_pirq() to not return the
already allocated pIRQ that matches the passed IRQ?

Overall we should likely adjust map_domain_pirq() to return -EEIXST,
and then fix ioapic_guest_write() to shallow such error so we can keep
the current behavior for that specific interface.

Regards, Roger.