From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35723) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UFvvl-0001AQ-Kw for qemu-devel@nongnu.org; Wed, 13 Mar 2013 20:22:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UFvvk-00028j-3z for qemu-devel@nongnu.org; Wed, 13 Mar 2013 20:22:01 -0400 Received: from mail-ia0-x235.google.com ([2607:f8b0:4001:c02::235]:46528) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UFvvj-00028S-Th for qemu-devel@nongnu.org; Wed, 13 Mar 2013 20:22:00 -0400 Received: by mail-ia0-f181.google.com with SMTP id u8so142666iag.12 for ; Wed, 13 Mar 2013 17:21:58 -0700 (PDT) Message-ID: <51411867.30204@ozlabs.ru> Date: Thu, 14 Mar 2013 11:23:03 +1100 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <20130313194643.GA9717@redhat.com> In-Reply-To: <20130313194643.GA9717@redhat.com> Content-Type: text/plain; charset=KOI8-R; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] virtio-pci: guest notifier mask without non-irqfd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org Michael, yes, that works fine on ppc64 with vhost=on. Thanks! On 14/03/13 06:46, Michael S. Tsirkin wrote: > non-irqfd setups are currently broken with vhost: > we start up masked and nothing unmasks the interrupts. > Fix by using mask notifiers, same as the irqfd path. > > Sharing irqchip/non irqchip code is always a good thing, > in this case it will help non irqchip benefit > from backend masking optimization. > > Reported-by: Alexey Kardashevskiy > Signed-off-by: Michael S. Tsirkin > --- > > Alexey, the following is a clean way to fix the > problem that you reported "Re: QEMU -netdev vhost=on + -device > virtio-net-pci bug" (previous patch was a quick hack but > not I think a good fix). > Lightly tested on x86/kvm and still under test, could you please try it > out and report whether it works for you? > > hw/virtio-pci.c | 79 ++++++++++++++++++++++++++++++++------------------------- > 1 file changed, 44 insertions(+), 35 deletions(-) > > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index ba56ab2..4f8a9cf 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -609,20 +609,23 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs) > } > } > > -static int kvm_virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy, > - unsigned int queue_no, > - unsigned int vector, > - MSIMessage msg) > +static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy, > + unsigned int queue_no, > + unsigned int vector, > + MSIMessage msg) > { > VirtQueue *vq = virtio_get_queue(proxy->vdev, queue_no); > EventNotifier *n = virtio_queue_get_guest_notifier(vq); > - VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector]; > + VirtIOIRQFD *irqfd; > int ret = 0; > > - if (irqfd->msg.data != msg.data || irqfd->msg.address != msg.address) { > - ret = kvm_irqchip_update_msi_route(kvm_state, irqfd->virq, msg); > - if (ret < 0) { > - return ret; > + if (proxy->vector_irqfd) { > + irqfd = &proxy->vector_irqfd[vector]; > + if (irqfd->msg.data != msg.data || irqfd->msg.address != msg.address) { > + ret = kvm_irqchip_update_msi_route(kvm_state, irqfd->virq, msg); > + if (ret < 0) { > + return ret; > + } > } > } > > @@ -642,7 +645,7 @@ static int kvm_virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy, > return ret; > } > > -static void kvm_virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy, > +static void virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy, > unsigned int queue_no, > unsigned int vector) > { > @@ -656,8 +659,8 @@ static void kvm_virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy, > } > } > > -static int kvm_virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector, > - MSIMessage msg) > +static int virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector, > + MSIMessage msg) > { > VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev); > VirtIODevice *vdev = proxy->vdev; > @@ -670,7 +673,7 @@ static int kvm_virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector, > if (virtio_queue_vector(vdev, queue_no) != vector) { > continue; > } > - ret = kvm_virtio_pci_vq_vector_unmask(proxy, queue_no, vector, msg); > + ret = virtio_pci_vq_vector_unmask(proxy, queue_no, vector, msg); > if (ret < 0) { > goto undo; > } > @@ -682,12 +685,12 @@ undo: > if (virtio_queue_vector(vdev, queue_no) != vector) { > continue; > } > - kvm_virtio_pci_vq_vector_mask(proxy, queue_no, vector); > + virtio_pci_vq_vector_mask(proxy, queue_no, vector); > } > return ret; > } > > -static void kvm_virtio_pci_vector_mask(PCIDevice *dev, unsigned vector) > +static void virtio_pci_vector_mask(PCIDevice *dev, unsigned vector) > { > VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev); > VirtIODevice *vdev = proxy->vdev; > @@ -700,13 +703,13 @@ static void kvm_virtio_pci_vector_mask(PCIDevice *dev, unsigned vector) > if (virtio_queue_vector(vdev, queue_no) != vector) { > continue; > } > - kvm_virtio_pci_vq_vector_mask(proxy, queue_no, vector); > + virtio_pci_vq_vector_mask(proxy, queue_no, vector); > } > } > > -static void kvm_virtio_pci_vector_poll(PCIDevice *dev, > - unsigned int vector_start, > - unsigned int vector_end) > +static void virtio_pci_vector_poll(PCIDevice *dev, > + unsigned int vector_start, > + unsigned int vector_end) > { > VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev); > VirtIODevice *vdev = proxy->vdev; > @@ -781,11 +784,13 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign) > proxy->nvqs_with_notifiers = nvqs; > > /* Must unset vector notifier while guest notifier is still assigned */ > - if (proxy->vector_irqfd && !assign) { > + if ((proxy->vector_irqfd || vdev->guest_notifier_mask) && !assign) { > msix_unset_vector_notifiers(&proxy->pci_dev); > - kvm_virtio_pci_vector_release(proxy, nvqs); > - g_free(proxy->vector_irqfd); > - proxy->vector_irqfd = NULL; > + if (proxy->vector_irqfd) { > + kvm_virtio_pci_vector_release(proxy, nvqs); > + g_free(proxy->vector_irqfd); > + proxy->vector_irqfd = NULL; > + } > } > > for (n = 0; n < nvqs; n++) { > @@ -801,18 +806,20 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign) > } > > /* Must set vector notifier after guest notifier has been assigned */ > - if (with_irqfd && assign) { > - proxy->vector_irqfd = > - g_malloc0(sizeof(*proxy->vector_irqfd) * > - msix_nr_vectors_allocated(&proxy->pci_dev)); > - r = kvm_virtio_pci_vector_use(proxy, nvqs); > - if (r < 0) { > - goto assign_error; > + if ((with_irqfd || vdev->guest_notifier_mask) && assign) { > + if (with_irqfd) { > + proxy->vector_irqfd = > + g_malloc0(sizeof(*proxy->vector_irqfd) * > + msix_nr_vectors_allocated(&proxy->pci_dev)); > + r = kvm_virtio_pci_vector_use(proxy, nvqs); > + if (r < 0) { > + goto assign_error; > + } > } > r = msix_set_vector_notifiers(&proxy->pci_dev, > - kvm_virtio_pci_vector_unmask, > - kvm_virtio_pci_vector_mask, > - kvm_virtio_pci_vector_poll); > + virtio_pci_vector_unmask, > + virtio_pci_vector_mask, > + virtio_pci_vector_poll); > if (r < 0) { > goto notifiers_error; > } > @@ -821,8 +828,10 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign) > return 0; > > notifiers_error: > - assert(assign); > - kvm_virtio_pci_vector_release(proxy, nvqs); > + if (with_irqfd) { > + assert(assign); > + kvm_virtio_pci_vector_release(proxy, nvqs); > + } > > assign_error: > /* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */ > -- Alexey