| Message ID | 1458208893-15949-2-git-send-email-cornelia.huck@de.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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.
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
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 --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 */
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(+)