Re: [PATCH] x86: avoid unnecessary low zone allocation in AMD IOMMU's alloc_coherent

[Joerg Roedel <joerg.roedel@amd.com>]

On Wed, Sep 10, 2008 at 08:19:40PM +0900, FUJITA Tomonori wrote:
> This is for tip/iommu.
> 
> btw, Joery, what happens if map_sg (or map_single) gets
> not-DMA-capable buffer in the case of !iommu || !domain?

It blows up :-)
I know it but currently it is no problem because this will never happen
in any real AMD IOMMU system (because the hardware just remaps every
device in the system). It needs a fix anyway and the
right solution here is to fall back to one of the software iommu
implementations. The stackable dma_ops patches I have currently in work
will do exactly that.

> 
> 
> ==
> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Subject: [PATCH] x86: avoid unnecessary low zone allocation in AMD IOMMU's alloc_coherent
> 
> x86's common alloc_coherent (dma_alloc_coherent in dma-mapping.h) sets
> up the gfp flag according to the device dma_mask but AMD IOMMU doesn't
> need it for devices that the IOMMU can do virtual mappings for. This
> patch avoids unnecessary low zone allocation.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> ---
>  arch/x86/kernel/amd_iommu.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
> index 01c68c3..8efd249 100644
> --- a/arch/x86/kernel/amd_iommu.c
> +++ b/arch/x86/kernel/amd_iommu.c
> @@ -999,6 +999,11 @@ static void *alloc_coherent(struct device *dev, size_t size,
>  	u16 devid;
>  	phys_addr_t paddr;
>  
> +	get_device_resources(dev, &iommu, &domain, &devid);
> +
> +	if (iommu && domain)
> +		flag &= ~(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32);
> +

These flags are already removed in the dma_alloc_coherent function which
calls this one. Further I think in the case of a remapping IOMMU like
this one we should avoid implementing any gfp hacks and just trust the
caller.

>  	virt_addr = (void *)__get_free_pages(flag, get_order(size));
>  	if (!virt_addr)
>  		return 0;
> @@ -1006,8 +1011,6 @@ static void *alloc_coherent(struct device *dev, size_t size,
>  	memset(virt_addr, 0, size);
>  	paddr = virt_to_phys(virt_addr);
>  
> -	get_device_resources(dev, &iommu, &domain, &devid);
> -
>  	if (!iommu || !domain) {
>  		*dma_addr = (dma_addr_t)paddr;
>  		return virt_addr;
> -- 
> 1.5.4.2
> 
> 

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy