* [Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure
2016-03-17 10:01 [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers Cornelia Huck
@ 2016-03-17 10:01 ` Cornelia Huck
2016-03-22 0:24 ` Fam Zheng
2016-03-24 11:20 ` Michael S. Tsirkin
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 2/6] virtio-bus: have callers tolerate new host notifier api Cornelia Huck
` (6 subsequent siblings)
7 siblings, 2 replies; 13+ messages in thread
From: Cornelia Huck @ 2016-03-17 10:01 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, mst, borntraeger, stefanha, Cornelia Huck, pbonzini
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);
+ /*
* 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure Cornelia Huck
@ 2016-03-22 0:24 ` Fam Zheng
2016-03-22 8:08 ` Cornelia Huck
2016-03-24 11:20 ` Michael S. Tsirkin
1 sibling, 1 reply; 13+ messages in thread
From: Fam Zheng @ 2016-03-22 0:24 UTC (permalink / raw)
To: Cornelia Huck; +Cc: pbonzini, borntraeger, qemu-devel, stefanha, mst
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure
2016-03-22 0:24 ` Fam Zheng
@ 2016-03-22 8:08 ` Cornelia Huck
0 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2016-03-22 8:08 UTC (permalink / raw)
To: Fam Zheng; +Cc: pbonzini, borntraeger, qemu-devel, stefanha, mst
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure Cornelia Huck
2016-03-22 0:24 ` Fam Zheng
@ 2016-03-24 11:20 ` Michael S. Tsirkin
2016-03-24 11:30 ` Cornelia Huck
1 sibling, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2016-03-24 11:20 UTC (permalink / raw)
To: Cornelia Huck; +Cc: pbonzini, famz, qemu-devel, stefanha, borntraeger
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure
2016-03-24 11:20 ` Michael S. Tsirkin
@ 2016-03-24 11:30 ` Cornelia Huck
0 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2016-03-24 11:30 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, famz, qemu-devel, stefanha, borntraeger
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH RFC 2/6] virtio-bus: have callers tolerate new host notifier api
2016-03-17 10:01 [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers Cornelia Huck
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure Cornelia Huck
@ 2016-03-17 10:01 ` Cornelia Huck
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 3/6] virtio-ccw: convert to ioeventfd callbacks Cornelia Huck
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2016-03-17 10:01 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, mst, borntraeger, stefanha, Cornelia Huck, pbonzini
Have vhost and dataplane use the new api for transports that
have been converted.
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
hw/block/dataplane/virtio-blk.c | 14 +++++++++++---
hw/scsi/virtio-scsi-dataplane.c | 20 +++++++++++++++-----
hw/virtio/vhost.c | 20 ++++++++++++++++----
3 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 36f3d2b..9ca41d7 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -132,7 +132,8 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
}
/* Don't try if transport does not support notifiers. */
- if (!k->set_guest_notifiers || !k->set_host_notifier) {
+ if (!k->set_guest_notifiers ||
+ (!k->set_host_notifier && !k->ioeventfd_started)) {
error_setg(errp,
"device is incompatible with dataplane "
"(transport does not support notifiers)");
@@ -209,7 +210,10 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
s->guest_notifier = virtio_queue_get_guest_notifier(s->vq);
/* Set up virtqueue notify */
- r = k->set_host_notifier(qbus->parent, 0, true);
+ r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, true);
+ if (r == -ENOSYS) {
+ r = k->set_host_notifier(qbus->parent, 0, true);
+ }
if (r != 0) {
fprintf(stderr, "virtio-blk failed to set host notifier (%d)\n", r);
goto fail_host_notifier;
@@ -244,6 +248,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s->vdev)));
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
+ int r;
if (!vblk->dataplane_started || s->stopping) {
return;
@@ -268,7 +273,10 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
aio_context_release(s->ctx);
- k->set_host_notifier(qbus->parent, 0, false);
+ r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, false);
+ if (r == -ENOSYS) {
+ k->set_host_notifier(qbus->parent, 0, false);
+ }
/* Clean up guest notifier (irq) */
k->set_guest_notifiers(qbus->parent, 1, false);
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index 367e476..88c3abd 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -32,7 +32,8 @@ void virtio_scsi_set_iothread(VirtIOSCSI *s, IOThread *iothread)
s->ctx = iothread_get_aio_context(vs->conf.iothread);
/* Don't try if transport does not support notifiers. */
- if (!k->set_guest_notifiers || !k->set_host_notifier) {
+ if (!k->set_guest_notifiers ||
+ (!k->set_host_notifier && !k->ioeventfd_started)) {
fprintf(stderr, "virtio-scsi: Failed to set iothread "
"(transport does not support notifiers)");
exit(1);
@@ -46,7 +47,10 @@ static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n)
int rc;
/* Set up virtqueue notify */
- rc = k->set_host_notifier(qbus->parent, n, true);
+ rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), n, true);
+ if (rc == -ENOSYS) {
+ rc = k->set_host_notifier(qbus->parent, n, true);
+ }
if (rc != 0) {
fprintf(stderr, "virtio-scsi: Failed to set host notifier (%d)\n",
rc);
@@ -129,7 +133,10 @@ fail_vrings:
virtio_scsi_clear_aio(s);
aio_context_release(s->ctx);
for (i = 0; i < vs->conf.num_queues + 2; i++) {
- k->set_host_notifier(qbus->parent, i, false);
+ rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
+ if (rc == -ENOSYS) {
+ k->set_host_notifier(qbus->parent, i, false);
+ }
}
k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, false);
fail_guest_notifiers:
@@ -144,7 +151,7 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
- int i;
+ int i, rc;
if (!s->dataplane_started || s->dataplane_stopping) {
return;
@@ -168,7 +175,10 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
aio_context_release(s->ctx);
for (i = 0; i < vs->conf.num_queues + 2; i++) {
- k->set_host_notifier(qbus->parent, i, false);
+ rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
+ if (rc == -ENOSYS) {
+ k->set_host_notifier(qbus->parent, i, false);
+ }
}
/* Clean up guest notifier (irq) */
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 392d848..38f54a2 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1113,14 +1113,18 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
VirtioBusState *vbus = VIRTIO_BUS(qbus);
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
int i, r, e;
- if (!k->set_host_notifier) {
+ if (!k->set_host_notifier || !k->ioeventfd_started) {
fprintf(stderr, "binding does not support host notifiers\n");
r = -ENOSYS;
goto fail;
}
for (i = 0; i < hdev->nvqs; ++i) {
- r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
+ r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +i,
+ true);
+ if (r == -ENOSYS) {
+ r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
+ }
if (r < 0) {
fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i, -r);
goto fail_vq;
@@ -1130,7 +1134,11 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
return 0;
fail_vq:
while (--i >= 0) {
- e = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
+ e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +i,
+ false);
+ if (e == -ENOSYS) {
+ e = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
+ }
if (e < 0) {
fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i, -r);
fflush(stderr);
@@ -1154,7 +1162,11 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
int i, r;
for (i = 0; i < hdev->nvqs; ++i) {
- r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
+ r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +i,
+ false);
+ if (r == -ENOSYS) {
+ r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
+ }
if (r < 0) {
fprintf(stderr, "vhost VQ %d notifier cleanup failed: %d\n", i, -r);
fflush(stderr);
--
2.6.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH RFC 3/6] virtio-ccw: convert to ioeventfd callbacks
2016-03-17 10:01 [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers Cornelia Huck
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure Cornelia Huck
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 2/6] virtio-bus: have callers tolerate new host notifier api Cornelia Huck
@ 2016-03-17 10:01 ` Cornelia Huck
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 4/6] virtio-pci: " Cornelia Huck
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2016-03-17 10:01 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, mst, borntraeger, stefanha, Cornelia Huck, pbonzini
Use the new interface.
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
hw/s390x/virtio-ccw.c | 133 +++++++++++++++++---------------------------------
1 file changed, 45 insertions(+), 88 deletions(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index cb887ba..8732b57 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -67,92 +67,58 @@ VirtIODevice *virtio_ccw_get_vdev(SubchDev *sch)
return vdev;
}
-static int virtio_ccw_set_guest2host_notifier(VirtioCcwDevice *dev, int n,
- bool assign, bool set_handler)
+static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
{
- VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
- VirtQueue *vq = virtio_get_queue(vdev, n);
- EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
- int r = 0;
- SubchDev *sch = dev->sch;
- uint32_t sch_id = (css_build_subchannel_id(sch) << 16) | sch->schid;
+ virtio_bus_start_ioeventfd(&dev->bus);
+}
- 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 = s390_assign_subch_ioeventfd(notifier, sch_id, 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);
- s390_assign_subch_ioeventfd(notifier, sch_id, n, assign);
- event_notifier_cleanup(notifier);
- }
- return r;
+static void virtio_ccw_stop_ioeventfd(VirtioCcwDevice *dev)
+{
+ virtio_bus_start_ioeventfd(&dev->bus);
}
-static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
+static bool virtio_ccw_ioeventfd_started(DeviceState *d)
{
- VirtIODevice *vdev;
- int n, r;
+ VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
- if (!(dev->flags & VIRTIO_CCW_FLAG_USE_IOEVENTFD) ||
- dev->ioeventfd_disabled ||
- dev->ioeventfd_started) {
- return;
- }
- vdev = virtio_bus_get_device(&dev->bus);
- for (n = 0; n < VIRTIO_CCW_QUEUE_MAX; n++) {
- if (!virtio_queue_get_num(vdev, n)) {
- continue;
- }
- r = virtio_ccw_set_guest2host_notifier(dev, n, true, true);
- if (r < 0) {
- goto assign_error;
- }
- }
- dev->ioeventfd_started = true;
- return;
+ return dev->ioeventfd_started;
+}
- assign_error:
- while (--n >= 0) {
- if (!virtio_queue_get_num(vdev, n)) {
- continue;
- }
- r = virtio_ccw_set_guest2host_notifier(dev, n, false, false);
- assert(r >= 0);
+static void virtio_ccw_ioeventfd_set_started(DeviceState *d, bool started,
+ bool err)
+{
+ VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+
+ dev->ioeventfd_started = started;
+ if (err) {
+ /* Disable ioeventfd for this device. */
+ dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
}
- dev->ioeventfd_started = false;
- /* Disable ioeventfd for this device. */
- dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
- error_report("%s: failed. Fallback to userspace (slower).", __func__);
}
-static void virtio_ccw_stop_ioeventfd(VirtioCcwDevice *dev)
+static bool virtio_ccw_ioeventfd_disabled(DeviceState *d)
{
- VirtIODevice *vdev;
- int n, r;
+ VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
- if (!dev->ioeventfd_started) {
- return;
- }
- vdev = virtio_bus_get_device(&dev->bus);
- for (n = 0; n < VIRTIO_CCW_QUEUE_MAX; n++) {
- if (!virtio_queue_get_num(vdev, n)) {
- continue;
- }
- r = virtio_ccw_set_guest2host_notifier(dev, n, false, false);
- assert(r >= 0);
- }
- dev->ioeventfd_started = false;
+ return dev->ioeventfd_disabled ||
+ !(dev->flags & VIRTIO_CCW_FLAG_USE_IOEVENTFD);
+}
+
+static void virtio_ccw_ioeventfd_set_disabled(DeviceState *d, bool disabled)
+{
+ VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+
+ dev->ioeventfd_disabled = disabled;
+}
+
+static int virtio_ccw_ioeventfd_assign(DeviceState *d, EventNotifier *notifier,
+ int n, bool assign)
+{
+ VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+ SubchDev *sch = dev->sch;
+ uint32_t sch_id = (css_build_subchannel_id(sch) << 16) | sch->schid;
+
+ return s390_assign_subch_ioeventfd(notifier, sch_id, n, assign);
}
VirtualCssBus *virtual_css_bus_init(void)
@@ -1193,19 +1159,6 @@ static bool virtio_ccw_query_guest_notifiers(DeviceState *d)
return !!(dev->sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA);
}
-static int virtio_ccw_set_host_notifier(DeviceState *d, int n, bool assign)
-{
- VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
-
- /* Stop using the generic ioeventfd, we are doing eventfd handling
- * ourselves below */
- dev->ioeventfd_disabled = assign;
- if (assign) {
- virtio_ccw_stop_ioeventfd(dev);
- }
- return virtio_ccw_set_guest2host_notifier(dev, n, assign, false);
-}
-
static int virtio_ccw_get_mappings(VirtioCcwDevice *dev)
{
int r;
@@ -1834,7 +1787,6 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
k->notify = virtio_ccw_notify;
k->vmstate_change = virtio_ccw_vmstate_change;
k->query_guest_notifiers = virtio_ccw_query_guest_notifiers;
- k->set_host_notifier = virtio_ccw_set_host_notifier;
k->set_guest_notifiers = virtio_ccw_set_guest_notifiers;
k->save_queue = virtio_ccw_save_queue;
k->load_queue = virtio_ccw_load_queue;
@@ -1843,6 +1795,11 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
k->device_plugged = virtio_ccw_device_plugged;
k->post_plugged = virtio_ccw_post_plugged;
k->device_unplugged = virtio_ccw_device_unplugged;
+ k->ioeventfd_started = virtio_ccw_ioeventfd_started;
+ k->ioeventfd_set_started = virtio_ccw_ioeventfd_set_started;
+ k->ioeventfd_disabled = virtio_ccw_ioeventfd_disabled;
+ k->ioeventfd_set_disabled = virtio_ccw_ioeventfd_set_disabled;
+ k->ioeventfd_assign = virtio_ccw_ioeventfd_assign;
}
static const TypeInfo virtio_ccw_bus_info = {
--
2.6.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH RFC 4/6] virtio-pci: convert to ioeventfd callbacks
2016-03-17 10:01 [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers Cornelia Huck
` (2 preceding siblings ...)
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 3/6] virtio-ccw: convert to ioeventfd callbacks Cornelia Huck
@ 2016-03-17 10:01 ` Cornelia Huck
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 5/6] virtio-mmio: " Cornelia Huck
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2016-03-17 10:01 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, mst, borntraeger, stefanha, Cornelia Huck, pbonzini
Convert to new interface.
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
hw/virtio/virtio-pci.c | 124 ++++++++++++++++---------------------------------
1 file changed, 41 insertions(+), 83 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 0dadb66..b5e59c6 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -261,14 +261,44 @@ static int virtio_pci_load_queue(DeviceState *d, int n, QEMUFile *f)
return 0;
}
+static bool virtio_pci_ioeventfd_started(DeviceState *d)
+{
+ VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+
+ return proxy->ioeventfd_started;
+}
+
+static void virtio_pci_ioeventfd_set_started(DeviceState *d, bool started,
+ bool err)
+{
+ VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+
+ proxy->ioeventfd_started = started;
+}
+
+static bool virtio_pci_ioeventfd_disabled(DeviceState *d)
+{
+ VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+
+ return proxy->ioeventfd_disabled ||
+ !(proxy->flags & VIRTIO_PCI_FLAG_USE_IOEVENTFD);
+}
+
+static void virtio_pci_ioeventfd_set_disabled(DeviceState *d, bool disabled)
+{
+ VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+
+ proxy->ioeventfd_disabled = disabled;
+}
+
#define QEMU_VIRTIO_PCI_QUEUE_MEM_MULT 0x1000
-static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
- int n, bool assign, bool set_handler)
+static int virtio_pci_ioeventfd_assign(DeviceState *d, EventNotifier *notifier,
+ int n, bool assign)
{
+ VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
VirtQueue *vq = virtio_get_queue(vdev, n);
- EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY);
bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
bool fast_mmio = kvm_ioeventfd_any_length_enabled();
@@ -279,16 +309,8 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
hwaddr modern_addr = QEMU_VIRTIO_PCI_QUEUE_MEM_MULT *
virtio_get_queue_index(vq);
hwaddr legacy_addr = VIRTIO_PCI_QUEUE_NOTIFY;
- 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);
if (modern) {
if (fast_mmio) {
memory_region_add_eventfd(modern_mr, modern_addr, 0,
@@ -324,68 +346,18 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
memory_region_del_eventfd(legacy_mr, legacy_addr, 2,
true, n, notifier);
}
- virtio_queue_set_host_notifier_fd_handler(vq, false, false);
- event_notifier_cleanup(notifier);
}
- return r;
+ return 0;
}
static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
{
- VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
- int n, r;
-
- if (!(proxy->flags & VIRTIO_PCI_FLAG_USE_IOEVENTFD) ||
- proxy->ioeventfd_disabled ||
- proxy->ioeventfd_started) {
- return;
- }
-
- for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
- if (!virtio_queue_get_num(vdev, n)) {
- continue;
- }
-
- r = virtio_pci_set_host_notifier_internal(proxy, n, true, true);
- if (r < 0) {
- goto assign_error;
- }
- }
- proxy->ioeventfd_started = true;
- return;
-
-assign_error:
- while (--n >= 0) {
- if (!virtio_queue_get_num(vdev, n)) {
- continue;
- }
-
- r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
- assert(r >= 0);
- }
- proxy->ioeventfd_started = false;
- error_report("%s: failed. Fallback to a userspace (slower).", __func__);
+ virtio_bus_start_ioeventfd(&proxy->bus);
}
static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
{
- VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
- int r;
- int n;
-
- if (!proxy->ioeventfd_started) {
- return;
- }
-
- for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
- if (!virtio_queue_get_num(vdev, n)) {
- continue;
- }
-
- r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
- assert(r >= 0);
- }
- proxy->ioeventfd_started = false;
+ virtio_bus_stop_ioeventfd(&proxy->bus, false);
}
static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
@@ -1111,24 +1083,6 @@ assign_error:
return r;
}
-static int virtio_pci_set_host_notifier(DeviceState *d, int n, bool assign)
-{
- VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
-
- /* Stop using ioeventfd for virtqueue kick if the device starts using host
- * notifiers. This makes it easy to avoid stepping on each others' toes.
- */
- proxy->ioeventfd_disabled = assign;
- if (assign) {
- virtio_pci_stop_ioeventfd(proxy);
- }
- /* We don't need to start here: it's not needed because backend
- * currently only stops on status change away from ok,
- * reset, vmstop and such. If we do add code to start here,
- * need to check vmstate, device state etc. */
- return virtio_pci_set_host_notifier_internal(proxy, n, assign, false);
-}
-
static void virtio_pci_vmstate_change(DeviceState *d, bool running)
{
VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
@@ -2489,12 +2443,16 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
k->load_extra_state = virtio_pci_load_extra_state;
k->has_extra_state = virtio_pci_has_extra_state;
k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
- k->set_host_notifier = virtio_pci_set_host_notifier;
k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
k->vmstate_change = virtio_pci_vmstate_change;
k->device_plugged = virtio_pci_device_plugged;
k->device_unplugged = virtio_pci_device_unplugged;
k->query_nvectors = virtio_pci_query_nvectors;
+ k->ioeventfd_started = virtio_pci_ioeventfd_started;
+ k->ioeventfd_set_started = virtio_pci_ioeventfd_set_started;
+ k->ioeventfd_disabled = virtio_pci_ioeventfd_disabled;
+ k->ioeventfd_set_disabled = virtio_pci_ioeventfd_set_disabled;
+ k->ioeventfd_assign = virtio_pci_ioeventfd_assign;
}
static const TypeInfo virtio_pci_bus_info = {
--
2.6.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH RFC 5/6] virtio-mmio: convert to ioeventfd callbacks
2016-03-17 10:01 [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers Cornelia Huck
` (3 preceding siblings ...)
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 4/6] virtio-pci: " Cornelia Huck
@ 2016-03-17 10:01 ` Cornelia Huck
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 6/6] virtio-bus: remove old set_host_notifier callback Cornelia Huck
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2016-03-17 10:01 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, mst, borntraeger, stefanha, Cornelia Huck, pbonzini
Convert to the new interface.
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
hw/virtio/virtio-mmio.c | 128 ++++++++++++++++--------------------------------
1 file changed, 41 insertions(+), 87 deletions(-)
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index d4cd91f..d2accbc 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -93,90 +93,59 @@ typedef struct {
bool ioeventfd_started;
} VirtIOMMIOProxy;
-static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy,
- int n, bool assign,
- bool set_handler)
+static bool virtio_mmio_ioeventfd_started(DeviceState *d)
{
- VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
- VirtQueue *vq = virtio_get_queue(vdev, n);
- EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
- int r = 0;
+ VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
- 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);
- memory_region_add_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
- true, n, notifier);
- } else {
- memory_region_del_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
- true, n, notifier);
- virtio_queue_set_host_notifier_fd_handler(vq, false, false);
- event_notifier_cleanup(notifier);
- }
- return r;
+ return proxy->ioeventfd_started;
}
-static void virtio_mmio_start_ioeventfd(VirtIOMMIOProxy *proxy)
+static void virtio_mmio_ioeventfd_set_started(DeviceState *d, bool started,
+ bool err)
{
- VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
- int n, r;
+ VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
- if (!kvm_eventfds_enabled() ||
- proxy->ioeventfd_disabled ||
- proxy->ioeventfd_started) {
- return;
- }
+ proxy->ioeventfd_started = started;
+}
- for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
- if (!virtio_queue_get_num(vdev, n)) {
- continue;
- }
+static bool virtio_mmio_ioeventfd_disabled(DeviceState *d)
+{
+ VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
- r = virtio_mmio_set_host_notifier_internal(proxy, n, true, true);
- if (r < 0) {
- goto assign_error;
- }
- }
- proxy->ioeventfd_started = true;
- return;
+ return !kvm_eventfds_enabled() || proxy->ioeventfd_disabled;
+}
-assign_error:
- while (--n >= 0) {
- if (!virtio_queue_get_num(vdev, n)) {
- continue;
- }
+static void virtio_mmio_ioeventfd_set_disabled(DeviceState *d, bool disabled)
+{
+ VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
- r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false);
- assert(r >= 0);
- }
- proxy->ioeventfd_started = false;
- error_report("%s: failed. Fallback to a userspace (slower).", __func__);
+ proxy->ioeventfd_disabled = disabled;
}
-static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy)
+static int virtio_mmio_ioeventfd_assign(DeviceState *d,
+ EventNotifier *notifier,
+ int n, bool assign)
{
- int r;
- int n;
- VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+ VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
- if (!proxy->ioeventfd_started) {
- return;
+ if (assign) {
+ memory_region_add_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
+ true, n, notifier);
+ } else {
+ memory_region_del_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
+ true, n, notifier);
}
+ return 0;
+}
- for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
- if (!virtio_queue_get_num(vdev, n)) {
- continue;
- }
+static void virtio_mmio_start_ioeventfd(VirtIOMMIOProxy *proxy)
+{
+ virtio_bus_start_ioeventfd(&proxy->bus);
+}
- r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false);
- assert(r >= 0);
- }
- proxy->ioeventfd_started = false;
+static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy)
+{
+ virtio_bus_stop_ioeventfd(&proxy->bus, false);
}
static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
@@ -498,25 +467,6 @@ assign_error:
return r;
}
-static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n,
- bool assign)
-{
- VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
-
- /* Stop using ioeventfd for virtqueue kick if the device starts using host
- * notifiers. This makes it easy to avoid stepping on each others' toes.
- */
- proxy->ioeventfd_disabled = assign;
- if (assign) {
- virtio_mmio_stop_ioeventfd(proxy);
- }
- /* We don't need to start here: it's not needed because backend
- * currently only stops on status change away from ok,
- * reset, vmstop and such. If we do add code to start here,
- * need to check vmstate, device state etc. */
- return virtio_mmio_set_host_notifier_internal(proxy, n, assign, false);
-}
-
/* virtio-mmio device */
static void virtio_mmio_realizefn(DeviceState *d, Error **errp)
@@ -558,8 +508,12 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
k->notify = virtio_mmio_update_irq;
k->save_config = virtio_mmio_save_config;
k->load_config = virtio_mmio_load_config;
- k->set_host_notifier = virtio_mmio_set_host_notifier;
k->set_guest_notifiers = virtio_mmio_set_guest_notifiers;
+ k->ioeventfd_started = virtio_mmio_ioeventfd_started;
+ k->ioeventfd_set_started = virtio_mmio_ioeventfd_set_started;
+ k->ioeventfd_disabled = virtio_mmio_ioeventfd_disabled;
+ k->ioeventfd_set_disabled = virtio_mmio_ioeventfd_set_disabled;
+ k->ioeventfd_assign = virtio_mmio_ioeventfd_assign;
k->has_variable_vring_alignment = true;
bus_class->max_dev = 1;
}
--
2.6.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH RFC 6/6] virtio-bus: remove old set_host_notifier callback
2016-03-17 10:01 [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers Cornelia Huck
` (4 preceding siblings ...)
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 5/6] virtio-mmio: " Cornelia Huck
@ 2016-03-17 10:01 ` Cornelia Huck
2016-03-21 15:02 ` [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers Stefan Hajnoczi
2016-03-23 9:34 ` Fam Zheng
7 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2016-03-17 10:01 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, mst, borntraeger, stefanha, Cornelia Huck, pbonzini
All users have been converted to the new ioevent callbacks.
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
hw/block/dataplane/virtio-blk.c | 12 ++----------
hw/scsi/virtio-scsi-dataplane.c | 19 ++++---------------
hw/virtio/vhost.c | 13 +------------
include/hw/virtio/virtio-bus.h | 1 -
4 files changed, 7 insertions(+), 38 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 9ca41d7..1b2d5fa 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -132,8 +132,7 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
}
/* Don't try if transport does not support notifiers. */
- if (!k->set_guest_notifiers ||
- (!k->set_host_notifier && !k->ioeventfd_started)) {
+ if (!k->set_guest_notifiers || !k->ioeventfd_started) {
error_setg(errp,
"device is incompatible with dataplane "
"(transport does not support notifiers)");
@@ -211,9 +210,6 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
/* Set up virtqueue notify */
r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, true);
- if (r == -ENOSYS) {
- r = k->set_host_notifier(qbus->parent, 0, true);
- }
if (r != 0) {
fprintf(stderr, "virtio-blk failed to set host notifier (%d)\n", r);
goto fail_host_notifier;
@@ -248,7 +244,6 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s->vdev)));
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
- int r;
if (!vblk->dataplane_started || s->stopping) {
return;
@@ -273,10 +268,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
aio_context_release(s->ctx);
- r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, false);
- if (r == -ENOSYS) {
- k->set_host_notifier(qbus->parent, 0, false);
- }
+ virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, false);
/* Clean up guest notifier (irq) */
k->set_guest_notifiers(qbus->parent, 1, false);
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index 88c3abd..88773d2 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -32,8 +32,7 @@ void virtio_scsi_set_iothread(VirtIOSCSI *s, IOThread *iothread)
s->ctx = iothread_get_aio_context(vs->conf.iothread);
/* Don't try if transport does not support notifiers. */
- if (!k->set_guest_notifiers ||
- (!k->set_host_notifier && !k->ioeventfd_started)) {
+ if (!k->set_guest_notifiers || !k->ioeventfd_started) {
fprintf(stderr, "virtio-scsi: Failed to set iothread "
"(transport does not support notifiers)");
exit(1);
@@ -43,14 +42,10 @@ void virtio_scsi_set_iothread(VirtIOSCSI *s, IOThread *iothread)
static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n)
{
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
- VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
int rc;
/* Set up virtqueue notify */
rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), n, true);
- if (rc == -ENOSYS) {
- rc = k->set_host_notifier(qbus->parent, n, true);
- }
if (rc != 0) {
fprintf(stderr, "virtio-scsi: Failed to set host notifier (%d)\n",
rc);
@@ -133,10 +128,7 @@ fail_vrings:
virtio_scsi_clear_aio(s);
aio_context_release(s->ctx);
for (i = 0; i < vs->conf.num_queues + 2; i++) {
- rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
- if (rc == -ENOSYS) {
- k->set_host_notifier(qbus->parent, i, false);
- }
+ virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
}
k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, false);
fail_guest_notifiers:
@@ -151,7 +143,7 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
- int i, rc;
+ int i;
if (!s->dataplane_started || s->dataplane_stopping) {
return;
@@ -175,10 +167,7 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
aio_context_release(s->ctx);
for (i = 0; i < vs->conf.num_queues + 2; i++) {
- rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
- if (rc == -ENOSYS) {
- k->set_host_notifier(qbus->parent, i, false);
- }
+ virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
}
/* Clean up guest notifier (irq) */
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 38f54a2..90febf2 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1113,7 +1113,7 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
VirtioBusState *vbus = VIRTIO_BUS(qbus);
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
int i, r, e;
- if (!k->set_host_notifier || !k->ioeventfd_started) {
+ if (!k->ioeventfd_started) {
fprintf(stderr, "binding does not support host notifiers\n");
r = -ENOSYS;
goto fail;
@@ -1122,9 +1122,6 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
for (i = 0; i < hdev->nvqs; ++i) {
r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +i,
true);
- if (r == -ENOSYS) {
- r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
- }
if (r < 0) {
fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i, -r);
goto fail_vq;
@@ -1136,9 +1133,6 @@ fail_vq:
while (--i >= 0) {
e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +i,
false);
- if (e == -ENOSYS) {
- e = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
- }
if (e < 0) {
fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i, -r);
fflush(stderr);
@@ -1157,16 +1151,11 @@ fail:
void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
{
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
- VirtioBusState *vbus = VIRTIO_BUS(qbus);
- VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
int i, r;
for (i = 0; i < hdev->nvqs; ++i) {
r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +i,
false);
- if (r == -ENOSYS) {
- r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
- }
if (r < 0) {
fprintf(stderr, "vhost VQ %d notifier cleanup failed: %d\n", i, -r);
fflush(stderr);
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 0281cbf..cbe279f 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -52,7 +52,6 @@ typedef struct VirtioBusClass {
bool (*has_extra_state)(DeviceState *d);
bool (*query_guest_notifiers)(DeviceState *d);
int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign);
- int (*set_host_notifier)(DeviceState *d, int n, bool assigned);
void (*vmstate_change)(DeviceState *d, bool running);
/*
* transport independent init function.
--
2.6.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers
2016-03-17 10:01 [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers Cornelia Huck
` (5 preceding siblings ...)
2016-03-17 10:01 ` [Qemu-devel] [PATCH RFC 6/6] virtio-bus: remove old set_host_notifier callback Cornelia Huck
@ 2016-03-21 15:02 ` Stefan Hajnoczi
2016-03-23 9:34 ` Fam Zheng
7 siblings, 0 replies; 13+ messages in thread
From: Stefan Hajnoczi @ 2016-03-21 15:02 UTC (permalink / raw)
To: Cornelia Huck; +Cc: pbonzini, famz, qemu-devel, borntraeger, mst
[-- Attachment #1: Type: text/plain, Size: 1654 bytes --]
On Thu, Mar 17, 2016 at 11:01:27AM +0100, Cornelia Huck wrote:
> As discussed in the virtio-blk dataplane start/stop thread, the
> various transports contain a lot of similar/identical boilerplate
> code for setting up ioeventfds. This makes the code hard to follow.
>
> Let's drag all of the common handling into the virtio-bus layer,
> hopefully reducing both the boilerplate code and the number of
> indirections.
>
> This RFC patchset is the result of hacking this up quickly, and I
> was rather surprised that it did not die immediately on my s390
> system (which is all I 'tested'). So be warned :)
>
> Feedback about the approach and the interface would be good.
>
> Cornelia Huck (6):
> virtio-bus: common ioeventfd infrastructure
> virtio-bus: have callers tolerate new host notifier api
> virtio-ccw: convert to ioeventfd callbacks
> virtio-pci: convert to ioeventfd callbacks
> virtio-mmio: convert to ioeventfd callbacks
> virtio-bus: remove old set_host_notifier callback
>
> hw/block/dataplane/virtio-blk.c | 6 +-
> hw/s390x/virtio-ccw.c | 133 ++++++++++++++--------------------------
> hw/scsi/virtio-scsi-dataplane.c | 9 ++-
> hw/virtio/vhost.c | 13 ++--
> hw/virtio/virtio-bus.c | 108 ++++++++++++++++++++++++++++++++
> hw/virtio/virtio-mmio.c | 128 +++++++++++++-------------------------
> hw/virtio/virtio-pci.c | 124 +++++++++++++------------------------
> include/hw/virtio/virtio-bus.h | 15 ++++-
> 8 files changed, 263 insertions(+), 273 deletions(-)
I haven't reviewed it line-by-line but overall it looks good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers
2016-03-17 10:01 [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers Cornelia Huck
` (6 preceding siblings ...)
2016-03-21 15:02 ` [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers Stefan Hajnoczi
@ 2016-03-23 9:34 ` Fam Zheng
7 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2016-03-23 9:34 UTC (permalink / raw)
To: Cornelia Huck, borntraeger; +Cc: pbonzini, qemu-devel, stefanha, mst
On Thu, 03/17 11:01, Cornelia Huck wrote:
> As discussed in the virtio-blk dataplane start/stop thread, the
> various transports contain a lot of similar/identical boilerplate
> code for setting up ioeventfds. This makes the code hard to follow.
>
> Let's drag all of the common handling into the virtio-bus layer,
> hopefully reducing both the boilerplate code and the number of
> indirections.
>
> This RFC patchset is the result of hacking this up quickly, and I
> was rather surprised that it did not die immediately on my s390
> system (which is all I 'tested'). So be warned :)
>
> Feedback about the approach and the interface would be good.
I tested this series with the two assertions (Paolo's "test" reentrancy
detection and my "assert(is_in_iothread())", in virtio-blk request handler),
and it seems to fix the crash . (A few seconds to crash on master vs. several
hours survival).
Fam
>
> Cornelia Huck (6):
> virtio-bus: common ioeventfd infrastructure
> virtio-bus: have callers tolerate new host notifier api
> virtio-ccw: convert to ioeventfd callbacks
> virtio-pci: convert to ioeventfd callbacks
> virtio-mmio: convert to ioeventfd callbacks
> virtio-bus: remove old set_host_notifier callback
>
> hw/block/dataplane/virtio-blk.c | 6 +-
> hw/s390x/virtio-ccw.c | 133 ++++++++++++++--------------------------
> hw/scsi/virtio-scsi-dataplane.c | 9 ++-
> hw/virtio/vhost.c | 13 ++--
> hw/virtio/virtio-bus.c | 108 ++++++++++++++++++++++++++++++++
> hw/virtio/virtio-mmio.c | 128 +++++++++++++-------------------------
> hw/virtio/virtio-pci.c | 124 +++++++++++++------------------------
> include/hw/virtio/virtio-bus.h | 15 ++++-
> 8 files changed, 263 insertions(+), 273 deletions(-)
>
> --
> 2.6.5
>
^ permalink raw reply [flat|nested] 13+ messages in thread