diff mbox series

[V4,2/2] misc: vop: do not allocate and reassign the used ring

Message ID 20201026085335.30048-3-sherry.sun@nxp.com (mailing list archive)
State Superseded, archived
Headers show
Series Change vring space from nomal memory to dma coherent memory | expand

Commit Message

Sherry Sun Oct. 26, 2020, 8:53 a.m. UTC 
We don't need to allocate and reassign the used ring here and remove the
used_address_updated flag.Since RC has allocated the entire vring,
including the used ring. Simply add the corresponding offset can get the
used ring address.

If following the orginal way to reassign the used ring, will encounter a
problem. When host finished with descriptor, it will update the used
ring with putused_kern api, if reassign used ring at EP side, used
ring will be io device memory for RC, use memcpy in putused_kern will
cause kernel panic.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/misc/mic/vop/vop_debugfs.c |  2 --
 drivers/misc/mic/vop/vop_main.c    | 51 ++++++------------------------
 drivers/misc/mic/vop/vop_vringh.c  | 31 ------------------
 include/uapi/linux/mic_common.h    |  5 ++-
 4 files changed, 11 insertions(+), 78 deletions(-)

Comments

kernel test robot Oct. 26, 2020, 10:35 a.m. UTC | #1 
Hi Sherry,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on soc/for-next linus/master v5.10-rc1 next-20201026]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sherry-Sun/Change-vring-space-from-nomal-memory-to-dma-coherent-memory/20201026-170616
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 726eb70e0d34dc4bc4dada71f52bba8ed638431e
config: i386-randconfig-s001-20201026 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-56-gc09e8239-dirty
        # https://github.com/0day-ci/linux/commit/9ec4633ad59551e4780046500b368eb7993588f3
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sherry-Sun/Change-vring-space-from-nomal-memory-to-dma-coherent-memory/20201026-170616
        git checkout 9ec4633ad59551e4780046500b368eb7993588f3
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


"sparse warnings: (new ones prefixed by >>)"
>> drivers/misc/mic/vop/vop_main.c:339:17: sparse: sparse: cast removes address space '__iomem' of expression

vim +/__iomem +339 drivers/misc/mic/vop/vop_main.c

   289	
   290	/*
   291	 * This routine will assign vring's allocated in host/io memory. Code in
   292	 * virtio_ring.c however continues to access this io memory as if it were local
   293	 * memory without io accessors.
   294	 */
   295	static struct virtqueue *vop_find_vq(struct virtio_device *dev,
   296					     unsigned index,
   297					     void (*callback)(struct virtqueue *vq),
   298					     const char *name, bool ctx)
   299	{
   300		struct _vop_vdev *vdev = to_vopvdev(dev);
   301		struct vop_device *vpdev = vdev->vpdev;
   302		struct mic_vqconfig __iomem *vqconfig;
   303		struct mic_vqconfig config;
   304		struct virtqueue *vq;
   305		void __iomem *va;
   306		struct _mic_vring_info __iomem *info;
   307		void *used;
   308		int vr_size, _vr_size, err, magic;
   309		u8 type = ioread8(&vdev->desc->type);
   310	
   311		if (index >= ioread8(&vdev->desc->num_vq))
   312			return ERR_PTR(-ENOENT);
   313	
   314		if (!name)
   315			return ERR_PTR(-ENOENT);
   316	
   317		/* First assign the vring's allocated in host memory */
   318		vqconfig = _vop_vq_config(vdev->desc) + index;
   319		memcpy_fromio(&config, vqconfig, sizeof(config));
   320		_vr_size = round_up(vring_size(le16_to_cpu(config.num), MIC_VIRTIO_RING_ALIGN), 4);
   321		vr_size = PAGE_ALIGN(_vr_size + sizeof(struct _mic_vring_info));
   322		va = vpdev->hw_ops->remap(vpdev, le64_to_cpu(config.address), vr_size);
   323		if (!va)
   324			return ERR_PTR(-ENOMEM);
   325		vdev->vr[index] = va;
   326		memset_io(va, 0x0, _vr_size);
   327	
   328		info = va + _vr_size;
   329		magic = ioread32(&info->magic);
   330	
   331		if (WARN(magic != MIC_MAGIC + type + index, "magic mismatch")) {
   332			err = -EIO;
   333			goto unmap;
   334		}
   335	
   336		vdev->used_size[index] = PAGE_ALIGN(sizeof(__u16) * 3 +
   337						     sizeof(struct vring_used_elem) *
   338						     le16_to_cpu(config.num));
 > 339		used = (void *)va + PAGE_ALIGN(sizeof(struct vring_desc) *
   340					       le16_to_cpu(config.num) + sizeof(__u16) *
   341					       (3 + le16_to_cpu(config.num)));
   342		vdev->used_virt[index] = used;
   343		if (!used) {
   344			err = -ENOMEM;
   345			dev_err(_vop_dev(vdev), "%s %d err %d\n",
   346				__func__, __LINE__, err);
   347			goto unmap;
   348		}
   349	
   350		vq = vop_new_virtqueue(index, le16_to_cpu(config.num), dev, ctx,
   351				       (void __force *)va, vop_notify, callback,
   352				       name, used);
   353		if (!vq) {
   354			err = -ENOMEM;
   355			goto unmap;
   356		}
   357	
   358		vdev->used[index] = le64_to_cpu(config.address) +
   359				    PAGE_ALIGN(sizeof(struct vring_desc) *
   360					       le16_to_cpu(config.num) + sizeof(__u16) *
   361					       (3 + le16_to_cpu(config.num)));
   362		writeq(vdev->used[index], &vqconfig->used_address);
   363	
   364		vq->priv = vdev;
   365		return vq;
   366	unmap:
   367		vpdev->hw_ops->unmap(vpdev, vdev->vr[index]);
   368		return ERR_PTR(err);
   369	}
   370	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/misc/mic/vop/vop_debugfs.c b/drivers/misc/mic/vop/vop_debugfs.c
index 9d4f175f4dd1..05eca4a98585 100644
--- a/drivers/misc/mic/vop/vop_debugfs.c
+++ b/drivers/misc/mic/vop/vop_debugfs.c
@@ -79,8 +79,6 @@  static int vop_dp_show(struct seq_file *s, void *pos)
 		seq_printf(s, "Vdev reset %d\n", dc->vdev_reset);
 		seq_printf(s, "Guest Ack %d ", dc->guest_ack);
 		seq_printf(s, "Host ack %d\n", dc->host_ack);
-		seq_printf(s, "Used address updated %d ",
-			   dc->used_address_updated);
 		seq_printf(s, "Vdev 0x%llx\n", dc->vdev);
 		seq_printf(s, "c2h doorbell %d ", dc->c2h_vdev_db);
 		seq_printf(s, "h2c doorbell %d\n", dc->h2c_vdev_db);
diff --git a/drivers/misc/mic/vop/vop_main.c b/drivers/misc/mic/vop/vop_main.c
index 714b94f42d38..07e2732b24bb 100644
--- a/drivers/misc/mic/vop/vop_main.c
+++ b/drivers/misc/mic/vop/vop_main.c
@@ -250,10 +250,6 @@  static void vop_del_vq(struct virtqueue *vq, int n)
 	struct _vop_vdev *vdev = to_vopvdev(vq->vdev);
 	struct vop_device *vpdev = vdev->vpdev;
 
-	dma_unmap_single(&vpdev->dev, vdev->used[n],
-			 vdev->used_size[n], DMA_BIDIRECTIONAL);
-	free_pages((unsigned long)vdev->used_virt[n],
-		   get_order(vdev->used_size[n]));
 	vring_del_virtqueue(vq);
 	vpdev->hw_ops->unmap(vpdev, vdev->vr[n]);
 	vdev->vr[n] = NULL;
@@ -340,8 +336,9 @@  static struct virtqueue *vop_find_vq(struct virtio_device *dev,
 	vdev->used_size[index] = PAGE_ALIGN(sizeof(__u16) * 3 +
 					     sizeof(struct vring_used_elem) *
 					     le16_to_cpu(config.num));
-	used = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
-					get_order(vdev->used_size[index]));
+	used = (void *)va + PAGE_ALIGN(sizeof(struct vring_desc) *
+				       le16_to_cpu(config.num) + sizeof(__u16) *
+				       (3 + le16_to_cpu(config.num)));
 	vdev->used_virt[index] = used;
 	if (!used) {
 		err = -ENOMEM;
@@ -355,27 +352,17 @@  static struct virtqueue *vop_find_vq(struct virtio_device *dev,
 			       name, used);
 	if (!vq) {
 		err = -ENOMEM;
-		goto free_used;
+		goto unmap;
 	}
 
-	vdev->used[index] = dma_map_single(&vpdev->dev, used,
-					    vdev->used_size[index],
-					    DMA_BIDIRECTIONAL);
-	if (dma_mapping_error(&vpdev->dev, vdev->used[index])) {
-		err = -ENOMEM;
-		dev_err(_vop_dev(vdev), "%s %d err %d\n",
-			__func__, __LINE__, err);
-		goto del_vq;
-	}
+	vdev->used[index] = le64_to_cpu(config.address) +
+			    PAGE_ALIGN(sizeof(struct vring_desc) *
+				       le16_to_cpu(config.num) + sizeof(__u16) *
+				       (3 + le16_to_cpu(config.num)));
 	writeq(vdev->used[index], &vqconfig->used_address);
 
 	vq->priv = vdev;
 	return vq;
-del_vq:
-	vring_del_virtqueue(vq);
-free_used:
-	free_pages((unsigned long)used,
-		   get_order(vdev->used_size[index]));
 unmap:
 	vpdev->hw_ops->unmap(vpdev, vdev->vr[index]);
 	return ERR_PTR(err);
@@ -388,9 +375,7 @@  static int vop_find_vqs(struct virtio_device *dev, unsigned nvqs,
 			struct irq_affinity *desc)
 {
 	struct _vop_vdev *vdev = to_vopvdev(dev);
-	struct vop_device *vpdev = vdev->vpdev;
-	struct mic_device_ctrl __iomem *dc = vdev->dc;
-	int i, err, retry, queue_idx = 0;
+	int i, err, queue_idx = 0;
 
 	/* We must have this many virtqueues. */
 	if (nvqs > ioread8(&vdev->desc->num_vq))
@@ -412,24 +397,6 @@  static int vop_find_vqs(struct virtio_device *dev, unsigned nvqs,
 		}
 	}
 
-	iowrite8(1, &dc->used_address_updated);
-	/*
-	 * Send an interrupt to the host to inform it that used
-	 * rings have been re-assigned.
-	 */
-	vpdev->hw_ops->send_intr(vpdev, vdev->c2h_vdev_db);
-	for (retry = 100; --retry;) {
-		if (!ioread8(&dc->used_address_updated))
-			break;
-		msleep(100);
-	}
-
-	dev_dbg(_vop_dev(vdev), "%s: retry: %d\n", __func__, retry);
-	if (!retry) {
-		err = -ENODEV;
-		goto error;
-	}
-
 	return 0;
 error:
 	vop_del_vqs(dev);
diff --git a/drivers/misc/mic/vop/vop_vringh.c b/drivers/misc/mic/vop/vop_vringh.c
index df77681c97e6..15dd3405789c 100644
--- a/drivers/misc/mic/vop/vop_vringh.c
+++ b/drivers/misc/mic/vop/vop_vringh.c
@@ -53,33 +53,6 @@  static void _vop_notify(struct vringh *vrh)
 		vpdev->hw_ops->send_intr(vpdev, db);
 }
 
-static void vop_virtio_init_post(struct vop_vdev *vdev)
-{
-	struct mic_vqconfig *vqconfig = mic_vq_config(vdev->dd);
-	struct vop_device *vpdev = vdev->vpdev;
-	int i, used_size;
-
-	for (i = 0; i < vdev->dd->num_vq; i++) {
-		used_size = PAGE_ALIGN(sizeof(u16) * 3 +
-				sizeof(struct vring_used_elem) *
-				le16_to_cpu(vqconfig->num));
-		if (!le64_to_cpu(vqconfig[i].used_address)) {
-			dev_warn(vop_dev(vdev), "used_address zero??\n");
-			continue;
-		}
-		vdev->vvr[i].vrh.vring.used =
-			(void __force *)vpdev->hw_ops->remap(
-			vpdev,
-			le64_to_cpu(vqconfig[i].used_address),
-			used_size);
-	}
-
-	vdev->dc->used_address_updated = 0;
-
-	dev_info(vop_dev(vdev), "%s: device type %d LINKUP\n",
-		 __func__, vdev->virtio_id);
-}
-
 static inline void vop_virtio_device_reset(struct vop_vdev *vdev)
 {
 	int i;
@@ -130,9 +103,6 @@  static void vop_bh_handler(struct work_struct *work)
 	struct vop_vdev *vdev = container_of(work, struct vop_vdev,
 			virtio_bh_work);
 
-	if (vdev->dc->used_address_updated)
-		vop_virtio_init_post(vdev);
-
 	if (vdev->dc->vdev_reset)
 		vop_virtio_device_reset(vdev);
 
@@ -250,7 +220,6 @@  static void vop_init_device_ctrl(struct vop_vdev *vdev,
 	dc->guest_ack = 0;
 	dc->vdev_reset = 0;
 	dc->host_ack = 0;
-	dc->used_address_updated = 0;
 	dc->c2h_vdev_db = -1;
 	dc->h2c_vdev_db = -1;
 	vdev->dc = dc;
diff --git a/include/uapi/linux/mic_common.h b/include/uapi/linux/mic_common.h
index 504e523f702c..fe660d486b20 100644
--- a/include/uapi/linux/mic_common.h
+++ b/include/uapi/linux/mic_common.h
@@ -56,8 +56,7 @@  struct mic_device_desc {
  * @vdev_reset: Set to 1 by guest to indicate virtio device has been reset.
  * @guest_ack: Set to 1 by guest to ack a command.
  * @host_ack: Set to 1 by host to ack a command.
- * @used_address_updated: Set to 1 by guest when the used address should be
- * updated.
+ * @must_be_zero: Reserved because this bit is no longer needed.
  * @c2h_vdev_db: The doorbell number to be used by guest. Set by host.
  * @h2c_vdev_db: The doorbell number to be used by host. Set by guest.
  */
@@ -67,7 +66,7 @@  struct mic_device_ctrl {
 	__u8 vdev_reset;
 	__u8 guest_ack;
 	__u8 host_ack;
-	__u8 used_address_updated;
+	__u8 must_be_zero;
 	__s8 c2h_vdev_db;
 	__s8 h2c_vdev_db;
 } __attribute__ ((aligned(8)));