diff mbox

[RFC,1/6] virtio-bus: common ioeventfd infrastructure

Message ID 1458208893-15949-2-git-send-email-cornelia.huck@de.ibm.com
State New
Headers show

Commit Message

Cornelia Huck March 17, 2016, 10:01 a.m. UTC
Introduce a set of ioeventfd callbacks on the virtio-bus level
that can be implemented by the individual transports. At the
virtio-bus level, do common handling for host notifiers (which
is actually most of it).

Two things of note:
- We always iterate over all possible virtio queues, even though
ccw (currently) has a lower limit. It does not really matter in
this place.
- We allow for the virtio-bus caller to pass an "assign" argument
down when stopping ioeventfd, which the old interface did not allow.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/virtio/virtio-bus.c         | 108 +++++++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-bus.h |  14 ++++++
 2 files changed, 122 insertions(+)

Comments

Fam Zheng March 22, 2016, 12:24 a.m. UTC | #1
On Thu, 03/17 11:01, Cornelia Huck wrote:
> Introduce a set of ioeventfd callbacks on the virtio-bus level
> that can be implemented by the individual transports. At the
> virtio-bus level, do common handling for host notifiers (which
> is actually most of it).
> 
> Two things of note:
> - We always iterate over all possible virtio queues, even though
> ccw (currently) has a lower limit. It does not really matter in
> this place.
> - We allow for the virtio-bus caller to pass an "assign" argument
> down when stopping ioeventfd, which the old interface did not allow.
> 
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  hw/virtio/virtio-bus.c         | 108 +++++++++++++++++++++++++++++++++++++++++
>  include/hw/virtio/virtio-bus.h |  14 ++++++
>  2 files changed, 122 insertions(+)
> 
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index 574f0e2..501300f 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -146,6 +146,114 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
>      }
>  }
>  
> +static int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
> +                                      int n, bool assign, bool set_handler)
> +{
> +    VirtIODevice *vdev = virtio_bus_get_device(bus);
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    VirtQueue *vq = virtio_get_queue(vdev, n);
> +    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
> +    int r = 0;
> +
> +    if (assign) {
> +        r = event_notifier_init(notifier, 1);
> +        if (r < 0) {
> +            error_report("%s: unable to init event notifier: %d", __func__, r);
> +            return r;
> +        }
> +        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
> +        r = k->ioeventfd_assign(proxy, notifier, n, assign);
> +        if (r < 0) {
> +            error_report("%s: unable to assign ioeventfd: %d", __func__, r);
> +            virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> +            event_notifier_cleanup(notifier);
> +            return r;
> +        }
> +    } else {
> +        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> +        k->ioeventfd_assign(proxy, notifier, n, assign);
> +        event_notifier_cleanup(notifier);
> +    }
> +    return r;
> +}
> +
> +void virtio_bus_start_ioeventfd(VirtioBusState *bus)
> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +    VirtIODevice *vdev;
> +    int n, r;
> +
> +    if (!k->ioeventfd_started || k->ioeventfd_started(proxy)) {
> +        return;
> +    }
> +    if (!k->ioeventfd_disabled(proxy)) {
> +        return;
> +    }
> +    vdev = virtio_bus_get_device(bus);
> +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
> +        r = set_host_notifier_internal(proxy, bus, n, true, true);
> +        if (r < 0) {
> +            goto assign_error;
> +        }
> +    }
> +    k->ioeventfd_set_started(proxy, true, false);
> +    return;
> +
> +assign_error:
> +    while (--n >= 0) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
> +
> +        r = set_host_notifier_internal(proxy, bus, n, false, false);
> +        assert(r >= 0);
> +    }
> +    k->ioeventfd_set_started(proxy, false, true);
> +    error_report("%s: failed. Fallback to userspace (slower).", __func__);
> +}
> +
> +void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign)
> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +    VirtIODevice *vdev;
> +    int n, r;
> +
> +    if (!k->ioeventfd_started || !k->ioeventfd_started(proxy)) {
> +        return;
> +    }
> +    vdev = virtio_bus_get_device(bus);
> +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
> +        r = set_host_notifier_internal(proxy, bus, n, assign, false);
> +        assert(r >= 0);
> +    }
> +    k->ioeventfd_set_started(proxy, false, false);
> +}
> +
> +int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +
> +    if (!k->ioeventfd_started) {
> +        return -ENOSYS;
> +    }
> +    /* Stop using the generic ioeventfd, we are doing eventfd handling
> +     * ourselves below */
> +    k->ioeventfd_set_disabled(proxy, assign);
> +    if (assign) {
> +        virtio_bus_stop_ioeventfd(bus, assign);
> +    }
> +    return set_host_notifier_internal(proxy, bus, n, assign, false);
> +}
> +
>  static char *virtio_bus_get_dev_path(DeviceState *dev)
>  {
>      BusState *bus = qdev_get_parent_bus(dev);
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index 3f2c136..0281cbf 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -71,6 +71,16 @@ typedef struct VirtioBusClass {
>      void (*device_unplugged)(DeviceState *d);
>      int (*query_nvectors)(DeviceState *d);
>      /*
> +     * ioeventfd handling: if the transport implements ioeventfd_started,
> +     * it must implement the other ioeventfd callbacks as well
> +     */
> +    bool (*ioeventfd_started)(DeviceState *d);
> +    void (*ioeventfd_set_started)(DeviceState *d, bool started, bool err);
> +    bool (*ioeventfd_disabled)(DeviceState *d);
> +    void (*ioeventfd_set_disabled)(DeviceState *d, bool disabled);
> +    int (*ioeventfd_assign)(DeviceState *d, EventNotifier *notifier,
> +                            int n, bool assign);

Maybe we should consider documenting these operations and parameters?

> +    /*
>       * Does the transport have variable vring alignment?
>       * (ie can it ever call virtio_queue_set_align()?)
>       * Note that changing this will break migration for this transport.
> @@ -111,4 +121,8 @@ static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
>      return (VirtIODevice *)qdev;
>  }
>  
> +void virtio_bus_start_ioeventfd(VirtioBusState *bus);
> +void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign);
> +int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign);
> +
>  #endif /* VIRTIO_BUS_H */
> -- 
> 2.6.5
>
Cornelia Huck March 22, 2016, 8:08 a.m. UTC | #2
On Tue, 22 Mar 2016 08:24:33 +0800
Fam Zheng <famz@redhat.com> wrote:

> On Thu, 03/17 11:01, Cornelia Huck wrote:

> > diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> > index 3f2c136..0281cbf 100644
> > --- a/include/hw/virtio/virtio-bus.h
> > +++ b/include/hw/virtio/virtio-bus.h
> > @@ -71,6 +71,16 @@ typedef struct VirtioBusClass {
> >      void (*device_unplugged)(DeviceState *d);
> >      int (*query_nvectors)(DeviceState *d);
> >      /*
> > +     * ioeventfd handling: if the transport implements ioeventfd_started,
> > +     * it must implement the other ioeventfd callbacks as well
> > +     */
> > +    bool (*ioeventfd_started)(DeviceState *d);
> > +    void (*ioeventfd_set_started)(DeviceState *d, bool started, bool err);
> > +    bool (*ioeventfd_disabled)(DeviceState *d);
> > +    void (*ioeventfd_set_disabled)(DeviceState *d, bool disabled);
> > +    int (*ioeventfd_assign)(DeviceState *d, EventNotifier *notifier,
> > +                            int n, bool assign);
> 
> Maybe we should consider documenting these operations and parameters?

Yes, we should :) I just wanted to make sure first that this is the way
to go.
Michael S. Tsirkin March 24, 2016, 11:20 a.m. UTC | #3
On Thu, Mar 17, 2016 at 11:01:28AM +0100, Cornelia Huck wrote:
> Introduce a set of ioeventfd callbacks on the virtio-bus level
> that can be implemented by the individual transports. At the
> virtio-bus level, do common handling for host notifiers (which
> is actually most of it).
> 
> Two things of note:
> - We always iterate over all possible virtio queues, even though
> ccw (currently) has a lower limit. It does not really matter in
> this place.
> - We allow for the virtio-bus caller to pass an "assign" argument
> down when stopping ioeventfd, which the old interface did not allow.
> 
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>

So it sounds like this is supposed to fix races
in current code, pls document how.


> ---
>  hw/virtio/virtio-bus.c         | 108 +++++++++++++++++++++++++++++++++++++++++
>  include/hw/virtio/virtio-bus.h |  14 ++++++
>  2 files changed, 122 insertions(+)
> 
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index 574f0e2..501300f 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -146,6 +146,114 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
>      }
>  }
>  
> +static int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
> +                                      int n, bool assign, bool set_handler)
> +{
> +    VirtIODevice *vdev = virtio_bus_get_device(bus);
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    VirtQueue *vq = virtio_get_queue(vdev, n);
> +    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
> +    int r = 0;
> +
> +    if (assign) {
> +        r = event_notifier_init(notifier, 1);
> +        if (r < 0) {
> +            error_report("%s: unable to init event notifier: %d", __func__, r);
> +            return r;
> +        }
> +        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
> +        r = k->ioeventfd_assign(proxy, notifier, n, assign);
> +        if (r < 0) {
> +            error_report("%s: unable to assign ioeventfd: %d", __func__, r);
> +            virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> +            event_notifier_cleanup(notifier);
> +            return r;
> +        }
> +    } else {
> +        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> +        k->ioeventfd_assign(proxy, notifier, n, assign);
> +        event_notifier_cleanup(notifier);
> +    }
> +    return r;
> +}
> +
> +void virtio_bus_start_ioeventfd(VirtioBusState *bus)
> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +    VirtIODevice *vdev;
> +    int n, r;
> +
> +    if (!k->ioeventfd_started || k->ioeventfd_started(proxy)) {
> +        return;
> +    }
> +    if (!k->ioeventfd_disabled(proxy)) {
> +        return;
> +    }
> +    vdev = virtio_bus_get_device(bus);
> +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
> +        r = set_host_notifier_internal(proxy, bus, n, true, true);
> +        if (r < 0) {
> +            goto assign_error;
> +        }
> +    }
> +    k->ioeventfd_set_started(proxy, true, false);
> +    return;
> +
> +assign_error:
> +    while (--n >= 0) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
> +
> +        r = set_host_notifier_internal(proxy, bus, n, false, false);
> +        assert(r >= 0);
> +    }
> +    k->ioeventfd_set_started(proxy, false, true);
> +    error_report("%s: failed. Fallback to userspace (slower).", __func__);
> +}
> +
> +void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign)
> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +    VirtIODevice *vdev;
> +    int n, r;
> +
> +    if (!k->ioeventfd_started || !k->ioeventfd_started(proxy)) {
> +        return;
> +    }
> +    vdev = virtio_bus_get_device(bus);
> +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
> +        r = set_host_notifier_internal(proxy, bus, n, assign, false);
> +        assert(r >= 0);
> +    }
> +    k->ioeventfd_set_started(proxy, false, false);
> +}
> +
> +int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)


Could you add documentation for what does "assign" mean here?
It was there as an internal API, but now it's external
so needs better docs.

> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +
> +    if (!k->ioeventfd_started) {
> +        return -ENOSYS;
> +    }
> +    /* Stop using the generic ioeventfd, we are doing eventfd handling
> +     * ourselves below */
> +    k->ioeventfd_set_disabled(proxy, assign);
> +    if (assign) {
> +        virtio_bus_stop_ioeventfd(bus, assign);
> +    }

Need comment to explain why don't we start on !assign.

> +    return set_host_notifier_internal(proxy, bus, n, assign, false);
> +}
> +
>  static char *virtio_bus_get_dev_path(DeviceState *dev)
>  {
>      BusState *bus = qdev_get_parent_bus(dev);
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index 3f2c136..0281cbf 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -71,6 +71,16 @@ typedef struct VirtioBusClass {
>      void (*device_unplugged)(DeviceState *d);
>      int (*query_nvectors)(DeviceState *d);
>      /*
> +     * ioeventfd handling: if the transport implements ioeventfd_started,
> +     * it must implement the other ioeventfd callbacks as well
> +     */
> +    bool (*ioeventfd_started)(DeviceState *d);
> +    void (*ioeventfd_set_started)(DeviceState *d, bool started, bool err);
> +    bool (*ioeventfd_disabled)(DeviceState *d);
> +    void (*ioeventfd_set_disabled)(DeviceState *d, bool disabled);
> +    int (*ioeventfd_assign)(DeviceState *d, EventNotifier *notifier,
> +                            int n, bool assign);
> +    /*
>       * Does the transport have variable vring alignment?
>       * (ie can it ever call virtio_queue_set_align()?)
>       * Note that changing this will break migration for this transport.
> @@ -111,4 +121,8 @@ static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
>      return (VirtIODevice *)qdev;
>  }
>  
> +void virtio_bus_start_ioeventfd(VirtioBusState *bus);
> +void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign);
> +int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign);
> +
>  #endif /* VIRTIO_BUS_H */
> -- 
> 2.6.5
Cornelia Huck March 24, 2016, 11:30 a.m. UTC | #4
On Thu, 24 Mar 2016 13:20:34 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Mar 17, 2016 at 11:01:28AM +0100, Cornelia Huck wrote:
> > Introduce a set of ioeventfd callbacks on the virtio-bus level
> > that can be implemented by the individual transports. At the
> > virtio-bus level, do common handling for host notifiers (which
> > is actually most of it).
> > 
> > Two things of note:
> > - We always iterate over all possible virtio queues, even though
> > ccw (currently) has a lower limit. It does not really matter in
> > this place.
> > - We allow for the virtio-bus caller to pass an "assign" argument
> > down when stopping ioeventfd, which the old interface did not allow.
> > 
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> 
> So it sounds like this is supposed to fix races
> in current code, pls document how.

Yes, final version will have more description + comments.

> 
> 
> > ---
> >  hw/virtio/virtio-bus.c         | 108 +++++++++++++++++++++++++++++++++++++++++
> >  include/hw/virtio/virtio-bus.h |  14 ++++++
> >  2 files changed, 122 insertions(+)
> > 
> > diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> > index 574f0e2..501300f 100644
> > --- a/hw/virtio/virtio-bus.c
> > +++ b/hw/virtio/virtio-bus.c
> > @@ -146,6 +146,114 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
> >      }
> >  }
> >  
> > +static int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
> > +                                      int n, bool assign, bool set_handler)
> > +{
> > +    VirtIODevice *vdev = virtio_bus_get_device(bus);
> > +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> > +    VirtQueue *vq = virtio_get_queue(vdev, n);
> > +    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
> > +    int r = 0;
> > +
> > +    if (assign) {
> > +        r = event_notifier_init(notifier, 1);
> > +        if (r < 0) {
> > +            error_report("%s: unable to init event notifier: %d", __func__, r);
> > +            return r;
> > +        }
> > +        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
> > +        r = k->ioeventfd_assign(proxy, notifier, n, assign);
> > +        if (r < 0) {
> > +            error_report("%s: unable to assign ioeventfd: %d", __func__, r);
> > +            virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> > +            event_notifier_cleanup(notifier);
> > +            return r;
> > +        }
> > +    } else {
> > +        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> > +        k->ioeventfd_assign(proxy, notifier, n, assign);
> > +        event_notifier_cleanup(notifier);
> > +    }
> > +    return r;
> > +}

Note that I'm currently trying to disentangle ioeventfd registration
and handler assignment, so this will look a bit different in the final
patch.

> > +
> > +void virtio_bus_start_ioeventfd(VirtioBusState *bus)
> > +{
> > +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> > +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> > +    VirtIODevice *vdev;
> > +    int n, r;
> > +
> > +    if (!k->ioeventfd_started || k->ioeventfd_started(proxy)) {
> > +        return;
> > +    }
> > +    if (!k->ioeventfd_disabled(proxy)) {

And this one is actually the wrong way around, as mentioned in the
other thread. Will be fixed.

> > +        return;
> > +    }
> > +    vdev = virtio_bus_get_device(bus);
> > +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> > +        if (!virtio_queue_get_num(vdev, n)) {
> > +            continue;
> > +        }
> > +        r = set_host_notifier_internal(proxy, bus, n, true, true);
> > +        if (r < 0) {
> > +            goto assign_error;
> > +        }
> > +    }
> > +    k->ioeventfd_set_started(proxy, true, false);
> > +    return;
> > +
> > +assign_error:
> > +    while (--n >= 0) {
> > +        if (!virtio_queue_get_num(vdev, n)) {
> > +            continue;
> > +        }
> > +
> > +        r = set_host_notifier_internal(proxy, bus, n, false, false);
> > +        assert(r >= 0);
> > +    }
> > +    k->ioeventfd_set_started(proxy, false, true);
> > +    error_report("%s: failed. Fallback to userspace (slower).", __func__);
> > +}
> > +
> > +void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign)
> > +{
> > +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> > +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> > +    VirtIODevice *vdev;
> > +    int n, r;
> > +
> > +    if (!k->ioeventfd_started || !k->ioeventfd_started(proxy)) {
> > +        return;
> > +    }
> > +    vdev = virtio_bus_get_device(bus);
> > +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> > +        if (!virtio_queue_get_num(vdev, n)) {
> > +            continue;
> > +        }
> > +        r = set_host_notifier_internal(proxy, bus, n, assign, false);
> > +        assert(r >= 0);
> > +    }
> > +    k->ioeventfd_set_started(proxy, false, false);
> > +}
> > +
> > +int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
> 
> 
> Could you add documentation for what does "assign" mean here?
> It was there as an internal API, but now it's external
> so needs better docs.

On my list.

> 
> > +{
> > +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> > +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> > +
> > +    if (!k->ioeventfd_started) {
> > +        return -ENOSYS;
> > +    }
> > +    /* Stop using the generic ioeventfd, we are doing eventfd handling
> > +     * ourselves below */
> > +    k->ioeventfd_set_disabled(proxy, assign);
> > +    if (assign) {
> > +        virtio_bus_stop_ioeventfd(bus, assign);
> > +    }
> 
> Need comment to explain why don't we start on !assign.

That's a part I'm currently reworking anyway, as we don't want to try
to assign ioeventfds twice, but just keep them. It will be hopefully
clearer then.

> 
> > +    return set_host_notifier_internal(proxy, bus, n, assign, false);
> > +}
> > +
> >  static char *virtio_bus_get_dev_path(DeviceState *dev)
> >  {
> >      BusState *bus = qdev_get_parent_bus(dev);
> > diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> > index 3f2c136..0281cbf 100644
> > --- a/include/hw/virtio/virtio-bus.h
> > +++ b/include/hw/virtio/virtio-bus.h
> > @@ -71,6 +71,16 @@ typedef struct VirtioBusClass {
> >      void (*device_unplugged)(DeviceState *d);
> >      int (*query_nvectors)(DeviceState *d);
> >      /*
> > +     * ioeventfd handling: if the transport implements ioeventfd_started,
> > +     * it must implement the other ioeventfd callbacks as well
> > +     */
> > +    bool (*ioeventfd_started)(DeviceState *d);
> > +    void (*ioeventfd_set_started)(DeviceState *d, bool started, bool err);
> > +    bool (*ioeventfd_disabled)(DeviceState *d);
> > +    void (*ioeventfd_set_disabled)(DeviceState *d, bool disabled);
> > +    int (*ioeventfd_assign)(DeviceState *d, EventNotifier *notifier,
> > +                            int n, bool assign);

I've also added a short description for the callbacks in the meantime.

> > +    /*
> >       * Does the transport have variable vring alignment?
> >       * (ie can it ever call virtio_queue_set_align()?)
> >       * Note that changing this will break migration for this transport.
> > @@ -111,4 +121,8 @@ static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
> >      return (VirtIODevice *)qdev;
> >  }
> >  
> > +void virtio_bus_start_ioeventfd(VirtioBusState *bus);
> > +void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign);
> > +int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign);
> > +
> >  #endif /* VIRTIO_BUS_H */
> > -- 
> > 2.6.5
>
diff mbox

Patch

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 574f0e2..501300f 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -146,6 +146,114 @@  void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
     }
 }
 
+static int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
+                                      int n, bool assign, bool set_handler)
+{
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
+    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
+    int r = 0;
+
+    if (assign) {
+        r = event_notifier_init(notifier, 1);
+        if (r < 0) {
+            error_report("%s: unable to init event notifier: %d", __func__, r);
+            return r;
+        }
+        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
+        r = k->ioeventfd_assign(proxy, notifier, n, assign);
+        if (r < 0) {
+            error_report("%s: unable to assign ioeventfd: %d", __func__, r);
+            virtio_queue_set_host_notifier_fd_handler(vq, false, false);
+            event_notifier_cleanup(notifier);
+            return r;
+        }
+    } else {
+        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
+        k->ioeventfd_assign(proxy, notifier, n, assign);
+        event_notifier_cleanup(notifier);
+    }
+    return r;
+}
+
+void virtio_bus_start_ioeventfd(VirtioBusState *bus)
+{
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
+    DeviceState *proxy = DEVICE(BUS(bus)->parent);
+    VirtIODevice *vdev;
+    int n, r;
+
+    if (!k->ioeventfd_started || k->ioeventfd_started(proxy)) {
+        return;
+    }
+    if (!k->ioeventfd_disabled(proxy)) {
+        return;
+    }
+    vdev = virtio_bus_get_device(bus);
+    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+        r = set_host_notifier_internal(proxy, bus, n, true, true);
+        if (r < 0) {
+            goto assign_error;
+        }
+    }
+    k->ioeventfd_set_started(proxy, true, false);
+    return;
+
+assign_error:
+    while (--n >= 0) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+
+        r = set_host_notifier_internal(proxy, bus, n, false, false);
+        assert(r >= 0);
+    }
+    k->ioeventfd_set_started(proxy, false, true);
+    error_report("%s: failed. Fallback to userspace (slower).", __func__);
+}
+
+void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign)
+{
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
+    DeviceState *proxy = DEVICE(BUS(bus)->parent);
+    VirtIODevice *vdev;
+    int n, r;
+
+    if (!k->ioeventfd_started || !k->ioeventfd_started(proxy)) {
+        return;
+    }
+    vdev = virtio_bus_get_device(bus);
+    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+        r = set_host_notifier_internal(proxy, bus, n, assign, false);
+        assert(r >= 0);
+    }
+    k->ioeventfd_set_started(proxy, false, false);
+}
+
+int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
+{
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
+    DeviceState *proxy = DEVICE(BUS(bus)->parent);
+
+    if (!k->ioeventfd_started) {
+        return -ENOSYS;
+    }
+    /* Stop using the generic ioeventfd, we are doing eventfd handling
+     * ourselves below */
+    k->ioeventfd_set_disabled(proxy, assign);
+    if (assign) {
+        virtio_bus_stop_ioeventfd(bus, assign);
+    }
+    return set_host_notifier_internal(proxy, bus, n, assign, false);
+}
+
 static char *virtio_bus_get_dev_path(DeviceState *dev)
 {
     BusState *bus = qdev_get_parent_bus(dev);
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 3f2c136..0281cbf 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -71,6 +71,16 @@  typedef struct VirtioBusClass {
     void (*device_unplugged)(DeviceState *d);
     int (*query_nvectors)(DeviceState *d);
     /*
+     * ioeventfd handling: if the transport implements ioeventfd_started,
+     * it must implement the other ioeventfd callbacks as well
+     */
+    bool (*ioeventfd_started)(DeviceState *d);
+    void (*ioeventfd_set_started)(DeviceState *d, bool started, bool err);
+    bool (*ioeventfd_disabled)(DeviceState *d);
+    void (*ioeventfd_set_disabled)(DeviceState *d, bool disabled);
+    int (*ioeventfd_assign)(DeviceState *d, EventNotifier *notifier,
+                            int n, bool assign);
+    /*
      * Does the transport have variable vring alignment?
      * (ie can it ever call virtio_queue_set_align()?)
      * Note that changing this will break migration for this transport.
@@ -111,4 +121,8 @@  static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
     return (VirtIODevice *)qdev;
 }
 
+void virtio_bus_start_ioeventfd(VirtioBusState *bus);
+void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign);
+int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign);
+
 #endif /* VIRTIO_BUS_H */