mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] x86/mm: Introduce helper for checking direct map 1G page support
@ 2026-09-02 19:47 Dave Hansen
  2026-09-03  0:34 ` Sohil Mehta
  0 siblings, 1 reply; 4+ messages in thread
From: Dave Hansen @ 2026-09-02 19:47 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dave Hansen, Andy Lutomirski, Borislav Petkov, H. Peter Anvin,
	Ingo Molnar, Peter Zijlstra, Thomas Gleixner, x86, Yeoreum Yun


From: Dave Hansen <dave.hansen@linux.intel.com>

There is an existing variable (direct_gbpages) that says whether the
kernel can and should use 1G pages in the direct map. It is driven
by a bunch of other machinery. At least:

 1. Hardware support for 1G pages
 2. Kconfig support for 1G direct mappings
 3. Kernel command line overrides

Most code just checks the 'direct_gbpages' variable itself.  But there
are cases where 1G mappings are compile-time disabled (via
X86_DIRECT_GBPAGES) and 'direct_gbpages' is always 0. Unfortunately,
that constraint is invisible to the compiler.

This opacity has been historically functionally harmless; it only
leaves a bit of dead code. But, there are plans to tigthen up the
compile-time checks around folded page table levels. The build will
break if that dead code appears reachable to the compiler. Making
the compile-time config visible to the compiler fixes the build.

Add a helper to replace 'direct_gbpages' checks. Check the Kconfig
option and base CPU support before looking at the variable.

This lets the compiler optimize things better, especially
collapse_pud_page() where most of the function can now be optimized
out when the PUD level is folded.

Notes:

Use boot_cpu_has() instead of cpu_feature_enabled(). There's no
required/disabled features for 1G pages themselves
(X86_DIRECT_GBPAGES is for kernel mappings only) and the
static_cpu_has() infrastructure is just gets in the compiler's way.

This makes 64-bit build marginally larger (20 bytes in one compile)
and 32-bit builds less marginally _smaller_ (~700 bytes).

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Tested-by: Yeoreum Yun <yeoreum.yun@arm.com>
Link: https://lore.kernel.org/all/20260902-dummy_ptxp3-v3-15-5d8f5b17c25c@arm.com/ [1]
---

 b/arch/x86/include/asm/pgtable.h     |   14 ++++++++++++++
 b/arch/x86/kernel/cpu/common.c       |    2 +-
 b/arch/x86/kernel/machine_kexec_64.c |    2 +-
 b/arch/x86/mm/init.c                 |    2 +-
 b/arch/x86/mm/pat/set_memory.c       |    4 ++--
 5 files changed, 19 insertions(+), 5 deletions(-)

diff -puN arch/x86/include/asm/pgtable.h~direct_gbpages-compiletime arch/x86/include/asm/pgtable.h
--- a/arch/x86/include/asm/pgtable.h~direct_gbpages-compiletime	2026-09-02 10:08:59.999169372 -0700
+++ b/arch/x86/include/asm/pgtable.h	2026-09-02 10:09:00.011170394 -0700
@@ -1163,6 +1163,20 @@ static inline int pgd_none(pgd_t pgd)
 #ifndef __ASSEMBLER__
 
 extern int direct_gbpages;
+static inline bool direct_gbpages_enabled(void)
+{
+	/* Check the direct map config option: */
+	if (!IS_ENABLED(CONFIG_X86_DIRECT_GBPAGES))
+		return false;
+
+	/* Check the CPU feature: */
+	if (!boot_cpu_has(X86_FEATURE_GBPAGES))
+		return false;
+
+	/* Check the command-line and early setup variable: */
+	return direct_gbpages;
+}
+
 void init_mem_mapping(void);
 void early_alloc_pgt_buf(void);
 void __init poking_init(void);
diff -puN arch/x86/mm/init.c~direct_gbpages-compiletime arch/x86/mm/init.c
--- a/arch/x86/mm/init.c~direct_gbpages-compiletime	2026-09-02 10:09:00.001169542 -0700
+++ b/arch/x86/mm/init.c	2026-09-02 10:09:00.011170394 -0700
@@ -251,7 +251,7 @@ static void __init probe_page_size_mask(
 		__default_kernel_pte_mask &= ~_PAGE_GLOBAL;
 
 	/* Enable 1 GB linear kernel mappings if available: */
-	if (direct_gbpages && boot_cpu_has(X86_FEATURE_GBPAGES)) {
+	if (direct_gbpages_enabled()) {
 		printk(KERN_INFO "Using GB pages for direct mapping\n");
 		page_size_mask |= 1 << PG_LEVEL_1G;
 	} else {
diff -puN arch/x86/kernel/cpu/common.c~direct_gbpages-compiletime arch/x86/kernel/cpu/common.c
--- a/arch/x86/kernel/cpu/common.c~direct_gbpages-compiletime	2026-09-02 10:09:00.003169713 -0700
+++ b/arch/x86/kernel/cpu/common.c	2026-09-02 10:09:00.012170479 -0700
@@ -2660,7 +2660,7 @@ void __init arch_cpu_finalize_init(void)
 		 * Right now we don't do that with gbpages because there seems
 		 * very little benefit for that case.
 		 */
-		if (!direct_gbpages)
+		if (!direct_gbpages_enabled())
 			set_memory_4k((unsigned long)__va(0), 1);
 	} else {
 		fpu__init_check_bugs();
diff -puN arch/x86/kernel/machine_kexec_64.c~direct_gbpages-compiletime arch/x86/kernel/machine_kexec_64.c
--- a/arch/x86/kernel/machine_kexec_64.c~direct_gbpages-compiletime	2026-09-02 10:09:00.004169798 -0700
+++ b/arch/x86/kernel/machine_kexec_64.c	2026-09-02 10:09:00.012170479 -0700
@@ -257,7 +257,7 @@ static int init_pgtable(struct kimage *i
 		info.kernpg_flag |= _PAGE_ENC;
 	}
 
-	if (direct_gbpages)
+	if (direct_gbpages_enabled())
 		info.direct_gbpages = true;
 
 	for (i = 0; i < nr_pfn_mapped; i++) {
diff -puN arch/x86/mm/pat/set_memory.c~direct_gbpages-compiletime arch/x86/mm/pat/set_memory.c
--- a/arch/x86/mm/pat/set_memory.c~direct_gbpages-compiletime	2026-09-02 10:09:00.008170139 -0700
+++ b/arch/x86/mm/pat/set_memory.c	2026-09-02 10:09:00.013170565 -0700
@@ -130,7 +130,7 @@ void arch_report_meminfo(struct seq_file
 	seq_printf(m, "DirectMap4M:    %8lu kB\n",
 			direct_pages_count[PG_LEVEL_2M] << 12);
 #endif
-	if (direct_gbpages)
+	if (direct_gbpages_enabled())
 		seq_printf(m, "DirectMap1G:    %8lu kB\n",
 			direct_pages_count[PG_LEVEL_1G] << 20);
 }
@@ -1340,7 +1340,7 @@ static int collapse_pud_page(pud_t *pud,
 	pmd_t *pmd, first;
 	int i;
 
-	if (!direct_gbpages)
+	if (!direct_gbpages_enabled())
 		return 0;
 
 	addr &= PUD_MASK;
_

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86/mm: Introduce helper for checking direct map 1G page support
  2026-09-02 19:47 [PATCH] x86/mm: Introduce helper for checking direct map 1G page support Dave Hansen
@ 2026-09-03  0:34 ` Sohil Mehta
  2026-09-03 14:05   ` Dave Hansen
  0 siblings, 1 reply; 4+ messages in thread
From: Sohil Mehta @ 2026-09-03  0:34 UTC (permalink / raw)
  To: Dave Hansen, linux-kernel
  Cc: Andy Lutomirski, Borislav Petkov, H. Peter Anvin, Ingo Molnar,
	Peter Zijlstra, Thomas Gleixner, x86, Yeoreum Yun

On 9/2/2026 12:47 PM, Dave Hansen wrote:
> 
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> There is an existing variable (direct_gbpages) that says whether the
> kernel can and should use 1G pages in the direct map. It is driven
> by a bunch of other machinery. At least:
> 
>  1. Hardware support for 1G pages
>  2. Kconfig support for 1G direct mappings
>  3. Kernel command line overrides
> 
> Most code just checks the 'direct_gbpages' variable itself.  But there
> are cases where 1G mappings are compile-time disabled (via
> X86_DIRECT_GBPAGES) and 'direct_gbpages' is always 0. Unfortunately,
> that constraint is invisible to the compiler.
> 
> This opacity has been historically functionally harmless; it only
> leaves a bit of dead code. But, there are plans to tigthen up the

s/tigthen/tighten


> compile-time checks around folded page table levels. The build will
> break if that dead code appears reachable to the compiler. Making
> the compile-time config visible to the compiler fixes the build.
> 
> Add a helper to replace 'direct_gbpages' checks. Check the Kconfig
> option and base CPU support before looking at the variable.
> 

It is a bit confusing when this variable is finalized and when the
helper can be called. I was assuming that early __init code modifies the
variable and later code consumes the helper.

That is mostly true except for probe_page_size_mask() which is an __init
consumer as well as a modifier. I don't see any functional issue. But it
may be worthy of a code comment to avoid future issues.

> This lets the compiler optimize things better, especially
> collapse_pud_page() where most of the function can now be optimized
> out when the PUD level is folded.
> 

The Link: tag below mentions [1]. Did you mean to reference it here?

> Notes:
> 
> Use boot_cpu_has() instead of cpu_feature_enabled(). There's no
> required/disabled features for 1G pages themselves
> (X86_DIRECT_GBPAGES is for kernel mappings only) and the
> static_cpu_has() infrastructure is just gets in the compiler's way.
> 

s/is//

> This makes 64-bit build marginally larger (20 bytes in one compile)
> and 32-bit builds less marginally _smaller_ (~700 bytes).
> 
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
> Tested-by: Yeoreum Yun <yeoreum.yun@arm.com>
> Link: https://lore.kernel.org/all/20260902-dummy_ptxp3-v3-15-5d8f5b17c25c@arm.com/ [1]
> ---
> 
>  b/arch/x86/include/asm/pgtable.h     |   14 ++++++++++++++
>  b/arch/x86/kernel/cpu/common.c       |    2 +-
>  b/arch/x86/kernel/machine_kexec_64.c |    2 +-
>  b/arch/x86/mm/init.c                 |    2 +-
>  b/arch/x86/mm/pat/set_memory.c       |    4 ++--
>  5 files changed, 19 insertions(+), 5 deletions(-)
> 

Overall, the changes look good to me.

Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>


> diff -puN arch/x86/include/asm/pgtable.h~direct_gbpages-compiletime arch/x86/include/asm/pgtable.h
> --- a/arch/x86/include/asm/pgtable.h~direct_gbpages-compiletime	2026-09-02 10:08:59.999169372 -0700
> +++ b/arch/x86/include/asm/pgtable.h	2026-09-02 10:09:00.011170394 -0700
> @@ -1163,6 +1163,20 @@ static inline int pgd_none(pgd_t pgd)
>  #ifndef __ASSEMBLER__
>  
>  extern int direct_gbpages;
> +static inline bool direct_gbpages_enabled(void)
> +{
> +	/* Check the direct map config option: */
> +	if (!IS_ENABLED(CONFIG_X86_DIRECT_GBPAGES))
> +		return false;
> +
> +	/* Check the CPU feature: */
> +	if (!boot_cpu_has(X86_FEATURE_GBPAGES))
> +		return false;
> +

The first two comments don't add much beyond the code. Would it be
useful to say why boot_cpu_has() instead of static_cpu_has() over here
(mainly to avoid accidental cleanup)?

> +	/* Check the command-line and early setup variable: */
> +	return direct_gbpages;
> +}
> +
>  void init_mem_mapping(void);
>  void early_alloc_pgt_buf(void);
>  void __init poking_init(void);
> diff -puN arch/x86/kernel/machine_kexec_64.c~direct_gbpages-compiletime arch/x86/kernel/machine_kexec_64.c
> --- a/arch/x86/kernel/machine_kexec_64.c~direct_gbpages-compiletime	2026-09-02 10:09:00.004169798 -0700
> +++ b/arch/x86/kernel/machine_kexec_64.c	2026-09-02 10:09:00.012170479 -0700
> @@ -257,7 +257,7 @@ static int init_pgtable(struct kimage *i
>  		info.kernpg_flag |= _PAGE_ENC;
>  	}
>  
> -	if (direct_gbpages)
> +	if (direct_gbpages_enabled())
>  		info.direct_gbpages = true;
>  

How about:

	info.direct_gbpages = direct_gbpages_enabled()


>  	for (i = 0; i < nr_pfn_mapped; i++) {

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86/mm: Introduce helper for checking direct map 1G page support
  2026-09-03  0:34 ` Sohil Mehta
@ 2026-09-03 14:05   ` Dave Hansen
  2026-09-03 17:52     ` Sohil Mehta
  0 siblings, 1 reply; 4+ messages in thread
From: Dave Hansen @ 2026-09-03 14:05 UTC (permalink / raw)
  To: Sohil Mehta, Dave Hansen, linux-kernel
  Cc: Andy Lutomirski, Borislav Petkov, H. Peter Anvin, Ingo Molnar,
	Peter Zijlstra, Thomas Gleixner, x86, Yeoreum Yun

On 9/2/26 17:34, Sohil Mehta wrote:
> On 9/2/2026 12:47 PM, Dave Hansen wrote:
>>  extern int direct_gbpages;
>> +static inline bool direct_gbpages_enabled(void)
>> +{
>> +	/* Check the direct map config option: */
>> +	if (!IS_ENABLED(CONFIG_X86_DIRECT_GBPAGES))
>> +		return false;
>> +
>> +	/* Check the CPU feature: */
>> +	if (!boot_cpu_has(X86_FEATURE_GBPAGES))
>> +		return false;
>> +
> 
> The first two comments don't add much beyond the code.
The "CPU feature" one is arguable. But even when writing this, I was
forgetful about what CONFIG_X86_DIRECT_GBPAGES actually did. I _think_
it was the fact that these:

	CONFIG_X86_DIRECT_GBPAGES
	X86_FEATURE_GBPAGES

kinda read similarly if you're reading fast. The config option also
doesn't have the most enlightening name.

The comments are more there to get the reader to slow down than anything
else.

> Would it be useful to say why boot_cpu_has() instead of
> static_cpu_has() over here (mainly to avoid accidental cleanup)?
It's a pretty minor thing. To me, it's changelog material, not comment
material.

>> +	/* Check the command-line and early setup variable: */
>> +	return direct_gbpages;
>> +}
>> +
>>  void init_mem_mapping(void);
>>  void early_alloc_pgt_buf(void);
>>  void __init poking_init(void);
>> diff -puN arch/x86/kernel/machine_kexec_64.c~direct_gbpages-compiletime arch/x86/kernel/machine_kexec_64.c
>> --- a/arch/x86/kernel/machine_kexec_64.c~direct_gbpages-compiletime	2026-09-02 10:09:00.004169798 -0700
>> +++ b/arch/x86/kernel/machine_kexec_64.c	2026-09-02 10:09:00.012170479 -0700
>> @@ -257,7 +257,7 @@ static int init_pgtable(struct kimage *i
>>  		info.kernpg_flag |= _PAGE_ENC;
>>  	}
>>  
>> -	if (direct_gbpages)
>> +	if (direct_gbpages_enabled())
>>  		info.direct_gbpages = true;
> 
> How about:
> 
> 	info.direct_gbpages = direct_gbpages_enabled()

First and foremost, in a refactoring patch, you must resist the urge to
do this. Refactoring patches' job is to show -- in the most plain way
possible -- that they are not hurting things. The easiest way to do that
is to make it as stupidly obvious as possible to the reader that nothing
is changing.

The moment you start making changes like the suggestion, you make
reviewers' lives harder.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86/mm: Introduce helper for checking direct map 1G page support
  2026-09-03 14:05   ` Dave Hansen
@ 2026-09-03 17:52     ` Sohil Mehta
  0 siblings, 0 replies; 4+ messages in thread
From: Sohil Mehta @ 2026-09-03 17:52 UTC (permalink / raw)
  To: Dave Hansen, Dave Hansen, linux-kernel
  Cc: Andy Lutomirski, Borislav Petkov, H. Peter Anvin, Ingo Molnar,
	Peter Zijlstra, Thomas Gleixner, x86, Yeoreum Yun

On 9/3/2026 7:05 AM, Dave Hansen wrote:
> But even when writing this, I was
> forgetful about what CONFIG_X86_DIRECT_GBPAGES actually did. I _think_
> it was the fact that these:
> 
> 	CONFIG_X86_DIRECT_GBPAGES
> 	X86_FEATURE_GBPAGES
> 
> kinda read similarly if you're reading fast. The config option also
> doesn't have the most enlightening name.

Yes, the config option is a bit vague.

> 
> The comments are more there to get the reader to slow down than anything
> else.

Sure, makes sense.

> First and foremost, in a refactoring patch, you must resist the urge to
> do this. Refactoring patches' job is to show -- in the most plain way
> possible -- that they are not hurting things. The easiest way to do that
> is to make it as stupidly obvious as possible to the reader that nothing
> is changing.
> 

Yeah, I was trying to sneak in a minor cleanup along with the refactor.
Will avoid it next time I do a refactor.







^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-03 17:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-02 19:47 [PATCH] x86/mm: Introduce helper for checking direct map 1G page support Dave Hansen
2026-09-03  0:34 ` Sohil Mehta
2026-09-03 14:05   ` Dave Hansen
2026-09-03 17:52     ` Sohil Mehta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®