[PATCH v2 0/2] virtio/vhost: fix alignment requirements

[PATCH v2 0/2] virtio/vhost: fix alignment requirements

[Michael S. Tsirkin]

vhost incorrectly asked for 8 byte alignment for
used ring pointer, it should be 4 byte.

Let's add explicit macros for ring element alignment,
this makes it easier to make sure our requirements
match the spec.

Rusty, OK to merge this through my vhost tree, or do you
prefer merging this through yours?

Michael S. Tsirkin (2):
  virtio_ring: document alignment requirements
  vhost: relax used address alignment

 include/uapi/linux/virtio_ring.h |  7 +++++++
 drivers/vhost/vhost.c            | 10 +++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

-- 
MST

[PATCH v2 1/2] virtio_ring: document alignment requirements

[Michael S. Tsirkin]

Host needs to know vring element alignment requirements:
simply doing alignof on structures doesn't work reliably: on some
platforms gcc has alignof(uint32_t) == 2.

Add macros for alignment as specified in virtio 1.0 cs01,
export them to userspace as well.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/uapi/linux/virtio_ring.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 61c818a..a3318f3 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -101,6 +101,13 @@ struct vring {
 	struct vring_used *used;
 };
 
+/* Alignment requirements for vring elements.
+ * When using pre-virtio 1.0 layout, these fall out naturally.
+ */
+#define VRING_AVAIL_ALIGN_SIZE 2
+#define VRING_USED_ALIGN_SIZE 4
+#define VRING_DESC_ALIGN_SIZE 16
+
 /* The standard layout for the ring is a continuous chunk of memory which looks
  * like this.  We assume num is a power of 2.
  *
-- 
MST

[PATCH v2 2/2] vhost: relax used address alignment

[Michael S. Tsirkin]

virtio 1.0 only requires used address to be 4 byte aligned,
vhost required 8 bytes (size of vring_used_elem).
Fix up vhost to match that.

Additionally, while vhost correctly requires 8 byte
alignment for log, it's unconnected to used ring:
it's a consequence that log has u64 entries.
Tweak code to make that clearer.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/vhost/vhost.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index ed71b53..cb807d0 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -713,9 +713,13 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
 			r = -EFAULT;
 			break;
 		}
-		if ((a.avail_user_addr & (sizeof *vq->avail->ring - 1)) ||
-		    (a.used_user_addr & (sizeof *vq->used->ring - 1)) ||
-		    (a.log_guest_addr & (sizeof *vq->used->ring - 1))) {
+
+		/* Make sure it's safe to cast pointers to vring types. */
+		BUILD_BUG_ON(__alignof__ *vq->avail > VRING_AVAIL_ALIGN_SIZE);
+		BUILD_BUG_ON(__alignof__ *vq->used > VRING_USED_ALIGN_SIZE);
+		if ((a.avail_user_addr & (VRING_AVAIL_ALIGN_SIZE - 1)) ||
+		    (a.used_user_addr & (VRING_USED_ALIGN_SIZE - 1)) ||
+		    (a.log_guest_addr & (sizeof(u64) - 1))) {
 			r = -EINVAL;
 			break;
 		}
-- 
MST

Re: [PATCH v2 0/2] virtio/vhost: fix alignment requirements

[Michael S. Tsirkin]

On Thu, Dec 25, 2014 at 05:05:01PM +0200, Michael S. Tsirkin wrote:
> vhost incorrectly asked for 8 byte alignment for
> used ring pointer, it should be 4 byte.
> 
> Let's add explicit macros for ring element alignment,
> this makes it easier to make sure our requirements
> match the spec.
> 
> Rusty, OK to merge this through my vhost tree, or do you
> prefer merging this through yours?

Just to clarify, this is needed in 3.19.

> Michael S. Tsirkin (2):
>   virtio_ring: document alignment requirements
>   vhost: relax used address alignment
> 
>  include/uapi/linux/virtio_ring.h |  7 +++++++
>  drivers/vhost/vhost.c            | 10 +++++++---
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> -- 
> MST
> 

Re: [PATCH v2 0/2] virtio/vhost: fix alignment requirements

[Rusty Russell]

"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Thu, Dec 25, 2014 at 05:05:01PM +0200, Michael S. Tsirkin wrote:
>> vhost incorrectly asked for 8 byte alignment for
>> used ring pointer, it should be 4 byte.
>> 
>> Let's add explicit macros for ring element alignment,
>> this makes it easier to make sure our requirements
>> match the spec.
>> 
>> Rusty, OK to merge this through my vhost tree, or do you
>> prefer merging this through yours?
>
> Just to clarify, this is needed in 3.19.

Sure, please take this via the vhost tree.

Acked-by: Rusty Russell <rusty@rustcorp.com.au>

Thanks,
Rusty.