Merge pull request #20 from swapnili/master

General cleanup and fixes 

5 files changed, 59 insertions, 115 deletions

diff --git a/kmod/muser.c b/kmod/muser.c
index 451d007..9c7ef9e 100644
--- a/kmod/muser.c
+++ b/kmod/muser.c
@@ -118,9 +118,6 @@ struct muser_dev {
 	struct radix_tree_root	devmem_tree;	/* Device memory */
 };
 
-/* function prototypes */
-static int dma_unmap_all(struct muser_dev *const mudev, const bool skip_user);
-
 static inline int muser_copyout(void __user *param, const void *address,
 				unsigned long size)
 {
@@ -628,8 +625,8 @@ static int muser_process_dma_unmap(struct muser_dev *mudev,
 	return muser_process_dma_request(mudev, dma_map, 0, MUSER_DMA_MUNMAP);
 }
 
-static int put_dma_map(struct muser_dev *mudev,
-		       struct vfio_dma_mapping *dma_map, int nr_pages)
+static void put_dma_map(struct muser_dev *mudev,
+			struct vfio_dma_mapping *dma_map, int nr_pages)
 {
 	unsigned long off, iova_pfn;
 	int i, ret;
@@ -637,14 +634,12 @@ static int put_dma_map(struct muser_dev *mudev,
 	for (i = 0, off = 0; i < nr_pages; i++, off += PAGE_SIZE) {
 		iova_pfn = (dma_map->iova + off) >> PAGE_SHIFT;
 		ret = vfio_unpin_pages(mdev_dev(mudev->mdev), &iova_pfn, 1);
-		if (WARN_ON(ret != 1))
-			return -EINVAL;
+		WARN_ON(ret != 1);
 
 		put_page(dma_map->pages[i]);
 	}
 
 	kfree(dma_map->pages);
-	return 0;
 }
 
 static int
@@ -800,15 +795,11 @@ static int muser_iommu_dma_unmap(struct muser_dev *const mudev,
 	if (unlikely(err))
 		muser_dbg("failed to request libmuser to munmap: %d", err);
 
-	err = put_dma_map(mudev, dma_map, NR_PAGES(len));
-	if (unlikely(err)) {
-		muser_dbg("failed to tear down DMA map: %d", err);
-		goto out;
-	}
+	put_dma_map(mudev, dma_map, NR_PAGES(len));
+	kfree(dma_map);
 
 	/* XXX: Do we need this? */
 	unmap->size = len;
-out:
 	return err;
 }
 
@@ -862,6 +853,38 @@ static int register_notifier(struct mdev_device *const mdev)
 				      &events, &mudev->iommu_notifier);
 }
 
+static int dma_unmap_all(struct muser_dev *mudev)
+{
+	struct vfio_dma_mapping *dma_map;
+	unsigned long length;
+	LIST_HEAD(head);
+
+	/*
+	 * TODO: Cleanup
+	 * Use better list functions like:
+	 * list_replace()/list_replace_init()
+	 * list_for_each_entry_safe()
+	 */
+
+	mutex_lock(&mudev->dev_lock);
+	while (!list_empty(&mudev->dma_list)) {
+		dma_map = list_first_entry(&mudev->dma_list,
+					   struct vfio_dma_mapping, entry);
+		list_move(&dma_map->entry, &head);
+	}
+	mutex_unlock(&mudev->dev_lock);
+
+	while (!list_empty(&head)) {
+		dma_map = list_first_entry(&head, struct vfio_dma_mapping,
+					   entry);
+		list_del(&dma_map->entry);
+		length = dma_map->length;
+		put_dma_map(mudev, dma_map, NR_PAGES(length));
+		kfree(dma_map);
+	}
+	return 0;
+}
+
 int muser_open(struct mdev_device *mdev)
 {
 	int err;
@@ -885,7 +908,7 @@ int muser_open(struct mdev_device *mdev)
 		 */
 		atomic_dec(&mudev->mdev_opened);
 		muser_dbg("failed to register notifier: %d", err);
-		err2 = dma_unmap_all(mudev, false);
+		err2 = dma_unmap_all(mudev);
 		if (unlikely(err2))
 			muser_dbg("failed to DMA unmap all regions: %d", err2);
 		err2 = vfio_unregister_notifier(mdev_dev(mdev),
@@ -899,50 +922,12 @@ int muser_open(struct mdev_device *mdev)
 	return err;
 }
 
-static int dma_unmap_all(struct muser_dev *mudev, bool skip_user)
-{
-	struct vfio_dma_mapping *dma_map;
-	unsigned long length;
-	LIST_HEAD(head);
-
-	mutex_lock(&mudev->dev_lock);
-	while (!list_empty(&mudev->dma_list)) {
-		dma_map = list_first_entry(&mudev->dma_list,
-					   struct vfio_dma_mapping, entry);
-		list_move(&dma_map->entry, &head);
-	}
-	mutex_unlock(&mudev->dev_lock);
-
-	while (!list_empty(&head)) {
-		int err;
-
-		dma_map = list_first_entry(&head, struct vfio_dma_mapping,
-					   entry);
-		list_del(&dma_map->entry);
-		if (!skip_user) {
-			err = muser_process_dma_unmap(mudev, dma_map);
-			if (unlikely(err)) {
-				muser_alert("unmap request failed IOVA=%lx: %d",
-					    dma_map->iova, err);
-				continue;
-			}
-		}
-
-		length = dma_map->length;
-		err = put_dma_map(mudev, dma_map, NR_PAGES(length));
-		if (unlikely(err))
-			muser_alert("failed to unmap DMA IOVA=%lx: %d",
-				    dma_map->iova, err);
-	}
-	return 0;
-}
-
 void muser_close(struct mdev_device *mdev)
 {
 	struct muser_dev *mudev = mdev_get_drvdata(mdev);
 	int err;
 
-	err = dma_unmap_all(mudev, false);
+	err = dma_unmap_all(mudev);
 	if (unlikely(err))
 		muser_alert("failed to remove one or more DMA maps");
 
@@ -1639,7 +1624,6 @@ static int libmuser_open(struct inode *inode, struct file *filep)
 static int libmuser_release(struct inode *inode, struct file *filep)
 {
 	struct muser_dev *mudev = filep->private_data;
-	int err;
 
 	WARN_ON(mudev == NULL);
 	mutex_lock(&mudev->dev_lock);
@@ -1653,10 +1637,6 @@ static int libmuser_release(struct inode *inode, struct file *filep)
 	}
 	mutex_unlock(&mudev->dev_lock);
 
-	err = dma_unmap_all(mudev, true);
-	if (unlikely(err))
-		muser_alert("failed to remove DMA maps");
-
 	filep->private_data = NULL;
 	atomic_dec(&mudev->srv_opened);
 
diff --git a/lib/dma.c b/lib/dma.c
index 27901a1..17e34e8 100644
--- a/lib/dma.c
+++ b/lib/dma.c
@@ -88,9 +88,7 @@ static void
 _dma_controller_do_remove_region(dma_memory_region_t *region)
 {
     assert(region);
-#if DMA_MAP_FAST_IMPL
     dma_unmap_region(region, region->virt_addr, region->size);
-#endif
     (void)close(region->fd);
 }
 
@@ -216,7 +214,7 @@ dma_controller_add_region(lm_ctx_t *lm_ctx, dma_controller_t *dma,
                strerror(errno));
         goto err;
     }
-#if DMA_MAP_FAST_IMPL
+
     region->virt_addr = dma_map_region(region, PROT_READ | PROT_WRITE,
                                        0, region->size);
     if (region->virt_addr == MAP_FAILED) {
@@ -225,8 +223,6 @@ dma_controller_add_region(lm_ctx_t *lm_ctx, dma_controller_t *dma,
         close(region->fd);
         goto err;
     }
-#endif
-
     dma->nregions++;
 
     return idx;
diff --git a/lib/dma.h b/lib/dma.h
index 9e7126e..1c41dce 100644
--- a/lib/dma.h
+++ b/lib/dma.h
@@ -48,22 +48,11 @@
  *   as an iovec for direct access, and unmapped using dma_unmap_sg() when done.
  * - dma_map_addr() and dma_unmap_addr() helper functions are provided
  *   for mapping DMA regions that can fit into one scatter-gather entry.
- *
- * This library can be compiled to function in two modes as defined by the
- * following macros.
- * - DMA_MAP_FAST (default): Every region is mapped into the application's
- *   virtual address space at registration time with R/W permissions.
+ *   Every region is mapped into the application's virtual address space
+ *   at registration time with R/W permissions.
  *   dma_map_sg() ignores all protection bits and only does lookups and
  *   returns pointers to the previously mapped regions. dma_unmap_sg() is
  *   effectively a no-op.
- * - DMA_MAP_PROTECTED: Every call to dma_map_sg() does mmap()s and
- *   dma_unmap_sg() does munmap()s. All permission bits are honored. This mode
- *   is obviously much slower if used in the fast path. It may be useful to
- *   have the exta protection if the fast path does not need direct virtual
- *   memory access to foreign memory and data is accessed using a different
- *   method (e.g. RDMA, vfio-iommu). It can also be useful in debugging to
- *   make sure we are not writing to guest memory that's readonly for the
- *   device.
  */
 
 #ifdef DMA_MAP_PROTECTED
@@ -89,9 +78,7 @@ typedef struct {
     int fd;                     // File descriptor to mmap
     int page_size;              // Page size of this fd
     off_t offset;               // File offset
-#if DMA_MAP_FAST_IMPL
     void *virt_addr;            // Virtual address of this region
-#endif
 } dma_memory_region_t;
 
 typedef struct {
@@ -174,57 +161,39 @@ void
 dma_unmap_region(dma_memory_region_t *region, void *virt_addr, size_t len);
 
 static inline int
-dma_map_sg(dma_controller_t *dma,
-#if DMA_MAP_FAST_IMPL
-           int prot __attribute__((unused)),
-#else
-           int prot,
-#endif
-           const dma_sg_t *sg, struct iovec *iov, int cnt)
+dma_map_sg(dma_controller_t *dma, const dma_sg_t *sg, struct iovec *iov,
+           int cnt)
 {
+    dma_memory_region_t *region;
     int i;
 
     for (i = 0; i < cnt; i++) {
-        dma_memory_region_t *const region = &dma->regions[sg[i].region];
-
-#if DMA_MAP_FAST_IMPL
-        iov[i].iov_base = (char *)region->virt_addr + sg[i].offset;
-#else
-        iov[i].iov_base = dma_map_region(region, prot,
-                                         sg[i].offset, sg[i].length);
-        if (iov[i].iov_base == MAP_FAILED) {
-            return -1;
-        }
-#endif
+        region = &dma->regions[sg[i].region];
+        iov[i].iov_base = region->virt_addr + sg[i].offset;
         iov[i].iov_len = sg[i].length;
     }
 
     return 0;
 }
 
+#define UNUSED __attribute__((unused))
+
 static inline void
-dma_unmap_sg(dma_controller_t *dma,
-             const dma_sg_t *sg, struct iovec *iov, int cnt)
+dma_unmap_sg(UNUSED dma_controller_t *dma, UNUSED const dma_sg_t *sg,
+	     UNUSED struct iovec *iov, UNUSED int cnt)
 {
-    int i;
-
-    for (i = 0; i < cnt; i++) {
-        dma_memory_region_t *const region = &dma->regions[sg[i].region];
-        if (!DMA_MAP_FAST_IMPL) {
-            dma_unmap_region(region, iov[i].iov_base, iov[i].iov_len);
-        }
-    }
+    /* just a placeholder for now */
+    return;
 }
 
 static inline void *
-dma_map_addr(dma_controller_t *dma, int prot,
-             dma_addr_t dma_addr, uint32_t len)
+dma_map_addr(dma_controller_t *dma, dma_addr_t dma_addr, uint32_t len)
 {
     dma_sg_t sg;
     struct iovec iov;
 
     if (dma_addr_to_sg(dma, dma_addr, len, &sg, 1) == 1 &&
-        dma_map_sg(dma, prot, &sg, &iov, 1) == 0) {
+        dma_map_sg(dma, &sg, &iov, 1) == 0) {
         return iov.iov_base;
     }
 
diff --git a/lib/libmuser.c b/lib/libmuser.c
index 547000e..e83b477 100644
--- a/lib/libmuser.c
+++ b/lib/libmuser.c
@@ -1311,10 +1311,10 @@ lm_addr_to_sg(lm_ctx_t *lm_ctx, dma_addr_t dma_addr,
 }
 
 inline int
-lm_map_sg(lm_ctx_t *lm_ctx, int prot,
-          const dma_sg_t *sg, struct iovec *iov, int cnt)
+lm_map_sg(lm_ctx_t *lm_ctx, const dma_sg_t *sg,
+	  struct iovec *iov, int cnt)
 {
-    return dma_map_sg(lm_ctx->dma, prot, sg, iov, cnt);
+    return dma_map_sg(lm_ctx->dma, sg, iov, cnt);
 }
 
 inline void
diff --git a/lib/muser.h b/lib/muser.h
index 58f9460..f3330fe 100644
--- a/lib/muser.h
+++ b/lib/muser.h
@@ -389,7 +389,6 @@ lm_addr_to_sg(lm_ctx_t *lm_ctx, dma_addr_t dma_addr, uint32_t len,
  * the mappings by calling lm_unmap_sg.
  *
  * @lm_ctx: the libmuser context
- * @prot: protection flags, defined as PROT_XXX in <sys/mman.h>
  * @sg: array of scatter/gather entries returned by lm_addr_to_sg
  * @iov: array of iovec structures (defined in <sys/uio.h>) to receive each
  *       mapping
@@ -398,7 +397,7 @@ lm_addr_to_sg(lm_ctx_t *lm_ctx, dma_addr_t dma_addr, uint32_t len,
  * @returns 0 on success, -1 on failure
  */
 int
-lm_map_sg(lm_ctx_t *lm_ctx, int prot, const dma_sg_t *sg,
+lm_map_sg(lm_ctx_t *lm_ctx, const dma_sg_t *sg,
           struct iovec *iov, int cnt);
 
 /**