swiotlb: Disable swiotlb overflow support when CONFIG_ISA is disabled

Disable iotlb overflow support when CONFIG_ISA is disabled. Add deprecation

notice warning message and deprecation schedule documentation. This is the first step towards removing overflow support, to be consistent with other iommu implementations and return DMA_ERROR_CODE. This disabling step is for finding drivers that don't call dma_mapping_error to check for errors returned by the mapping interface. Once drivers are fixed overflow support can be removed.

Signed-off-by Shuah Khan <[email protected]>

---
 Documentation/feature-removal-schedule.txt |   22 ++++++--
 lib/swiotlb.c                              |   79 +++++++++++++++++++++-------
 2 files changed, 79 insertions(+), 22 deletions(-)

Documentation/feature-removal-schedule.txt +0 -0

index 5979c3e..ce6c0ae 100644

Documentation/feature-removal-schedule.txt +16 -0

 	So it _could_ be removed _any_ time now, if it conflicts with something new.
 Why:	The support code for the old firmware hurts code readability/maintainability
 	and slightly hurts runtime performance. Bugfixes for the old firmware

@@ -608,3 +608,19 @@ When:	June 2013
 Why:	Unsupported/unmaintained/unused since 2.6
 
 ----------------------------
+
+What:	SWIOTLB overflow buffer support.
+When:	3.8
+Why:	Remove SWIOTLB overflow buffer support and return DMA_ERROR_CODE
+	(a value of zero) to make it consistent with iommu implementation
+	on Intel, AMD, and swiotlb-xen. In 3.6, Disable iotlb overflow
+	support when CONFIG_ISA is disabled with the intent to find drivers
+	that don't call dma_mapping_error to check for errors returned by the
+	mapping interface. Once drivers are fixed overflow support can be
+	removed.
+	If you see any problems related to disabling SWIOTLB overflow buffer,
+	please report to us!
+	E-mail us at: [email protected]
+Who:	Shuah Khan <[email protected]> <[email protected]>
+
+----------------------------

lib/swiotlb.c +0 -0

index 45bc1f8..0123bb8 100644

lib/swiotlb.c +57 -19

  */
 
 #include <linux/cache.h>

@@ -68,7 +71,11 @@ static unsigned long io_tlb_nslabs;
 /*
  * When the IOMMU overflows we return a fallback buffer. This sets the size.
  */
+#if defined(CONFIG_ISA)
 static unsigned long io_tlb_overflow = 32*1024;
+#else
+static unsigned long io_tlb_overflow;
+#endif
 
 static void *io_tlb_overflow_buffer;
 

@@ -92,6 +99,24 @@ static DEFINE_SPINLOCK(io_tlb_lock);
 
 static int late_alloc;
 
+static void swiotlb_print_overflow_deprecation_notice(void)
+{
+	if (io_tlb_overflow) {
+		pr_warn("SWIOTLB overflow buffer will be deprecated.\n"
+			"  If you have a driver that depends on this feature\n"
+			"  please Email us at: [email protected],\n"
+			"  Shuah Khan ([email protected]), and\n"
+			"  Konrad Wilk ([email protected])\n");
+	} else {
+		pr_warn("SWIOTLB overflow buffer is disabled and will be\n"
+			"  deprecated. Please report problems related to\n"
+			"  disabling overflow buffer to\n"
+			"  [email protected],\n"
+			"  Shuah Khan ([email protected]), and\n"
+			"  Konrad Wilk ([email protected])\n");
+	}
+}
+
 static int __init
 setup_io_tlb_npages(char *str)
 {

@@ -108,7 +133,6 @@ setup_io_tlb_npages(char *str)
 	return 1;
 }
 __setup("swiotlb=", setup_io_tlb_npages);
-/* make io_tlb_overflow tunable too? */
 
 unsigned long swiotlb_nr_tbl(void)
 {

@@ -156,12 +180,18 @@ void __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
 	io_tlb_index = 0;
 	io_tlb_orig_addr = alloc_bootmem_pages(PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t)));
 
-	/*
-	 * Get the overflow emergency buffer
-	 */
-	io_tlb_overflow_buffer = alloc_bootmem_low_pages(PAGE_ALIGN(io_tlb_overflow));
-	if (!io_tlb_overflow_buffer)
-		panic("Cannot allocate SWIOTLB overflow buffer!\n");
+	if (io_tlb_overflow) {
+		/*
+		 * Get the overflow emergency buffer
+		 */
+		io_tlb_overflow_buffer = alloc_bootmem_low_pages(
+						PAGE_ALIGN(io_tlb_overflow));
+		if (!io_tlb_overflow_buffer)
+			panic("Cannot allocate SWIOTLB overflow buffer!\n");
+	}
+
+	swiotlb_print_overflow_deprecation_notice();
+
 	if (verbose)
 		swiotlb_print_info();
 }

@@ -264,14 +294,17 @@ swiotlb_late_init_with_default_size(size_t default_size)
 
 	memset(io_tlb_orig_addr, 0, io_tlb_nslabs * sizeof(phys_addr_t));
 
-	/*
-	 * Get the overflow emergency buffer
-	 */
-	io_tlb_overflow_buffer = (void *)__get_free_pages(GFP_DMA,
-	                                          get_order(io_tlb_overflow));
-	if (!io_tlb_overflow_buffer)
-		goto cleanup4;
+	if (io_tlb_overflow) {
+		/*
+		 * Get the overflow emergency buffer
+		 */
+		io_tlb_overflow_buffer = (void *)
+			__get_free_pages(GFP_DMA, get_order(io_tlb_overflow));
+		if (!io_tlb_overflow_buffer)
+			goto cleanup4;
+	}
 
+	swiotlb_print_overflow_deprecation_notice();
 	swiotlb_print_info();
 
 	late_alloc = 1;

@@ -297,12 +330,13 @@ cleanup1:
 
 void __init swiotlb_free(void)
 {
-	if (!io_tlb_overflow_buffer)
+	if (!io_tlb_orig_addr)
 		return;
 
 	if (late_alloc) {
-		free_pages((unsigned long)io_tlb_overflow_buffer,
-			   get_order(io_tlb_overflow));
+		if (io_tlb_overflow_buffer)
+			free_pages((unsigned long)io_tlb_overflow_buffer,
+				   get_order(io_tlb_overflow));
 		free_pages((unsigned long)io_tlb_orig_addr,
 			   get_order(io_tlb_nslabs * sizeof(phys_addr_t)));
 		free_pages((unsigned long)io_tlb_list, get_order(io_tlb_nslabs *

@@ -310,8 +344,9 @@ void __init swiotlb_free(void)
 		free_pages((unsigned long)io_tlb_start,
 			   get_order(io_tlb_nslabs << IO_TLB_SHIFT));
 	} else {
-		free_bootmem_late(__pa(io_tlb_overflow_buffer),
-				  PAGE_ALIGN(io_tlb_overflow));
+		if (io_tlb_overflow_buffer)
+			free_bootmem_late(__pa(io_tlb_overflow_buffer),
+					  PAGE_ALIGN(io_tlb_overflow));
 		free_bootmem_late(__pa(io_tlb_orig_addr),
 				  PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t)));
 		free_bootmem_late(__pa(io_tlb_list),

@@ -681,6 +716,8 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 	map = map_single(dev, phys, size, dir);
 	if (!map) {
 		swiotlb_full(dev, size, dir, 1);
+		if (!io_tlb_overflow)
+			return DMA_ERROR_CODE;
 		map = io_tlb_overflow_buffer;
 	}
 

@@ -691,6 +728,8 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 	 */
 	if (!dma_capable(dev, dev_addr, size)) {
 		swiotlb_tbl_unmap_single(dev, map, size, dir);
+		if (!io_tlb_overflow)
+			return DMA_ERROR_CODE;
 		dev_addr = swiotlb_virt_to_bus(dev, io_tlb_overflow_buffer);
 	}
 

@@ -910,6 +949,8 @@ EXPORT_SYMBOL(swiotlb_sync_sg_for_device);
 int
 swiotlb_dma_mapping_error(struct device *hwdev, dma_addr_t dma_addr)
 {
+	if (!io_tlb_overflow)
+		return DMA_ERROR_CODE;
 	return (dma_addr == swiotlb_virt_to_bus(hwdev, io_tlb_overflow_buffer));
 }
 EXPORT_SYMBOL(swiotlb_dma_mapping_error);

Signature · 9 lines

1.7.9.5



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Quoted 1 line · Shuah Khan

Disable iotlb overflow support when CONFIG_ISA is disabled. Add deprecation

You need to check one more thing. In the email I mentioned that the bulk of the drivers that utilize this are ISA, but there are also some that are PCI dependent.

So I grepped for anything that does 'dma_map_page' and some of them are even PCIe, for example: drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c

Which means that the check for CONFIG_ISA is not sufficient.

Quoted 9 lines

notice warning message and deprecation schedule documentation. This is the first step towards removing overflow support, to be consistent with other iommu implementations and return DMA_ERROR_CODE. This disabling step is for finding drivers that don't call dma_mapping_error to check for errors returned by the mapping interface. Once drivers are fixed overflow support can be removed.

Signed-off-by Shuah Khan <[email protected]>

Did you do a cross compile on IA64 just to double check? Comments below.

Quoted 10 lines

---
 Documentation/feature-removal-schedule.txt |   22 ++++++--
 lib/swiotlb.c                              |   79 +++++++++++++++++++++-------
 2 files changed, 79 insertions(+), 22 deletions(-)

Documentation/feature-removal-schedule.txt +0 -0

index 5979c3e..ce6c0ae 100644

Documentation/feature-removal-schedule.txt What: x86_32 "no-hlt" cmdline param · line 38

Quoted 5 lines

Documentation/feature-removal-schedule.txt +1 -1

@@ -35,7 +35,7 @@ What:	x86_32 "no-hlt" cmdline param
 When:	2012
 Why:	remove a branch from idle path, simplify code used by everybody.
 	This option disabled the use of HLT in idle and machine_halt()
-	for hardware that was flakey 15-years ago.  Today we have
+	for hardware that was flaky 15-years ago.  Today we have

Ummm, that is not part of what this patch should be doing.

Documentation/feature-removal-schedule.txt Files: arch/*/kernel/*_ksyms.c · line 163

Quoted 9 lines

Documentation/feature-removal-schedule.txt +1 -1

 	"idle=poll" that removed HLT from idle, and so if such a machine
 	is still running the upstream kernel, "idle=poll" is likely sufficient.
 Who:	Len Brown <[email protected]>

@@ -160,7 +160,7 @@ Files:	arch/*/kernel/*_ksyms.c
 Check:	kernel_thread
 Why:	kernel_thread is a low-level implementation detail.  Drivers should
         use the <linux/kthread.h> API instead which shields them from
-	implementation details and provides a higherlevel interface that
+	implementation details and provides a higher level interface that

Neither is this.

Documentation/feature-removal-schedule.txt Who: David Brownell <[email protected]> · line 239

Quoted 9 lines

Documentation/feature-removal-schedule.txt +1 -1

 	prevents bugs and code duplication
 Who:	Christoph Hellwig <[email protected]>
 

@@ -236,7 +236,7 @@ Who:	David Brownell <[email protected]>
 
 What:	b43 support for firmware revision < 410
 When:	The schedule was July 2008, but it was decided that we are going to keep the
-        code as long as there are no major maintanance headaches.
+        code as long as there are no major maintenance headaches.

Or this.

Quoted 28 lines

Documentation/feature-removal-schedule.txt +16 -0

 	So it _could_ be removed _any_ time now, if it conflicts with something new.
 Why:	The support code for the old firmware hurts code readability/maintainability
 	and slightly hurts runtime performance. Bugfixes for the old firmware

@@ -608,3 +608,19 @@ When:	June 2013
 Why:	Unsupported/unmaintained/unused since 2.6
 
 ----------------------------
+
+What:	SWIOTLB overflow buffer support.
+When:	3.8
+Why:	Remove SWIOTLB overflow buffer support and return DMA_ERROR_CODE
+	(a value of zero) to make it consistent with iommu implementation
+	on Intel, AMD, and swiotlb-xen. In 3.6, Disable iotlb overflow
+	support when CONFIG_ISA is disabled with the intent to find drivers
+	that don't call dma_mapping_error to check for errors returned by the
+	mapping interface. Once drivers are fixed overflow support can be
+	removed.
+	If you see any problems related to disabling SWIOTLB overflow buffer,
+	please report to us!
+	E-mail us at: [email protected]
+Who:	Shuah Khan <[email protected]> <[email protected]>
+
+----------------------------

lib/swiotlb.c +0 -0

index 45bc1f8..0123bb8 100644

lib/swiotlb.c line 20

Quoted 6 lines

lib/swiotlb.c +3 -0

@@ -15,6 +15,9 @@
  * 05/09/10 linville	Add support for syncing ranges, support syncing for
  *			DMA_BIDIRECTIONAL mappings, miscellaneous cleanup.
  * 08/12/11 beckyb	Add highmem support
+ * 07/2012  shuahkhan	Disable iotlb overflow support when CONFIG_ISA
+ *			is enabled. Remove it for all configs when drivers
+ *			that don't check for mapping errors are fixed.

Don't bother. The changelog is now in the git commit.

Quoted 164 lines

lib/swiotlb.c +57 -19

  */
 
 #include <linux/cache.h>

@@ -68,7 +71,11 @@ static unsigned long io_tlb_nslabs;
 /*
  * When the IOMMU overflows we return a fallback buffer. This sets the size.
  */
+#if defined(CONFIG_ISA)
 static unsigned long io_tlb_overflow = 32*1024;
+#else
+static unsigned long io_tlb_overflow;
+#endif
 
 static void *io_tlb_overflow_buffer;
 

@@ -92,6 +99,24 @@ static DEFINE_SPINLOCK(io_tlb_lock);
 
 static int late_alloc;
 
+static void swiotlb_print_overflow_deprecation_notice(void)
+{
+	if (io_tlb_overflow) {
+		pr_warn("SWIOTLB overflow buffer will be deprecated.\n"
+			"  If you have a driver that depends on this feature\n"
+			"  please Email us at: [email protected],\n"
+			"  Shuah Khan ([email protected]), and\n"
+			"  Konrad Wilk ([email protected])\n");
+	} else {
+		pr_warn("SWIOTLB overflow buffer is disabled and will be\n"
+			"  deprecated. Please report problems related to\n"
+			"  disabling overflow buffer to\n"
+			"  [email protected],\n"
+			"  Shuah Khan ([email protected]), and\n"
+			"  Konrad Wilk ([email protected])\n");
+	}
+}
+
 static int __init
 setup_io_tlb_npages(char *str)
 {

@@ -108,7 +133,6 @@ setup_io_tlb_npages(char *str)
 	return 1;
 }
 __setup("swiotlb=", setup_io_tlb_npages);
-/* make io_tlb_overflow tunable too? */
 
 unsigned long swiotlb_nr_tbl(void)
 {

@@ -156,12 +180,18 @@ void __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
 	io_tlb_index = 0;
 	io_tlb_orig_addr = alloc_bootmem_pages(PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t)));
 
-	/*
-	 * Get the overflow emergency buffer
-	 */
-	io_tlb_overflow_buffer = alloc_bootmem_low_pages(PAGE_ALIGN(io_tlb_overflow));
-	if (!io_tlb_overflow_buffer)
-		panic("Cannot allocate SWIOTLB overflow buffer!\n");
+	if (io_tlb_overflow) {
+		/*
+		 * Get the overflow emergency buffer
+		 */
+		io_tlb_overflow_buffer = alloc_bootmem_low_pages(
+						PAGE_ALIGN(io_tlb_overflow));
+		if (!io_tlb_overflow_buffer)
+			panic("Cannot allocate SWIOTLB overflow buffer!\n");
+	}
+
+	swiotlb_print_overflow_deprecation_notice();
+
 	if (verbose)
 		swiotlb_print_info();
 }

@@ -264,14 +294,17 @@ swiotlb_late_init_with_default_size(size_t default_size)
 
 	memset(io_tlb_orig_addr, 0, io_tlb_nslabs * sizeof(phys_addr_t));
 
-	/*
-	 * Get the overflow emergency buffer
-	 */
-	io_tlb_overflow_buffer = (void *)__get_free_pages(GFP_DMA,
-	                                          get_order(io_tlb_overflow));
-	if (!io_tlb_overflow_buffer)
-		goto cleanup4;
+	if (io_tlb_overflow) {
+		/*
+		 * Get the overflow emergency buffer
+		 */
+		io_tlb_overflow_buffer = (void *)
+			__get_free_pages(GFP_DMA, get_order(io_tlb_overflow));
+		if (!io_tlb_overflow_buffer)
+			goto cleanup4;
+	}
 
+	swiotlb_print_overflow_deprecation_notice();
 	swiotlb_print_info();
 
 	late_alloc = 1;

@@ -297,12 +330,13 @@ cleanup1:
 
 void __init swiotlb_free(void)
 {
-	if (!io_tlb_overflow_buffer)
+	if (!io_tlb_orig_addr)
 		return;
 
 	if (late_alloc) {
-		free_pages((unsigned long)io_tlb_overflow_buffer,
-			   get_order(io_tlb_overflow));
+		if (io_tlb_overflow_buffer)
+			free_pages((unsigned long)io_tlb_overflow_buffer,
+				   get_order(io_tlb_overflow));
 		free_pages((unsigned long)io_tlb_orig_addr,
 			   get_order(io_tlb_nslabs * sizeof(phys_addr_t)));
 		free_pages((unsigned long)io_tlb_list, get_order(io_tlb_nslabs *

@@ -310,8 +344,9 @@ void __init swiotlb_free(void)
 		free_pages((unsigned long)io_tlb_start,
 			   get_order(io_tlb_nslabs << IO_TLB_SHIFT));
 	} else {
-		free_bootmem_late(__pa(io_tlb_overflow_buffer),
-				  PAGE_ALIGN(io_tlb_overflow));
+		if (io_tlb_overflow_buffer)
+			free_bootmem_late(__pa(io_tlb_overflow_buffer),
+					  PAGE_ALIGN(io_tlb_overflow));
 		free_bootmem_late(__pa(io_tlb_orig_addr),
 				  PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t)));
 		free_bootmem_late(__pa(io_tlb_list),

@@ -681,6 +716,8 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 	map = map_single(dev, phys, size, dir);
 	if (!map) {
 		swiotlb_full(dev, size, dir, 1);
+		if (!io_tlb_overflow)
+			return DMA_ERROR_CODE;
 		map = io_tlb_overflow_buffer;
 	}
 

@@ -691,6 +728,8 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 	 */
 	if (!dma_capable(dev, dev_addr, size)) {
 		swiotlb_tbl_unmap_single(dev, map, size, dir);
+		if (!io_tlb_overflow)
+			return DMA_ERROR_CODE;
 		dev_addr = swiotlb_virt_to_bus(dev, io_tlb_overflow_buffer);
 	}
 

@@ -910,6 +949,8 @@ EXPORT_SYMBOL(swiotlb_sync_sg_for_device);
 int
 swiotlb_dma_mapping_error(struct device *hwdev, dma_addr_t dma_addr)
 {
+	if (!io_tlb_overflow)
+		return DMA_ERROR_CODE;
 	return (dma_addr == swiotlb_virt_to_bus(hwdev, io_tlb_overflow_buffer));
 }
 EXPORT_SYMBOL(swiotlb_dma_mapping_error);

Signature · 9 lines

1.7.9.5



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/