Re: [PATCH RFC 46/51] ARM: DMA-API: better handing of DMA masks for coherent allocations
On 08/02/2013 03:50 AM, Russell King wrote: > We need to start treating DMA masks as something which is specific to > the bus that the device resides on, otherwise we're going to hit all > sorts of nasty issues with LPAE and 32-bit DMA controllers in >32-bit > systems, where memory is offset from PFN 0. > > In order to start doing this, we convert the DMA mask to a PFN using > the device specific dma_to_pfn() macro. This is the reverse of the > pfn_to_dma() macro which is used to get the DMA address for the device. > > This gives us a PFN mask, which we can then check against the PFN > limit of the DMA zone. > > Signed-off-by: Russell King > --- > arch/arm/mm/dma-mapping.c | 49 > arch/arm/mm/init.c|2 + > arch/arm/mm/mm.h |2 + > 3 files changed, 48 insertions(+), 5 deletions(-) > Tested on Arndale board with ARM_LPAE enabled. Tested-by: Tushar Behera -- Tushar Behera -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 46/51] ARM: DMA-API: better handing of DMA masks for coherent allocations
On Mon, Aug 05, 2013 at 05:43:47PM -0500, Rob Herring wrote: > On Thu, Aug 1, 2013 at 5:20 PM, Russell King > wrote: > > We need to start treating DMA masks as something which is specific to > > the bus that the device resides on, otherwise we're going to hit all > > sorts of nasty issues with LPAE and 32-bit DMA controllers in >32-bit > > systems, where memory is offset from PFN 0. > > > > In order to start doing this, we convert the DMA mask to a PFN using > > the device specific dma_to_pfn() macro. This is the reverse of the > > pfn_to_dma() macro which is used to get the DMA address for the device. > > > > This gives us a PFN mask, which we can then check against the PFN > > limit of the DMA zone. > > > > Signed-off-by: Russell King > > --- > > arch/arm/mm/dma-mapping.c | 49 > > > > arch/arm/mm/init.c|2 + > > arch/arm/mm/mm.h |2 + > > 3 files changed, 48 insertions(+), 5 deletions(-) > > I believe you missed handling __dma_alloc. I have a different fix than > what Andreas posted. I think DMA zone handling is broken in all cases > here. Feel free to combine this in to your patch if you agree. I was starting to wonder if whether anyone was going to look at those patches... > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c > index 7f9b179..3d9bdfb 100644 > --- a/arch/arm/mm/dma-mapping.c > +++ b/arch/arm/mm/dma-mapping.c > @@ -651,7 +651,7 @@ static void *__dma_alloc(struct device *dev, > size_t size, dma_addr_t *handle, > if (!mask) > return NULL; > > - if (mask < 0xULL) > + if (mask <= (u64)arm_dma_limit) > gfp |= GFP_DMA; I'm not convinced on that - I think you've missed the entire point in this patch series about what address space the 'mask' is in. 'mask' is in the device's address space, which may not be the same as the physical address space. With LPAE, the two can become quite different address spaces with a 4GB offset between them. That's why we must stop the old-school thinking that DMA addresses and physical addresses are the same thing. We also need to stop doing stuff like passing dma_addr_t variables into phys_to_virt(). (All those short-cuts are going to break!) arm_dma_limit is the physical address space. So, comparing the two makes no sense what so ever. However, the use of arm_dma_limit at the top of get_coherent_dma_mask() is a bug, I think that should just become something like a 24-bit constant mask, so the NULL device gets GFP_DMA allocations like they do on x86 for that case. I also think that 'mask' should be converted to a pfn at the location you point out before comparing with the amount of memory in the DMA zone. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 46/51] ARM: DMA-API: better handing of DMA masks for coherent allocations
On Thu, Aug 1, 2013 at 5:20 PM, Russell King
wrote:
> We need to start treating DMA masks as something which is specific to
> the bus that the device resides on, otherwise we're going to hit all
> sorts of nasty issues with LPAE and 32-bit DMA controllers in >32-bit
> systems, where memory is offset from PFN 0.
>
> In order to start doing this, we convert the DMA mask to a PFN using
> the device specific dma_to_pfn() macro. This is the reverse of the
> pfn_to_dma() macro which is used to get the DMA address for the device.
>
> This gives us a PFN mask, which we can then check against the PFN
> limit of the DMA zone.
>
> Signed-off-by: Russell King
> ---
> arch/arm/mm/dma-mapping.c | 49
> arch/arm/mm/init.c|2 +
> arch/arm/mm/mm.h |2 +
> 3 files changed, 48 insertions(+), 5 deletions(-)
I believe you missed handling __dma_alloc. I have a different fix than
what Andreas posted. I think DMA zone handling is broken in all cases
here. Feel free to combine this in to your patch if you agree.
Author: Rob Herring
Date: Thu Aug 1 15:51:17 2013 -0500
ARM: fix dma-mapping on LPAE
With LPAE, the DMA zone size may be 4GB, so the GFP_DMA flag needs to be
set when the mask is less than or equal to the arm_dma_limit. This also
fixes a bug with DMA zone mask handling. GFP_DMA should be set for any
mask less than or equal to arm_dma_limit, not just less than ~0UL.
Signed-off-by: Rob Herring
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 7f9b179..3d9bdfb 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -651,7 +651,7 @@ static void *__dma_alloc(struct device *dev,
size_t size, dma_addr_t *handle,
if (!mask)
return NULL;
- if (mask < 0xULL)
+ if (mask <= (u64)arm_dma_limit)
gfp |= GFP_DMA;
/*
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 7f9b179..ef0efab 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -173,10 +173,30 @@ static u64 get_coherent_dma_mask(struct device *dev)
> return 0;
> }
>
> - if ((~mask) & (u64)arm_dma_limit) {
> - dev_warn(dev, "coherent DMA mask %#llx is smaller "
> -"than system GFP_DMA mask %#llx\n",
> -mask, (u64)arm_dma_limit);
> + /*
> +* If the mask allows for more memory than we can address,
> +* and we actually have that much memory, then fail the
> +* allocation.
> +*/
> + if (sizeof(mask) != sizeof(dma_addr_t) &&
> + mask > (dma_addr_t)~0 &&
> + dma_to_pfn(dev, ~0) > arm_dma_pfn_limit) {
> + dev_warn(dev, "Coherent DMA mask %#llx is larger than
> dma_addr_t allows\n",
> +mask);
> + dev_warn(dev, "Driver did not use or check the return
> value from dma_set_coherent_mask()?\n");
> + return 0;
> + }
> +
> + /*
> +* Now check that the mask, when translated to a PFN,
> +* fits within the allowable addresses which we can
> +* allocate.
> +*/
> + if (dma_to_pfn(dev, mask) < arm_dma_pfn_limit) {
> + dev_warn(dev, "Coherent DMA mask %#llx (pfn
> %#lx-%#lx) covers a smaller range of system memory than the DMA zone pfn
> 0x0-%#lx\n",
> +mask,
> +dma_to_pfn(dev, 0), dma_to_pfn(dev, mask) +
> 1,
> +arm_dma_pfn_limit + 1);
> return 0;
> }
> }
> @@ -1008,8 +1028,27 @@ void arm_dma_sync_sg_for_device(struct device *dev,
> struct scatterlist *sg,
> */
> int dma_supported(struct device *dev, u64 mask)
> {
> - if (mask < (u64)arm_dma_limit)
> + unsigned long limit;
> +
> + /*
> +* If the mask allows for more memory than we can address,
> +* and we actually have that much memory, then we must
> +* indicate that DMA to this device is not supported.
> +*/
> + if (sizeof(mask) != sizeof(dma_addr_t) &&
> + mask > (dma_addr_t)~0 &&
> + dma_to_pfn(dev, ~0) > arm_dma_pfn_limit)
> + return 0;
> +
> + /*
> +* Translate the device's DMA mask to a PFN limit. This
> +* PFN number includes the page which we can DMA to.
> +*/
> + limit = dma_to_pfn(dev, mask);
> +
> + if (limit < arm_dma_pfn_limit)
> return 0;
> +
> return 1;
> }
> EXPORT_SYMBOL(dma_supported);
> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> index 15225d8..b5b5836 100644
