Re: [PATCH RFC] virtio-pci: share config interrupt between virtio devices

[Stefan Fritsch <sf@sfritsch.de>]

On Sunday 21 September 2014 13:21:06, Michael S. Tsirkin wrote:
> On Sun, Sep 21, 2014 at 11:36:44AM +0200, Stefan Fritsch wrote:
> > On Sunday 21 September 2014 11:09:14, Michael S. Tsirkin wrote:
> > > On Thu, Sep 18, 2014 at 09:18:37PM +0200, Stefan Fritsch wrote:
> > > > On Monday 01 September 2014 09:37:30, Michael S. Tsirkin 
wrote:
> > > > > Why do we need INT#x?
> > > > > How about setting IRQF_SHARED for the config interrupt
> > > > > while using MSI-X? You'd have to read ISR to check that the
> > > > > interrupt was intended for your device.
> > > >
> > > > 
> > > >
> > > > The virtio 0.9.5 spec says that ISR is "unused" when in MSI-X
> > > > mode. I  don't think that you can depend on the device to set
> > > > the
> > > > configuration changed bit.
> > > > The virtio 1.0 spec seems to have fixed that.
> > >
> > > 
> > >
> > > Yes, virtio 0.9.5 has this bug. But in practice qemu always set
> > > this bit, so for qemu we could do that
> > > unconditionally.  Pekka's lkvm tool doesn't
> > > unfortunately.  It's easy to fix that, but it would be nicer to
> > > additionally probe for old versions of the tool, and disable
> > > IRQF_SHARED in that case.
> >
> > 
> >
> > What about other implementations? I think Linux should try to
> > conform  to the spec so that all device implementations which
> > conform to the spec just work.
> >
> > 
> >
> > One implementation that comes to mind is virtualbox. But from a
> > quick  look at the source, it seems that it sets the ISR bit
> > always, too. And it uses qemu's subsystem vendor id.
> >
> > 
> >
> > But there are other implementations. For example bhyve.
> 
> I couldn't find any code in bhyve that sets VTCFG_ISR_CONF_CHANGED.
> Maybe it doesn't generate config changed interrupts?
> 
> bhyve sets subsystem vendor to 0 apparently?
> We could use that to detect it.

My point was that there are many virtio implementations by now and you 
can't assume you know all of them.

> But maybe we should just make it a 1.0 only feature.

FWIW, I think that would be the better option.