| Submitter | Alex Williamson |
|---|---|
| Date | 2009-10-26 23:24:58 |
| Message ID | <20091026232458.9646.36818.stgit@nehalem.aw> |
| Download | mbox | patch |
| Permalink | /patch/56005/ |
| State | Not Applicable |
| Headers | show |
Comments
On Mon, Oct 26, 2009 at 05:24:58PM -0600, Alex Williamson wrote: > Move dma_generic_alloc_coherent() out of x86 code and create > corresponding dma_generic_free_coherent() for symmetry. These > can then be used by IOMMU drivers attempting to implement > passthrough mode. > Except that dma_generic_alloc_coherent() is only "generic" for platforms with consistent DMA. Everyone else will need a cacheflush and potentially a remap. It's not even obvious from looking at the consistent DMA platforms that they'll be able to use it out of the box due to expecting something other than page_address(), which all suggests that this is better off remaining an x86-only routine. This is also making changes to the DMA-API without any documentation updates and without consulting with any other architecture people. If you wish to make a proposal for a DMA-API addition, then this should be made to linux-arch rather than hidden in an iommu patchset. Beyond that, we presently have: - dma_alloc_coherent() - dma_alloc_noncoherent() - dma_alloc_from_coherent() defined in the API. Making an addition to this for your fallback case seems workable, but it's something that will have to be handled differently for each architecture. You might be able to get away with something generic enough to stash in drivers/base/dma-coherent.c, but from a first look, I wouldn't count on it. It was bad enough when the x86-specific flush_write_buffers() snuck in to the "generic" dma mapping code, but this particular case is much more problematic. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 26 Oct 2009 17:24:58 -0600 Alex Williamson <alex.williamson@hp.com> wrote: > Move dma_generic_alloc_coherent() out of x86 code and create > corresponding dma_generic_free_coherent() for symmetry. These > can then be used by IOMMU drivers attempting to implement > passthrough mode. > > Signed-off-by: Alex Williamson <alex.williamson@hp.com> > --- > > arch/x86/include/asm/dma-mapping.h | 3 -- > arch/x86/kernel/pci-dma.c | 31 ------------------------- > arch/x86/kernel/pci-nommu.c | 10 +++++++- > include/linux/dma-mapping.h | 44 ++++++++++++++++++++++++++++++++++++ > 4 files changed, 52 insertions(+), 36 deletions(-) dma_generic_alloc_coherent() is x86 specific; other sane architectures don't need such GFP_ hack. So moving it to the generic place is not a good idea. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2009-10-27 at 09:57 +0900, Paul Mundt wrote: > On Mon, Oct 26, 2009 at 05:24:58PM -0600, Alex Williamson wrote: > > Move dma_generic_alloc_coherent() out of x86 code and create > > corresponding dma_generic_free_coherent() for symmetry. These > > can then be used by IOMMU drivers attempting to implement > > passthrough mode. > > > Except that dma_generic_alloc_coherent() is only "generic" for platforms > with consistent DMA. Everyone else will need a cacheflush and potentially > a remap. It's not even obvious from looking at the consistent DMA > platforms that they'll be able to use it out of the box due to expecting > something other than page_address(), which all suggests that this is > better off remaining an x86-only routine. > > This is also making changes to the DMA-API without any documentation > updates and without consulting with any other architecture people. Ok, thanks for the feedback. Let's forget this patch and I'll see how amenable David is to doing something within intel-iommu since it only supports platforms with consistent DMA. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patch
diff --git a/arch/x86/include/asm/dma-mapping.h b/arch/x86/include/asm/dma-mapping.h index 0ee770d..e6d2c9f 100644 --- a/arch/x86/include/asm/dma-mapping.h +++ b/arch/x86/include/asm/dma-mapping.h @@ -52,9 +52,6 @@ static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr) extern int dma_supported(struct device *hwdev, u64 mask); extern int dma_set_mask(struct device *dev, u64 mask); -extern void *dma_generic_alloc_coherent(struct device *dev, size_t size, - dma_addr_t *dma_addr, gfp_t flag); - static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size) { if (!dev->dma_mask) diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c index b2a71dc..ecd9df0 100644 --- a/arch/x86/kernel/pci-dma.c +++ b/arch/x86/kernel/pci-dma.c @@ -144,37 +144,6 @@ void __init pci_iommu_alloc(void) pci_swiotlb_init(); } -void *dma_generic_alloc_coherent(struct device *dev, size_t size, - dma_addr_t *dma_addr, gfp_t flag) -{ - unsigned long dma_mask; - struct page *page; - dma_addr_t addr; - - dma_mask = dma_alloc_coherent_mask(dev, flag); - - flag |= __GFP_ZERO; -again: - page = alloc_pages_node(dev_to_node(dev), flag, get_order(size)); - if (!page) - return NULL; - - addr = page_to_phys(page); - if (addr + size > dma_mask) { - __free_pages(page, get_order(size)); - - if (dma_mask < DMA_BIT_MASK(32) && !(flag & GFP_DMA)) { - flag = (flag & ~GFP_DMA32) | GFP_DMA; - goto again; - } - - return NULL; - } - - *dma_addr = addr; - return page_address(page); -} - /* * See <Documentation/x86_64/boot-options.txt> for the iommu kernel parameter * documentation. diff --git a/arch/x86/kernel/pci-nommu.c b/arch/x86/kernel/pci-nommu.c index a3933d4..ed9e12e 100644 --- a/arch/x86/kernel/pci-nommu.c +++ b/arch/x86/kernel/pci-nommu.c @@ -73,10 +73,16 @@ static int nommu_map_sg(struct device *hwdev, struct scatterlist *sg, return nents; } +static void *nommu_alloc_coherent(struct device *dev, size_t size, + dma_addr_t *dma_addr, gfp_t flag) +{ + return dma_generic_alloc_coherent(dev, size, dma_addr, flag); +} + static void nommu_free_coherent(struct device *dev, size_t size, void *vaddr, dma_addr_t dma_addr) { - free_pages((unsigned long)vaddr, get_order(size)); + dma_generic_free_coherent(dev, size, vaddr, dma_addr); } static void nommu_sync_single_for_device(struct device *dev, @@ -95,7 +101,7 @@ static void nommu_sync_sg_for_device(struct device *dev, } struct dma_map_ops nommu_dma_ops = { - .alloc_coherent = dma_generic_alloc_coherent, + .alloc_coherent = nommu_alloc_coherent, .free_coherent = nommu_free_coherent, .map_sg = nommu_map_sg, .map_page = nommu_map_page, diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index 91b7618..285043c 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -232,4 +232,48 @@ struct dma_attrs; #endif /* CONFIG_HAVE_DMA_ATTRS */ +static inline void *dma_generic_alloc_coherent(struct device *dev, size_t size, + dma_addr_t *dma_addr, gfp_t flag) +{ + unsigned long dma_mask; + struct page *page; + dma_addr_t addr; + + dma_mask = dev->coherent_dma_mask; + if (!dma_mask) { +#ifdef CONFIG_ISA + dma_mask = (flag & GFP_DMA) ? DMA_BIT_MASK(24) + : DMA_BIT_MASK(32); +#else + dma_mask = DMA_BIT_MASK(32); +#endif + } + + flag |= __GFP_ZERO; +again: + page = alloc_pages_node(dev_to_node(dev), flag, get_order(size)); + if (!page) + return NULL; + + addr = page_to_phys(page); + if (addr + size > dma_mask) { + __free_pages(page, get_order(size)); + + if (dma_mask < DMA_BIT_MASK(32) && !(flag & GFP_DMA)) { + flag = (flag & ~GFP_DMA32) | GFP_DMA; + goto again; + } + + return NULL; + } + + *dma_addr = addr; + return page_address(page); +} + +static inline void dma_generic_free_coherent(struct device *dev, size_t size, + void *vaddr, dma_addr_t dma_addr) +{ + free_pages((unsigned long)vaddr, get_order(size)); +} #endif
Move dma_generic_alloc_coherent() out of x86 code and create corresponding dma_generic_free_coherent() for symmetry. These can then be used by IOMMU drivers attempting to implement passthrough mode. Signed-off-by: Alex Williamson <alex.williamson@hp.com> --- arch/x86/include/asm/dma-mapping.h | 3 -- arch/x86/kernel/pci-dma.c | 31 ------------------------- arch/x86/kernel/pci-nommu.c | 10 +++++++- include/linux/dma-mapping.h | 44 ++++++++++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 36 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html