[Qemu-devel] Re: [PATCH] vhost: force vhost off for non-MSI guests

[Sridhar Samudrala <sri@us.ibm.com>]

On Thu, 2011-01-20 at 19:47 +0200, Michael S. Tsirkin wrote:
> On Thu, Jan 20, 2011 at 08:31:53AM -0800, Sridhar Samudrala wrote:
> > 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.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > > 
> > > 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?
> > 
> > Should this error be EVHOST_DISABLED rather than EVIRTIO_DISABLED?
> > 
> > -Sridhar
> 
> The error is reported by virtio-pci which does not know about vhost.
> I started with EVIRTIO_MSIX_DISABLED and made is shorter.
> Would EVIRTIO_MSIX_DISABLED be better?

I think so. This makes it more clear.
-Sridhar
> 
> > > 
> > >  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);
> > > +	}
> > >          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