Re: [PATCH for-5.10] swiotlb: remove the tbl_dma_addr argument to swiotlb_tbl_map_single

[Christoph Hellwig <hch@xxxxxx>]

ping?

On Fri, Oct 23, 2020 at 08:33:09AM +0200, Christoph Hellwig wrote:
> The tbl_dma_addr argument is used to check the DMA boundary for the
> allocations, and thus needs to be a dma_addr_t.  swiotlb-xen instead
> passed a physical address, which could lead to incorrect results for
> strange offsets.  Fix this by removing the parameter entirely and hard
> code the DMA address for io_tlb_start instead.
> 
> Fixes: 91ffe4ad534a ("swiotlb-xen: introduce phys_to_dma/dma_to_phys 
> translations")
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> ---
>  drivers/iommu/intel/iommu.c |  5 ++---
>  drivers/xen/swiotlb-xen.c   |  3 +--
>  include/linux/swiotlb.h     | 10 +++-------
>  kernel/dma/swiotlb.c        | 16 ++++++----------
>  4 files changed, 12 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 8651f6d4dfa032..6b560e6f193096 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -3815,9 +3815,8 @@ bounce_map_single(struct device *dev, phys_addr_t 
> paddr, size_t size,
>        * page aligned, we don't need to use a bounce page.
>        */
>       if (!IS_ALIGNED(paddr | size, VTD_PAGE_SIZE)) {
> -             tlb_addr = swiotlb_tbl_map_single(dev,
> -                             phys_to_dma_unencrypted(dev, io_tlb_start),
> -                             paddr, size, aligned_size, dir, attrs);
> +             tlb_addr = swiotlb_tbl_map_single(dev, paddr, size,
> +                             aligned_size, dir, attrs);
>               if (tlb_addr == DMA_MAPPING_ERROR) {
>                       goto swiotlb_error;
>               } else {
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 71ce1b7a23d1cc..2b385c1b4a99cb 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -395,8 +395,7 @@ static dma_addr_t xen_swiotlb_map_page(struct device 
> *dev, struct page *page,
>        */
>       trace_swiotlb_bounced(dev, dev_addr, size, swiotlb_force);
>  
> -     map = swiotlb_tbl_map_single(dev, virt_to_phys(xen_io_tlb_start),
> -                                  phys, size, size, dir, attrs);
> +     map = swiotlb_tbl_map_single(dev, phys, size, size, dir, attrs);
>       if (map == (phys_addr_t)DMA_MAPPING_ERROR)
>               return DMA_MAPPING_ERROR;
>  
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 513913ff748626..3bb72266a75a1d 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -45,13 +45,9 @@ enum dma_sync_target {
>       SYNC_FOR_DEVICE = 1,
>  };
>  
> -extern phys_addr_t swiotlb_tbl_map_single(struct device *hwdev,
> -                                       dma_addr_t tbl_dma_addr,
> -                                       phys_addr_t phys,
> -                                       size_t mapping_size,
> -                                       size_t alloc_size,
> -                                       enum dma_data_direction dir,
> -                                       unsigned long attrs);
> +phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t phys,
> +             size_t mapping_size, size_t alloc_size,
> +             enum dma_data_direction dir, unsigned long attrs);
>  
>  extern void swiotlb_tbl_unmap_single(struct device *hwdev,
>                                    phys_addr_t tlb_addr,
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index b4eea0abc3f002..92e2f54f24c01b 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -441,14 +441,11 @@ static void swiotlb_bounce(phys_addr_t orig_addr, 
> phys_addr_t tlb_addr,
>       }
>  }
>  
> -phys_addr_t swiotlb_tbl_map_single(struct device *hwdev,
> -                                dma_addr_t tbl_dma_addr,
> -                                phys_addr_t orig_addr,
> -                                size_t mapping_size,
> -                                size_t alloc_size,
> -                                enum dma_data_direction dir,
> -                                unsigned long attrs)
> +phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t 
> orig_addr,
> +             size_t mapping_size, size_t alloc_size,
> +             enum dma_data_direction dir, unsigned long attrs)
>  {
> +     dma_addr_t tbl_dma_addr = phys_to_dma_unencrypted(hwdev, io_tlb_start);
>       unsigned long flags;
>       phys_addr_t tlb_addr;
>       unsigned int nslots, stride, index, wrap;
> @@ -667,9 +664,8 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t 
> paddr, size_t size,
>       trace_swiotlb_bounced(dev, phys_to_dma(dev, paddr), size,
>                             swiotlb_force);
>  
> -     swiotlb_addr = swiotlb_tbl_map_single(dev,
> -                     phys_to_dma_unencrypted(dev, io_tlb_start),
> -                     paddr, size, size, dir, attrs);
> +     swiotlb_addr = swiotlb_tbl_map_single(dev, paddr, size, size, dir,
> +                     attrs);
>       if (swiotlb_addr == (phys_addr_t)DMA_MAPPING_ERROR)
>               return DMA_MAPPING_ERROR;
>  
> -- 
> 2.28.0
> 
> _______________________________________________
> iommu mailing list
> iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
---end quoted text---