thr3ads.net - Virtualization - [PATCH RFC v2 01/16] virtio: memory access APIs [Oct 2014]

If this information is useful, please help other people find it:
Share via:

Michael S. Tsirkin

2014-Oct-22 15:50 UTC

[PATCH RFC v2 01/16] virtio: memory access APIs

virtio 1.0 makes all memory structures LE, so
we need APIs to conditionally do a byteswap on BE
architectures.

To make it easier to check code statically,
add virtio specific types for multi-byte integers
in memory.

Add low level wrappers that do a byteswap conditionally, these will be
useful e.g. for vhost.  Add high level wrappers that will (in the
future) query device endian-ness and act accordingly.

At the moment, stub them out and assume native endian-ness everywhere.

Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
---
 include/linux/virtio_config.h    | 16 +++++++++++++
 include/uapi/linux/virtio_ring.h | 49 ++++++++++++++++++++--------------------
 include/uapi/linux/Kbuild        |  1 +
 3 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 7f4ef66..d38d3c2 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -4,6 +4,7 @@
 #include <linux/err.h>
 #include <linux/bug.h>
 #include <linux/virtio.h>
+#include <linux/virtio_byteorder.h>
 #include <uapi/linux/virtio_config.h>
 
 /**
@@ -152,6 +153,21 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
 	return 0;
 }
 
+/* Memory accessors */
+#define DEFINE_VIRTIO_XX_TO_CPU(bits) \
+static inline u##bits virtio##bits##_to_cpu(struct virtio_device *vdev,
__virtio##bits val) \
+{ \
+	return __virtio##bits##_to_cpu(false, val); \
+} \
+static inline __virtio##bits cpu_to_virtio##bits(struct virtio_device *vdev,
u##bits val) \
+{ \
+	return __cpu_to_virtio##bits(false, val); \
+}
+
+DEFINE_VIRTIO_XX_TO_CPU(16)
+DEFINE_VIRTIO_XX_TO_CPU(32)
+DEFINE_VIRTIO_XX_TO_CPU(64)
+
 /* Config space accessors. */
 #define virtio_cread(vdev, structname, member, ptr)			\
 	do {								\
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index a99f9b7..6c00632 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -32,6 +32,7 @@
  *
  * Copyright Rusty Russell IBM Corporation 2007. */
 #include <linux/types.h>
+#include <linux/virtio_types.h>
 
 /* This marks a buffer as continuing via the next field. */
 #define VRING_DESC_F_NEXT	1
@@ -61,32 +62,32 @@
 /* Virtio ring descriptors: 16 bytes.  These can chain together via
"next". */
 struct vring_desc {
 	/* Address (guest-physical). */
-	__u64 addr;
+	__virtio64 addr;
 	/* Length. */
-	__u32 len;
+	__virtio32 len;
 	/* The flags as indicated above. */
-	__u16 flags;
+	__virtio16 flags;
 	/* We chain unused descriptors via this, too */
-	__u16 next;
+	__virtio16 next;
 };
 
 struct vring_avail {
-	__u16 flags;
-	__u16 idx;
-	__u16 ring[];
+	__virtio16 flags;
+	__virtio16 idx;
+	__virtio16 ring[];
 };
 
 /* u32 is used here for ids for padding reasons. */
 struct vring_used_elem {
 	/* Index of start of used descriptor chain. */
-	__u32 id;
+	__virtio32 id;
 	/* Total length of the descriptor chain which was used (written to) */
-	__u32 len;
+	__virtio32 len;
 };
 
 struct vring_used {
-	__u16 flags;
-	__u16 idx;
+	__virtio16 flags;
+	__virtio16 idx;
 	struct vring_used_elem ring[];
 };
 
@@ -109,25 +110,25 @@ struct vring {
  *	struct vring_desc desc[num];
  *
  *	// A ring of available descriptor heads with free-running index.
- *	__u16 avail_flags;
- *	__u16 avail_idx;
- *	__u16 available[num];
- *	__u16 used_event_idx;
+ *	__virtio16 avail_flags;
+ *	__virtio16 avail_idx;
+ *	__virtio16 available[num];
+ *	__virtio16 used_event_idx;
  *
  *	// Padding to the next align boundary.
  *	char pad[];
  *
  *	// A ring of used descriptor heads with free-running index.
- *	__u16 used_flags;
- *	__u16 used_idx;
+ *	__virtio16 used_flags;
+ *	__virtio16 used_idx;
  *	struct vring_used_elem used[num];
- *	__u16 avail_event_idx;
+ *	__virtio16 avail_event_idx;
  * };
  */
 /* We publish the used event index at the end of the available ring, and vice
  * versa. They are at the end for backwards compatibility. */
 #define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
-#define vring_avail_event(vr) (*(__u16
*)&(vr)->used->ring[(vr)->num])
+#define vring_avail_event(vr) (*(__virtio16
*)&(vr)->used->ring[(vr)->num])
 
 static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 			      unsigned long align)
@@ -135,29 +136,29 @@ static inline void vring_init(struct vring *vr, unsigned
int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] +
sizeof(__u16)
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] +
sizeof(__virtio16)
 		+ align-1) & ~(align - 1));
 }
 
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__virtio16) * (3 + num)
 		 + align - 1) & ~(align - 1))
-		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
+		+ sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
 }
 
 /* The following is used with USED_EVENT_IDX and AVAIL_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 inline int vring_need_event(__u16 event_idx, __u16 new_idx, __u16 old)
+static inline int vring_need_event(__virtio16 event_idx, __virtio16 new_idx,
__virtio16 old)
 {
 	/* Note: Xen has similar logic for notification hold-off
 	 * in include/xen/interface/io/ring.h with req_event and req_prod
 	 * corresponding to event_idx + 1 and new_idx respectively.
 	 * Note also that req_event and req_prod in Xen start at 1,
 	 * event indexes in virtio start at 0. */
-	return (__u16)(new_idx - event_idx - 1) < (__u16)(new_idx - old);
+	return (__virtio16)(new_idx - event_idx - 1) < (__virtio16)(new_idx - old);
 }
 
 #endif /* _UAPI_LINUX_VIRTIO_RING_H */
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 6cad974..39c161a 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -419,6 +419,7 @@ header-y += virtio_blk.h
 header-y += virtio_config.h
 header-y += virtio_console.h
 header-y += virtio_ids.h
+header-y += virtio_types.h
 header-y += virtio_net.h
 header-y += virtio_pci.h
 header-y += virtio_ring.h
-- 
MST

Christopher Covington

2014-Oct-22 17:45 UTC

head link

[PATCH RFC v2 01/16] virtio: memory access APIs

Hi Michael,

On 10/22/2014 11:50 AM, Michael S. Tsirkin wrote:> virtio 1.0 makes all memory structures LE, so
> we need APIs to conditionally do a byteswap on BE
> architectures.
> 
> To make it easier to check code statically,
> add virtio specific types for multi-byte integers
> in memory.
> 
> Add low level wrappers that do a byteswap conditionally, these will be
> useful e.g. for vhost.  Add high level wrappers that will (in the
> future) query device endian-ness and act accordingly.
> 
> At the moment, stub them out and assume native endian-ness everywhere.
> 
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> ---
>  include/linux/virtio_config.h    | 16 +++++++++++++
>  include/uapi/linux/virtio_ring.h | 49
++++++++++++++++++++--------------------
>  include/uapi/linux/Kbuild        |  1 +
>  3 files changed, 42 insertions(+), 24 deletions(-)
> 
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 7f4ef66..d38d3c2 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -4,6 +4,7 @@
>  #include <linux/err.h>
>  #include <linux/bug.h>
>  #include <linux/virtio.h>
> +#include <linux/virtio_byteorder.h>
What patch creates this file?
>  #include <uapi/linux/virtio_config.h>
>  
>  /**
> @@ -152,6 +153,21 @@ int virtqueue_set_affinity(struct virtqueue *vq, int
cpu)
>  	return 0;
>  }
>  
> +/* Memory accessors */
> +#define DEFINE_VIRTIO_XX_TO_CPU(bits) \
> +static inline u##bits virtio##bits##_to_cpu(struct virtio_device *vdev,
__virtio##bits val) \
> +{ \
> +	return __virtio##bits##_to_cpu(false, val); \
> +} \
> +static inline __virtio##bits cpu_to_virtio##bits(struct virtio_device
*vdev, u##bits val) \
> +{ \
> +	return __cpu_to_virtio##bits(false, val); \
> +}
> +
> +DEFINE_VIRTIO_XX_TO_CPU(16)
> +DEFINE_VIRTIO_XX_TO_CPU(32)
> +DEFINE_VIRTIO_XX_TO_CPU(64)
> +
>  /* Config space accessors. */
>  #define virtio_cread(vdev, structname, member, ptr)			\
>  	do {								\
> diff --git a/include/uapi/linux/virtio_ring.h
b/include/uapi/linux/virtio_ring.h
> index a99f9b7..6c00632 100644
> --- a/include/uapi/linux/virtio_ring.h
> +++ b/include/uapi/linux/virtio_ring.h
> @@ -32,6 +32,7 @@
>   *
>   * Copyright Rusty Russell IBM Corporation 2007. */
>  #include <linux/types.h>
> +#include <linux/virtio_types.h>
>  
>  /* This marks a buffer as continuing via the next field. */
>  #define VRING_DESC_F_NEXT	1
> @@ -61,32 +62,32 @@
>  /* Virtio ring descriptors: 16 bytes.  These can chain together via
"next". */
>  struct vring_desc {
>  	/* Address (guest-physical). */
> -	__u64 addr;
> +	__virtio64 addr;
>  	/* Length. */
> -	__u32 len;
> +	__virtio32 len;
>  	/* The flags as indicated above. */
> -	__u16 flags;
> +	__virtio16 flags;
>  	/* We chain unused descriptors via this, too */
> -	__u16 next;
> +	__virtio16 next;
>  };
How does __virtio64 differ from __le64?

Thanks,
Chris

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Michael S. Tsirkin

2014-Oct-22 18:40 UTC

head link

[PATCH RFC v2 01/16] virtio: memory access APIs

On Wed, Oct 22, 2014 at 01:45:55PM -0400, Christopher Covington
wrote:> Hi Michael,
> 
> On 10/22/2014 11:50 AM, Michael S. Tsirkin wrote:
> > virtio 1.0 makes all memory structures LE, so
> > we need APIs to conditionally do a byteswap on BE
> > architectures.
> > 
> > To make it easier to check code statically,
> > add virtio specific types for multi-byte integers
> > in memory.
> > 
> > Add low level wrappers that do a byteswap conditionally, these will be
> > useful e.g. for vhost.  Add high level wrappers that will (in the
> > future) query device endian-ness and act accordingly.
> > 
> > At the moment, stub them out and assume native endian-ness everywhere.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> > ---
> >  include/linux/virtio_config.h    | 16 +++++++++++++
> >  include/uapi/linux/virtio_ring.h | 49
++++++++++++++++++++--------------------
> >  include/uapi/linux/Kbuild        |  1 +
> >  3 files changed, 42 insertions(+), 24 deletions(-)
> > 
> > diff --git a/include/linux/virtio_config.h
b/include/linux/virtio_config.h
> > index 7f4ef66..d38d3c2 100644
> > --- a/include/linux/virtio_config.h
> > +++ b/include/linux/virtio_config.h
> > @@ -4,6 +4,7 @@
> >  #include <linux/err.h>
> >  #include <linux/bug.h>
> >  #include <linux/virtio.h>
> > +#include <linux/virtio_byteorder.h>
> 
> What patch creates this file?
Oops I forgot to git add it.
Will resend.
> >  #include <uapi/linux/virtio_config.h>
> >  
> >  /**
> > @@ -152,6 +153,21 @@ int virtqueue_set_affinity(struct virtqueue *vq,
int cpu)
> >  	return 0;
> >  }
> >  
> > +/* Memory accessors */
> > +#define DEFINE_VIRTIO_XX_TO_CPU(bits) \
> > +static inline u##bits virtio##bits##_to_cpu(struct virtio_device
*vdev, __virtio##bits val) \
> > +{ \
> > +	return __virtio##bits##_to_cpu(false, val); \
> > +} \
> > +static inline __virtio##bits cpu_to_virtio##bits(struct virtio_device
*vdev, u##bits val) \
> > +{ \
> > +	return __cpu_to_virtio##bits(false, val); \
> > +}
> > +
> > +DEFINE_VIRTIO_XX_TO_CPU(16)
> > +DEFINE_VIRTIO_XX_TO_CPU(32)
> > +DEFINE_VIRTIO_XX_TO_CPU(64)
> > +
> >  /* Config space accessors. */
> >  #define virtio_cread(vdev, structname, member, ptr)			\
> >  	do {								\
> > diff --git a/include/uapi/linux/virtio_ring.h
b/include/uapi/linux/virtio_ring.h
> > index a99f9b7..6c00632 100644
> > --- a/include/uapi/linux/virtio_ring.h
> > +++ b/include/uapi/linux/virtio_ring.h
> > @@ -32,6 +32,7 @@
> >   *
> >   * Copyright Rusty Russell IBM Corporation 2007. */
> >  #include <linux/types.h>
> > +#include <linux/virtio_types.h>
> >  
> >  /* This marks a buffer as continuing via the next field. */
> >  #define VRING_DESC_F_NEXT	1
> > @@ -61,32 +62,32 @@
> >  /* Virtio ring descriptors: 16 bytes.  These can chain together via
"next". */
> >  struct vring_desc {
> >  	/* Address (guest-physical). */
> > -	__u64 addr;
> > +	__virtio64 addr;
> >  	/* Length. */
> > -	__u32 len;
> > +	__virtio32 len;
> >  	/* The flags as indicated above. */
> > -	__u16 flags;
> > +	__virtio16 flags;
> >  	/* We chain unused descriptors via this, too */
> > -	__u16 next;
> > +	__virtio16 next;
> >  };
> 
> How does __virtio64 differ from __le64?
> 
> Thanks,
> Chris
__le64 would require cpu_to_le, I wanted to force the use
of our byte swapping macros.
> -- 
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project

Possibly Parallel Threads

Search for more possibly parallel threads

Virtualization - Oct 2014 - [PATCH RFC v2 01/16] virtio: memory access APIs

[PATCH RFC v2 01/16] virtio: memory access APIs

[PATCH RFC v2 01/16] virtio: memory access APIs

[PATCH RFC v2 01/16] virtio: memory access APIs

Possibly Parallel Threads