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
