From 15b28bbcd567a9199481ecfef39702b258f9baff Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 16 Apr 2018 17:22:28 +0200 Subject: dma-debug: move initialization to common code Most mainstream architectures are using 65536 entries, so lets stick to that. If someone is really desperate to override it that can still be done through , but I'd rather see a really good rationale for that. dma_debug_init is now called as a core_initcall, which for many architectures means much earlier, and provides dma-debug functionality earlier in the boot process. This should be safe as it only relies on the memory allocator already being available. Signed-off-by: Christoph Hellwig Acked-by: Marek Szyprowski Reviewed-by: Robin Murphy --- lib/dma-debug.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'lib/dma-debug.c') diff --git a/lib/dma-debug.c b/lib/dma-debug.c index 7f5cdc1e6b29..712a897174e4 100644 --- a/lib/dma-debug.c +++ b/lib/dma-debug.c @@ -41,6 +41,11 @@ #define HASH_FN_SHIFT 13 #define HASH_FN_MASK (HASH_SIZE - 1) +/* allow architectures to override this if absolutely required */ +#ifndef PREALLOC_DMA_DEBUG_ENTRIES +#define PREALLOC_DMA_DEBUG_ENTRIES (1 << 16) +#endif + enum { dma_debug_single, dma_debug_page, @@ -1004,18 +1009,16 @@ void dma_debug_add_bus(struct bus_type *bus) bus_register_notifier(bus, nb); } -/* - * Let the architectures decide how many entries should be preallocated. - */ -void dma_debug_init(u32 num_entries) +static int dma_debug_init(void) { + u32 num_entries; int i; /* Do not use dma_debug_initialized here, since we really want to be * called to set dma_debug_initialized */ if (global_disable) - return; + return 0; for (i = 0; i < HASH_SIZE; ++i) { INIT_LIST_HEAD(&dma_entry_hash[i].list); @@ -1026,17 +1029,19 @@ void dma_debug_init(u32 num_entries) pr_err("DMA-API: error creating debugfs entries - disabling\n"); global_disable = true; - return; + return 0; } if (req_entries) num_entries = req_entries; + else + num_entries = PREALLOC_DMA_DEBUG_ENTRIES; if (prealloc_memory(num_entries) != 0) { pr_err("DMA-API: debugging out of memory error - disabled\n"); global_disable = true; - return; + return 0; } nr_total_entries = num_free_entries; @@ -1044,7 +1049,9 @@ void dma_debug_init(u32 num_entries) dma_debug_initialized = true; pr_info("DMA-API: debugging enabled by kernel config\n"); + return 0; } +core_initcall(dma_debug_init); static __init int dma_debug_cmdline(char *str) { -- cgit From bcebe324cb44f0298f6bb5fcb5eb92a3ec55feeb Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 24 Apr 2018 09:40:51 +0200 Subject: dma-debug: simplify counting of preallocated requests Just keep a single variable with a descriptive name instead of two with confusing names. Signed-off-by: Christoph Hellwig Reviewed-by: Robin Murphy --- lib/dma-debug.c | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) (limited to 'lib/dma-debug.c') diff --git a/lib/dma-debug.c b/lib/dma-debug.c index 712a897174e4..075253cb613b 100644 --- a/lib/dma-debug.c +++ b/lib/dma-debug.c @@ -132,7 +132,7 @@ static u32 min_free_entries; static u32 nr_total_entries; /* number of preallocated entries requested by kernel cmdline */ -static u32 req_entries; +static u32 nr_prealloc_entries = PREALLOC_DMA_DEBUG_ENTRIES; /* debugfs dentry's for the stuff above */ static struct dentry *dma_debug_dent __read_mostly; @@ -1011,7 +1011,6 @@ void dma_debug_add_bus(struct bus_type *bus) static int dma_debug_init(void) { - u32 num_entries; int i; /* Do not use dma_debug_initialized here, since we really want to be @@ -1032,12 +1031,7 @@ static int dma_debug_init(void) return 0; } - if (req_entries) - num_entries = req_entries; - else - num_entries = PREALLOC_DMA_DEBUG_ENTRIES; - - if (prealloc_memory(num_entries) != 0) { + if (prealloc_memory(nr_prealloc_entries) != 0) { pr_err("DMA-API: debugging out of memory error - disabled\n"); global_disable = true; @@ -1068,16 +1062,10 @@ static __init int dma_debug_cmdline(char *str) 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; - + if (!get_option(&str, &nr_prealloc_entries)) + nr_prealloc_entries = PREALLOC_DMA_DEBUG_ENTRIES; return 0; } -- cgit From 9f22bbbdd8e31a38872855b4ff402d76efb4c621 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 24 Apr 2018 09:37:18 +0200 Subject: dma-debug: unexport dma_debug_resize_entries and debug_dma_dump_mappings Only used by the AMD GART and Intel VT-D drivers, which must be built in. Signed-off-by: Christoph Hellwig Reviewed-by: Robin Murphy --- lib/dma-debug.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'lib/dma-debug.c') diff --git a/lib/dma-debug.c b/lib/dma-debug.c index 075253cb613b..6a1ebaa83623 100644 --- a/lib/dma-debug.c +++ b/lib/dma-debug.c @@ -444,7 +444,6 @@ void debug_dma_dump_mappings(struct device *dev) spin_unlock_irqrestore(&bucket->lock, flags); } } -EXPORT_SYMBOL(debug_dma_dump_mappings); /* * For each mapping (initial cacheline in the case of @@ -753,7 +752,6 @@ int dma_debug_resize_entries(u32 num_entries) return ret; } -EXPORT_SYMBOL(dma_debug_resize_entries); /* * DMA-API debugging init code -- cgit From 78c47830a5cbb5d22dc91819a95af3d5c4bec084 Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Mon, 21 May 2018 12:35:13 +0100 Subject: dma-debug: check scatterlist segments Drivers/subsystems creating scatterlists for DMA should be taking care to respect the scatter-gather limitations of the appropriate device, as described by dma_parms. A DMA API implementation cannot feasibly split a scatterlist into *more* entries than originally passed, so it is not well defined what they should do when given a segment larger than the limit they are also required to respect. Conversely, devices which are less limited than the rather conservative defaults, or indeed have no limitations at all (e.g. GPUs with their own internal MMU), should be encouraged to set appropriate dma_parms, as they may get more efficient DMA mapping performance out of it. Signed-off-by: Robin Murphy Signed-off-by: Christoph Hellwig --- lib/Kconfig.debug | 17 +++++++++++++++++ lib/dma-debug.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) (limited to 'lib/dma-debug.c') diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index d5175eb7b917..76555479ae36 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1651,6 +1651,23 @@ config DMA_API_DEBUG If unsure, say N. +config DMA_API_DEBUG_SG + bool "Debug DMA scatter-gather usage" + default y + depends on DMA_API_DEBUG + help + Perform extra checking that callers of dma_map_sg() have respected the + appropriate segment length/boundary limits for the given device when + preparing DMA scatterlists. + + This is particularly likely to have been overlooked in cases where the + dma_map_sg() API is used for general bulk mapping of pages rather than + preparing literal scatter-gather descriptors, where there is a risk of + unexpected behaviour from DMA API implementations if the scatterlist + is technically out-of-spec. + + If unsure, say N. + menuconfig RUNTIME_TESTING_MENU bool "Runtime Testing" def_bool y diff --git a/lib/dma-debug.c b/lib/dma-debug.c index 6a1ebaa83623..c007d25bee09 100644 --- a/lib/dma-debug.c +++ b/lib/dma-debug.c @@ -1286,6 +1286,32 @@ out: put_hash_bucket(bucket, &flags); } +static void check_sg_segment(struct device *dev, struct scatterlist *sg) +{ +#ifdef CONFIG_DMA_API_DEBUG_SG + unsigned int max_seg = dma_get_max_seg_size(dev); + u64 start, end, boundary = dma_get_seg_boundary(dev); + + /* + * Either the driver forgot to set dma_parms appropriately, or + * whoever generated the list forgot to check them. + */ + if (sg->length > max_seg) + err_printk(dev, NULL, "DMA-API: mapping sg segment longer than device claims to support [len=%u] [max=%u]\n", + sg->length, max_seg); + /* + * In some cases this could potentially be the DMA API + * implementation's fault, but it would usually imply that + * the scatterlist was built inappropriately to begin with. + */ + start = sg_dma_address(sg); + end = start + sg_dma_len(sg) - 1; + if ((start ^ end) & ~boundary) + err_printk(dev, NULL, "DMA-API: mapping sg segment across boundary [start=0x%016llx] [end=0x%016llx] [boundary=0x%016llx]\n", + start, end, boundary); +#endif +} + 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) @@ -1416,6 +1442,8 @@ void debug_dma_map_sg(struct device *dev, struct scatterlist *sg, check_for_illegal_area(dev, sg_virt(s), sg_dma_len(s)); } + check_sg_segment(dev, s); + add_dma_entry(entry); } } -- cgit