Re: [PATCH v5 15/15] emul/ns16x50: implement IRQ emulation via vIOAPIC

[dmukhin@xxxxxxx]

On Fri, Aug 29, 2025 at 03:21:13PM -0700, Stefano Stabellini wrote:
> On Thu, 28 Aug 2025, dmukhin@xxxxxxx wrote:
> > From: Denis Mukhin <dmukhin@xxxxxxxx> 
> > 
> > PVH domains use vIOAPIC, not vPIC and NS16550 emulates ISA IRQs which cannot
> > be asserted on vIOAPIC.
> 
> One option is to enable the vPIT for PVH domains when the NS16550
> emulator is enabled. Would that resolve the problem? That would be a
> simpler solution compared to adding IRQ_EMU because the IRQ_* logic is
> already quite complex.

vPIC? (PIT is timer).

I tried to enable vPIC for hwdom, but there's some more work to make it work
:-/

> 
> Alternatively...
[..]
> 
> > --- a/xen/arch/x86/hvm/vioapic.c
> > +++ b/xen/arch/x86/hvm/vioapic.c
> > @@ -177,6 +177,16 @@ static int vioapic_hwdom_map_gsi(unsigned int gsi, 
> > unsigned int trig,
> >  
> >      ASSERT(is_hardware_domain(currd));
> >  
> > +    /*
> > +     * Interrupt is claimed by one of the platform virtual devices (e.g.
> > +     * NS16550); do nothing.
> > +     */
> > +    write_lock(&currd->event_lock);
> > +    ret = domain_irq_to_emuirq(currd, gsi);
> > +    write_unlock(&currd->event_lock);
> > +    if ( ret != IRQ_UNBOUND )
> > +        return 0;
> 
> ..alternatively, we could have an add-hoc check here? Not very nice but
> at least it would be very simple.
> 
> In other words, adding vPIT is preferable in my opinion but as a second
> option I would consider an ad-hoc check. I would try to avoid adding
> IRQ_EMU -- that should be the last resort in my view.

In my opinion, IRQ_EMU falls into category of an ad-hoc.

I think vioapic_hwdom_map_gsi() should not depend on the presence of certain
virtual devices but rely on some global flag, e.g. via quering
domain_irq_to_emuirq() infra.