From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=37797 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pfysn-0007dT-B7 for qemu-devel@nongnu.org; Thu, 20 Jan 2011 13:05:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pfysl-0001c2-Vm for qemu-devel@nongnu.org; Thu, 20 Jan 2011 13:05:17 -0500 Received: from mx1.redhat.com ([209.132.183.28]:1110) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pfysl-0001ay-MG for qemu-devel@nongnu.org; Thu, 20 Jan 2011 13:05:15 -0500 From: Alex Williamson In-Reply-To: <20110120153521.GA24357@redhat.com> References: <20110120153521.GA24357@redhat.com> Content-Type: text/plain; charset="UTF-8" Date: Thu, 20 Jan 2011 11:05:09 -0700 Message-ID: <1295546709.2878.6.camel@x201> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH] vhost: force vhost off for non-MSI guests List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Jes.Sorensen@redhat.com, jasowang@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, Juan Quintela On Thu, 2011-01-20 at 17:35 +0200, Michael S. Tsirkin wrote: > When MSI is off, each interrupt needs to be bounced through the io > thread when it's set/cleared, so vhost-net causes more context switches and > higher CPU utilization than userspace virtio which handles networking in > the same thread. > > We'll need to fix this by adding level irq support in kvm irqfd, > for now disable vhost-net in these configurations. We need this if we want avoid bouncing vfio INtx through qemu too. > Signed-off-by: Michael S. Tsirkin > --- > > I need to report some error from virtio-pci > that would be handled specially (disable but don't > report an error) so I wanted one that's never likely to be used by a > userspace ioctl. I selected ERANGE but it'd > be easy to switch to something else. Comments? > > hw/vhost.c | 4 +++- > hw/virtio-net.c | 6 ++++-- > hw/virtio-pci.c | 3 +++ > hw/virtio.h | 2 ++ > 4 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/hw/vhost.c b/hw/vhost.c > index 1d09ed0..c79765a 100644 > --- a/hw/vhost.c > +++ b/hw/vhost.c > @@ -649,7 +649,9 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) > > r = vdev->binding->set_guest_notifiers(vdev->binding_opaque, true); > if (r < 0) { > - fprintf(stderr, "Error binding guest notifier: %d\n", -r); > + if (r != -EVIRTIO_DISABLED) { > + fprintf(stderr, "Error binding guest notifier: %d\n", -r); > + } style - the above is tab indented. > goto fail_notifiers; > } > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > index ccb3e63..5de3fee 100644 > --- a/hw/virtio-net.c > +++ b/hw/virtio-net.c > @@ -121,8 +121,10 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) > if (!n->vhost_started) { > int r = vhost_net_start(tap_get_vhost_net(n->nic->nc.peer), &n->vdev); > if (r < 0) { > - error_report("unable to start vhost net: %d: " > - "falling back on userspace virtio", -r); > + if (r != -EVIRTIO_DISABLED) { > + error_report("unable to start vhost net: %d: " > + "falling back on userspace virtio", -r); > + } > } else { > n->vhost_started = 1; > } > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index dd8887a..dbf4be0 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -628,6 +628,9 @@ static int virtio_pci_set_guest_notifier(void *opaque, int n, bool assign) > EventNotifier *notifier = virtio_queue_get_guest_notifier(vq); > > if (assign) { > + if (!msix_enabled(&proxy->pci_dev)) { > + return -EVIRTIO_DISABLED; > + } > int r = event_notifier_init(notifier, 0); > if (r < 0) { > return r; > diff --git a/hw/virtio.h b/hw/virtio.h > index d8546d5..53bbdba 100644 > --- a/hw/virtio.h > +++ b/hw/virtio.h > @@ -98,6 +98,8 @@ typedef struct { > void (*vmstate_change)(void * opaque, bool running); > } VirtIOBindings; > > +#define EVIRTIO_DISABLED ERANGE > + > #define VIRTIO_PCI_QUEUE_MAX 64 > > #define VIRTIO_NO_VECTOR 0xffff I'm not a fan of having this special return value. Why doesn't virtio-pci only setup the set_guest_notifiers function pointer when msix is enabled? If not that, it could at least expose some virtio_foo_enabled() type feature that vhost could check. Thanks, Alex