Re: [PATCH] hw/i386/pc: Fix level interrupt sharing for Xen event channel GSI

[David Woodhouse]

On Tue, 2025-01-07 at 11:07 -0500, Michael S. Tsirkin wrote:
> On Thu, Dec 19, 2024 at 05:24:11PM +0100, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@amazon.co.uk>
> > 
> > The system GSIs are not designed for sharing. One device might assert a
> > shared interrupt with qemu_set_irq() and another might deassert it, and
> > the level from the first device is lost.
> > 
> > This could be solved by using a multiplexer which functions as an OR
> > gate, much like the PCI code already implements for pci_set_irq() for
> > muxing the INTx lines.
> > 
> > Alternatively, it could be solved by having a 'resample' callback which
> > is invoked when the interrupt is acked at the interrupt controller, and
> > causes the devices to re-trigger the interrupt if it should still be
> > pending. This is the model that VFIO in Linux uses, with a 'resampler'
> > eventfd that actually unmasks the interrupt on the hardware device and
> > thus triggers a new interrupt from it if needed. QEMU currently doesn't
> > use that VFIO interface correctly, and just bashes on the resampler for
> > every MMIO access to the device "just in case".
> > 
> > This does neither of those. The Xen event channel GSI support *already*
> > has hooks into the PC gsi_handler() code, for routing GSIs to PIRQs. So
> > we can implement the logical OR of the external input (from PCI INTx,
> > serial etc.) with the Xen event channel GSI by allowing that existing
> > hook to modify the 'level' being asserted.
> > 
> > Closes: https://gitlab.com/qemu-project/qemu/-/issues/2731
> > Reported-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> 
> Xen things so feel free to merge.
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>

Thanks.

Further testing shows I need one minor fix...

> > @@ -1596,7 +1607,7 @@ static int allocate_pirq(XenEvtchnState *s, int type, 
> > int gsi)
> >      return pirq;
> >  }
> >  
> > -bool xen_evtchn_set_gsi(int gsi, int level)
> > +bool xen_evtchn_set_gsi(int gsi, int *level)
> >  {
> >      XenEvtchnState *s = xen_evtchn_singleton;
> >      int pirq;

...

@@ -1628,7 +1656,7 @@ bool xen_evtchn_set_gsi(int gsi, int level)
         return false;
     }
 
-    if (level) {
+    if (*level) {
         int port = s->pirq[pirq].port;
 
         s->pirq_gsi_set |= (1U << gsi);

smime.p7s
Description: S/MIME cryptographic signature