Re: [PATCH v4 1/2] Revert "powerpc: Set max_mapnr correctly"

From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Michael Ellerman <mpe@ellerman.id.au>,
	Kefeng Wang <wangkefeng.wang@huawei.com>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "npiggin@gmail.com" <npiggin@gmail.com>,
	"songyuanzheng@huawei.com" <songyuanzheng@huawei.com>
Subject: Re: [PATCH v4 1/2] Revert "powerpc: Set max_mapnr correctly"
Date: Mon, 28 Mar 2022 10:59:24 +0000	[thread overview]
Message-ID: <34355f36-1122-9c22-e717-73a957a31a3e@csgroup.eu> (raw)
In-Reply-To: <87sfr2fjms.fsf@mpe.ellerman.id.au>

Le 28/03/2022 à 12:37, Michael Ellerman a écrit :
> Kefeng Wang <wangkefeng.wang@huawei.com> writes:
>> Hi maintainers，
>>
>> I saw the patches has been reviewed[1], could they be merged?
> 
> Maybe I'm just misreading the change log, but it seems wrong that we
> need to add extra checks. pfn_valid() shouldn't return true for vmalloc
> addresses in the first place, shouldn't we fix that instead? Who knows
> what else that might be broken because of that.
> 

pfn_valid() doesn't take an address but a PFN

If you have 1Gbyte of memory you have 256k PFNs.

In a generic config the kernel will map 768 Mbytes of mémory (From 
0xc0000000 to 0xe0000000) and will use 0xf0000000-0xffffffff for 
everything else including vmalloc.

If you take a page above that 768 Mbytes limit, and tries to linarly 
convert it's PFN to a va, you'll hip vmalloc space. Anyway that PFN is 
valid.

So the check really needs to be done in virt_addr_valid().

There is another thing however that would be worth fixing (in another 
patch): that's the virt_to_pfn() in PPC64:

#define virt_to_pfn(kaddr)	(__pa(kaddr) >> PAGE_SHIFT)

#define __pa(x)								\
({									\
	VIRTUAL_BUG_ON((unsigned long)(x) < PAGE_OFFSET);		\
	(unsigned long)(x) & 0x0fffffffffffffffUL;			\
})

So 0xc000000000000000 and 0xd000000000000000 have the same PFN. That's 
wrong.

Christophe