Re: [PATCH v7 31/46] vhost: virtio 1.0 endian-ness support

On Mon, Dec 01, 2014 at 01:33:53PM +0100, Cornelia Huck wrote:

On Sun, 30 Nov 2014 17:11:49 +0200
"Michael S. Tsirkin" [off-list ref] wrote:

quoted

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

quoted

@@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue *vq,
 {
 	struct vring_desc desc;
 	unsigned int i = 0, count, found = 0;
+	u32 len = vhost32_to_cpu(vq, indirect->len);
 	int ret;

 	/* Sanity check */
-	if (unlikely(indirect->len % sizeof desc)) {
+	if (unlikely(len % sizeof desc)) {
 		vq_err(vq, "Invalid length in indirect descriptor: "
 		       "len 0x%llx not multiple of 0x%zx\n",
-		       (unsigned long long)indirect->len,
+		       (unsigned long long)vhost32_to_cpu(vq, indirect->len),

Can't you use len here?

quoted

 		       sizeof desc);
 		return -EINVAL;
 	}

-	ret = translate_desc(vq, indirect->addr, indirect->len, vq->indirect,
+	ret = translate_desc(vq, vhost64_to_cpu(vq, indirect->addr), len, vq->indirect,
 			     UIO_MAXIOV);
 	if (unlikely(ret < 0)) {
 		vq_err(vq, "Translation failure %d in indirect.\n", ret);

quoted

@@ -1404,7 +1422,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,

 	/* Make sure buffer is written before we update index. */
 	smp_wmb();
-	if (put_user(vq->last_used_idx, &vq->used->idx)) {
+	if (__put_user(cpu_to_vhost16(vq, vq->last_used_idx), &vq->used->idx)) {

Why s/put_user/__put_user/ - I don't see how endianness conversions
should have an influence there?

quoted

 		vq_err(vq, "Failed to increment used idx");
 		return -EFAULT;
 	}

quoted

@@ -1449,11 +1468,11 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
 	if (unlikely(!v))
 		return true;

-	if (get_user(event, vhost_used_event(vq))) {
+	if (__get_user(event, vhost_used_event(vq))) {

Dito: why the change?

I remember now.
put_user/get_user macros don't work well
with __virtio tags.

As __ are a good idea anyway, I just switched to that
everywhere.

quoted

 		vq_err(vq, "Failed to get used event idx");
 		return true;
 	}
-	return vring_need_event(event, new, old);
+	return vring_need_event(vhost16_to_cpu(vq, event), new, old);
 }

 /* This actually signals the guest, using eventfd. */