Inter-revision diff: patch 6

Comparing v12 (message) to v7 (message)

--- v12
+++ v7
@@ -1,256 +1,38 @@
-This moves page pinning (get_user_pages_fast()/put_page()) code out of
-the platform IOMMU code and puts it to VFIO IOMMU driver where it belongs
-to as the platform code does not deal with page pinning.
+At the moment DMA map/unmap requests are handled irrespective to
+the container's state. This allows the user space to pin memory which
+it might not be allowed to pin.
 
-This makes iommu_take_ownership()/iommu_release_ownership() deal with
-the IOMMU table bitmap only.
-
-This removes page unpinning from iommu_take_ownership() as the actual
-TCE table might contain garbage and doing put_page() on it is undefined
-behaviour.
-
-Besides the last part, the rest of the patch is mechanical.
+This adds checks to MAP/UNMAP that the container is enabled, otherwise
+-EPERM is returned.
 
 Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
-[aw: for the vfio related changes]
-Acked-by: Alex Williamson <alex.williamson@redhat.com>
-Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
-Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
 ---
-Changes:
-v9:
-* added missing tce_iommu_clear call after iommu_release_ownership()
-* brought @offset (a local variable) back to make patch even more
-mechanical
+ drivers/vfio/vfio_iommu_spapr_tce.c | 6 ++++++
+ 1 file changed, 6 insertions(+)
 
-v4:
-* s/iommu_tce_build(tbl, entry + 1/iommu_tce_build(tbl, entry + i/
----
- arch/powerpc/include/asm/iommu.h    |  4 --
- arch/powerpc/kernel/iommu.c         | 55 -------------------------
- drivers/vfio/vfio_iommu_spapr_tce.c | 80 +++++++++++++++++++++++++++++++------
- 3 files changed, 67 insertions(+), 72 deletions(-)
-
-diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
-index 8353c86..e94a5e3 100644
---- a/arch/powerpc/include/asm/iommu.h
-+++ b/arch/powerpc/include/asm/iommu.h
-@@ -194,10 +194,6 @@ extern int iommu_tce_build(struct iommu_table *tbl, unsigned long entry,
- 		unsigned long hwaddr, enum dma_data_direction direction);
- extern unsigned long iommu_clear_tce(struct iommu_table *tbl,
- 		unsigned long entry);
--extern int iommu_clear_tces_and_put_pages(struct iommu_table *tbl,
--		unsigned long entry, unsigned long pages);
--extern int iommu_put_tce_user_mode(struct iommu_table *tbl,
--		unsigned long entry, unsigned long tce);
- 
- extern void iommu_flush_tce(struct iommu_table *tbl);
- extern int iommu_take_ownership(struct iommu_table *tbl);
-diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
-index 73eb39a..0019c80 100644
---- a/arch/powerpc/kernel/iommu.c
-+++ b/arch/powerpc/kernel/iommu.c
-@@ -986,30 +986,6 @@ unsigned long iommu_clear_tce(struct iommu_table *tbl, unsigned long entry)
- }
- EXPORT_SYMBOL_GPL(iommu_clear_tce);
- 
--int iommu_clear_tces_and_put_pages(struct iommu_table *tbl,
--		unsigned long entry, unsigned long pages)
--{
--	unsigned long oldtce;
--	struct page *page;
--
--	for ( ; pages; --pages, ++entry) {
--		oldtce = iommu_clear_tce(tbl, entry);
--		if (!oldtce)
--			continue;
--
--		page = pfn_to_page(oldtce >> PAGE_SHIFT);
--		WARN_ON(!page);
--		if (page) {
--			if (oldtce & TCE_PCI_WRITE)
--				SetPageDirty(page);
--			put_page(page);
--		}
--	}
--
--	return 0;
--}
--EXPORT_SYMBOL_GPL(iommu_clear_tces_and_put_pages);
--
- /*
-  * hwaddr is a kernel virtual address here (0xc... bazillion),
-  * tce_build converts it to a physical address.
-@@ -1039,35 +1015,6 @@ int iommu_tce_build(struct iommu_table *tbl, unsigned long entry,
- }
- EXPORT_SYMBOL_GPL(iommu_tce_build);
- 
--int iommu_put_tce_user_mode(struct iommu_table *tbl, unsigned long entry,
--		unsigned long tce)
--{
--	int ret;
--	struct page *page = NULL;
--	unsigned long hwaddr, offset = tce & IOMMU_PAGE_MASK(tbl) & ~PAGE_MASK;
--	enum dma_data_direction direction = iommu_tce_direction(tce);
--
--	ret = get_user_pages_fast(tce & PAGE_MASK, 1,
--			direction != DMA_TO_DEVICE, &page);
--	if (unlikely(ret != 1)) {
--		/* pr_err("iommu_tce: get_user_pages_fast failed tce=%lx ioba=%lx ret=%d\n",
--				tce, entry << tbl->it_page_shift, ret); */
--		return -EFAULT;
--	}
--	hwaddr = (unsigned long) page_address(page) + offset;
--
--	ret = iommu_tce_build(tbl, entry, hwaddr, direction);
--	if (ret)
--		put_page(page);
--
--	if (ret < 0)
--		pr_err("iommu_tce: %s failed ioba=%lx, tce=%lx, ret=%d\n",
--			__func__, entry << tbl->it_page_shift, tce, ret);
--
--	return ret;
--}
--EXPORT_SYMBOL_GPL(iommu_put_tce_user_mode);
--
- int iommu_take_ownership(struct iommu_table *tbl)
- {
- 	unsigned long sz = (tbl->it_size + 7) >> 3;
-@@ -1081,7 +1028,6 @@ int iommu_take_ownership(struct iommu_table *tbl)
- 	}
- 
- 	memset(tbl->it_map, 0xff, sz);
--	iommu_clear_tces_and_put_pages(tbl, tbl->it_offset, tbl->it_size);
- 
- 	/*
- 	 * Disable iommu bypass, otherwise the user can DMA to all of
-@@ -1099,7 +1045,6 @@ void iommu_release_ownership(struct iommu_table *tbl)
- {
- 	unsigned long sz = (tbl->it_size + 7) >> 3;
- 
--	iommu_clear_tces_and_put_pages(tbl, tbl->it_offset, tbl->it_size);
- 	memset(tbl->it_map, 0, sz);
- 
- 	/* Restore bit#0 set by iommu_init_table() */
 diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
-index 730b4ef..b95fa2b 100644
+index 9448e39..c137bb3 100644
 --- a/drivers/vfio/vfio_iommu_spapr_tce.c
 +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
-@@ -147,6 +147,67 @@ static void tce_iommu_release(void *iommu_data)
- 	kfree(container);
- }
+@@ -325,6 +325,9 @@ static long tce_iommu_ioctl(void *iommu_data,
+ 		struct iommu_table *tbl = container->tbl;
+ 		unsigned long tce;
  
-+static int tce_iommu_clear(struct tce_container *container,
-+		struct iommu_table *tbl,
-+		unsigned long entry, unsigned long pages)
-+{
-+	unsigned long oldtce;
-+	struct page *page;
++		if (!container->enabled)
++			return -EPERM;
 +
-+	for ( ; pages; --pages, ++entry) {
-+		oldtce = iommu_clear_tce(tbl, entry);
-+		if (!oldtce)
-+			continue;
-+
-+		page = pfn_to_page(oldtce >> PAGE_SHIFT);
-+		WARN_ON(!page);
-+		if (page) {
-+			if (oldtce & TCE_PCI_WRITE)
-+				SetPageDirty(page);
-+			put_page(page);
-+		}
-+	}
-+
-+	return 0;
-+}
-+
-+static long tce_iommu_build(struct tce_container *container,
-+		struct iommu_table *tbl,
-+		unsigned long entry, unsigned long tce, unsigned long pages)
-+{
-+	long i, ret = 0;
-+	struct page *page = NULL;
-+	unsigned long hva;
-+	enum dma_data_direction direction = iommu_tce_direction(tce);
-+
-+	for (i = 0; i < pages; ++i) {
-+		unsigned long offset = tce & IOMMU_PAGE_MASK(tbl) & ~PAGE_MASK;
-+
-+		ret = get_user_pages_fast(tce & PAGE_MASK, 1,
-+				direction != DMA_TO_DEVICE, &page);
-+		if (unlikely(ret != 1)) {
-+			ret = -EFAULT;
-+			break;
-+		}
-+		hva = (unsigned long) page_address(page) + offset;
-+
-+		ret = iommu_tce_build(tbl, entry + i, hva, direction);
-+		if (ret) {
-+			put_page(page);
-+			pr_err("iommu_tce: %s failed ioba=%lx, tce=%lx, ret=%ld\n",
-+					__func__, entry << tbl->it_page_shift,
-+					tce, ret);
-+			break;
-+		}
-+		tce += IOMMU_PAGE_SIZE_4K;
-+	}
-+
-+	if (ret)
-+		tce_iommu_clear(container, tbl, entry, i);
-+
-+	return ret;
-+}
-+
- static long tce_iommu_ioctl(void *iommu_data,
- 				 unsigned int cmd, unsigned long arg)
- {
-@@ -195,7 +256,7 @@ static long tce_iommu_ioctl(void *iommu_data,
- 	case VFIO_IOMMU_MAP_DMA: {
- 		struct vfio_iommu_type1_dma_map param;
- 		struct iommu_table *tbl = container->tbl;
--		unsigned long tce, i;
-+		unsigned long tce;
- 
  		if (!tbl)
  			return -ENXIO;
-@@ -229,17 +290,9 @@ static long tce_iommu_ioctl(void *iommu_data,
- 		if (ret)
- 			return ret;
  
--		for (i = 0; i < (param.size >> IOMMU_PAGE_SHIFT_4K); ++i) {
--			ret = iommu_put_tce_user_mode(tbl,
--					(param.iova >> IOMMU_PAGE_SHIFT_4K) + i,
--					tce);
--			if (ret)
--				break;
--			tce += IOMMU_PAGE_SIZE_4K;
--		}
--		if (ret)
--			iommu_clear_tces_and_put_pages(tbl,
--					param.iova >> IOMMU_PAGE_SHIFT_4K, i);
-+		ret = tce_iommu_build(container, tbl,
-+				param.iova >> IOMMU_PAGE_SHIFT_4K,
-+				tce, param.size >> IOMMU_PAGE_SHIFT_4K);
+@@ -369,6 +372,9 @@ static long tce_iommu_ioctl(void *iommu_data,
+ 		struct vfio_iommu_type1_dma_unmap param;
+ 		struct iommu_table *tbl = container->tbl;
  
- 		iommu_flush_tce(tbl);
++		if (!container->enabled)
++			return -EPERM;
++
+ 		if (WARN_ON(!tbl))
+ 			return -ENXIO;
  
-@@ -273,7 +326,7 @@ static long tce_iommu_ioctl(void *iommu_data,
- 		if (ret)
- 			return ret;
- 
--		ret = iommu_clear_tces_and_put_pages(tbl,
-+		ret = tce_iommu_clear(container, tbl,
- 				param.iova >> IOMMU_PAGE_SHIFT_4K,
- 				param.size >> IOMMU_PAGE_SHIFT_4K);
- 		iommu_flush_tce(tbl);
-@@ -357,6 +410,7 @@ static void tce_iommu_detach_group(void *iommu_data,
- 		/* pr_debug("tce_vfio: detaching group #%u from iommu %p\n",
- 				iommu_group_id(iommu_group), iommu_group); */
- 		container->tbl = NULL;
-+		tce_iommu_clear(container, tbl, tbl->it_offset, tbl->it_size);
- 		iommu_release_ownership(tbl);
- 	}
- 	mutex_unlock(&container->lock);
 -- 
-2.4.0.rc3.8.gfb3e7d5
+2.0.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help