| Message ID | 1515495828-190409-1-git-send-email-junjie.j.chen@intel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Yuanhan Liu |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | success | coding style OK |
| ci/Intel-compilation | success | Compilation OK |
> -----Original Message----- > From: Chen, Junjie J > Sent: Tuesday, January 9, 2018 7:04 PM > To: yliu@fridaylinux.org; maxime.coquelin@redhat.com; Wang, Xiao W > <xiao.w.wang@intel.com>; Bie, Tiwei <tiwei.bie@intel.com>; Yao, Lei A > <lei.a.yao@intel.com> > Cc: dev@dpdk.org; Chen, Junjie J <junjie.j.chen@intel.com> > Subject: [PATCH v7] vhost: support virtqueue interrupt/notification > suppression > > The driver can suppress interrupt when VIRTIO_F_EVENT_IDX feature bit is > negotiated. The driver set vring flags to 0, and MAY use used_event in > available ring to advise device interrupt util reach an index specified > by used_event. The device ignore the lower bit of vring flags, and send > an interrupt when index reach used_event. > > The device can suppress notification in a manner analogous to the ways > driver suppress interrupt. The device manipulates flags or avail_event in > the used ring in the same way the driver manipulates flags or used_event in > available ring. > > Signed-off-by: Junjie Chen <junjie.j.chen@intel.com> Tested-by: Lei Yao <lei.a.yao@intel.com> VM2VM Iperf test has been executed with virtio-net driver after apply this patch. No performance drop. CPU: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz Host OS: Ubuntu 16.04 Guest OS: Ubuntu 16.04 Kernel: 4.4.0 > --- > v7: > Add vhost_need_event definition and update code for next virtio. > > v6: > Use volatile qualifier to access avail event idx. > > v5: > Remove updating avail event index in backend. > > v2-v4: > Use definition of VIRTIO_F_EVENT_IDX from kernel. > > lib/librte_vhost/vhost.c | 2 +- > lib/librte_vhost/vhost.h | 44 > ++++++++++++++++++++++++++++++++++++++----- > lib/librte_vhost/virtio_net.c | 6 +++--- > 3 files changed, 43 insertions(+), 9 deletions(-) > > diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c > index 6f7ef7f..400caa0 100644 > --- a/lib/librte_vhost/vhost.c > +++ b/lib/librte_vhost/vhost.c > @@ -538,7 +538,7 @@ rte_vhost_vring_call(int vid, uint16_t vring_idx) > if (!vq) > return -1; > > - vhost_vring_call(vq); > + vhost_vring_call(dev, vq); > return 0; > } > > diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h > index 1d9366e..ec79991 100644 > --- a/lib/librte_vhost/vhost.h > +++ b/lib/librte_vhost/vhost.h > @@ -103,6 +103,8 @@ struct vhost_virtqueue { > > uint16_t last_avail_idx; > uint16_t last_used_idx; > + /* Last used index we notify to front end. */ > + uint16_t signalled_used; > #define VIRTIO_INVALID_EVENTFD (-1) > #define VIRTIO_UNINITIALIZED_EVENTFD (-2) > > @@ -214,6 +216,7 @@ struct vhost_msg { > (1ULL << VIRTIO_NET_F_GUEST_TSO6) | \ > (1ULL << VIRTIO_NET_F_GUEST_UFO) | \ > (1ULL << VIRTIO_RING_F_INDIRECT_DESC) | > \ > + (1ULL << VIRTIO_RING_F_EVENT_IDX) | \ > (1ULL << VIRTIO_NET_F_MTU) | \ > (1ULL << VIRTIO_F_IOMMU_PLATFORM)) > > @@ -399,16 +402,47 @@ vhost_iova_to_vva(struct virtio_net *dev, struct > vhost_virtqueue *vq, > return __vhost_iova_to_vva(dev, vq, iova, size, perm); > } > > +#define vhost_used_event(vr) \ > + (*(volatile uint16_t*)&(vr)->avail->ring[(vr)->size]) > + > +/* > + * The following is used with VIRTIO_RING_F_EVENT_IDX. > + * Assuming a given event_idx value from the other size, if we have > + * just incremented index from old to new_idx, should we trigger an > + * event? > + */ > +static __rte_always_inline int > +vhost_need_event(uint16_t event_idx, uint16_t new_idx, uint16_t old) > +{ > + return (uint16_t)(new_idx - event_idx - 1) < (uint16_t)(new_idx - > old); > +} > + > static __rte_always_inline void > -vhost_vring_call(struct vhost_virtqueue *vq) > +vhost_vring_call(struct virtio_net *dev, struct vhost_virtqueue *vq) > { > /* Flush used->idx update before we read avail->flags. */ > rte_mb(); > > - /* Kick the guest if necessary. */ > - if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT) > - && (vq->callfd >= 0)) > - eventfd_write(vq->callfd, (eventfd_t)1); > + /* Don't kick guest if we don't reach index specified by guest. */ > + if (dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX)) { > + uint16_t old = vq->signalled_used; > + uint16_t new = vq->last_used_idx; > + > + LOG_DEBUG(VHOST_DATA, "%s: used_event_idx=%d, > old=%d, new=%d\n", > + __func__, > + vhost_used_event(vq), > + old, new); > + if (vhost_need_event(vhost_used_event(vq), new, old) > + && (vq->callfd >= 0)) { > + vq->signalled_used = vq->last_used_idx; > + eventfd_write(vq->callfd, (eventfd_t) 1); > + } > + } else { > + /* Kick the guest if necessary. */ > + if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT) > + && (vq->callfd >= 0)) > + eventfd_write(vq->callfd, (eventfd_t)1); > + } > } > > #endif /* _VHOST_NET_CDEV_H_ */ > diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c > index eb496e1..d0f9221 100644 > --- a/lib/librte_vhost/virtio_net.c > +++ b/lib/librte_vhost/virtio_net.c > @@ -413,7 +413,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t > queue_id, > offsetof(struct vring_used, idx), > sizeof(vq->used->idx)); > > - vhost_vring_call(vq); > + vhost_vring_call(dev, vq); > out: > if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) > vhost_user_iotlb_rd_unlock(vq); > @@ -700,7 +700,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t > queue_id, > > if (likely(vq->shadow_used_idx)) { > flush_shadow_used_ring(dev, vq); > - vhost_vring_call(vq); > + vhost_vring_call(dev, vq); > } > > out: > @@ -1104,7 +1104,7 @@ update_used_idx(struct virtio_net *dev, struct > vhost_virtqueue *vq, > vq->used->idx += count; > vhost_log_used_vring(dev, vq, offsetof(struct vring_used, idx), > sizeof(vq->used->idx)); > - vhost_vring_call(vq); > + vhost_vring_call(dev, vq); > } > > static __rte_always_inline struct zcopy_mbuf * > -- > 2.0.1
On 01/09/2018 12:03 PM, Junjie Chen wrote: > The driver can suppress interrupt when VIRTIO_F_EVENT_IDX feature bit is > negotiated. The driver set vring flags to 0, and MAY use used_event in > available ring to advise device interrupt util reach an index specified > by used_event. The device ignore the lower bit of vring flags, and send > an interrupt when index reach used_event. > > The device can suppress notification in a manner analogous to the ways > driver suppress interrupt. The device manipulates flags or avail_event in > the used ring in the same way the driver manipulates flags or used_event in > available ring. > > Signed-off-by: Junjie Chen<junjie.j.chen@intel.com> > --- > v7: > Add vhost_need_event definition and update code for next virtio. > > v6: > Use volatile qualifier to access avail event idx. > > v5: > Remove updating avail event index in backend. > > v2-v4: > Use definition of VIRTIO_F_EVENT_IDX from kernel. > > lib/librte_vhost/vhost.c | 2 +- > lib/librte_vhost/vhost.h | 44 ++++++++++++++++++++++++++++++++++++++----- > lib/librte_vhost/virtio_net.c | 6 +++--- > 3 files changed, 43 insertions(+), 9 deletions(-) Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> Thanks, Maxime
On Tue, Jan 09, 2018 at 07:34:17AM +0000, Yao, Lei A wrote: > > > > -----Original Message----- > > From: Chen, Junjie J > > Sent: Tuesday, January 9, 2018 7:04 PM > > To: yliu@fridaylinux.org; maxime.coquelin@redhat.com; Wang, Xiao W > > <xiao.w.wang@intel.com>; Bie, Tiwei <tiwei.bie@intel.com>; Yao, Lei A > > <lei.a.yao@intel.com> > > Cc: dev@dpdk.org; Chen, Junjie J <junjie.j.chen@intel.com> > > Subject: [PATCH v7] vhost: support virtqueue interrupt/notification > > suppression > > > > The driver can suppress interrupt when VIRTIO_F_EVENT_IDX feature bit is > > negotiated. The driver set vring flags to 0, and MAY use used_event in > > available ring to advise device interrupt util reach an index specified > > by used_event. The device ignore the lower bit of vring flags, and send > > an interrupt when index reach used_event. > > > > The device can suppress notification in a manner analogous to the ways > > driver suppress interrupt. The device manipulates flags or avail_event in > > the used ring in the same way the driver manipulates flags or used_event in > > available ring. > > > > Signed-off-by: Junjie Chen <junjie.j.chen@intel.com> > Tested-by: Lei Yao <lei.a.yao@intel.com> Applied to dpdk-next-virtio. Thanks. --yliu > VM2VM Iperf test has been executed with virtio-net driver after apply this patch. > No performance drop. > CPU: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz > Host OS: Ubuntu 16.04 > Guest OS: Ubuntu 16.04 > Kernel: 4.4.0
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c index 6f7ef7f..400caa0 100644 --- a/lib/librte_vhost/vhost.c +++ b/lib/librte_vhost/vhost.c @@ -538,7 +538,7 @@ rte_vhost_vring_call(int vid, uint16_t vring_idx) if (!vq) return -1; - vhost_vring_call(vq); + vhost_vring_call(dev, vq); return 0; } diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h index 1d9366e..ec79991 100644 --- a/lib/librte_vhost/vhost.h +++ b/lib/librte_vhost/vhost.h @@ -103,6 +103,8 @@ struct vhost_virtqueue { uint16_t last_avail_idx; uint16_t last_used_idx; + /* Last used index we notify to front end. */ + uint16_t signalled_used; #define VIRTIO_INVALID_EVENTFD (-1) #define VIRTIO_UNINITIALIZED_EVENTFD (-2) @@ -214,6 +216,7 @@ struct vhost_msg { (1ULL << VIRTIO_NET_F_GUEST_TSO6) | \ (1ULL << VIRTIO_NET_F_GUEST_UFO) | \ (1ULL << VIRTIO_RING_F_INDIRECT_DESC) | \ + (1ULL << VIRTIO_RING_F_EVENT_IDX) | \ (1ULL << VIRTIO_NET_F_MTU) | \ (1ULL << VIRTIO_F_IOMMU_PLATFORM)) @@ -399,16 +402,47 @@ vhost_iova_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq, return __vhost_iova_to_vva(dev, vq, iova, size, perm); } +#define vhost_used_event(vr) \ + (*(volatile uint16_t*)&(vr)->avail->ring[(vr)->size]) + +/* + * The following is used with VIRTIO_RING_F_EVENT_IDX. + * Assuming a given event_idx value from the other size, if we have + * just incremented index from old to new_idx, should we trigger an + * event? + */ +static __rte_always_inline int +vhost_need_event(uint16_t event_idx, uint16_t new_idx, uint16_t old) +{ + return (uint16_t)(new_idx - event_idx - 1) < (uint16_t)(new_idx - old); +} + static __rte_always_inline void -vhost_vring_call(struct vhost_virtqueue *vq) +vhost_vring_call(struct virtio_net *dev, struct vhost_virtqueue *vq) { /* Flush used->idx update before we read avail->flags. */ rte_mb(); - /* Kick the guest if necessary. */ - if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT) - && (vq->callfd >= 0)) - eventfd_write(vq->callfd, (eventfd_t)1); + /* Don't kick guest if we don't reach index specified by guest. */ + if (dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX)) { + uint16_t old = vq->signalled_used; + uint16_t new = vq->last_used_idx; + + LOG_DEBUG(VHOST_DATA, "%s: used_event_idx=%d, old=%d, new=%d\n", + __func__, + vhost_used_event(vq), + old, new); + if (vhost_need_event(vhost_used_event(vq), new, old) + && (vq->callfd >= 0)) { + vq->signalled_used = vq->last_used_idx; + eventfd_write(vq->callfd, (eventfd_t) 1); + } + } else { + /* Kick the guest if necessary. */ + if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT) + && (vq->callfd >= 0)) + eventfd_write(vq->callfd, (eventfd_t)1); + } } #endif /* _VHOST_NET_CDEV_H_ */ diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c index eb496e1..d0f9221 100644 --- a/lib/librte_vhost/virtio_net.c +++ b/lib/librte_vhost/virtio_net.c @@ -413,7 +413,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, offsetof(struct vring_used, idx), sizeof(vq->used->idx)); - vhost_vring_call(vq); + vhost_vring_call(dev, vq); out: if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) vhost_user_iotlb_rd_unlock(vq); @@ -700,7 +700,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id, if (likely(vq->shadow_used_idx)) { flush_shadow_used_ring(dev, vq); - vhost_vring_call(vq); + vhost_vring_call(dev, vq); } out: @@ -1104,7 +1104,7 @@ update_used_idx(struct virtio_net *dev, struct vhost_virtqueue *vq, vq->used->idx += count; vhost_log_used_vring(dev, vq, offsetof(struct vring_used, idx), sizeof(vq->used->idx)); - vhost_vring_call(vq); + vhost_vring_call(dev, vq); } static __rte_always_inline struct zcopy_mbuf *
The driver can suppress interrupt when VIRTIO_F_EVENT_IDX feature bit is negotiated. The driver set vring flags to 0, and MAY use used_event in available ring to advise device interrupt util reach an index specified by used_event. The device ignore the lower bit of vring flags, and send an interrupt when index reach used_event. The device can suppress notification in a manner analogous to the ways driver suppress interrupt. The device manipulates flags or avail_event in the used ring in the same way the driver manipulates flags or used_event in available ring. Signed-off-by: Junjie Chen <junjie.j.chen@intel.com> --- v7: Add vhost_need_event definition and update code for next virtio. v6: Use volatile qualifier to access avail event idx. v5: Remove updating avail event index in backend. v2-v4: Use definition of VIRTIO_F_EVENT_IDX from kernel. lib/librte_vhost/vhost.c | 2 +- lib/librte_vhost/vhost.h | 44 ++++++++++++++++++++++++++++++++++++++----- lib/librte_vhost/virtio_net.c | 6 +++--- 3 files changed, 43 insertions(+), 9 deletions(-)