From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S938732AbcKLDjW (ORCPT ); Fri, 11 Nov 2016 22:39:22 -0500 Received: from mail-yb0-f196.google.com ([209.85.213.196]:36021 "EHLO mail-yb0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935102AbcKLDjU (ORCPT ); Fri, 11 Nov 2016 22:39:20 -0500 Subject: Re: [PATCH RFC] mm: Add debug_virt_to_phys() To: Nicolas Pitre References: <20161112004449.30566-1-f.fainelli@gmail.com> Cc: linux-kernel@vger.kernel.org, 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" From: Florian Fainelli Message-ID: <55bd0bb5-c11c-12bc-7d73-520ae3901f03@gmail.com> Date: Fri, 11 Nov 2016 19:39:14 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> --- >> 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: [PATCH RFC] mm: Add debug_virt_to_phys() Date: Fri, 11 Nov 2016 19:39:14 -0800 Message-ID: <55bd0bb5-c11c-12bc-7d73-520ae3901f03@gmail.com> References: <20161112004449.30566-1-f.fainelli@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Nicolas Pitre Cc: linux-kernel@vger.kernel.org, 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 P List-Id: linux-arch.vger.kernel.org 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 >> --- >> 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb0-f196.google.com ([209.85.213.196]:36021 "EHLO mail-yb0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935102AbcKLDjU (ORCPT ); Fri, 11 Nov 2016 22:39:20 -0500 Subject: Re: [PATCH RFC] mm: Add debug_virt_to_phys() References: <20161112004449.30566-1-f.fainelli@gmail.com> From: Florian Fainelli Message-ID: <55bd0bb5-c11c-12bc-7d73-520ae3901f03@gmail.com> Date: Fri, 11 Nov 2016 19:39:14 -0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-arch-owner@vger.kernel.org List-ID: To: Nicolas Pitre Cc: linux-kernel@vger.kernel.org, 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" Message-ID: <20161112033914.wYgGa05F_tBdt0u_N5KjglbBOYQb8iiPovQD-la_AsY@z> 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 >> --- >> 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw0-f199.google.com (mail-yw0-f199.google.com [209.85.161.199]) by kanga.kvack.org (Postfix) with ESMTP id 10B2F28029E for ; Fri, 11 Nov 2016 22:39:21 -0500 (EST) Received: by mail-yw0-f199.google.com with SMTP id t11so92949652ywe.3 for ; Fri, 11 Nov 2016 19:39:21 -0800 (PST) Received: from mail-yb0-x243.google.com (mail-yb0-x243.google.com. [2607:f8b0:4002:c09::243]) by mx.google.com with ESMTPS id r71si3466076ywg.155.2016.11.11.19.39.20 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 11 Nov 2016 19:39:20 -0800 (PST) Received: by mail-yb0-x243.google.com with SMTP id d128so945897ybh.3 for ; Fri, 11 Nov 2016 19:39:20 -0800 (PST) Subject: Re: [PATCH RFC] mm: Add debug_virt_to_phys() References: <20161112004449.30566-1-f.fainelli@gmail.com> From: Florian Fainelli Message-ID: <55bd0bb5-c11c-12bc-7d73-520ae3901f03@gmail.com> Date: Fri, 11 Nov 2016 19:39:14 -0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: owner-linux-mm@kvack.org List-ID: To: Nicolas Pitre Cc: linux-kernel@vger.kernel.org, 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 a 17:49, Nicolas Pitre a ecrit : > 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 >> --- >> 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 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: f.fainelli@gmail.com (Florian Fainelli) Date: Fri, 11 Nov 2016 19:39:14 -0800 Subject: [PATCH RFC] mm: Add debug_virt_to_phys() In-Reply-To: References: <20161112004449.30566-1-f.fainelli@gmail.com> Message-ID: <55bd0bb5-c11c-12bc-7d73-520ae3901f03@gmail.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 >> --- >> 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