[PATCH] dma-debug: Check for drivers mapping vmalloc addresses

[PATCH] dma-debug: Check for drivers mapping vmalloc addresses

[Stephen Boyd]

I recently debugged a DMA mapping oops where a driver was trying to map
a buffer returned from request_firmware() with dma_map_single(). Memory
returned from request_firmware() is mapped into the vmalloc region and
this isn't a valid region to map with dma_map_single() per the DMA
documentation's "What memory is DMA'able?" section.

Unfortunately, we don't really check that in the DMA debugging code, so
enabling DMA debugging doesn't help catch this problem. Let's add a new
DMA debug function to check for a vmalloc address and print a warning if
this happens. This makes it a little easier to debug these sorts of
problems, instead of seeing odd behavior or crashes when drivers attempt
to map the vmalloc space for DMA.

Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 include/linux/dma-debug.h   |  8 ++++++++
 include/linux/dma-mapping.h |  1 +
 kernel/dma/debug.c          | 12 ++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index a785f2507159..5aec2ca8a426 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -32,6 +32,9 @@ extern void dma_debug_add_bus(struct bus_type *bus);
 
 extern int dma_debug_resize_entries(u32 num_entries);
 
+extern void debug_dma_check_vmalloc(struct device *dev, const void *addr,
+				    unsigned long len);
+
 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,
@@ -103,6 +106,11 @@ static inline int dma_debug_resize_entries(u32 num_entries)
 	return 0;
 }
 
+static inline void debug_dma_check_vmalloc(struct device *dev, const void *addr,
+					   unsigned long len)
+{
+}
+
 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,
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index eafd6f318e78..a0e67fd5433c 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -232,6 +232,7 @@ static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
 	dma_addr_t addr;
 
 	BUG_ON(!valid_dma_direction(dir));
+	debug_dma_check_vmalloc(dev, ptr, size);
 	addr = ops->map_page(dev, virt_to_page(ptr),
 			     offset_in_page(ptr), size,
 			     dir, attrs);
diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
index c007d25bee09..7ee7978868d4 100644
--- a/kernel/dma/debug.c
+++ b/kernel/dma/debug.c
@@ -1312,6 +1312,18 @@ static void check_sg_segment(struct device *dev, struct scatterlist *sg)
 #endif
 }
 
+void debug_dma_check_vmalloc(struct device *dev, const void *addr,
+		unsigned long len)
+{
+	if (unlikely(dma_debug_disabled()))
+		return;
+
+	if (is_vmalloc_addr(addr))
+		err_printk(dev, NULL, "DMA-API: device driver maps memory from vmalloc area [addr=%p] [len=%lu]\n",
+			   addr, len);
+}
+EXPORT_SYMBOL(debug_dma_check_vmalloc);
+
 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)
-- 
Sent by a computer through tubes

Re: [PATCH] dma-debug: Check for drivers mapping vmalloc addresses

[Christoph Hellwig]

On Thu, Sep 20, 2018 at 03:35:59PM -0700, Stephen Boyd wrote:
> I recently debugged a DMA mapping oops where a driver was trying to map
> a buffer returned from request_firmware() with dma_map_single(). Memory
> returned from request_firmware() is mapped into the vmalloc region and
> this isn't a valid region to map with dma_map_single() per the DMA
> documentation's "What memory is DMA'able?" section.
> 
> Unfortunately, we don't really check that in the DMA debugging code, so
> enabling DMA debugging doesn't help catch this problem. Let's add a new
> DMA debug function to check for a vmalloc address and print a warning if
> this happens. This makes it a little easier to debug these sorts of
> problems, instead of seeing odd behavior or crashes when drivers attempt
> to map the vmalloc space for DMA.

This looks sensible to me.  I wonder if we shouldn't just throw in
an unconditional

	WARN_ON_ONCE(is_vmalloc_addr(ptr)));

into dma_map_single_attrs, though.

Re: [PATCH] dma-debug: Check for drivers mapping vmalloc addresses

[Stephen Boyd]

Quoting Christoph Hellwig (2018-09-20 22:25:37)
> On Thu, Sep 20, 2018 at 03:35:59PM -0700, Stephen Boyd wrote:
> > I recently debugged a DMA mapping oops where a driver was trying to map
> > a buffer returned from request_firmware() with dma_map_single(). Memory
> > returned from request_firmware() is mapped into the vmalloc region and
> > this isn't a valid region to map with dma_map_single() per the DMA
> > documentation's "What memory is DMA'able?" section.
> > 
> > Unfortunately, we don't really check that in the DMA debugging code, so
> > enabling DMA debugging doesn't help catch this problem. Let's add a new
> > DMA debug function to check for a vmalloc address and print a warning if
> > this happens. This makes it a little easier to debug these sorts of
> > problems, instead of seeing odd behavior or crashes when drivers attempt
> > to map the vmalloc space for DMA.
> 
> This looks sensible to me.  I wonder if we shouldn't just throw in
> an unconditional
> 
>         WARN_ON_ONCE(is_vmalloc_addr(ptr)));
> 
> into dma_map_single_attrs, though.

Yes that works just as well if we're willing to take the overhead of the
check all the time. This isn't a hotpath?

Re: [PATCH] dma-debug: Check for drivers mapping vmalloc addresses

[Robin Murphy]

Hi Stephen,

On 20/09/18 23:35, Stephen Boyd wrote:
> I recently debugged a DMA mapping oops where a driver was trying to map
> a buffer returned from request_firmware() with dma_map_single(). Memory
> returned from request_firmware() is mapped into the vmalloc region and
> this isn't a valid region to map with dma_map_single() per the DMA
> documentation's "What memory is DMA'able?" section.
> 
> Unfortunately, we don't really check that in the DMA debugging code, so
> enabling DMA debugging doesn't help catch this problem. Let's add a new
> DMA debug function to check for a vmalloc address and print a warning if
> this happens. This makes it a little easier to debug these sorts of
> problems, instead of seeing odd behavior or crashes when drivers attempt
> to map the vmalloc space for DMA.

Good idea!

> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>   include/linux/dma-debug.h   |  8 ++++++++
>   include/linux/dma-mapping.h |  1 +
>   kernel/dma/debug.c          | 12 ++++++++++++
>   3 files changed, 21 insertions(+)

However I can't help thinking this looks a little heavyweight for a 
single specific check. It seems like it would be enough to simply pass 
the VA as an extra argument to debug_dma_map_page(), since we already 
have the map_single argument which would indicate when it's valid. What 
do you reckon?

Robin.

> diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
> index a785f2507159..5aec2ca8a426 100644
> --- a/include/linux/dma-debug.h
> +++ b/include/linux/dma-debug.h
> @@ -32,6 +32,9 @@ extern void dma_debug_add_bus(struct bus_type *bus);
>   
>   extern int dma_debug_resize_entries(u32 num_entries);
>   
> +extern void debug_dma_check_vmalloc(struct device *dev, const void *addr,
> +				    unsigned long len);
> +
>   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,
> @@ -103,6 +106,11 @@ static inline int dma_debug_resize_entries(u32 num_entries)
>   	return 0;
>   }
>   
> +static inline void debug_dma_check_vmalloc(struct device *dev, const void *addr,
> +					   unsigned long len)
> +{
> +}
> +
>   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,
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index eafd6f318e78..a0e67fd5433c 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -232,6 +232,7 @@ static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
>   	dma_addr_t addr;
>   
>   	BUG_ON(!valid_dma_direction(dir));
> +	debug_dma_check_vmalloc(dev, ptr, size);
>   	addr = ops->map_page(dev, virt_to_page(ptr),
>   			     offset_in_page(ptr), size,
>   			     dir, attrs);
> diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
> index c007d25bee09..7ee7978868d4 100644
> --- a/kernel/dma/debug.c
> +++ b/kernel/dma/debug.c
> @@ -1312,6 +1312,18 @@ static void check_sg_segment(struct device *dev, struct scatterlist *sg)
>   #endif
>   }
>   
> +void debug_dma_check_vmalloc(struct device *dev, const void *addr,
> +		unsigned long len)
> +{
> +	if (unlikely(dma_debug_disabled()))
> +		return;
> +
> +	if (is_vmalloc_addr(addr))
> +		err_printk(dev, NULL, "DMA-API: device driver maps memory from vmalloc area [addr=%p] [len=%lu]\n",
> +			   addr, len);
> +}
> +EXPORT_SYMBOL(debug_dma_check_vmalloc);
> +
>   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)
> 

Re: [PATCH] dma-debug: Check for drivers mapping vmalloc addresses

[Stephen Boyd]

Quoting Robin Murphy (2018-09-21 04:09:50)
> Hi Stephen,
> 
> On 20/09/18 23:35, Stephen Boyd wrote:
> > I recently debugged a DMA mapping oops where a driver was trying to map
> > a buffer returned from request_firmware() with dma_map_single(). Memory
> > returned from request_firmware() is mapped into the vmalloc region and
> > this isn't a valid region to map with dma_map_single() per the DMA
> > documentation's "What memory is DMA'able?" section.
> > 
> > Unfortunately, we don't really check that in the DMA debugging code, so
> > enabling DMA debugging doesn't help catch this problem. Let's add a new
> > DMA debug function to check for a vmalloc address and print a warning if
> > this happens. This makes it a little easier to debug these sorts of
> > problems, instead of seeing odd behavior or crashes when drivers attempt
> > to map the vmalloc space for DMA.
> 
> Good idea!
> 
> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > Cc: Robin Murphy <robin.murphy@arm.com>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >   include/linux/dma-debug.h   |  8 ++++++++
> >   include/linux/dma-mapping.h |  1 +
> >   kernel/dma/debug.c          | 12 ++++++++++++
> >   3 files changed, 21 insertions(+)
> 
> However I can't help thinking this looks a little heavyweight for a 
> single specific check. It seems like it would be enough to simply pass 
> the VA as an extra argument to debug_dma_map_page(), since we already 
> have the map_single argument which would indicate when it's valid. What 
> do you reckon?

I thought about augmenting debug_dma_map_page() but that has another
problem where those checks are done after the page has been mapped. The
kernel can oops when it's operating on the incorrect page so we need to
check things before calling the dma ops.

So I split that check up into pre_map and post_map parts but then that
seemed like overkill just to check for this vmalloc problem. Here's the
patch if you're interested. I can make it a little nicer if you think
it's also worthwhile.

---8<----
 include/linux/dma-debug.h   |  8 ++++++++
 include/linux/dma-mapping.h |  3 +++
 kernel/dma/debug.c          | 24 ++++++++++++++++--------
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index 5aec2ca8a426..3287240a474c 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -35,6 +35,9 @@ extern int dma_debug_resize_entries(u32 num_entries);
 extern void debug_dma_check_vmalloc(struct device *dev, const void *addr,
 				    unsigned long len);
 
+extern void debug_dma_pre_map_page(struct device *dev, struct page *page,
+				   size_t offset, size_t size);
+
 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,
@@ -111,6 +114,11 @@ static inline void debug_dma_check_vmalloc(struct device *dev, const void *addr,
 {
 }
 
+static inline void debug_dma_pre_map_page(struct device *dev, struct page *page,
+					  size_t offset, size_t size)
+{
+}
+
 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,
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index a0e67fd5433c..4839bbb4fdc7 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -233,6 +233,8 @@ static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
 
 	BUG_ON(!valid_dma_direction(dir));
 	debug_dma_check_vmalloc(dev, ptr, size);
+	debug_dma_pre_map_page(dev, virt_to_page(ptr),
+			       offset_in_page(ptr), size);
 	addr = ops->map_page(dev, virt_to_page(ptr),
 			     offset_in_page(ptr), size,
 			     dir, attrs);
@@ -296,6 +298,7 @@ static inline dma_addr_t dma_map_page_attrs(struct device *dev,
 	dma_addr_t addr;
 
 	BUG_ON(!valid_dma_direction(dir));
+	debug_dma_pre_map_page(dev, page, offset, size);
 	addr = ops->map_page(dev, page, offset, size, dir, attrs);
 	debug_dma_map_page(dev, page, offset, size, dir, addr, false);
 
diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
index 7ee7978868d4..dd1ff9f7d783 100644
--- a/kernel/dma/debug.c
+++ b/kernel/dma/debug.c
@@ -1324,6 +1324,22 @@ void debug_dma_check_vmalloc(struct device *dev, const void *addr,
 }
 EXPORT_SYMBOL(debug_dma_check_vmalloc);
 
+void debug_dma_pre_map_page(struct device *dev, struct page *page, size_t offset,
+			    size_t size)
+{
+	if (unlikely(dma_debug_disabled()))
+		return;
+
+	check_for_stack(dev, page, offset);
+
+	if (!PageHighMem(page)) {
+		void *addr = page_address(page) + offset;
+
+		check_for_illegal_area(dev, addr, size);
+	}
+}
+EXPORT_SYMBOL(debug_dma_pre_map_page);
+
 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)
@@ -1352,14 +1368,6 @@ void debug_dma_map_page(struct device *dev, struct page *page, size_t offset,
 	if (map_single)
 		entry->type = dma_debug_single;
 
-	check_for_stack(dev, page, offset);
-
-	if (!PageHighMem(page)) {
-		void *addr = page_address(page) + offset;
-
-		check_for_illegal_area(dev, addr, size);
-	}
-
 	add_dma_entry(entry);
 }
 EXPORT_SYMBOL(debug_dma_map_page);
-- 
Sent by a computer through tubes

Re: [PATCH] dma-debug: Check for drivers mapping vmalloc addresses

[Robin Murphy]

On 21/09/18 18:39, Stephen Boyd wrote:
> Quoting Robin Murphy (2018-09-21 04:09:50)
>> Hi Stephen,
>>
>> On 20/09/18 23:35, Stephen Boyd wrote:
>>> I recently debugged a DMA mapping oops where a driver was trying to map
>>> a buffer returned from request_firmware() with dma_map_single(). Memory
>>> returned from request_firmware() is mapped into the vmalloc region and
>>> this isn't a valid region to map with dma_map_single() per the DMA
>>> documentation's "What memory is DMA'able?" section.
>>>
>>> Unfortunately, we don't really check that in the DMA debugging code, so
>>> enabling DMA debugging doesn't help catch this problem. Let's add a new
>>> DMA debug function to check for a vmalloc address and print a warning if
>>> this happens. This makes it a little easier to debug these sorts of
>>> problems, instead of seeing odd behavior or crashes when drivers attempt
>>> to map the vmalloc space for DMA.
>>
>> Good idea!
>>
>>> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Cc: Robin Murphy <robin.murphy@arm.com>
>>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>>> ---
>>>    include/linux/dma-debug.h   |  8 ++++++++
>>>    include/linux/dma-mapping.h |  1 +
>>>    kernel/dma/debug.c          | 12 ++++++++++++
>>>    3 files changed, 21 insertions(+)
>>
>> However I can't help thinking this looks a little heavyweight for a
>> single specific check. It seems like it would be enough to simply pass
>> the VA as an extra argument to debug_dma_map_page(), since we already
>> have the map_single argument which would indicate when it's valid. What
>> do you reckon?
> 
> I thought about augmenting debug_dma_map_page() but that has another
> problem where those checks are done after the page has been mapped. The
> kernel can oops when it's operating on the incorrect page so we need to
> check things before calling the dma ops.

Oh, right, as usual I've managed to overlook a subtlety :)

In fact, with memory now jogged, I think the virt_to_page() itself is 
allowed to blow up if !virt_addr_valid(), so we probably do need 
something like your original idea to be properly robust. I guess it 
probably makes sense to implement that as debug_dma_map_single() then, 
since the purpose is really to cover any VA checks specific to that case 
which debug_dma_map_page() can't be expected to do after the fact.

Thanks,
Robin.

> So I split that check up into pre_map and post_map parts but then that
> seemed like overkill just to check for this vmalloc problem. Here's the
> patch if you're interested. I can make it a little nicer if you think
> it's also worthwhile.
> 
> ---8<----
>   include/linux/dma-debug.h   |  8 ++++++++
>   include/linux/dma-mapping.h |  3 +++
>   kernel/dma/debug.c          | 24 ++++++++++++++++--------
>   3 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
> index 5aec2ca8a426..3287240a474c 100644
> --- a/include/linux/dma-debug.h
> +++ b/include/linux/dma-debug.h
> @@ -35,6 +35,9 @@ extern int dma_debug_resize_entries(u32 num_entries);
>   extern void debug_dma_check_vmalloc(struct device *dev, const void *addr,
>   				    unsigned long len);
>   
> +extern void debug_dma_pre_map_page(struct device *dev, struct page *page,
> +				   size_t offset, size_t size);
> +
>   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,
> @@ -111,6 +114,11 @@ static inline void debug_dma_check_vmalloc(struct device *dev, const void *addr,
>   {
>   }
>   
> +static inline void debug_dma_pre_map_page(struct device *dev, struct page *page,
> +					  size_t offset, size_t size)
> +{
> +}
> +
>   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,
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index a0e67fd5433c..4839bbb4fdc7 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -233,6 +233,8 @@ static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
>   
>   	BUG_ON(!valid_dma_direction(dir));
>   	debug_dma_check_vmalloc(dev, ptr, size);
> +	debug_dma_pre_map_page(dev, virt_to_page(ptr),
> +			       offset_in_page(ptr), size);
>   	addr = ops->map_page(dev, virt_to_page(ptr),
>   			     offset_in_page(ptr), size,
>   			     dir, attrs);
> @@ -296,6 +298,7 @@ static inline dma_addr_t dma_map_page_attrs(struct device *dev,
>   	dma_addr_t addr;
>   
>   	BUG_ON(!valid_dma_direction(dir));
> +	debug_dma_pre_map_page(dev, page, offset, size);
>   	addr = ops->map_page(dev, page, offset, size, dir, attrs);
>   	debug_dma_map_page(dev, page, offset, size, dir, addr, false);
>   
> diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
> index 7ee7978868d4..dd1ff9f7d783 100644
> --- a/kernel/dma/debug.c
> +++ b/kernel/dma/debug.c
> @@ -1324,6 +1324,22 @@ void debug_dma_check_vmalloc(struct device *dev, const void *addr,
>   }
>   EXPORT_SYMBOL(debug_dma_check_vmalloc);
>   
> +void debug_dma_pre_map_page(struct device *dev, struct page *page, size_t offset,
> +			    size_t size)
> +{
> +	if (unlikely(dma_debug_disabled()))
> +		return;
> +
> +	check_for_stack(dev, page, offset);
> +
> +	if (!PageHighMem(page)) {
> +		void *addr = page_address(page) + offset;
> +
> +		check_for_illegal_area(dev, addr, size);
> +	}
> +}
> +EXPORT_SYMBOL(debug_dma_pre_map_page);
> +
>   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)
> @@ -1352,14 +1368,6 @@ void debug_dma_map_page(struct device *dev, struct page *page, size_t offset,
>   	if (map_single)
>   		entry->type = dma_debug_single;
>   
> -	check_for_stack(dev, page, offset);
> -
> -	if (!PageHighMem(page)) {
> -		void *addr = page_address(page) + offset;
> -
> -		check_for_illegal_area(dev, addr, size);
> -	}
> -
>   	add_dma_entry(entry);
>   }
>   EXPORT_SYMBOL(debug_dma_map_page);
> 

Re: [PATCH] dma-debug: Check for drivers mapping vmalloc addresses

[Stephen Boyd]

Quoting Robin Murphy (2018-09-26 06:24:42)
> On 21/09/18 18:39, Stephen Boyd wrote:
> > Quoting Robin Murphy (2018-09-21 04:09:50)
> >> Hi Stephen,
> >>
> >> On 20/09/18 23:35, Stephen Boyd wrote:
> >>> I recently debugged a DMA mapping oops where a driver was trying to map
> >>> a buffer returned from request_firmware() with dma_map_single(). Memory
> >>> returned from request_firmware() is mapped into the vmalloc region and
> >>> this isn't a valid region to map with dma_map_single() per the DMA
> >>> documentation's "What memory is DMA'able?" section.
> >>>
> >>> Unfortunately, we don't really check that in the DMA debugging code, so
> >>> enabling DMA debugging doesn't help catch this problem. Let's add a new
> >>> DMA debug function to check for a vmalloc address and print a warning if
> >>> this happens. This makes it a little easier to debug these sorts of
> >>> problems, instead of seeing odd behavior or crashes when drivers attempt
> >>> to map the vmalloc space for DMA.
> >>
> >> Good idea!
> >>
> >>> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> >>> Cc: Robin Murphy <robin.murphy@arm.com>
> >>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> >>> ---
> >>>    include/linux/dma-debug.h   |  8 ++++++++
> >>>    include/linux/dma-mapping.h |  1 +
> >>>    kernel/dma/debug.c          | 12 ++++++++++++
> >>>    3 files changed, 21 insertions(+)
> >>
> >> However I can't help thinking this looks a little heavyweight for a
> >> single specific check. It seems like it would be enough to simply pass
> >> the VA as an extra argument to debug_dma_map_page(), since we already
> >> have the map_single argument which would indicate when it's valid. What
> >> do you reckon?
> > 
> > I thought about augmenting debug_dma_map_page() but that has another
> > problem where those checks are done after the page has been mapped. The
> > kernel can oops when it's operating on the incorrect page so we need to
> > check things before calling the dma ops.
> 
> Oh, right, as usual I've managed to overlook a subtlety :)
> 
> In fact, with memory now jogged, I think the virt_to_page() itself is 
> allowed to blow up if !virt_addr_valid(), so we probably do need 
> something like your original idea to be properly robust. I guess it 
> probably makes sense to implement that as debug_dma_map_single() then, 
> since the purpose is really to cover any VA checks specific to that case 
> which debug_dma_map_page() can't be expected to do after the fact.
> 

Ok. I can rename it to dma_debug_map_single() and throw in a
virt_addr_valid() check too. Thanks for the suggestion.