From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53288) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c6hgp-0003HW-E9 for qemu-devel@nongnu.org; Tue, 15 Nov 2016 12:38:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c6hgm-0006SM-AT for qemu-devel@nongnu.org; Tue, 15 Nov 2016 12:38:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58898) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c6hgm-0006Rj-4I for qemu-devel@nongnu.org; Tue, 15 Nov 2016 12:38:32 -0500 Date: Tue, 15 Nov 2016 19:38:30 +0200 From: "Michael S. Tsirkin" Message-ID: <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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <61184b66-b6dc-b766-a629-537537a92776@redhat.com> 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: Paolo Bonzini Cc: qemu-devel@nongnu.org, alex.williamson@redhat.com, borntraeger@de.ibm.com, felipe@nutanix.com 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