| Message ID | 20201023063309.3472987-1-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | fc0021aa340af65a0a37d77be39e22aa886a6132 |
| Headers | show |
| Series | [for-5.10] swiotlb: remove the tbl_dma_addr argument to swiotlb_tbl_map_single | expand |
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@lst.de> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > 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@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu ---end quoted text---
On Tue, Nov 03, 2020 at 10:46:43AM +0100, Christoph Hellwig wrote: > ping? Hopefully this goes through. I am in the process of testing it but ran into testing issues that I believe are unrelated. > > 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@lst.de> > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > --- > > 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@lists.linux-foundation.org > > https://lists.linuxfoundation.org/mailman/listinfo/iommu > ---end quoted text---
On Wed, Nov 04, 2020 at 09:04:38AM -0500, Konrad Rzeszutek Wilk wrote: > On Tue, Nov 03, 2020 at 10:46:43AM +0100, Christoph Hellwig wrote: > > ping? > > Hopefully this goes through. I am in the process of testing it but ran > into testing issues that I believe are unrelated. Did you manage to make any progress? This fixes an issue with the new support for systems with DMA offsets in 5.10..
On Tue, Nov 10, 2020 at 10:14:21AM +0100, Christoph Hellwig wrote: > On Wed, Nov 04, 2020 at 09:04:38AM -0500, Konrad Rzeszutek Wilk wrote: > > On Tue, Nov 03, 2020 at 10:46:43AM +0100, Christoph Hellwig wrote: > > > ping? > > > > Hopefully this goes through. I am in the process of testing it but ran > > into testing issues that I believe are unrelated. > > Did you manage to make any progress? This fixes an issue with the YES!! > new support for systems with DMA offsets in 5.10.. OK. Sending the git pull request in a minute or two.
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;
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@lst.de> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> --- 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(-)