* [PATCHv3 0/2] CMA for arm64 @ 2013-12-10 21:43 Laura Abbott 2013-12-10 21:43 ` [PATCHv3 1/2] arm64: Check for NULL device before getting the coherent_dma_mask Laura Abbott 2013-12-10 21:43 ` [PATCHv3 2/2] arm64: Enable CMA Laura Abbott 0 siblings, 2 replies; 11+ messages in thread From: Laura Abbott @ 2013-12-10 21:43 UTC (permalink / raw) To: linux-arm-kernel Hi, This adds support for CMA in arm64 v3: Fix a missed line to actually set the dma_handle correctly v2: Move the dma contiguous functions from swiotlb to arm64 directly arch/arm64/Kconfig | 1 + arch/arm64/include/asm/dma-contiguous.h | 29 +++++++++++++++++++++++++++++ arch/arm64/mm/dma-mapping.c | 27 ++++++++++++++++++++++++--- arch/arm64/mm/init.c | 3 +++ 4 files changed, 57 insertions(+), 3 deletions(-) Thanks, Laura ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv3 1/2] arm64: Check for NULL device before getting the coherent_dma_mask 2013-12-10 21:43 [PATCHv3 0/2] CMA for arm64 Laura Abbott @ 2013-12-10 21:43 ` Laura Abbott 2013-12-11 10:42 ` Will Deacon 2013-12-10 21:43 ` [PATCHv3 2/2] arm64: Enable CMA Laura Abbott 1 sibling, 1 reply; 11+ messages in thread From: Laura Abbott @ 2013-12-10 21:43 UTC (permalink / raw) To: linux-arm-kernel The device passed in to dma_alloc may be NULL. Check for this before trying to get the coherent_dma_mask. Cc: Will Deacon <will.deacon@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Laura Abbott <lauraa@codeaurora.org> --- arch/arm64/mm/dma-mapping.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c index 4bd7579..4134212 100644 --- a/arch/arm64/mm/dma-mapping.c +++ b/arch/arm64/mm/dma-mapping.c @@ -33,7 +33,7 @@ static void *arm64_swiotlb_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle, gfp_t flags, struct dma_attrs *attrs) { - if (IS_ENABLED(CONFIG_ZONE_DMA32) && + if (dev && IS_ENABLED(CONFIG_ZONE_DMA32) && dev->coherent_dma_mask <= DMA_BIT_MASK(32)) flags |= GFP_DMA32; return swiotlb_alloc_coherent(dev, size, dma_handle, flags); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCHv3 1/2] arm64: Check for NULL device before getting the coherent_dma_mask 2013-12-10 21:43 ` [PATCHv3 1/2] arm64: Check for NULL device before getting the coherent_dma_mask Laura Abbott @ 2013-12-11 10:42 ` Will Deacon 2013-12-11 17:48 ` Laura Abbott 0 siblings, 1 reply; 11+ messages in thread From: Will Deacon @ 2013-12-11 10:42 UTC (permalink / raw) To: linux-arm-kernel On Tue, Dec 10, 2013 at 09:43:35PM +0000, Laura Abbott wrote: > The device passed in to dma_alloc may be NULL. Check for this before > trying to get the coherent_dma_mask. > > Cc: Will Deacon <will.deacon@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Laura Abbott <lauraa@codeaurora.org> > --- > arch/arm64/mm/dma-mapping.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c > index 4bd7579..4134212 100644 > --- a/arch/arm64/mm/dma-mapping.c > +++ b/arch/arm64/mm/dma-mapping.c > @@ -33,7 +33,7 @@ static void *arm64_swiotlb_alloc_coherent(struct device *dev, size_t size, > dma_addr_t *dma_handle, gfp_t flags, > struct dma_attrs *attrs) > { > - if (IS_ENABLED(CONFIG_ZONE_DMA32) && > + if (dev && IS_ENABLED(CONFIG_ZONE_DMA32) && > dev->coherent_dma_mask <= DMA_BIT_MASK(32)) > flags |= GFP_DMA32; > return swiotlb_alloc_coherent(dev, size, dma_handle, flags); Unless I'm misreading the code, it looks like there are paths through swiotlb_alloc_coherent that will dereference the dev parameter without a NULL check. Are you sure we should allow for NULL devices here? Will ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv3 1/2] arm64: Check for NULL device before getting the coherent_dma_mask 2013-12-11 10:42 ` Will Deacon @ 2013-12-11 17:48 ` Laura Abbott 2013-12-11 17:51 ` Will Deacon 0 siblings, 1 reply; 11+ messages in thread From: Laura Abbott @ 2013-12-11 17:48 UTC (permalink / raw) To: linux-arm-kernel On 12/11/2013 2:42 AM, Will Deacon wrote: > On Tue, Dec 10, 2013 at 09:43:35PM +0000, Laura Abbott wrote: >> The device passed in to dma_alloc may be NULL. Check for this before >> trying to get the coherent_dma_mask. >> >> Cc: Will Deacon <will.deacon@arm.com> >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> Cc: Marek Szyprowski <m.szyprowski@samsung.com> >> Signed-off-by: Laura Abbott <lauraa@codeaurora.org> >> --- >> arch/arm64/mm/dma-mapping.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c >> index 4bd7579..4134212 100644 >> --- a/arch/arm64/mm/dma-mapping.c >> +++ b/arch/arm64/mm/dma-mapping.c >> @@ -33,7 +33,7 @@ static void *arm64_swiotlb_alloc_coherent(struct device *dev, size_t size, >> dma_addr_t *dma_handle, gfp_t flags, >> struct dma_attrs *attrs) >> { >> - if (IS_ENABLED(CONFIG_ZONE_DMA32) && >> + if (dev && IS_ENABLED(CONFIG_ZONE_DMA32) && >> dev->coherent_dma_mask <= DMA_BIT_MASK(32)) >> flags |= GFP_DMA32; >> return swiotlb_alloc_coherent(dev, size, dma_handle, flags); > > Unless I'm misreading the code, it looks like there are paths through > swiotlb_alloc_coherent that will dereference the dev parameter without a > NULL check. Are you sure we should allow for NULL devices here? > The current ARM code allows for NULL devices so that would be a difference in behavior between arm and arm64. We're also relying on this behavior in some code. Where exactly in swiotlb_alloc_coherent does this dereference happen? The only one I see is checked with 'if (hwdev && hwdev->coherent_dma_mask)' > Will > Thanks, Laura -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv3 1/2] arm64: Check for NULL device before getting the coherent_dma_mask 2013-12-11 17:48 ` Laura Abbott @ 2013-12-11 17:51 ` Will Deacon 2013-12-11 18:10 ` Laura Abbott 0 siblings, 1 reply; 11+ messages in thread From: Will Deacon @ 2013-12-11 17:51 UTC (permalink / raw) To: linux-arm-kernel On Wed, Dec 11, 2013 at 05:48:10PM +0000, Laura Abbott wrote: > On 12/11/2013 2:42 AM, Will Deacon wrote: > > On Tue, Dec 10, 2013 at 09:43:35PM +0000, Laura Abbott wrote: > >> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c > >> index 4bd7579..4134212 100644 > >> --- a/arch/arm64/mm/dma-mapping.c > >> +++ b/arch/arm64/mm/dma-mapping.c > >> @@ -33,7 +33,7 @@ static void *arm64_swiotlb_alloc_coherent(struct device *dev, size_t size, > >> dma_addr_t *dma_handle, gfp_t flags, > >> struct dma_attrs *attrs) > >> { > >> - if (IS_ENABLED(CONFIG_ZONE_DMA32) && > >> + if (dev && IS_ENABLED(CONFIG_ZONE_DMA32) && > >> dev->coherent_dma_mask <= DMA_BIT_MASK(32)) > >> flags |= GFP_DMA32; > >> return swiotlb_alloc_coherent(dev, size, dma_handle, flags); > > > > Unless I'm misreading the code, it looks like there are paths through > > swiotlb_alloc_coherent that will dereference the dev parameter without a > > NULL check. Are you sure we should allow for NULL devices here? > > > > The current ARM code allows for NULL devices so that would be a > difference in behavior between arm and arm64. We're also relying on this > behavior in some code. Where exactly in swiotlb_alloc_coherent does this > dereference happen? The only one I see is checked with 'if (hwdev && > hwdev->coherent_dma_mask)' phys_to_dma could, but doesn't. The one I spotted was buried down in: map_single -> swiotlb_tbl_map_single -> dma_get_seg_boundary Will ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv3 1/2] arm64: Check for NULL device before getting the coherent_dma_mask 2013-12-11 17:51 ` Will Deacon @ 2013-12-11 18:10 ` Laura Abbott 2013-12-12 12:18 ` Will Deacon 0 siblings, 1 reply; 11+ messages in thread From: Laura Abbott @ 2013-12-11 18:10 UTC (permalink / raw) To: linux-arm-kernel On 12/11/2013 9:51 AM, Will Deacon wrote: > On Wed, Dec 11, 2013 at 05:48:10PM +0000, Laura Abbott wrote: >> On 12/11/2013 2:42 AM, Will Deacon wrote: >>> On Tue, Dec 10, 2013 at 09:43:35PM +0000, Laura Abbott wrote: >>>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c >>>> index 4bd7579..4134212 100644 >>>> --- a/arch/arm64/mm/dma-mapping.c >>>> +++ b/arch/arm64/mm/dma-mapping.c >>>> @@ -33,7 +33,7 @@ static void *arm64_swiotlb_alloc_coherent(struct device *dev, size_t size, >>>> dma_addr_t *dma_handle, gfp_t flags, >>>> struct dma_attrs *attrs) >>>> { >>>> - if (IS_ENABLED(CONFIG_ZONE_DMA32) && >>>> + if (dev && IS_ENABLED(CONFIG_ZONE_DMA32) && >>>> dev->coherent_dma_mask <= DMA_BIT_MASK(32)) >>>> flags |= GFP_DMA32; >>>> return swiotlb_alloc_coherent(dev, size, dma_handle, flags); >>> >>> Unless I'm misreading the code, it looks like there are paths through >>> swiotlb_alloc_coherent that will dereference the dev parameter without a >>> NULL check. Are you sure we should allow for NULL devices here? >>> >> >> The current ARM code allows for NULL devices so that would be a >> difference in behavior between arm and arm64. We're also relying on this >> behavior in some code. Where exactly in swiotlb_alloc_coherent does this >> dereference happen? The only one I see is checked with 'if (hwdev && >> hwdev->coherent_dma_mask)' > > phys_to_dma could, but doesn't. The one I spotted was buried down in: > > map_single -> swiotlb_tbl_map_single -> dma_get_seg_boundary > Ah yes I see that now. I guess the question still stands though, should we fixup the parts of the code to fully support the NULL devices or do we tell clients to fix their code and fail the allocation with a warning not to pass NULL? > Will > Laura -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv3 1/2] arm64: Check for NULL device before getting the coherent_dma_mask 2013-12-11 18:10 ` Laura Abbott @ 2013-12-12 12:18 ` Will Deacon 2013-12-12 19:00 ` Laura Abbott 0 siblings, 1 reply; 11+ messages in thread From: Will Deacon @ 2013-12-12 12:18 UTC (permalink / raw) To: linux-arm-kernel On Wed, Dec 11, 2013 at 06:10:59PM +0000, Laura Abbott wrote: > On 12/11/2013 9:51 AM, Will Deacon wrote: > > On Wed, Dec 11, 2013 at 05:48:10PM +0000, Laura Abbott wrote: > >> On 12/11/2013 2:42 AM, Will Deacon wrote: > >>> On Tue, Dec 10, 2013 at 09:43:35PM +0000, Laura Abbott wrote: > >>>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c > >>>> index 4bd7579..4134212 100644 > >>>> --- a/arch/arm64/mm/dma-mapping.c > >>>> +++ b/arch/arm64/mm/dma-mapping.c > >>>> @@ -33,7 +33,7 @@ static void *arm64_swiotlb_alloc_coherent(struct device *dev, size_t size, > >>>> dma_addr_t *dma_handle, gfp_t flags, > >>>> struct dma_attrs *attrs) > >>>> { > >>>> - if (IS_ENABLED(CONFIG_ZONE_DMA32) && > >>>> + if (dev && IS_ENABLED(CONFIG_ZONE_DMA32) && > >>>> dev->coherent_dma_mask <= DMA_BIT_MASK(32)) > >>>> flags |= GFP_DMA32; > >>>> return swiotlb_alloc_coherent(dev, size, dma_handle, flags); > >>> > >>> Unless I'm misreading the code, it looks like there are paths through > >>> swiotlb_alloc_coherent that will dereference the dev parameter without a > >>> NULL check. Are you sure we should allow for NULL devices here? > >>> > >> > >> The current ARM code allows for NULL devices so that would be a > >> difference in behavior between arm and arm64. We're also relying on this > >> behavior in some code. Where exactly in swiotlb_alloc_coherent does this > >> dereference happen? The only one I see is checked with 'if (hwdev && > >> hwdev->coherent_dma_mask)' > > > > phys_to_dma could, but doesn't. The one I spotted was buried down in: > > > > map_single -> swiotlb_tbl_map_single -> dma_get_seg_boundary > > > > Ah yes I see that now. > > I guess the question still stands though, should we fixup the parts of > the code to fully support the NULL devices or do we tell clients to fix > their code and fail the allocation with a warning not to pass NULL? Assuming it's always possible to pass in a non-NULL device, I'd be tempted to fix the callers. Can you think of a valid use-case where the caller doesn't have a device handle to pass in? Will ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv3 1/2] arm64: Check for NULL device before getting the coherent_dma_mask 2013-12-12 12:18 ` Will Deacon @ 2013-12-12 19:00 ` Laura Abbott 0 siblings, 0 replies; 11+ messages in thread From: Laura Abbott @ 2013-12-12 19:00 UTC (permalink / raw) To: linux-arm-kernel On 12/12/2013 4:18 AM, Will Deacon wrote: > On Wed, Dec 11, 2013 at 06:10:59PM +0000, Laura Abbott wrote: >> On 12/11/2013 9:51 AM, Will Deacon wrote: >>> On Wed, Dec 11, 2013 at 05:48:10PM +0000, Laura Abbott wrote: >>>> On 12/11/2013 2:42 AM, Will Deacon wrote: >>>>> On Tue, Dec 10, 2013 at 09:43:35PM +0000, Laura Abbott wrote: >>>>>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c >>>>>> index 4bd7579..4134212 100644 >>>>>> --- a/arch/arm64/mm/dma-mapping.c >>>>>> +++ b/arch/arm64/mm/dma-mapping.c >>>>>> @@ -33,7 +33,7 @@ static void *arm64_swiotlb_alloc_coherent(struct device *dev, size_t size, >>>>>> dma_addr_t *dma_handle, gfp_t flags, >>>>>> struct dma_attrs *attrs) >>>>>> { >>>>>> - if (IS_ENABLED(CONFIG_ZONE_DMA32) && >>>>>> + if (dev && IS_ENABLED(CONFIG_ZONE_DMA32) && >>>>>> dev->coherent_dma_mask <= DMA_BIT_MASK(32)) >>>>>> flags |= GFP_DMA32; >>>>>> return swiotlb_alloc_coherent(dev, size, dma_handle, flags); >>>>> >>>>> Unless I'm misreading the code, it looks like there are paths through >>>>> swiotlb_alloc_coherent that will dereference the dev parameter without a >>>>> NULL check. Are you sure we should allow for NULL devices here? >>>>> >>>> >>>> The current ARM code allows for NULL devices so that would be a >>>> difference in behavior between arm and arm64. We're also relying on this >>>> behavior in some code. Where exactly in swiotlb_alloc_coherent does this >>>> dereference happen? The only one I see is checked with 'if (hwdev && >>>> hwdev->coherent_dma_mask)' >>> >>> phys_to_dma could, but doesn't. The one I spotted was buried down in: >>> >>> map_single -> swiotlb_tbl_map_single -> dma_get_seg_boundary >>> >> >> Ah yes I see that now. >> >> I guess the question still stands though, should we fixup the parts of >> the code to fully support the NULL devices or do we tell clients to fix >> their code and fail the allocation with a warning not to pass NULL? > > Assuming it's always possible to pass in a non-NULL device, I'd be tempted > to fix the callers. Can you think of a valid use-case where the caller > doesn't have a device handle to pass in? > Depends on your definition of 'valid' ;) The only cases we've been able to come up with are places where we've been abusing the DMA APIs slightly to get chunks of contiguous memory through CMA for miscellaneous debugging/testing purposes. I only stumbled across this problem at all while writing a quick test case for CMA. I'd argue that if there is a real use case for non-device large contiguous allocations there should be a new dedicated API that doesn't break the dma abstraction. I suggest for now we add a big WARN when a NULL device is passed in and see how many people complain. Laura -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv3 2/2] arm64: Enable CMA 2013-12-10 21:43 [PATCHv3 0/2] CMA for arm64 Laura Abbott 2013-12-10 21:43 ` [PATCHv3 1/2] arm64: Check for NULL device before getting the coherent_dma_mask Laura Abbott @ 2013-12-10 21:43 ` Laura Abbott 2013-12-11 10:40 ` Will Deacon 1 sibling, 1 reply; 11+ messages in thread From: Laura Abbott @ 2013-12-10 21:43 UTC (permalink / raw) To: linux-arm-kernel arm64 bit targets need the features CMA provides. Add the appropriate hooks, header files, and Kconfig to allow this to happen. Cc: Will Deacon <will.deacon@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Laura Abbott <lauraa@codeaurora.org> --- arch/arm64/Kconfig | 1 + arch/arm64/include/asm/dma-contiguous.h | 29 +++++++++++++++++++++++++++++ arch/arm64/mm/dma-mapping.c | 25 +++++++++++++++++++++++-- arch/arm64/mm/init.c | 3 +++ 4 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 arch/arm64/include/asm/dma-contiguous.h diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 9737e97..26e4bef 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -22,6 +22,7 @@ config ARM64 select HAVE_DEBUG_KMEMLEAK select HAVE_DMA_API_DEBUG select HAVE_DMA_ATTRS + select HAVE_DMA_CONTIGUOUS select HAVE_GENERIC_DMA_COHERENT select HAVE_GENERIC_HARDIRQS select HAVE_HW_BREAKPOINT if PERF_EVENTS diff --git a/arch/arm64/include/asm/dma-contiguous.h b/arch/arm64/include/asm/dma-contiguous.h new file mode 100644 index 0000000..bc32516 --- /dev/null +++ b/arch/arm64/include/asm/dma-contiguous.h @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2013, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef ASMARM64_DMA_CONTIGUOUS_H +#define ASMARM64_DMA_CONTIGUOUS_H + +#ifdef __KERNEL__ +#ifdef CONFIG_CMA + +#include <linux/types.h> +#include <asm-generic/dma-contiguous.h> + +static inline void +dma_contiguous_early_fixup(phys_addr_t base, unsigned long size) { } + +#endif +#endif + +#endif diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c index 4134212..29d10b9 100644 --- a/arch/arm64/mm/dma-mapping.c +++ b/arch/arm64/mm/dma-mapping.c @@ -21,6 +21,7 @@ #include <linux/export.h> #include <linux/slab.h> #include <linux/dma-mapping.h> +#include <linux/dma-contiguous.h> #include <linux/vmalloc.h> #include <linux/swiotlb.h> @@ -36,14 +37,34 @@ static void *arm64_swiotlb_alloc_coherent(struct device *dev, size_t size, if (dev && IS_ENABLED(CONFIG_ZONE_DMA32) && dev->coherent_dma_mask <= DMA_BIT_MASK(32)) flags |= GFP_DMA32; - return swiotlb_alloc_coherent(dev, size, dma_handle, flags); + if (IS_ENABLED(CONFIG_DMA_CMA)) { + struct page *page; + + page = dma_alloc_from_contiguous(dev, size >> PAGE_SHIFT, + get_order(size)); + if (!page) + return NULL; + + *dma_handle = phys_to_dma(dev, page_to_phys(page)); + return page_address(page); + } else { + return swiotlb_alloc_coherent(dev, size, dma_handle, flags); + } } static void arm64_swiotlb_free_coherent(struct device *dev, size_t size, void *vaddr, dma_addr_t dma_handle, struct dma_attrs *attrs) { - swiotlb_free_coherent(dev, size, vaddr, dma_handle); + if (IS_ENABLED(CONFIG_DMA_CMA)) { + phys_addr_t paddr = dma_to_phys(dev, dma_handle); + + dma_release_from_contiguous(dev, + phys_to_page(paddr), + size >> PAGE_SHIFT); + } else { + swiotlb_free_coherent(dev, size, vaddr, dma_handle); + } } static struct dma_map_ops arm64_swiotlb_dma_ops = { diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index 67e8d7c..74b7da1 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -30,6 +30,7 @@ #include <linux/memblock.h> #include <linux/sort.h> #include <linux/of_fdt.h> +#include <linux/dma-contiguous.h> #include <asm/prom.h> #include <asm/sections.h> @@ -173,6 +174,8 @@ void __init arm64_memblock_init(void) memblock_reserve(base, size); } + dma_contiguous_reserve(0); + memblock_allow_resize(); memblock_dump_all(); } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCHv3 2/2] arm64: Enable CMA 2013-12-10 21:43 ` [PATCHv3 2/2] arm64: Enable CMA Laura Abbott @ 2013-12-11 10:40 ` Will Deacon 2013-12-11 17:54 ` Laura Abbott 0 siblings, 1 reply; 11+ messages in thread From: Will Deacon @ 2013-12-11 10:40 UTC (permalink / raw) To: linux-arm-kernel Hi Laura, Couple of really minor comments... On Tue, Dec 10, 2013 at 09:43:36PM +0000, Laura Abbott wrote: > arm64 bit targets need the features CMA provides. Add the appropriate > hooks, header files, and Kconfig to allow this to happen. > > Cc: Will Deacon <will.deacon@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Laura Abbott <lauraa@codeaurora.org> > --- > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/dma-contiguous.h | 29 +++++++++++++++++++++++++++++ > arch/arm64/mm/dma-mapping.c | 25 +++++++++++++++++++++++-- > arch/arm64/mm/init.c | 3 +++ > 4 files changed, 56 insertions(+), 2 deletions(-) > create mode 100644 arch/arm64/include/asm/dma-contiguous.h > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 9737e97..26e4bef 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -22,6 +22,7 @@ config ARM64 > select HAVE_DEBUG_KMEMLEAK > select HAVE_DMA_API_DEBUG > select HAVE_DMA_ATTRS > + select HAVE_DMA_CONTIGUOUS > select HAVE_GENERIC_DMA_COHERENT > select HAVE_GENERIC_HARDIRQS > select HAVE_HW_BREAKPOINT if PERF_EVENTS > diff --git a/arch/arm64/include/asm/dma-contiguous.h b/arch/arm64/include/asm/dma-contiguous.h > new file mode 100644 > index 0000000..bc32516 > --- /dev/null > +++ b/arch/arm64/include/asm/dma-contiguous.h > @@ -0,0 +1,29 @@ > +/* > + * Copyright (c) 2013, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#ifndef ASMARM64_DMA_CONTIGUOUS_H > +#define ASMARM64_DMA_CONTIGUOUS_H We've tried to keep these guards consistent for the arm64 headers, so this would be: __ASM_DMA_CONTIGUOUS_H. > + > +#ifdef __KERNEL__ > +#ifdef CONFIG_CMA Why is this not CONFIG_DMA_CMA? > +#include <linux/types.h> > +#include <asm-generic/dma-contiguous.h> > + > +static inline void > +dma_contiguous_early_fixup(phys_addr_t base, unsigned long size) { } > + > +#endif > +#endif > + > +#endif > diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c > index 4134212..29d10b9 100644 > --- a/arch/arm64/mm/dma-mapping.c > +++ b/arch/arm64/mm/dma-mapping.c > @@ -21,6 +21,7 @@ > #include <linux/export.h> > #include <linux/slab.h> > #include <linux/dma-mapping.h> > +#include <linux/dma-contiguous.h> > #include <linux/vmalloc.h> > #include <linux/swiotlb.h> > > @@ -36,14 +37,34 @@ static void *arm64_swiotlb_alloc_coherent(struct device *dev, size_t size, > if (dev && IS_ENABLED(CONFIG_ZONE_DMA32) && > dev->coherent_dma_mask <= DMA_BIT_MASK(32)) > flags |= GFP_DMA32; > - return swiotlb_alloc_coherent(dev, size, dma_handle, flags); > + if (IS_ENABLED(CONFIG_DMA_CMA)) { > + struct page *page; > + > + page = dma_alloc_from_contiguous(dev, size >> PAGE_SHIFT, > + get_order(size)); > + if (!page) > + return NULL; Seems a shame to fail the allocation if CMA can't manage it. Is there a good reason not to fall back to swiotlb (other than complicating the freeing paths)? Will ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv3 2/2] arm64: Enable CMA 2013-12-11 10:40 ` Will Deacon @ 2013-12-11 17:54 ` Laura Abbott 0 siblings, 0 replies; 11+ messages in thread From: Laura Abbott @ 2013-12-11 17:54 UTC (permalink / raw) To: linux-arm-kernel On 12/11/2013 2:40 AM, Will Deacon wrote: > Hi Laura, > > Couple of really minor comments... > > On Tue, Dec 10, 2013 at 09:43:36PM +0000, Laura Abbott wrote: >> arm64 bit targets need the features CMA provides. Add the appropriate >> hooks, header files, and Kconfig to allow this to happen. >> >> Cc: Will Deacon <will.deacon@arm.com> >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> Cc: Marek Szyprowski <m.szyprowski@samsung.com> >> Signed-off-by: Laura Abbott <lauraa@codeaurora.org> >> --- >> arch/arm64/Kconfig | 1 + >> arch/arm64/include/asm/dma-contiguous.h | 29 +++++++++++++++++++++++++++++ >> arch/arm64/mm/dma-mapping.c | 25 +++++++++++++++++++++++-- >> arch/arm64/mm/init.c | 3 +++ >> 4 files changed, 56 insertions(+), 2 deletions(-) >> create mode 100644 arch/arm64/include/asm/dma-contiguous.h >> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index 9737e97..26e4bef 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -22,6 +22,7 @@ config ARM64 >> select HAVE_DEBUG_KMEMLEAK >> select HAVE_DMA_API_DEBUG >> select HAVE_DMA_ATTRS >> + select HAVE_DMA_CONTIGUOUS >> select HAVE_GENERIC_DMA_COHERENT >> select HAVE_GENERIC_HARDIRQS >> select HAVE_HW_BREAKPOINT if PERF_EVENTS >> diff --git a/arch/arm64/include/asm/dma-contiguous.h b/arch/arm64/include/asm/dma-contiguous.h >> new file mode 100644 >> index 0000000..bc32516 >> --- /dev/null >> +++ b/arch/arm64/include/asm/dma-contiguous.h >> @@ -0,0 +1,29 @@ >> +/* >> + * Copyright (c) 2013, The Linux Foundation. All rights reserved. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 and >> + * only version 2 as published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> + >> +#ifndef ASMARM64_DMA_CONTIGUOUS_H >> +#define ASMARM64_DMA_CONTIGUOUS_H > > We've tried to keep these guards consistent for the arm64 headers, so this > would be: __ASM_DMA_CONTIGUOUS_H. > Fine. >> + >> +#ifdef __KERNEL__ >> +#ifdef CONFIG_CMA > > Why is this not CONFIG_DMA_CMA? > Whoops. >> +#include <linux/types.h> >> +#include <asm-generic/dma-contiguous.h> >> + >> +static inline void >> +dma_contiguous_early_fixup(phys_addr_t base, unsigned long size) { } >> + >> +#endif >> +#endif >> + >> +#endif >> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c >> index 4134212..29d10b9 100644 >> --- a/arch/arm64/mm/dma-mapping.c >> +++ b/arch/arm64/mm/dma-mapping.c >> @@ -21,6 +21,7 @@ >> #include <linux/export.h> >> #include <linux/slab.h> >> #include <linux/dma-mapping.h> >> +#include <linux/dma-contiguous.h> >> #include <linux/vmalloc.h> >> #include <linux/swiotlb.h> >> >> @@ -36,14 +37,34 @@ static void *arm64_swiotlb_alloc_coherent(struct device *dev, size_t size, >> if (dev && IS_ENABLED(CONFIG_ZONE_DMA32) && >> dev->coherent_dma_mask <= DMA_BIT_MASK(32)) >> flags |= GFP_DMA32; >> - return swiotlb_alloc_coherent(dev, size, dma_handle, flags); >> + if (IS_ENABLED(CONFIG_DMA_CMA)) { >> + struct page *page; >> + >> + page = dma_alloc_from_contiguous(dev, size >> PAGE_SHIFT, >> + get_order(size)); >> + if (!page) >> + return NULL; > > Seems a shame to fail the allocation if CMA can't manage it. Is there a good > reason not to fall back to swiotlb (other than complicating the freeing > paths)? > The current ARM code doesn't currently fall back but perhaps more importantly, the entire point of CMA is to be able to get allocations that are larger than the buddy allocator can handle. I'm not sure how likely it would be that if a CMA allocation failed the buddy allocator would be able to handle it successfully. > Will > Thanks, Laura -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-12-12 19:00 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-12-10 21:43 [PATCHv3 0/2] CMA for arm64 Laura Abbott 2013-12-10 21:43 ` [PATCHv3 1/2] arm64: Check for NULL device before getting the coherent_dma_mask Laura Abbott 2013-12-11 10:42 ` Will Deacon 2013-12-11 17:48 ` Laura Abbott 2013-12-11 17:51 ` Will Deacon 2013-12-11 18:10 ` Laura Abbott 2013-12-12 12:18 ` Will Deacon 2013-12-12 19:00 ` Laura Abbott 2013-12-10 21:43 ` [PATCHv3 2/2] arm64: Enable CMA Laura Abbott 2013-12-11 10:40 ` Will Deacon 2013-12-11 17:54 ` Laura Abbott
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.