Skip to contentSkip to conversation

SWIOTLB fixes for 4.20

PATCH In review Robin Murphy 3 participants 5 messages Nov 21, 2018 – Nov 27, 2018 DMA mapping helpers

lore
Share & tools
lore

Robin Murphy

author reviewer
#0 lore raw

v2 cover SWIOTLB fixes for 4.20

Here's a quick v2 addressing Christoph's nits and collecting the tags given so far. I've reproduced my USB problem with a commit prior to the dma-mapping pull, so it looks like the cache maintenance changes are off the hook for that one (and I'll have to venture into USB/UAS territory).

No objection from me if Konrad wants to pick this up instead depending on timing.

Robin.

Robin Murphy (2):
dma-direct: Make DIRECT_MAPPING_ERROR viable for SWIOTLB
swiotlb: Skip cache maintenance on map error

 include/linux/dma-direct.h | 2 +-
 kernel/dma/swiotlb.c       | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)
Signature · 1 line
2.19.1.dirty
author reviewer
#1 lore raw

PATCH v2 1/2 dma-direct: Make DIRECT_MAPPING_ERROR viable for SWIOTLB

With the overflow buffer removed, we no longer have a unique address which is guaranteed not to be a valid DMA target to use as an error token. The DIRECT_MAPPING_ERROR value of 0 tries to at least represent an unlikely DMA target, but unfortunately there are already SWIOTLB users with DMA-able memory at physical address 0 which now gets falsely treated as a mapping failure and leads to all manner of misbehaviour.

The best we can do to mitigate that is flip DIRECT_MAPPING_ERROR to the other commonly-used error value of all-bits-set, since the last single byte of memory is by far the least-likely-valid DMA target.

---

v2: Add parentheses, tweak commit message, collect tags

 include/linux/dma-direct.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/dma-direct.h +1 -1
index bd73e7a91410..9e66bfe369aa 100644
@@ -5,7 +5,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/mem_encrypt.h>
 
-#define DIRECT_MAPPING_ERROR		0
+#define DIRECT_MAPPING_ERROR		(~(dma_addr_t)0)
 
 #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
 #include <asm/dma-direct.h>
Signature · 1 line
2.19.1.dirty
author reviewer
#2 lore raw

PATCH v2 2/2 swiotlb: Skip cache maintenance on map error

In reply to Robin Murphy

If swiotlb_bounce_page() failed, calling arch_sync_dma_for_device() may lead to such delights as performing cache maintenance on whatever address phys_to_virt(SWIOTLB_MAP_ERROR) looks like, which is typically outside the kernel memory map and goes about as well as expected.

Don't do that.

Fixes a4a4330db46a ("swiotlb: add support for non-coherent DMA")

---

v2: Collect tags

 kernel/dma/swiotlb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/dma/swiotlb.c +2 -1
index 5731daa09a32..045930e32c0e 100644
@@ -679,7 +679,8 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 	}
 
 	if (!dev_is_dma_coherent(dev) &&
-	    (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
+	    (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0 &&
+	    dev_addr != DIRECT_MAPPING_ERROR)
 		arch_sync_dma_for_device(dev, phys, size, dir);
 
 	return dev_addr;
Signature · 1 line
2.19.1.dirty
#4 lore raw
In reply to Robin Murphy

Thanks,

aplied to the dma-mapping for-linus tree.

I'll wait a little longer for a review from Stefano, but I really want it in linux-next asap and hopefully off to Linus for the weekend.

Acked-by on v2 2/2
#3 lore raw

Re: [PATCH v2 2/2] swiotlb: Skip cache maintenance on map error

In reply to Robin Murphy
Quoted 11 lines · Robin Murphy

If swiotlb_bounce_page() failed, calling arch_sync_dma_for_device() may lead to such delights as performing cache maintenance on whatever address phys_to_virt(SWIOTLB_MAP_ERROR) looks like, which is typically outside the kernel memory map and goes about as well as expected.

Don't do that.

Fixes a4a4330db46a ("swiotlb: add support for non-coherent DMA")
Quoted 24 lines

---

v2: Collect tags

 kernel/dma/swiotlb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/dma/swiotlb.c +2 -1
index 5731daa09a32..045930e32c0e 100644
@@ -679,7 +679,8 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 	}
 
 	if (!dev_is_dma_coherent(dev) &&
-	    (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
+	    (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0 &&
+	    dev_addr != DIRECT_MAPPING_ERROR)
 		arch_sync_dma_for_device(dev, phys, size, dir);
 
 	return dev_addr;
Signature · 1 line
2.19.1.dirty