Le 04/07/2022 à 09:45, Aneesh Kumar K V a écrit : > On 7/4/22 12:43 PM, Christophe Leroy wrote: >> >> >> Le 04/07/2022 à 08:39, Aneesh Kumar K.V a écrit : >>> Instead of high_memory use is_vmalloc_addr to validate that the address is >>> not in the vmalloc range. >> >> >> Do we really need even more extra checks, and a function that is not >> inlined anymore ? >> >> virt_addr_valid() used to be pretty simple. Some extra tests were added >> by commit ffa0b64e3be5 ("powerpc: Fix virt_addr_valid() for 64-bit >> Book3E & 32-bit") in order to work around some corner cases, and the >> commit message say they are temporary. >> >> virt_addr_valid() is there to check that an address is a valid linear >> mapping, not that an address IS NOT a vmalloc address. What will happen >> with your check if you pass an address that is from an ioremap done >> prior to the start of the vmalloc system ? >> > > I was expecting the io range to be handled by pfn_valid(). IS there a memory > layout > ascii diagram of book3e/64 like asm/book3s/64/radix.h:51 ? My goal with the > change was to make it more explicit what is it being validated.
Yes you are right it should be handled by pfn_valid(), just like the entire VMALLOC range indeed. But on PPC32 a valid pfn might hit above vmalloc space as well. You can find the new layout here : https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=c7b9ed7c34a9f5dbf8222d63e3e313cef9f3150b The only problem we have with pfn_valid() is for PPC32 because pfn_valid() also include highmem memory. That's the reason why we need to check that the address is below high_memory in addition. For everything else, pfn_valid() should be enough. For PPC64, we may want to add a verification that we are in the 0xc.... range, because of the way __pa/__va work. On PPC32 that's not needed. > >> WIth the series I send last week to add KASAN to book3e/64, we now have >> VMALLOC above PAGE_OFFSET on all platforms so we should be able to come >> back to the original virt_addr_valid(), based exclusively on pfn_valid() >> for PPC64, and pfn_valid() && high_memory for PPC32 (Or maybe only for >> PPC32 having HIGHMEM) >> >> >> Christophe >> >> >>> >>> Cc: Kefeng Wang <wangkefeng.w...@huawei.com> >>> Cc: Christophe Leroy <christophe.le...@csgroup.eu> >>> Signed-off-by: Aneesh Kumar K.V <aneesh.ku...@linux.ibm.com> >>> - >>> --- >>> arch/powerpc/include/asm/page.h | 10 ++++------ >>> arch/powerpc/mm/mem.c | 11 +++++++++++ >>> 2 files changed, 15 insertions(+), 6 deletions(-) >>> >>> diff --git a/arch/powerpc/include/asm/page.h >>> b/arch/powerpc/include/asm/page.h >>> index e5f75c70eda8..977835570db3 100644 >>> --- a/arch/powerpc/include/asm/page.h >>> +++ b/arch/powerpc/include/asm/page.h >>> @@ -131,12 +131,10 @@ static inline bool pfn_valid(unsigned long pfn) >>> #define virt_to_pfn(kaddr) (__pa(kaddr) >> PAGE_SHIFT) >>> #define virt_to_page(kaddr) pfn_to_page(virt_to_pfn(kaddr)) >>> #define pfn_to_kaddr(pfn) __va((pfn) << PAGE_SHIFT) >>> - >>> -#define virt_addr_valid(vaddr) ({ >>> \ >>> - unsigned long _addr = (unsigned long)vaddr; \ >>> - _addr >= PAGE_OFFSET && _addr < (unsigned long)high_memory && \ >>> - pfn_valid(virt_to_pfn(_addr)); \ >>> -}) >>> +#ifndef __ASSEMBLY__ >>> +extern bool __virt_addr_valid(unsigned long kaddr); >>> +#endif >>> +#define virt_addr_valid(kaddr) __virt_addr_valid((unsigned long) >>> (kaddr)) >>> >>> /* >>> * On Book-E parts we need __va to parse the device tree and we can't >>> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c >>> index 7b0d286bf9ba..622f8bac808b 100644 >>> --- a/arch/powerpc/mm/mem.c >>> +++ b/arch/powerpc/mm/mem.c >>> @@ -406,3 +406,14 @@ int devmem_is_allowed(unsigned long pfn) >>> * the EHEA driver. Drop this when drivers/net/ethernet/ibm/ehea is >>> removed. >>> */ >>> EXPORT_SYMBOL_GPL(walk_system_ram_range); >>> + >>> +bool __virt_addr_valid(unsigned long kaddr) >>> +{ >>> + if (kaddr < PAGE_OFFSET) >>> + return false; >>> + if (is_vmalloc_addr((void *) kaddr)) >>> + return false; >>> + return pfn_valid(virt_to_pfn(kaddr)); >>> +} >>> +EXPORT_SYMBOL(__virt_addr_valid); >>> + >