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.
> 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);
>> +
