* [PATCH] dma-debug: fix physical address calculation for struct dma_debug_entry
@ 2024-11-27 18:59 Fedor Pchelkin
2024-11-28 3:50 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Fedor Pchelkin @ 2024-11-27 18:59 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Fedor Pchelkin, Marek Szyprowski, Robin Murphy, iommu,
linux-kernel, lvc-project
Offset into the page should also be considered while calculating a physical
address for struct dma_debug_entry. page_to_phys() just shifts the value
PAGE_SHIFT bits to the left so offset part is zero-filled.
An example (wrong) debug assertion failure with CONFIG_DMA_API_DEBUG
enabled which is observed during systemd boot process after recent
dma-debug changes:
DMA-API: e1000 0000:00:03.0: cacheline tracking EEXIST, overlapping mappings aren't supported
WARNING: CPU: 4 PID: 941 at kernel/dma/debug.c:596 add_dma_entry
CPU: 4 UID: 0 PID: 941 Comm: ip Not tainted 6.12.0+ #288
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:add_dma_entry kernel/dma/debug.c:596
Call Trace:
<TASK>
debug_dma_map_page kernel/dma/debug.c:1236
dma_map_page_attrs kernel/dma/mapping.c:179
e1000_alloc_rx_buffers drivers/net/ethernet/intel/e1000/e1000_main.c:4616
...
Found by Linux Verification Center (linuxtesting.org).
Fixes: 9d4f645a1fd4 ("dma-debug: store a phys_addr_t in struct dma_debug_entry")
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
kernel/dma/debug.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
index 295396226f31..27ade2bab531 100644
--- a/kernel/dma/debug.c
+++ b/kernel/dma/debug.c
@@ -1219,7 +1219,7 @@ void debug_dma_map_page(struct device *dev, struct page *page, size_t offset,
entry->dev = dev;
entry->type = dma_debug_single;
- entry->paddr = page_to_phys(page);
+ entry->paddr = page_to_phys(page) + offset;
entry->dev_addr = dma_addr;
entry->size = size;
entry->direction = direction;
@@ -1400,7 +1400,8 @@ void debug_dma_alloc_coherent(struct device *dev, size_t size,
entry->type = dma_debug_coherent;
entry->dev = dev;
entry->paddr = page_to_phys((is_vmalloc_addr(virt) ?
- vmalloc_to_page(virt) : virt_to_page(virt)));
+ vmalloc_to_page(virt) : virt_to_page(virt))) +
+ offset_in_page(virt);
entry->size = size;
entry->dev_addr = dma_addr;
entry->direction = DMA_BIDIRECTIONAL;
@@ -1424,7 +1425,8 @@ void debug_dma_free_coherent(struct device *dev, size_t size,
return;
ref.paddr = page_to_phys((is_vmalloc_addr(virt) ?
- vmalloc_to_page(virt) : virt_to_page(virt)));
+ vmalloc_to_page(virt) : virt_to_page(virt))) +
+ offset_in_page(virt);
if (unlikely(dma_debug_disabled()))
return;
--
2.39.5
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] dma-debug: fix physical address calculation for struct dma_debug_entry
2024-11-27 18:59 [PATCH] dma-debug: fix physical address calculation for struct dma_debug_entry Fedor Pchelkin
@ 2024-11-28 3:50 ` Christoph Hellwig
2024-11-28 8:50 ` Fedor Pchelkin
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2024-11-28 3:50 UTC (permalink / raw)
To: Fedor Pchelkin
Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, iommu,
linux-kernel, lvc-project
On Wed, Nov 27, 2024 at 09:59:26PM +0300, Fedor Pchelkin wrote:
> Offset into the page should also be considered while calculating a physical
> address for struct dma_debug_entry. page_to_phys() just shifts the value
> PAGE_SHIFT bits to the left so offset part is zero-filled.
>
> An example (wrong) debug assertion failure with CONFIG_DMA_API_DEBUG
> enabled which is observed during systemd boot process after recent
> dma-debug changes:
Thanks!
Is it ok for you if I fold in the following cleanup to have a helper
instead of the duplicate very dense expression?
diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
index 27ade2bab531..e43c6de2bce4 100644
--- a/kernel/dma/debug.c
+++ b/kernel/dma/debug.c
@@ -1377,6 +1377,18 @@ void debug_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
}
}
+static phys_addr_t virt_to_paddr(void *virt)
+{
+ struct page *page;
+
+ if (is_vmalloc_addr(virt))
+ page = vmalloc_to_page(virt);
+ else
+ page = virt_to_page(virt);
+
+ return page_to_phys(page) + offset_in_page(virt);
+}
+
void debug_dma_alloc_coherent(struct device *dev, size_t size,
dma_addr_t dma_addr, void *virt,
unsigned long attrs)
@@ -1399,9 +1411,7 @@ void debug_dma_alloc_coherent(struct device *dev, size_t size,
entry->type = dma_debug_coherent;
entry->dev = dev;
- entry->paddr = page_to_phys((is_vmalloc_addr(virt) ?
- vmalloc_to_page(virt) : virt_to_page(virt))) +
- offset_in_page(virt);
+ entry->paddr = virt_to_paddr(virt);
entry->size = size;
entry->dev_addr = dma_addr;
entry->direction = DMA_BIDIRECTIONAL;
@@ -1424,9 +1434,7 @@ void debug_dma_free_coherent(struct device *dev, size_t size,
if (!is_vmalloc_addr(virt) && !virt_addr_valid(virt))
return;
- ref.paddr = page_to_phys((is_vmalloc_addr(virt) ?
- vmalloc_to_page(virt) : virt_to_page(virt))) +
- offset_in_page(virt);
+ ref.paddr = virt_to_paddr(virt);
if (unlikely(dma_debug_disabled()))
return;
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] dma-debug: fix physical address calculation for struct dma_debug_entry
2024-11-28 3:50 ` Christoph Hellwig
@ 2024-11-28 8:50 ` Fedor Pchelkin
0 siblings, 0 replies; 3+ messages in thread
From: Fedor Pchelkin @ 2024-11-28 8:50 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Marek Szyprowski, Robin Murphy, iommu, linux-kernel, lvc-project
On Thu, 28. Nov 04:50, Christoph Hellwig wrote:
> Is it ok for you if I fold in the following cleanup to have a helper
> instead of the duplicate very dense expression?
>
> diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
> index 27ade2bab531..e43c6de2bce4 100644
> --- a/kernel/dma/debug.c
> +++ b/kernel/dma/debug.c
> @@ -1377,6 +1377,18 @@ void debug_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
> }
> }
>
> +static phys_addr_t virt_to_paddr(void *virt)
> +{
> + struct page *page;
> +
> + if (is_vmalloc_addr(virt))
> + page = vmalloc_to_page(virt);
> + else
> + page = virt_to_page(virt);
> +
> + return page_to_phys(page) + offset_in_page(virt);
> +}
> +
> void debug_dma_alloc_coherent(struct device *dev, size_t size,
> dma_addr_t dma_addr, void *virt,
> unsigned long attrs)
> @@ -1399,9 +1411,7 @@ void debug_dma_alloc_coherent(struct device *dev, size_t size,
>
> entry->type = dma_debug_coherent;
> entry->dev = dev;
> - entry->paddr = page_to_phys((is_vmalloc_addr(virt) ?
> - vmalloc_to_page(virt) : virt_to_page(virt))) +
> - offset_in_page(virt);
> + entry->paddr = virt_to_paddr(virt);
> entry->size = size;
> entry->dev_addr = dma_addr;
> entry->direction = DMA_BIDIRECTIONAL;
> @@ -1424,9 +1434,7 @@ void debug_dma_free_coherent(struct device *dev, size_t size,
> if (!is_vmalloc_addr(virt) && !virt_addr_valid(virt))
> return;
>
> - ref.paddr = page_to_phys((is_vmalloc_addr(virt) ?
> - vmalloc_to_page(virt) : virt_to_page(virt))) +
> - offset_in_page(virt);
> + ref.paddr = virt_to_paddr(virt);
>
> if (unlikely(dma_debug_disabled()))
> return;
No problem. It actually looks more readable.
--
Thanks,
Fedor
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-11-28 8:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-27 18:59 [PATCH] dma-debug: fix physical address calculation for struct dma_debug_entry Fedor Pchelkin
2024-11-28 3:50 ` Christoph Hellwig
2024-11-28 8:50 ` Fedor Pchelkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox