From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935839AbcKNTXf (ORCPT ); Mon, 14 Nov 2016 14:23:35 -0500 Received: from mail-pg0-f68.google.com ([74.125.83.68]:33964 "EHLO mail-pg0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934223AbcKNTXb (ORCPT ); Mon, 14 Nov 2016 14:23:31 -0500 Subject: Re: [PATCH RFC] mm: Add debug_virt_to_phys() To: Laura Abbott , linux-kernel@vger.kernel.org References: <20161112004449.30566-1-f.fainelli@gmail.com> 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" From: Florian Fainelli Message-ID: <40e2260c-9b09-0c0e-5b0f-0d61fdb8ba1e@gmail.com> Date: Mon, 14 Nov 2016 11:23:27 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> --- >> 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 >> +#include >> + >> +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