2009-01-19 14:52:34

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 0/16] DMA-API debugging facility v3

Hi,

this is version 3 of the patchset which introduces code to debug drivers
usage of the DMA-API. Many thanks to all the reviewers and the useful
comments on the previous versions of this patchset. Tests with hardware
IOMMUs have shown several bugs in drivers regarding the usage of that
API.

These bugs often don't show up or have any negative impact if there is
no hardware IOMMU in use in the system. But with an hardware IOMMU these
bugs turn the hardware unusable or, in the worst case, cause data
corruption on devices which are managed by other (good) drivers.

With the code these patches introduce driver developers can find several
bugs of misusing the DMA-API in their drivers. But be aware, it can not
find all possible bugs. What it can not find are use-after-free bugs.

If anyone wants to try this stuff and don't want to apply them one by
one, they can also be pulled from (branch is based on tip/core/iommu)

git://git.kernel.org/pub/scm/linux/kernel/git/joro/linux-2.6-iommu.git dma-api/debug

Changes from v2 -> v3:

- rebased patches against tip/core/iommu branch
- changed storage of virtual address to physical address in
struct dma_debug_entry (thanks Fujita)
- removed usage of x86 specific bad_dma_address (thanks Fujita)
- changed a error log message to be more clear (thanks Roel)
- fixed a bug with wrong handling of map_page/unmap_page requests
(thanks Michael)
- various improvements and fixes suggested by Ingo, thanks
- added more comments

Changes from v1 -> v2:

- moved code to lib/ and include/linux to make it usable for all
architectures
- more fine grained hash locking (locking is now per hash bucket, no
global lock anymore)
- dma_debug_entries are preallocated
- per default the code will only print one warning and is silent then
- added a debugfs interface to see some statistics and to enable more
verbose error reporting in the kernel log
- added command line parameter to disable debugging code
- allocation errors are now handled correctly
- added documentation about this facility for driver developers

Here is the diffstat:

Documentation/DMA-API.txt | 106 +++++
Documentation/kernel-parameters.txt | 10 +
arch/Kconfig | 2 +
arch/x86/Kconfig | 1 +
arch/x86/include/asm/dma-mapping.h | 45 ++-
arch/x86/kernel/pci-dma.c | 6 +
include/linux/dma-debug.h | 160 ++++++++
lib/Kconfig.debug | 11 +
lib/Makefile | 2 +
lib/dma-debug.c | 753 +++++++++++++++++++++++++++++++++++
10 files changed, 1090 insertions(+), 6 deletions(-)



2009-01-19 14:47:33

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 01/16] dma-debug: add Kconfig entry

Impact: add a Kconfig entry for DMA-API debugging

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/Kconfig | 2 ++
lib/Kconfig.debug | 11 +++++++++++
2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 550dab2..830c16a 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -106,3 +106,5 @@ config HAVE_CLK
The <linux/clk.h> calls support software clock gating and
thus are a key power management tool on many systems.

+config HAVE_DMA_API_DEBUG
+ bool
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 4c9ae60..92120f7 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -906,6 +906,17 @@ config DYNAMIC_PRINTK_DEBUG
debugging for all modules. This mode can be turned off via the above
disable command.

+config DMA_API_DEBUG
+ bool "Enable debugging of DMA-API usage"
+ depends on HAVE_DMA_API_DEBUG
+ 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 degredation. Use only if you want
+ to debug device drivers. If unsure, say N.
+
source "samples/Kconfig"

source "lib/Kconfig.kgdb"
--
1.5.6.4

2009-01-19 14:47:55

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 05/16] dma-debug: add initialization code

Impact: add code to initialize dma-debug core data structures

Signed-off-by: Joerg Roedel <[email protected]>
---
include/linux/dma-debug.h | 14 +++++++++
lib/dma-debug.c | 66 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index ce4ace7..345d538 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -20,6 +20,20 @@
#ifndef __DMA_DEBUG_H
#define __DMA_DEBUG_H

+#include <linux/types.h>
+
struct device;

+#ifdef CONFIG_DMA_API_DEBUG
+
+extern void dma_debug_init(u32 num_entries);
+
+#else /* CONFIG_DMA_API_DEBUG */
+
+static inline void dma_debug_init(u32 num_entries)
+{
+}
+
+#endif /* CONFIG_DMA_API_DEBUG */
+
#endif /* __DMA_DEBUG_H */
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index c9e44e8..d46c76e 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -21,6 +21,7 @@
#include <linux/spinlock.h>
#include <linux/types.h>
#include <linux/list.h>
+#include <linux/slab.h>

#define HASH_SIZE 256
#define HASH_FN_SHIFT 13
@@ -196,3 +197,68 @@ static void dma_entry_free(struct dma_debug_entry *entry)
spin_unlock_irqrestore(&free_entries_lock, flags);
}

+/*
+ * DMA-API debugging init code
+ *
+ * The init code does two things:
+ * 1. Initialize core data structures
+ * 2. Preallocate a given number of dma_debug_entry structs
+ */
+
+static int prealloc_memory(u32 num_entries)
+{
+ struct dma_debug_entry *entry, *next_entry;
+ int i;
+
+ for (i = 0; i < num_entries; ++i) {
+ entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+ if (!entry)
+ goto out_err;
+
+ list_add_tail(&entry->list, &free_entries);
+ }
+
+ num_free_entries = num_entries;
+ min_free_entries = num_entries;
+
+ printk(KERN_INFO "DMA-API: preallocated %d debug entries\n",
+ num_entries);
+
+ return 0;
+
+out_err:
+
+ list_for_each_entry_safe(entry, next_entry, &free_entries, list) {
+ list_del(&entry->list);
+ kfree(entry);
+ }
+
+ return -ENOMEM;
+}
+
+/*
+ * Let the architectures decide how many entries should be preallocated.
+ */
+void dma_debug_init(u32 num_entries)
+{
+ int i;
+
+ if (global_disable)
+ return;
+
+ for (i = 0; i < HASH_SIZE; ++i) {
+ INIT_LIST_HEAD(&dma_entry_hash[i].list);
+ dma_entry_hash[i].lock = SPIN_LOCK_UNLOCKED;
+ }
+
+ if (prealloc_memory(num_entries) != 0) {
+ printk(KERN_ERR "DMA-API: debugging out of memory error "
+ "- disabled\n");
+ global_disable = true;
+
+ return;
+ }
+
+ printk(KERN_INFO "DMA-API: debugging enabled by kernel config\n");
+}
+
--
1.5.6.4

2009-01-19 14:48:24

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 06/16] dma-debug: add kernel command line parameters

Impact: add dma_debug= and dma_debug_entries= kernel parameters

Signed-off-by: Joerg Roedel <[email protected]>
---
Documentation/kernel-parameters.txt | 10 +++++++++
lib/dma-debug.c | 38 +++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 8511d35..fef67ed 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -491,6 +491,16 @@ and is between 256 and 4096 characters. It is defined in the file
Range: 0 - 8192
Default: 64

+ dma_debug=off If the kernel is compiled with DMA_API_DEBUG support
+ this option disables the debugging code at boot.
+
+ dma_debug_entries=<number>
+ This option allows to tune the number of preallocated
+ entries for DMA-API debugging code. One entry is
+ required per DMA-API allocation. Use this if the
+ DMA-API debugging code disables itself because the
+ architectural default is too low.
+
hpet= [X86-32,HPET] option to control HPET usage
Format: { enable (default) | disable | force }
disable: disable HPET and use PIT instead
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index d46c76e..66ed8b1 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -62,6 +62,9 @@ static bool global_disable __read_mostly;
static u32 num_free_entries;
static u32 min_free_entries;

+/* number of preallocated entries requested by kernel cmdline */
+static u32 req_entries;
+
/*
* Hash related functions
*
@@ -251,6 +254,9 @@ void dma_debug_init(u32 num_entries)
dma_entry_hash[i].lock = SPIN_LOCK_UNLOCKED;
}

+ if (req_entries)
+ num_entries = req_entries;
+
if (prealloc_memory(num_entries) != 0) {
printk(KERN_ERR "DMA-API: debugging out of memory error "
"- disabled\n");
@@ -262,3 +268,35 @@ void dma_debug_init(u32 num_entries)
printk(KERN_INFO "DMA-API: debugging enabled by kernel config\n");
}

+static __init int dma_debug_cmdline(char *str)
+{
+ if (!str)
+ return -EINVAL;
+
+ if (strncmp(str, "off", 3) == 0) {
+ printk(KERN_INFO "DMA-API: debugging disabled on kernel "
+ "command line\n");
+ global_disable = true;
+ }
+
+ return 0;
+}
+
+static __init int dma_debug_entries_cmdline(char *str)
+{
+ int res;
+
+ if (!str)
+ return -EINVAL;
+
+ res = get_option(&str, &req_entries);
+
+ if (!res)
+ req_entries = 0;
+
+ return 0;
+}
+
+__setup("dma_debug=", dma_debug_cmdline);
+__setup("dma_debug_entries=", dma_debug_entries_cmdline);
+
--
1.5.6.4

2009-01-19 14:48:46

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 09/16] dma-debug: add checking for map/unmap_page/single

Impact: add debug callbacks for dma_{un}map_[page|single]

Signed-off-by: Joerg Roedel <[email protected]>
---
include/linux/dma-debug.h | 23 ++++++++++++++++++++
lib/dma-debug.c | 51 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 74 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index 345d538..ea6a283 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -28,12 +28,35 @@ struct device;

extern void dma_debug_init(u32 num_entries);

+extern void debug_dma_map_page(struct device *dev, struct page *page,
+ size_t offset, size_t size,
+ int direction, dma_addr_t dma_addr,
+ bool map_single);
+
+extern void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
+ size_t size, int direction, bool map_single);
+
+
#else /* CONFIG_DMA_API_DEBUG */

static inline void dma_debug_init(u32 num_entries)
{
}

+static inline void debug_dma_map_page(struct device *dev, struct page *page,
+ size_t offset size_t size,
+ int direction, dma_addr_t dma_addr,
+ bool map_single)
+{
+}
+
+static inline void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
+ size_t size, int direction,
+ bool map_single)
+{
+}
+
+
#endif /* CONFIG_DMA_API_DEBUG */

#endif /* __DMA_DEBUG_H */
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 4d98811..9219c41 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -525,3 +525,54 @@ out:

}

+void debug_dma_map_page(struct device *dev, struct page *page, size_t offset,
+ size_t size, int direction, dma_addr_t dma_addr,
+ bool map_single)
+{
+ struct dma_debug_entry *entry;
+
+ if (unlikely(global_disable))
+ return;
+
+ if (unlikely(dma_mapping_error(dev, dma_addr)))
+ return;
+
+ entry = dma_entry_alloc();
+ if (!entry)
+ return;
+
+ entry->dev = dev;
+ entry->type = dma_debug_page;
+ entry->paddr = page_to_phys(page) + offset;
+ entry->dev_addr = dma_addr;
+ entry->size = size;
+ entry->direction = direction;
+
+ if (map_single)
+ entry->type = dma_debug_single;
+
+ add_dma_entry(entry);
+}
+EXPORT_SYMBOL(debug_dma_map_page);
+
+void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
+ size_t size, int direction, bool map_single)
+{
+ struct dma_debug_entry ref = {
+ .type = dma_debug_page,
+ .dev = dev,
+ .dev_addr = addr,
+ .size = size,
+ .direction = direction,
+ };
+
+ if (unlikely(global_disable))
+ return;
+
+ if (map_single)
+ ref.type = dma_debug_single;
+
+ check_unmap(&ref);
+}
+EXPORT_SYMBOL(debug_dma_unmap_page);
+
--
1.5.6.4

2009-01-19 14:49:13

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 16/16] dma-debug: Documentation update

Impact: add documentation about DMA-API debugging to DMA-API.txt

Signed-off-by: Joerg Roedel <[email protected]>
---
Documentation/DMA-API.txt | 106 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 106 insertions(+), 0 deletions(-)

diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index 5244169..dca8be3 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -609,3 +609,109 @@ size is the size (and should be a page-sized multiple).
The return value will be either a pointer to the processor virtual
address of the memory, or an error (via PTR_ERR()) if any part of the
region is occupied.
+
+Part III - Debug drivers use of the DMA-API
+-------------------------------------------
+
+The DMA-API as described above as some constraints. DMA addresses must be
+released with the corresponding function with the same size for example. With
+the advent of hardware IOMMUs it becomes more and more important that drivers
+do not violate those constraints. In the worst case such a violation can
+result in data corruption up to destroyed filesystems.
+
+To debug drivers and find bugs in the usage of the DMA-API checking code can
+be compiled into the kernel which will tell the developer about those
+violations. If your architecture supports it you can select the "Enable
+debugging of DMA-API usage" option in your kernel configuration. Enabling this
+option has a performance impact. Do not enable it in production kernels.
+
+If you boot the resulting kernel will contain code which does some bookkeeping
+about what DMA memory was allocated for which device. If this code detects an
+error it prints a warning message with some details into your kernel log. An
+example warning message may look like this:
+
+------------[ cut here ]------------
+WARNING: at /data2/repos/linux-2.6-iommu/lib/dma-debug.c:448
+ check_unmap+0x203/0x490()
+Hardware name:
+forcedeth 0000:00:08.0: DMA-API: device driver frees DMA memory with wrong
+ function [device address=0x00000000640444be] [size=66 bytes] [mapped as
+single] [unmapped as page]
+Modules linked in: nfsd exportfs bridge stp llc r8169
+Pid: 0, comm: swapper Tainted: G W 2.6.28-dmatest-09289-g8bb99c0 #1
+Call Trace:
+ <IRQ> [<ffffffff80240b22>] warn_slowpath+0xf2/0x130
+ [<ffffffff80647b70>] _spin_unlock+0x10/0x30
+ [<ffffffff80537e75>] usb_hcd_link_urb_to_ep+0x75/0xc0
+ [<ffffffff80647c22>] _spin_unlock_irqrestore+0x12/0x40
+ [<ffffffff8055347f>] ohci_urb_enqueue+0x19f/0x7c0
+ [<ffffffff80252f96>] queue_work+0x56/0x60
+ [<ffffffff80237e10>] enqueue_task_fair+0x20/0x50
+ [<ffffffff80539279>] usb_hcd_submit_urb+0x379/0xbc0
+ [<ffffffff803b78c3>] cpumask_next_and+0x23/0x40
+ [<ffffffff80235177>] find_busiest_group+0x207/0x8a0
+ [<ffffffff8064784f>] _spin_lock_irqsave+0x1f/0x50
+ [<ffffffff803c7ea3>] check_unmap+0x203/0x490
+ [<ffffffff803c8259>] debug_dma_unmap_page+0x49/0x50
+ [<ffffffff80485f26>] nv_tx_done_optimized+0xc6/0x2c0
+ [<ffffffff80486c13>] nv_nic_irq_optimized+0x73/0x2b0
+ [<ffffffff8026df84>] handle_IRQ_event+0x34/0x70
+ [<ffffffff8026ffe9>] handle_edge_irq+0xc9/0x150
+ [<ffffffff8020e3ab>] do_IRQ+0xcb/0x1c0
+ [<ffffffff8020c093>] ret_from_intr+0x0/0xa
+ <EOI> <4>---[ end trace f6435a98e2a38c0e ]---
+
+The driver developer can find the driver and the device including a stacktrace
+of the DMA-API call which caused this warning.
+
+Per default only the first error will result in a warning message. All other
+errors will only silently counted. This limitation exist to prevent the code
+from flooding your kernel log. To support debugging a device driver this can
+be disabled via debugfs. See the debugfs interface documentation below for
+details.
+
+The debugfs directory for the DMA-API debugging code is called dma-api/. In
+this directory the following files can currently be found:
+
+ dma-api/all_errors This file contains a numeric value. If this
+ value is not equal to zero the debugging code
+ will print a warning for every error it finds
+ into the kernel log. Be carefull with this
+ option. It can easily flood your logs.
+
+ dma-api/disabled This read-only file contains the character 'Y'
+ if the debugging code is disabled. This can
+ happen when it runs out of memory or if it was
+ disabled at boot time
+
+ dma-api/error_count This file is read-only and shows the total
+ numbers of errors found.
+
+ dma-api/num_errors The number in this file shows how many
+ warnings will be printed to the kernel log
+ before it stops. This number is initialized to
+ one at system boot and be set by writing into
+ this file
+
+ dma-api/min_free_entries
+ This read-only file can be read to get the
+ minimum number of free dma_debug_entries the
+ allocator has ever seen. If this value goes
+ down to zero the code will disable itself
+ because it is not longer reliable.
+
+ dma-api/num_free_entries
+ The current number of free dma_debug_entries
+ in the allocator.
+
+If you have this code compiled into your kernel it will be enabled by default.
+If you want to boot without the bookkeeping anyway you can provide
+'dma_debug=off' as a boot parameter. This will disable DMA-API debugging.
+Notice that you can not enable it again at runtime. You have to reboot to do
+so.
+
+When the code disables itself at runtime this is most likely because it ran
+out of dma_debug_entries. These entries are preallocated at boot. The number
+of preallocated entries is defined per architecture. If it is too low for you
+boot with 'dma_debug_entries=<your_desired_number>' to overwrite the
+architectural default.
--
1.5.6.4

2009-01-19 14:49:33

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 15/16] dma-debug: x86 architecture bindings

Impact: make use of DMA-API debugging code in x86

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/Kconfig | 1 +
arch/x86/include/asm/dma-mapping.h | 45 +++++++++++++++++++++++++++++++----
arch/x86/kernel/pci-dma.c | 6 ++++
3 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 73f7fe8..9e7aa97 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -40,6 +40,7 @@ config X86
select HAVE_GENERIC_DMA_COHERENT if X86_32
select HAVE_EFFICIENT_UNALIGNED_ACCESS
select USER_STACKTRACE_SUPPORT
+ select HAVE_DMA_API_DEBUG

config ARCH_DEFCONFIG
string
diff --git a/arch/x86/include/asm/dma-mapping.h b/arch/x86/include/asm/dma-mapping.h
index 5a34780..6a5e070 100644
--- a/arch/x86/include/asm/dma-mapping.h
+++ b/arch/x86/include/asm/dma-mapping.h
@@ -7,6 +7,7 @@
*/

#include <linux/scatterlist.h>
+#include <linux/dma-debug.h>
#include <linux/dma-attrs.h>
#include <asm/io.h>
#include <asm/swiotlb.h>
@@ -56,11 +57,16 @@ dma_map_single(struct device *hwdev, void *ptr, size_t size,
enum dma_data_direction dir)
{
struct dma_map_ops *ops = get_dma_ops(hwdev);
+ dma_addr_t addr;

BUG_ON(!valid_dma_direction(dir));
- return ops->map_page(hwdev, virt_to_page(ptr),
+ addr = ops->map_page(hwdev, virt_to_page(ptr),
(unsigned long)ptr & ~PAGE_MASK, size,
dir, NULL);
+ debug_dma_map_page(hwdev, virt_to_page(ptr),
+ (unsigned long)ptr & ~PAGE_MASK, size,
+ dir, addr, true);
+ return addr;
}

static inline void
@@ -72,6 +78,7 @@ dma_unmap_single(struct device *dev, dma_addr_t addr, size_t size,
BUG_ON(!valid_dma_direction(dir));
if (ops->unmap_page)
ops->unmap_page(dev, addr, size, dir, NULL);
+ debug_dma_unmap_page(dev, addr, size, dir, true);
}

static inline int
@@ -79,9 +86,13 @@ dma_map_sg(struct device *hwdev, struct scatterlist *sg,
int nents, enum dma_data_direction dir)
{
struct dma_map_ops *ops = get_dma_ops(hwdev);
+ int ents;

BUG_ON(!valid_dma_direction(dir));
- return ops->map_sg(hwdev, sg, nents, dir, NULL);
+ ents = ops->map_sg(hwdev, sg, nents, dir, NULL);
+ debug_dma_map_sg(hwdev, sg, ents, dir);
+
+ return ents;
}

static inline void
@@ -91,6 +102,7 @@ dma_unmap_sg(struct device *hwdev, struct scatterlist *sg, int nents,
struct dma_map_ops *ops = get_dma_ops(hwdev);

BUG_ON(!valid_dma_direction(dir));
+ debug_dma_unmap_sg(hwdev, sg, nents, dir);
if (ops->unmap_sg)
ops->unmap_sg(hwdev, sg, nents, dir, NULL);
}
@@ -104,6 +116,7 @@ dma_sync_single_for_cpu(struct device *hwdev, dma_addr_t dma_handle,
BUG_ON(!valid_dma_direction(dir));
if (ops->sync_single_for_cpu)
ops->sync_single_for_cpu(hwdev, dma_handle, size, dir);
+ debug_dma_sync_single_for_cpu(hwdev, dma_handle, size, dir);
flush_write_buffers();
}

@@ -116,6 +129,7 @@ dma_sync_single_for_device(struct device *hwdev, dma_addr_t dma_handle,
BUG_ON(!valid_dma_direction(dir));
if (ops->sync_single_for_device)
ops->sync_single_for_device(hwdev, dma_handle, size, dir);
+ debug_dma_sync_single_for_device(hwdev, dma_handle, size, dir);
flush_write_buffers();
}

@@ -130,6 +144,8 @@ dma_sync_single_range_for_cpu(struct device *hwdev, dma_addr_t dma_handle,
if (ops->sync_single_range_for_cpu)
ops->sync_single_range_for_cpu(hwdev, dma_handle, offset,
size, dir);
+ debug_dma_sync_single_range_for_cpu(hwdev, dma_handle,
+ offset, size, dir);
flush_write_buffers();
}

@@ -144,6 +160,8 @@ dma_sync_single_range_for_device(struct device *hwdev, dma_addr_t dma_handle,
if (ops->sync_single_range_for_device)
ops->sync_single_range_for_device(hwdev, dma_handle,
offset, size, dir);
+ debug_dma_sync_single_range_for_device(hwdev, dma_handle,
+ offset, size, dir);
flush_write_buffers();
}

@@ -156,6 +174,7 @@ dma_sync_sg_for_cpu(struct device *hwdev, struct scatterlist *sg,
BUG_ON(!valid_dma_direction(dir));
if (ops->sync_sg_for_cpu)
ops->sync_sg_for_cpu(hwdev, sg, nelems, dir);
+ debug_dma_sync_sg_for_cpu(hwdev, sg, nelems, dir);
flush_write_buffers();
}

@@ -168,6 +187,7 @@ dma_sync_sg_for_device(struct device *hwdev, struct scatterlist *sg,
BUG_ON(!valid_dma_direction(dir));
if (ops->sync_sg_for_device)
ops->sync_sg_for_device(hwdev, sg, nelems, dir);
+ debug_dma_sync_sg_for_device(hwdev, sg, nelems, dir);

flush_write_buffers();
}
@@ -177,15 +197,24 @@ static inline dma_addr_t dma_map_page(struct device *dev, struct page *page,
enum dma_data_direction dir)
{
struct dma_map_ops *ops = get_dma_ops(dev);
+ dma_addr_t addr;

BUG_ON(!valid_dma_direction(dir));
- return ops->map_page(dev, page, offset, size, dir, NULL);
+ addr = ops->map_page(dev, page, offset, size, dir, NULL);
+ debug_dma_map_page(dev, page, offset, size, dir, addr, false);
+
+ return addr;
}

static inline void dma_unmap_page(struct device *dev, dma_addr_t addr,
size_t size, enum dma_data_direction dir)
{
- dma_unmap_single(dev, addr, size, dir);
+ struct dma_map_ops *ops = get_dma_ops(dev);
+
+ BUG_ON(!valid_dma_direction(dir));
+ if (ops->unmap_page)
+ ops->unmap_page(dev, addr, size, dir, NULL);
+ debug_dma_unmap_page(dev, addr, size, dir, false);
}

static inline void
@@ -250,8 +279,11 @@ dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
if (!ops->alloc_coherent)
return NULL;

- return ops->alloc_coherent(dev, size, dma_handle,
- dma_alloc_coherent_gfp_flags(dev, gfp));
+ memory = ops->alloc_coherent(dev, size, dma_handle,
+ dma_alloc_coherent_gfp_flags(dev, gfp));
+ debug_dma_alloc_coherent(dev, size, *dma_handle, memory);
+
+ return memory;
}

static inline void dma_free_coherent(struct device *dev, size_t size,
@@ -264,6 +296,7 @@ static inline void dma_free_coherent(struct device *dev, size_t size,
if (dma_release_from_coherent(dev, get_order(size), vaddr))
return;

+ debug_dma_free_coherent(dev, size, vaddr, bus);
if (ops->free_coherent)
ops->free_coherent(dev, size, vaddr, bus);
}
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index f293a8d..50d28eb 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -1,4 +1,5 @@
#include <linux/dma-mapping.h>
+#include <linux/dma-debug.h>
#include <linux/dmar.h>
#include <linux/bootmem.h>
#include <linux/pci.h>
@@ -44,6 +45,9 @@ struct device x86_dma_fallback_dev = {
};
EXPORT_SYMBOL(x86_dma_fallback_dev);

+/* Number of entries preallocated for DMA-API debugging */
+#define PREALLOC_DMA_DEBUG_ENTRIES 8192 /* needs 512kb */
+
int dma_set_mask(struct device *dev, u64 mask)
{
if (!dev->dma_mask || !dma_supported(dev, mask))
@@ -265,6 +269,8 @@ EXPORT_SYMBOL(dma_supported);

static int __init pci_iommu_init(void)
{
+ dma_debug_init(PREALLOC_DMA_DEBUG_ENTRIES);
+
calgary_iommu_init();

intel_iommu_init();
--
1.5.6.4

2009-01-19 14:49:50

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 07/16] dma-debug: add debugfs interface

Impact: add debugfs interface for configuring DMA-API debugging

Signed-off-by: Joerg Roedel <[email protected]>
---
lib/dma-debug.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 78 insertions(+), 0 deletions(-)

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 66ed8b1..1e16bd4 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -19,6 +19,7 @@

#include <linux/dma-debug.h>
#include <linux/spinlock.h>
+#include <linux/debugfs.h>
#include <linux/types.h>
#include <linux/list.h>
#include <linux/slab.h>
@@ -59,12 +60,29 @@ static DEFINE_SPINLOCK(free_entries_lock);
/* Global disable flag - will be set in case of an error */
static bool global_disable __read_mostly;

+/* Global error count */
+static u32 error_count;
+
+/* Global error show enable*/
+static u32 show_all_errors __read_mostly;
+/* Number of errors to show */
+static u32 show_num_errors = 1;
+
static u32 num_free_entries;
static u32 min_free_entries;

/* number of preallocated entries requested by kernel cmdline */
static u32 req_entries;

+/* debugfs dentry's for the stuff above */
+static struct dentry *dma_debug_dent __read_mostly;
+static struct dentry *global_disable_dent __read_mostly;
+static struct dentry *error_count_dent __read_mostly;
+static struct dentry *show_all_errors_dent __read_mostly;
+static struct dentry *show_num_errors_dent __read_mostly;
+static struct dentry *num_free_entries_dent __read_mostly;
+static struct dentry *min_free_entries_dent __read_mostly;
+
/*
* Hash related functions
*
@@ -239,6 +257,58 @@ out_err:
return -ENOMEM;
}

+static int dma_debug_fs_init(void)
+{
+ dma_debug_dent = debugfs_create_dir("dma-api", NULL);
+ if (!dma_debug_dent) {
+ printk(KERN_ERR "DMA-API: can not create debugfs directory\n");
+ return -ENOMEM;
+ }
+
+ global_disable_dent = debugfs_create_bool("disabled", 0444,
+ dma_debug_dent,
+ (u32 *)&global_disable);
+ if (!global_disable_dent)
+ goto out_err;
+
+ error_count_dent = debugfs_create_u32("error_count", 0444,
+ dma_debug_dent, &error_count);
+ if (!error_count_dent)
+ goto out_err;
+
+ show_all_errors_dent = debugfs_create_u32("all_errors", 0644,
+ dma_debug_dent,
+ &show_all_errors);
+ if (!show_all_errors_dent)
+ goto out_err;
+
+ show_num_errors_dent = debugfs_create_u32("num_errors", 0644,
+ dma_debug_dent,
+ &show_num_errors);
+ if (!show_num_errors_dent)
+ goto out_err;
+
+ num_free_entries_dent = debugfs_create_u32("num_free_entries", 0444,
+ dma_debug_dent,
+ &num_free_entries);
+ if (!num_free_entries_dent)
+ goto out_err;
+
+ min_free_entries_dent = debugfs_create_u32("min_free_entries", 0444,
+ dma_debug_dent,
+ &min_free_entries);
+ if (!min_free_entries_dent)
+ goto out_err;
+
+ return 0;
+
+out_err:
+ debugfs_remove_recursive(dma_debug_dent);
+
+ return -ENOMEM;
+}
+
+
/*
* Let the architectures decide how many entries should be preallocated.
*/
@@ -254,6 +324,14 @@ void dma_debug_init(u32 num_entries)
dma_entry_hash[i].lock = SPIN_LOCK_UNLOCKED;
}

+ if (dma_debug_fs_init() != 0) {
+ printk(KERN_ERR "DMA-API: error creating debugfs entries "
+ "- disabling\n");
+ global_disable = true;
+
+ return;
+ }
+
if (req_entries)
num_entries = req_entries;

--
1.5.6.4

2009-01-19 14:50:13

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 12/16] dma-debug: add checks for sync_single_*

Impact: add debug callbacks for dma_sync_single_for_* functions

Signed-off-by: Joerg Roedel <[email protected]>
---
include/linux/dma-debug.h | 20 ++++++++++++++++++++
lib/dma-debug.c | 21 +++++++++++++++++++++
2 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index 15fa361..fea15f3 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -48,6 +48,14 @@ extern void debug_dma_alloc_coherent(struct device *dev, size_t size,
extern void debug_dma_free_coherent(struct device *dev, size_t size,
void *virt, dma_addr_t addr);

+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 dma_debug_init(u32 num_entries)
@@ -88,6 +96,18 @@ static inline void debug_dma_free_coherent(struct device *dev, size_t size,
{
}

+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 /* __DMA_DEBUG_H */
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 01a79d1..9715f40 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -674,3 +674,24 @@ void debug_dma_free_coherent(struct device *dev, size_t size,
}
EXPORT_SYMBOL(debug_dma_free_coherent);

+void debug_dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle,
+ size_t size, int direction)
+{
+ if (unlikely(global_disable))
+ return;
+
+ check_sync(dev, dma_handle, size, 0, direction, true);
+}
+EXPORT_SYMBOL(debug_dma_sync_single_for_cpu);
+
+void debug_dma_sync_single_for_device(struct device *dev,
+ dma_addr_t dma_handle, size_t size,
+ int direction)
+{
+ if (unlikely(global_disable))
+ return;
+
+ check_sync(dev, dma_handle, size, 0, direction, false);
+}
+EXPORT_SYMBOL(debug_dma_sync_single_for_device);
+
--
1.5.6.4

2009-01-19 14:50:44

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 02/16] dma-debug: add header file and core data structures

Impact: add groundwork for DMA-API debugging

Signed-off-by: Joerg Roedel <[email protected]>
---
include/linux/dma-debug.h | 25 +++++++++++++++++++++++++
lib/Makefile | 2 ++
lib/dma-debug.c | 40 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 0 deletions(-)
create mode 100644 include/linux/dma-debug.h
create mode 100644 lib/dma-debug.c

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
new file mode 100644
index 0000000..ce4ace7
--- /dev/null
+++ b/include/linux/dma-debug.h
@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) 2008 Advanced Micro Devices, Inc.
+ *
+ * Author: Joerg Roedel <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef __DMA_DEBUG_H
+#define __DMA_DEBUG_H
+
+struct device;
+
+#endif /* __DMA_DEBUG_H */
diff --git a/lib/Makefile b/lib/Makefile
index 32b0e64..50b48cf 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -84,6 +84,8 @@ obj-$(CONFIG_HAVE_ARCH_TRACEHOOK) += syscall.o

obj-$(CONFIG_DYNAMIC_PRINTK_DEBUG) += dynamic_printk.o

+obj-$(CONFIG_DMA_API_DEBUG) += dma-debug.o
+
hostprogs-y := gen_crc32table
clean-files := crc32table.h

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
new file mode 100644
index 0000000..c944b8c
--- /dev/null
+++ b/lib/dma-debug.c
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2008 Advanced Micro Devices, Inc.
+ *
+ * Author: Joerg Roedel <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/dma-debug.h>
+#include <linux/types.h>
+#include <linux/list.h>
+
+enum {
+ dma_debug_single,
+ dma_debug_page,
+ dma_debug_sg,
+ dma_debug_coherent,
+};
+
+struct dma_debug_entry {
+ struct list_head list;
+ struct device *dev;
+ int type;
+ phys_addr_t paddr;
+ u64 dev_addr;
+ u64 size;
+ int direction;
+};
+
--
1.5.6.4

2009-01-19 14:51:10

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 13/16] dma-debug: add checks for sync_single_range_*

Impact: add debug callbacks for dma_sync_single_range_for_* functions

Signed-off-by: Joerg Roedel <[email protected]>
---
include/linux/dma-debug.h | 27 +++++++++++++++++++++++++++
lib/dma-debug.c | 24 ++++++++++++++++++++++++
2 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index fea15f3..4daaa82 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -56,6 +56,17 @@ extern void debug_dma_sync_single_for_device(struct device *dev,
dma_addr_t dma_handle,
size_t size, int direction);

+extern void debug_dma_sync_single_range_for_cpu(struct device *dev,
+ dma_addr_t dma_handle,
+ unsigned long offset,
+ size_t size,
+ int direction);
+
+extern void debug_dma_sync_single_range_for_device(struct device *dev,
+ dma_addr_t dma_handle,
+ unsigned long offset,
+ size_t size, int direction);
+
#else /* CONFIG_DMA_API_DEBUG */

static inline void dma_debug_init(u32 num_entries)
@@ -108,6 +119,22 @@ static inline void debug_dma_sync_single_for_device(struct device *dev,
{
}

+static inline void debug_dma_sync_single_range_for_cpu(struct device *dev,
+ dma_addr_t dma_handle,
+ unsigned long offset,
+ size_t size,
+ int direction)
+{
+}
+
+static inline void debug_dma_sync_single_range_for_device(struct device *dev,
+ dma_addr_t dma_handle,
+ unsigned long offset,
+ size_t size,
+ int direction)
+{
+}
+
#endif /* CONFIG_DMA_API_DEBUG */

#endif /* __DMA_DEBUG_H */
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 9715f40..125b6a2 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -695,3 +695,27 @@ void debug_dma_sync_single_for_device(struct device *dev,
}
EXPORT_SYMBOL(debug_dma_sync_single_for_device);

+void debug_dma_sync_single_range_for_cpu(struct device *dev,
+ dma_addr_t dma_handle,
+ unsigned long offset, size_t size,
+ int direction)
+{
+ if (unlikely(global_disable))
+ return;
+
+ check_sync(dev, dma_handle, size, offset, direction, true);
+}
+EXPORT_SYMBOL(debug_dma_sync_single_range_for_cpu);
+
+void debug_dma_sync_single_range_for_device(struct device *dev,
+ dma_addr_t dma_handle,
+ unsigned long offset,
+ size_t size, int direction)
+{
+ if (unlikely(global_disable))
+ return;
+
+ check_sync(dev, dma_handle, size, offset, direction, false);
+}
+EXPORT_SYMBOL(debug_dma_sync_single_range_for_device);
+
--
1.5.6.4

2009-01-19 14:51:42

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 08/16] dma-debug: add core checking functions

Impact: add functions to check on dma unmap and sync

Signed-off-by: Joerg Roedel <[email protected]>
---
lib/dma-debug.c | 147 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 147 insertions(+), 0 deletions(-)

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 1e16bd4..4d98811 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -17,9 +17,11 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/

+#include <linux/dma-mapping.h>
#include <linux/dma-debug.h>
#include <linux/spinlock.h>
#include <linux/debugfs.h>
+#include <linux/device.h>
#include <linux/types.h>
#include <linux/list.h>
#include <linux/slab.h>
@@ -83,6 +85,36 @@ static struct dentry *show_num_errors_dent __read_mostly;
static struct dentry *num_free_entries_dent __read_mostly;
static struct dentry *min_free_entries_dent __read_mostly;

+static const char *type2name[4] = { "single", "page",
+ "scather-gather", "coherent" };
+
+static const char *dir2name[4] = { "DMA_BIDIRECTIONAL", "DMA_TO_DEVICE",
+ "DMA_FROM_DEVICE", "DMA_NONE" };
+
+/*
+ * The access to some variables in this macro is racy. We can't use atomic_t
+ * here because all these variables are exported to debugfs. Some of them even
+ * writeable. This is also the reason why a lock won't help much. But anyway,
+ * the races are no big deal. Here is why:
+ *
+ * error_count: the addition is racy, but the worst thing that can happen is
+ * that we don't count some errors
+ * show_num_errors: the subtraction is racy. Also no big deal because in
+ * worst case this will result in one warning more in the
+ * system log than the user configured. This variable is
+ * writeable via debugfs.
+ */
+#define err_printk(dev, format, arg...) do { \
+ error_count += 1; \
+ if (show_all_errors || show_num_errors > 0) { \
+ WARN(1, "%s %s: " format, \
+ dev_driver_string(dev), \
+ dev_name(dev) , ## arg); \
+ } \
+ if (!show_all_errors && show_num_errors > 0) \
+ show_num_errors -= 1; \
+ } while (0);
+
/*
* Hash related functions
*
@@ -378,3 +410,118 @@ static __init int dma_debug_entries_cmdline(char *str)
__setup("dma_debug=", dma_debug_cmdline);
__setup("dma_debug_entries=", dma_debug_entries_cmdline);

+static void check_unmap(struct dma_debug_entry *ref)
+{
+ struct dma_debug_entry *entry;
+ struct hash_bucket *bucket;
+ unsigned long flags;
+
+ if (dma_mapping_error(ref->dev, ref->dev_addr))
+ return;
+
+ bucket = get_hash_bucket(ref, &flags);
+ entry = hash_bucket_find(bucket, ref);
+
+ if (!entry) {
+ err_printk(ref->dev, "DMA-API: device driver tries "
+ "to free DMA memory it has not allocated "
+ "[device address=0x%016llx] [size=%llu bytes]\n",
+ ref->dev_addr, ref->size);
+ goto out;
+ }
+
+ if (ref->size != entry->size) {
+ err_printk(ref->dev, "DMA-API: device driver frees "
+ "DMA memory with different size "
+ "[device address=0x%016llx] [map size=%llu bytes] "
+ "[unmap size=%llu bytes]\n",
+ ref->dev_addr, entry->size, ref->size);
+ }
+
+ if (ref->type != entry->type) {
+ err_printk(ref->dev, "DMA-API: device driver frees "
+ "DMA memory with wrong function "
+ "[device address=0x%016llx] [size=%llu bytes] "
+ "[mapped as %s] [unmapped as %s]\n",
+ ref->dev_addr, ref->size,
+ type2name[entry->type], type2name[ref->type]);
+ } else if ((entry->type == dma_debug_coherent) &&
+ (ref->paddr != entry->paddr)) {
+ err_printk(ref->dev, "DMA-API: device driver frees "
+ "DMA memory with different CPU address "
+ "[device address=0x%016llx] [size=%llu bytes] "
+ "[cpu alloc address=%p] [cpu free address=%p]",
+ ref->dev_addr, ref->size,
+ entry->paddr, ref->paddr);
+ }
+
+ /*
+ * This may be no bug in reality - but most implementations of the
+ * DMA API don't handle this properly, so check for it here
+ */
+ if (ref->direction != entry->direction) {
+ err_printk(ref->dev, "DMA-API: device driver frees "
+ "DMA memory with different direction "
+ "[device address=0x%016llx] [size=%llu bytes] "
+ "[mapped with %s] [unmapped with %s]\n",
+ ref->dev_addr, ref->size,
+ dir2name[entry->direction],
+ dir2name[ref->direction]);
+ }
+
+ hash_bucket_del(entry);
+ dma_entry_free(entry);
+
+out:
+ put_hash_bucket(bucket, &flags);
+}
+
+static void check_sync(struct device *dev, dma_addr_t addr,
+ u64 size, u64 offset, int direction, bool to_cpu)
+{
+ struct dma_debug_entry ref = {
+ .dev = dev,
+ .dev_addr = addr,
+ .size = size,
+ .direction = direction,
+ };
+ struct dma_debug_entry *entry;
+ struct hash_bucket *bucket;
+ unsigned long flags;
+
+ bucket = get_hash_bucket(&ref, &flags);
+
+ entry = hash_bucket_find(bucket, &ref);
+
+ if (!entry) {
+ err_printk(dev, "DMA-API: device driver tries "
+ "to sync DMA memory it has not allocated "
+ "[device address=0x%016llx] [size=%llu bytes]\n",
+ addr, size);
+ goto out;
+ }
+
+ if ((offset + size) > entry->size) {
+ err_printk(dev, "DMA-API: device driver syncs"
+ " DMA memory outside allocated range "
+ "[device address=0x%016llx] "
+ "[allocation size=%llu bytes] [sync offset=%llu] "
+ "[sync size=%llu]\n", entry->dev_addr, entry->size,
+ offset, size);
+ }
+
+ if (direction != entry->direction) {
+ err_printk(dev, "DMA-API: device driver syncs "
+ "DMA memory with different direction "
+ "[device address=0x%016llx] [size=%llu bytes] "
+ "[mapped with %s] [synced with %s]\n",
+ addr, entry->size,
+ dir2name[entry->direction],
+ dir2name[direction]);
+ }
+
+out:
+ put_hash_bucket(bucket, &flags);
+
+}
+
--
1.5.6.4

2009-01-19 14:52:07

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 10/16] dma-debug: add add checking for map/unmap_sg

Impact: add debug callbacks for dma_{un}map_sg

Signed-off-by: Joerg Roedel <[email protected]>
---
include/linux/dma-debug.h | 15 ++++++++++++
lib/dma-debug.c | 53 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index ea6a283..61f559d 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -36,6 +36,11 @@ extern void debug_dma_map_page(struct device *dev, struct page *page,
extern void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
size_t size, int direction, bool map_single);

+extern void debug_dma_map_sg(struct device *dev, struct scatterlist *sg,
+ int nents, int direction);
+
+extern void debug_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
+ int nelems, int dir);

#else /* CONFIG_DMA_API_DEBUG */

@@ -56,6 +61,16 @@ static inline void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
{
}

+static inline void debug_dma_map_sg(struct device *dev, struct scatterlist *sg,
+ int nents, int direction)
+{
+}
+
+static inline void debug_dma_unmap_sg(struct device *dev,
+ struct scatterlist *sglist,
+ int nelems, int dir)
+{
+}

#endif /* CONFIG_DMA_API_DEBUG */

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 9219c41..f3e0145 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -17,6 +17,7 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/

+#include <linux/scatterlist.h>
#include <linux/dma-mapping.h>
#include <linux/dma-debug.h>
#include <linux/spinlock.h>
@@ -576,3 +577,55 @@ void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
}
EXPORT_SYMBOL(debug_dma_unmap_page);

+void debug_dma_map_sg(struct device *dev, struct scatterlist *sg,
+ int nents, int direction)
+{
+ struct dma_debug_entry *entry;
+ struct scatterlist *s;
+ int i;
+
+ if (unlikely(global_disable))
+ return;
+
+ for_each_sg(sg, s, nents, i) {
+ entry = dma_entry_alloc();
+ if (!entry)
+ return;
+
+ entry->type = dma_debug_sg;
+ entry->dev = dev;
+ entry->paddr = sg_phys(s);
+ entry->size = s->length;
+ entry->dev_addr = s->dma_address;
+ entry->direction = direction;
+
+ add_dma_entry(entry);
+ }
+}
+EXPORT_SYMBOL(debug_dma_map_sg);
+
+void debug_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
+ int nelems, int dir)
+{
+ struct scatterlist *s;
+ int i;
+
+ if (unlikely(global_disable))
+ return;
+
+ for_each_sg(sglist, s, nelems, i) {
+
+ struct dma_debug_entry ref = {
+ .type = dma_debug_sg,
+ .dev = dev,
+ .paddr = sg_phys(s),
+ .dev_addr = s->dma_address,
+ .size = s->length,
+ .direction = dir,
+ };
+
+ check_unmap(&ref);
+ }
+}
+EXPORT_SYMBOL(debug_dma_unmap_sg);
+
--
1.5.6.4

2009-01-19 14:52:53

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 03/16] dma-debug: add hash functions for dma_debug_entries

Impact: implement necessary functions for the core hash

Signed-off-by: Joerg Roedel <[email protected]>
---
lib/dma-debug.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 101 insertions(+), 0 deletions(-)

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index c944b8c..a90f9c4 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -18,9 +18,14 @@
*/

#include <linux/dma-debug.h>
+#include <linux/spinlock.h>
#include <linux/types.h>
#include <linux/list.h>

+#define HASH_SIZE 256
+#define HASH_FN_SHIFT 13
+#define HASH_FN_MASK 0xffULL
+
enum {
dma_debug_single,
dma_debug_page,
@@ -38,3 +43,99 @@ struct dma_debug_entry {
int direction;
};

+struct hash_bucket {
+ struct list_head list;
+ spinlock_t lock;
+} __cacheline_aligned_in_smp;
+
+/* Hash list to save the allocated dma addresses */
+static struct hash_bucket dma_entry_hash[HASH_SIZE];
+
+/*
+ * Hash related functions
+ *
+ * Every DMA-API request is saved into a struct dma_debug_entry. To
+ * have quick access to these structs they are stored into a hash.
+ */
+static int hash_fn(struct dma_debug_entry *entry)
+{
+ /*
+ * Hash function is based on the dma address.
+ * We use bits 20-27 here as the index into the hash
+ */
+ return (entry->dev_addr >> HASH_FN_SHIFT) & HASH_FN_MASK;
+}
+
+/*
+ * Request exclusive access to a hash bucket for a given dma_debug_entry.
+ */
+static struct hash_bucket *get_hash_bucket(struct dma_debug_entry *entry,
+ unsigned long *flags)
+{
+ int idx = hash_fn(entry);
+ unsigned long __flags;
+
+ spin_lock_irqsave(&dma_entry_hash[idx].lock, __flags);
+ *flags = __flags;
+ return &dma_entry_hash[idx];
+}
+
+/*
+ * Give up exclusive access to the hash bucket
+ */
+static void put_hash_bucket(struct hash_bucket *bucket,
+ unsigned long *flags)
+{
+ unsigned long __flags = *flags;
+
+ spin_unlock_irqrestore(&bucket->lock, __flags);
+}
+
+/*
+ * Search a given entry in the hash bucket list
+ */
+static struct dma_debug_entry *hash_bucket_find(struct hash_bucket *bucket,
+ struct dma_debug_entry *ref)
+{
+ struct dma_debug_entry *entry;
+
+ list_for_each_entry(entry, &bucket->list, list) {
+ if ((entry->dev_addr == ref->dev_addr) &&
+ (entry->dev == ref->dev))
+ return entry;
+ }
+
+ return NULL;
+}
+
+/*
+ * Add an entry to a hash bucket
+ */
+static void hash_bucket_add(struct hash_bucket *bucket,
+ struct dma_debug_entry *entry)
+{
+ list_add_tail(&entry->list, &bucket->list);
+}
+
+/*
+ * Remove entry from a hash bucket list
+ */
+static void hash_bucket_del(struct dma_debug_entry *entry)
+{
+ list_del(&entry->list);
+}
+
+/*
+ * Wrapper function for adding an entry to the hash.
+ * This function takes care of locking itself.
+ */
+static void add_dma_entry(struct dma_debug_entry *entry)
+{
+ struct hash_bucket *bucket;
+ unsigned long flags;
+
+ bucket = get_hash_bucket(entry, &flags);
+ hash_bucket_add(bucket, entry);
+ put_hash_bucket(bucket, &flags);
+}
+
--
1.5.6.4

2009-01-19 14:53:17

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 14/16] dma-debug: add checks for sync_single_sg_*

Impact: add debug callbacks for dma_sync_sg_* functions

Signed-off-by: Joerg Roedel <[email protected]>
---
include/linux/dma-debug.h | 20 ++++++++++++++++++++
lib/dma-debug.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index 4daaa82..8a8aae4 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -67,6 +67,14 @@ extern void debug_dma_sync_single_range_for_device(struct device *dev,
unsigned long offset,
size_t size, int direction);

+extern void debug_dma_sync_sg_for_cpu(struct device *dev,
+ struct scatterlist *sg,
+ int nelems, int direction);
+
+extern void debug_dma_sync_sg_for_device(struct device *dev,
+ struct scatterlist *sg,
+ int nelems, int direction);
+
#else /* CONFIG_DMA_API_DEBUG */

static inline void dma_debug_init(u32 num_entries)
@@ -135,6 +143,18 @@ static inline void debug_dma_sync_single_range_for_device(struct device *dev,
{
}

+static inline void debug_dma_sync_sg_for_cpu(struct device *dev,
+ struct scatterlist *sg,
+ int nelems, int direction)
+{
+}
+
+static inline void debug_dma_sync_sg_for_device(struct device *dev,
+ struct scatterlist *sg,
+ int nelems, int direction)
+{
+}
+
#endif /* CONFIG_DMA_API_DEBUG */

#endif /* __DMA_DEBUG_H */
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 125b6a2..469e5b9 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -719,3 +719,35 @@ void debug_dma_sync_single_range_for_device(struct device *dev,
}
EXPORT_SYMBOL(debug_dma_sync_single_range_for_device);

+void debug_dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
+ int nelems, int direction)
+{
+ struct scatterlist *s;
+ int i;
+
+ if (unlikely(global_disable))
+ return;
+
+ for_each_sg(sg, s, nelems, i) {
+ check_sync(dev, s->dma_address, s->dma_length, 0,
+ direction, true);
+ }
+}
+EXPORT_SYMBOL(debug_dma_sync_sg_for_cpu);
+
+void debug_dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
+ int nelems, int direction)
+{
+ struct scatterlist *s;
+ int i;
+
+ if (unlikely(global_disable))
+ return;
+
+ for_each_sg(sg, s, nelems, i) {
+ check_sync(dev, s->dma_address, s->dma_length, 0,
+ direction, false);
+ }
+}
+EXPORT_SYMBOL(debug_dma_sync_sg_for_device);
+
--
1.5.6.4

2009-01-19 14:53:41

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 04/16] dma-debug: add allocator code

Impact: add allocator code for struct dma_debug_entry

Signed-off-by: Joerg Roedel <[email protected]>
---
lib/dma-debug.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index a90f9c4..c9e44e8 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -50,6 +50,16 @@ struct hash_bucket {

/* Hash list to save the allocated dma addresses */
static struct hash_bucket dma_entry_hash[HASH_SIZE];
+/* List of pre-allocated dma_debug_entry's */
+static LIST_HEAD(free_entries);
+/* Lock for the list above */
+static DEFINE_SPINLOCK(free_entries_lock);
+
+/* Global disable flag - will be set in case of an error */
+static bool global_disable __read_mostly;
+
+static u32 num_free_entries;
+static u32 min_free_entries;

/*
* Hash related functions
@@ -139,3 +149,50 @@ static void add_dma_entry(struct dma_debug_entry *entry)
put_hash_bucket(bucket, &flags);
}

+/* struct dma_entry allocator
+ *
+ * The next two functions implement the allocator for
+ * struct dma_debug_entries.
+ */
+static struct dma_debug_entry *dma_entry_alloc(void)
+{
+ struct dma_debug_entry *entry = NULL;
+ unsigned long flags;
+
+ spin_lock_irqsave(&free_entries_lock, flags);
+
+ if (list_empty(&free_entries)) {
+ printk(KERN_ERR "DMA-API: debugging out of memory "
+ "- disabling\n");
+ global_disable = true;
+ goto out;
+ }
+
+ entry = list_entry(free_entries.next, struct dma_debug_entry, list);
+ list_del(&entry->list);
+ memset(entry, 0, sizeof(*entry));
+
+ num_free_entries -= 1;
+ if (num_free_entries < min_free_entries)
+ min_free_entries = num_free_entries;
+
+out:
+ spin_unlock_irqrestore(&free_entries_lock, flags);
+
+ return entry;
+}
+
+static void dma_entry_free(struct dma_debug_entry *entry)
+{
+ unsigned long flags;
+
+ /*
+ * add to beginning of the list - this way the entries are
+ * more likely cache hot when they are reallocated.
+ */
+ spin_lock_irqsave(&free_entries_lock, flags);
+ list_add(&entry->list, &free_entries);
+ num_free_entries += 1;
+ spin_unlock_irqrestore(&free_entries_lock, flags);
+}
+
--
1.5.6.4

2009-01-19 14:54:06

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 11/16] dma-debug: add checking for [alloc|free]_coherent

Impact: add debug callbacks for dma_[alloc|free]_coherent

Signed-off-by: Joerg Roedel <[email protected]>
---
include/linux/dma-debug.h | 16 ++++++++++++++++
lib/dma-debug.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index 61f559d..15fa361 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -42,6 +42,12 @@ extern void debug_dma_map_sg(struct device *dev, struct scatterlist *sg,
extern void debug_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
int nelems, int dir);

+extern void debug_dma_alloc_coherent(struct device *dev, size_t size,
+ dma_addr_t dma_addr, void *virt);
+
+extern void debug_dma_free_coherent(struct device *dev, size_t size,
+ void *virt, dma_addr_t addr);
+
#else /* CONFIG_DMA_API_DEBUG */

static inline void dma_debug_init(u32 num_entries)
@@ -72,6 +78,16 @@ static inline void debug_dma_unmap_sg(struct device *dev,
{
}

+static inline void debug_dma_alloc_coherent(struct device *dev, size_t size,
+ dma_addr_t dma_addr, void *virt)
+{
+}
+
+static inline void debug_dma_free_coherent(struct device *dev, size_t size,
+ void *virt, dma_addr_t addr)
+{
+}
+
#endif /* CONFIG_DMA_API_DEBUG */

#endif /* __DMA_DEBUG_H */
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index f3e0145..01a79d1 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -629,3 +629,48 @@ void debug_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
}
EXPORT_SYMBOL(debug_dma_unmap_sg);

+void debug_dma_alloc_coherent(struct device *dev, size_t size,
+ dma_addr_t dma_addr, void *virt)
+{
+ struct dma_debug_entry *entry;
+
+ if (unlikely(global_disable))
+ return;
+
+ if (unlikely(virt == NULL))
+ return;
+
+ entry = dma_entry_alloc();
+ if (!entry)
+ return;
+
+ entry->type = dma_debug_coherent;
+ entry->dev = dev;
+ entry->paddr = virt_to_phys(virt);
+ entry->size = size;
+ entry->dev_addr = dma_addr;
+ entry->direction = DMA_BIDIRECTIONAL;
+
+ add_dma_entry(entry);
+}
+EXPORT_SYMBOL(debug_dma_alloc_coherent);
+
+void debug_dma_free_coherent(struct device *dev, size_t size,
+ void *virt, dma_addr_t addr)
+{
+ struct dma_debug_entry ref = {
+ .type = dma_debug_coherent,
+ .dev = dev,
+ .paddr = virt_to_phys(virt),
+ .dev_addr = addr,
+ .size = size,
+ .direction = DMA_BIDIRECTIONAL,
+ };
+
+ if (unlikely(global_disable))
+ return;
+
+ check_unmap(&ref);
+}
+EXPORT_SYMBOL(debug_dma_free_coherent);
+
--
1.5.6.4

2009-01-21 00:18:24

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 09/16] dma-debug: add checking for map/unmap_page/single

On Monday 19 January 2009, Joerg Roedel wrote:
> Impact: add debug callbacks for dma_{un}map_[page|single]

In the discussion about "[PATCH] usb/mcs7830: Don't use buffers from
stack for USB transfers", I have raised the question of whether we can
add a check for the DMA address being on the stack, I guess this would
be the right place for it.

Maybe you can add another check like:

WARN_ON(is_kernel_stack(page_to_virt(page)));

Unfortunately, I can't think of a way to define a generic is_kernel_stack
function. On most architectures (all that put thread_info at the top of the
stack), something like this should work:

#ifndef is_kernel_stack
static inline int is_kernel_stack(void *p)
{
return (unsigned long)p & ~(THREAD_SIZE - 1) == current_thread_info();
}
#endif

Arnd <><

2009-01-23 10:28:44

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 09/16] dma-debug: add checking for map/unmap_page/single

On Wed, Jan 21, 2009 at 01:17:32AM +0100, Arnd Bergmann wrote:
> On Monday 19 January 2009, Joerg Roedel wrote:
> > Impact: add debug callbacks for dma_{un}map_[page|single]
>
> In the discussion about "[PATCH] usb/mcs7830: Don't use buffers from
> stack for USB transfers", I have raised the question of whether we can
> add a check for the DMA address being on the stack, I guess this would
> be the right place for it.
>
> Maybe you can add another check like:
>
> WARN_ON(is_kernel_stack(page_to_virt(page)));

Good idea. This would be the right place for such a check.

> Unfortunately, I can't think of a way to define a generic is_kernel_stack
> function. On most architectures (all that put thread_info at the top of the
> stack), something like this should work:
>
> #ifndef is_kernel_stack
> static inline int is_kernel_stack(void *p)
> {
> return (unsigned long)p & ~(THREAD_SIZE - 1) == current_thread_info();
> }
> #endif

Hmm, this function is indeed required on all architectures. If you send
a patchset which adds it I will add the check for stack addresses to
this debugging code :)

Joerg

2009-01-25 13:18:39

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH 09/16] dma-debug: add checking for map/unmap_page/single

On Fri, 23 Jan 2009 11:28:27 +0100
Joerg Roedel <[email protected]> wrote:

> On Wed, Jan 21, 2009 at 01:17:32AM +0100, Arnd Bergmann wrote:
> > On Monday 19 January 2009, Joerg Roedel wrote:
> > > Impact: add debug callbacks for dma_{un}map_[page|single]
> >
> > In the discussion about "[PATCH] usb/mcs7830: Don't use buffers from
> > stack for USB transfers", I have raised the question of whether we can
> > add a check for the DMA address being on the stack, I guess this would
> > be the right place for it.
> >
> > Maybe you can add another check like:
> >
> > WARN_ON(is_kernel_stack(page_to_virt(page)));
>
> Good idea. This would be the right place for such a check.
>
> > Unfortunately, I can't think of a way to define a generic is_kernel_stack
> > function. On most architectures (all that put thread_info at the top of the
> > stack), something like this should work:
> >
> > #ifndef is_kernel_stack
> > static inline int is_kernel_stack(void *p)
> > {
> > return (unsigned long)p & ~(THREAD_SIZE - 1) == current_thread_info();
> > }
> > #endif
>
> Hmm, this function is indeed required on all architectures. If you send
> a patchset which adds it I will add the check for stack addresses to
> this debugging code :)

object_is_on_stack()

2009-01-25 13:51:26

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 09/16] dma-debug: add checking for map/unmap_page/single

On Sun, Jan 25, 2009 at 10:17:19PM +0900, FUJITA Tomonori wrote:
> On Fri, 23 Jan 2009 11:28:27 +0100
> Joerg Roedel <[email protected]> wrote:
>
> > On Wed, Jan 21, 2009 at 01:17:32AM +0100, Arnd Bergmann wrote:
> > > On Monday 19 January 2009, Joerg Roedel wrote:
> > > > Impact: add debug callbacks for dma_{un}map_[page|single]
> > >
> > > In the discussion about "[PATCH] usb/mcs7830: Don't use buffers from
> > > stack for USB transfers", I have raised the question of whether we can
> > > add a check for the DMA address being on the stack, I guess this would
> > > be the right place for it.
> > >
> > > Maybe you can add another check like:
> > >
> > > WARN_ON(is_kernel_stack(page_to_virt(page)));
> >
> > Good idea. This would be the right place for such a check.
> >
> > > Unfortunately, I can't think of a way to define a generic is_kernel_stack
> > > function. On most architectures (all that put thread_info at the top of the
> > > stack), something like this should work:
> > >
> > > #ifndef is_kernel_stack
> > > static inline int is_kernel_stack(void *p)
> > > {
> > > return (unsigned long)p & ~(THREAD_SIZE - 1) == current_thread_info();
> > > }
> > > #endif
> >
> > Hmm, this function is indeed required on all architectures. If you send
> > a patchset which adds it I will add the check for stack addresses to
> > this debugging code :)
>
> object_is_on_stack()

Oh cool. This function exists already. Thanks for the hint :)

Joerg