[PATCH v2 1/3] arch/x86: Drop own definition of pgd,p4d_leaf

[PATCH v2 1/3] arch/x86: Drop own definition of pgd,p4d_leaf

[Christophe Leroy]

From: Oscar Salvador <osalvador@suse.de>

We provide generic definitions of pXd_leaf in pgtable.h when the arch
do not define their own, where the generic pXd_leaf always return false.

Although x86 defines {pgd,p4d}_leaf, they end up being a no-op, so drop them
and make them fallback to the generic one.

Signed-off-by: Oscar Salvador <osalvador@suse.de>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/x86/include/asm/pgtable.h | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 65b8e5bb902c..772f778bac06 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -252,13 +252,6 @@ static inline unsigned long pgd_pfn(pgd_t pgd)
 	return (pgd_val(pgd) & PTE_PFN_MASK) >> PAGE_SHIFT;
 }
 
-#define p4d_leaf p4d_leaf
-static inline bool p4d_leaf(p4d_t p4d)
-{
-	/* No 512 GiB pages yet */
-	return 0;
-}
-
 #define pte_page(pte)	pfn_to_page(pte_pfn(pte))
 
 #define pmd_leaf pmd_leaf
@@ -1396,9 +1389,6 @@ static inline bool pgdp_maps_userspace(void *__ptr)
 	return (((ptr & ~PAGE_MASK) / sizeof(pgd_t)) < PGD_KERNEL_START);
 }
 
-#define pgd_leaf	pgd_leaf
-static inline bool pgd_leaf(pgd_t pgd) { return false; }
-
 #ifdef CONFIG_MITIGATION_PAGE_TABLE_ISOLATION
 /*
  * All top-level MITIGATION_PAGE_TABLE_ISOLATION page tables are order-1 pages
-- 
2.44.0

[PATCH v2 2/3] mm: Remove pud_user() from asm-generic/pgtable-nopmd.h

[Christophe Leroy]

Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related
issues") added pud_user() in include/asm-generic/pgtable-nopmd.h

But pud_user() only exists on ARM64 and RISCV and is not expected
by any part of MM.

Add the missing definition in arch/riscv/include/asm/pgtable-32.h
and remove it from asm-generic/pgtable-nopmd.h

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/riscv/include/asm/pgtable-32.h | 5 +++++
 include/asm-generic/pgtable-nopmd.h | 1 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/pgtable-32.h b/arch/riscv/include/asm/pgtable-32.h
index 00f3369570a8..37878ef37466 100644
--- a/arch/riscv/include/asm/pgtable-32.h
+++ b/arch/riscv/include/asm/pgtable-32.h
@@ -36,4 +36,9 @@
 static const __maybe_unused int pgtable_l4_enabled;
 static const __maybe_unused int pgtable_l5_enabled;
 
+static inline int pud_user(pud_t pud)
+{
+	return 0;
+}
+
 #endif /* _ASM_RISCV_PGTABLE_32_H */
diff --git a/include/asm-generic/pgtable-nopmd.h b/include/asm-generic/pgtable-nopmd.h
index 8ffd64e7a24c..b01349a312fa 100644
--- a/include/asm-generic/pgtable-nopmd.h
+++ b/include/asm-generic/pgtable-nopmd.h
@@ -30,7 +30,6 @@ typedef struct { pud_t pud; } pmd_t;
 static inline int pud_none(pud_t pud)		{ return 0; }
 static inline int pud_bad(pud_t pud)		{ return 0; }
 static inline int pud_present(pud_t pud)	{ return 1; }
-static inline int pud_user(pud_t pud)		{ return 0; }
 static inline int pud_leaf(pud_t pud)		{ return 0; }
 static inline void pud_clear(pud_t *pud)	{ }
 #define pmd_ERROR(pmd)				(pud_ERROR((pmd).pud))
-- 
2.44.0

Re: [PATCH v2 2/3] mm: Remove pud_user() from asm-generic/pgtable-nopmd.h

[Peter Xu]

On Wed, Jul 10, 2024 at 09:51:21AM +0200, Christophe Leroy wrote:
> Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related
> issues") added pud_user() in include/asm-generic/pgtable-nopmd.h
> 
> But pud_user() only exists on ARM64 and RISCV and is not expected
> by any part of MM.
> 
> Add the missing definition in arch/riscv/include/asm/pgtable-32.h
> and remove it from asm-generic/pgtable-nopmd.h
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>

I assume you double checked the riscv cross-builds on both 32/64. It looks
correct:

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Re: [PATCH v2 2/3] mm: Remove pud_user() from asm-generic/pgtable-nopmd.h

[Oscar Salvador]

On Wed, Jul 10, 2024 at 09:51:21AM +0200, Christophe Leroy wrote:
> Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related
> issues") added pud_user() in include/asm-generic/pgtable-nopmd.h
> 
> But pud_user() only exists on ARM64 and RISCV and is not expected
> by any part of MM.
> 
> Add the missing definition in arch/riscv/include/asm/pgtable-32.h
> and remove it from asm-generic/pgtable-nopmd.h
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>

Reviewed-by: Oscar Salvador <osalvador@suse.de>


-- 
Oscar Salvador
SUSE Labs

[PATCH v2 3/3] mm: Add p{g/4}d_leaf() in asm-generic/pgtable-nop{4/u}d.h

[Christophe Leroy]

Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related
issues") added pud_leaf() in include/asm-generic/pgtable-nopmd.h

Do the same for p4d_leaf() and pgd_leaf() to avoid getting them
erroneously defined by architectures that do not implement the
related page level.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: Added pXd_leaf macro as well in asm-generic/pgtable-nopXd.h to cohabit with the fallback
---
 include/asm-generic/pgtable-nop4d.h | 2 ++
 include/asm-generic/pgtable-nopmd.h | 1 +
 include/asm-generic/pgtable-nopud.h | 2 ++
 3 files changed, 5 insertions(+)

diff --git a/include/asm-generic/pgtable-nop4d.h b/include/asm-generic/pgtable-nop4d.h
index 03b7dae47dd4..ed7ba008469f 100644
--- a/include/asm-generic/pgtable-nop4d.h
+++ b/include/asm-generic/pgtable-nop4d.h
@@ -21,6 +21,8 @@ typedef struct { pgd_t pgd; } p4d_t;
 static inline int pgd_none(pgd_t pgd)		{ return 0; }
 static inline int pgd_bad(pgd_t pgd)		{ return 0; }
 static inline int pgd_present(pgd_t pgd)	{ return 1; }
+static inline int pgd_leaf(pgd_t pgd)		{ return 0; }
+#define pgd_leaf pgd_leaf
 static inline void pgd_clear(pgd_t *pgd)	{ }
 #define p4d_ERROR(p4d)				(pgd_ERROR((p4d).pgd))
 
diff --git a/include/asm-generic/pgtable-nopmd.h b/include/asm-generic/pgtable-nopmd.h
index b01349a312fa..e178ace2e23e 100644
--- a/include/asm-generic/pgtable-nopmd.h
+++ b/include/asm-generic/pgtable-nopmd.h
@@ -31,6 +31,7 @@ static inline int pud_none(pud_t pud)		{ return 0; }
 static inline int pud_bad(pud_t pud)		{ return 0; }
 static inline int pud_present(pud_t pud)	{ return 1; }
 static inline int pud_leaf(pud_t pud)		{ return 0; }
+#define pud_leaf pud_leaf
 static inline void pud_clear(pud_t *pud)	{ }
 #define pmd_ERROR(pmd)				(pud_ERROR((pmd).pud))
 
diff --git a/include/asm-generic/pgtable-nopud.h b/include/asm-generic/pgtable-nopud.h
index eb70c6d7ceff..655dfebea91c 100644
--- a/include/asm-generic/pgtable-nopud.h
+++ b/include/asm-generic/pgtable-nopud.h
@@ -28,6 +28,8 @@ typedef struct { p4d_t p4d; } pud_t;
 static inline int p4d_none(p4d_t p4d)		{ return 0; }
 static inline int p4d_bad(p4d_t p4d)		{ return 0; }
 static inline int p4d_present(p4d_t p4d)	{ return 1; }
+static inline int p4d_leaf(p4d_t p4d)		{ return 0; }
+#define p4d_leaf p4d_leaf
 static inline void p4d_clear(p4d_t *p4d)	{ }
 #define pud_ERROR(pud)				(p4d_ERROR((pud).p4d))
 
-- 
2.44.0

Re: [PATCH v2 3/3] mm: Add p{g/4}d_leaf() in asm-generic/pgtable-nop{4/u}d.h

[Peter Xu]

On Wed, Jul 10, 2024 at 09:51:22AM +0200, Christophe Leroy wrote:
> Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related
> issues") added pud_leaf() in include/asm-generic/pgtable-nopmd.h
> 
> Do the same for p4d_leaf() and pgd_leaf() to avoid getting them
> erroneously defined by architectures that do not implement the
> related page level.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> v2: Added pXd_leaf macro as well in asm-generic/pgtable-nopXd.h to cohabit with the fallback
> ---

Thanks.  I'd drop the inline functions, but no strong opinions.

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Re: [PATCH v2 3/3] mm: Add p{g/4}d_leaf() in asm-generic/pgtable-nop{4/u}d.h

[LEROY Christophe]

Le 10/07/2024 à 16:46, Peter Xu a écrit :
> On Wed, Jul 10, 2024 at 09:51:22AM +0200, Christophe Leroy wrote:
>> Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related
>> issues") added pud_leaf() in include/asm-generic/pgtable-nopmd.h
>>
>> Do the same for p4d_leaf() and pgd_leaf() to avoid getting them
>> erroneously defined by architectures that do not implement the
>> related page level.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> v2: Added pXd_leaf macro as well in asm-generic/pgtable-nopXd.h to cohabit with the fallback
>> ---
> 
> Thanks.  I'd drop the inline functions, but no strong opinions.

Inline functions enable type checking.

With a macro you would be able to write pud_leaf(pgd) without the 
compiler noticing the mistake.

All other helpers in asm-generic/pgtable-nopXd.h are functions so from 
my point of view it makes sense to keep them as functions not macros.

> 
> Reviewed-by: Peter Xu <peterx@redhat.com>
> 

Thanks.

Re: [PATCH v2 3/3] mm: Add p{g/4}d_leaf() in asm-generic/pgtable-nop{4/u}d.h

[Peter Xu]

On Wed, Jul 10, 2024 at 02:54:36PM +0000, LEROY Christophe wrote:
> 
> 
> Le 10/07/2024 à 16:46, Peter Xu a écrit :
> > On Wed, Jul 10, 2024 at 09:51:22AM +0200, Christophe Leroy wrote:
> >> Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related
> >> issues") added pud_leaf() in include/asm-generic/pgtable-nopmd.h
> >>
> >> Do the same for p4d_leaf() and pgd_leaf() to avoid getting them
> >> erroneously defined by architectures that do not implement the
> >> related page level.
> >>
> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> >> ---
> >> v2: Added pXd_leaf macro as well in asm-generic/pgtable-nopXd.h to cohabit with the fallback
> >> ---
> > 
> > Thanks.  I'd drop the inline functions, but no strong opinions.
> 
> Inline functions enable type checking.
> 
> With a macro you would be able to write pud_leaf(pgd) without the 
> compiler noticing the mistake.
> 
> All other helpers in asm-generic/pgtable-nopXd.h are functions so from 
> my point of view it makes sense to keep them as functions not macros.

Whoever fallbacks to the pgtable.h pxx_leaf() will still use macros and
lose the type check again.  I'd rather rely on cross-arch builds and most
of real *_leaf() users will always detect a type mismatch.

Totally no big deal, and I agree keeping them match nopxd.h rules makes
sense.

Thanks,

-- 
Peter Xu

Re: [PATCH v2 3/3] mm: Add p{g/4}d_leaf() in asm-generic/pgtable-nop{4/u}d.h

[LEROY Christophe]

Le 10/07/2024 à 20:41, Peter Xu a écrit :
> On Wed, Jul 10, 2024 at 02:54:36PM +0000, LEROY Christophe wrote:
>>
>>
>> Le 10/07/2024 à 16:46, Peter Xu a écrit :
>>> On Wed, Jul 10, 2024 at 09:51:22AM +0200, Christophe Leroy wrote:
>>>> Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related
>>>> issues") added pud_leaf() in include/asm-generic/pgtable-nopmd.h
>>>>
>>>> Do the same for p4d_leaf() and pgd_leaf() to avoid getting them
>>>> erroneously defined by architectures that do not implement the
>>>> related page level.
>>>>
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>>> ---
>>>> v2: Added pXd_leaf macro as well in asm-generic/pgtable-nopXd.h to cohabit with the fallback
>>>> ---
>>>
>>> Thanks.  I'd drop the inline functions, but no strong opinions.
>>
>> Inline functions enable type checking.
>>
>> With a macro you would be able to write pud_leaf(pgd) without the
>> compiler noticing the mistake.
>>
>> All other helpers in asm-generic/pgtable-nopXd.h are functions so from
>> my point of view it makes sense to keep them as functions not macros.
> 
> Whoever fallbacks to the pgtable.h pxx_leaf() will still use macros and
> lose the type check again.  I'd rather rely on cross-arch builds and most
> of real *_leaf() users will always detect a type mismatch.
> 
> Totally no big deal, and I agree keeping them match nopxd.h rules makes
> sense.
> 

Surprisingly, having both a macro and a static inline simultaneously 
defining pud_leaf() on loongarch was not a problem but as soon as there 
are two macros the compiler cries.

I will wait a bit more to see if robots report anything else then I'll 
send an update fix.

Christophe

Re: [PATCH v2 3/3] mm: Add p{g/4}d_leaf() in asm-generic/pgtable-nop{4/u}d.h

[Peter Xu]

On Thu, Jul 11, 2024 at 01:40:17PM +0000, LEROY Christophe wrote:
> I will wait a bit more to see if robots report anything else then I'll 
> send an update fix.

In case helpful, I normally use this small bunch of scripts to test
cross-compiles (and I keep adding new configs that can fail any of my
trees):

https://gitlab.com/peterx/lkb-harness/

Just FYI.. I think different people use different set of tools.

-- 
Peter Xu

Re: [PATCH v2 1/3] arch/x86: Drop own definition of pgd,p4d_leaf

[Peter Xu]

On Wed, Jul 10, 2024 at 09:51:20AM +0200, Christophe Leroy wrote:
> From: Oscar Salvador <osalvador@suse.de>
> 
> We provide generic definitions of pXd_leaf in pgtable.h when the arch
> do not define their own, where the generic pXd_leaf always return false.
> 
> Although x86 defines {pgd,p4d}_leaf, they end up being a no-op, so drop them
> and make them fallback to the generic one.
> 
> Signed-off-by: Oscar Salvador <osalvador@suse.de>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu