From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55672) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c6hqG-0006lS-Tc for qemu-devel@nongnu.org; Tue, 15 Nov 2016 12:48:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c6hqD-0003MG-Pc for qemu-devel@nongnu.org; Tue, 15 Nov 2016 12:48:20 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57036) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c6hqD-0003LQ-Hs for qemu-devel@nongnu.org; Tue, 15 Nov 2016 12:48:17 -0500 Date: Tue, 15 Nov 2016 10:48:15 -0700 From: Alex Williamson Message-ID: <20161115104815.61c1f539@t450s.home> In-Reply-To: <20161115184631-mutt-send-email-mst@kernel.org> References: <20161115134629.23161-1-pbonzini@redhat.com> <20161115134629.23161-4-pbonzini@redhat.com> <20161115172336-mutt-send-email-mst@kernel.org> <9f83bd81-b077-4234-d2e2-1ffea4d35168@redhat.com> <20161115173913-mutt-send-email-mst@kernel.org> <61184b66-b6dc-b766-a629-537537a92776@redhat.com> <20161115184631-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Paolo Bonzini , qemu-devel@nongnu.org, borntraeger@de.ibm.com, felipe@nutanix.com On Tue, 15 Nov 2016 19:38:30 +0200 "Michael S. Tsirkin" wrote: > 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. The fact that Windows is trying to do some sort of hibernation is not visible to the user, I'm simply trying to shutdown the VM. That's rather important on my scale of functionality. If we have drivers in the wild doing this, does it matter what's in the spec? Smells like a regression from an end user perspective. Thanks, Alex > > > 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. >