linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC] mm: Add debug_virt_to_phys()
@ 2016-11-12  0:44 Florian Fainelli
  2016-11-12  1:49 ` Nicolas Pitre
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Florian Fainelli @ 2016-11-12  0:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Florian Fainelli, Russell King, Catalin Marinas, Will Deacon,
	Arnd Bergmann, Nicolas Pitre, Chris Brandt, Pratyush Anand,
	Ard Biesheuvel, Mark Rutland, James Morse, Neeraj Upadhyay,
	Laura Abbott, Andrew Morton, Vlastimil Babka, Michal Hocko,
	Kirill A. Shutemov, Jerome Marchand, Konstantin Khlebnikov,
	Joonsoo Kim, moderated list:ARM PORT,
	open list:GENERIC INCLUDE/ASM HEADER FILES,
	open list:MEMORY MANAGEMENT

When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
debug_virt_to_phys() which helps catch vmalloc space addresses being
passed. This is helpful in debugging bogus drivers that just assume
linear mappings all over the place.

For ARM, ARM64, Unicore32 and Microblaze, the architectures define
__virt_to_phys() as being the functional implementation of the address
translation, so we special case the debug stub to call into
__virt_to_phys directly.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm/include/asm/memory.h      |  4 ++++
 arch/arm64/include/asm/memory.h    |  4 ++++
 include/asm-generic/memory_model.h |  4 ++++
 mm/debug.c                         | 15 +++++++++++++++
 4 files changed, 27 insertions(+)

diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index 76cbd9c674df..448dec9b8b00 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -260,11 +260,15 @@ static inline unsigned long __phys_to_virt(phys_addr_t x)
  * translation for translating DMA addresses.  Use the driver
  * DMA support - see dma-mapping.h.
  */
+#ifndef CONFIG_DEBUG_VM
 #define virt_to_phys virt_to_phys
 static inline phys_addr_t virt_to_phys(const volatile void *x)
 {
 	return __virt_to_phys((unsigned long)(x));
 }
+#else
+#define virt_to_phys debug_virt_to_phys
+#endif
 
 #define phys_to_virt phys_to_virt
 static inline void *phys_to_virt(phys_addr_t x)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index b71086d25195..c9e436b28523 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -186,11 +186,15 @@ extern u64			kimage_voffset;
  * translation for translating DMA addresses.  Use the driver
  * DMA support - see dma-mapping.h.
  */
+#ifndef CONFIG_DEBUG_VM
 #define virt_to_phys virt_to_phys
 static inline phys_addr_t virt_to_phys(const volatile void *x)
 {
 	return __virt_to_phys((unsigned long)(x));
 }
+#else
+#define virt_to_phys debug_virt_to_phys
+#endif
 
 #define phys_to_virt phys_to_virt
 static inline void *phys_to_virt(phys_addr_t x)
diff --git a/include/asm-generic/memory_model.h b/include/asm-generic/memory_model.h
index 5148150cc80b..426085757258 100644
--- a/include/asm-generic/memory_model.h
+++ b/include/asm-generic/memory_model.h
@@ -80,6 +80,10 @@
 #define page_to_pfn __page_to_pfn
 #define pfn_to_page __pfn_to_page
 
+#ifdef CONFIG_DEBUG_VM
+unsigned long debug_virt_to_phys(volatile void *address);
+#endif /* CONFIG_DEBUG_VM */
+
 #endif /* __ASSEMBLY__ */
 
 #endif
diff --git a/mm/debug.c b/mm/debug.c
index 9feb699c5d25..72b2ca9b11f4 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -161,4 +161,19 @@ void dump_mm(const struct mm_struct *mm)
 	);
 }
 
+#include <asm/memory.h>
+#include <linux/mm.h>
+
+unsigned long debug_virt_to_phys(volatile void *address)
+{
+	BUG_ON(is_vmalloc_addr((const void *)address));
+#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_UNICORE32) || \
+	defined(CONFIG_MICROBLAZE)
+	return __virt_to_phys(address);
+#else
+	return virt_to_phys(address);
+#endif
+}
+EXPORT_SYMBOL(debug_virt_to_phys);
+
 #endif		/* CONFIG_DEBUG_VM */
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] mm: Add debug_virt_to_phys()
  2016-11-12  0:44 [PATCH RFC] mm: Add debug_virt_to_phys() Florian Fainelli
@ 2016-11-12  1:49 ` Nicolas Pitre
  2016-11-12  3:39   ` Florian Fainelli
  2016-11-12  5:43 ` Will Deacon
  2016-11-14 18:45 ` Laura Abbott
  2 siblings, 1 reply; 8+ messages in thread
From: Nicolas Pitre @ 2016-11-12  1:49 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-kernel, Russell King, Catalin Marinas, Will Deacon,
	Arnd Bergmann, Chris Brandt, Pratyush Anand, Ard Biesheuvel,
	Mark Rutland, James Morse, Neeraj Upadhyay, Laura Abbott,
	Andrew Morton, Vlastimil Babka, Michal Hocko, Kirill A. Shutemov,
	Jerome Marchand, Konstantin Khlebnikov, Joonsoo Kim,
	moderated list:ARM PORT,
	open list:GENERIC INCLUDE/ASM HEADER FILES,
	open list:MEMORY MANAGEMENT

On Fri, 11 Nov 2016, Florian Fainelli wrote:

> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
> debug_virt_to_phys() which helps catch vmalloc space addresses being
> passed. This is helpful in debugging bogus drivers that just assume
> linear mappings all over the place.
> 
> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
> __virt_to_phys() as being the functional implementation of the address
> translation, so we special case the debug stub to call into
> __virt_to_phys directly.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arm/include/asm/memory.h      |  4 ++++
>  arch/arm64/include/asm/memory.h    |  4 ++++
>  include/asm-generic/memory_model.h |  4 ++++
>  mm/debug.c                         | 15 +++++++++++++++
>  4 files changed, 27 insertions(+)
> 
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 76cbd9c674df..448dec9b8b00 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -260,11 +260,15 @@ static inline unsigned long __phys_to_virt(phys_addr_t x)
>   * translation for translating DMA addresses.  Use the driver
>   * DMA support - see dma-mapping.h.
>   */
> +#ifndef CONFIG_DEBUG_VM
>  #define virt_to_phys virt_to_phys
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
>  	return __virt_to_phys((unsigned long)(x));
>  }
> +#else
> +#define virt_to_phys debug_virt_to_phys
> +#endif
[...]

Why don't you do something more like:

 static inline phys_addr_t virt_to_phys(const volatile void *x)
 {
+        debug_virt_to_phys(x);
         return __virt_to_phys((unsigned long)(x));
 }

[...]

static inline void debug_virt_to_phys(const void *address)
{
#ifdef CONFIG_DEBUG_VM
        BUG_ON(is_vmalloc_addr(address));
#endif
}

?


Nicolas

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] mm: Add debug_virt_to_phys()
  2016-11-12  1:49 ` Nicolas Pitre
@ 2016-11-12  3:39   ` Florian Fainelli
  2016-11-12 16:32     ` Nicolas Pitre
  0 siblings, 1 reply; 8+ messages in thread
From: Florian Fainelli @ 2016-11-12  3:39 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: linux-kernel, Russell King, Catalin Marinas, Will Deacon,
	Arnd Bergmann, Chris Brandt, Pratyush Anand, Ard Biesheuvel,
	Mark Rutland, James Morse, Neeraj Upadhyay, Laura Abbott,
	Andrew Morton, Vlastimil Babka, Michal Hocko, Kirill A. Shutemov,
	Jerome Marchand, Konstantin Khlebnikov, Joonsoo Kim,
	moderated list:ARM PORT,
	open list:GENERIC INCLUDE/ASM HEADER FILES,
	open list:MEMORY MANAGEMENT

Le 11/11/2016 à 17:49, Nicolas Pitre a écrit :
> On Fri, 11 Nov 2016, Florian Fainelli wrote:
> 
>> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
>> debug_virt_to_phys() which helps catch vmalloc space addresses being
>> passed. This is helpful in debugging bogus drivers that just assume
>> linear mappings all over the place.
>>
>> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
>> __virt_to_phys() as being the functional implementation of the address
>> translation, so we special case the debug stub to call into
>> __virt_to_phys directly.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  arch/arm/include/asm/memory.h      |  4 ++++
>>  arch/arm64/include/asm/memory.h    |  4 ++++
>>  include/asm-generic/memory_model.h |  4 ++++
>>  mm/debug.c                         | 15 +++++++++++++++
>>  4 files changed, 27 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
>> index 76cbd9c674df..448dec9b8b00 100644
>> --- a/arch/arm/include/asm/memory.h
>> +++ b/arch/arm/include/asm/memory.h
>> @@ -260,11 +260,15 @@ static inline unsigned long __phys_to_virt(phys_addr_t x)
>>   * translation for translating DMA addresses.  Use the driver
>>   * DMA support - see dma-mapping.h.
>>   */
>> +#ifndef CONFIG_DEBUG_VM
>>  #define virt_to_phys virt_to_phys
>>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>>  {
>>  	return __virt_to_phys((unsigned long)(x));
>>  }
>> +#else
>> +#define virt_to_phys debug_virt_to_phys
>> +#endif
> [...]
> 
> Why don't you do something more like:
> 
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
> +        debug_virt_to_phys(x);
>          return __virt_to_phys((unsigned long)(x));
>  }
> 
> [...]
> 
> static inline void debug_virt_to_phys(const void *address)
> {
> #ifdef CONFIG_DEBUG_VM
>         BUG_ON(is_vmalloc_addr(address));
> #endif
> }
> 
> ?

This is how I started doing it initially, but to get the
is_vmalloc_addr() definition, we need to include linux/mm.h and then
everything falls apart because of the include and dependencies chain. We
could open code the is_vmalloc_addr() check because that's simple
enough, but we still need VMALLOC_START and VMALLOC_END and to get there
we need to include pgtable.h, and there are still some inclusion
problems in doing so.

The other reason was to avoid putting the same checks in architecture
specific code, except for those like ARM/ARM64/Unicore32/Microblaze
where I could not find a simple way to undefined virt_to_phys and
redefine it to debug_virt_to_phys.

Do you see an other way around this? Thanks!
-- 
Florian

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] mm: Add debug_virt_to_phys()
  2016-11-12  0:44 [PATCH RFC] mm: Add debug_virt_to_phys() Florian Fainelli
  2016-11-12  1:49 ` Nicolas Pitre
@ 2016-11-12  5:43 ` Will Deacon
  2016-11-12 19:29   ` Florian Fainelli
  2016-11-14 18:45 ` Laura Abbott
  2 siblings, 1 reply; 8+ messages in thread
From: Will Deacon @ 2016-11-12  5:43 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-kernel, Russell King, Catalin Marinas, Arnd Bergmann,
	Nicolas Pitre, Chris Brandt, Pratyush Anand, Ard Biesheuvel,
	Mark Rutland, James Morse, Neeraj Upadhyay, Laura Abbott,
	Andrew Morton, Vlastimil Babka, Michal Hocko, Kirill A. Shutemov,
	Jerome Marchand, Konstantin Khlebnikov, Joonsoo Kim,
	moderated list:ARM PORT,
	open list:GENERIC INCLUDE/ASM HEADER FILES,
	open list:MEMORY MANAGEMENT

On Fri, Nov 11, 2016 at 04:44:43PM -0800, Florian Fainelli wrote:
> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
> debug_virt_to_phys() which helps catch vmalloc space addresses being
> passed. This is helpful in debugging bogus drivers that just assume
> linear mappings all over the place.
> 
> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
> __virt_to_phys() as being the functional implementation of the address
> translation, so we special case the debug stub to call into
> __virt_to_phys directly.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arm/include/asm/memory.h      |  4 ++++
>  arch/arm64/include/asm/memory.h    |  4 ++++
>  include/asm-generic/memory_model.h |  4 ++++
>  mm/debug.c                         | 15 +++++++++++++++
>  4 files changed, 27 insertions(+)

What's the interaction between this and the DEBUG_VIRTUAL patches from Laura?

http://lkml.kernel.org/r/20161102210054.16621-7-labbott@redhat.com

They seem to be tackling the exact same problem afaict.

Will

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] mm: Add debug_virt_to_phys()
  2016-11-12  3:39   ` Florian Fainelli
@ 2016-11-12 16:32     ` Nicolas Pitre
  0 siblings, 0 replies; 8+ messages in thread
From: Nicolas Pitre @ 2016-11-12 16:32 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-kernel, Russell King, Catalin Marinas, Will Deacon,
	Arnd Bergmann, Chris Brandt, Pratyush Anand, Ard Biesheuvel,
	Mark Rutland, James Morse, Neeraj Upadhyay, Laura Abbott,
	Andrew Morton, Vlastimil Babka, Michal Hocko, Kirill A. Shutemov,
	Jerome Marchand, Konstantin Khlebnikov, Joonsoo Kim,
	moderated list:ARM PORT,
	open list:GENERIC INCLUDE/ASM HEADER FILES,
	open list:MEMORY MANAGEMENT

[-- Attachment #1: Type: text/plain, Size: 3008 bytes --]

On Fri, 11 Nov 2016, Florian Fainelli wrote:

> Le 11/11/2016 à 17:49, Nicolas Pitre a écrit :
> > On Fri, 11 Nov 2016, Florian Fainelli wrote:
> > 
> >> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
> >> debug_virt_to_phys() which helps catch vmalloc space addresses being
> >> passed. This is helpful in debugging bogus drivers that just assume
> >> linear mappings all over the place.
> >>
> >> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
> >> __virt_to_phys() as being the functional implementation of the address
> >> translation, so we special case the debug stub to call into
> >> __virt_to_phys directly.
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >> ---
> >>  arch/arm/include/asm/memory.h      |  4 ++++
> >>  arch/arm64/include/asm/memory.h    |  4 ++++
> >>  include/asm-generic/memory_model.h |  4 ++++
> >>  mm/debug.c                         | 15 +++++++++++++++
> >>  4 files changed, 27 insertions(+)
> >>
> >> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> >> index 76cbd9c674df..448dec9b8b00 100644
> >> --- a/arch/arm/include/asm/memory.h
> >> +++ b/arch/arm/include/asm/memory.h
> >> @@ -260,11 +260,15 @@ static inline unsigned long __phys_to_virt(phys_addr_t x)
> >>   * translation for translating DMA addresses.  Use the driver
> >>   * DMA support - see dma-mapping.h.
> >>   */
> >> +#ifndef CONFIG_DEBUG_VM
> >>  #define virt_to_phys virt_to_phys
> >>  static inline phys_addr_t virt_to_phys(const volatile void *x)
> >>  {
> >>  	return __virt_to_phys((unsigned long)(x));
> >>  }
> >> +#else
> >> +#define virt_to_phys debug_virt_to_phys
> >> +#endif
> > [...]
> > 
> > Why don't you do something more like:
> > 
> >  static inline phys_addr_t virt_to_phys(const volatile void *x)
> >  {
> > +        debug_virt_to_phys(x);
> >          return __virt_to_phys((unsigned long)(x));
> >  }
> > 
> > [...]
> > 
> > static inline void debug_virt_to_phys(const void *address)
> > {
> > #ifdef CONFIG_DEBUG_VM
> >         BUG_ON(is_vmalloc_addr(address));
> > #endif
> > }
> > 
> > ?
> 
> This is how I started doing it initially, but to get the
> is_vmalloc_addr() definition, we need to include linux/mm.h and then
> everything falls apart because of the include and dependencies chain. We
> could open code the is_vmalloc_addr() check because that's simple
> enough, but we still need VMALLOC_START and VMALLOC_END and to get there
> we need to include pgtable.h, and there are still some inclusion
> problems in doing so.
> 
> The other reason was to avoid putting the same checks in architecture
> specific code, except for those like ARM/ARM64/Unicore32/Microblaze
> where I could not find a simple way to undefined virt_to_phys and
> redefine it to debug_virt_to_phys.

You could still move the check out of line like in your patch. But the 
debug function doesn't have to be the one returning the translated 
address. This has the advantage of cutting on the amount of ifdefery.


Nicolas

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] mm: Add debug_virt_to_phys()
  2016-11-12  5:43 ` Will Deacon
@ 2016-11-12 19:29   ` Florian Fainelli
  0 siblings, 0 replies; 8+ messages in thread
From: Florian Fainelli @ 2016-11-12 19:29 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-kernel, Russell King, Catalin Marinas, Arnd Bergmann,
	Nicolas Pitre, Chris Brandt, Pratyush Anand, Ard Biesheuvel,
	Mark Rutland, James Morse, Neeraj Upadhyay, Laura Abbott,
	Andrew Morton, Vlastimil Babka, Michal Hocko, Kirill A. Shutemov,
	Jerome Marchand, Konstantin Khlebnikov, Joonsoo Kim,
	moderated list:ARM PORT,
	open list:GENERIC INCLUDE/ASM HEADER FILES,
	open list:MEMORY MANAGEMENT

Le 11/11/2016 à 21:43, Will Deacon a écrit :
> On Fri, Nov 11, 2016 at 04:44:43PM -0800, Florian Fainelli wrote:
>> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
>> debug_virt_to_phys() which helps catch vmalloc space addresses being
>> passed. This is helpful in debugging bogus drivers that just assume
>> linear mappings all over the place.
>>
>> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
>> __virt_to_phys() as being the functional implementation of the address
>> translation, so we special case the debug stub to call into
>> __virt_to_phys directly.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  arch/arm/include/asm/memory.h      |  4 ++++
>>  arch/arm64/include/asm/memory.h    |  4 ++++
>>  include/asm-generic/memory_model.h |  4 ++++
>>  mm/debug.c                         | 15 +++++++++++++++
>>  4 files changed, 27 insertions(+)
> 
> What's the interaction between this and the DEBUG_VIRTUAL patches from Laura?
> 
> http://lkml.kernel.org/r/20161102210054.16621-7-labbott@redhat.com
> 
> They seem to be tackling the exact same problem afaict.

Indeed thanks for pointing that out, I guess I could piggy back on this
patchset and try to cover ARM.

Thanks!
-- 
Florian

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] mm: Add debug_virt_to_phys()
  2016-11-12  0:44 [PATCH RFC] mm: Add debug_virt_to_phys() Florian Fainelli
  2016-11-12  1:49 ` Nicolas Pitre
  2016-11-12  5:43 ` Will Deacon
@ 2016-11-14 18:45 ` Laura Abbott
  2016-11-14 19:23   ` Florian Fainelli
  2 siblings, 1 reply; 8+ messages in thread
From: Laura Abbott @ 2016-11-14 18:45 UTC (permalink / raw)
  To: Florian Fainelli, linux-kernel
  Cc: Russell King, Catalin Marinas, Will Deacon, Arnd Bergmann,
	Nicolas Pitre, Chris Brandt, Pratyush Anand, Ard Biesheuvel,
	Mark Rutland, James Morse, Neeraj Upadhyay, Andrew Morton,
	Vlastimil Babka, Michal Hocko, Kirill A. Shutemov,
	Jerome Marchand, Konstantin Khlebnikov, Joonsoo Kim,
	moderated list:ARM PORT,
	open list:GENERIC INCLUDE/ASM HEADER FILES,
	open list:MEMORY MANAGEMENT

On 11/11/2016 04:44 PM, Florian Fainelli wrote:
> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
> debug_virt_to_phys() which helps catch vmalloc space addresses being
> passed. This is helpful in debugging bogus drivers that just assume
> linear mappings all over the place.
> 
> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
> __virt_to_phys() as being the functional implementation of the address
> translation, so we special case the debug stub to call into
> __virt_to_phys directly.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arm/include/asm/memory.h      |  4 ++++
>  arch/arm64/include/asm/memory.h    |  4 ++++
>  include/asm-generic/memory_model.h |  4 ++++
>  mm/debug.c                         | 15 +++++++++++++++
>  4 files changed, 27 insertions(+)
> 
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 76cbd9c674df..448dec9b8b00 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -260,11 +260,15 @@ static inline unsigned long __phys_to_virt(phys_addr_t x)
>   * translation for translating DMA addresses.  Use the driver
>   * DMA support - see dma-mapping.h.
>   */
> +#ifndef CONFIG_DEBUG_VM
>  #define virt_to_phys virt_to_phys
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
>  	return __virt_to_phys((unsigned long)(x));
>  }
> +#else
> +#define virt_to_phys debug_virt_to_phys
> +#endif
>  
>  #define phys_to_virt phys_to_virt
>  static inline void *phys_to_virt(phys_addr_t x)
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index b71086d25195..c9e436b28523 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -186,11 +186,15 @@ extern u64			kimage_voffset;
>   * translation for translating DMA addresses.  Use the driver
>   * DMA support - see dma-mapping.h.
>   */
> +#ifndef CONFIG_DEBUG_VM
>  #define virt_to_phys virt_to_phys
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
>  	return __virt_to_phys((unsigned long)(x));
>  }
> +#else
> +#define virt_to_phys debug_virt_to_phys
> +#endif
>  
>  #define phys_to_virt phys_to_virt
>  static inline void *phys_to_virt(phys_addr_t x)
> diff --git a/include/asm-generic/memory_model.h b/include/asm-generic/memory_model.h
> index 5148150cc80b..426085757258 100644
> --- a/include/asm-generic/memory_model.h
> +++ b/include/asm-generic/memory_model.h
> @@ -80,6 +80,10 @@
>  #define page_to_pfn __page_to_pfn
>  #define pfn_to_page __pfn_to_page
>  
> +#ifdef CONFIG_DEBUG_VM
> +unsigned long debug_virt_to_phys(volatile void *address);
> +#endif /* CONFIG_DEBUG_VM */
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif
> diff --git a/mm/debug.c b/mm/debug.c
> index 9feb699c5d25..72b2ca9b11f4 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -161,4 +161,19 @@ void dump_mm(const struct mm_struct *mm)
>  	);
>  }
>  
> +#include <asm/memory.h>
> +#include <linux/mm.h>
> +
> +unsigned long debug_virt_to_phys(volatile void *address)
> +{
> +	BUG_ON(is_vmalloc_addr((const void *)address));
> +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_UNICORE32) || \
> +	defined(CONFIG_MICROBLAZE)
> +	return __virt_to_phys(address);
> +#else
> +	return virt_to_phys(address);
> +#endif
> +}
> +EXPORT_SYMBOL(debug_virt_to_phys);
> +
>  #endif		/* CONFIG_DEBUG_VM */
> 

is_vmalloc_addr is necessary but not sufficient. This misses
cases like module addresses. The x86 version (CONFIG_DEBUG_VIRTUAL)
bounds checks against the known linear map to catch all cases.
I'm for a generic approach to this if it can catch all cases
that an architecture specific version would catch.

Thanks,
Laura

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] mm: Add debug_virt_to_phys()
  2016-11-14 18:45 ` Laura Abbott
@ 2016-11-14 19:23   ` Florian Fainelli
  0 siblings, 0 replies; 8+ messages in thread
From: Florian Fainelli @ 2016-11-14 19:23 UTC (permalink / raw)
  To: Laura Abbott, linux-kernel
  Cc: Russell King, Catalin Marinas, Will Deacon, Arnd Bergmann,
	Nicolas Pitre, Chris Brandt, Pratyush Anand, Ard Biesheuvel,
	Mark Rutland, James Morse, Neeraj Upadhyay, Andrew Morton,
	Vlastimil Babka, Michal Hocko, Kirill A. Shutemov,
	Jerome Marchand, Konstantin Khlebnikov, Joonsoo Kim,
	moderated list:ARM PORT,
	open list:GENERIC INCLUDE/ASM HEADER FILES,
	open list:MEMORY MANAGEMENT

On 11/14/2016 10:45 AM, Laura Abbott wrote:
> On 11/11/2016 04:44 PM, Florian Fainelli wrote:
>> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
>> debug_virt_to_phys() which helps catch vmalloc space addresses being
>> passed. This is helpful in debugging bogus drivers that just assume
>> linear mappings all over the place.
>>
>> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
>> __virt_to_phys() as being the functional implementation of the address
>> translation, so we special case the debug stub to call into
>> __virt_to_phys directly.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  arch/arm/include/asm/memory.h      |  4 ++++
>>  arch/arm64/include/asm/memory.h    |  4 ++++
>>  include/asm-generic/memory_model.h |  4 ++++
>>  mm/debug.c                         | 15 +++++++++++++++
>>  4 files changed, 27 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
>> index 76cbd9c674df..448dec9b8b00 100644
>> --- a/arch/arm/include/asm/memory.h
>> +++ b/arch/arm/include/asm/memory.h
>> @@ -260,11 +260,15 @@ static inline unsigned long __phys_to_virt(phys_addr_t x)
>>   * translation for translating DMA addresses.  Use the driver
>>   * DMA support - see dma-mapping.h.
>>   */
>> +#ifndef CONFIG_DEBUG_VM
>>  #define virt_to_phys virt_to_phys
>>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>>  {
>>  	return __virt_to_phys((unsigned long)(x));
>>  }
>> +#else
>> +#define virt_to_phys debug_virt_to_phys
>> +#endif
>>  
>>  #define phys_to_virt phys_to_virt
>>  static inline void *phys_to_virt(phys_addr_t x)
>> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
>> index b71086d25195..c9e436b28523 100644
>> --- a/arch/arm64/include/asm/memory.h
>> +++ b/arch/arm64/include/asm/memory.h
>> @@ -186,11 +186,15 @@ extern u64			kimage_voffset;
>>   * translation for translating DMA addresses.  Use the driver
>>   * DMA support - see dma-mapping.h.
>>   */
>> +#ifndef CONFIG_DEBUG_VM
>>  #define virt_to_phys virt_to_phys
>>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>>  {
>>  	return __virt_to_phys((unsigned long)(x));
>>  }
>> +#else
>> +#define virt_to_phys debug_virt_to_phys
>> +#endif
>>  
>>  #define phys_to_virt phys_to_virt
>>  static inline void *phys_to_virt(phys_addr_t x)
>> diff --git a/include/asm-generic/memory_model.h b/include/asm-generic/memory_model.h
>> index 5148150cc80b..426085757258 100644
>> --- a/include/asm-generic/memory_model.h
>> +++ b/include/asm-generic/memory_model.h
>> @@ -80,6 +80,10 @@
>>  #define page_to_pfn __page_to_pfn
>>  #define pfn_to_page __pfn_to_page
>>  
>> +#ifdef CONFIG_DEBUG_VM
>> +unsigned long debug_virt_to_phys(volatile void *address);
>> +#endif /* CONFIG_DEBUG_VM */
>> +
>>  #endif /* __ASSEMBLY__ */
>>  
>>  #endif
>> diff --git a/mm/debug.c b/mm/debug.c
>> index 9feb699c5d25..72b2ca9b11f4 100644
>> --- a/mm/debug.c
>> +++ b/mm/debug.c
>> @@ -161,4 +161,19 @@ void dump_mm(const struct mm_struct *mm)
>>  	);
>>  }
>>  
>> +#include <asm/memory.h>
>> +#include <linux/mm.h>
>> +
>> +unsigned long debug_virt_to_phys(volatile void *address)
>> +{
>> +	BUG_ON(is_vmalloc_addr((const void *)address));
>> +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_UNICORE32) || \
>> +	defined(CONFIG_MICROBLAZE)
>> +	return __virt_to_phys(address);
>> +#else
>> +	return virt_to_phys(address);
>> +#endif
>> +}
>> +EXPORT_SYMBOL(debug_virt_to_phys);
>> +
>>  #endif		/* CONFIG_DEBUG_VM */
>>
> 
> is_vmalloc_addr is necessary but not sufficient. This misses
> cases like module addresses.

Indeed, thanks.

> The x86 version (CONFIG_DEBUG_VIRTUAL)
> bounds checks against the known linear map to catch all cases.
> I'm for a generic approach to this if it can catch all cases
> that an architecture specific version would catch.

For one, my patch causes an early BUG to occur on ARM64 during
arch/arm64/kernel/setup.c::setup_arch when we call
cpu_uninstall_idmap(). I suspect there could be a bunch of little checks
like these where we'd have to have an architecture specific "is this
physical/virtual address valid" that may make a generic implementation
hard to come up with.
-- 
Florian

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-11-14 19:23 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-12  0:44 [PATCH RFC] mm: Add debug_virt_to_phys() Florian Fainelli
2016-11-12  1:49 ` Nicolas Pitre
2016-11-12  3:39   ` Florian Fainelli
2016-11-12 16:32     ` Nicolas Pitre
2016-11-12  5:43 ` Will Deacon
2016-11-12 19:29   ` Florian Fainelli
2016-11-14 18:45 ` Laura Abbott
2016-11-14 19:23   ` Florian Fainelli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).