* [PATCH v4 1/6] dma-iommu: fix sync_sg with swiotlb
2021-08-13 7:38 [PATCH v4 0/6] Fixes for dma-iommu swiotlb bounce buffers David Stevens
@ 2021-08-13 7:38 ` David Stevens
2021-08-13 7:58 ` Christoph Hellwig
2021-08-13 7:38 ` [PATCH v4 2/6] dma-iommu: fix arch_sync_dma for map David Stevens
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: David Stevens @ 2021-08-13 7:38 UTC (permalink / raw)
To: Robin Murphy, Christoph Hellwig
Cc: Joerg Roedel, Will Deacon, Lu Baolu, Tom Murphy, iommu,
linux-kernel, David Stevens
From: David Stevens <stevensd@chromium.org>
The is_swiotlb_buffer function takes the physical address of the swiotlb
buffer, not the physical address of the original buffer. The sglist
contains the physical addresses of the original buffer, so for the
sync_sg functions to work properly when a bounce buffer might have been
used, we need to use iommu_iova_to_phys to look up the physical address.
This is what sync_single does, so call that function on each sglist
segment.
The previous code mostly worked because swiotlb does the transfer on map
and unmap. However, any callers which use DMA_ATTR_SKIP_CPU_SYNC with
sglists or which call sync_sg would not have had anything copied to the
bounce buffer.
Fixes: 82612d66d51d ("iommu: Allow the dma-iommu api to use bounce buffers")
Signed-off-by: David Stevens <stevensd@chromium.org>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 33 +++++++++++++--------------------
1 file changed, 13 insertions(+), 20 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 98ba927aee1a..968e0150c95e 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -810,17 +810,13 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev,
struct scatterlist *sg;
int i;
- if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
- return;
-
- for_each_sg(sgl, sg, nelems, i) {
- if (!dev_is_dma_coherent(dev))
+ if (dev_is_untrusted(dev))
+ for_each_sg(sgl, sg, nelems, i)
+ iommu_dma_sync_single_for_cpu(dev, sg_dma_address(sg),
+ sg->length, dir);
+ else if (!dev_is_dma_coherent(dev))
+ for_each_sg(sgl, sg, nelems, i)
arch_sync_dma_for_cpu(sg_phys(sg), sg->length, dir);
-
- if (is_swiotlb_buffer(sg_phys(sg)))
- swiotlb_sync_single_for_cpu(dev, sg_phys(sg),
- sg->length, dir);
- }
}
static void iommu_dma_sync_sg_for_device(struct device *dev,
@@ -830,17 +826,14 @@ static void iommu_dma_sync_sg_for_device(struct device *dev,
struct scatterlist *sg;
int i;
- if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
- return;
-
- for_each_sg(sgl, sg, nelems, i) {
- if (is_swiotlb_buffer(sg_phys(sg)))
- swiotlb_sync_single_for_device(dev, sg_phys(sg),
- sg->length, dir);
-
- if (!dev_is_dma_coherent(dev))
+ if (dev_is_untrusted(dev))
+ for_each_sg(sgl, sg, nelems, i)
+ iommu_dma_sync_single_for_device(dev,
+ sg_dma_address(sg),
+ sg->length, dir);
+ else if (!dev_is_dma_coherent(dev))
+ for_each_sg(sgl, sg, nelems, i)
arch_sync_dma_for_device(sg_phys(sg), sg->length, dir);
- }
}
static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
--
2.33.0.rc1.237.g0d66db33f3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 2/6] dma-iommu: fix arch_sync_dma for map
2021-08-13 7:38 [PATCH v4 0/6] Fixes for dma-iommu swiotlb bounce buffers David Stevens
2021-08-13 7:38 ` [PATCH v4 1/6] dma-iommu: fix sync_sg with swiotlb David Stevens
@ 2021-08-13 7:38 ` David Stevens
2021-08-13 8:02 ` Christoph Hellwig
2021-08-13 7:38 ` [PATCH v4 3/6] dma-iommu: skip extra sync during unmap w/swiotlb David Stevens
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: David Stevens @ 2021-08-13 7:38 UTC (permalink / raw)
To: Robin Murphy, Christoph Hellwig
Cc: Joerg Roedel, Will Deacon, Lu Baolu, Tom Murphy, iommu,
linux-kernel, David Stevens
From: David Stevens <stevensd@chromium.org>
When calling arch_sync_dma, we need to pass it the memory that's
actually being used for dma. When using swiotlb bounce buffers, this is
the bounce buffer. Fold __iommu_dma_map_swiotlb into iommu_dma_map_page
so it can sync the right phys_addr_t.
Now that iommu_dma_map_sg delegates to a function which takes care of
architectural syncing in the untrusted device case, the call to
iommu_dma_sync_sg_for_device can be moved so it only occurs in for
trusted devices. Doing the sync there for untrusted devices never really
worked, since it needs to be able to target swiotlb buffers.
This also moves the architectural sync to before the call to
__iommu_dma_map, to guarantee that untrusted devices can't see stale
data they shouldn't see.
Fixes: 82612d66d51d ("iommu: Allow the dma-iommu api to use bounce buffers")
Signed-off-by: David Stevens <stevensd@chromium.org>
---
drivers/iommu/dma-iommu.c | 105 +++++++++++++++++---------------------
1 file changed, 47 insertions(+), 58 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 968e0150c95e..e9119ff93535 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -536,52 +536,6 @@ static dma_addr_t __iommu_dma_map(struct device *dev, phys_addr_t phys,
return iova + iova_off;
}
-static dma_addr_t __iommu_dma_map_swiotlb(struct device *dev, phys_addr_t phys,
- size_t org_size, dma_addr_t dma_mask, bool coherent,
- enum dma_data_direction dir, unsigned long attrs)
-{
- int prot = dma_info_to_prot(dir, coherent, attrs);
- struct iommu_domain *domain = iommu_get_dma_domain(dev);
- struct iommu_dma_cookie *cookie = domain->iova_cookie;
- struct iova_domain *iovad = &cookie->iovad;
- size_t aligned_size = org_size;
- void *padding_start;
- size_t padding_size;
- dma_addr_t iova;
-
- /*
- * If both the physical buffer start address and size are
- * page aligned, we don't need to use a bounce page.
- */
- if (IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev) &&
- iova_offset(iovad, phys | org_size)) {
- aligned_size = iova_align(iovad, org_size);
- phys = swiotlb_tbl_map_single(dev, phys, org_size,
- aligned_size, dir, attrs);
-
- if (phys == DMA_MAPPING_ERROR)
- return DMA_MAPPING_ERROR;
-
- /* Cleanup the padding area. */
- padding_start = phys_to_virt(phys);
- padding_size = aligned_size;
-
- if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
- (dir == DMA_TO_DEVICE ||
- dir == DMA_BIDIRECTIONAL)) {
- padding_start += org_size;
- padding_size -= org_size;
- }
-
- memset(padding_start, 0, padding_size);
- }
-
- iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask);
- if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(phys))
- swiotlb_tbl_unmap_single(dev, phys, org_size, dir, attrs);
- return iova;
-}
-
static void __iommu_dma_free_pages(struct page **pages, int count)
{
while (count--)
@@ -842,14 +796,50 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
{
phys_addr_t phys = page_to_phys(page) + offset;
bool coherent = dev_is_dma_coherent(dev);
- dma_addr_t dma_handle;
+ int prot = dma_info_to_prot(dir, coherent, attrs);
+ struct iommu_domain *domain = iommu_get_dma_domain(dev);
+ struct iommu_dma_cookie *cookie = domain->iova_cookie;
+ struct iova_domain *iovad = &cookie->iovad;
+ size_t aligned_size = size;
+ dma_addr_t iova, dma_mask = dma_get_mask(dev);
+
+ /*
+ * If both the physical buffer start address and size are
+ * page aligned, we don't need to use a bounce page.
+ */
+ if (IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev) &&
+ iova_offset(iovad, phys | size)) {
+ void *padding_start;
+ size_t padding_size;
+
+ aligned_size = iova_align(iovad, size);
+ phys = swiotlb_tbl_map_single(dev, phys, size,
+ aligned_size, dir, attrs);
+
+ if (phys == DMA_MAPPING_ERROR)
+ return DMA_MAPPING_ERROR;
+
+ /* Cleanup the padding area. */
+ padding_start = phys_to_virt(phys);
+ padding_size = aligned_size;
+
+ if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
+ (dir == DMA_TO_DEVICE ||
+ dir == DMA_BIDIRECTIONAL)) {
+ padding_start += size;
+ padding_size -= size;
+ }
- dma_handle = __iommu_dma_map_swiotlb(dev, phys, size, dma_get_mask(dev),
- coherent, dir, attrs);
- if (!coherent && !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
- dma_handle != DMA_MAPPING_ERROR)
+ memset(padding_start, 0, padding_size);
+ }
+
+ if (!coherent && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
arch_sync_dma_for_device(phys, size, dir);
- return dma_handle;
+
+ iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask);
+ if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(phys))
+ swiotlb_tbl_unmap_single(dev, phys, size, dir, attrs);
+ return iova;
}
static void iommu_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
@@ -953,9 +943,8 @@ static int iommu_dma_map_sg_swiotlb(struct device *dev, struct scatterlist *sg,
int i;
for_each_sg(sg, s, nents, i) {
- sg_dma_address(s) = __iommu_dma_map_swiotlb(dev, sg_phys(s),
- s->length, dma_get_mask(dev),
- dev_is_dma_coherent(dev), dir, attrs);
+ sg_dma_address(s) = iommu_dma_map_page(dev, sg_page(s),
+ s->offset, s->length, dir, attrs);
if (sg_dma_address(s) == DMA_MAPPING_ERROR)
goto out_unmap;
sg_dma_len(s) = s->length;
@@ -992,12 +981,12 @@ static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
iommu_deferred_attach(dev, domain))
return 0;
- if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
- iommu_dma_sync_sg_for_device(dev, sg, nents, dir);
-
if (dev_is_untrusted(dev))
return iommu_dma_map_sg_swiotlb(dev, sg, nents, dir, attrs);
+ if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+ iommu_dma_sync_sg_for_device(dev, sg, nents, dir);
+
/*
* Work out how much IOVA space we need, and align the segments to
* IOVA granules for the IOMMU driver to handle. With some clever
--
2.33.0.rc1.237.g0d66db33f3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 3/6] dma-iommu: skip extra sync during unmap w/swiotlb
2021-08-13 7:38 [PATCH v4 0/6] Fixes for dma-iommu swiotlb bounce buffers David Stevens
2021-08-13 7:38 ` [PATCH v4 1/6] dma-iommu: fix sync_sg with swiotlb David Stevens
2021-08-13 7:38 ` [PATCH v4 2/6] dma-iommu: fix arch_sync_dma for map David Stevens
@ 2021-08-13 7:38 ` David Stevens
2021-08-13 8:03 ` Christoph Hellwig
2021-08-13 7:38 ` [PATCH v4 4/6] dma-iommu: Check CONFIG_SWIOTLB more broadly David Stevens
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: David Stevens @ 2021-08-13 7:38 UTC (permalink / raw)
To: Robin Murphy, Christoph Hellwig
Cc: Joerg Roedel, Will Deacon, Lu Baolu, Tom Murphy, iommu,
linux-kernel, David Stevens
From: David Stevens <stevensd@chromium.org>
Calling the iommu_dma_sync_*_for_cpu functions during unmap can cause
two copies out of the swiotlb buffer. Fold __iommu_dma_unmap_swiotlb
into iommu_dma_unmap_page, and directly call arch_sync_dma_for_cpu
instead of iommu_dma_sync_single_for_cpu to avoid this double sync. With
this refactor, calling iommu_dma_sync_sg_for_cpu for untrusted devices
in iommu_dma_unmap_sg is also no longer necessary, so move that
invocation later in the function.
Signed-off-by: David Stevens <stevensd@chromium.org>
---
drivers/iommu/dma-iommu.c | 42 +++++++++++++++++----------------------
1 file changed, 18 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index e9119ff93535..f7da4934f7e6 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -493,23 +493,6 @@ static void __iommu_dma_unmap(struct device *dev, dma_addr_t dma_addr,
iommu_dma_free_iova(cookie, dma_addr, size, iotlb_gather.freelist);
}
-static void __iommu_dma_unmap_swiotlb(struct device *dev, dma_addr_t dma_addr,
- size_t size, enum dma_data_direction dir,
- unsigned long attrs)
-{
- struct iommu_domain *domain = iommu_get_dma_domain(dev);
- phys_addr_t phys;
-
- phys = iommu_iova_to_phys(domain, dma_addr);
- if (WARN_ON(!phys))
- return;
-
- __iommu_dma_unmap(dev, dma_addr, size);
-
- if (unlikely(is_swiotlb_buffer(phys)))
- swiotlb_tbl_unmap_single(dev, phys, size, dir, attrs);
-}
-
static dma_addr_t __iommu_dma_map(struct device *dev, phys_addr_t phys,
size_t size, int prot, u64 dma_mask)
{
@@ -845,9 +828,20 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
static void iommu_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
size_t size, enum dma_data_direction dir, unsigned long attrs)
{
- if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
- iommu_dma_sync_single_for_cpu(dev, dma_handle, size, dir);
- __iommu_dma_unmap_swiotlb(dev, dma_handle, size, dir, attrs);
+ struct iommu_domain *domain = iommu_get_dma_domain(dev);
+ phys_addr_t phys;
+
+ phys = iommu_iova_to_phys(domain, dma_handle);
+ if (WARN_ON(!phys))
+ return;
+
+ if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) && !dev_is_dma_coherent(dev))
+ arch_sync_dma_for_cpu(phys, size, dir);
+
+ __iommu_dma_unmap(dev, dma_handle, size);
+
+ if (unlikely(is_swiotlb_buffer(phys)))
+ swiotlb_tbl_unmap_single(dev, phys, size, dir, attrs);
}
/*
@@ -932,7 +926,7 @@ static void iommu_dma_unmap_sg_swiotlb(struct device *dev, struct scatterlist *s
int i;
for_each_sg(sg, s, nents, i)
- __iommu_dma_unmap_swiotlb(dev, sg_dma_address(s),
+ iommu_dma_unmap_page(dev, sg_dma_address(s),
sg_dma_len(s), dir, attrs);
}
@@ -1053,14 +1047,14 @@ static void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
struct scatterlist *tmp;
int i;
- if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
- iommu_dma_sync_sg_for_cpu(dev, sg, nents, dir);
-
if (dev_is_untrusted(dev)) {
iommu_dma_unmap_sg_swiotlb(dev, sg, nents, dir, attrs);
return;
}
+ if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+ iommu_dma_sync_sg_for_cpu(dev, sg, nents, dir);
+
/*
* The scatterlist segments are mapped into a single
* contiguous IOVA allocation, so this is incredibly easy.
--
2.33.0.rc1.237.g0d66db33f3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 4/6] dma-iommu: Check CONFIG_SWIOTLB more broadly
2021-08-13 7:38 [PATCH v4 0/6] Fixes for dma-iommu swiotlb bounce buffers David Stevens
` (2 preceding siblings ...)
2021-08-13 7:38 ` [PATCH v4 3/6] dma-iommu: skip extra sync during unmap w/swiotlb David Stevens
@ 2021-08-13 7:38 ` David Stevens
2021-08-13 8:03 ` Christoph Hellwig
2021-08-13 7:38 ` [PATCH v4 5/6] swiotlb: support aligned swiotlb buffers David Stevens
2021-08-13 7:38 ` [PATCH v4 6/6] dma-iommu: account for min_align_mask David Stevens
5 siblings, 1 reply; 13+ messages in thread
From: David Stevens @ 2021-08-13 7:38 UTC (permalink / raw)
To: Robin Murphy, Christoph Hellwig
Cc: Joerg Roedel, Will Deacon, Lu Baolu, Tom Murphy, iommu,
linux-kernel, David Stevens
From: David Stevens <stevensd@chromium.org>
Introduce a new dev_use_swiotlb function to guard swiotlb code, instead
of overloading dev_is_untrusted. This allows CONFIG_SWIOTLB to be
checked more broadly, so the swiotlb related code can be removed more
aggressively.
Signed-off-by: David Stevens <stevensd@chromium.org>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index f7da4934f7e6..bad813d63ea6 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -317,6 +317,11 @@ static bool dev_is_untrusted(struct device *dev)
return dev_is_pci(dev) && to_pci_dev(dev)->untrusted;
}
+static bool dev_use_swiotlb(struct device *dev)
+{
+ return IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev);
+}
+
/**
* iommu_dma_init_domain - Initialise a DMA mapping domain
* @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
@@ -713,7 +718,7 @@ static void iommu_dma_sync_single_for_cpu(struct device *dev,
{
phys_addr_t phys;
- if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
+ if (dev_is_dma_coherent(dev) && !dev_use_swiotlb(dev))
return;
phys = iommu_iova_to_phys(iommu_get_dma_domain(dev), dma_handle);
@@ -729,7 +734,7 @@ static void iommu_dma_sync_single_for_device(struct device *dev,
{
phys_addr_t phys;
- if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
+ if (dev_is_dma_coherent(dev) && !dev_use_swiotlb(dev))
return;
phys = iommu_iova_to_phys(iommu_get_dma_domain(dev), dma_handle);
@@ -747,7 +752,7 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev,
struct scatterlist *sg;
int i;
- if (dev_is_untrusted(dev))
+ if (dev_use_swiotlb(dev))
for_each_sg(sgl, sg, nelems, i)
iommu_dma_sync_single_for_cpu(dev, sg_dma_address(sg),
sg->length, dir);
@@ -763,7 +768,7 @@ static void iommu_dma_sync_sg_for_device(struct device *dev,
struct scatterlist *sg;
int i;
- if (dev_is_untrusted(dev))
+ if (dev_use_swiotlb(dev))
for_each_sg(sgl, sg, nelems, i)
iommu_dma_sync_single_for_device(dev,
sg_dma_address(sg),
@@ -790,7 +795,7 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
* If both the physical buffer start address and size are
* page aligned, we don't need to use a bounce page.
*/
- if (IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev) &&
+ if (IS_ENABLED(CONFIG_SWIOTLB) && dev_use_swiotlb(dev) &&
iova_offset(iovad, phys | size)) {
void *padding_start;
size_t padding_size;
@@ -975,7 +980,7 @@ static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
iommu_deferred_attach(dev, domain))
return 0;
- if (dev_is_untrusted(dev))
+ if (dev_use_swiotlb(dev))
return iommu_dma_map_sg_swiotlb(dev, sg, nents, dir, attrs);
if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
@@ -1047,7 +1052,7 @@ static void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
struct scatterlist *tmp;
int i;
- if (dev_is_untrusted(dev)) {
+ if (dev_use_swiotlb(dev)) {
iommu_dma_unmap_sg_swiotlb(dev, sg, nents, dir, attrs);
return;
}
--
2.33.0.rc1.237.g0d66db33f3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 5/6] swiotlb: support aligned swiotlb buffers
2021-08-13 7:38 [PATCH v4 0/6] Fixes for dma-iommu swiotlb bounce buffers David Stevens
` (3 preceding siblings ...)
2021-08-13 7:38 ` [PATCH v4 4/6] dma-iommu: Check CONFIG_SWIOTLB more broadly David Stevens
@ 2021-08-13 7:38 ` David Stevens
2021-08-13 8:04 ` Christoph Hellwig
2021-08-14 8:39 ` kernel test robot
2021-08-13 7:38 ` [PATCH v4 6/6] dma-iommu: account for min_align_mask David Stevens
5 siblings, 2 replies; 13+ messages in thread
From: David Stevens @ 2021-08-13 7:38 UTC (permalink / raw)
To: Robin Murphy, Christoph Hellwig
Cc: Joerg Roedel, Will Deacon, Lu Baolu, Tom Murphy, iommu,
linux-kernel, David Stevens
From: David Stevens <stevensd@chromium.org>
Add an argument to swiotlb_tbl_map_single that specifies the desired
alignment of the allocated buffer. This is used by dma-iommu to ensure
the buffer is aligned to the iova granule size when using swiotlb with
untrusted sub-granule mappings. This addresses an issue where adjacent
slots could be exposed to the untrusted device if IO_TLB_SIZE < iova
granule < PAGE_SIZE.
Signed-off-by: David Stevens <stevensd@chromium.org>
---
drivers/iommu/dma-iommu.c | 4 ++--
include/linux/swiotlb.h | 3 ++-
kernel/dma/swiotlb.c | 11 +++++++----
3 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index bad813d63ea6..b1b0327cc2f6 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -801,8 +801,8 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
size_t padding_size;
aligned_size = iova_align(iovad, size);
- phys = swiotlb_tbl_map_single(dev, phys, size,
- aligned_size, dir, attrs);
+ phys = swiotlb_tbl_map_single(dev, phys, size, aligned_size,
+ iova_mask(iovad), dir, attrs);
if (phys == DMA_MAPPING_ERROR)
return DMA_MAPPING_ERROR;
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 216854a5e513..93d82e43eb3a 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -44,7 +44,8 @@ extern void __init swiotlb_update_mem_attributes(void);
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);
+ unsigned int alloc_aligned_mask, 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 e50df8d8f87e..d4c45d8cd1fa 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -427,7 +427,7 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, unsigned int index)
* allocate a buffer from that IO TLB pool.
*/
static int find_slots(struct device *dev, phys_addr_t orig_addr,
- size_t alloc_size)
+ size_t alloc_size, unsigned int alloc_align_mask)
{
struct io_tlb_mem *mem = io_tlb_default_mem;
unsigned long boundary_mask = dma_get_seg_boundary(dev);
@@ -450,6 +450,7 @@ static int find_slots(struct device *dev, phys_addr_t orig_addr,
stride = (iotlb_align_mask >> IO_TLB_SHIFT) + 1;
if (alloc_size >= PAGE_SIZE)
stride = max(stride, stride << (PAGE_SHIFT - IO_TLB_SHIFT));
+ stride = max(stride, (alloc_align_mask >> IO_TLB_SHIFT) + 1);
spin_lock_irqsave(&mem->lock, flags);
if (unlikely(nslots > mem->nslabs - mem->used))
@@ -504,7 +505,8 @@ static int find_slots(struct device *dev, phys_addr_t orig_addr,
phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
size_t mapping_size, size_t alloc_size,
- enum dma_data_direction dir, unsigned long attrs)
+ unsigned int alloc_align_mask, enum dma_data_direction dir,
+ unsigned long attrs)
{
struct io_tlb_mem *mem = io_tlb_default_mem;
unsigned int offset = swiotlb_align_offset(dev, orig_addr);
@@ -524,7 +526,8 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
return (phys_addr_t)DMA_MAPPING_ERROR;
}
- index = find_slots(dev, orig_addr, alloc_size + offset);
+ index = find_slots(dev, orig_addr,
+ alloc_size + offset, alloc_align_mask);
if (index == -1) {
if (!(attrs & DMA_ATTR_NO_WARN))
dev_warn_ratelimited(dev,
@@ -636,7 +639,7 @@ 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, paddr, size, size, dir,
+ swiotlb_addr = swiotlb_tbl_map_single(dev, paddr, size, size, 0, dir,
attrs);
if (swiotlb_addr == (phys_addr_t)DMA_MAPPING_ERROR)
return DMA_MAPPING_ERROR;
--
2.33.0.rc1.237.g0d66db33f3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v4 5/6] swiotlb: support aligned swiotlb buffers
2021-08-13 7:38 ` [PATCH v4 5/6] swiotlb: support aligned swiotlb buffers David Stevens
@ 2021-08-13 8:04 ` Christoph Hellwig
2021-08-14 8:39 ` kernel test robot
1 sibling, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2021-08-13 8:04 UTC (permalink / raw)
To: David Stevens
Cc: Robin Murphy, Christoph Hellwig, Joerg Roedel, Will Deacon,
Lu Baolu, Tom Murphy, iommu, linux-kernel
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 5/6] swiotlb: support aligned swiotlb buffers
2021-08-13 7:38 ` [PATCH v4 5/6] swiotlb: support aligned swiotlb buffers David Stevens
2021-08-13 8:04 ` Christoph Hellwig
@ 2021-08-14 8:39 ` kernel test robot
1 sibling, 0 replies; 13+ messages in thread
From: kernel test robot @ 2021-08-14 8:39 UTC (permalink / raw)
To: David Stevens, Robin Murphy, Christoph Hellwig
Cc: kbuild-all, Joerg Roedel, Will Deacon, Lu Baolu, Tom Murphy,
iommu, linux-kernel, David Stevens
[-- Attachment #1: Type: text/plain, Size: 7246 bytes --]
Hi David,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on iommu/next]
[also build test ERROR on hch-configfs/for-next linus/master v5.14-rc5]
[cannot apply to swiotlb/linux-next next-20210813]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/David-Stevens/Fixes-for-dma-iommu-swiotlb-bounce-buffers/20210813-154739
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: x86_64-randconfig-a003-20210812 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/50aeec27cc4ccaa914c0bbefa59e349278646b6e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review David-Stevens/Fixes-for-dma-iommu-swiotlb-bounce-buffers/20210813-154739
git checkout 50aeec27cc4ccaa914c0bbefa59e349278646b6e
# save the attached .config to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/xen/swiotlb-xen.c: In function 'xen_swiotlb_map_page':
>> drivers/xen/swiotlb-xen.c:385:8: error: too few arguments to function 'swiotlb_tbl_map_single'
385 | map = swiotlb_tbl_map_single(dev, phys, size, size, dir, attrs);
| ^~~~~~~~~~~~~~~~~~~~~~
In file included from arch/x86/include/asm/swiotlb.h:5,
from arch/x86/include/asm/dma-mapping.h:12,
from include/linux/dma-map-ops.h:75,
from include/linux/dma-direct.h:10,
from drivers/xen/swiotlb-xen.c:30:
include/linux/swiotlb.h:45:13: note: declared here
45 | phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t phys,
| ^~~~~~~~~~~~~~~~~~~~~~
vim +/swiotlb_tbl_map_single +385 drivers/xen/swiotlb-xen.c
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 352
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 353 /*
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 354 * Map a single buffer of the indicated size for DMA in streaming mode. The
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 355 * physical address to use is returned.
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 356 *
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 357 * Once the device is given the dma address, the device owns this memory until
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 358 * either xen_swiotlb_unmap_page or xen_swiotlb_dma_sync_single is performed.
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 359 */
dceb1a6819ab2c Christoph Hellwig 2017-05-21 360 static dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 361 unsigned long offset, size_t size,
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 362 enum dma_data_direction dir,
00085f1efa387a Krzysztof Kozlowski 2016-08-03 363 unsigned long attrs)
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 364 {
e05ed4d1fad9e7 Alexander Duyck 2012-10-15 365 phys_addr_t map, phys = page_to_phys(page) + offset;
91ffe4ad534ab2 Stefano Stabellini 2020-07-10 366 dma_addr_t dev_addr = xen_phys_to_dma(dev, phys);
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 367
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 368 BUG_ON(dir == DMA_NONE);
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 369 /*
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 370 * If the address happens to be in the device's DMA window,
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 371 * we can safely return the device addr and not worry about bounce
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 372 * buffering it.
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 373 */
68a33b1794665b Christoph Hellwig 2019-11-19 374 if (dma_capable(dev, dev_addr, size, true) &&
a4dba130891271 Stefano Stabellini 2014-11-21 375 !range_straddles_page_boundary(phys, size) &&
291be10fd75111 Julien Grall 2015-09-09 376 !xen_arch_need_swiotlb(dev, phys, dev_addr) &&
063b8271ec8f70 Christoph Hellwig 2019-04-11 377 swiotlb_force != SWIOTLB_FORCE)
063b8271ec8f70 Christoph Hellwig 2019-04-11 378 goto done;
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 379
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 380 /*
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 381 * Oh well, have to allocate and map a bounce buffer.
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 382 */
2b2b614dd24e4e Zoltan Kiss 2013-09-04 383 trace_swiotlb_bounced(dev, dev_addr, size, swiotlb_force);
2b2b614dd24e4e Zoltan Kiss 2013-09-04 384
fc0021aa340af6 Christoph Hellwig 2020-10-23 @385 map = swiotlb_tbl_map_single(dev, phys, size, size, dir, attrs);
9c106119f6538f Arnd Bergmann 2019-06-17 386 if (map == (phys_addr_t)DMA_MAPPING_ERROR)
a4abe0ad10654b Christoph Hellwig 2018-11-21 387 return DMA_MAPPING_ERROR;
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 388
b4dca1512941aa Christoph Hellwig 2019-09-05 389 phys = map;
91ffe4ad534ab2 Stefano Stabellini 2020-07-10 390 dev_addr = xen_phys_to_dma(dev, map);
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 391
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 392 /*
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 393 * Ensure that the address returned is DMA'ble
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 394 */
68a33b1794665b Christoph Hellwig 2019-11-19 395 if (unlikely(!dma_capable(dev, dev_addr, size, true))) {
2973073a80b46d Christoph Hellwig 2021-03-01 396 swiotlb_tbl_unmap_single(dev, map, size, dir,
063b8271ec8f70 Christoph Hellwig 2019-04-11 397 attrs | DMA_ATTR_SKIP_CPU_SYNC);
a4abe0ad10654b Christoph Hellwig 2018-11-21 398 return DMA_MAPPING_ERROR;
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 399 }
b097186fd29d5b Konrad Rzeszutek Wilk 2010-05-11 400
063b8271ec8f70 Christoph Hellwig 2019-04-11 401 done:
63f0620cc552c4 Stefano Stabellini 2020-07-10 402 if (!dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC)) {
63f0620cc552c4 Stefano Stabellini 2020-07-10 403 if (pfn_valid(PFN_DOWN(dma_to_phys(dev, dev_addr))))
63f0620cc552c4 Stefano Stabellini 2020-07-10 404 arch_sync_dma_for_device(phys, size, dir);
63f0620cc552c4 Stefano Stabellini 2020-07-10 405 else
63f0620cc552c4 Stefano Stabellini 2020-07-10 406 xen_dma_sync_for_device(dev, dev_addr, size, dir);
63f0620cc552c4 Stefano Stabellini 2020-07-10 407 }
063b8271ec8f70 Christoph Hellwig 2019-04-11 408 return dev_addr;
063b8271ec8f70 Christoph Hellwig 2019-04-11 409 }
063b8271ec8f70 Christoph Hellwig 2019-04-11 410
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37812 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 6/6] dma-iommu: account for min_align_mask
2021-08-13 7:38 [PATCH v4 0/6] Fixes for dma-iommu swiotlb bounce buffers David Stevens
` (4 preceding siblings ...)
2021-08-13 7:38 ` [PATCH v4 5/6] swiotlb: support aligned swiotlb buffers David Stevens
@ 2021-08-13 7:38 ` David Stevens
5 siblings, 0 replies; 13+ messages in thread
From: David Stevens @ 2021-08-13 7:38 UTC (permalink / raw)
To: Robin Murphy, Christoph Hellwig
Cc: Joerg Roedel, Will Deacon, Lu Baolu, Tom Murphy, iommu,
linux-kernel, David Stevens
From: David Stevens <stevensd@chromium.org>
For devices which set min_align_mask, swiotlb preserves the offset of
the original physical address within that mask. Since __iommu_dma_map
accounts for non-aligned addresses, passing a non-aligned swiotlb
address with the swiotlb aligned size results in the offset being
accounted for twice in the size passed to iommu_map_atomic. The extra
page exposed to DMA is also not cleaned up by __iommu_dma_unmap, since
that function unmaps with the correct size. This causes mapping failures
if the iova gets reused, due to collisions in the iommu page tables.
To fix this, pass the original size to __iommu_dma_map, since that
function already handles alignment.
Additionally, when swiotlb returns non-aligned addresses, there is
padding at the start of the bounce buffer that needs to be cleared.
Fixes: 1f221a0d0dbf ("swiotlb: respect min_align_mask")
Signed-off-by: David Stevens <stevensd@chromium.org>
---
drivers/iommu/dma-iommu.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index b1b0327cc2f6..b44b367839c2 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -788,7 +788,6 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
struct iommu_domain *domain = iommu_get_dma_domain(dev);
struct iommu_dma_cookie *cookie = domain->iova_cookie;
struct iova_domain *iovad = &cookie->iovad;
- size_t aligned_size = size;
dma_addr_t iova, dma_mask = dma_get_mask(dev);
/*
@@ -797,8 +796,8 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
*/
if (IS_ENABLED(CONFIG_SWIOTLB) && dev_use_swiotlb(dev) &&
iova_offset(iovad, phys | size)) {
- void *padding_start;
- size_t padding_size;
+ void *tlb_start;
+ size_t aligned_size, iova_off, mapping_end_off;
aligned_size = iova_align(iovad, size);
phys = swiotlb_tbl_map_single(dev, phys, size, aligned_size,
@@ -807,24 +806,27 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
if (phys == DMA_MAPPING_ERROR)
return DMA_MAPPING_ERROR;
- /* Cleanup the padding area. */
- padding_start = phys_to_virt(phys);
- padding_size = aligned_size;
+ iova_off = iova_offset(iovad, phys);
+ tlb_start = phys_to_virt(phys - iova_off);
if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
(dir == DMA_TO_DEVICE ||
dir == DMA_BIDIRECTIONAL)) {
- padding_start += size;
- padding_size -= size;
+ /* Cleanup the padding area. */
+ mapping_end_off = iova_off + size;
+ memset(tlb_start, 0, iova_off);
+ memset(tlb_start + mapping_end_off, 0,
+ aligned_size - mapping_end_off);
+ } else {
+ /* Nothing was sync'ed, so clear the whole buffer. */
+ memset(tlb_start, 0, aligned_size);
}
-
- memset(padding_start, 0, padding_size);
}
if (!coherent && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
arch_sync_dma_for_device(phys, size, dir);
- iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask);
+ iova = __iommu_dma_map(dev, phys, size, prot, dma_mask);
if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(phys))
swiotlb_tbl_unmap_single(dev, phys, size, dir, attrs);
return iova;
--
2.33.0.rc1.237.g0d66db33f3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread