[PATCH] dma-debug: ensure mappings are created and released with matching attributes

[Leon Romanovsky <leon@kernel.org>]

From: Leon Romanovsky <leonro@nvidia.com>

The DMA API expects that callers use the same attributes when mapping
and unmapping. Add tracking to verify this and catch mismatches.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 kernel/dma/debug.c | 62 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 43 insertions(+), 19 deletions(-)

---
Marek,

This patch is based on f5ebf241c407 ("mm/hmm: Indicate that HMM requires DMA coherency"),
just to minimize merge conflicts, but it is definitely for -next. If the
patch is ok and that f5ebf241c407 commit won't be backmerged to -next, I can resend
it in next cycle. This debug aid helped me to see this case:
https://lore.kernel.org/all/20260323-umem-dma-attrs-v1-1-d6890f2e6a1e@nvidia.com/

diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
index 0677918f06a80..6e5e69b8bc4d8 100644
--- a/kernel/dma/debug.c
+++ b/kernel/dma/debug.c
@@ -63,7 +63,7 @@ enum map_err_types {
  * @sg_mapped_ents: 'mapped_ents' from dma_map_sg
  * @paddr: physical start address of the mapping
  * @map_err_type: track whether dma_mapping_error() was checked
- * @is_cache_clean: driver promises not to write to buffer while mapped
+ * @attrs: dma attributes
  * @stack_len: number of backtrace entries in @stack_entries
  * @stack_entries: stack of backtrace history
  */
@@ -78,7 +78,7 @@ struct dma_debug_entry {
 	int		 sg_mapped_ents;
 	phys_addr_t	 paddr;
 	enum map_err_types map_err_type;
-	bool		 is_cache_clean;
+	unsigned long    attrs;
 #ifdef CONFIG_STACKTRACE
 	unsigned int	stack_len;
 	unsigned long	stack_entries[DMA_DEBUG_STACKTRACE_ENTRIES];
@@ -478,6 +478,9 @@ static int active_cacheline_insert(struct dma_debug_entry *entry,
 				   bool *overlap_cache_clean)
 {
 	phys_addr_t cln = to_cacheline_number(entry);
+	bool is_cache_clean = entry->attrs &
+			      (DMA_ATTR_DEBUGGING_IGNORE_CACHELINES |
+			       DMA_ATTR_REQUIRE_COHERENT);
 	unsigned long flags;
 	int rc;
 
@@ -495,12 +498,15 @@ static int active_cacheline_insert(struct dma_debug_entry *entry,
 	if (rc == -EEXIST) {
 		struct dma_debug_entry *existing;
 
-		active_cacheline_inc_overlap(cln, entry->is_cache_clean);
+		active_cacheline_inc_overlap(cln, is_cache_clean);
 		existing = radix_tree_lookup(&dma_active_cacheline, cln);
 		/* A lookup failure here after we got -EEXIST is unexpected. */
 		WARN_ON(!existing);
 		if (existing)
-			*overlap_cache_clean = existing->is_cache_clean;
+			*overlap_cache_clean =
+				existing->attrs &
+				(DMA_ATTR_DEBUGGING_IGNORE_CACHELINES |
+				 DMA_ATTR_REQUIRE_COHERENT);
 	}
 	spin_unlock_irqrestore(&radix_lock, flags);
 
@@ -544,12 +550,13 @@ void debug_dma_dump_mappings(struct device *dev)
 			if (!dev || dev == entry->dev) {
 				cln = to_cacheline_number(entry);
 				dev_info(entry->dev,
-					 "%s idx %d P=%pa D=%llx L=%llx cln=%pa %s %s\n",
+					 "%s idx %d P=%pa D=%llx L=%llx cln=%pa %s %s attrs=0x%lx\n",
 					 type2name[entry->type], idx,
 					 &entry->paddr, entry->dev_addr,
 					 entry->size, &cln,
 					 dir2name[entry->direction],
-					 maperr2str[entry->map_err_type]);
+					 maperr2str[entry->map_err_type],
+					 entry->attrs);
 			}
 		}
 		spin_unlock_irqrestore(&bucket->lock, flags);
@@ -575,14 +582,15 @@ static int dump_show(struct seq_file *seq, void *v)
 		list_for_each_entry(entry, &bucket->list, list) {
 			cln = to_cacheline_number(entry);
 			seq_printf(seq,
-				   "%s %s %s idx %d P=%pa D=%llx L=%llx cln=%pa %s %s\n",
+				   "%s %s %s idx %d P=%pa D=%llx L=%llx cln=%pa %s %s attrs=0x%lx\n",
 				   dev_driver_string(entry->dev),
 				   dev_name(entry->dev),
 				   type2name[entry->type], idx,
 				   &entry->paddr, entry->dev_addr,
 				   entry->size, &cln,
 				   dir2name[entry->direction],
-				   maperr2str[entry->map_err_type]);
+				   maperr2str[entry->map_err_type],
+				   entry->attrs);
 		}
 		spin_unlock_irqrestore(&bucket->lock, flags);
 	}
@@ -594,16 +602,14 @@ DEFINE_SHOW_ATTRIBUTE(dump);
  * 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, unsigned long attrs)
+static void add_dma_entry(struct dma_debug_entry *entry)
 {
+	unsigned long attrs = entry->attrs;
 	bool overlap_cache_clean;
 	struct hash_bucket *bucket;
 	unsigned long flags;
 	int rc;
 
-	entry->is_cache_clean = attrs & (DMA_ATTR_DEBUGGING_IGNORE_CACHELINES |
-					 DMA_ATTR_REQUIRE_COHERENT);
-
 	bucket = get_hash_bucket(entry, &flags);
 	hash_bucket_add(bucket, entry);
 	put_hash_bucket(bucket, flags);
@@ -612,9 +618,10 @@ static void add_dma_entry(struct dma_debug_entry *entry, unsigned long attrs)
 	if (rc == -ENOMEM) {
 		pr_err_once("cacheline tracking ENOMEM, dma-debug disabled\n");
 		global_disable = true;
-	} else if (rc == -EEXIST &&
-		   !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
-		   !(entry->is_cache_clean && overlap_cache_clean) &&
+	} else if (rc == -EEXIST && !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
+		   !(attrs & (DMA_ATTR_DEBUGGING_IGNORE_CACHELINES |
+			      DMA_ATTR_REQUIRE_COHERENT) &&
+		     overlap_cache_clean) &&
 		   !(IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) &&
 		     is_swiotlb_active(entry->dev))) {
 		err_printk(entry->dev, entry,
@@ -1066,6 +1073,19 @@ static void check_unmap(struct dma_debug_entry *ref)
 			   type2name[entry->type]);
 	}
 
+	/*
+	 * This may be no bug in reality - but DMA API is still expects
+	 * that entry is unmapped with same attributes as it was mapped.
+	 */
+	if (ref->attrs != entry->attrs) {
+		err_printk(ref->dev, entry,
+			   "device driver frees "
+			   "DMA memory with different attributes "
+			   "[device address=0x%016llx] [size=%llu bytes] "
+			   "[mapped with 0x%lx] [unmapped with 0x%lx]\n",
+			   ref->dev_addr, ref->size, entry->attrs, ref->attrs);
+	}
+
 	hash_bucket_del(entry);
 	put_hash_bucket(bucket, flags);
 
@@ -1249,6 +1269,7 @@ void debug_dma_map_phys(struct device *dev, phys_addr_t phys, size_t size,
 	entry->size      = size;
 	entry->direction = direction;
 	entry->map_err_type = MAP_ERR_NOT_CHECKED;
+	entry->attrs     = attrs;
 
 	if (!(attrs & DMA_ATTR_MMIO)) {
 		check_for_stack(dev, phys);
@@ -1257,7 +1278,7 @@ void debug_dma_map_phys(struct device *dev, phys_addr_t phys, size_t size,
 			check_for_illegal_area(dev, phys_to_virt(phys), size);
 	}
 
-	add_dma_entry(entry, attrs);
+	add_dma_entry(entry);
 }
 
 void debug_dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
@@ -1344,10 +1365,11 @@ void debug_dma_map_sg(struct device *dev, struct scatterlist *sg,
 		entry->direction      = direction;
 		entry->sg_call_ents   = nents;
 		entry->sg_mapped_ents = mapped_ents;
+		entry->attrs          = attrs;
 
 		check_sg_segment(dev, s);
 
-		add_dma_entry(entry, attrs);
+		add_dma_entry(entry);
 	}
 }
 
@@ -1439,8 +1461,9 @@ void debug_dma_alloc_coherent(struct device *dev, size_t size,
 	entry->size      = size;
 	entry->dev_addr  = dma_addr;
 	entry->direction = DMA_BIDIRECTIONAL;
+	entry->attrs     = attrs;
 
-	add_dma_entry(entry, attrs);
+	add_dma_entry(entry);
 }
 
 void debug_dma_free_coherent(struct device *dev, size_t size,
@@ -1584,8 +1607,9 @@ void debug_dma_alloc_pages(struct device *dev, struct page *page,
 	entry->size      = size;
 	entry->dev_addr  = dma_addr;
 	entry->direction = direction;
+	entry->attrs     = attrs;
 
-	add_dma_entry(entry, attrs);
+	add_dma_entry(entry);
 }
 
 void debug_dma_free_pages(struct device *dev, struct page *page,

---
base-commit: f5ebf241c407dbf629fcf515015e139fcea2c2f0
change-id: 20260323-dma-attrs-debug-85e282d6f3bb

Best regards,
--  
Leon Romanovsky <leonro@nvidia.com>