[PATCH] x86/mm: Define PTRS_PER_PMD for assembly code too

Ingo Molnar posted 1 patch 11 months, 1 week ago
There is a newer version of this series
arch/x86/include/asm/pgtable-2level_types.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH] x86/mm: Define PTRS_PER_PMD for assembly code too
Posted by Ingo Molnar 11 months, 1 week ago

* Ingo Molnar <mingo@kernel.org> wrote:

> Separating out the assembler-compatible defines from the types 
> headers appears to be a bigger patch, since it's all mixed in with C 
> syntax:
> 
> <=-----------------------------------===============================
> typedef struct { pud_t pud; } pmd_t;
> 
> #define PMD_SHIFT       PUD_SHIFT
> #define PTRS_PER_PMD    1
> #define PMD_SIZE        (1UL << PMD_SHIFT)
> #define PMD_MASK        (~(PMD_SIZE-1))
> 
> /*
>  * The "pud_xxx()" functions here are trivial for a folded two-level
>  * setup: the pmd is never bad, and a pmd always exists (as it's folded
>  * into the pud entry)
>  */
> 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; }
> ================================================================>
> 
> In any case I've removed the commit for the time being until this all 
> is cleared up.

So there's a simple solution: define it on i386 too, via the patch 
below. It appears the double-definition doesn't create any warnings, on 
GCC at least.

But if it's an issue, we could do something like this in 
<asm-generic/pgtable-nopmd.h>:

 #if defined(PTRS_PER_PMD) && (PTRS_PER_PMD != 1)
 # error "mm: Wait a minute, that's a super confusing pagetable setup ..."
 #endif

?

Thanks,

	Ingo

=========================>
From: Ingo Molnar <mingo@kernel.org>
Date: Thu, 6 Mar 2025 22:53:49 +0100
Subject: [PATCH] x86/mm: Define PTRS_PER_PMD for assembly code too

Andy reported the following build warning from head_32.S:

  In file included from arch/x86/kernel/head_32.S:29:
  arch/x86/include/asm/pgtable_32.h:59:5: error: "PTRS_PER_PMD" is not defined, evaluates to 0 [-Werror=undef]
       59 | #if PTRS_PER_PMD > 1

The reason is that on 2-level i386 paging the folded in PMD's
PTRS_PER_PMD constant is not defined in assembly headers,
only in generic MM C headers.

Instead of trying to fish out the definition from the generic
headers, just define it - it even has a comment for it already...

Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/pgtable-2level_types.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/pgtable-2level_types.h b/arch/x86/include/asm/pgtable-2level_types.h
index 7f6ccff0ba72..4a12c276b181 100644
--- a/arch/x86/include/asm/pgtable-2level_types.h
+++ b/arch/x86/include/asm/pgtable-2level_types.h
@@ -23,17 +23,17 @@ typedef union {
 #define ARCH_PAGE_TABLE_SYNC_MASK	PGTBL_PMD_MODIFIED
 
 /*
- * traditional i386 two-level paging structure:
+ * Traditional i386 two-level paging structure:
  */
 
 #define PGDIR_SHIFT	22
 #define PTRS_PER_PGD	1024
 
-
 /*
- * the i386 is two-level, so we don't really have any
- * PMD directory physically.
+ * The i386 is two-level, so we don't really have any
+ * PMD directory physically:
  */
+#define PTRS_PER_PMD	1
 
 #define PTRS_PER_PTE	1024
Re: [PATCH] x86/mm: Define PTRS_PER_PMD for assembly code too
Posted by Andy Shevchenko 11 months, 1 week ago
On Thu, Mar 06, 2025 at 11:00:16PM +0100, Ingo Molnar wrote:
> * Ingo Molnar <mingo@kernel.org> wrote:
> 
> > Separating out the assembler-compatible defines from the types 
> > headers appears to be a bigger patch, since it's all mixed in with C 
> > syntax:
> > 
> > <=-----------------------------------===============================
> > typedef struct { pud_t pud; } pmd_t;
> > 
> > #define PMD_SHIFT       PUD_SHIFT
> > #define PTRS_PER_PMD    1
> > #define PMD_SIZE        (1UL << PMD_SHIFT)
> > #define PMD_MASK        (~(PMD_SIZE-1))
> > 
> > /*
> >  * The "pud_xxx()" functions here are trivial for a folded two-level
> >  * setup: the pmd is never bad, and a pmd always exists (as it's folded
> >  * into the pud entry)
> >  */
> > 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; }
> > ================================================================>
> > 
> > In any case I've removed the commit for the time being until this all 
> > is cleared up.
> 
> So there's a simple solution: define it on i386 too, via the patch 
> below. It appears the double-definition doesn't create any warnings, on 
> GCC at least.

Fine by me as long as it gets fixed. Currently it prevents the WERROR=y
to be used along with `make W=1` for x86_32 by both compilers.

> But if it's an issue, we could do something like this in 
> <asm-generic/pgtable-nopmd.h>:
> 
>  #if defined(PTRS_PER_PMD) && (PTRS_PER_PMD != 1)
>  # error "mm: Wait a minute, that's a super confusing pagetable setup ..."
>  #endif
> 
> ?
> 
> Thanks,
> 
> 	Ingo
> 
> =========================>
> From: Ingo Molnar <mingo@kernel.org>
> Date: Thu, 6 Mar 2025 22:53:49 +0100
> Subject: [PATCH] x86/mm: Define PTRS_PER_PMD for assembly code too
> 
> Andy reported the following build warning from head_32.S:
> 
>   In file included from arch/x86/kernel/head_32.S:29:
>   arch/x86/include/asm/pgtable_32.h:59:5: error: "PTRS_PER_PMD" is not defined, evaluates to 0 [-Werror=undef]
>        59 | #if PTRS_PER_PMD > 1
> 
> The reason is that on 2-level i386 paging the folded in PMD's
> PTRS_PER_PMD constant is not defined in assembly headers,
> only in generic MM C headers.
> 
> Instead of trying to fish out the definition from the generic
> headers, just define it - it even has a comment for it already...
> 
> Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
>  arch/x86/include/asm/pgtable-2level_types.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/pgtable-2level_types.h b/arch/x86/include/asm/pgtable-2level_types.h
> index 7f6ccff0ba72..4a12c276b181 100644
> --- a/arch/x86/include/asm/pgtable-2level_types.h
> +++ b/arch/x86/include/asm/pgtable-2level_types.h
> @@ -23,17 +23,17 @@ typedef union {
>  #define ARCH_PAGE_TABLE_SYNC_MASK	PGTBL_PMD_MODIFIED
>  
>  /*
> - * traditional i386 two-level paging structure:
> + * Traditional i386 two-level paging structure:
>   */
>  
>  #define PGDIR_SHIFT	22
>  #define PTRS_PER_PGD	1024
>  
> -
>  /*
> - * the i386 is two-level, so we don't really have any
> - * PMD directory physically.
> + * The i386 is two-level, so we don't really have any
> + * PMD directory physically:
>   */
> +#define PTRS_PER_PMD	1

Should I give a try?

Okay, just

Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

for x86_32 with Clang 19.1.7 and GCC 14.2.0.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] x86/mm: Define PTRS_PER_PMD for assembly code too
Posted by Ingo Molnar 11 months, 1 week ago
* Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Okay, just
> 
> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> for x86_32 with Clang 19.1.7 and GCC 14.2.0.

Thanks!

	Ingo
[tip: x86/urgent] x86/mm: Define PTRS_PER_PMD for assembly code too
Posted by tip-bot2 for Ingo Molnar 11 months, 1 week ago
The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     6914f7e2e25fac9d1d2b62c208eaa5f2bf810fe9
Gitweb:        https://git.kernel.org/tip/6914f7e2e25fac9d1d2b62c208eaa5f2bf810fe9
Author:        Ingo Molnar <mingo@kernel.org>
AuthorDate:    Thu, 06 Mar 2025 23:00:16 +01:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Sat, 08 Mar 2025 00:09:09 +01:00

x86/mm: Define PTRS_PER_PMD for assembly code too

Andy reported the following build warning from head_32.S:

  In file included from arch/x86/kernel/head_32.S:29:
  arch/x86/include/asm/pgtable_32.h:59:5: error: "PTRS_PER_PMD" is not defined, evaluates to 0 [-Werror=undef]
       59 | #if PTRS_PER_PMD > 1

The reason is that on 2-level i386 paging the folded in PMD's
PTRS_PER_PMD constant is not defined in assembly headers,
only in generic MM C headers.

Instead of trying to fish out the definition from the generic
headers, just define it - it even has a comment for it already...

Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/r/Z8oa8AUVyi2HWfo9@gmail.com
---
 arch/x86/include/asm/pgtable-2level_types.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/pgtable-2level_types.h b/arch/x86/include/asm/pgtable-2level_types.h
index 7f6ccff..4a12c27 100644
--- a/arch/x86/include/asm/pgtable-2level_types.h
+++ b/arch/x86/include/asm/pgtable-2level_types.h
@@ -23,17 +23,17 @@ typedef union {
 #define ARCH_PAGE_TABLE_SYNC_MASK	PGTBL_PMD_MODIFIED
 
 /*
- * traditional i386 two-level paging structure:
+ * Traditional i386 two-level paging structure:
  */
 
 #define PGDIR_SHIFT	22
 #define PTRS_PER_PGD	1024
 
-
 /*
- * the i386 is two-level, so we don't really have any
- * PMD directory physically.
+ * The i386 is two-level, so we don't really have any
+ * PMD directory physically:
  */
+#define PTRS_PER_PMD	1
 
 #define PTRS_PER_PTE	1024