[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140115201224.GD5201@phenom.dumpdata.com>
Date: Wed, 15 Jan 2014 15:12:24 -0500
From: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To: Akinobu Mita <akinobu.mita@...il.com>
Cc: linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
Marek Szyprowski <m.szyprowski@...sung.com>,
David Woodhouse <dwmw2@...radead.org>,
Don Dutile <ddutile@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>, Andi Kleen <andi@...stfloor.org>,
x86@...nel.org, iommu@...ts.linux-foundation.org
Subject: Re: [PATCH v2 2/5] x86: enable DMA CMA with swiotlb
On Tue, Jan 14, 2014 at 11:13:47PM +0900, Akinobu Mita wrote:
> The DMA Contiguous Memory Allocator support on x86 is disabled when
> swiotlb config option is enabled. So DMA CMA is always disabled on
> x86_64 because swiotlb is always enabled. This attempts to support
> for DMA CMA with enabling swiotlb config option.
>
> The contiguous memory allocator on x86 is integrated in the function
> dma_generic_alloc_coherent() which is .alloc callback in nommu_dma_ops
> for dma_alloc_coherent().
>
> x86_swiotlb_alloc_coherent() which is .alloc callback in swiotlb_dma_ops
> tries to allocate with dma_generic_alloc_coherent() firstly and then
> swiotlb_alloc_coherent() is called as a fallback.
>
> The main part of supporting DMA CMA with swiotlb is that changing
> x86_swiotlb_free_coherent() which is .free callback in swiotlb_dma_ops
> for dma_free_coherent() so that it can distinguish memory allocated by
> dma_generic_alloc_coherent() from one allocated by swiotlb_alloc_coherent()
> and release it with dma_generic_free_coherent() which can handle contiguous
> memory. This change requires making is_swiotlb_buffer() global function.
>
> This also needs to change .free callback in the dma_map_ops for amd_gart
> and sta2x11, because these dma_ops are also using
> dma_generic_alloc_coherent().
>
> Cc: Marek Szyprowski <m.szyprowski@...sung.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
> Cc: David Woodhouse <dwmw2@...radead.org>
> Cc: Don Dutile <ddutile@...hat.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: "H. Peter Anvin" <hpa@...or.com>
> Cc: Andi Kleen <andi@...stfloor.org>
> Cc: x86@...nel.org
> Cc: iommu@...ts.linux-foundation.org
> Signed-off-by: Akinobu Mita <akinobu.mita@...il.com>
> Acked-by: Marek Szyprowski <m.szyprowski@...sung.com>
> ---
> No change from the previous version
>
> arch/x86/Kconfig | 2 +-
> arch/x86/include/asm/swiotlb.h | 7 +++++++
> arch/x86/kernel/amd_gart_64.c | 2 +-
> arch/x86/kernel/pci-swiotlb.c | 9 ++++++---
> arch/x86/pci/sta2x11-fixup.c | 6 ++----
> include/linux/swiotlb.h | 2 ++
> lib/swiotlb.c | 2 +-
It looks reasonable from my perspective (as swiotlb maintainer).
Not too thrilled about the 'is_swiotlb_buffer' but that code is
quite small so it should be fast enough.
> 7 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 0952ecd..1b6275d 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -40,7 +40,7 @@ config X86
> select ARCH_WANT_OPTIONAL_GPIOLIB
> select ARCH_WANT_FRAME_POINTERS
> select HAVE_DMA_ATTRS
> - select HAVE_DMA_CONTIGUOUS if !SWIOTLB
> + select HAVE_DMA_CONTIGUOUS
> select HAVE_KRETPROBES
> select HAVE_OPTPROBES
> select HAVE_KPROBES_ON_FTRACE
> diff --git a/arch/x86/include/asm/swiotlb.h b/arch/x86/include/asm/swiotlb.h
> index 977f176..ab05d73 100644
> --- a/arch/x86/include/asm/swiotlb.h
> +++ b/arch/x86/include/asm/swiotlb.h
> @@ -29,4 +29,11 @@ static inline void pci_swiotlb_late_init(void)
>
> static inline void dma_mark_clean(void *addr, size_t size) {}
>
> +extern void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> + dma_addr_t *dma_handle, gfp_t flags,
> + struct dma_attrs *attrs);
> +extern void x86_swiotlb_free_coherent(struct device *dev, size_t size,
> + void *vaddr, dma_addr_t dma_addr,
> + struct dma_attrs *attrs);
> +
> #endif /* _ASM_X86_SWIOTLB_H */
> diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c
> index b574b29..8e3842f 100644
> --- a/arch/x86/kernel/amd_gart_64.c
> +++ b/arch/x86/kernel/amd_gart_64.c
> @@ -512,7 +512,7 @@ gart_free_coherent(struct device *dev, size_t size, void *vaddr,
> dma_addr_t dma_addr, struct dma_attrs *attrs)
> {
> gart_unmap_page(dev, dma_addr, size, DMA_BIDIRECTIONAL, NULL);
> - free_pages((unsigned long)vaddr, get_order(size));
> + dma_generic_free_coherent(dev, size, vaddr, dma_addr, attrs);
> }
>
> static int gart_mapping_error(struct device *dev, dma_addr_t dma_addr)
> diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
> index 6c483ba..77dd0ad 100644
> --- a/arch/x86/kernel/pci-swiotlb.c
> +++ b/arch/x86/kernel/pci-swiotlb.c
> @@ -14,7 +14,7 @@
> #include <asm/iommu_table.h>
> int swiotlb __read_mostly;
>
> -static void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> +void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> dma_addr_t *dma_handle, gfp_t flags,
> struct dma_attrs *attrs)
> {
> @@ -28,11 +28,14 @@ static void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> return swiotlb_alloc_coherent(hwdev, size, dma_handle, flags);
> }
>
> -static void x86_swiotlb_free_coherent(struct device *dev, size_t size,
> +void x86_swiotlb_free_coherent(struct device *dev, size_t size,
> void *vaddr, dma_addr_t dma_addr,
> struct dma_attrs *attrs)
> {
> - swiotlb_free_coherent(dev, size, vaddr, dma_addr);
> + if (is_swiotlb_buffer(dma_to_phys(dev, dma_addr)))
> + swiotlb_free_coherent(dev, size, vaddr, dma_addr);
> + else
> + dma_generic_free_coherent(dev, size, vaddr, dma_addr, attrs);
> }
>
> static struct dma_map_ops swiotlb_dma_ops = {
> diff --git a/arch/x86/pci/sta2x11-fixup.c b/arch/x86/pci/sta2x11-fixup.c
> index 9d8a509..5ceda85 100644
> --- a/arch/x86/pci/sta2x11-fixup.c
> +++ b/arch/x86/pci/sta2x11-fixup.c
> @@ -173,9 +173,7 @@ static void *sta2x11_swiotlb_alloc_coherent(struct device *dev,
> {
> void *vaddr;
>
> - vaddr = dma_generic_alloc_coherent(dev, size, dma_handle, flags, attrs);
> - if (!vaddr)
> - vaddr = swiotlb_alloc_coherent(dev, size, dma_handle, flags);
> + vaddr = x86_swiotlb_alloc_coherent(dev, size, dma_handle, flags, attrs);
> *dma_handle = p2a(*dma_handle, to_pci_dev(dev));
> return vaddr;
> }
> @@ -183,7 +181,7 @@ static void *sta2x11_swiotlb_alloc_coherent(struct device *dev,
> /* We have our own dma_ops: the same as swiotlb but from alloc (above) */
> static struct dma_map_ops sta2x11_dma_ops = {
> .alloc = sta2x11_swiotlb_alloc_coherent,
> - .free = swiotlb_free_coherent,
> + .free = x86_swiotlb_free_coherent,
> .map_page = swiotlb_map_page,
> .unmap_page = swiotlb_unmap_page,
> .map_sg = swiotlb_map_sg_attrs,
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index a5ffd32..e7a018e 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -116,4 +116,6 @@ static inline void swiotlb_free(void) { }
> #endif
>
> extern void swiotlb_print_info(void);
> +extern int is_swiotlb_buffer(phys_addr_t paddr);
> +
> #endif /* __LINUX_SWIOTLB_H */
> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> index fe978e0..6e4a798 100644
> --- a/lib/swiotlb.c
> +++ b/lib/swiotlb.c
> @@ -369,7 +369,7 @@ void __init swiotlb_free(void)
> io_tlb_nslabs = 0;
> }
>
> -static int is_swiotlb_buffer(phys_addr_t paddr)
> +int is_swiotlb_buffer(phys_addr_t paddr)
> {
> return paddr >= io_tlb_start && paddr < io_tlb_end;
> }
> --
> 1.8.3.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists