* [PATCH for-5.10] swiotlb: remove the tbl_dma_addr argument to swiotlb_tbl_map_single
@ 2020-10-23 6:33 Christoph Hellwig
2020-11-03 9:46 ` Christoph Hellwig
0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2020-10-23 6:33 UTC (permalink / raw)
To: konrad.wilk; +Cc: xen-devel, iommu, Stefano Stabellini
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH for-5.10] swiotlb: remove the tbl_dma_addr argument to swiotlb_tbl_map_single
2020-10-23 6:33 [PATCH for-5.10] swiotlb: remove the tbl_dma_addr argument to swiotlb_tbl_map_single Christoph Hellwig
@ 2020-11-03 9:46 ` Christoph Hellwig
2020-11-04 14:04 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2020-11-03 9:46 UTC (permalink / raw)
To: konrad.wilk; +Cc: xen-devel, iommu, Stefano Stabellini
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---
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH for-5.10] swiotlb: remove the tbl_dma_addr argument to swiotlb_tbl_map_single
2020-11-03 9:46 ` Christoph Hellwig
@ 2020-11-04 14:04 ` Konrad Rzeszutek Wilk
2020-11-10 9:14 ` Christoph Hellwig
0 siblings, 1 reply; 5+ messages in thread
From: Konrad Rzeszutek Wilk @ 2020-11-04 14:04 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xen-devel, iommu, Stefano Stabellini
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---
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH for-5.10] swiotlb: remove the tbl_dma_addr argument to swiotlb_tbl_map_single
2020-11-04 14:04 ` Konrad Rzeszutek Wilk
@ 2020-11-10 9:14 ` Christoph Hellwig
2020-11-10 21:27 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2020-11-10 9:14 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: xen-devel, iommu, Stefano Stabellini, Christoph Hellwig
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..
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH for-5.10] swiotlb: remove the tbl_dma_addr argument to swiotlb_tbl_map_single
2020-11-10 9:14 ` Christoph Hellwig
@ 2020-11-10 21:27 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 5+ messages in thread
From: Konrad Rzeszutek Wilk @ 2020-11-10 21:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xen-devel, iommu, Stefano Stabellini
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.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-10 21:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-23 6:33 [PATCH for-5.10] swiotlb: remove the tbl_dma_addr argument to swiotlb_tbl_map_single Christoph Hellwig
2020-11-03 9:46 ` Christoph Hellwig
2020-11-04 14:04 ` Konrad Rzeszutek Wilk
2020-11-10 9:14 ` Christoph Hellwig
2020-11-10 21:27 ` Konrad Rzeszutek Wilk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).