linux-kernel - Re: [PATCH] x86: avoid unnecessary low zone allocation in AMD IOMMU's alloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080910120310.GD6329@amd.com>
Date:	Wed, 10 Sep 2008 14:03:10 +0200
From:	Joerg Roedel <joerg.roedel@....com>
To:	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
CC:	linux-kernel@...r.kernel.org, mingo@...e.hu
Subject: Re: [PATCH] x86: avoid unnecessary low zone allocation in AMD IOMMU's alloc_coherent

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@....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@....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

--
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/