From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37554) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aSlnm-00047R-6d for qemu-devel@nongnu.org; Mon, 08 Feb 2016 08:24:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aSlnh-0007jg-FC for qemu-devel@nongnu.org; Mon, 08 Feb 2016 08:24:26 -0500 Received: from mail-wm0-x229.google.com ([2a00:1450:400c:c09::229]:34207) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aSlnh-0007jW-3r for qemu-devel@nongnu.org; Mon, 08 Feb 2016 08:24:21 -0500 Received: by mail-wm0-x229.google.com with SMTP id 128so154848062wmz.1 for ; Mon, 08 Feb 2016 05:24:20 -0800 (PST) Message-ID: <56B896F4.60608@6wind.com> Date: Mon, 08 Feb 2016 14:24:04 +0100 From: Didier Pallard MIME-Version: 1.0 References: <1449136399-4158-1-git-send-email-didier.pallard@6wind.com> <1449136399-4158-3-git-send-email-didier.pallard@6wind.com> <20160204150650-mutt-send-email-mst@redhat.com> In-Reply-To: <20160204150650-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/3] virtio-pci: add an option to bypass guest_notifier_mask List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: thibaut.collet@6wind.com, jmg@6wind.com, qemu-devel@nongnu.org On 02/04/2016 02:08 PM, Michael S. Tsirkin wrote: > On Thu, Dec 03, 2015 at 10:53:18AM +0100, Didier Pallard wrote: >> Using guest_notifier_mask function in vhost-user case may >> break interrupt mask paradigm, because mask/unmask is not >> really done when returning from guest_notifier_mask call, instead >> message is posted in a unix socket, and processed later. >> >> Add an option bit to disable the use of guest_notifier_mask >> in virtio pci. >> >> Signed-off-by: Didier Pallard >> Reviewed-by: Thibaut Collet >> --- >> hw/virtio/virtio-pci.c | 29 +++++++++++++++++++++++------ >> hw/virtio/virtio-pci.h | 6 ++++++ >> 2 files changed, 29 insertions(+), 6 deletions(-) >> >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index dd48562..26bb617 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -806,7 +806,8 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs) >> /* If guest supports masking, set up irqfd now. >> * Otherwise, delay until unmasked in the frontend. >> */ >> - if (k->guest_notifier_mask) { >> + if ((proxy->flags & VIRTIO_PCI_FLAG_USE_NOTIFIERMASK) && >> + k->guest_notifier_mask) { >> ret = kvm_virtio_pci_irqfd_use(proxy, queue_no, vector); >> if (ret < 0) { >> kvm_virtio_pci_vq_vector_release(proxy, vector); >> @@ -822,7 +823,8 @@ undo: >> if (vector >= msix_nr_vectors_allocated(dev)) { >> continue; >> } >> - if (k->guest_notifier_mask) { >> + if ((proxy->flags & VIRTIO_PCI_FLAG_USE_NOTIFIERMASK) && >> + k->guest_notifier_mask) { >> kvm_virtio_pci_irqfd_release(proxy, queue_no, vector); >> } >> kvm_virtio_pci_vq_vector_release(proxy, vector); >> @@ -849,7 +851,8 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs) >> /* If guest supports masking, clean up irqfd now. >> * Otherwise, it was cleaned when masked in the frontend. >> */ >> - if (k->guest_notifier_mask) { >> + if ((proxy->flags & VIRTIO_PCI_FLAG_USE_NOTIFIERMASK) && >> + k->guest_notifier_mask) { >> kvm_virtio_pci_irqfd_release(proxy, queue_no, vector); >> } >> kvm_virtio_pci_vq_vector_release(proxy, vector); >> @@ -882,7 +885,8 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy, >> /* If guest supports masking, irqfd is already setup, unmask it. >> * Otherwise, set it up now. >> */ >> - if (k->guest_notifier_mask) { >> + if ((proxy->flags & VIRTIO_PCI_FLAG_USE_NOTIFIERMASK) && >> + k->guest_notifier_mask) { >> k->guest_notifier_mask(vdev, queue_no, false); >> /* Test after unmasking to avoid losing events. */ >> if (k->guest_notifier_pending && >> @@ -905,7 +909,8 @@ static void virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy, >> /* If guest supports masking, keep irqfd but mask it. >> * Otherwise, clean it up now. >> */ >> - if (k->guest_notifier_mask) { >> + if ((proxy->flags & VIRTIO_PCI_FLAG_USE_NOTIFIERMASK) && >> + k->guest_notifier_mask) { >> k->guest_notifier_mask(vdev, queue_no, true); >> } else { >> kvm_virtio_pci_irqfd_release(proxy, queue_no, vector); >> @@ -1022,7 +1027,9 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign, >> event_notifier_cleanup(notifier); >> } >> >> - if (!msix_enabled(&proxy->pci_dev) && vdc->guest_notifier_mask) { >> + if (!msix_enabled(&proxy->pci_dev) && >> + (proxy->flags & VIRTIO_PCI_FLAG_USE_NOTIFIERMASK) && >> + vdc->guest_notifier_mask) { >> vdc->guest_notifier_mask(vdev, n, !assign); >> } >> >> @@ -1164,6 +1171,8 @@ static void virtio_9p_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) >> static Property virtio_9p_pci_properties[] = { >> DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, >> VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), >> + DEFINE_PROP_BIT("usenotifiermask", VirtIOPCIProxy, flags, >> + VIRTIO_PCI_FLAG_USE_NOTIFIERMASK_BIT, true), >> DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), >> DEFINE_PROP_END_OF_LIST(), >> }; >> @@ -1908,6 +1917,8 @@ static Property virtio_blk_pci_properties[] = { >> DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0), >> DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, >> VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), >> + DEFINE_PROP_BIT("usenotifiermask", VirtIOPCIProxy, flags, >> + VIRTIO_PCI_FLAG_USE_NOTIFIERMASK_BIT, true), >> DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), >> DEFINE_PROP_END_OF_LIST(), >> }; >> @@ -1961,6 +1972,8 @@ static const TypeInfo virtio_blk_pci_info = { >> static Property virtio_scsi_pci_properties[] = { >> DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, >> VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), >> + DEFINE_PROP_BIT("usenotifiermask", VirtIOPCIProxy, flags, >> + VIRTIO_PCI_FLAG_USE_NOTIFIERMASK_BIT, true), >> DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, >> DEV_NVECTORS_UNSPECIFIED), >> DEFINE_PROP_END_OF_LIST(), >> @@ -2175,6 +2188,8 @@ static void virtio_serial_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) >> static Property virtio_serial_pci_properties[] = { >> DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, >> VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), >> + DEFINE_PROP_BIT("usenotifiermask", VirtIOPCIProxy, flags, >> + VIRTIO_PCI_FLAG_USE_NOTIFIERMASK_BIT, true), >> DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), >> DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0), >> DEFINE_PROP_END_OF_LIST(), >> @@ -2215,6 +2230,8 @@ static const TypeInfo virtio_serial_pci_info = { >> static Property virtio_net_properties[] = { >> DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, >> VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false), >> + DEFINE_PROP_BIT("usenotifiermask", VirtIOPCIProxy, flags, >> + VIRTIO_PCI_FLAG_USE_NOTIFIERMASK_BIT, true), >> DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3), >> DEFINE_PROP_END_OF_LIST(), >> }; > I think we should rename this x-use-notifier mask. > And I'd rather drop it from all devices where it's not relevant. I will rename the option x-use-notifier-mask. Currently, only vhost-net has a vhost-user implementation, isn't it? so it is the only place were i need to set the option, that's it? > >> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h >> index ffb74bb..aecd4eb 100644 >> --- a/hw/virtio/virtio-pci.h >> +++ b/hw/virtio/virtio-pci.h >> @@ -86,6 +86,12 @@ typedef struct VirtioBusClass VirtioPCIBusClass; >> #define VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY \ >> (1 << VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT) >> >> +/* Where vhost-user implementation exists, using the guest notifier mask >> + * feature can lead to improper interrupt management. Add a flag to >> + * allow to disable this guest notifier mask if desired */ >> +#define VIRTIO_PCI_FLAG_USE_NOTIFIERMASK_BIT 6 >> +#define VIRTIO_PCI_FLAG_USE_NOTIFIERMASK (1 << VIRTIO_PCI_FLAG_USE_NOTIFIERMASK_BIT) >> + >> typedef struct { >> MSIMessage msg; >> int virq; >> -- >> 2.1.4 >>