* [PATCH RFC] swiotlb: Remove SWIOTLB overflow buffer support @ 2012-07-06 23:06 Shuah Khan 2012-07-09 20:25 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 14+ messages in thread From: Shuah Khan @ 2012-07-06 23:06 UTC (permalink / raw) To: LKML; +Cc: shuahkhan, akpm, paul.gortmaker, konrad.wilk, bhelgaas, amwang 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. Tested only on x86. Signed-off-by: Shuah Khan <shuah.khan@hp.com> --- lib/swiotlb.c | 44 ++++++++------------------------------------ 1 file changed, 8 insertions(+), 36 deletions(-) diff --git a/lib/swiotlb.c b/lib/swiotlb.c index 45bc1f8..7f0a5d1 100644 --- a/lib/swiotlb.c +++ b/lib/swiotlb.c @@ -15,6 +15,7 @@ * 05/09/10 linville Add support for syncing ranges, support syncing for * DMA_BIDIRECTIONAL mappings, miscellaneous cleanup. * 08/12/11 beckyb Add highmem support + * 06/12 shuahkhan Remove io tlb overflow support */ #include <linux/cache.h> @@ -66,13 +67,6 @@ static char *io_tlb_start, *io_tlb_end; static unsigned long io_tlb_nslabs; /* - * When the IOMMU overflows we return a fallback buffer. This sets the size. - */ -static unsigned long io_tlb_overflow = 32*1024; - -static void *io_tlb_overflow_buffer; - -/* * This is a free list describing the number of free entries available from * each index */ @@ -108,7 +102,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 +149,6 @@ 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 (verbose) swiotlb_print_info(); } @@ -195,7 +182,8 @@ swiotlb_init_with_default_size(size_t default_size, int verbose) void __init swiotlb_init(int verbose) { - swiotlb_init_with_default_size(64 * (1<<20), verbose); /* default to 64MB */ + /* default to 64MB */ + swiotlb_init_with_default_size(64 * (1<<20), verbose); } /* @@ -264,24 +252,12 @@ 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; - swiotlb_print_info(); late_alloc = 1; return 0; -cleanup4: - free_pages((unsigned long)io_tlb_orig_addr, - get_order(io_tlb_nslabs * sizeof(phys_addr_t))); - io_tlb_orig_addr = NULL; cleanup3: free_pages((unsigned long)io_tlb_list, get_order(io_tlb_nslabs * sizeof(int))); @@ -297,12 +273,10 @@ 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)); 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 +284,6 @@ 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)); free_bootmem_late(__pa(io_tlb_orig_addr), PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t))); free_bootmem_late(__pa(io_tlb_list), @@ -639,7 +611,7 @@ swiotlb_full(struct device *dev, size_t size, enum dma_data_direction dir, printk(KERN_ERR "DMA: Out of SW-IOMMU space for %zu bytes at " "device %s\n", size, dev ? dev_name(dev) : "?"); - if (size <= io_tlb_overflow || !do_panic) + if (!do_panic) return; if (dir == DMA_BIDIRECTIONAL) @@ -681,7 +653,7 @@ 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); - map = io_tlb_overflow_buffer; + return DMA_ERROR_CODE; } dev_addr = swiotlb_virt_to_bus(dev, map); @@ -691,7 +663,7 @@ 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); - dev_addr = swiotlb_virt_to_bus(dev, io_tlb_overflow_buffer); + dev_addr = 0; } return dev_addr; @@ -910,7 +882,7 @@ EXPORT_SYMBOL(swiotlb_sync_sg_for_device); int swiotlb_dma_mapping_error(struct device *hwdev, dma_addr_t dma_addr) { - return (dma_addr == swiotlb_virt_to_bus(hwdev, io_tlb_overflow_buffer)); + return !dma_addr; } EXPORT_SYMBOL(swiotlb_dma_mapping_error); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH RFC] swiotlb: Remove SWIOTLB overflow buffer support 2012-07-06 23:06 [PATCH RFC] swiotlb: Remove SWIOTLB overflow buffer support Shuah Khan @ 2012-07-09 20:25 ` Konrad Rzeszutek Wilk 2012-07-10 16:33 ` FUJITA Tomonori 2012-07-10 16:55 ` Shuah Khan 0 siblings, 2 replies; 14+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-07-09 20:25 UTC (permalink / raw) To: Shuah Khan, fujita.tomonori Cc: LKML, shuahkhan, akpm, paul.gortmaker, bhelgaas, amwang On Fri, Jul 06, 2012 at 05:06:12PM -0600, Shuah Khan wrote: > 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. While this is a good forward step and this needs to be done eventually, you should first send out patches for the drivers that don't check for the DMA_ERROR_CODE when doing mapping. In other words for the drivers that map but don't call dma_mapping_error to check. When that is fixed and *all the drivers that don't call dma_mapping_error are fixed, then this patch makes sense. So for right now, NACK. Also you missed CC-ing fujita.tomonori@lab.ntt.co.jp, so let me add that to this email. *: for those that it makes sense. I am not sure if folks are still using ancient drivers.. > > Tested only on x86. > > Signed-off-by: Shuah Khan <shuah.khan@hp.com> > --- > lib/swiotlb.c | 44 ++++++++------------------------------------ > 1 file changed, 8 insertions(+), 36 deletions(-) > > diff --git a/lib/swiotlb.c b/lib/swiotlb.c > index 45bc1f8..7f0a5d1 100644 > --- a/lib/swiotlb.c > +++ b/lib/swiotlb.c > @@ -15,6 +15,7 @@ > * 05/09/10 linville Add support for syncing ranges, support syncing for > * DMA_BIDIRECTIONAL mappings, miscellaneous cleanup. > * 08/12/11 beckyb Add highmem support > + * 06/12 shuahkhan Remove io tlb overflow support > */ > > #include <linux/cache.h> > @@ -66,13 +67,6 @@ static char *io_tlb_start, *io_tlb_end; > static unsigned long io_tlb_nslabs; > > /* > - * When the IOMMU overflows we return a fallback buffer. This sets the size. > - */ > -static unsigned long io_tlb_overflow = 32*1024; > - > -static void *io_tlb_overflow_buffer; > - > -/* > * This is a free list describing the number of free entries available from > * each index > */ > @@ -108,7 +102,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 +149,6 @@ 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 (verbose) > swiotlb_print_info(); > } > @@ -195,7 +182,8 @@ swiotlb_init_with_default_size(size_t default_size, int verbose) > void __init > swiotlb_init(int verbose) > { > - swiotlb_init_with_default_size(64 * (1<<20), verbose); /* default to 64MB */ > + /* default to 64MB */ > + swiotlb_init_with_default_size(64 * (1<<20), verbose); > } > > /* > @@ -264,24 +252,12 @@ 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; > - > swiotlb_print_info(); > > late_alloc = 1; > > return 0; > > -cleanup4: > - free_pages((unsigned long)io_tlb_orig_addr, > - get_order(io_tlb_nslabs * sizeof(phys_addr_t))); > - io_tlb_orig_addr = NULL; > cleanup3: > free_pages((unsigned long)io_tlb_list, get_order(io_tlb_nslabs * > sizeof(int))); > @@ -297,12 +273,10 @@ 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)); > 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 +284,6 @@ 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)); > free_bootmem_late(__pa(io_tlb_orig_addr), > PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t))); > free_bootmem_late(__pa(io_tlb_list), > @@ -639,7 +611,7 @@ swiotlb_full(struct device *dev, size_t size, enum dma_data_direction dir, > printk(KERN_ERR "DMA: Out of SW-IOMMU space for %zu bytes at " > "device %s\n", size, dev ? dev_name(dev) : "?"); > > - if (size <= io_tlb_overflow || !do_panic) > + if (!do_panic) > return; > > if (dir == DMA_BIDIRECTIONAL) > @@ -681,7 +653,7 @@ 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); > - map = io_tlb_overflow_buffer; > + return DMA_ERROR_CODE; > } > > dev_addr = swiotlb_virt_to_bus(dev, map); > @@ -691,7 +663,7 @@ 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); > - dev_addr = swiotlb_virt_to_bus(dev, io_tlb_overflow_buffer); > + dev_addr = 0; > } > > return dev_addr; > @@ -910,7 +882,7 @@ EXPORT_SYMBOL(swiotlb_sync_sg_for_device); > int > swiotlb_dma_mapping_error(struct device *hwdev, dma_addr_t dma_addr) > { > - return (dma_addr == swiotlb_virt_to_bus(hwdev, io_tlb_overflow_buffer)); > + return !dma_addr; > } > EXPORT_SYMBOL(swiotlb_dma_mapping_error); > > -- > 1.7.9.5 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC] swiotlb: Remove SWIOTLB overflow buffer support 2012-07-09 20:25 ` Konrad Rzeszutek Wilk @ 2012-07-10 16:33 ` FUJITA Tomonori 2012-07-10 16:55 ` Shuah Khan 1 sibling, 0 replies; 14+ messages in thread From: FUJITA Tomonori @ 2012-07-10 16:33 UTC (permalink / raw) To: konrad.wilk Cc: shuah.khan, linux-kernel, shuahkhan, akpm, paul.gortmaker, bhelgaas, amwang On Mon, 9 Jul 2012 16:25:05 -0400 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > On Fri, Jul 06, 2012 at 05:06:12PM -0600, Shuah Khan wrote: >> 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. > > While this is a good forward step and this needs to be done eventually, > you should first send out patches for the drivers that don't check > for the DMA_ERROR_CODE when doing mapping. In other words for the > drivers that map but don't call dma_mapping_error to check. > > When that is fixed and *all the drivers that don't call dma_mapping_error > are fixed, then this patch makes sense. > > So for right now, NACK. Yeah, I'm not sure we could fix (or remove) *all the drivers though. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC] swiotlb: Remove SWIOTLB overflow buffer support 2012-07-09 20:25 ` Konrad Rzeszutek Wilk 2012-07-10 16:33 ` FUJITA Tomonori @ 2012-07-10 16:55 ` Shuah Khan 2012-07-10 17:32 ` Konrad Rzeszutek Wilk 1 sibling, 1 reply; 14+ messages in thread From: Shuah Khan @ 2012-07-10 16:55 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: fujita.tomonori, LKML, akpm, paul.gortmaker, bhelgaas, amwang, shuahkhan On Mon, 2012-07-09 at 16:25 -0400, Konrad Rzeszutek Wilk wrote: > On Fri, Jul 06, 2012 at 05:06:12PM -0600, Shuah Khan wrote: > > 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. > > While this is a good forward step and this needs to be done eventually, > you should first send out patches for the drivers that don't check > for the DMA_ERROR_CODE when doing mapping. In other words for the > drivers that map but don't call dma_mapping_error to check. > > When that is fixed and *all the drivers that don't call dma_mapping_error > are fixed, then this patch makes sense. The challenge will be catching all the drivers and have confidence that all of them are covered. I will start looking into this to get a feel for how many drivers needs fixing. Also, isn't this problem today with other iommu implementations? All of the other iommu implementation don't have support for overflow buffers? -- Shuah ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC] swiotlb: Remove SWIOTLB overflow buffer support 2012-07-10 16:55 ` Shuah Khan @ 2012-07-10 17:32 ` Konrad Rzeszutek Wilk 2012-07-10 23:06 ` Shuah Khan 2012-07-12 16:17 ` [PATCH RFC] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is enabled Shuah Khan 0 siblings, 2 replies; 14+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-07-10 17:32 UTC (permalink / raw) To: Shuah Khan Cc: fujita.tomonori, LKML, akpm, paul.gortmaker, bhelgaas, amwang, shuahkhan On Tue, Jul 10, 2012 at 10:55:07AM -0600, Shuah Khan wrote: > On Mon, 2012-07-09 at 16:25 -0400, Konrad Rzeszutek Wilk wrote: > > On Fri, Jul 06, 2012 at 05:06:12PM -0600, Shuah Khan wrote: > > > 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. > > > > While this is a good forward step and this needs to be done eventually, > > you should first send out patches for the drivers that don't check > > for the DMA_ERROR_CODE when doing mapping. In other words for the > > drivers that map but don't call dma_mapping_error to check. > > > > When that is fixed and *all the drivers that don't call dma_mapping_error > > are fixed, then this patch makes sense. > > The challenge will be catching all the drivers and have confidence that > all of them are covered. I will start looking into this to get a feel > for how many drivers needs fixing. I don't know if all is needed. Some of them might be dead or not used at all anymore - who knows? This treasure hunt would give a good idea of which ones are not using the PCI/DMA API right at least. If it is say, CONFIG_ISA enabled, well, we could #ifdef the overflow buffer in the swiotlb with that option. And then work through fixing up those drivers - except that finding folks with that driver to see if it works .. yuck.. I do have some few ISA cards and some boxes with ISA slots :-) > > Also, isn't this problem today with other iommu implementations? All of > the other iommu implementation don't have support for overflow buffers? Right, but you are in high likehood not going to run the drivers that don't check this with an IOMMU. Because the drivers are mostly the PCI variants .. or PCMCIA. And the hardware that would use this did not have IOMMU. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC] swiotlb: Remove SWIOTLB overflow buffer support 2012-07-10 17:32 ` Konrad Rzeszutek Wilk @ 2012-07-10 23:06 ` Shuah Khan 2012-07-10 23:13 ` Shuah Khan 2012-07-12 16:17 ` [PATCH RFC] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is enabled Shuah Khan 1 sibling, 1 reply; 14+ messages in thread From: Shuah Khan @ 2012-07-10 23:06 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: fujita.tomonori, LKML, akpm, paul.gortmaker, bhelgaas, amwang On Tue, 2012-07-10 at 13:32 -0400, Konrad Rzeszutek Wilk wrote: > On Tue, Jul 10, 2012 at 10:55:07AM -0600, Shuah Khan wrote: > > On Mon, 2012-07-09 at 16:25 -0400, Konrad Rzeszutek Wilk wrote: > > > On Fri, Jul 06, 2012 at 05:06:12PM -0600, Shuah Khan wrote: > > > > 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. > > > > > > While this is a good forward step and this needs to be done eventually, > > > you should first send out patches for the drivers that don't check > > > for the DMA_ERROR_CODE when doing mapping. In other words for the > > > drivers that map but don't call dma_mapping_error to check. > > > > > > When that is fixed and *all the drivers that don't call dma_mapping_error > > > are fixed, then this patch makes sense. > > > > The challenge will be catching all the drivers and have confidence that > > all of them are covered. I will start looking into this to get a feel > > for how many drivers needs fixing. > > I don't know if all is needed. Some of them might be dead or not used > at all anymore - who knows? This treasure hunt would give a good idea > of which ones are not using the PCI/DMA API right at least. > > If it is say, CONFIG_ISA enabled, well, we could #ifdef the overflow > buffer in the swiotlb with that option. And then work through fixing > up those drivers - except that finding folks with that driver to > see if it works .. yuck.. I do have some few ISA cards and some boxes > with ISA slots :-) Looking at the history CONFIG_ISA was disabled back in 2002 and CONFIG_ISA_DMA_API was disabled in 2004. Sounds like it will be fun finding drivers that could fail. Would it make sense to make io_tlb_overflow a tunable and disable by default instead of ifdef? Currently it is always enabled, disabling it by default is another way to find problems? Good to know you have a few ISA cards. :) -- Shuah ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC] swiotlb: Remove SWIOTLB overflow buffer support 2012-07-10 23:06 ` Shuah Khan @ 2012-07-10 23:13 ` Shuah Khan 0 siblings, 0 replies; 14+ messages in thread From: Shuah Khan @ 2012-07-10 23:13 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: fujita.tomonori, LKML, akpm, paul.gortmaker, bhelgaas, amwang, shuahkhan On Tue, 2012-07-10 at 17:06 -0600, Shuah Khan wrote: > On Tue, 2012-07-10 at 13:32 -0400, Konrad Rzeszutek Wilk wrote: > > On Tue, Jul 10, 2012 at 10:55:07AM -0600, Shuah Khan wrote: > > > On Mon, 2012-07-09 at 16:25 -0400, Konrad Rzeszutek Wilk wrote: > > > > On Fri, Jul 06, 2012 at 05:06:12PM -0600, Shuah Khan wrote: > > > > > 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. > > > > > > > > While this is a good forward step and this needs to be done eventually, > > > > you should first send out patches for the drivers that don't check > > > > for the DMA_ERROR_CODE when doing mapping. In other words for the > > > > drivers that map but don't call dma_mapping_error to check. > > > > > > > > When that is fixed and *all the drivers that don't call dma_mapping_error > > > > are fixed, then this patch makes sense. > > > > > > The challenge will be catching all the drivers and have confidence that > > > all of them are covered. I will start looking into this to get a feel > > > for how many drivers needs fixing. > > > > I don't know if all is needed. Some of them might be dead or not used > > at all anymore - who knows? This treasure hunt would give a good idea > > of which ones are not using the PCI/DMA API right at least. > > > > If it is say, CONFIG_ISA enabled, well, we could #ifdef the overflow > > buffer in the swiotlb with that option. And then work through fixing > > up those drivers - except that finding folks with that driver to > > see if it works .. yuck.. I do have some few ISA cards and some boxes > > with ISA slots :-) > > Looking at the history CONFIG_ISA was disabled back in 2002 and > CONFIG_ISA_DMA_API was disabled in 2004. Sounds like it will be fun > finding drivers that could fail. Correction. CONFIG_ISA_DMA_API is made configurable on x86_64 it appears back in 2011. -- Shuah ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH RFC] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is enabled 2012-07-10 17:32 ` Konrad Rzeszutek Wilk 2012-07-10 23:06 ` Shuah Khan @ 2012-07-12 16:17 ` Shuah Khan 2012-07-16 14:45 ` Konrad Rzeszutek Wilk 1 sibling, 1 reply; 14+ messages in thread From: Shuah Khan @ 2012-07-12 16:17 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: fujita.tomonori, LKML, akpm, paul.gortmaker, bhelgaas, amwang, shuahkhan Disable iotlb overflow support when CONFIG_ISA is enabled. 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. Tested on x86 with io_tlb_overflow = 32*1024 and io_tlb_overflow = 0. Signed-off-by: Shuah Khan <shuah.khan@hp.com> --- lib/swiotlb.c | 65 +++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/lib/swiotlb.c b/lib/swiotlb.c index 45bc1f8..f7d285c 100644 --- a/lib/swiotlb.c +++ b/lib/swiotlb.c @@ -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. */ #include <linux/cache.h> @@ -68,7 +71,19 @@ static unsigned long io_tlb_nslabs; /* * When the IOMMU overflows we return a fallback buffer. This sets the size. */ +#if defined(CONFIG_ISA) +/* + * Disable iotlb overflow support when CONFIG_ISA is enabled. 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. +*/ +static unsigned long io_tlb_overflow; +#else static unsigned long io_tlb_overflow = 32*1024; +#endif static void *io_tlb_overflow_buffer; @@ -156,12 +171,16 @@ 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"); + } + if (verbose) swiotlb_print_info(); } @@ -264,13 +283,15 @@ 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_info(); @@ -297,12 +318,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 +332,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 +704,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 +716,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 +937,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); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH RFC] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is enabled 2012-07-12 16:17 ` [PATCH RFC] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is enabled Shuah Khan @ 2012-07-16 14:45 ` Konrad Rzeszutek Wilk 2012-07-16 15:48 ` Shuah Khan 0 siblings, 1 reply; 14+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-07-16 14:45 UTC (permalink / raw) To: Shuah Khan Cc: fujita.tomonori, LKML, akpm, paul.gortmaker, bhelgaas, amwang, shuahkhan On Thu, Jul 12, 2012 at 10:17:50AM -0600, Shuah Khan wrote: > Disable iotlb overflow support when CONFIG_ISA is enabled. 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. I think I explained myself poorly. We don't want to break old drivers. If the old drivers expect this behavior (while they should be updated to check the DMA), we need to retain this behavior. So I think the patch should be done the other way: Disable IOTLB overflow when CONFIG_ISA is disabled. But we also need to check whether there are other drivers that don't properly check the DMA address. And if so, add them to this list of must have enabled b/c you might be using this driver. The first goal is to figure out which of the drivers aren't doing this properly. This should be possible by just grepping for 'dma_map' and seeing which ones don't do the 'dma_check' right after. > > Tested on x86 with io_tlb_overflow = 32*1024 and io_tlb_overflow = 0. > > Signed-off-by: Shuah Khan <shuah.khan@hp.com> > --- > lib/swiotlb.c | 65 +++++++++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 47 insertions(+), 18 deletions(-) > > diff --git a/lib/swiotlb.c b/lib/swiotlb.c > index 45bc1f8..f7d285c 100644 > --- a/lib/swiotlb.c > +++ b/lib/swiotlb.c > @@ -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. > */ > > #include <linux/cache.h> > @@ -68,7 +71,19 @@ static unsigned long io_tlb_nslabs; > /* > * When the IOMMU overflows we return a fallback buffer. This sets the size. > */ > +#if defined(CONFIG_ISA) > +/* > + * Disable iotlb overflow support when CONFIG_ISA is enabled. 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. > +*/ > +static unsigned long io_tlb_overflow; > +#else > static unsigned long io_tlb_overflow = 32*1024; > +#endif > > static void *io_tlb_overflow_buffer; > > @@ -156,12 +171,16 @@ 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"); > + } > + > if (verbose) > swiotlb_print_info(); > } > @@ -264,13 +283,15 @@ 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_info(); > > @@ -297,12 +318,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 +332,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 +704,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 +716,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 +937,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); > -- > 1.7.9.5 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is enabled 2012-07-16 14:45 ` Konrad Rzeszutek Wilk @ 2012-07-16 15:48 ` Shuah Khan 2012-07-16 16:01 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 14+ messages in thread From: Shuah Khan @ 2012-07-16 15:48 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: fujita.tomonori, LKML, akpm, paul.gortmaker, bhelgaas, amwang, shuahkhan On Mon, 2012-07-16 at 10:45 -0400, Konrad Rzeszutek Wilk wrote: > On Thu, Jul 12, 2012 at 10:17:50AM -0600, Shuah Khan wrote: > > Disable iotlb overflow support when CONFIG_ISA is enabled. 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. > > I think I explained myself poorly. We don't want to break old drivers. > > If the old drivers expect this behavior (while they should be updated > to check the DMA), we need to retain this behavior. > > So I think the patch should be done the other way: > > Disable IOTLB overflow when CONFIG_ISA is disabled. That makes lot of sense :) I will redo the patch. It might be good to a tunable to control enable/disable behavior. Even though support for this new tunable "swiotlb_overflow" projected to be short lived, it will be lot less painful to disable/enable as needed. Thought and comments? > > But we also need to check whether there are other drivers > that don't properly check the DMA address. And if so, add them > to this list of must have enabled b/c you might be using this > driver. Sound good. Again having a tunable would help in this case as well. > > The first goal is to figure out which of the drivers aren't doing this > properly. This should be possible by just grepping for 'dma_map' and > seeing which ones don't do the 'dma_check' right after. I started some research into this and will continue and provide an update. -- Shuah ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is enabled 2012-07-16 15:48 ` Shuah Khan @ 2012-07-16 16:01 ` Konrad Rzeszutek Wilk 2012-07-16 16:47 ` Shuah Khan 2012-07-17 18:27 ` Shuah Khan 0 siblings, 2 replies; 14+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-07-16 16:01 UTC (permalink / raw) To: Shuah Khan Cc: fujita.tomonori, LKML, akpm, paul.gortmaker, bhelgaas, amwang, shuahkhan On Mon, Jul 16, 2012 at 09:48:20AM -0600, Shuah Khan wrote: > On Mon, 2012-07-16 at 10:45 -0400, Konrad Rzeszutek Wilk wrote: > > On Thu, Jul 12, 2012 at 10:17:50AM -0600, Shuah Khan wrote: > > > Disable iotlb overflow support when CONFIG_ISA is enabled. 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. > > > > I think I explained myself poorly. We don't want to break old drivers. > > > > If the old drivers expect this behavior (while they should be updated > > to check the DMA), we need to retain this behavior. > > > > So I think the patch should be done the other way: > > > > Disable IOTLB overflow when CONFIG_ISA is disabled. > > That makes lot of sense :) I will redo the patch. It might be good to a > tunable to control enable/disable behavior. Even though support for this > new tunable "swiotlb_overflow" projected to be short lived, it will be > lot less painful to disable/enable as needed. Thought and comments? Tunnable have a habit of becoming permanant and we really don't want that. The right solution is to drop the overflow - as you have rightly pointed out in the first patch. But the road is blocked by these old drivers that are making it hard. I would say if you really really want a patch in for 3.6 before the final mega-patch of drivers-that-suck-and-need-to-be-updated-here-is -the-big-patch-that-does-it, then just add a WARN mentioning your name and mine (and LKML) saying to report what you see. That way we can at least get feedback from the field. And for good measure include in the Documentation/*deprecate-schedule that the overflow functionality is going away when all the drivers that depend on it have been fixed up. > > > > > But we also need to check whether there are other drivers > > that don't properly check the DMA address. And if so, add them > > to this list of must have enabled b/c you might be using this > > driver. > > Sound good. Again having a tunable would help in this case as well. > > > > The first goal is to figure out which of the drivers aren't doing this > > properly. This should be possible by just grepping for 'dma_map' and > > seeing which ones don't do the 'dma_check' right after. > > I started some research into this and will continue and provide an > update. Thank you! It is pretty griddy work so I really appreciate you taking the time to do this. If are at the LinuxCon this year the beers (or coffee if that is your preference) are on me for tackling on this cleanup. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is enabled 2012-07-16 16:01 ` Konrad Rzeszutek Wilk @ 2012-07-16 16:47 ` Shuah Khan 2012-07-17 18:27 ` Shuah Khan 1 sibling, 0 replies; 14+ messages in thread From: Shuah Khan @ 2012-07-16 16:47 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: fujita.tomonori, LKML, akpm, paul.gortmaker, bhelgaas, amwang, shuahkhan On Mon, 2012-07-16 at 12:01 -0400, Konrad Rzeszutek Wilk wrote: > On Mon, Jul 16, 2012 at 09:48:20AM -0600, Shuah Khan wrote: > > On Mon, 2012-07-16 at 10:45 -0400, Konrad Rzeszutek Wilk wrote: > > > On Thu, Jul 12, 2012 at 10:17:50AM -0600, Shuah Khan wrote: > > > > Disable iotlb overflow support when CONFIG_ISA is enabled. 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. > > > > > > I think I explained myself poorly. We don't want to break old drivers. > > > > > > If the old drivers expect this behavior (while they should be updated > > > to check the DMA), we need to retain this behavior. > > > > > > So I think the patch should be done the other way: > > > > > > Disable IOTLB overflow when CONFIG_ISA is disabled. > > > > That makes lot of sense :) I will redo the patch. It might be good to a > > tunable to control enable/disable behavior. Even though support for this > > new tunable "swiotlb_overflow" projected to be short lived, it will be > > lot less painful to disable/enable as needed. Thought and comments? > > Tunnable have a habit of becoming permanant and we really don't want that. > The right solution is to drop the overflow - as you have rightly > pointed out in the first patch. But the road is blocked by these old > drivers that are making it hard. > > I would say if you really really want a patch in for 3.6 before the > final mega-patch of drivers-that-suck-and-need-to-be-updated-here-is > -the-big-patch-that-does-it, then just add a WARN mentioning your > name and mine (and LKML) saying to report what you see. That way > we can at least get feedback from the field. > > And for good measure include in the Documentation/*deprecate-schedule > that the overflow functionality is going away when all the drivers that > depend on it have been fixed up. Will work on it and get the patch with deprecation schedule document out soon. > > > > > > > > > But we also need to check whether there are other drivers > > > that don't properly check the DMA address. And if so, add them > > > to this list of must have enabled b/c you might be using this > > > driver. > > > > Sound good. Again having a tunable would help in this case as well. > > > > > > The first goal is to figure out which of the drivers aren't doing this > > > properly. This should be possible by just grepping for 'dma_map' and > > > seeing which ones don't do the 'dma_check' right after. > > > > I started some research into this and will continue and provide an > > update. > > Thank you! It is pretty griddy work so I really appreciate you taking > the time to do this. If are at the LinuxCon this year the beers (or coffee > if that is your preference) are on me for tackling on this cleanup. Thanks. Unfortunately I won't be at this year's LinuxCon, maybe I get to take you up on your offer of coffee at one of the future ones. -- Shuah ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is enabled 2012-07-16 16:01 ` Konrad Rzeszutek Wilk 2012-07-16 16:47 ` Shuah Khan @ 2012-07-17 18:27 ` Shuah Khan 2012-07-17 20:13 ` [PATCH] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is disabled Shuah Khan 1 sibling, 1 reply; 14+ messages in thread From: Shuah Khan @ 2012-07-17 18:27 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: fujita.tomonori, LKML, akpm, paul.gortmaker, bhelgaas, amwang, shuahkhan On Mon, 2012-07-16 at 12:01 -0400, Konrad Rzeszutek Wilk wrote: > > > The first goal is to figure out which of the drivers aren't doing this > > > properly. This should be possible by just grepping for 'dma_map' and > > > seeing which ones don't do the 'dma_check' right after. > > I did cscope searches two different ways on linux-next July 12th git map_page - in this case I looked at all of the dma_ops.map_page() calls to see if they are followed by dma_mapping_error() calls. Found the following cases that don't check map_page() return value. arch/powerpc/platforms/cell/iommu.c - dma_fixed_map_page() drivers/net/ethernet/sun/niu.c - niu_rbr_add_page() net/sunrpc/xprtrdma/svc_rdma_sendto.c - dma_map_xdr() dma_map - in this case again I looked for calls to dma_map(). Found the following cases where return isn't checked: drivers/atm/fore200e.c drivers/isdn/hardware/eicon/di.c I am somewhat surprised to find only a few cases. Hoping I covered all the bases, can you think of anything else I should look for before concluding these are the cases that need fixing? -- Shuah ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is disabled 2012-07-17 18:27 ` Shuah Khan @ 2012-07-17 20:13 ` Shuah Khan 0 siblings, 0 replies; 14+ messages in thread From: Shuah Khan @ 2012-07-17 20:13 UTC (permalink / raw) To: fujita.tomonori, akpm, paul.gortmaker, bhelgaas, amwang, Konrad Rzeszutek Wilk, rob Cc: LKML, linux-doc, shuahkhan 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 <shuah.khan@hp.com> --- Documentation/feature-removal-schedule.txt | 22 ++++++-- lib/swiotlb.c | 79 +++++++++++++++++++++------- 2 files changed, 79 insertions(+), 22 deletions(-) diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt index 5979c3e..ce6c0ae 100644 --- a/Documentation/feature-removal-schedule.txt +++ b/Documentation/feature-removal-schedule.txt @@ -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 "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 <len.brown@intel.com> @@ -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 prevents bugs and code duplication Who: Christoph Hellwig <hch@lst.de> @@ -236,7 +236,7 @@ Who: David Brownell <dbrownell@users.sourceforge.net> 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. 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: linux-kernel@vger.kernel.org +Who: Shuah Khan <shuah.khan@hp.com> <shuahkhan@gmail.com> + +---------------------------- diff --git a/lib/swiotlb.c b/lib/swiotlb.c index 45bc1f8..0123bb8 100644 --- a/lib/swiotlb.c +++ b/lib/swiotlb.c @@ -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. */ #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: linux-kernel@vger.kernel.org,\n" + " Shuah Khan (shuahkhan@gmail.com), and\n" + " Konrad Wilk (konrad.wilk@oracle.com)\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" + " linux-kernel@vger.kernel.org,\n" + " Shuah Khan (shuahkhan@gmail.com), and\n" + " Konrad Wilk (konrad.wilk@oracle.com)\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); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-07-17 20:14 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-07-06 23:06 [PATCH RFC] swiotlb: Remove SWIOTLB overflow buffer support Shuah Khan 2012-07-09 20:25 ` Konrad Rzeszutek Wilk 2012-07-10 16:33 ` FUJITA Tomonori 2012-07-10 16:55 ` Shuah Khan 2012-07-10 17:32 ` Konrad Rzeszutek Wilk 2012-07-10 23:06 ` Shuah Khan 2012-07-10 23:13 ` Shuah Khan 2012-07-12 16:17 ` [PATCH RFC] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is enabled Shuah Khan 2012-07-16 14:45 ` Konrad Rzeszutek Wilk 2012-07-16 15:48 ` Shuah Khan 2012-07-16 16:01 ` Konrad Rzeszutek Wilk 2012-07-16 16:47 ` Shuah Khan 2012-07-17 18:27 ` Shuah Khan 2012-07-17 20:13 ` [PATCH] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is disabled Shuah Khan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.