On Thu, Jan 17, 2013 at 08:59:38PM +1030, Rusty Russell wrote: > Getting use of virtio rings correct is tricky, and a recent patch saw > an implementation of in-kernel rings (as separate from userspace). > > This patch attempts to abstract the business of dealing with the > virtio ring layout from the access (userspace or direct); to do this, > we use function pointers, which gcc inlines correctly. > > Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx> > --- > drivers/Makefile | 2 +- > drivers/vhost/Kconfig | 8 + > drivers/vhost/Kconfig.tcm | 1 + > drivers/vhost/Makefile | 2 + > drivers/vhost/vringh.c | 818 ++++++++++++++++++++++++++++++++++++++++++ > drivers/virtio/virtio_ring.c | 33 +- > include/linux/virtio_ring.h | 57 +++ > include/linux/vringh.h | 115 ++++++ > 8 files changed, 1008 insertions(+), 28 deletions(-) > create mode 100644 drivers/vhost/vringh.c > create mode 100644 include/linux/vringh.h > > diff --git a/drivers/Makefile b/drivers/Makefile > index 7863b9f..351a34f 100644 > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -123,7 +123,7 @@ obj-$(CONFIG_PPC_PS3) += ps3/ > obj-$(CONFIG_OF) += of/ > obj-$(CONFIG_SSB) += ssb/ > obj-$(CONFIG_BCMA) += bcma/ > -obj-$(CONFIG_VHOST_NET) += vhost/ > +obj-$(CONFIG_VHOST_RING) += vhost/ > obj-$(CONFIG_VLYNQ) += vlynq/ > obj-$(CONFIG_STAGING) += staging/ > obj-y += platform/ > diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig > index 202bba6..613b074 100644 > --- a/drivers/vhost/Kconfig > +++ b/drivers/vhost/Kconfig > @@ -1,6 +1,7 @@ > config VHOST_NET > tristate "Host kernel accelerator for virtio net (EXPERIMENTAL)" > depends on NET && EVENTFD && (TUN || !TUN) && (MACVTAP || !MACVTAP) && EXPERIMENTAL > + select VHOST_RING > ---help--- > This kernel module can be loaded in host kernel to accelerate > guest networking with virtio_net. Not to be confused with virtio_net > @@ -12,3 +13,10 @@ config VHOST_NET > if STAGING > source "drivers/vhost/Kconfig.tcm" > endif > + > +config VHOST_RING > + tristate > + ---help--- > + This option is selected by any driver which needs to access > + the host side of a virtio ring. > + > diff --git a/drivers/vhost/Kconfig.tcm b/drivers/vhost/Kconfig.tcm > index a9c6f76..0218f77 100644 > --- a/drivers/vhost/Kconfig.tcm > +++ b/drivers/vhost/Kconfig.tcm > @@ -1,6 +1,7 @@ > config TCM_VHOST > tristate "TCM_VHOST fabric module (EXPERIMENTAL)" > depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && m > + select VHOST_RING > default n > ---help--- > Say M here to enable the TCM_VHOST fabric module for use with virtio-scsi guests > diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile > index a27b053..1d37f5e 100644 > --- a/drivers/vhost/Makefile > +++ b/drivers/vhost/Makefile > @@ -2,3 +2,5 @@ obj-$(CONFIG_VHOST_NET) += vhost_net.o > vhost_net-y := vhost.o net.o > > obj-$(CONFIG_TCM_VHOST) += tcm_vhost.o > + > +obj-$(CONFIG_VHOST_RING) += vringh.o > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c > new file mode 100644 > index 0000000..b28670f > --- /dev/null > +++ b/drivers/vhost/vringh.c > @@ -0,0 +1,818 @@ > +/* > + * Helpers for the host side of a virtio ring. > + * > + * Since these may be in userspace, we use (inline) accessors. > + */ > +#include <linux/vringh.h> > +#include <linux/virtio_ring.h> > +#include <linux/kernel.h> > +#include <linux/ratelimit.h> > +#include <linux/uaccess.h> > +#include <linux/slab.h> > + > +static __printf(1,2) __cold void vringh_bad(const char *fmt, ...) > +{ > + static DEFINE_RATELIMIT_STATE(vringh_rs, > + DEFAULT_RATELIMIT_INTERVAL, > + DEFAULT_RATELIMIT_BURST); > + if (__ratelimit(&vringh_rs)) { > + va_list ap; > + va_start(ap, fmt); > + printk(KERN_NOTICE "vringh:"); > + vprintk(fmt, ap); > + va_end(ap); > + } > +} > + > +/* Returns vring->num if empty, -ve on error. */ > +static inline int __vringh_get_head(const struct vringh *vrh, > + int (*getu16)(u16 *val, const u16 *p), I think int (*getu16)(const u16 *p) would be cleaner than returning through a pointer, then callers check that value < 0 for error. > + u16 *last_avail_idx) > +{ > + u16 avail_idx, i, head; > + int err; > + > + err = getu16(&avail_idx, &vrh->vring.avail->idx); > + if (err) { > + vringh_bad("Failed to access avail idx at %p", > + &vrh->vring.avail->idx); > + return err; > + } > + > + if (*last_avail_idx == avail_idx) > + return vrh->vring.num; > + > + /* Only get avail ring entries after they have been exposed by guest. */ > + virtio_rmb(vrh->weak_barriers); > + > + i = *last_avail_idx & (vrh->vring.num - 1); > + > + err = getu16(&head, &vrh->vring.avail->ring[i]); > + if (err) { > + vringh_bad("Failed to read head: idx %d address %p", > + *last_avail_idx, &vrh->vring.avail->ring[i]); > + return err; > + } > + > + if (head >= vrh->vring.num) { > + vringh_bad("Guest says index %u > %u is available", > + head, vrh->vring.num); > + return -EINVAL; > + } > + > + (*last_avail_idx)++; > + return head; > +} > + > +/* Copy some bytes to/from the iovec. Returns num copied. */ > +static inline ssize_t vringh_iov_xfer(struct vringh_iov *iov, > + void *ptr, size_t len, > + int (*xfer)(void __user *addr, void *ptr, > + size_t len)) > +{ > + int err, done = 0; > + > + while (len && iov->i < iov->max) { > + size_t partlen; > + > + partlen = min(iov->iov[iov->i].iov_len, len); > + err = xfer(iov->iov[iov->i].iov_base, ptr, partlen); > + if (err) > + return err; > + done += partlen; > + len -= partlen; > + ptr += partlen; > + iov->iov[iov->i].iov_base += partlen; > + iov->iov[iov->i].iov_len -= partlen; > + > + if (iov->iov[iov->i].iov_len == 0) > + iov->i++; > + } > + return done; > +} > + > +static inline bool check_range(u64 addr, u32 len, > + struct vringh_range *range, > + bool (*getrange)(u64, struct vringh_range *)) > +{ > + if (addr < range->start || addr > range->end_incl) { > + if (!getrange(addr, range)) > + goto bad; > + } > + BUG_ON(addr < range->start || addr > range->end_incl); > + > + /* To end of memory? */ > + if (unlikely(addr + len == 0)) { > + if (range->end_incl == -1ULL) > + return true; > + goto bad; > + } > + > + /* Otherwise, don't wrap. */ > + if (unlikely(addr + len < addr)) > + goto bad; > + if (unlikely(addr + len - 1 > range->end_incl)) > + goto bad; > + return true; > + > +bad: > + vringh_bad("Malformed descriptor address %u@0x%llx", len, addr); > + return false; > +} > + > +/* No reason for this code to be inline. */ > +static int move_to_indirect(int *up_next, u16 *i, void *addr, > + const struct vring_desc *desc, > + struct vring_desc **descs, int *desc_max) > +{ > + /* Indirect tables can't have indirect. */ > + if (*up_next != -1) { > + vringh_bad("Multilevel indirect %u->%u", *up_next, *i); > + return -EINVAL; > + } > + > + if (unlikely(desc->len % sizeof(struct vring_desc))) { > + vringh_bad("Strange indirect len %u", desc->len); > + return -EINVAL; > + } > + > + /* We will check this when we follow it! */ > + if (desc->flags & VRING_DESC_F_NEXT) > + *up_next = desc->next; > + else > + *up_next = -2; > + *descs = addr; > + *desc_max = desc->len / sizeof(struct vring_desc); > + > + /* Now, start at the first indirect. */ > + *i = 0; > + return 0; > +} > + > +static int resize_iovec(struct vringh_iov *iov, gfp_t gfp) > +{ > + struct iovec *new; > + unsigned int new_num = iov->max * 2; > + > + if (new_num < 8) > + new_num = 8; > + > + if (iov->allocated) > + new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp); > + else { > + new = kmalloc(new_num * sizeof(struct iovec), gfp); > + if (new) { > + memcpy(new, iov->iov, iov->i * sizeof(struct iovec)); > + iov->allocated = true; > + } > + } > + if (!new) > + return -ENOMEM; > + iov->iov = new; > + iov->max = new_num; > + return 0; > +} > + > +static u16 __cold return_from_indirect(const struct vringh *vrh, int *up_next, > + struct vring_desc **descs, int *desc_max) > +{ > + u16 i = *up_next; > + > + *up_next = -1; > + *descs = vrh->vring.desc; > + *desc_max = vrh->vring.num; > + return i; > +} > + > +static inline int > +__vringh_iov(struct vringh *vrh, u16 i, > + struct vringh_iov *riov, > + struct vringh_iov *wiov, > + bool (*getrange)(u64 addr, struct vringh_range *r), > + gfp_t gfp, > + int (*getdesc)(struct vring_desc *dst, const struct vring_desc *s)) > +{ > + int err, count = 0, up_next, desc_max; > + struct vring_desc desc, *descs; > + struct vringh_range range = { -1ULL, 0 }; > + > + /* We start traversing vring's descriptor table. */ > + descs = vrh->vring.desc; > + desc_max = vrh->vring.num; > + up_next = -1; > + > + riov->i = wiov->i = 0; > + for (;;) { > + void *addr; > + struct vringh_iov *iov; > + > + err = getdesc(&desc, &descs[i]); > + if (unlikely(err)) > + goto fail; > + > + /* Make sure it's OK, and get offset. */ > + if (!check_range(desc.addr, desc.len, &range, getrange)) { > + err = -EINVAL; > + goto fail; > + } > + addr = (void *)(long)desc.addr + range.offset; Should probably be (void *)(long)(desc.addr + range.offset). Otherwise we risk signed integer overflow. > + > + if (unlikely(desc.flags & VRING_DESC_F_INDIRECT)) { > + err = move_to_indirect(&up_next, &i, addr, &desc, > + &descs, &desc_max); > + if (err) > + goto fail; > + continue; > + } > + > + if (count++ == vrh->vring.num) { > + vringh_bad("Descriptor loop in %p", descs); > + err = -ELOOP; > + goto fail; > + } > + > + if (desc.flags & VRING_DESC_F_WRITE) > + iov = wiov; > + else { > + iov = riov; > + if (unlikely(wiov->i)) { > + vringh_bad("Readable desc %p after writable", > + &descs[i]); > + err = -EINVAL; > + goto fail; > + } > + } > + > + if (unlikely(iov->i == iov->max)) { > + err = resize_iovec(iov, gfp); > + if (err) > + goto fail; > + } > + > + iov->iov[iov->i].iov_base = (__force __user void *)addr; > + iov->iov[iov->i].iov_len = desc.len; The following comment from the previous version still applies: > This looks like it won't do the right thing if desc.len spans multiple > ranges. I don't know if this happens in practice but this is something > vhost supports ATM. in otgher words, we might need to split a single desc to multiple iov entries. > + iov->i++; > + > + if (desc.flags & VRING_DESC_F_NEXT) { > + i = desc.next; > + } else { > + /* Just in case we need to finish traversing above. */ > + if (unlikely(up_next > 0)) > + i = return_from_indirect(vrh, &up_next, > + &descs, &desc_max); > + else > + break; > + } > + > + if (i >= desc_max) { > + vringh_bad("Chained index %u > %u", i, desc_max); > + err = -EINVAL; > + goto fail; > + } > + } > + > + /* Reset for fresh iteration. */ > + riov->max = riov->i; > + wiov->max = wiov->i; > + riov->i = wiov->i = 0; > + return 0; > + > +fail: > + if (riov->allocated) > + kfree(riov->iov); > + if (wiov->allocated) > + kfree(wiov->iov); > + return err; > +} > + > +static inline int __vringh_complete(struct vringh *vrh, u16 idx, u32 len, > + int (*getu16)(u16 *val, const u16 *p), > + int (*putu16)(u16 *p, u16 val), > + int (*putused)(struct vring_used_elem *dst, > + const struct vring_used_elem > + *s), > + bool *notify) > +{ > + struct vring_used_elem used; > + struct vring_used *used_ring; > + int err; > + u16 used_idx, old, used_event; > + > + used.id = idx; > + used.len = len; > + > + err = getu16(&used_idx, &vring_used_event(&vrh->vring)); > + if (err) { > + vringh_bad("Failed to access used event %p", > + &vring_used_event(&vrh->vring)); > + return err; > + } > + > + used_ring = vrh->vring.used; > + used_idx = vrh->last_used_idx; > + > + err = putused(&used_ring->ring[used_idx % vrh->vring.num], > + &used); > + if (err) { > + vringh_bad("Failed to write used entry %u at %p", > + used_idx % vrh->vring.num, > + &used_ring->ring[used_idx % vrh->vring.num]); > + return err; > + } > + > + /* Make sure buffer is written before we update index. */ > + virtio_wmb(vrh->weak_barriers); > + > + old = vrh->last_used_idx; > + vrh->last_used_idx++; > + > + err = putu16(&vrh->vring.used->idx, vrh->last_used_idx); > + if (err) { > + vringh_bad("Failed to update used index at %p", > + &vrh->vring.used->idx); > + return err; One thing vhost does is roll back everything on error, so you can for example have an invalid range of memory and handle writes there in userspace. I think it's worth preserving though this is currently unused. > + } > + > + /* If we already know we need to notify, skip re-checking */ > + if (*notify) > + return 0; > + > + /* Flush out used index update. This is paired with the > + * barrier that the Guest executes when enabling > + * interrupts. */ > + virtio_mb(vrh->weak_barriers); > + > + /* Old-style, without event indices. */ > + if (!vrh->event_indices) { > + u16 flags; > + err = getu16(&flags, &vrh->vring.avail->flags); > + if (err) { > + vringh_bad("Failed to get flags at %p", > + &vrh->vring.avail->flags); > + return err; > + } > + if (!(flags & VRING_AVAIL_F_NO_INTERRUPT)) > + *notify = true; > + return 0; > + } > + > + /* Modern: we know where other side is up to. */ > + err = getu16(&used_event, &vring_used_event(&vrh->vring)); > + if (err) { > + vringh_bad("Failed to get used event idx at %p", > + &vring_used_event(&vrh->vring)); > + return err; > + } > + if (vring_need_event(used_event, vrh->last_used_idx, old)) > + *notify = true; > + return 0; > +} > + > +static inline bool __vringh_notify_enable(struct vringh *vrh, > + int (*getu16)(u16 *val, const u16 *p), > + int (*putu16)(u16 *p, u16 val)) > +{ > + u16 avail; > + > + /* Already enabled? */ > + if (vrh->listening) > + return false; > + > + vrh->listening = true; > + > + if (!vrh->event_indices) { > + /* Old-school; update flags. */ > + if (putu16(&vrh->vring.used->flags, 0) != 0) { > + vringh_bad("Clearing used flags %p", > + &vrh->vring.used->flags); > + return false; > + } > + } else { > + if (putu16(&vring_avail_event(&vrh->vring), > + vrh->last_avail_idx) != 0) { > + vringh_bad("Updating avail event index %p", > + &vring_avail_event(&vrh->vring)); > + return false; > + } > + } > + > + /* They could have slipped one in as we were doing that: make > + * sure it's written, then check again. */ > + virtio_mb(vrh->weak_barriers); > + > + if (getu16(&avail, &vrh->vring.avail->idx) != 0) { Hmm above has implicit != 0 why not here? > + vringh_bad("Failed to check avail idx at %p", > + &vrh->vring.avail->idx); > + return false; > + } > + > + /* This is so unlikely, we just leave notifications enabled. */ > + return avail != vrh->last_avail_idx; > +} > + > +static inline void __vringh_notify_disable(struct vringh *vrh, > + int (*putu16)(u16 *p, u16 val)) > +{ > + /* Already disabled? */ > + if (!vrh->listening) > + return; > + > + vrh->listening = false; > + if (!vrh->event_indices) { > + /* Old-school; update flags. */ > + if (putu16(&vrh->vring.used->flags, VRING_USED_F_NO_NOTIFY)) { > + vringh_bad("Setting used flags %p", > + &vrh->vring.used->flags); > + } > + } > +} > + > +/* Userspace access helpers. */ > +static inline int getu16_user(u16 *val, const u16 *p) > +{ > + return get_user(*val, (__force u16 __user *)p); > +} > + > +static inline int putu16_user(u16 *p, u16 val) > +{ > + return put_user(val, (__force u16 __user *)p); > +} > + > +static inline int getdesc_user(struct vring_desc *dst, > + const struct vring_desc *src) > +{ > + return copy_from_user(dst, (__force void *)src, sizeof(*dst)) == 0 ? 0 : > + -EFAULT; confused about __force above. Shouldn't it cast to __user? I have not tried does this patch pass the checker? > +} > + > +static inline int putused_user(struct vring_used_elem *dst, > + const struct vring_used_elem *s) > +{ > + return copy_to_user((__force void __user *)dst, s, sizeof(*dst)) == 0 > + ? 0 : -EFAULT; > +} > + > +static inline int xfer_from_user(void *src, void *dst, size_t len) > +{ > + return copy_from_user(dst, (__force void *)src, len) == 0 ? 0 : > + -EFAULT; > +} > + > +static inline int xfer_to_user(void *dst, void *src, size_t len) > +{ > + return copy_to_user((__force void *)dst, src, len) == 0 ? 0 : > + -EFAULT; > +} > + > +/** > + * vringh_init_user - initialize a vringh for a userspace vring. > + * @vrh: the vringh to initialize. > + * @features: the feature bits for this ring. > + * @num: the number of elements. > + * @weak_barriers: true if we only need memory barriers, not I/O. > + * @desc: the userpace descriptor pointer. > + * @avail: the userpace avail pointer. > + * @used: the userpace used pointer. > + * > + * Returns an error if num is invalid: you should check pointers > + * yourself! > + */ > +int vringh_init_user(struct vringh *vrh, u32 features, > + unsigned int num, bool weak_barriers, > + struct vring_desc __user *desc, > + struct vring_avail __user *avail, > + struct vring_used __user *used) > +{ > + /* Sane power of 2 please! */ > + if (!num || num > 0xffff || (num & (num - 1))) { > + vringh_bad("Bad ring size %zu", num); > + return -EINVAL; > + } > + > + vrh->event_indices = (features & (1 << VIRTIO_RING_F_EVENT_IDX)); > + vrh->weak_barriers = weak_barriers; > + vrh->listening = false; > + vrh->last_avail_idx = 0; > + vrh->last_used_idx = 0; > + vrh->vring.num = num; > + vrh->vring.desc = (__force struct vring_desc *)desc; > + vrh->vring.avail = (__force struct vring_avail *)avail; > + vrh->vring.used = (__force struct vring_used *)used; I counted 3 separate chunks that do __force casts. Let's try to isolate them and comment why it's safe. > + return 0; > +} > + > +/** > + * vringh_getdesc_user - get next available descriptor from userspace ring. > + * @vrh: the userspace vring. > + * @riov: where to put the readable descriptors. > + * @wiov: where to put the writable descriptors. > + * @getrange: function to call to check ranges. > + * @head: head index we received, for passing to vringh_complete_user(). > + * @gfp: flags for allocating larger riov/wiov. > + * > + * Returns 0 if there was no descriptor, 1 if there was, or -errno. > + * > + * If it returns 1, riov->allocated and wiov->allocated indicate if you > + * have to kfree riov->iov and wiov->iov respectively. > + */ > +int vringh_getdesc_user(struct vringh *vrh, > + struct vringh_iov *riov, > + struct vringh_iov *wiov, > + bool (*getrange)(u64 addr, struct vringh_range *r), > + u16 *head, > + gfp_t gfp) > +{ > + int err; > + > + err = __vringh_get_head(vrh, getu16_user, &vrh->last_avail_idx); > + if (err < 0) > + return err; > + > + /* Empty... */ > + if (err == vrh->vring.num) > + return 0; > + > + *head = err; > + err = __vringh_iov(vrh, *head, riov, wiov, getrange, gfp, getdesc_user); > + if (err) > + return err; > + > + return 1; > +} > + > +/** > + * vringh_iov_pull_user - copy bytes from vring_iov. > + * @riov: the riov as passed to vringh_getdesc_user() (updated as we consume) > + * @dst: the place to copy. > + * @len: the maximum length to copy. > + * > + * Returns the bytes copied <= len or a negative errno. > + */ > +ssize_t vringh_iov_pull_user(struct vringh_iov *riov, void *dst, size_t len) > +{ > + return vringh_iov_xfer(riov, dst, len, xfer_from_user); > +} > + > +/** > + * vringh_iov_push_user - copy bytes into vring_iov. > + * @wiov: the wiov as passed to vringh_getdesc_user() (updated as we consume) > + * @dst: the place to copy. > + * @len: the maximum length to copy. > + * > + * Returns the bytes copied <= len or a negative errno. > + */ > +ssize_t vringh_iov_push_user(struct vringh_iov *wiov, > + const void *src, size_t len) > +{ > + return vringh_iov_xfer(wiov, (void *)src, len, xfer_to_user); > +} > + > +/** > + * vringh_abandon_user - we've decided not to handle the descriptor(s). > + * @vrh: the vring. > + * @num: the number of descriptors to put back (ie. num > + * vringh_get_user() to undo). > + * > + * The next vringh_get_user() will return the old descriptor(s) again. > + */ > +void vringh_abandon_user(struct vringh *vrh, unsigned int num) > +{ > + /* We only update vring_avail_event(vr) when we want to be notified, > + * so we haven't changed that yet. */ > + vrh->last_avail_idx -= num; > +} > + > +/** > + * vringh_complete_user - we've finished with descriptor, publish it. > + * @vrh: the vring. > + * @head: the head as filled in by vringh_getdesc_user. > + * @len: the length of data we have written. > + * @notify: set if we should notify the other side, otherwise left alone. > + */ > +int vringh_complete_user(struct vringh *vrh, u16 head, u32 len, > + bool *notify) > +{ > + return __vringh_complete(vrh, head, len, > + getu16_user, putu16_user, putused_user, > + notify); > +} > + > +/** > + * vringh_notify_enable_user - we want to know if something changes. > + * @vrh: the vring. > + * > + * This always enables notifications, but returns true if there are > + * now more buffers available in the vring. > + */ > +bool vringh_notify_enable_user(struct vringh *vrh) > +{ > + return __vringh_notify_enable(vrh, getu16_user, putu16_user); > +} > + > +/** > + * vringh_notify_disable_user - don't tell us if something changes. > + * @vrh: the vring. > + * > + * This is our normal running state: we disable and then only enable when > + * we're going to sleep. > + */ > +void vringh_notify_disable_user(struct vringh *vrh) > +{ > + __vringh_notify_disable(vrh, putu16_user); > +} > + > +/* Kernelspace access helpers. */ > +static inline int getu16_kern(u16 *val, const u16 *p) > +{ > + *val = *p; > + return 0; > +} > + > +static inline int putu16_kern(u16 *p, u16 val) > +{ > + *p = val; > + return 0; > +} > + > +static inline int getdesc_kern(struct vring_desc *dst, > + const struct vring_desc *src) > +{ > + *dst = *src; > + return 0; > +} > + > +static inline int putused_kern(struct vring_used_elem *dst, > + const struct vring_used_elem *s) > +{ > + *dst = *s; > + return 0; > +} > + > +static inline int xfer_kern(void *src, void *dst, size_t len) > +{ > + memcpy(dst, src, len); > + return 0; > +} > + > +static inline bool noop_getrange(u64 addr, struct vringh_range *r) > +{ > + r->start = 0; > + r->end_incl = -1ULL; > + r->offset = 0; > + return true; > +} > + > +/** > + * vringh_init_kern - initialize a vringh for a kernelspace vring. > + * @vrh: the vringh to initialize. > + * @features: the feature bits for this ring. > + * @num: the number of elements. > + * @weak_barriers: true if we only need memory barriers, not I/O. > + * @desc: the userpace descriptor pointer. > + * @avail: the userpace avail pointer. > + * @used: the userpace used pointer. > + * > + * Returns an error if num is invalid. > + */ > +int vringh_init_kern(struct vringh *vrh, u32 features, > + unsigned int num, bool weak_barriers, > + struct vring_desc *desc, > + struct vring_avail *avail, > + struct vring_used *used) > +{ > + /* Sane power of 2 please! */ > + if (!num || num > 0xffff || (num & (num - 1))) { > + vringh_bad("Bad ring size %zu", num); > + return -EINVAL; > + } > + > + vrh->event_indices = (features & (1 << VIRTIO_RING_F_EVENT_IDX)); > + vrh->weak_barriers = weak_barriers; > + vrh->listening = false; > + vrh->last_avail_idx = 0; > + vrh->last_used_idx = 0; > + vrh->vring.num = num; > + vrh->vring.desc = desc; > + vrh->vring.avail = avail; > + vrh->vring.used = used; > + return 0; > +} > + > +/** > + * vringh_getdesc_kern - get next available descriptor from kernelspace ring. > + * @vrh: the kernelspace vring. > + * @riov: where to put the readable descriptors. > + * @wiov: where to put the writable descriptors. > + * @head: head index we received, for passing to vringh_complete_kern(). > + * @gfp: flags for allocating larger riov/wiov. > + * > + * Returns 0 if there was no descriptor, 1 if there was, or -errno. > + * > + * If it returns 1, riov->allocated and wiov->allocated indicate if you > + * have to kfree riov->iov and wiov->iov respectively. > + */ > +int vringh_getdesc_kern(struct vringh *vrh, > + struct vringh_iov *riov, > + struct vringh_iov *wiov, > + u16 *head, > + gfp_t gfp) > +{ > + int err; > + > + err = __vringh_get_head(vrh, getu16_kern, &vrh->last_avail_idx); > + if (err < 0) > + return err; > + > + /* Empty... */ > + if (err == vrh->vring.num) > + return 0; > + > + *head = err; > + err = __vringh_iov(vrh, *head, riov, wiov, noop_getrange, > + gfp, getdesc_kern); > + if (err) > + return err; > + > + return 1; > +} > + > +/** > + * vringh_iov_pull_kern - copy bytes from vring_iov. > + * @riov: the riov as passed to vringh_getdesc_kern() (updated as we consume) > + * @dst: the place to copy. > + * @len: the maximum length to copy. > + * > + * Returns the bytes copied <= len or a negative errno. > + */ > +ssize_t vringh_iov_pull_kern(struct vringh_iov *riov, void *dst, size_t len) > +{ > + return vringh_iov_xfer(riov, dst, len, xfer_kern); > +} > + > +/** > + * vringh_iov_push_kern - copy bytes into vring_iov. > + * @wiov: the wiov as passed to vringh_getdesc_kern() (updated as we consume) > + * @dst: the place to copy. > + * @len: the maximum length to copy. > + * > + * Returns the bytes copied <= len or a negative errno. > + */ > +ssize_t vringh_iov_push_kern(struct vringh_iov *wiov, > + const void *src, size_t len) > +{ > + return vringh_iov_xfer(wiov, (void *)src, len, xfer_kern); > +} > + > +/** > + * vringh_abandon_kern - we've decided not to handle the descriptor(s). > + * @vrh: the vring. > + * @num: the number of descriptors to put back (ie. num > + * vringh_get_kern() to undo). > + * > + * The next vringh_get_kern() will return the old descriptor(s) again. > + */ > +void vringh_abandon_kern(struct vringh *vrh, unsigned int num) > +{ > + /* We only update vring_avail_event(vr) when we want to be notified, > + * so we haven't changed that yet. */ > + vrh->last_avail_idx -= num; > +} > + > +/** > + * vringh_complete_kern - we've finished with descriptor, publish it. > + * @vrh: the vring. > + * @head: the head as filled in by vringh_getdesc_kern. > + * @len: the length of data we have written. > + * @notify: set if we should notify the other side, otherwise left alone. > + */ > +int vringh_complete_kern(struct vringh *vrh, u16 head, u32 len, > + bool *notify) > +{ > + return __vringh_complete(vrh, head, len, > + getu16_kern, putu16_kern, putused_kern, > + notify); > +} > + > +/** > + * vringh_notify_enable_kern - we want to know if something changes. > + * @vrh: the vring. > + * > + * This always enables notifications, but returns true if there are > + * now more buffers available in the vring. > + */ > +bool vringh_notify_enable_kern(struct vringh *vrh) > +{ > + return __vringh_notify_enable(vrh, getu16_kern, putu16_kern); > +} > + > +/** > + * vringh_notify_disable_kern - don't tell us if something changes. > + * @vrh: the vring. > + * > + * This is our normal running state: we disable and then only enable when > + * we're going to sleep. > + */ > +void vringh_notify_disable_kern(struct vringh *vrh) > +{ > + __vringh_notify_disable(vrh, putu16_kern); > +} > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index ffd7e7d..245177c 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -24,27 +24,6 @@ > #include <linux/module.h> > #include <linux/hrtimer.h> > > -/* virtio guest is communicating with a virtual "device" that actually runs on > - * a host processor. Memory barriers are used to control SMP effects. */ > -#ifdef CONFIG_SMP > -/* Where possible, use SMP barriers which are more lightweight than mandatory > - * barriers, because mandatory barriers control MMIO effects on accesses > - * through relaxed memory I/O windows (which virtio-pci does not use). */ > -#define virtio_mb(vq) \ > - do { if ((vq)->weak_barriers) smp_mb(); else mb(); } while(0) > -#define virtio_rmb(vq) \ > - do { if ((vq)->weak_barriers) smp_rmb(); else rmb(); } while(0) > -#define virtio_wmb(vq) \ > - do { if ((vq)->weak_barriers) smp_wmb(); else wmb(); } while(0) > -#else > -/* We must force memory ordering even if guest is UP since host could be > - * running on another CPU, but SMP barriers are defined to barrier() in that > - * configuration. So fall back to mandatory barriers instead. */ > -#define virtio_mb(vq) mb() > -#define virtio_rmb(vq) rmb() > -#define virtio_wmb(vq) wmb() > -#endif > - > #ifdef DEBUG > /* For development, we want to crash whenever the ring is screwed. */ > #define BAD_RING(_vq, fmt, args...) \ > @@ -276,7 +255,7 @@ add_head: > > /* Descriptors and available array need to be set before we expose the > * new available array entries. */ > - virtio_wmb(vq); > + virtio_wmb(vq->weak_barriers); > vq->vring.avail->idx++; > vq->num_added++; > > @@ -312,7 +291,7 @@ bool virtqueue_kick_prepare(struct virtqueue *_vq) > START_USE(vq); > /* We need to expose available array entries before checking avail > * event. */ > - virtio_mb(vq); > + virtio_mb(vq->weak_barriers); > > old = vq->vring.avail->idx - vq->num_added; > new = vq->vring.avail->idx; > @@ -436,7 +415,7 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) > } > > /* Only get used array entries after they have been exposed by host. */ > - virtio_rmb(vq); > + virtio_rmb(vq->weak_barriers); > > last_used = (vq->last_used_idx & (vq->vring.num - 1)); > i = vq->vring.used->ring[last_used].id; > @@ -460,7 +439,7 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) > * the read in the next get_buf call. */ > if (!(vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT)) { > vring_used_event(&vq->vring) = vq->last_used_idx; > - virtio_mb(vq); > + virtio_mb(vq->weak_barriers); > } > > #ifdef DEBUG > @@ -513,7 +492,7 @@ bool virtqueue_enable_cb(struct virtqueue *_vq) > * entry. Always do both to keep code simple. */ > vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; > vring_used_event(&vq->vring) = vq->last_used_idx; > - virtio_mb(vq); > + virtio_mb(vq->weak_barriers); > if (unlikely(more_used(vq))) { > END_USE(vq); > return false; > @@ -553,7 +532,7 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq) > /* TODO: tune this threshold */ > bufs = (u16)(vq->vring.avail->idx - vq->last_used_idx) * 3 / 4; > vring_used_event(&vq->vring) = vq->last_used_idx + bufs; > - virtio_mb(vq); > + virtio_mb(vq->weak_barriers); > if (unlikely((u16)(vq->vring.used->idx - vq->last_used_idx) > bufs)) { > END_USE(vq); > return false; > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > index 63c6ea1..ca3ad41 100644 > --- a/include/linux/virtio_ring.h > +++ b/include/linux/virtio_ring.h > @@ -4,6 +4,63 @@ > #include <linux/irqreturn.h> > #include <uapi/linux/virtio_ring.h> > > +/* > + * Barriers in virtio are tricky. Non-SMP virtio guests can't assume > + * they're not on an SMP host system, so they need to assume real > + * barriers. Non-SMP virtio hosts could skip the barriers, but does > + * anyone care? > + * > + * For virtio_pci on SMP, we don't need to order with respect to MMIO > + * accesses through relaxed memory I/O windows, so smp_mb() et al are > + * sufficient. > + * > + * For using virtio to talk to real devices (eg. other heterogeneous > + * CPUs) we do need real barriers. In theory, we could be using both > + * kinds of virtio, so it's a runtime decision, and the branch is > + * actually quite cheap. > + */ > + > +#ifdef CONFIG_SMP > +static inline void virtio_mb(bool weak_barriers) > +{ > + if (weak_barriers) > + smp_mb(); > + else > + mb(); > +} > + > +static inline void virtio_rmb(bool weak_barriers) > +{ > + if (weak_barriers) > + smp_rmb(); > + else > + rmb(); > +} > + > +static inline void virtio_wmb(bool weak_barriers) > +{ > + if (weak_barriers) > + smp_wmb(); > + else > + wmb(); > +} > +#else > +static inline void virtio_mb(bool weak_barriers) > +{ > + mb(); > +} > + > +static inline void virtio_rmb(bool weak_barriers) > +{ > + rmb(); > +} > + > +static inline void virtio_wmb(bool weak_barriers) > +{ > + wmb(); > +} > +#endif > + > struct virtio_device; > struct virtqueue; > > diff --git a/include/linux/vringh.h b/include/linux/vringh.h > new file mode 100644 > index 0000000..508b5e5 > --- /dev/null > +++ b/include/linux/vringh.h > @@ -0,0 +1,115 @@ > +/* > + * Linux host-side vring helpers; for when the kernel needs to access > + * someone else's vring. > + * > + * Copyright IBM Corporation, 2013. > + * Parts taken from drivers/vhost/vhost.c Copyright 2009 Red Hat, Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. > + * > + * Written by: Rusty Russell <rusty@xxxxxxxxxxxxxxx> > + */ > +#ifndef _LINUX_VRINGH_H > +#define _LINUX_VRINGH_H > +#include <uapi/linux/virtio_ring.h> > +#include <uapi/linux/uio.h> > +#include <asm/barrier.h> > + > +/* virtio_ring with information needed for host access. */ > +struct vringh { > + /* Guest publishes used event idx (note: we always do). */ > + bool event_indices; > + > + /* Have we told the other end we want to be notified? */ > + bool listening; > + > + /* Can we get away with weak barriers? */ > + bool weak_barriers; > + > + /* Last available index we saw (ie. where we're up to). */ > + u16 last_avail_idx; > + > + /* Last index we used. */ > + u16 last_used_idx; > + > + /* The vring (note: it may contain user pointers!) */ > + struct vring vring; > +}; > + > +/* The memory the vring can access, and what offset to apply. */ > +struct vringh_range { > + u64 start, end_incl; > + u64 offset; > +}; > + > +/* All the information about an iovec. */ > +struct vringh_iov { > + struct iovec *iov; > + unsigned i, max; > + bool allocated; > +}; > + > +/* Helpers for userspace vrings. */ > +int vringh_init_user(struct vringh *vrh, u32 features, > + unsigned int num, bool weak_barriers, > + struct vring_desc __user *desc, > + struct vring_avail __user *avail, > + struct vring_used __user *used); > + > +/* Convert a descriptor into iovecs. */ > +int vringh_getdesc_user(struct vringh *vrh, > + struct vringh_iov *riov, > + struct vringh_iov *wiov, > + bool (*getrange)(u64 addr, struct vringh_range *r), > + u16 *head, > + gfp_t gfp); > + > +/* Copy bytes from readable vsg, consuming it (and incrementing wiov->i). */ > +ssize_t vringh_iov_pull_user(struct vringh_iov *riov, void *dst, size_t len); > + > +/* Copy bytes into writable vsg, consuming it (and incrementing wiov->i). */ > +ssize_t vringh_iov_push_user(struct vringh_iov *wiov, > + const void *src, size_t len); > + > +/* Mark a descriptor as used. Sets notify if you should fire eventfd. */ > +int vringh_complete_user(struct vringh *vrh, u16 head, u32 len, > + bool *notify); > + > +/* Pretend we've never seen descriptor (for easy error handling). */ > +void vringh_abandon_user(struct vringh *vrh, unsigned int num); > + > +/* Helpers for kernelspace vrings. */ > +int vringh_init_kern(struct vringh *vrh, u32 features, > + unsigned int num, bool weak_barriers, > + struct vring_desc *desc, > + struct vring_avail *avail, > + struct vring_used *used); > + > +int vringh_getdesc_kern(struct vringh *vrh, > + struct vringh_iov *riov, > + struct vringh_iov *wiov, > + u16 *head, > + gfp_t gfp); > + > +ssize_t vringh_iov_pull_kern(struct vringh_iov *riov, void *dst, size_t len); > +ssize_t vringh_iov_push_user(struct vringh_iov *wiov, > + const void *src, size_t len); > +void vringh_abandon_kern(struct vringh *vrh, unsigned int num); > +int vringh_complete_kern(struct vringh *vrh, u16 head, u32 len, bool *notify); > + > +bool vringh_notify_enable_kern(struct vringh *vrh); > +void vringh_notify_disable_kern(struct vringh *vrh); > + > +#endif /* _LINUX_VRINGH_H */ > -- > 1.7.10.4 _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization