On Fri, Sep 05, 2025 at 04:01:15PM -0700, Stefano Stabellini wrote: > On Fri, 5 Sep 2025, dmuk...@xen.org wrote: > > On Fri, Aug 29, 2025 at 03:21:13PM -0700, Stefano Stabellini wrote: > > > On Thu, 28 Aug 2025, dmuk...@xen.org wrote: > > > > From: Denis Mukhin <dmuk...@ford.com> > > > > > > > > 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. > > Hi Denis, the reason why IRQ_EMU is dangerous is that there are a bunch > of checks emuirq != IRQ_PT and also emuirq != IRQ_MSI_EMU which I don't > know if they would still behave correctly after the introduction of > IRQ_EMU.
!= IRQ_PT checks (arch/x86/irq.c) are done from map_domain_emuirq_pirq() and unmap_domain_pirq_emuirq() which I use for IRQ_EMU case. Those checks are needed to correctly allocate an "ad-hoc flag" in a radix "IRQ_EMU" tree, i.e. IRQ_EMU case is handled correctly. != IRQ_MSI_EMU check (there's only one in arch/x86/hvm/irq.c) is done from hvm_inject_msi(), it guarantees that IRQ_EMU case is handled correctly. > > I would prefer to avoid the problem if possible...
