Cornelia Huck
2015-Apr-24 07:19 UTC
[PATCH v5 7/8] vhost: cross-endian support for legacy devices
On Thu, 23 Apr 2015 17:29:42 +0200 Greg Kurz <gkurz at linux.vnet.ibm.com> wrote:> This patch brings cross-endian support to vhost when used to implement > legacy virtio devices. Since it is a relatively rare situation, the > feature availability is controlled by a kernel config option (not set > by default). > > The vq->is_le boolean field is added to cache the endianness to be > used for ring accesses. It defaults to native endian, as expected > by legacy virtio devices. When the ring gets active, we force little > endian if the device is modern. When the ring is deactivated, we > revert to the native endian default. > > If cross-endian was compiled in, a vq->user_be boolean field is added > so that userspace may request a specific endianness. This field is > used to override the default when activating the ring of a legacy > device. It has no effect on modern devices. > > Signed-off-by: Greg Kurz <gkurz at linux.vnet.ibm.com> > --- > > Changes since v4: > - rewrote patch title to mention cross-endian > - renamed config to VHOST_CROSS_ENDIAN_LEGACY > - rewrote config description and help > - moved ifdefery to top of vhost.c > - added a detailed comment about the lifecycle of vq->user_be in > vhost_init_is_le() > - renamed ioctls to VHOST_[GS]ET_VRING_ENDIAN > - added LE/BE defines to the ioctl API > - rewrote ioctl sanity check with the LE/BE defines > - updated comment in <uapi/linux/vhost.h> to mention that the availibility > of both SET and GET ioctls depends on the kernel config > > drivers/vhost/Kconfig | 15 ++++++++ > drivers/vhost/vhost.c | 86 +++++++++++++++++++++++++++++++++++++++++++- > drivers/vhost/vhost.h | 10 +++++ > include/uapi/linux/vhost.h | 12 ++++++ > 4 files changed, 121 insertions(+), 2 deletions(-) > > diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig > index 017a1e8..74d7380 100644 > --- a/drivers/vhost/Kconfig > +++ b/drivers/vhost/Kconfig > @@ -32,3 +32,18 @@ config VHOST > ---help--- > This option is selected by any driver which needs to access > the core of vhost. > + > +config VHOST_CROSS_ENDIAN_LEGACY > + bool "Cross-endian support for vhost" > + default n > + ---help--- > + This option allows vhost to support guests with a different byte > + ordering from host."...while using legacy virtio." Might help to explain the "LEGACY" in the config option ;)> + > + Userspace programs can control the feature using the > + VHOST_SET_VRING_ENDIAN and VHOST_GET_VRING_ENDIAN ioctls. > + > + This is only useful on a few platforms (ppc64 and arm64). Since it > + adds some overhead, it is disabled default.s/default/by default/> + > + If unsure, say "N". > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 2ee2826..8c4390d 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -36,6 +36,78 @@ enum { > #define vhost_used_event(vq) ((__virtio16 __user *)&vq->avail->ring[vq->num]) > #define vhost_avail_event(vq) ((__virtio16 __user *)&vq->used->ring[vq->num]) > > +#ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY > +static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq) > +{ > + vq->user_be = !virtio_legacy_is_little_endian(); > +} > + > +static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp) > +{ > + struct vhost_vring_state s; > + > + if (vq->private_data) > + return -EBUSY; > + > + if (copy_from_user(&s, argp, sizeof(s))) > + return -EFAULT; > + > + if (s.num != VHOST_VRING_LITTLE_ENDIAN && > + s.num != VHOST_VRING_BIG_ENDIAN) > + return -EINVAL; > + > + vq->user_be = s.num; > + > + return 0; > +} > + > +static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx, > + int __user *argp) > +{ > + struct vhost_vring_state s = { > + .index = idx, > + .num = vq->user_be > + }; > + > + if (copy_to_user(argp, &s, sizeof(s))) > + return -EFAULT; > + > + return 0; > +} > + > +static void vhost_init_is_le(struct vhost_virtqueue *vq) > +{ > + /* Note for legacy virtio: user_be is initialized at reset time > + * according to the host endianness. If userspace does not set an > + * explicit endianness, the default behavior is native endian, as > + * expected by legacy virtio. > + */ > + vq->is_le = vhost_has_feature(vq, VIRTIO_F_VERSION_1) || !vq->user_be; > +} > +#else > +static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq) > +{ > + ;Just leave the function body empty?> +} > + > +static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp) > +{ > + return -ENOIOCTLCMD; > +} > + > +static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx, > + int __user *argp) > +{ > + return -ENOIOCTLCMD; > +} > + > +static void vhost_init_is_le(struct vhost_virtqueue *vq) > +{ > + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) > + vq->is_le = true; > +} > +#endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */ > + > static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh, > poll_table *pt) > {(...)> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h > index bb6a5b4..b980b53 100644 > --- a/include/uapi/linux/vhost.h > +++ b/include/uapi/linux/vhost.h > @@ -103,6 +103,18 @@ struct vhost_memory { > /* Get accessor: reads index, writes value in num */ > #define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x12, struct vhost_vring_state) > > +/* Set the vring byte order in num. Valid values are VHOST_VRING_LITTLE_ENDIAN > + * or VHOST_VRING_BIG_ENDIAN (other values return EINVAL).-EINVAL? Should you also mention when you return -EBUSY?> + * This is a legacy only API that is simply ignored when VIRTIO_F_VERSION_1 is > + * set. > + * Not all kernel configurations support this ioctl, but all configurations that > + * support SET also support GET. > + */ > +#define VHOST_VRING_LITTLE_ENDIAN 0 > +#define VHOST_VRING_BIG_ENDIAN 1 > +#define VHOST_SET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x13, struct vhost_vring_state) > +#define VHOST_GET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x14, struct vhost_vring_state) > + > /* The following ioctls use eventfd file descriptors to signal and poll > * for events. */Apart from style nitpicks, looks good.
Greg Kurz
2015-Apr-24 08:06 UTC
[PATCH v5 7/8] vhost: cross-endian support for legacy devices
On Fri, 24 Apr 2015 09:19:26 +0200 Cornelia Huck <cornelia.huck at de.ibm.com> wrote:> On Thu, 23 Apr 2015 17:29:42 +0200 > Greg Kurz <gkurz at linux.vnet.ibm.com> wrote: > > > This patch brings cross-endian support to vhost when used to implement > > legacy virtio devices. Since it is a relatively rare situation, the > > feature availability is controlled by a kernel config option (not set > > by default). > > > > The vq->is_le boolean field is added to cache the endianness to be > > used for ring accesses. It defaults to native endian, as expected > > by legacy virtio devices. When the ring gets active, we force little > > endian if the device is modern. When the ring is deactivated, we > > revert to the native endian default. > > > > If cross-endian was compiled in, a vq->user_be boolean field is added > > so that userspace may request a specific endianness. This field is > > used to override the default when activating the ring of a legacy > > device. It has no effect on modern devices. > > > > Signed-off-by: Greg Kurz <gkurz at linux.vnet.ibm.com> > > --- > > > > Changes since v4: > > - rewrote patch title to mention cross-endian > > - renamed config to VHOST_CROSS_ENDIAN_LEGACY > > - rewrote config description and help > > - moved ifdefery to top of vhost.c > > - added a detailed comment about the lifecycle of vq->user_be in > > vhost_init_is_le() > > - renamed ioctls to VHOST_[GS]ET_VRING_ENDIAN > > - added LE/BE defines to the ioctl API > > - rewrote ioctl sanity check with the LE/BE defines > > - updated comment in <uapi/linux/vhost.h> to mention that the availibility > > of both SET and GET ioctls depends on the kernel config > > > > drivers/vhost/Kconfig | 15 ++++++++ > > drivers/vhost/vhost.c | 86 +++++++++++++++++++++++++++++++++++++++++++- > > drivers/vhost/vhost.h | 10 +++++ > > include/uapi/linux/vhost.h | 12 ++++++ > > 4 files changed, 121 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig > > index 017a1e8..74d7380 100644 > > --- a/drivers/vhost/Kconfig > > +++ b/drivers/vhost/Kconfig > > @@ -32,3 +32,18 @@ config VHOST > > ---help--- > > This option is selected by any driver which needs to access > > the core of vhost. > > + > > +config VHOST_CROSS_ENDIAN_LEGACY > > + bool "Cross-endian support for vhost" > > + default n > > + ---help--- > > + This option allows vhost to support guests with a different byte > > + ordering from host. > > "...while using legacy virtio." > > Might help to explain the "LEGACY" in the config option ;) >Makes sense indeed !> > + > > + Userspace programs can control the feature using the > > + VHOST_SET_VRING_ENDIAN and VHOST_GET_VRING_ENDIAN ioctls. > > + > > + This is only useful on a few platforms (ppc64 and arm64). Since it > > + adds some overhead, it is disabled default. > > s/default/by default/ >Ok.> > + > > + If unsure, say "N". > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > > index 2ee2826..8c4390d 100644 > > --- a/drivers/vhost/vhost.c > > +++ b/drivers/vhost/vhost.c > > @@ -36,6 +36,78 @@ enum { > > #define vhost_used_event(vq) ((__virtio16 __user *)&vq->avail->ring[vq->num]) > > #define vhost_avail_event(vq) ((__virtio16 __user *)&vq->used->ring[vq->num]) > > > > +#ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY > > +static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq) > > +{ > > + vq->user_be = !virtio_legacy_is_little_endian(); > > +} > > + > > +static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp) > > +{ > > + struct vhost_vring_state s; > > + > > + if (vq->private_data) > > + return -EBUSY; > > + > > + if (copy_from_user(&s, argp, sizeof(s))) > > + return -EFAULT; > > + > > + if (s.num != VHOST_VRING_LITTLE_ENDIAN && > > + s.num != VHOST_VRING_BIG_ENDIAN) > > + return -EINVAL; > > + > > + vq->user_be = s.num; > > + > > + return 0; > > +} > > + > > +static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx, > > + int __user *argp) > > +{ > > + struct vhost_vring_state s = { > > + .index = idx, > > + .num = vq->user_be > > + }; > > + > > + if (copy_to_user(argp, &s, sizeof(s))) > > + return -EFAULT; > > + > > + return 0; > > +} > > + > > +static void vhost_init_is_le(struct vhost_virtqueue *vq) > > +{ > > + /* Note for legacy virtio: user_be is initialized at reset time > > + * according to the host endianness. If userspace does not set an > > + * explicit endianness, the default behavior is native endian, as > > + * expected by legacy virtio. > > + */ > > + vq->is_le = vhost_has_feature(vq, VIRTIO_F_VERSION_1) || !vq->user_be; > > +} > > +#else > > +static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq) > > +{ > > + ; > > Just leave the function body empty? >Ok.> > +} > > + > > +static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp) > > +{ > > + return -ENOIOCTLCMD; > > +} > > + > > +static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx, > > + int __user *argp) > > +{ > > + return -ENOIOCTLCMD; > > +} > > + > > +static void vhost_init_is_le(struct vhost_virtqueue *vq) > > +{ > > + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) > > + vq->is_le = true; > > +} > > +#endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */ > > + > > static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh, > > poll_table *pt) > > { > (...) > > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h > > index bb6a5b4..b980b53 100644 > > --- a/include/uapi/linux/vhost.h > > +++ b/include/uapi/linux/vhost.h > > @@ -103,6 +103,18 @@ struct vhost_memory { > > /* Get accessor: reads index, writes value in num */ > > #define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x12, struct vhost_vring_state) > > > > +/* Set the vring byte order in num. Valid values are VHOST_VRING_LITTLE_ENDIAN > > + * or VHOST_VRING_BIG_ENDIAN (other values return EINVAL). > > -EINVAL? >Oops, yes. :)> Should you also mention when you return -EBUSY? >Indeed... what about: "The byte order cannot be changed when the device is active: trying to do so returns -EBUSY."> > + * This is a legacy only API that is simply ignored when VIRTIO_F_VERSION_1 is > > + * set. > > + * Not all kernel configurations support this ioctl, but all configurations that > > + * support SET also support GET. > > + */ > > +#define VHOST_VRING_LITTLE_ENDIAN 0 > > +#define VHOST_VRING_BIG_ENDIAN 1 > > +#define VHOST_SET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x13, struct vhost_vring_state) > > +#define VHOST_GET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x14, struct vhost_vring_state) > > + > > /* The following ioctls use eventfd file descriptors to signal and poll > > * for events. */ > > Apart from style nitpicks, looks good. >Cool ! :)> -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Cornelia Huck
2015-Apr-24 08:09 UTC
[PATCH v5 7/8] vhost: cross-endian support for legacy devices
On Fri, 24 Apr 2015 10:06:19 +0200 Greg Kurz <gkurz at linux.vnet.ibm.com> wrote:> On Fri, 24 Apr 2015 09:19:26 +0200 > Cornelia Huck <cornelia.huck at de.ibm.com> wrote: > > > On Thu, 23 Apr 2015 17:29:42 +0200 > > Greg Kurz <gkurz at linux.vnet.ibm.com> wrote:> > > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h > > > index bb6a5b4..b980b53 100644 > > > --- a/include/uapi/linux/vhost.h > > > +++ b/include/uapi/linux/vhost.h > > > @@ -103,6 +103,18 @@ struct vhost_memory { > > > /* Get accessor: reads index, writes value in num */ > > > #define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x12, struct vhost_vring_state) > > > > > > +/* Set the vring byte order in num. Valid values are VHOST_VRING_LITTLE_ENDIAN > > > + * or VHOST_VRING_BIG_ENDIAN (other values return EINVAL). > > > > -EINVAL? > > > > Oops, yes. :) > > > Should you also mention when you return -EBUSY? > > > > Indeed... what about: > > "The byte order cannot be changed when the device is active: trying to do so > returns -EBUSY."s/when/while/ ? But sounds good.
Maybe Matching Threads
- [PATCH v5 7/8] vhost: cross-endian support for legacy devices
- [PATCH v6 7/8] vhost: cross-endian support for legacy devices
- [PATCH v5 7/8] vhost: cross-endian support for legacy devices
- [PATCH v5 7/8] vhost: cross-endian support for legacy devices
- [PATCH 1/2] vhost: helpers to enable/disable vring endianness