Commit 5aa60834 authored by Vincent Whitchurch's avatar Vincent Whitchurch Committed by Greg Kroah-Hartman
Browse files

mic: vop: Fix broken virtqueues 



VOP is broken in mainline since commit 1ce9e605 ("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
2a2d1382 ("virtio: Add improved queue allocation API") in order to
address mic's usecase, according to the commit message.

Fixes: 1ce9e605 ("virtio_ring: introduce packed ring support")
Signed-off-by: default avatarVincent Whitchurch <vincent.whitchurch@axis.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent cee4c4d6

drivers/misc/mic/vop/vop_main.c

+34 −26
Original line number Diff line number Diff line
@@ -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);

CRA Git | Maintained and supported by SUSTech CRA and CCSE