[PATCH v2] iommu/io-pgtable-arm: Don't use dma_to_phys()

[Yong Wu]

On Thu, 2015-09-17 at 17:42 +0100, Robin Murphy wrote:
> In checking whether DMA addresses differ from physical addresses, using
> dma_to_phys() is actually the wrong thing to do, since it may hide any
> DMA offset, which is precisely one of the things we are checking for.
> Simply casting between the two address types, whilst ugly, is in fact
> the appropriate course of action. Further care (and ugliness) is also
> necessary in the comparison to avoid truncation if phys_addr_t and
> dma_addr_t differ in size.
> 
> We can also reject any device with a fixed DMA offset up-front at page
> table creation, leaving the allocation-time check for the more subtle
> cases like bounce buffering due to an incorrect DMA mask.
> 
> Furthermore, we can then fix the hackish KConfig dependency so that
> architectures without a dma_to_phys() implementation may still
> COMPILE_TEST (or even use!) the code. The true dependency is on the
> DMA API, so use the appropriate symbol for that.
> 
> Signed-off-by: Robin Murphy <robin.murphy at arm.com>
> ---
[...]
>  
>  static bool selftest_running = false;
>  
> -static dma_addr_t __arm_lpae_dma_addr(struct device *dev, void *pages)
> +static dma_addr_t __arm_lpae_dma_addr(void *pages)
>  {
> -	return phys_to_dma(dev, virt_to_phys(pages));
> +	return (dma_addr_t)virt_to_phys(pages);
>  }
>  
>  static void *__arm_lpae_alloc_pages(size_t size, gfp_t gfp,
> @@ -223,10 +223,10 @@ static void *__arm_lpae_alloc_pages(size_t size, gfp_t gfp,
>  			goto out_free;
>  		/*
>  		 * We depend on the IOMMU being able to work with any physical
> -		 * address directly, so if the DMA layer suggests it can't by
> -		 * giving us back some translation, that bodes very badly...
> +		 * address directly, so if the DMA layer suggests otherwise by
> +		 * translating or truncating them, that bodes very badly...
>  		 */
> -		if (dma != __arm_lpae_dma_addr(dev, pages))
> +		if (dma != virt_to_phys(pages))

Could I ask why not use __arm_lpae_dma_addr(pages) here?
dma is dma_addr_t.

>  			goto out_unmap;
>  	}
>  
> @@ -243,10 +243,8 @@ out_free:
>  static void __arm_lpae_free_pages(void *pages, size_t size,
>  				  struct io_pgtable_cfg *cfg)
>  {
> -	struct device *dev = cfg->iommu_dev;
> -
>  	if (!selftest_running)
> -		dma_unmap_single(dev, __arm_lpae_dma_addr(dev, pages),
> +		dma_unmap_single(cfg->iommu_dev, __arm_lpae_dma_addr(pages),
>  				 size, DMA_TO_DEVICE);
>  	free_pages_exact(pages, size);
>  }
> @@ -254,12 +252,11 @@ static void __arm_lpae_free_pages(void *pages, size_t size,
>  static void __arm_lpae_set_pte(arm_lpae_iopte *ptep, arm_lpae_iopte pte,
>  			       struct io_pgtable_cfg *cfg)
>  {
> -	struct device *dev = cfg->iommu_dev;
> -
>  	*ptep = pte;
>  
>  	if (!selftest_running)
> -		dma_sync_single_for_device(dev, __arm_lpae_dma_addr(dev, ptep),
> +		dma_sync_single_for_device(cfg->iommu_dev,
> +					   __arm_lpae_dma_addr(ptep),
>  					   sizeof(pte), DMA_TO_DEVICE);
>  }
>  
> @@ -629,6 +626,11 @@ arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg)
>  	if (cfg->oas > ARM_LPAE_MAX_ADDR_BITS)
>  		return NULL;
>  
> +	if (cfg->iommu_dev->dma_pfn_offset) {
> +		dev_err(cfg->iommu_dev, "Cannot accommodate DMA offset for IOMMU page tables\n");
> +		return NULL;
> +	}
> +
>  	data = kmalloc(sizeof(*data), GFP_KERNEL);
>  	if (!data)
>  		return NULL;