[PATCH 2/4] dma: add DMA API debugging support
Ahmad Fatoum
a.fatoum at pengutronix.de
Tue Nov 28 22:17:56 PST 2023
For DMA_FROM_DEVICE, calling dma_sync_single_for_cpu
before arch_sync_dma_for_device has been called is wrong:
- Memory region is dirty in CPU cache
- Device writes packet into region
- CPU cache lines are written back
- Buffer memory is corrupted
In order to spot such issues, let's add a new CONFIG_DMA_API_DEBUG
that will warn about mismatch in order.
Signed-off-by: Ahmad Fatoum <a.fatoum at pengutronix.de>
---
common/Kconfig | 14 ++++
drivers/dma/Makefile | 1 +
drivers/dma/debug.c | 183 +++++++++++++++++++++++++++++++++++++++++++
drivers/dma/debug.h | 56 +++++++++++++
drivers/dma/map.c | 13 ++-
5 files changed, 266 insertions(+), 1 deletion(-)
create mode 100644 drivers/dma/debug.c
create mode 100644 drivers/dma/debug.h
diff --git a/common/Kconfig b/common/Kconfig
index 8bd8fa8df655..c8c23a8e03a2 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -1690,6 +1690,20 @@ config DEBUG_PROBES
Most consoles do not implement a remove callback to remain operable until
the very end. Consoles using DMA, however, must be removed.
+config DMA_API_DEBUG
+ bool "Enable debugging of DMA-API usage"
+ depends on HAS_DMA
+ help
+ Enable this option to debug the use of the DMA API by device drivers.
+ With this option you will be able to detect common bugs in device
+ drivers like double-freeing of DMA mappings or freeing mappings that
+ were never allocated.
+
+ This option causes a performance degradation. Use only if you want to
+ debug device drivers and dma interactions.
+
+ If unsure, say N.
+
config PBL_BREAK
bool "Execute software break on pbl start"
depends on ARM && (!CPU_32v4T && !ARCH_TEGRA)
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index e45476c23f14..b55c16e768d5 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_HAS_DMA) += map.o
+obj-$(CONFIG_DMA_API_DEBUG) += debug.o
obj-$(CONFIG_MXS_APBH_DMA) += apbh_dma.o
diff --git a/drivers/dma/debug.c b/drivers/dma/debug.c
new file mode 100644
index 000000000000..b3bfbff9b2f5
--- /dev/null
+++ b/drivers/dma/debug.c
@@ -0,0 +1,183 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <dma.h>
+#include <linux/list.h>
+#include "debug.h"
+
+static LIST_HEAD(dma_mappings);
+
+struct dma_debug_entry {
+ struct list_head list;
+ struct device *dev;
+ dma_addr_t dev_addr;
+ size_t size;
+ int direction;
+};
+
+static const char *dir2name[] = {
+ [DMA_BIDIRECTIONAL] = "bidirectional",
+ [DMA_TO_DEVICE] = "to-device",
+ [DMA_FROM_DEVICE] = "from-device",
+ [DMA_NONE] = "none",
+};
+
+#define dma_dev_printf(level, args...) do { \
+ if (level > LOGLEVEL) \
+ break; \
+ dev_printf((level), args); \
+ if ((level) <= MSG_WARNING) \
+ dump_stack(); \
+} while (0)
+
+#define dma_dev_warn(args...) dma_dev_printf(MSG_WARNING, args)
+
+static void dma_printf(int level, struct dma_debug_entry *entry,
+ const char *fmt, ...)
+{
+ struct va_format vaf;
+ va_list va;
+
+ va_start(va, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &va;
+
+ dma_dev_printf(level, entry->dev, "%s mapping 0x%llx+0x%zx: %pV\n",
+ dir2name[(entry)->direction], (u64)(entry)->dev_addr,
+ (entry)->size, &vaf);
+
+ va_end(va);
+}
+
+#define dma_warn(args...) dma_printf(MSG_WARNING, args)
+#define dma_debug(args...) dma_printf(MSG_DEBUG, args)
+
+static inline int region_contains(struct dma_debug_entry *entry,
+ dma_addr_t buf_start, size_t buf_size)
+{
+ dma_addr_t dev_addr_end = entry->dev_addr + entry->size - 1;
+ dma_addr_t buf_end = buf_start + buf_size - 1;
+
+ /* Is the buffer completely within the mapping? */
+ if (entry->dev_addr <= buf_start && dev_addr_end >= buf_end)
+ return 1;
+
+ /* Does the buffer partially overlap the mapping? */
+ if (entry->dev_addr <= buf_end && dev_addr_end >= buf_start)
+ return -1;
+
+ return 0;
+}
+
+static struct dma_debug_entry *
+dma_debug_entry_find(struct device *dev, dma_addr_t dev_addr, size_t size)
+{
+ struct dma_debug_entry *entry;
+
+ /*
+ * DMA functions should be called with a device argument to support
+ * non-1:1 device mappings.
+ */
+ if (!dev)
+ dma_dev_warn(NULL, "unportable NULL device passed with buffer 0x%llx+0x%zx!\n",
+ (u64)dev_addr, size);
+
+ list_for_each_entry(entry, &dma_mappings, list) {
+ if (dev != entry->dev)
+ continue;
+
+ switch (region_contains(entry, dev_addr, size)) {
+ case 1:
+ return entry;
+ case -1:
+ /* The same device shouldn't have two mappings for the same address */
+ dma_warn(entry, "unexpected partial overlap looking for 0x%llx+0x%zx!\n",
+ (u64)dev_addr, size);
+ fallthrough;
+ case 0:
+ continue;
+ }
+ }
+
+ return NULL;
+}
+
+void debug_dma_map(struct device *dev, void *addr,
+ size_t size,
+ int direction, dma_addr_t dev_addr)
+{
+ struct dma_debug_entry *entry;
+
+ entry = dma_debug_entry_find(dev, dev_addr, size);
+ if (entry) {
+ /* The same device shouldn't have two mappings for the same address */
+ dma_warn(entry, "duplicate mapping\n");
+ return;
+ }
+
+ entry = xmalloc(sizeof(*entry));
+
+ entry->dev = dev;
+ entry->dev_addr = dev_addr;
+ entry->size = size;
+ entry->direction = direction;
+
+ list_add(&entry->list, &dma_mappings);
+
+ dma_debug(entry, "allocated\n");
+}
+
+void debug_dma_unmap(struct device *dev, dma_addr_t addr,
+ size_t size, int direction)
+{
+ struct dma_debug_entry *entry;
+
+ entry = dma_debug_entry_find(dev, addr, size);
+ if (!entry) {
+ /* Potential double free */
+ dma_dev_warn(dev, "Unmapping non-mapped %s buffer 0x%llx+0x%zx!\n",
+ dir2name[direction], (u64)addr, size);
+ return;
+ }
+
+ /* Mismatched size or direction may result in memory corruption */
+ if (entry->size != size)
+ dma_warn(entry, "mismatch unmapping 0x%zx bytes\n", size);
+ if (entry->direction != direction)
+ dma_warn(entry, "mismatch unmapping %s\n",
+ dir2name[direction]);
+
+ dma_debug(entry, "deallocating\n");
+ list_del(&entry->list);
+ free(entry);
+}
+
+void debug_dma_sync_single_for_cpu(struct device *dev,
+ dma_addr_t dma_handle, size_t size,
+ int direction)
+{
+ struct dma_debug_entry *entry;
+
+ entry = dma_debug_entry_find(dev, dma_handle, size);
+ if (!entry)
+ dma_dev_warn(dev, "sync for CPU of never-mapped %s buffer 0x%llx+0x%zx!\n",
+ dir2name[direction], (u64)dma_handle, size);
+}
+
+void debug_dma_sync_single_for_device(struct device *dev,
+ dma_addr_t dma_handle,
+ size_t size, int direction)
+{
+ struct dma_debug_entry *entry;
+
+ /*
+ * If dma_map_single was omitted, CPU cache may contain dirty cache lines
+ * for a buffer used for DMA. These lines may be evicted and written back
+ * after device DMA and before consumption by CPU, resulting in memory
+ * corruption
+ */
+ entry = dma_debug_entry_find(dev, dma_handle, size);
+ if (!entry)
+ dma_dev_warn(dev, "Syncing for device of never-mapped %s buffer 0x%llx+0x%zx!\n",
+ dir2name[direction], (u64)dma_handle, size);
+}
diff --git a/drivers/dma/debug.h b/drivers/dma/debug.h
new file mode 100644
index 000000000000..020bb5c19678
--- /dev/null
+++ b/drivers/dma/debug.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2008 Advanced Micro Devices, Inc.
+ *
+ * Author: Joerg Roedel <joerg.roedel at amd.com>
+ */
+
+#ifndef _KERNEL_DMA_DEBUG_H
+#define _KERNEL_DMA_DEBUG_H
+
+#include <linux/types.h>
+
+struct device;
+
+#ifdef CONFIG_DMA_API_DEBUG
+extern void debug_dma_map(struct device *dev, void *addr,
+ size_t size,
+ int direction, dma_addr_t dma_addr);
+
+extern void debug_dma_unmap(struct device *dev, dma_addr_t addr,
+ size_t size, int direction);
+
+extern void debug_dma_sync_single_for_cpu(struct device *dev,
+ dma_addr_t dma_handle, size_t size,
+ int direction);
+
+extern void debug_dma_sync_single_for_device(struct device *dev,
+ dma_addr_t dma_handle,
+ size_t size, int direction);
+
+#else /* CONFIG_DMA_API_DEBUG */
+static inline void debug_dma_map(struct device *dev, void *addr,
+ size_t size,
+ int direction, dma_addr_t dma_addr)
+{
+}
+
+static inline void debug_dma_unmap(struct device *dev, dma_addr_t addr,
+ size_t size, int direction)
+{
+}
+
+static inline void debug_dma_sync_single_for_cpu(struct device *dev,
+ dma_addr_t dma_handle,
+ size_t size, int direction)
+{
+}
+
+static inline void debug_dma_sync_single_for_device(struct device *dev,
+ dma_addr_t dma_handle,
+ size_t size, int direction)
+{
+}
+
+#endif /* CONFIG_DMA_API_DEBUG */
+#endif /* _KERNEL_DMA_DEBUG_H */
diff --git a/drivers/dma/map.c b/drivers/dma/map.c
index 270a4899fd05..e320f6aad4ac 100644
--- a/drivers/dma/map.c
+++ b/drivers/dma/map.c
@@ -1,11 +1,14 @@
/* SPDX-License-Identifier: GPL-2.0-only */
#include <dma.h>
+#include "debug.h"
void dma_sync_single_for_cpu(struct device *dev, dma_addr_t address,
size_t size, enum dma_data_direction dir)
{
void *ptr = dma_to_cpu(dev, address);
+ debug_dma_sync_single_for_cpu(dev, address, size, dir);
+
arch_sync_dma_for_cpu(ptr, size, dir);
}
@@ -14,19 +17,27 @@ void dma_sync_single_for_device(struct device *dev, dma_addr_t address,
{
void *ptr = dma_to_cpu(dev, address);
+ debug_dma_sync_single_for_device(dev, address, size, dir);
+
arch_sync_dma_for_device(ptr, size, dir);
}
dma_addr_t dma_map_single(struct device *dev, void *ptr,
size_t size, enum dma_data_direction dir)
{
+ dma_addr_t dma_addr = cpu_to_dma(dev, ptr);
+
+ debug_dma_map(dev, ptr, size, dir, dma_addr);
+
arch_sync_dma_for_device(ptr, size, dir);
- return cpu_to_dma(dev, ptr);
+ return dma_addr;
}
void dma_unmap_single(struct device *dev, dma_addr_t dma_addr,
size_t size, enum dma_data_direction dir)
{
dma_sync_single_for_cpu(dev, dma_addr, size, dir);
+
+ debug_dma_unmap(dev, dma_addr, size, dir);
}
--
2.39.2
More information about the barebox
mailing list