Re: [PATCH v2 1/2] powerpc: Fix virt_addr_valid() check

On 2022/1/20 15:31, Christophe Leroy wrote:

Le 19/01/2022 à 02:15, Kefeng Wang a écrit :

quoted

On 2022/1/11 14:04, Christophe Leroy wrote:

quoted

Le 11/01/2022 à 05:37, Nicholas Piggin a écrit :

quoted

Excerpts from Kefeng Wang's message of January 8, 2022 9:58 pm:

quoted

Hi PPC maintainers， ping..

Hmm. I might have confused myself about this. I'm going back and
trying to work out what I was thinking when I suggested it. This
works on 64e because vmalloc space is below the kernel linear map,
right?

On 64s it is the other way around and it is still possible to enable
flatmem on 64s. Altough we might just not hit the problem there because
__pa() will not mask away the vmalloc offset for 64s so it will still
return something that's outside the pfn_valid range for flatmem. That's
very subtle though.

That's the way it works on PPC32 at least, so for me it's not chocking
to have it work the same way on PPC64s.

The main issue here is the way __pa() works. On PPC32 __pa = va -
PAGE_OFFSET, so it works correctly for any address.
On PPC64, __pa() works by masking out the 2 top bits instead of
substracting PAGE_OFFSET, so the test must add a verification that we
really have the 2 top bits set at first. This is what (addr >=
PAGE_OFFSET) does. Once this first test is done, we can perfectly rely
on pfn_valid() just like PPC32, I see absolutely no point in an
additionnal test checking the addr is below KERN_VIRT_START.

Hi Christophe and Nicholas， for ppc32， I think we need check the upper
limit,

Why ? Have you experimented any problem at all on PPC32 with the way it
is done at the moment ?

I don't think we have to change PPC32 at all unless we have a real
reason to do it.

yes, I missed this commit in old kernel(lts5.10), you have fixed the 
upper limit.

commit 602946ec2f90d5bd965857753880db29d2d9a1e9
Author: Christophe Leroy [off-list ref]
Date:   Tue Oct 12 12:40:37 2021 +0200

     powerpc: Set max_mapnr correctly

quoted

eg,  addr >= PAGE_OFFSET && addr < high_memory

Isn't it exactly what pfn_valid() already do today ?
Why change that at all ?

Christophe

quoted

arch/powerpc/mm/mem.c:  high_memory = (void *) __va(max_low_pfn *
PAGE_SIZE);

for ppc32 max_low_pfn is the upper low memory pfn,  and For ppc64,
high_memory is

the max memory pfn, it looks good too, correct me if I'm wrong, if the
above check

is ok, I will send a new v3,  thanks.

quoted

quoted

The checks added to __pa actually don't prevent vmalloc memory from
being passed to it either on 64s, only a more basic test.

That's correct. It is the role of pfn_valid() to check that.

Christophe

quoted

I think 64s wants (addr >= PAGE_OFFSET && addr < KERN_VIRT_START) as
the condition.  Could possibly add that check to __pa as well to
catch vmalloc addresses.

Thanks,
Nick