]> asedeno.scripts.mit.edu Git - linux.git/commitdiff
mic: vop: Fix broken virtqueues
authorVincent Whitchurch <vincent.whitchurch@axis.com>
Tue, 29 Jan 2019 10:22:07 +0000 (11:22 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 30 Jan 2019 14:42:26 +0000 (15:42 +0100)
VOP is broken in mainline since commit 1ce9e6055fa0a9043 ("virtio_ring:
introduce packed ring support"); attempting to use the virtqueues leads
to various kernel crashes.  I'm testing it with my not-yet-merged
loopback patches, but even the in-tree MIC hardware cannot work.

The problem is not in the referenced commit per se, but is due to the
following hack in vop_find_vq() which depends on the layout of private
structures in other source files, which that commit happened to change:

  /*
   * To reassign the used ring here we are directly accessing
   * struct vring_virtqueue which is a private data structure
   * in virtio_ring.c. At the minimum, a BUILD_BUG_ON() in
   * vring_new_virtqueue() would ensure that
   *  (&vq->vring == (struct vring *) (&vq->vq + 1));
   */
  vr = (struct vring *)(vq + 1);
  vr->used = used;

Fix vop by using __vring_new_virtqueue() to create the needed vring
layout from the start, instead of attempting to patch in the used ring
later.  __vring_new_virtqueue() was added way back in commit
2a2d1382fe9dcc ("virtio: Add improved queue allocation API") in order to
address mic's usecase, according to the commit message.

Fixes: 1ce9e6055fa0 ("virtio_ring: introduce packed ring support")
Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/misc/mic/vop/vop_main.c

index 2bfa3a903bf9a574f1d573b94e36ea4f6bc86273..2bd57c2ca02bca6b2f0579b0db72d6fa4232ac3a 100644 (file)
@@ -283,6 +283,26 @@ static void vop_del_vqs(struct virtio_device *dev)
                vop_del_vq(vq, idx++);
 }
 
+static struct virtqueue *vop_new_virtqueue(unsigned int index,
+                                     unsigned int num,
+                                     struct virtio_device *vdev,
+                                     bool context,
+                                     void *pages,
+                                     bool (*notify)(struct virtqueue *vq),
+                                     void (*callback)(struct virtqueue *vq),
+                                     const char *name,
+                                     void *used)
+{
+       bool weak_barriers = false;
+       struct vring vring;
+
+       vring_init(&vring, num, pages, MIC_VIRTIO_RING_ALIGN);
+       vring.used = used;
+
+       return __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
+                                    notify, callback, name);
+}
+
 /*
  * This routine will assign vring's allocated in host/io memory. Code in
  * virtio_ring.c however continues to access this io memory as if it were local
@@ -302,7 +322,6 @@ static struct virtqueue *vop_find_vq(struct virtio_device *dev,
        struct _mic_vring_info __iomem *info;
        void *used;
        int vr_size, _vr_size, err, magic;
-       struct vring *vr;
        u8 type = ioread8(&vdev->desc->type);
 
        if (index >= ioread8(&vdev->desc->num_vq))
@@ -322,17 +341,7 @@ static struct virtqueue *vop_find_vq(struct virtio_device *dev,
                return ERR_PTR(-ENOMEM);
        vdev->vr[index] = va;
        memset_io(va, 0x0, _vr_size);
-       vq = vring_new_virtqueue(
-                               index,
-                               le16_to_cpu(config.num), MIC_VIRTIO_RING_ALIGN,
-                               dev,
-                               false,
-                               ctx,
-                               (void __force *)va, vop_notify, callback, name);
-       if (!vq) {
-               err = -ENOMEM;
-               goto unmap;
-       }
+
        info = va + _vr_size;
        magic = ioread32(&info->magic);
 
@@ -341,7 +350,6 @@ static struct virtqueue *vop_find_vq(struct virtio_device *dev,
                goto unmap;
        }
 
-       /* Allocate and reassign used ring now */
        vdev->used_size[index] = PAGE_ALIGN(sizeof(__u16) * 3 +
                                             sizeof(struct vring_used_elem) *
                                             le16_to_cpu(config.num));
@@ -351,8 +359,17 @@ static struct virtqueue *vop_find_vq(struct virtio_device *dev,
                err = -ENOMEM;
                dev_err(_vop_dev(vdev), "%s %d err %d\n",
                        __func__, __LINE__, err);
-               goto del_vq;
+               goto unmap;
+       }
+
+       vq = vop_new_virtqueue(index, le16_to_cpu(config.num), dev, ctx,
+                              (void __force *)va, vop_notify, callback,
+                              name, used);
+       if (!vq) {
+               err = -ENOMEM;
+               goto free_used;
        }
+
        vdev->used[index] = dma_map_single(&vpdev->dev, used,
                                            vdev->used_size[index],
                                            DMA_BIDIRECTIONAL);
@@ -360,26 +377,17 @@ static struct virtqueue *vop_find_vq(struct virtio_device *dev,
                err = -ENOMEM;
                dev_err(_vop_dev(vdev), "%s %d err %d\n",
                        __func__, __LINE__, err);
-               goto free_used;
+               goto del_vq;
        }
        writeq(vdev->used[index], &vqconfig->used_address);
-       /*
-        * To reassign the used ring here we are directly accessing
-        * struct vring_virtqueue which is a private data structure
-        * in virtio_ring.c. At the minimum, a BUILD_BUG_ON() in
-        * vring_new_virtqueue() would ensure that
-        *  (&vq->vring == (struct vring *) (&vq->vq + 1));
-        */
-       vr = (struct vring *)(vq + 1);
-       vr->used = used;
 
        vq->priv = vdev;
        return vq;
+del_vq:
+       vring_del_virtqueue(vq);
 free_used:
        free_pages((unsigned long)used,
                   get_order(vdev->used_size[index]));
-del_vq:
-       vring_del_virtqueue(vq);
 unmap:
        vpdev->hw_ops->iounmap(vpdev, vdev->vr[index]);
        return ERR_PTR(err);