* [PATCH V2 RFC 1/9] virtio_ring: change host notification API @ 2013-10-28 10:30 Heinz Graalfs 2013-10-28 10:30 ` Heinz Graalfs 2013-10-28 23:13 ` Rusty Russell 0 siblings, 2 replies; 5+ messages in thread From: Heinz Graalfs @ 2013-10-28 10:30 UTC (permalink / raw) To: rusty, virtualization; +Cc: borntraeger, mst Rusty, here is just patch 1 (using bool as return value in notify API). Thanks. Heinz Graalfs (9): virtio_ring: change host notification API virtio_ring: let virtqueue_{kick()/notify()} return a bool virtio_net: verify if virtqueue_kick() succeeded virtio_test: verify if virtqueue_kick() succeeded virtio_ring: add new function virtqueue_is_broken() virtio_blk: verify if queue is broken after virtqueue_get_buf() virtio_console: verify if queue is broken after virtqueue_get_buf() virtio_net: verify if queue is broken after virtqueue_get_buf() virtio_scsi: verify if queue is broken after virtqueue_get_buf() drivers/block/virtio_blk.c | 2 ++ drivers/char/virtio_console.c | 6 ++++-- drivers/lguest/lguest_device.c | 3 ++- drivers/net/virtio_net.c | 12 +++++++----- drivers/remoteproc/remoteproc_virtio.c | 3 ++- drivers/s390/kvm/kvm_virtio.c | 8 ++++++-- drivers/s390/kvm/virtio_ccw.c | 5 ++++- drivers/scsi/virtio_scsi.c | 3 ++- drivers/virtio/virtio_mmio.c | 3 ++- drivers/virtio/virtio_pci.c | 3 ++- drivers/virtio/virtio_ring.c | 32 ++++++++++++++++++++++++++------ include/linux/virtio.h | 6 ++++-- include/linux/virtio_ring.h | 2 +- tools/virtio/virtio_test.c | 6 ++++-- tools/virtio/vringh_test.c | 13 +++++++++---- 15 files changed, 77 insertions(+), 30 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH V2 RFC 1/9] virtio_ring: change host notification API 2013-10-28 10:30 [PATCH V2 RFC 1/9] virtio_ring: change host notification API Heinz Graalfs @ 2013-10-28 10:30 ` Heinz Graalfs 2013-10-28 23:13 ` Rusty Russell 1 sibling, 0 replies; 5+ messages in thread From: Heinz Graalfs @ 2013-10-28 10:30 UTC (permalink / raw) To: rusty, virtualization; +Cc: borntraeger, mst Currently a host kick error is silently ignored and not reflected in the virtqueue of a particular virtio device. Changing the notify API for guest->host notification seems to be one prerequisite in order to be able to handle such errors in the context where the kick is triggered. This patch changes the notify API. The notify function must return a bool return value. It returns false if the host notification failed. Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com> --- drivers/lguest/lguest_device.c | 3 ++- drivers/remoteproc/remoteproc_virtio.c | 3 ++- drivers/s390/kvm/kvm_virtio.c | 8 ++++++-- drivers/s390/kvm/virtio_ccw.c | 5 ++++- drivers/virtio/virtio_mmio.c | 3 ++- drivers/virtio/virtio_pci.c | 3 ++- drivers/virtio/virtio_ring.c | 4 ++-- include/linux/virtio_ring.h | 2 +- tools/virtio/virtio_test.c | 3 ++- tools/virtio/vringh_test.c | 13 +++++++++---- 10 files changed, 32 insertions(+), 15 deletions(-) diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c index b3256ff..d0a1d8a 100644 --- a/drivers/lguest/lguest_device.c +++ b/drivers/lguest/lguest_device.c @@ -229,7 +229,7 @@ struct lguest_vq_info { * make a hypercall. We hand the physical address of the virtqueue so the Host * knows which virtqueue we're talking about. */ -static void lg_notify(struct virtqueue *vq) +static bool lg_notify(struct virtqueue *vq) { /* * We store our virtqueue information in the "priv" pointer of the @@ -238,6 +238,7 @@ static void lg_notify(struct virtqueue *vq) struct lguest_vq_info *lvq = vq->priv; hcall(LHCALL_NOTIFY, lvq->config.pfn << PAGE_SHIFT, 0, 0, 0); + return true; } /* An extern declaration inside a C file is bad form. Don't do it. */ diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index b09c75c..a34b506 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -30,7 +30,7 @@ #include "remoteproc_internal.h" /* kick the remote processor, and let it know which virtqueue to poke at */ -static void rproc_virtio_notify(struct virtqueue *vq) +static bool rproc_virtio_notify(struct virtqueue *vq) { struct rproc_vring *rvring = vq->priv; struct rproc *rproc = rvring->rvdev->rproc; @@ -39,6 +39,7 @@ static void rproc_virtio_notify(struct virtqueue *vq) dev_dbg(&rproc->dev, "kicking vq index: %d\n", notifyid); rproc->ops->kick(rproc, notifyid); + return true; } /** diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c index 2ea6165..0b51fa7 100644 --- a/drivers/s390/kvm/kvm_virtio.c +++ b/drivers/s390/kvm/kvm_virtio.c @@ -166,11 +166,15 @@ static void kvm_reset(struct virtio_device *vdev) * make a hypercall. We hand the address of the virtqueue so the Host * knows which virtqueue we're talking about. */ -static void kvm_notify(struct virtqueue *vq) +static bool kvm_notify(struct virtqueue *vq) { + long rc; struct kvm_vqconfig *config = vq->priv; - kvm_hypercall1(KVM_S390_VIRTIO_NOTIFY, config->address); + rc = kvm_hypercall1(KVM_S390_VIRTIO_NOTIFY, config->address); + if (rc < 0) + return false; + return true; } /* diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c index 6a410d4..be2ed27 100644 --- a/drivers/s390/kvm/virtio_ccw.c +++ b/drivers/s390/kvm/virtio_ccw.c @@ -375,7 +375,7 @@ static inline long do_kvm_notify(struct subchannel_id schid, return __rc; } -static void virtio_ccw_kvm_notify(struct virtqueue *vq) +static bool virtio_ccw_kvm_notify(struct virtqueue *vq) { struct virtio_ccw_vq_info *info = vq->priv; struct virtio_ccw_device *vcdev; @@ -384,6 +384,9 @@ static void virtio_ccw_kvm_notify(struct virtqueue *vq) vcdev = to_vc_device(info->vq->vdev); ccw_device_get_schid(vcdev->cdev, &schid); info->cookie = do_kvm_notify(schid, vq->index, info->cookie); + if (info->cookie < 0) + return false; + return true; } static int virtio_ccw_read_vq_conf(struct virtio_ccw_device *vcdev, diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index 1ba0d68..e9fdeb8 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -219,13 +219,14 @@ static void vm_reset(struct virtio_device *vdev) /* Transport interface */ /* the notify function used when creating a virt queue */ -static void vm_notify(struct virtqueue *vq) +static bool vm_notify(struct virtqueue *vq) { struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vq->vdev); /* We write the queue's selector into the notification register to * signal the other end */ writel(vq->index, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY); + return true; } /* Notify all virtqueues on an interrupt. */ diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index a7ce730..f1e5c17 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -197,13 +197,14 @@ static void vp_reset(struct virtio_device *vdev) } /* the notify function used when creating a virt queue */ -static void vp_notify(struct virtqueue *vq) +static bool vp_notify(struct virtqueue *vq) { struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev); /* we write the queue's selector into the notification register to * signal the other end */ iowrite16(vq->index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NOTIFY); + return true; } /* Handle a configuration change: Tell driver if it wants to know. */ diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 5217baf..6d20cf3 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -81,7 +81,7 @@ struct vring_virtqueue u16 last_used_idx; /* How to notify other side. FIXME: commonalize hcalls! */ - void (*notify)(struct virtqueue *vq); + bool (*notify)(struct virtqueue *vq); #ifdef DEBUG /* They're supposed to lock for us. */ @@ -741,7 +741,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int index, struct virtio_device *vdev, bool weak_barriers, void *pages, - void (*notify)(struct virtqueue *), + bool (*notify)(struct virtqueue *), void (*callback)(struct virtqueue *), const char *name) { diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h index ca3ad41..be4e5b7 100644 --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -70,7 +70,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int index, struct virtio_device *vdev, bool weak_barriers, void *pages, - void (*notify)(struct virtqueue *vq), + bool (*notify)(struct virtqueue *vq), void (*callback)(struct virtqueue *vq), const char *name); void vring_del_virtqueue(struct virtqueue *vq); diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c index da7a195..059cb72 100644 --- a/tools/virtio/virtio_test.c +++ b/tools/virtio/virtio_test.c @@ -41,13 +41,14 @@ struct vdev_info { struct vhost_memory *mem; }; -void vq_notify(struct virtqueue *vq) +bool vq_notify(struct virtqueue *vq) { struct vq_info *info = vq->priv; unsigned long long v = 1; int r; r = write(info->kick, &v, sizeof v); assert(r == sizeof v); + return true; } void vq_callback(struct virtqueue *vq) diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c index d053ea4..14a4f4c 100644 --- a/tools/virtio/vringh_test.c +++ b/tools/virtio/vringh_test.c @@ -22,7 +22,7 @@ static u64 user_addr_offset; #define RINGSIZE 256 #define ALIGN 4096 -static void never_notify_host(struct virtqueue *vq) +static bool never_notify_host(struct virtqueue *vq) { abort(); } @@ -65,17 +65,22 @@ struct guest_virtio_device { unsigned long notifies; }; -static void parallel_notify_host(struct virtqueue *vq) +static bool parallel_notify_host(struct virtqueue *vq) { + int rc; struct guest_virtio_device *gvdev; gvdev = container_of(vq->vdev, struct guest_virtio_device, vdev); - write(gvdev->to_host_fd, "", 1); + rc = write(gvdev->to_host_fd, "", 1); + if (rc < 0) + return false; gvdev->notifies++; + return true; } -static void no_notify_host(struct virtqueue *vq) +static bool no_notify_host(struct virtqueue *vq) { + return true; } #define NUM_XFERS (10000000) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V2 RFC 1/9] virtio_ring: change host notification API 2013-10-28 10:30 [PATCH V2 RFC 1/9] virtio_ring: change host notification API Heinz Graalfs 2013-10-28 10:30 ` Heinz Graalfs @ 2013-10-28 23:13 ` Rusty Russell 1 sibling, 0 replies; 5+ messages in thread From: Rusty Russell @ 2013-10-28 23:13 UTC (permalink / raw) To: Heinz Graalfs, virtualization; +Cc: borntraeger, mst Heinz Graalfs <graalfs@linux.vnet.ibm.com> writes: > Rusty, > > here is just patch 1 (using bool as return value in notify API). > > Thanks. > > Heinz Graalfs (9): > virtio_ring: change host notification API > virtio_ring: let virtqueue_{kick()/notify()} return a bool > virtio_net: verify if virtqueue_kick() succeeded > virtio_test: verify if virtqueue_kick() succeeded > virtio_ring: add new function virtqueue_is_broken() > virtio_blk: verify if queue is broken after virtqueue_get_buf() > virtio_console: verify if queue is broken after virtqueue_get_buf() > virtio_net: verify if queue is broken after virtqueue_get_buf() > virtio_scsi: verify if queue is broken after virtqueue_get_buf() Thanks, I've applied all of these. Cheers, Rusty. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH V2 RFC 0/9] virtio: fix hang(loop) after hot-unplug vlan @ 2013-10-24 15:23 Heinz Graalfs 2013-10-24 15:23 ` [PATCH V2 RFC 1/9] virtio_ring: change host notification API Heinz Graalfs 0 siblings, 1 reply; 5+ messages in thread From: Heinz Graalfs @ 2013-10-24 15:23 UTC (permalink / raw) To: rusty, mst, virtualization; +Cc: borntraeger Hi, this patch-set solves a hang situation when a vlan network device is hot-unplugged from a KVM guest. On System z there exists no handshake mechanism between host and guest when a device is hot-unplugged. The device is removed and no further I/O is possible. The guest is notified about the hard removal with a CRW machine check. As per architecture, the host must repond to any I/O operation for the removed device with an error condition as if the device had never been there. During machine check handling in the guest, virtio exit functions try to perform cleanup operations by triggering final I/O, including appropriate host kicks. These operations fail, or do not complete, and lead to several kinds of hang situations. In particular, virtio-ccw guest->host notification on an unplugged device will receive an error; this is, however, not reflected back to the affected virtqueues. Here are the details of the error. A hang (loop) occurs when a machine check is handled on System z due to a vlan device removal. A loop spinning for a response for final IO in virtnet_send_command() will never complete successfully because of a previous unsuccessfull host kick operation (virtqueue_kick()). Patch [1] changes the guest->host notification API. A potential error returned by the host during notify() should not be ignored, but used in order to reflect the error back to the affected virtqueue. Patch [2] changes virtqueue_kick() and virtqueue_notify() to return a bool depending on the result of the host notification operation. If the host kick failed the current virtqueue is now flagged as 'broken'. Patches [3,4] add code to verify host kicks by testing the return value of virtqueue_kick() in order to avoid potential loops. Patch [5] adds a new function virtqueue_is_broken(). This function should be used to verify the state of a virtqueue when a previous virtqueue_get_buf() returned a NULL pointer. Patch [6,7,8,9] add virtqueue_is_broken() calls to handle potential errors when a virtqueue_bet_buf() doesn't deliver any more buffers. Heinz Graalfs (9): virtio_ring: change host notification API virtio_ring: let virtqueue_{kick()/notify()} return a bool virtio_net: verify if virtqueue_kick() succeeded virtio_test: verify if virtqueue_kick() succeeded virtio_ring: add new function virtqueue_is_broken() virtio_blk: verify if queue is broken after virtqueue_get_buf() virtio_console: verify if queue is broken after virtqueue_get_buf() virtio_net: verify if queue is broken after virtqueue_get_buf() virtio_scsi: verify if queue is broken after virtqueue_get_buf() drivers/block/virtio_blk.c | 2 ++ drivers/char/virtio_console.c | 6 ++++-- drivers/lguest/lguest_device.c | 3 ++- drivers/net/virtio_net.c | 12 +++++++----- drivers/remoteproc/remoteproc_virtio.c | 3 ++- drivers/s390/kvm/kvm_virtio.c | 8 ++++++-- drivers/s390/kvm/virtio_ccw.c | 5 ++++- drivers/scsi/virtio_scsi.c | 3 ++- drivers/virtio/virtio_mmio.c | 3 ++- drivers/virtio/virtio_pci.c | 3 ++- drivers/virtio/virtio_ring.c | 32 ++++++++++++++++++++++++++------ include/linux/virtio.h | 6 ++++-- include/linux/virtio_ring.h | 2 +- tools/virtio/virtio_test.c | 6 ++++-- tools/virtio/vringh_test.c | 13 +++++++++---- 15 files changed, 77 insertions(+), 30 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH V2 RFC 1/9] virtio_ring: change host notification API 2013-10-24 15:23 [PATCH V2 RFC 0/9] virtio: fix hang(loop) after hot-unplug vlan Heinz Graalfs @ 2013-10-24 15:23 ` Heinz Graalfs 2013-10-27 23:27 ` Rusty Russell 0 siblings, 1 reply; 5+ messages in thread From: Heinz Graalfs @ 2013-10-24 15:23 UTC (permalink / raw) To: rusty, mst, virtualization; +Cc: borntraeger Currently a host kick error is silently ignored and not reflected in the virtqueue of a particular virtio device. Changing the notify API for guest->host notification seems to be one prerequisite in order to be able to handle such errors in the context where the kick is triggered. This patch changes the notify API. The notify function must return a negative int return value in case the host notification failed. Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com> --- drivers/lguest/lguest_device.c | 3 ++- drivers/remoteproc/remoteproc_virtio.c | 3 ++- drivers/s390/kvm/kvm_virtio.c | 8 ++++++-- drivers/s390/kvm/virtio_ccw.c | 5 ++++- drivers/virtio/virtio_mmio.c | 3 ++- drivers/virtio/virtio_pci.c | 3 ++- drivers/virtio/virtio_ring.c | 4 ++-- include/linux/virtio_ring.h | 2 +- tools/virtio/virtio_test.c | 3 ++- tools/virtio/vringh_test.c | 13 +++++++++---- 10 files changed, 32 insertions(+), 15 deletions(-) diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c index b3256ff..1275f18 100644 --- a/drivers/lguest/lguest_device.c +++ b/drivers/lguest/lguest_device.c @@ -229,7 +229,7 @@ struct lguest_vq_info { * make a hypercall. We hand the physical address of the virtqueue so the Host * knows which virtqueue we're talking about. */ -static void lg_notify(struct virtqueue *vq) +static int lg_notify(struct virtqueue *vq) { /* * We store our virtqueue information in the "priv" pointer of the @@ -238,6 +238,7 @@ static void lg_notify(struct virtqueue *vq) struct lguest_vq_info *lvq = vq->priv; hcall(LHCALL_NOTIFY, lvq->config.pfn << PAGE_SHIFT, 0, 0, 0); + return 0; } /* An extern declaration inside a C file is bad form. Don't do it. */ diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index b09c75c..39723ee 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -30,7 +30,7 @@ #include "remoteproc_internal.h" /* kick the remote processor, and let it know which virtqueue to poke at */ -static void rproc_virtio_notify(struct virtqueue *vq) +static int rproc_virtio_notify(struct virtqueue *vq) { struct rproc_vring *rvring = vq->priv; struct rproc *rproc = rvring->rvdev->rproc; @@ -39,6 +39,7 @@ static void rproc_virtio_notify(struct virtqueue *vq) dev_dbg(&rproc->dev, "kicking vq index: %d\n", notifyid); rproc->ops->kick(rproc, notifyid); + return 0; } /** diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c index 2ea6165..4097a46 100644 --- a/drivers/s390/kvm/kvm_virtio.c +++ b/drivers/s390/kvm/kvm_virtio.c @@ -166,11 +166,15 @@ static void kvm_reset(struct virtio_device *vdev) * make a hypercall. We hand the address of the virtqueue so the Host * knows which virtqueue we're talking about. */ -static void kvm_notify(struct virtqueue *vq) +static int kvm_notify(struct virtqueue *vq) { + long rc; struct kvm_vqconfig *config = vq->priv; - kvm_hypercall1(KVM_S390_VIRTIO_NOTIFY, config->address); + rc = kvm_hypercall1(KVM_S390_VIRTIO_NOTIFY, config->address); + if (rc < 0) + return rc; + return 0; } /* diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c index 6a410d4..c640ecd 100644 --- a/drivers/s390/kvm/virtio_ccw.c +++ b/drivers/s390/kvm/virtio_ccw.c @@ -375,7 +375,7 @@ static inline long do_kvm_notify(struct subchannel_id schid, return __rc; } -static void virtio_ccw_kvm_notify(struct virtqueue *vq) +static int virtio_ccw_kvm_notify(struct virtqueue *vq) { struct virtio_ccw_vq_info *info = vq->priv; struct virtio_ccw_device *vcdev; @@ -384,6 +384,9 @@ static void virtio_ccw_kvm_notify(struct virtqueue *vq) vcdev = to_vc_device(info->vq->vdev); ccw_device_get_schid(vcdev->cdev, &schid); info->cookie = do_kvm_notify(schid, vq->index, info->cookie); + if (info->cookie < 0) + return info->cookie; + return 0; } static int virtio_ccw_read_vq_conf(struct virtio_ccw_device *vcdev, diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index 1ba0d68..e213991 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -219,13 +219,14 @@ static void vm_reset(struct virtio_device *vdev) /* Transport interface */ /* the notify function used when creating a virt queue */ -static void vm_notify(struct virtqueue *vq) +static int vm_notify(struct virtqueue *vq) { struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vq->vdev); /* We write the queue's selector into the notification register to * signal the other end */ writel(vq->index, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY); + return 0; } /* Notify all virtqueues on an interrupt. */ diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index a7ce730..7988137 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -197,13 +197,14 @@ static void vp_reset(struct virtio_device *vdev) } /* the notify function used when creating a virt queue */ -static void vp_notify(struct virtqueue *vq) +static int vp_notify(struct virtqueue *vq) { struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev); /* we write the queue's selector into the notification register to * signal the other end */ iowrite16(vq->index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NOTIFY); + return 0; } /* Handle a configuration change: Tell driver if it wants to know. */ diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 5217baf..aa40f6c 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -81,7 +81,7 @@ struct vring_virtqueue u16 last_used_idx; /* How to notify other side. FIXME: commonalize hcalls! */ - void (*notify)(struct virtqueue *vq); + int (*notify)(struct virtqueue *vq); #ifdef DEBUG /* They're supposed to lock for us. */ @@ -741,7 +741,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int index, struct virtio_device *vdev, bool weak_barriers, void *pages, - void (*notify)(struct virtqueue *), + int (*notify)(struct virtqueue *), void (*callback)(struct virtqueue *), const char *name) { diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h index ca3ad41..74fa580 100644 --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -70,7 +70,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int index, struct virtio_device *vdev, bool weak_barriers, void *pages, - void (*notify)(struct virtqueue *vq), + int (*notify)(struct virtqueue *vq), void (*callback)(struct virtqueue *vq), const char *name); void vring_del_virtqueue(struct virtqueue *vq); diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c index da7a195..8e3e432 100644 --- a/tools/virtio/virtio_test.c +++ b/tools/virtio/virtio_test.c @@ -41,13 +41,14 @@ struct vdev_info { struct vhost_memory *mem; }; -void vq_notify(struct virtqueue *vq) +int vq_notify(struct virtqueue *vq) { struct vq_info *info = vq->priv; unsigned long long v = 1; int r; r = write(info->kick, &v, sizeof v); assert(r == sizeof v); + return 0; } void vq_callback(struct virtqueue *vq) diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c index d053ea4..965ccfe 100644 --- a/tools/virtio/vringh_test.c +++ b/tools/virtio/vringh_test.c @@ -22,7 +22,7 @@ static u64 user_addr_offset; #define RINGSIZE 256 #define ALIGN 4096 -static void never_notify_host(struct virtqueue *vq) +static int never_notify_host(struct virtqueue *vq) { abort(); } @@ -65,17 +65,22 @@ struct guest_virtio_device { unsigned long notifies; }; -static void parallel_notify_host(struct virtqueue *vq) +static int parallel_notify_host(struct virtqueue *vq) { + int rc; struct guest_virtio_device *gvdev; gvdev = container_of(vq->vdev, struct guest_virtio_device, vdev); - write(gvdev->to_host_fd, "", 1); + rc = write(gvdev->to_host_fd, "", 1); + if (rc < 0) + return rc; gvdev->notifies++; + return 0; } -static void no_notify_host(struct virtqueue *vq) +static int no_notify_host(struct virtqueue *vq) { + return 0; } #define NUM_XFERS (10000000) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V2 RFC 1/9] virtio_ring: change host notification API 2013-10-24 15:23 ` [PATCH V2 RFC 1/9] virtio_ring: change host notification API Heinz Graalfs @ 2013-10-27 23:27 ` Rusty Russell 0 siblings, 0 replies; 5+ messages in thread From: Rusty Russell @ 2013-10-27 23:27 UTC (permalink / raw) To: Heinz Graalfs, mst, virtualization; +Cc: borntraeger Heinz Graalfs <graalfs@linux.vnet.ibm.com> writes: > Currently a host kick error is silently ignored and not reflected in > the virtqueue of a particular virtio device. > > Changing the notify API for guest->host notification seems to be one > prerequisite in order to be able to handle such errors in the context > where the kick is triggered. > > This patch changes the notify API. The notify function must return a > negative int return value in case the host notification failed. I think we need a bool here: > - kvm_hypercall1(KVM_S390_VIRTIO_NOTIFY, config->address); > + rc = kvm_hypercall1(KVM_S390_VIRTIO_NOTIFY, config->address); > + if (rc < 0) > + return rc; > + return 0; > } I have no idea what this hypercall returns on failure... > -static void virtio_ccw_kvm_notify(struct virtqueue *vq) > +static int virtio_ccw_kvm_notify(struct virtqueue *vq) > { > struct virtio_ccw_vq_info *info = vq->priv; > struct virtio_ccw_device *vcdev; > @@ -384,6 +384,9 @@ static void virtio_ccw_kvm_notify(struct virtqueue *vq) > vcdev = to_vc_device(info->vq->vdev); > ccw_device_get_schid(vcdev->cdev, &schid); > info->cookie = do_kvm_notify(schid, vq->index, info->cookie); > + if (info->cookie < 0) > + return info->cookie; > + return 0; Nor this one. Since the caller can't really use the return value, I think a bool is correct. Cheers, Rusty. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-10-28 23:13 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-28 10:30 [PATCH V2 RFC 1/9] virtio_ring: change host notification API Heinz Graalfs 2013-10-28 10:30 ` Heinz Graalfs 2013-10-28 23:13 ` Rusty Russell -- strict thread matches above, loose matches on Subject: below -- 2013-10-24 15:23 [PATCH V2 RFC 0/9] virtio: fix hang(loop) after hot-unplug vlan Heinz Graalfs 2013-10-24 15:23 ` [PATCH V2 RFC 1/9] virtio_ring: change host notification API Heinz Graalfs 2013-10-27 23:27 ` Rusty Russell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).