Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications

[Michael S. Tsirkin]

On Tue, Nov 15, 2016 at 05:22:49PM +0100, Paolo Bonzini wrote:
> 
> 
> On 15/11/2016 16:44, Michael S. Tsirkin wrote:
> > True. We could drop it from non-data plane, it's just that we never had
> > a reason to. vhost in kernel does not set ISR in MSI mode, either.
> 
> Yeah, I suspected that.  But dropping it from non-dataplane would break
> Windows hibernation and crashdump, just like it did for Alex.

I guess it's just a question of updating the drivers,
isn't it? To me, hibernation/crashdump doesn't sound important
enough to warrant work-arounds, but if you feel otherwise,
I'm fine with doing this work-around for dataplane.


> > What we have in spec is:
> > 
> > The device MUST set the Device Configuration Interrupt bit in ISR status
> > before sending a device configu-
> > ration change notification to the driver.
> > If MSI-X capability is disabled, the device MUST set the Queue Interrupt
> > bit in ISR status before sending a
> > virtqueue notification to the driver.
> > If MSI-X capability is disabled, the device MUST set the Interrupt
> > Status bit in the PCI Status register in the
> > PCI Configuration Header of the device to the logical OR of all bits in
> > ISR status of the device. The device
> > then asserts/deasserts INT#x interrupts unless masked according to
> > standard PCI rules [PCI].
> > The device MUST reset ISR status to 0 on driver read.
> > 
> > 
> > 
> > 
> > If MSI-X capability is enabled, the driver SHOULD NOT access ISR status
> > upon detecting a Queue Interrupt.
> > 
> > 
> > 
> > It can be clearer, but IMHO it's reasonably clear that devices
> > do not have to set this bit in MSI mode.
> 
> Yes, it is.  We can just document it in the release notes, but then the
> fix is not particularly intrusive.
> 
> Paolo

It has a slight performance cost but it's not too bad.

I'd rather document it in a code comment though.
Explain the motivation and which driver versions are affected.

-- 
MST