Skip to content

Commit 5f89468

Browse files
bumyongleekonradwilk
authored andcommitted
swiotlb: manipulate orig_addr when tlb_addr has offset
in case of driver wants to sync part of ranges with offset, swiotlb_tbl_sync_single() copies from orig_addr base to tlb_addr with offset and ends up with data mismatch. It was removed from "swiotlb: don't modify orig_addr in swiotlb_tbl_sync_single", but said logic has to be added back in. From Linus's email: "That commit which the removed the offset calculation entirely, because the old (unsigned long)tlb_addr & (IO_TLB_SIZE - 1) was wrong, but instead of removing it, I think it should have just fixed it to be (tlb_addr - mem->start) & (IO_TLB_SIZE - 1); instead. That way the slot offset always matches the slot index calculation." (Unfortunatly that broke NVMe). The use-case that drivers are hitting is as follow: 1. Get dma_addr_t from dma_map_single() dma_addr_t tlb_addr = dma_map_single(dev, vaddr, vsize, DMA_TO_DEVICE); |<---------------vsize------------->| +-----------------------------------+ | | original buffer +-----------------------------------+ vaddr swiotlb_align_offset |<----->|<---------------vsize------------->| +-------+-----------------------------------+ | | | swiotlb buffer +-------+-----------------------------------+ tlb_addr 2. Do something 3. Sync dma_addr_t through dma_sync_single_for_device(..) dma_sync_single_for_device(dev, tlb_addr + offset, size, DMA_TO_DEVICE); Error case. Copy data to original buffer but it is from base addr (instead of base addr + offset) in original buffer: swiotlb_align_offset |<----->|<- offset ->|<- size ->| +-------+-----------------------------------+ | | |##########| | swiotlb buffer +-------+-----------------------------------+ tlb_addr |<- size ->| +-----------------------------------+ |##########| | original buffer +-----------------------------------+ vaddr The fix is to copy the data to the original buffer and take into account the offset, like so: swiotlb_align_offset |<----->|<- offset ->|<- size ->| +-------+-----------------------------------+ | | |##########| | swiotlb buffer +-------+-----------------------------------+ tlb_addr |<- offset ->|<- size ->| +-----------------------------------+ | |##########| | original buffer +-----------------------------------+ vaddr [One fix which was Linus's that made more sense to as it created a symmetry would break NVMe. The reason for that is the: unsigned int offset = (tlb_addr - mem->start) & (IO_TLB_SIZE - 1); would come up with the proper offset, but it would lose the alignment (which this patch contains).] Fixes: 16fc3ce ("swiotlb: don't modify orig_addr in swiotlb_tbl_sync_single") Signed-off-by: Bumyong Lee <[email protected]> Signed-off-by: Chanho Park <[email protected]> Reviewed-by: Christoph Hellwig <[email protected]> Reported-by: Dominique MARTINET <[email protected]> Reported-by: Horia Geantă <[email protected]> Tested-by: Horia Geantă <[email protected]> CC: [email protected] Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
1 parent dfc06b3 commit 5f89468

File tree

1 file changed

+15
-8
lines changed

1 file changed

+15
-8
lines changed

kernel/dma/swiotlb.c

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,14 @@ void __init swiotlb_exit(void)
334334
io_tlb_default_mem = NULL;
335335
}
336336

337+
/*
338+
* Return the offset into a iotlb slot required to keep the device happy.
339+
*/
340+
static unsigned int swiotlb_align_offset(struct device *dev, u64 addr)
341+
{
342+
return addr & dma_get_min_align_mask(dev) & (IO_TLB_SIZE - 1);
343+
}
344+
337345
/*
338346
* Bounce: copy the swiotlb buffer from or back to the original dma location
339347
*/
@@ -346,10 +354,17 @@ static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size
346354
size_t alloc_size = mem->slots[index].alloc_size;
347355
unsigned long pfn = PFN_DOWN(orig_addr);
348356
unsigned char *vaddr = phys_to_virt(tlb_addr);
357+
unsigned int tlb_offset;
349358

350359
if (orig_addr == INVALID_PHYS_ADDR)
351360
return;
352361

362+
tlb_offset = (tlb_addr & (IO_TLB_SIZE - 1)) -
363+
swiotlb_align_offset(dev, orig_addr);
364+
365+
orig_addr += tlb_offset;
366+
alloc_size -= tlb_offset;
367+
353368
if (size > alloc_size) {
354369
dev_WARN_ONCE(dev, 1,
355370
"Buffer overflow detected. Allocation size: %zu. Mapping size: %zu.\n",
@@ -390,14 +405,6 @@ static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size
390405

391406
#define slot_addr(start, idx) ((start) + ((idx) << IO_TLB_SHIFT))
392407

393-
/*
394-
* Return the offset into a iotlb slot required to keep the device happy.
395-
*/
396-
static unsigned int swiotlb_align_offset(struct device *dev, u64 addr)
397-
{
398-
return addr & dma_get_min_align_mask(dev) & (IO_TLB_SIZE - 1);
399-
}
400-
401408
/*
402409
* Carefully handle integer overflow which can occur when boundary_mask == ~0UL.
403410
*/

0 commit comments

Comments
 (0)