[PATCH -v2 00/32] x86/boot/e820: Assorted E820 table handling features and cleanups

[PATCH -v2 00/32] x86/boot/e820: Assorted E820 table handling features and cleanups

[Ingo Molnar]

Changes in -v2:

 - Further refine type-13 handling as suggested by hpa, and
   don't change behavior for other undefined E820 types.

 - Incorporate review feedback from Mike Rapoport and Andy Shevchenko:

    - Clean up the ppro_with_ram_bug() quirk some more
    - Use FW_BUG to print out firmware bugs
    - Simplify the new size-printing code
    - Standardize on the 'System RAM' string for E820_TYPE_RAM
    - Change e820_type_to_string() to take a 'type' parameter
    - Unify e820_print_type() and e820_type_to_string()
    - Move index increments outside accessors in e820__update_table()
    - Reorder the series a bit for easier review
    - Utilize more SZ_ constants

 - Rebase on top of v6.15-rc6 + tip:x86/core

The latest version of this series can be found at:

   git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git WIP.x86/e820

Thanks,

	Ingo

===========
Original -v1 announcement:

So I was looking into a E820 table related bug report by
Paul Menzel, and I wanted to implement the behavior
suggested by the ACPI specification, which bug/problem
results in unbootable Linux systems with certain bootloaders:

  https://lore.kernel.org/r/074c2637-1b65-428e-b3e2-24384780e936@molgen.mpg.de

One thing led to another, and now I'm here 29 patches later,
trying to explain what they all do. :-/

In order of importance:

 - The bugfix / change of Linux kernel E820 table parsing behavior:

      x86/boot/e820: Treat non-type-2 'reserved' E820 region types as E820_TYPE_RESERVED

 - A change to e820_search_gap() to fix an implementational oddity
   that would prefer lower-address same-size PCI gaps over larger-address
   PCI gaps. Now the implementation searches for the largest gap:

      x86/boot/e820: Change e820_search_gap() to search for the highest-address PCI gap

 - A rewrite of e820_search_gap() to search the E820 table in ascending
   order to make sure even weird PCI holes get found, as claimed by the
   comments around the code (but not properly implemented):

      x86/boot/e820: Make sure e820_search_gap() finds all gaps

 - Remove the exclusion of single-entry E820 tables passed in by
   firmware:

      x86/boot/e820: Simplify append_e820_table() and remove restriction on single-entry tables

 - A debuggability improvement, to print the sizes of the e820 entries
   and the holes as well, because parsing raw hexadecimal ranges is hard
   for humans:

      x86/boot/e820: Print gaps in the E820 table
      x86/boot/e820: Print out sizes of E820 memory ranges

   Before:
		BIOS-provided physical RAM map:
		BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
		BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
		BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
		BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] usable
		BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] reserved
		BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] reserved
		BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
		BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] reserved
		BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
		BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] reserved
   After:
		BIOS-provided physical RAM map:
		BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff]  639   KB kernel usable RAM
		BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff]    1   KB device reserved
		BIOS-e820: [gap 0x00000000000a0000-0x00000000000effff]  320   KB ...
		BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff]   64   KB device reserved
		BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff]    1.9 GB kernel usable RAM
		BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff]  144   KB device reserved
		BIOS-e820: [gap 0x0000000080000000-0x00000000afffffff]  768   MB ...
		BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff]  256   MB device reserved
		BIOS-e820: [gap 0x00000000c0000000-0x00000000fed1bfff] 1005.1 MB ...
		BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff]   16   KB device reserved
		BIOS-e820: [gap 0x00000000fed20000-0x00000000feffbfff]    2.8 MB ...
		BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff]   16   KB device reserved
		BIOS-e820: [gap 0x00000000ff000000-0x00000000fffbffff]   15.7 MB ...
		BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff]  256   KB device reserved
		BIOS-e820: [gap 0x0000000100000000-0x000000fcffffffff] 1008   GB ...
		BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff]   12   GB device reserved

   Note how weirdly broken up ranges are printed with fractional size
   values, while 'round' ranges are printed as natural numbers.

 - Assorted cleanups: type cleanups, simplifications, standardization
   of coding patterns, etc.

Thanks,

	Ingo

===============>

Ingo Molnar (32):
  x86/boot/e820: Remove inverted boolean logic from the e820_nomerge() function name, rename it to e820_type_mergeable()
  x86/boot/e820: Simplify e820__print_table() a bit
  x86/boot/e820: Simplify the PPro Erratum #50 workaround
  x86/boot/e820: Mark e820__print_table() static
  x86/boot/e820: Print gaps in the E820 table
  x86/boot/e820: Make the field separator space character part of e820_print_type()
  x86/boot/e820: Print out sizes of E820 memory ranges
  x86/boot/e820: Print E820_TYPE_RAM entries as ... RAM entries
  x86/boot/e820: Call the PCI gap a 'gap' in the boot log printout
  x86/boot/e820: Use 'u64' consistently instead of 'unsigned long long'
  x86/boot/e820: Remove pointless early_panic() indirection
  x86/boot/e820: Clean up confusing and self-contradictory verbiage around E820 related resource allocations
  x86/boot/e820: Improve e820_print_type() messages
  x86/boot/e820: Clean up __e820__range_add() a bit
  x86/boot/e820: Clean up __refdata use a bit
  x86/boot/e820: Remove unnecessary header inclusions
  x86/boot/e820: Standardize e820 table index variable names under 'idx'
  x86/boot/e820: Standardize e820 table index variable types under 'u32'
  x86/boot/e820: Change struct e820_table::nr_entries type from __u32 to u32
  x86/boot/e820: Clean up e820__setup_pci_gap()/e820_search_gap() a bit
  x86/boot/e820: Change e820_search_gap() to search for the highest-address PCI gap
  x86/boot/e820: Rename gap_start/gap_size to max_gap_start/max_gap_start in e820_search_gap() et al
  x86/boot/e820: Simplify & clarify __e820__range_add() a bit
  x86/boot/e820: Standardize __init/__initdata tag placement
  x86/boot/e820: Simplify append_e820_table() and remove restriction on single-entry tables
  x86/boot/e820: Remove e820__range_remove()'s unused return parameter
  x86/boot/e820: Simplify the e820__range_remove() API
  x86/boot/e820: Make sure e820_search_gap() finds all gaps
  x86/boot/e820: Introduce E820_TYPE_13 and treat it as a device region
  x86/boot/e820: Change e820_type_to_string() to take a 'type' parameter
  x86/boot/e820: Unify e820_print_type() and e820_type_to_string()
  x86/boot/e820: Move index increments outside accessors in e820__update_table()

 arch/x86/include/asm/e820/api.h   |   3 +-
 arch/x86/include/asm/e820/types.h |   6 +-
 arch/x86/kernel/e820.c            | 555 ++++++++++++++++++++++----------------
 arch/x86/kernel/setup.c           |  10 +-
 arch/x86/platform/efi/efi.c       |   3 +-
 5 files changed, 328 insertions(+), 249 deletions(-)

-- 
2.45.2

[PATCH 01/32] x86/boot/e820: Remove inverted boolean logic from the e820_nomerge() function name, rename it to e820_type_mergeable()

[Ingo Molnar]

It's a bad practice to put inverted logic into function names,
flip it back and rename it to e820_type_mergeable().

Add/update a few comments about this function while at it.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 9920122018a0..b95f0519a877 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -304,18 +304,22 @@ static int __init cpcompare(const void *a, const void *b)
 	return (ap->addr != ap->entry->addr) - (bp->addr != bp->entry->addr);
 }
 
-static bool e820_nomerge(enum e820_type type)
+/*
+ * Can two consecutive E820 entries of this same E820 type be merged?
+ */
+static bool e820_type_mergeable(enum e820_type type)
 {
 	/*
 	 * These types may indicate distinct platform ranges aligned to
-	 * numa node, protection domain, performance domain, or other
+	 * NUMA node, protection domain, performance domain, or other
 	 * boundaries. Do not merge them.
 	 */
 	if (type == E820_TYPE_PRAM)
-		return true;
+		return false;
 	if (type == E820_TYPE_SOFT_RESERVED)
-		return true;
-	return false;
+		return false;
+
+	return true;
 }
 
 int __init e820__update_table(struct e820_table *table)
@@ -393,7 +397,7 @@ int __init e820__update_table(struct e820_table *table)
 		}
 
 		/* Continue building up new map based on this information: */
-		if (current_type != last_type || e820_nomerge(current_type)) {
+		if (current_type != last_type || !e820_type_mergeable(current_type)) {
 			if (last_type) {
 				new_entries[new_nr_entries].size = change_point[chg_idx]->addr - last_addr;
 				/* Move forward only if the new size was non-zero: */
-- 
2.45.2

Re: [PATCH 01/32] x86/boot/e820: Remove inverted boolean logic from the e820_nomerge() function name, rename it to e820_type_mergeable()

[Nikolay Borisov]

On 5/15/25 15:05, Ingo Molnar wrote:
> It's a bad practice to put inverted logic into function names,
> flip it back and rename it to e820_type_mergeable().
> 
> Add/update a few comments about this function while at it.
> 
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: Arnd Bergmann <arnd@kernel.org>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>

Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>

[PATCH 02/32] x86/boot/e820: Simplify e820__print_table() a bit

[Ingo Molnar]

Introduce 'entry' for the current table entry and shorten
repetitious use of e820_table->entries[i].

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index b95f0519a877..904925159560 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -204,12 +204,14 @@ void __init e820__print_table(char *who)
 	int i;
 
 	for (i = 0; i < e820_table->nr_entries; i++) {
+		struct e820_entry *entry = e820_table->entries + i;
+
 		pr_info("%s: [mem %#018Lx-%#018Lx] ",
 			who,
-			e820_table->entries[i].addr,
-			e820_table->entries[i].addr + e820_table->entries[i].size - 1);
+			entry->addr,
+			entry->addr + entry->size-1);
 
-		e820_print_type(e820_table->entries[i].type);
+		e820_print_type(entry->type);
 		pr_cont("\n");
 	}
 }
-- 
2.45.2

[PATCH 03/32] x86/boot/e820: Simplify the PPro Erratum #50 workaround

[Ingo Molnar]

No need to print out the table - users won't really be able
to tell much from it anyway and the messages around this
erratum are unnecessarily obtuse.

Instead clearly inform the user that a 256 kB hole is being
punched in their memory map at the 1.75 GB physical address.

Not that there are many PPro users left. :-)

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/setup.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 7d9ed79a93c0..f40dffc3e014 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -984,11 +984,9 @@ void __init setup_arch(char **cmdline_p)
 	trim_bios_range();
 #ifdef CONFIG_X86_32
 	if (ppro_with_ram_bug()) {
-		e820__range_update(0x70000000ULL, 0x40000ULL, E820_TYPE_RAM,
-				  E820_TYPE_RESERVED);
+		pr_info("Applying PPro RAM bug workaround: punching 256 kB hole at 1.75 GB physical.\n");
+		e820__range_update(0x70000000ULL, SZ_256K, E820_TYPE_RAM, E820_TYPE_RESERVED);
 		e820__update_table(e820_table);
-		printk(KERN_INFO "fixed physical RAM map:\n");
-		e820__print_table("bad_ppro");
 	}
 #else
 	early_gart_iommu_check();
-- 
2.45.2

[PATCH 04/32] x86/boot/e820: Mark e820__print_table() static

[Ingo Molnar]

There are no external users of this function left.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/include/asm/e820/api.h | 1 -
 arch/x86/kernel/e820.c          | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/e820/api.h b/arch/x86/include/asm/e820/api.h
index c83645d5b2a8..54427b77bc19 100644
--- a/arch/x86/include/asm/e820/api.h
+++ b/arch/x86/include/asm/e820/api.h
@@ -19,7 +19,6 @@ extern u64  e820__range_update(u64 start, u64 size, enum e820_type old_type, enu
 extern u64  e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type);
 extern u64  e820__range_update_table(struct e820_table *t, u64 start, u64 size, enum e820_type old_type, enum e820_type new_type);
 
-extern void e820__print_table(char *who);
 extern int  e820__update_table(struct e820_table *table);
 extern void e820__update_table_print(void);
 
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 904925159560..c7911d2054fe 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -199,7 +199,7 @@ static void __init e820_print_type(enum e820_type type)
 	}
 }
 
-void __init e820__print_table(char *who)
+static void __init e820__print_table(const char *who)
 {
 	int i;
 
-- 
2.45.2

[PATCH 05/32] x86/boot/e820: Print gaps in the E820 table

[Ingo Molnar]

Gaps in the E820 table are not obvious at a glance and can
easily be overlooked.

Print out gaps in the E820 table:

Before:

	BIOS-provided physical RAM map:
	BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
	BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
	BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
	BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] usable
	BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] reserved
	BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] reserved
	BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
	BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] reserved
	BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
	BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] reserved
After:

	BIOS-provided physical RAM map:
	BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
	BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
	BIOS-e820: [gap 0x00000000000a0000-0x00000000000effff]
	BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
	BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] usable
	BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] reserved
	BIOS-e820: [gap 0x0000000080000000-0x00000000afffffff]
	BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] reserved
	BIOS-e820: [gap 0x00000000c0000000-0x00000000fed1bfff]
	BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
	BIOS-e820: [gap 0x00000000fed20000-0x00000000feffbfff]
	BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] reserved
	BIOS-e820: [gap 0x00000000ff000000-0x00000000fffbffff]
	BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
	BIOS-e820: [gap 0x0000000100000000-0x000000fcffffffff]
	BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] reserved

Also warn about badly ordered E820 table entries:

	BUG: out of order E820 entry!

( this is printed before the entry is printed, so there's no need to
  print any additional data with the warning. )

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index c7911d2054fe..c8833ac9cbf9 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -201,18 +201,32 @@ static void __init e820_print_type(enum e820_type type)
 
 static void __init e820__print_table(const char *who)
 {
+	u64 range_end_prev = 0;
 	int i;
 
 	for (i = 0; i < e820_table->nr_entries; i++) {
 		struct e820_entry *entry = e820_table->entries + i;
+		u64 range_start, range_end;
 
-		pr_info("%s: [mem %#018Lx-%#018Lx] ",
-			who,
-			entry->addr,
-			entry->addr + entry->size-1);
+		range_start = entry->addr;
+		range_end   = entry->addr + entry->size;
 
+		/* Out of order E820 maps should not happen: */
+		if (range_start < range_end_prev)
+			pr_info(FW_BUG "out of order E820 entry!\n");
+
+		if (range_start > range_end_prev) {
+			pr_info("%s: [gap %#018Lx-%#018Lx]\n",
+				who,
+				range_end_prev,
+				range_start-1);
+		}
+
+		pr_info("%s: [mem %#018Lx-%#018Lx] ", who, range_start, range_end-1);
 		e820_print_type(entry->type);
 		pr_cont("\n");
+
+		range_end_prev = range_end;
 	}
 }
 
-- 
2.45.2

[PATCH 06/32] x86/boot/e820: Make the field separator space character part of e820_print_type()

[Ingo Molnar]

We are going to add more columns to the E820 table printout,
so make e820_print_type()'s field separator (space character)
part of the function itself.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index c8833ac9cbf9..b5895b142707 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -187,15 +187,15 @@ void __init e820__range_add(u64 start, u64 size, enum e820_type type)
 static void __init e820_print_type(enum e820_type type)
 {
 	switch (type) {
-	case E820_TYPE_RAM:		pr_cont("usable");			break;
-	case E820_TYPE_RESERVED:	pr_cont("reserved");			break;
-	case E820_TYPE_SOFT_RESERVED:	pr_cont("soft reserved");		break;
-	case E820_TYPE_ACPI:		pr_cont("ACPI data");			break;
-	case E820_TYPE_NVS:		pr_cont("ACPI NVS");			break;
-	case E820_TYPE_UNUSABLE:	pr_cont("unusable");			break;
+	case E820_TYPE_RAM:		pr_cont(" usable");			break;
+	case E820_TYPE_RESERVED:	pr_cont(" reserved");			break;
+	case E820_TYPE_SOFT_RESERVED:	pr_cont(" soft reserved");		break;
+	case E820_TYPE_ACPI:		pr_cont(" ACPI data");			break;
+	case E820_TYPE_NVS:		pr_cont(" ACPI NVS");			break;
+	case E820_TYPE_UNUSABLE:	pr_cont(" unusable");			break;
 	case E820_TYPE_PMEM:		/* Fall through: */
-	case E820_TYPE_PRAM:		pr_cont("persistent (type %u)", type);	break;
-	default:			pr_cont("type %u", type);		break;
+	case E820_TYPE_PRAM:		pr_cont(" persistent (type %u)", type);	break;
+	default:			pr_cont(" type %u", type);		break;
 	}
 }
 
@@ -491,9 +491,9 @@ __e820__range_update(struct e820_table *table, u64 start, u64 size, enum e820_ty
 		size = ULLONG_MAX - start;
 
 	end = start + size;
-	printk(KERN_DEBUG "e820: update [mem %#010Lx-%#010Lx] ", start, end - 1);
+	printk(KERN_DEBUG "e820: update [mem %#010Lx-%#010Lx]", start, end - 1);
 	e820_print_type(old_type);
-	pr_cont(" ==> ");
+	pr_cont(" ==>");
 	e820_print_type(new_type);
 	pr_cont("\n");
 
@@ -568,7 +568,7 @@ u64 __init e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool
 		size = ULLONG_MAX - start;
 
 	end = start + size;
-	printk(KERN_DEBUG "e820: remove [mem %#010Lx-%#010Lx] ", start, end - 1);
+	printk(KERN_DEBUG "e820: remove [mem %#010Lx-%#010Lx]", start, end - 1);
 	if (check_type)
 		e820_print_type(old_type);
 	pr_cont("\n");
-- 
2.45.2

[PATCH 07/32] x86/boot/e820: Print out sizes of E820 memory ranges

[Ingo Molnar]

Before:

        BIOS-provided physical RAM map:
        BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
        BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
        BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
        BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] usable
        BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] reserved
        BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] reserved
        BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
        BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] reserved
        BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
        BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] reserved

After:

	BIOS-provided physical RAM map:
	BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff]  639   KB kernel usable RAM
	BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff]    1   KB reserved
	BIOS-e820: [gap 0x00000000000a0000-0x00000000000effff]  320   KB ...
	BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff]   64   KB reserved
	BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff]    1.9 GB kernel usable RAM
	BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff]  144   KB reserved
	BIOS-e820: [gap 0x0000000080000000-0x00000000afffffff]  768   MB ...
	BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff]  256   MB reserved
	BIOS-e820: [gap 0x00000000c0000000-0x00000000fed1bfff] 1005.1 MB ...
	BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff]   16   KB reserved
	BIOS-e820: [gap 0x00000000fed20000-0x00000000feffbfff]    2.8 MB ...
	BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff]   16   KB reserved
	BIOS-e820: [gap 0x00000000ff000000-0x00000000fffbffff]   15.7 MB ...
	BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff]  256   KB reserved
	BIOS-e820: [gap 0x0000000100000000-0x000000fcffffffff] 1008   GB ...
	BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff]   12   GB reserved

Note how a 1-digit precision field is printed out if a range is
fractional in its largest-enclosing natural size unit.

So the "256 MB" and "12 GB" fields above denote exactly 256 MB and
12 GB regions, while "1.9 GB" signals the region's fractional nature
and it being just below 2GB.

Printing E820 maps with such details visualizes 'weird' ranges
at a glance, and gives users a better understanding of how
large the various ranges are, without having to perform hexadecimal
subtraction in their minds.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
(cherry picked from commit d1ac6b8718575a7ea2f0a1ff347835a8879df673)
---
 arch/x86/kernel/e820.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index b5895b142707..7f600d32a999 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -199,6 +199,41 @@ static void __init e820_print_type(enum e820_type type)
 	}
 }
 
+/*
+ * Print out the size of a E820 region, in human-readable
+ * fashion, going from KB, MB, GB to TB units.
+ *
+ * Print out fractional sizes with a single digit of precision.
+ */
+static void e820_print_size(u64 size)
+{
+	if (size < SZ_1M) {
+		if (size & (SZ_1K-1))
+			pr_cont(" %4llu.%01llu KB", size/SZ_1K, 10*(size & (SZ_1K-1))/SZ_1K);
+		else
+			pr_cont(" %4llu   KB", size/SZ_1K);
+		return;
+	}
+	if (size < SZ_1G) {
+		if (size & (SZ_1M-1))
+			pr_cont(" %4llu.%01llu MB", size/SZ_1M, 10*(size & (SZ_1M-1))/SZ_1M);
+		else
+			pr_cont(" %4llu   MB", size/SZ_1M);
+		return;
+	}
+	if (size < SZ_1T) {
+		if (size & (SZ_1G-1))
+			pr_cont(" %4llu.%01llu GB", size/SZ_1G, 10*(size & (SZ_1G-1))/SZ_1G);
+		else
+			pr_cont(" %4llu   GB", size/SZ_1G);
+		return;
+	}
+	if (size & (SZ_1T-1))
+		pr_cont(" %4llu.%01llu TB", size/SZ_1T, 10*(size & (SZ_1T-1))/SZ_1T);
+	else
+		pr_cont(" %4llu   TB", size/SZ_1T);
+}
+
 static void __init e820__print_table(const char *who)
 {
 	u64 range_end_prev = 0;
@@ -215,14 +250,22 @@ static void __init e820__print_table(const char *who)
 		if (range_start < range_end_prev)
 			pr_info(FW_BUG "out of order E820 entry!\n");
 
+		/* Print gaps, if any: */
 		if (range_start > range_end_prev) {
-			pr_info("%s: [gap %#018Lx-%#018Lx]\n",
+			u64 gap_size = range_start - range_end_prev;
+
+			pr_info("%s: [gap %#018Lx-%#018Lx]",
 				who,
 				range_end_prev,
 				range_start-1);
+
+			e820_print_size(gap_size);
+			pr_cont(" ...\n");
 		}
 
-		pr_info("%s: [mem %#018Lx-%#018Lx] ", who, range_start, range_end-1);
+		/* Print allocated ranges: */
+		pr_info("%s: [mem %#018Lx-%#018Lx]", who, range_start, range_end-1);
+		e820_print_size(entry->size);
 		e820_print_type(entry->type);
 		pr_cont("\n");
 
-- 
2.45.2

Re: [PATCH 07/32] x86/boot/e820: Print out sizes of E820 memory ranges

[Andy Shevchenko]

On Thu, May 15, 2025 at 02:05:23PM +0200, Ingo Molnar wrote:
> Before:
> 
>         BIOS-provided physical RAM map:
>         BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
>         BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
>         BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
>         BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] usable
>         BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] reserved
>         BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] reserved
>         BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
>         BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] reserved
>         BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
>         BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] reserved
> 
> After:
> 
> 	BIOS-provided physical RAM map:
> 	BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff]  639   KB kernel usable RAM
> 	BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff]    1   KB reserved
> 	BIOS-e820: [gap 0x00000000000a0000-0x00000000000effff]  320   KB ...
> 	BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff]   64   KB reserved
> 	BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff]    1.9 GB kernel usable RAM
> 	BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff]  144   KB reserved
> 	BIOS-e820: [gap 0x0000000080000000-0x00000000afffffff]  768   MB ...
> 	BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff]  256   MB reserved
> 	BIOS-e820: [gap 0x00000000c0000000-0x00000000fed1bfff] 1005.1 MB ...
> 	BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff]   16   KB reserved
> 	BIOS-e820: [gap 0x00000000fed20000-0x00000000feffbfff]    2.8 MB ...
> 	BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff]   16   KB reserved
> 	BIOS-e820: [gap 0x00000000ff000000-0x00000000fffbffff]   15.7 MB ...
> 	BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff]  256   KB reserved
> 	BIOS-e820: [gap 0x0000000100000000-0x000000fcffffffff] 1008   GB ...
> 	BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff]   12   GB reserved
> 
> Note how a 1-digit precision field is printed out if a range is
> fractional in its largest-enclosing natural size unit.
> 
> So the "256 MB" and "12 GB" fields above denote exactly 256 MB and
> 12 GB regions, while "1.9 GB" signals the region's fractional nature
> and it being just below 2GB.
> 
> Printing E820 maps with such details visualizes 'weird' ranges
> at a glance, and gives users a better understanding of how
> large the various ranges are, without having to perform hexadecimal
> subtraction in their minds.

Returning to the v1 discussion for sring_get_size(). Looking at its code
it seems to me that you haven't tried to use it. It should give no .0 for
the exact numbers. If it's not the case, please confirm that we have a
bug/feature. If it's documented (read: we have a test case), then we would
need an additional flag to avoid this behaviour for your case. No need
to reinvent a wheel here.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 07/32] x86/boot/e820: Print out sizes of E820 memory ranges

[Ingo Molnar]

* Andy Shevchenko <andy@kernel.org> wrote:

> > Printing E820 maps with such details visualizes 'weird' ranges at a 
> > glance, and gives users a better understanding of how large the 
> > various ranges are, without having to perform hexadecimal 
> > subtraction in their minds.
> 
> Returning to the v1 discussion for sring_get_size(). Looking at its 
> code it seems to me that you haven't tried to use it. It should give 
> no .0 for the exact numbers. If it's not the case, please confirm 
> that we have a bug/feature. If it's documented (read: we have a test 
> case), then we would need an additional flag to avoid this behaviour 
> for your case. No need to reinvent a wheel here.

I briefly looked at get_string_size(), and it insists on the KiB/GiB 
sort of nonsense for base-2 sizes that we rarely use in x86 debug 
output, nor do we want to.

And I'm not convinced about 'struct range' and '%pre' either: I'd 
prefer to control both th data format and the output, while pra 
seems to insist both on using 'struct range', and on naming the
%output 'range', right?

I'm not convinced such a 'struct range' over-abstraction will actually 
simplify the code. The e820 debug printout code I extended in this 
series basically follows the data structure patterns and nomenclature 
of the e820 code itself.

It might or might not make sense to convert the e820 code to 'struct 
range' in a followup series, but that is beyond the scope of this 
series that refines the debug output.

Thanks,

	Ingo

[PATCH 08/32] x86/boot/e820: Print E820_TYPE_RAM entries as ... RAM entries

[Ingo Molnar]

So it is a bit weird that the actual RAM entries of the E820 table
are not actually called RAM, but 'usable':

	BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff]    1.9 GB usable

'usable' is pretty passive-aggressive in that context and ambiguous,
most E820 entries denote 'usable' address ranges - reserved ranges
may be used by devices, or the platform.

Clarify and disambiguate this by making the boot log entry
explicitly say 'System RAM', like in /proc/iomem:

	BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff]    1.9 GB System RAM

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 7f600d32a999..0a324d0db60e 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -187,7 +187,7 @@ void __init e820__range_add(u64 start, u64 size, enum e820_type type)
 static void __init e820_print_type(enum e820_type type)
 {
 	switch (type) {
-	case E820_TYPE_RAM:		pr_cont(" usable");			break;
+	case E820_TYPE_RAM:		pr_cont(" System RAM");			break;
 	case E820_TYPE_RESERVED:	pr_cont(" reserved");			break;
 	case E820_TYPE_SOFT_RESERVED:	pr_cont(" soft reserved");		break;
 	case E820_TYPE_ACPI:		pr_cont(" ACPI data");			break;
-- 
2.45.2

[PATCH 09/32] x86/boot/e820: Call the PCI gap a 'gap' in the boot log printout

[Ingo Molnar]

It is a bit weird and inconsistent that the PCI gap is
advertised during bootup as 'mem'ory:

  [mem 0xc0000000-0xfed1bfff] available for PCI devices
   ^^^

It's not really memory, it's a gap that PCI devices can decode
and use and they often do not map it to any memory themselves.

So advertise it for what it is, a gap:

  [gap 0xc0000000-0xfed1bfff] available for PCI devices

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 0a324d0db60e..d85623c9ee1b 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -741,7 +741,7 @@ __init void e820__setup_pci_gap(void)
 	 */
 	pci_mem_start = gapstart;
 
-	pr_info("[mem %#010lx-%#010lx] available for PCI devices\n",
+	pr_info("[gap %#010lx-%#010lx] available for PCI devices\n",
 		gapstart, gapstart + gapsize - 1);
 }
 
-- 
2.45.2

Re: [PATCH 09/32] x86/boot/e820: Call the PCI gap a 'gap' in the boot log printout

[Andy Shevchenko]

On Thu, May 15, 2025 at 02:05:25PM +0200, Ingo Molnar wrote:
> It is a bit weird and inconsistent that the PCI gap is
> advertised during bootup as 'mem'ory:
> 
>   [mem 0xc0000000-0xfed1bfff] available for PCI devices
>    ^^^
> 
> It's not really memory, it's a gap that PCI devices can decode
> and use and they often do not map it to any memory themselves.
> 
> So advertise it for what it is, a gap:
> 
>   [gap 0xc0000000-0xfed1bfff] available for PCI devices

Why not use word 'range' instead of the 'gap'? This will allow to switch to
%pra without modifying the output.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 09/32] x86/boot/e820: Call the PCI gap a 'gap' in the boot log printout

[Ingo Molnar]

* Andy Shevchenko <andy@kernel.org> wrote:

> On Thu, May 15, 2025 at 02:05:25PM +0200, Ingo Molnar wrote:
> > It is a bit weird and inconsistent that the PCI gap is
> > advertised during bootup as 'mem'ory:
> > 
> >   [mem 0xc0000000-0xfed1bfff] available for PCI devices
> >    ^^^
> > 
> > It's not really memory, it's a gap that PCI devices can decode
> > and use and they often do not map it to any memory themselves.
> > 
> > So advertise it for what it is, a gap:
> > 
> >   [gap 0xc0000000-0xfed1bfff] available for PCI devices
> 
> Why not use word 'range' instead of the 'gap'? This will allow to switch to
> %pra without modifying the output.

Well, it's not a range, it's a gap in the memory map. Why does %pra 
dictate details of the output in such a fashion?

Thanks,

	Ingo

[PATCH 10/32] x86/boot/e820: Use 'u64' consistently instead of 'unsigned long long'

[Ingo Molnar]

There's a number of structure fields and local variables related
to E820 entry physical addresses that are defined as 'unsigned long long',
but then are compared to u64 fields.

Make the types all consistently u64.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index d85623c9ee1b..ba957f2ef210 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -338,7 +338,7 @@ struct change_member {
 	/* Pointer to the original entry: */
 	struct e820_entry	*entry;
 	/* Address for this change point: */
-	unsigned long long	addr;
+	u64			addr;
 };
 
 static struct change_member	change_point_list[2*E820_MAX_ENTRIES]	__initdata;
@@ -386,7 +386,7 @@ int __init e820__update_table(struct e820_table *table)
 	struct e820_entry *entries = table->entries;
 	u32 max_nr_entries = ARRAY_SIZE(table->entries);
 	enum e820_type current_type, last_type;
-	unsigned long long last_addr;
+	u64 last_addr;
 	u32 new_nr_entries, overlap_entries;
 	u32 i, chg_idx, chg_nr;
 
@@ -683,13 +683,13 @@ static void __init e820__update_table_kexec(void)
  */
 static int __init e820_search_gap(unsigned long *gapstart, unsigned long *gapsize)
 {
-	unsigned long long last = MAX_GAP_END;
+	u64 last = MAX_GAP_END;
 	int i = e820_table->nr_entries;
 	int found = 0;
 
 	while (--i >= 0) {
-		unsigned long long start = e820_table->entries[i].addr;
-		unsigned long long end = start + e820_table->entries[i].size;
+		u64 start = e820_table->entries[i].addr;
+		u64 end = start + e820_table->entries[i].size;
 
 		/*
 		 * Since "last" is at most 4GB, we know we'll
-- 
2.45.2

[PATCH 11/32] x86/boot/e820: Remove pointless early_panic() indirection

[Ingo Molnar]

early_panic() is a pointless wrapper around panic():

	static void __init early_panic(char *msg)
	{
		early_printk(msg);
		panic(msg);
	}

panic() will already do a printk() of 'msg', and an early_printk() if
earlyprintk is enabled. There's no need to print it separately.

Remove the function.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index ba957f2ef210..5eb0849b492f 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -935,12 +935,6 @@ unsigned long __init e820__end_of_low_ram_pfn(void)
 	return e820__end_ram_pfn(1UL << (32 - PAGE_SHIFT));
 }
 
-static void __init early_panic(char *msg)
-{
-	early_printk(msg);
-	panic(msg);
-}
-
 static int userdef __initdata;
 
 /* The "mem=nopentium" boot option disables 4MB page tables on 32-bit kernels: */
@@ -1060,7 +1054,7 @@ void __init e820__finish_early_params(void)
 {
 	if (userdef) {
 		if (e820__update_table(e820_table) < 0)
-			early_panic("Invalid user supplied memory map");
+			panic("Invalid user supplied memory map");
 
 		pr_info("user-defined physical RAM map:\n");
 		e820__print_table("user");
-- 
2.45.2

[PATCH 12/32] x86/boot/e820: Clean up confusing and self-contradictory verbiage around E820 related resource allocations

[Ingo Molnar]

So the E820 code has a rather confusing area of code at around
e820__reserve_resources(), which is, by its plain reading,
rather self-contradictory. For example, the comment explaining
e820__reserve_resources() claims:

 - '* Mark E820 reserved areas as busy for the resource manager'

By 'E820 reserved areas' one can naively conclude that it's
talking about E820_TYPE_RESERVED areas - while those areas
are treated in exactly the opposite fashion by do_mark_busy():

        switch (type) {
        case E820_TYPE_RESERVED:
        case E820_TYPE_SOFT_RESERVED:
        case E820_TYPE_PRAM:
        case E820_TYPE_PMEM:
                return false;

Ie. E820_TYPE_RESERVED areas are *not* marked busy for the
resource manager, because E820_TYPE_RESERVED areas are
device regions that might eventually be claimed by a device driver.

This type of confusion permeates this whole area of code,
making it exceedingly difficult to read (for me at least).

So untangle it bit by bit:

 - Instead of talking about ambiguous 'reserved areas',
   talk about 'E820 device address regions' instead,
   and 'register'/'lock' them.

 - The do_mark_busy() function is a misnomer as well, because despite
   its name it 'does' nothing - it only determines what type
   of resource handling an E820 type should receive from the
   kernel. Rename it to e820_device_region() and negate its
   meaning, to avoid the 'busy/reserved' confusion. Because
   that's what this code is really about: filtering out
   device regions such as E820_TYPE_RESERVED, E820_TYPE_PRAM,
   E820_TYPE_PMEM, etc., and allowing them to be claimed
   by device drivers later on.

 - All other E820 regions (system regions) are registered and
   locked early on, before the PCI resource manager does its
   search for device BAR addresses, etc.

Also fix this somewhat misleading comment:

	/*
	 * Try to bump up RAM regions to reasonable boundaries, to
	 * avoid stolen RAM:
	 */

and explain that here we register artificial 'gap' resources
at the end of suspiciously sized RAM regions, as heuristics
to try to avoid buggy firmware with undeclared 'stolen RAM' regions:

	/*
	 * Create additional 'gaps' at the end of RAM regions,
	 * rounding them up to 64k/1MB/64MB boundaries, should
	 * they be weirdly sized, and register extra, locked
	 * resource regions for them, to make sure drivers
	 * won't claim those addresses.
	 *
	 * These are basically blind guesses and heuristics to
	 * avoid resource conflicts with broken firmware that
	 * doesn't properly list 'stolen RAM' as a system region
	 * in the E820 map.
	 */

Also improve the printout of this extra resource a bit: make the
message more unambiguous, and upgrade it from pr_debug() (where
very few people will see it), to pr_info() (where it will make
it into the syslog on default distro configs).

Also fix spelling and improve comment placement.

No change in functionality intended.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 55 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 5eb0849b492f..c9bb808c4888 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1106,37 +1106,44 @@ static unsigned long __init e820_type_to_iores_desc(struct e820_entry *entry)
 	}
 }
 
-static bool __init do_mark_busy(enum e820_type type, struct resource *res)
+/*
+ * We assign one resource entry for each E820 map entry:
+ */
+static struct resource __initdata *e820_res;
+
+/*
+ * Is this a device address region that should not be marked busy?
+ * (Versus system address regions that we register & lock early.)
+ */
+static bool __init e820_device_region(enum e820_type type, struct resource *res)
 {
-	/* this is the legacy bios/dos rom-shadow + mmio region */
+	/* This is the legacy BIOS/DOS ROM-shadow + MMIO region: */
 	if (res->start < (1ULL<<20))
-		return true;
+		return false;
 
 	/*
 	 * Treat persistent memory and other special memory ranges like
-	 * device memory, i.e. reserve it for exclusive use of a driver
+	 * device memory, i.e. keep it available for exclusive use of a
+	 * driver:
 	 */
 	switch (type) {
 	case E820_TYPE_RESERVED:
 	case E820_TYPE_SOFT_RESERVED:
 	case E820_TYPE_PRAM:
 	case E820_TYPE_PMEM:
-		return false;
+		return true;
 	case E820_TYPE_RAM:
 	case E820_TYPE_ACPI:
 	case E820_TYPE_NVS:
 	case E820_TYPE_UNUSABLE:
 	default:
-		return true;
+		return false;
 	}
 }
 
 /*
- * Mark E820 reserved areas as busy for the resource manager:
+ * Mark E820 system regions as busy for the resource manager:
  */
-
-static struct resource __initdata *e820_res;
-
 void __init e820__reserve_resources(void)
 {
 	int i;
@@ -1162,18 +1169,18 @@ void __init e820__reserve_resources(void)
 		res->desc  = e820_type_to_iores_desc(entry);
 
 		/*
-		 * Don't register the region that could be conflicted with
-		 * PCI device BAR resources and insert them later in
-		 * pcibios_resource_survey():
+		 * Skip and don't register device regions that could be conflicted
+		 * with PCI device BAR resources. They get inserted later in
+		 * pcibios_resource_survey() -> e820__reserve_resources_late():
 		 */
-		if (do_mark_busy(entry->type, res)) {
+		if (!e820_device_region(entry->type, res)) {
 			res->flags |= IORESOURCE_BUSY;
 			insert_resource(&iomem_resource, res);
 		}
 		res++;
 	}
 
-	/* Expose the kexec e820 table to the sysfs. */
+	/* Expose the kexec e820 table to sysfs: */
 	for (i = 0; i < e820_table_kexec->nr_entries; i++) {
 		struct e820_entry *entry = e820_table_kexec->entries + i;
 
@@ -1207,6 +1214,10 @@ void __init e820__reserve_resources_late(void)
 	int i;
 	struct resource *res;
 
+	/*
+	 * Register device address regions listed in the E820 map,
+	 * these can be claimed by device drivers later on:
+	 */
 	res = e820_res;
 	for (i = 0; i < e820_table->nr_entries; i++) {
 		if (!res->parent && res->end)
@@ -1215,8 +1226,16 @@ void __init e820__reserve_resources_late(void)
 	}
 
 	/*
-	 * Try to bump up RAM regions to reasonable boundaries, to
-	 * avoid stolen RAM:
+	 * Create additional 'gaps' at the end of RAM regions,
+	 * rounding them up to 64k/1MB/64MB boundaries, should
+	 * they be weirdly sized, and register extra, locked
+	 * resource regions for them, to make sure drivers
+	 * won't claim those addresses.
+	 *
+	 * These are basically blind guesses and heuristics to
+	 * avoid resource conflicts with broken firmware that
+	 * doesn't properly list 'stolen RAM' as a system region
+	 * in the E820 map.
 	 */
 	for (i = 0; i < e820_table->nr_entries; i++) {
 		struct e820_entry *entry = &e820_table->entries[i];
@@ -1232,7 +1251,7 @@ void __init e820__reserve_resources_late(void)
 		if (start >= end)
 			continue;
 
-		printk(KERN_DEBUG "e820: reserve RAM buffer [mem %#010llx-%#010llx]\n", start, end);
+		pr_info("e820: register RAM buffer resource [mem %#010llx-%#010llx]\n", start, end);
 		reserve_region_with_split(&iomem_resource, start, end, "RAM buffer");
 	}
 }
-- 
2.45.2

Re: [PATCH 12/32] x86/boot/e820: Clean up confusing and self-contradictory verbiage around E820 related resource allocations

[Nikolay Borisov]

On 5/15/25 15:05, Ingo Molnar wrote:
> So the E820 code has a rather confusing area of code at around
> e820__reserve_resources(), which is, by its plain reading,
> rather self-contradictory. For example, the comment explaining
> e820__reserve_resources() claims:
> 
>   - '* Mark E820 reserved areas as busy for the resource manager'
> 
> By 'E820 reserved areas' one can naively conclude that it's
> talking about E820_TYPE_RESERVED areas - while those areas
> are treated in exactly the opposite fashion by do_mark_busy():
> 
>          switch (type) {
>          case E820_TYPE_RESERVED:
>          case E820_TYPE_SOFT_RESERVED:
>          case E820_TYPE_PRAM:
>          case E820_TYPE_PMEM:
>                  return false;
> 
> Ie. E820_TYPE_RESERVED areas are *not* marked busy for the
> resource manager, because E820_TYPE_RESERVED areas are
> device regions that might eventually be claimed by a device driver.
> 
> This type of confusion permeates this whole area of code,
> making it exceedingly difficult to read (for me at least).
> 
> So untangle it bit by bit:
> 
>   - Instead of talking about ambiguous 'reserved areas',
>     talk about 'E820 device address regions' instead,
>     and 'register'/'lock' them.
> 
>   - The do_mark_busy() function is a misnomer as well, because despite
>     its name it 'does' nothing - it only determines what type
>     of resource handling an E820 type should receive from the
>     kernel. Rename it to e820_device_region() and negate its
>     meaning, to avoid the 'busy/reserved' confusion. Because
>     that's what this code is really about: filtering out
>     device regions such as E820_TYPE_RESERVED, E820_TYPE_PRAM,
>     E820_TYPE_PMEM, etc., and allowing them to be claimed
>     by device drivers later on.
> 
>   - All other E820 regions (system regions) are registered and
>     locked early on, before the PCI resource manager does its
>     search for device BAR addresses, etc.
> 
> Also fix this somewhat misleading comment:
> 
> 	/*
> 	 * Try to bump up RAM regions to reasonable boundaries, to
> 	 * avoid stolen RAM:
> 	 */
> 
> and explain that here we register artificial 'gap' resources
> at the end of suspiciously sized RAM regions, as heuristics
> to try to avoid buggy firmware with undeclared 'stolen RAM' regions:
> 
> 	/*
> 	 * Create additional 'gaps' at the end of RAM regions,
> 	 * rounding them up to 64k/1MB/64MB boundaries, should
> 	 * they be weirdly sized, and register extra, locked
> 	 * resource regions for them, to make sure drivers
> 	 * won't claim those addresses.
> 	 *
> 	 * These are basically blind guesses and heuristics to
> 	 * avoid resource conflicts with broken firmware that
> 	 * doesn't properly list 'stolen RAM' as a system region
> 	 * in the E820 map.
> 	 */
> 
> Also improve the printout of this extra resource a bit: make the
> message more unambiguous, and upgrade it from pr_debug() (where
> very few people will see it), to pr_info() (where it will make
> it into the syslog on default distro configs).
> 
> Also fix spelling and improve comment placement.
> 
> No change in functionality intended.
> 
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: Arnd Bergmann <arnd@kernel.org>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>   arch/x86/kernel/e820.c | 55 +++++++++++++++++++++++++++++++++-----------------
>   1 file changed, 37 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 5eb0849b492f..c9bb808c4888 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -1106,37 +1106,44 @@ static unsigned long __init e820_type_to_iores_desc(struct e820_entry *entry)
>   	}
>   }
>   
> -static bool __init do_mark_busy(enum e820_type type, struct resource *res)
> +/*
> + * We assign one resource entry for each E820 map entry:
> + */
> +static struct resource __initdata *e820_res;
> +
> +/*
> + * Is this a device address region that should not be marked busy?
> + * (Versus system address regions that we register & lock early.)
> + */
> +static bool __init e820_device_region(enum e820_type type, struct resource *res)
>   {
> -	/* this is the legacy bios/dos rom-shadow + mmio region */
> +	/* This is the legacy BIOS/DOS ROM-shadow + MMIO region: */
>   	if (res->start < (1ULL<<20))

nit: While at it, change this to also use SZ_1M define rather than this 
shift.

<snip>

[PATCH 13/32] x86/boot/e820: Improve e820_print_type() messages

[Ingo Molnar]

For E820_TYPE_RESERVED, print:

  'reserved'  -> 'device reserved'

For E820_TYPE_PRAM and E820_TYPE_PMEM:

  'persistent' -> 'persistent RAM'

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index c9bb808c4888..6cb1e23dfc89 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -187,15 +187,15 @@ void __init e820__range_add(u64 start, u64 size, enum e820_type type)
 static void __init e820_print_type(enum e820_type type)
 {
 	switch (type) {
-	case E820_TYPE_RAM:		pr_cont(" System RAM");			break;
-	case E820_TYPE_RESERVED:	pr_cont(" reserved");			break;
-	case E820_TYPE_SOFT_RESERVED:	pr_cont(" soft reserved");		break;
-	case E820_TYPE_ACPI:		pr_cont(" ACPI data");			break;
-	case E820_TYPE_NVS:		pr_cont(" ACPI NVS");			break;
-	case E820_TYPE_UNUSABLE:	pr_cont(" unusable");			break;
+	case E820_TYPE_RAM:		pr_cont(" System RAM");				break;
+	case E820_TYPE_RESERVED:	pr_cont(" device reserved");			break;
+	case E820_TYPE_SOFT_RESERVED:	pr_cont(" soft reserved");			break;
+	case E820_TYPE_ACPI:		pr_cont(" ACPI data");				break;
+	case E820_TYPE_NVS:		pr_cont(" ACPI NVS");				break;
+	case E820_TYPE_UNUSABLE:	pr_cont(" unusable");				break;
 	case E820_TYPE_PMEM:		/* Fall through: */
-	case E820_TYPE_PRAM:		pr_cont(" persistent (type %u)", type);	break;
-	default:			pr_cont(" type %u", type);		break;
+	case E820_TYPE_PRAM:		pr_cont(" persistent RAM (type %u)", type);	break;
+	default:			pr_cont(" type %u", type);			break;
 	}
 }
 
-- 
2.45.2

[PATCH 14/32] x86/boot/e820: Clean up __e820__range_add() a bit

[Ingo Molnar]

 - Use 'idx' index variable instead of a weird 'x'
 - Make the error message E820-specific
 - Group the code a bit better

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 6cb1e23dfc89..ba4e6e6bb0a5 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -165,17 +165,18 @@ int e820__get_entry_type(u64 start, u64 end)
  */
 static void __init __e820__range_add(struct e820_table *table, u64 start, u64 size, enum e820_type type)
 {
-	int x = table->nr_entries;
+	int idx = table->nr_entries;
 
-	if (x >= ARRAY_SIZE(table->entries)) {
-		pr_err("too many entries; ignoring [mem %#010llx-%#010llx]\n",
-		       start, start + size - 1);
+	if (idx >= ARRAY_SIZE(table->entries)) {
+		pr_err("too many E820 table entries; ignoring [mem %#010llx-%#010llx]\n",
+		       start, start + size-1);
 		return;
 	}
 
-	table->entries[x].addr = start;
-	table->entries[x].size = size;
-	table->entries[x].type = type;
+	table->entries[idx].addr = start;
+	table->entries[idx].size = size;
+	table->entries[idx].type = type;
+
 	table->nr_entries++;
 }
 
-- 
2.45.2

[PATCH 15/32] x86/boot/e820: Clean up __refdata use a bit

[Ingo Molnar]

So __refdata, like __init, is more of a storage class specifier,
so move the attribute in front of the type, not after the variable
name. This also aligns it vertically.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index ba4e6e6bb0a5..bb48dcc8a8ee 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -60,9 +60,9 @@ static struct e820_table e820_table_init		__initdata;
 static struct e820_table e820_table_kexec_init		__initdata;
 static struct e820_table e820_table_firmware_init	__initdata;
 
-struct e820_table *e820_table __refdata			= &e820_table_init;
-struct e820_table *e820_table_kexec __refdata		= &e820_table_kexec_init;
-struct e820_table *e820_table_firmware __refdata	= &e820_table_firmware_init;
+__refdata struct e820_table *e820_table			= &e820_table_init;
+__refdata struct e820_table *e820_table_kexec		= &e820_table_kexec_init;
+__refdata struct e820_table *e820_table_firmware	= &e820_table_firmware_init;
 
 /* For PCI or other memory-mapped resources */
 unsigned long pci_mem_start = 0xaeedbabe;
-- 
2.45.2

[PATCH 16/32] x86/boot/e820: Remove unnecessary header inclusions

[Ingo Molnar]

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index bb48dcc8a8ee..806d69ca09af 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -9,13 +9,11 @@
  * quirks and other tweaks, and feeds that into the generic Linux memory
  * allocation code routines via a platform independent interface (memblock, etc.).
  */
-#include <linux/crash_dump.h>
 #include <linux/memblock.h>
 #include <linux/suspend.h>
 #include <linux/acpi.h>
 #include <linux/firmware-map.h>
 #include <linux/sort.h>
-#include <linux/memory_hotplug.h>
 
 #include <asm/e820/api.h>
 #include <asm/setup.h>
-- 
2.45.2

[PATCH 17/32] x86/boot/e820: Standardize e820 table index variable names under 'idx'

[Ingo Molnar]

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 114 ++++++++++++++++++++++++-------------------------
 1 file changed, 57 insertions(+), 57 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 806d69ca09af..3ee266673fee 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -75,10 +75,10 @@ EXPORT_SYMBOL(pci_mem_start);
 static bool _e820__mapped_any(struct e820_table *table,
 			      u64 start, u64 end, enum e820_type type)
 {
-	int i;
+	int idx;
 
-	for (i = 0; i < table->nr_entries; i++) {
-		struct e820_entry *entry = &table->entries[i];
+	for (idx = 0; idx < table->nr_entries; idx++) {
+		struct e820_entry *entry = &table->entries[idx];
 
 		if (type && entry->type != type)
 			continue;
@@ -110,10 +110,10 @@ EXPORT_SYMBOL_GPL(e820__mapped_any);
 static struct e820_entry *__e820__mapped_all(u64 start, u64 end,
 					     enum e820_type type)
 {
-	int i;
+	int idx;
 
-	for (i = 0; i < e820_table->nr_entries; i++) {
-		struct e820_entry *entry = &e820_table->entries[i];
+	for (idx = 0; idx < e820_table->nr_entries; idx++) {
+		struct e820_entry *entry = &e820_table->entries[idx];
 
 		if (type && entry->type != type)
 			continue;
@@ -236,10 +236,10 @@ static void e820_print_size(u64 size)
 static void __init e820__print_table(const char *who)
 {
 	u64 range_end_prev = 0;
-	int i;
+	int idx;
 
-	for (i = 0; i < e820_table->nr_entries; i++) {
-		struct e820_entry *entry = e820_table->entries + i;
+	for (idx = 0; idx < e820_table->nr_entries; idx++) {
+		struct e820_entry *entry = e820_table->entries + idx;
 		u64 range_start, range_end;
 
 		range_start = entry->addr;
@@ -387,7 +387,7 @@ int __init e820__update_table(struct e820_table *table)
 	enum e820_type current_type, last_type;
 	u64 last_addr;
 	u32 new_nr_entries, overlap_entries;
-	u32 i, chg_idx, chg_nr;
+	u32 idx, chg_idx, chg_nr;
 
 	/* If there's only one memory region, don't bother: */
 	if (table->nr_entries < 2)
@@ -396,26 +396,26 @@ int __init e820__update_table(struct e820_table *table)
 	BUG_ON(table->nr_entries > max_nr_entries);
 
 	/* Bail out if we find any unreasonable addresses in the map: */
-	for (i = 0; i < table->nr_entries; i++) {
-		if (entries[i].addr + entries[i].size < entries[i].addr)
+	for (idx = 0; idx < table->nr_entries; idx++) {
+		if (entries[idx].addr + entries[idx].size < entries[idx].addr)
 			return -1;
 	}
 
 	/* Create pointers for initial change-point information (for sorting): */
-	for (i = 0; i < 2 * table->nr_entries; i++)
-		change_point[i] = &change_point_list[i];
+	for (idx = 0; idx < 2 * table->nr_entries; idx++)
+		change_point[idx] = &change_point_list[idx];
 
 	/*
 	 * Record all known change-points (starting and ending addresses),
 	 * omitting empty memory regions:
 	 */
 	chg_idx = 0;
-	for (i = 0; i < table->nr_entries; i++)	{
-		if (entries[i].size != 0) {
-			change_point[chg_idx]->addr	= entries[i].addr;
-			change_point[chg_idx++]->entry	= &entries[i];
-			change_point[chg_idx]->addr	= entries[i].addr + entries[i].size;
-			change_point[chg_idx++]->entry	= &entries[i];
+	for (idx = 0; idx < table->nr_entries; idx++)	{
+		if (entries[idx].size != 0) {
+			change_point[chg_idx]->addr	= entries[idx].addr;
+			change_point[chg_idx++]->entry	= &entries[idx];
+			change_point[chg_idx]->addr	= entries[idx].addr + entries[idx].size;
+			change_point[chg_idx++]->entry	= &entries[idx];
 		}
 	}
 	chg_nr = chg_idx;
@@ -437,9 +437,9 @@ int __init e820__update_table(struct e820_table *table)
 			overlap_list[overlap_entries++] = change_point[chg_idx]->entry;
 		} else {
 			/* Remove entry from list (order independent, so swap with last): */
-			for (i = 0; i < overlap_entries; i++) {
-				if (overlap_list[i] == change_point[chg_idx]->entry)
-					overlap_list[i] = overlap_list[overlap_entries-1];
+			for (idx = 0; idx < overlap_entries; idx++) {
+				if (overlap_list[idx] == change_point[chg_idx]->entry)
+					overlap_list[idx] = overlap_list[overlap_entries-1];
 			}
 			overlap_entries--;
 		}
@@ -449,9 +449,9 @@ int __init e820__update_table(struct e820_table *table)
 		 * 1=usable, 2,3,4,4+=unusable)
 		 */
 		current_type = 0;
-		for (i = 0; i < overlap_entries; i++) {
-			if (overlap_list[i]->type > current_type)
-				current_type = overlap_list[i]->type;
+		for (idx = 0; idx < overlap_entries; idx++) {
+			if (overlap_list[idx]->type > current_type)
+				current_type = overlap_list[idx]->type;
 		}
 
 		/* Continue building up new map based on this information: */
@@ -524,7 +524,7 @@ static u64 __init
 __e820__range_update(struct e820_table *table, u64 start, u64 size, enum e820_type old_type, enum e820_type new_type)
 {
 	u64 end;
-	unsigned int i;
+	unsigned int idx;
 	u64 real_updated_size = 0;
 
 	BUG_ON(old_type == new_type);
@@ -539,8 +539,8 @@ __e820__range_update(struct e820_table *table, u64 start, u64 size, enum e820_ty
 	e820_print_type(new_type);
 	pr_cont("\n");
 
-	for (i = 0; i < table->nr_entries; i++) {
-		struct e820_entry *entry = &table->entries[i];
+	for (idx = 0; idx < table->nr_entries; idx++) {
+		struct e820_entry *entry = &table->entries[idx];
 		u64 final_start, final_end;
 		u64 entry_end;
 
@@ -602,7 +602,7 @@ u64 __init e820__range_update_table(struct e820_table *t, u64 start, u64 size,
 /* Remove a range of memory from the E820 table: */
 u64 __init e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type)
 {
-	int i;
+	int idx;
 	u64 end;
 	u64 real_removed_size = 0;
 
@@ -615,8 +615,8 @@ u64 __init e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool
 		e820_print_type(old_type);
 	pr_cont("\n");
 
-	for (i = 0; i < e820_table->nr_entries; i++) {
-		struct e820_entry *entry = &e820_table->entries[i];
+	for (idx = 0; idx < e820_table->nr_entries; idx++) {
+		struct e820_entry *entry = &e820_table->entries[idx];
 		u64 final_start, final_end;
 		u64 entry_end;
 
@@ -683,12 +683,12 @@ static void __init e820__update_table_kexec(void)
 static int __init e820_search_gap(unsigned long *gapstart, unsigned long *gapsize)
 {
 	u64 last = MAX_GAP_END;
-	int i = e820_table->nr_entries;
+	int idx = e820_table->nr_entries;
 	int found = 0;
 
-	while (--i >= 0) {
-		u64 start = e820_table->entries[i].addr;
-		u64 end = start + e820_table->entries[i].size;
+	while (--idx >= 0) {
+		u64 start = e820_table->entries[idx].addr;
+		u64 end = start + e820_table->entries[idx].size;
 
 		/*
 		 * Since "last" is at most 4GB, we know we'll
@@ -814,11 +814,11 @@ void __init e820__memory_setup_extended(u64 phys_addr, u32 data_len)
  */
 void __init e820__register_nosave_regions(unsigned long limit_pfn)
 {
-	int i;
+	int idx;
 	u64 last_addr = 0;
 
-	for (i = 0; i < e820_table->nr_entries; i++) {
-		struct e820_entry *entry = &e820_table->entries[i];
+	for (idx = 0; idx < e820_table->nr_entries; idx++) {
+		struct e820_entry *entry = &e820_table->entries[idx];
 
 		if (entry->type != E820_TYPE_RAM)
 			continue;
@@ -839,10 +839,10 @@ void __init e820__register_nosave_regions(unsigned long limit_pfn)
  */
 static int __init e820__register_nvs_regions(void)
 {
-	int i;
+	int idx;
 
-	for (i = 0; i < e820_table->nr_entries; i++) {
-		struct e820_entry *entry = &e820_table->entries[i];
+	for (idx = 0; idx < e820_table->nr_entries; idx++) {
+		struct e820_entry *entry = &e820_table->entries[idx];
 
 		if (entry->type == E820_TYPE_NVS)
 			acpi_nvs_register(entry->addr, entry->size);
@@ -890,12 +890,12 @@ u64 __init e820__memblock_alloc_reserved(u64 size, u64 align)
  */
 static unsigned long __init e820__end_ram_pfn(unsigned long limit_pfn)
 {
-	int i;
+	int idx;
 	unsigned long last_pfn = 0;
 	unsigned long max_arch_pfn = MAX_ARCH_PFN;
 
-	for (i = 0; i < e820_table->nr_entries; i++) {
-		struct e820_entry *entry = &e820_table->entries[i];
+	for (idx = 0; idx < e820_table->nr_entries; idx++) {
+		struct e820_entry *entry = &e820_table->entries[idx];
 		unsigned long start_pfn;
 		unsigned long end_pfn;
 
@@ -1145,7 +1145,7 @@ static bool __init e820_device_region(enum e820_type type, struct resource *res)
  */
 void __init e820__reserve_resources(void)
 {
-	int i;
+	int idx;
 	struct resource *res;
 	u64 end;
 
@@ -1153,8 +1153,8 @@ void __init e820__reserve_resources(void)
 			     SMP_CACHE_BYTES);
 	e820_res = res;
 
-	for (i = 0; i < e820_table->nr_entries; i++) {
-		struct e820_entry *entry = e820_table->entries + i;
+	for (idx = 0; idx < e820_table->nr_entries; idx++) {
+		struct e820_entry *entry = e820_table->entries + idx;
 
 		end = entry->addr + entry->size - 1;
 		if (end != (resource_size_t)end) {
@@ -1180,8 +1180,8 @@ void __init e820__reserve_resources(void)
 	}
 
 	/* Expose the kexec e820 table to sysfs: */
-	for (i = 0; i < e820_table_kexec->nr_entries; i++) {
-		struct e820_entry *entry = e820_table_kexec->entries + i;
+	for (idx = 0; idx < e820_table_kexec->nr_entries; idx++) {
+		struct e820_entry *entry = e820_table_kexec->entries + idx;
 
 		firmware_map_add_early(entry->addr, entry->addr + entry->size, e820_type_to_string(entry));
 	}
@@ -1210,7 +1210,7 @@ static unsigned long __init ram_alignment(resource_size_t pos)
 
 void __init e820__reserve_resources_late(void)
 {
-	int i;
+	int idx;
 	struct resource *res;
 
 	/*
@@ -1218,7 +1218,7 @@ void __init e820__reserve_resources_late(void)
 	 * these can be claimed by device drivers later on:
 	 */
 	res = e820_res;
-	for (i = 0; i < e820_table->nr_entries; i++) {
+	for (idx = 0; idx < e820_table->nr_entries; idx++) {
 		if (!res->parent && res->end)
 			insert_resource_expand_to_fit(&iomem_resource, res);
 		res++;
@@ -1236,8 +1236,8 @@ void __init e820__reserve_resources_late(void)
 	 * doesn't properly list 'stolen RAM' as a system region
 	 * in the E820 map.
 	 */
-	for (i = 0; i < e820_table->nr_entries; i++) {
-		struct e820_entry *entry = &e820_table->entries[i];
+	for (idx = 0; idx < e820_table->nr_entries; idx++) {
+		struct e820_entry *entry = &e820_table->entries[idx];
 		u64 start, end;
 
 		if (entry->type != E820_TYPE_RAM)
@@ -1314,7 +1314,7 @@ void __init e820__memory_setup(void)
 
 void __init e820__memblock_setup(void)
 {
-	int i;
+	int idx;
 	u64 end;
 
 #ifdef CONFIG_MEMORY_HOTPLUG
@@ -1358,8 +1358,8 @@ void __init e820__memblock_setup(void)
 	 */
 	memblock_allow_resize();
 
-	for (i = 0; i < e820_table->nr_entries; i++) {
-		struct e820_entry *entry = &e820_table->entries[i];
+	for (idx = 0; idx < e820_table->nr_entries; idx++) {
+		struct e820_entry *entry = &e820_table->entries[idx];
 
 		end = entry->addr + entry->size;
 		if (end != (resource_size_t)end)
-- 
2.45.2

Re: [PATCH 17/32] x86/boot/e820: Standardize e820 table index variable names under 'idx'

[Nikolay Borisov]

On 5/15/25 15:05, Ingo Molnar wrote:
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: Arnd Bergmann <arnd@kernel.org>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>   arch/x86/kernel/e820.c | 114 ++++++++++++++++++++++++-------------------------
>   1 file changed, 57 insertions(+), 57 deletions(-)
> 
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 806d69ca09af..3ee266673fee 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -75,10 +75,10 @@ EXPORT_SYMBOL(pci_mem_start);
>   static bool _e820__mapped_any(struct e820_table *table,
>   			      u64 start, u64 end, enum e820_type type)
>   {
> -	int i;
> +	int idx;
>   
> -	for (i = 0; i < table->nr_entries; i++) {
> -		struct e820_entry *entry = &table->entries[i];
> +	for (idx = 0; idx < table->nr_entries; idx++) {
> +		struct e820_entry *entry = &table->entries[idx];

nit: Since the kernel supports gnu11 since e8c07082a810f what's your 
take on defining the variables inside the for loop?

>   
>   		if (type && entry->type != type)
>   			continue;

<snip>

[PATCH 18/32] x86/boot/e820: Standardize e820 table index variable types under 'u32'

[Ingo Molnar]

So we have 'idx' types of 'int' and 'unsigned int', and sometimes
we assign 'u32' fields such as e820_table::nr_entries to these 'int'
values.

While there's no real risk of overflow with these tables, make it
all cleaner by standardizing on a single type: u32.

This also happens to shrink the code a bit:

   text      data      bss        dec        hex    filename
   7745     44072        0      51817       ca69    e820.o.before
   7613     44072        0      51685       c9e5    e820.o.after

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 3ee266673fee..ee8b56605e4a 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -75,7 +75,7 @@ EXPORT_SYMBOL(pci_mem_start);
 static bool _e820__mapped_any(struct e820_table *table,
 			      u64 start, u64 end, enum e820_type type)
 {
-	int idx;
+	u32 idx;
 
 	for (idx = 0; idx < table->nr_entries; idx++) {
 		struct e820_entry *entry = &table->entries[idx];
@@ -110,7 +110,7 @@ EXPORT_SYMBOL_GPL(e820__mapped_any);
 static struct e820_entry *__e820__mapped_all(u64 start, u64 end,
 					     enum e820_type type)
 {
-	int idx;
+	u32 idx;
 
 	for (idx = 0; idx < e820_table->nr_entries; idx++) {
 		struct e820_entry *entry = &e820_table->entries[idx];
@@ -163,7 +163,7 @@ int e820__get_entry_type(u64 start, u64 end)
  */
 static void __init __e820__range_add(struct e820_table *table, u64 start, u64 size, enum e820_type type)
 {
-	int idx = table->nr_entries;
+	u32 idx = table->nr_entries;
 
 	if (idx >= ARRAY_SIZE(table->entries)) {
 		pr_err("too many E820 table entries; ignoring [mem %#010llx-%#010llx]\n",
@@ -236,7 +236,7 @@ static void e820_print_size(u64 size)
 static void __init e820__print_table(const char *who)
 {
 	u64 range_end_prev = 0;
-	int idx;
+	u32 idx;
 
 	for (idx = 0; idx < e820_table->nr_entries; idx++) {
 		struct e820_entry *entry = e820_table->entries + idx;
@@ -524,7 +524,7 @@ static u64 __init
 __e820__range_update(struct e820_table *table, u64 start, u64 size, enum e820_type old_type, enum e820_type new_type)
 {
 	u64 end;
-	unsigned int idx;
+	u32 idx;
 	u64 real_updated_size = 0;
 
 	BUG_ON(old_type == new_type);
@@ -602,7 +602,7 @@ u64 __init e820__range_update_table(struct e820_table *t, u64 start, u64 size,
 /* Remove a range of memory from the E820 table: */
 u64 __init e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type)
 {
-	int idx;
+	u32 idx;
 	u64 end;
 	u64 real_removed_size = 0;
 
@@ -814,7 +814,7 @@ void __init e820__memory_setup_extended(u64 phys_addr, u32 data_len)
  */
 void __init e820__register_nosave_regions(unsigned long limit_pfn)
 {
-	int idx;
+	u32 idx;
 	u64 last_addr = 0;
 
 	for (idx = 0; idx < e820_table->nr_entries; idx++) {
@@ -839,7 +839,7 @@ void __init e820__register_nosave_regions(unsigned long limit_pfn)
  */
 static int __init e820__register_nvs_regions(void)
 {
-	int idx;
+	u32 idx;
 
 	for (idx = 0; idx < e820_table->nr_entries; idx++) {
 		struct e820_entry *entry = &e820_table->entries[idx];
@@ -890,7 +890,7 @@ u64 __init e820__memblock_alloc_reserved(u64 size, u64 align)
  */
 static unsigned long __init e820__end_ram_pfn(unsigned long limit_pfn)
 {
-	int idx;
+	u32 idx;
 	unsigned long last_pfn = 0;
 	unsigned long max_arch_pfn = MAX_ARCH_PFN;
 
@@ -1145,7 +1145,7 @@ static bool __init e820_device_region(enum e820_type type, struct resource *res)
  */
 void __init e820__reserve_resources(void)
 {
-	int idx;
+	u32 idx;
 	struct resource *res;
 	u64 end;
 
@@ -1210,7 +1210,7 @@ static unsigned long __init ram_alignment(resource_size_t pos)
 
 void __init e820__reserve_resources_late(void)
 {
-	int idx;
+	u32 idx;
 	struct resource *res;
 
 	/*
@@ -1314,7 +1314,7 @@ void __init e820__memory_setup(void)
 
 void __init e820__memblock_setup(void)
 {
-	int idx;
+	u32 idx;
 	u64 end;
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-- 
2.45.2

[PATCH 19/32] x86/boot/e820: Change struct e820_table::nr_entries type from __u32 to u32

[Ingo Molnar]

__u32 is for UAPI headers, and this definition is the only place
in the kernel-internal E820 code that uses __u32. Change it to u32.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/include/asm/e820/types.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/e820/types.h b/arch/x86/include/asm/e820/types.h
index 80c4a7266629..df12f7ee75d3 100644
--- a/arch/x86/include/asm/e820/types.h
+++ b/arch/x86/include/asm/e820/types.h
@@ -83,7 +83,7 @@ struct e820_entry {
  * The whole array of E820 entries:
  */
 struct e820_table {
-	__u32 nr_entries;
+	u32 nr_entries;
 	struct e820_entry entries[E820_MAX_ENTRIES];
 };
 
-- 
2.45.2

[PATCH 20/32] x86/boot/e820: Clean up e820__setup_pci_gap()/e820_search_gap() a bit

[Ingo Molnar]

Apply misc cleanups:

 - Use a bit more readable variable names, we haven't run out of
   underscore characters in the kernel yet.

 - s/0x400000/SZ_4M

 - s/1024*1024/SZ_1M

Suggested-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index ee8b56605e4a..d00ca2dd20b7 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -680,7 +680,7 @@ static void __init e820__update_table_kexec(void)
 /*
  * Search for a gap in the E820 memory space from 0 to MAX_GAP_END (4GB).
  */
-static int __init e820_search_gap(unsigned long *gapstart, unsigned long *gapsize)
+static int __init e820_search_gap(unsigned long *gap_start, unsigned long *gap_size)
 {
 	u64 last = MAX_GAP_END;
 	int idx = e820_table->nr_entries;
@@ -697,9 +697,9 @@ static int __init e820_search_gap(unsigned long *gapstart, unsigned long *gapsiz
 		if (last > end) {
 			unsigned long gap = last - end;
 
-			if (gap >= *gapsize) {
-				*gapsize = gap;
-				*gapstart = end;
+			if (gap >= *gap_size) {
+				*gap_size = gap;
+				*gap_start = end;
 				found = 1;
 			}
 		}
@@ -719,29 +719,29 @@ static int __init e820_search_gap(unsigned long *gapstart, unsigned long *gapsiz
  */
 __init void e820__setup_pci_gap(void)
 {
-	unsigned long gapstart, gapsize;
+	unsigned long gap_start, gap_size;
 	int found;
 
-	gapsize = 0x400000;
-	found  = e820_search_gap(&gapstart, &gapsize);
+	gap_size = SZ_4M;
+	found  = e820_search_gap(&gap_start, &gap_size);
 
 	if (!found) {
 #ifdef CONFIG_X86_64
-		gapstart = (max_pfn << PAGE_SHIFT) + 1024*1024;
+		gap_start = (max_pfn << PAGE_SHIFT) + SZ_1M;
 		pr_err("Cannot find an available gap in the 32-bit address range\n");
 		pr_err("PCI devices with unassigned 32-bit BARs may not work!\n");
 #else
-		gapstart = 0x10000000;
+		gap_start = 0x10000000;
 #endif
 	}
 
 	/*
 	 * e820__reserve_resources_late() protects stolen RAM already:
 	 */
-	pci_mem_start = gapstart;
+	pci_mem_start = gap_start;
 
 	pr_info("[gap %#010lx-%#010lx] available for PCI devices\n",
-		gapstart, gapstart + gapsize - 1);
+		gap_start, gap_start + gap_size - 1);
 }
 
 /*
-- 
2.45.2

Re: [PATCH 20/32] x86/boot/e820: Clean up e820__setup_pci_gap()/e820_search_gap() a bit

[Nikolay Borisov]

On 5/15/25 15:05, Ingo Molnar wrote:
> Apply misc cleanups:
> 
>   - Use a bit more readable variable names, we haven't run out of
>     underscore characters in the kernel yet.
> 
>   - s/0x400000/SZ_4M
> 
>   - s/1024*1024/SZ_1M
> 
> Suggested-by: Andy Shevchenko <andy@kernel.org>
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> Cc: Arnd Bergmann <arnd@kernel.org>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>   arch/x86/kernel/e820.c | 22 +++++++++++-----------
>   1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index ee8b56605e4a..d00ca2dd20b7 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -680,7 +680,7 @@ static void __init e820__update_table_kexec(void)
>   /*
>    * Search for a gap in the E820 memory space from 0 to MAX_GAP_END (4GB).
>    */
> -static int __init e820_search_gap(unsigned long *gapstart, unsigned long *gapsize)
> +static int __init e820_search_gap(unsigned long *gap_start, unsigned long *gap_size)
>   {
>   	u64 last = MAX_GAP_END;
>   	int idx = e820_table->nr_entries;
> @@ -697,9 +697,9 @@ static int __init e820_search_gap(unsigned long *gapstart, unsigned long *gapsiz
>   		if (last > end) {
>   			unsigned long gap = last - end;
>   
> -			if (gap >= *gapsize) {
> -				*gapsize = gap;
> -				*gapstart = end;
> +			if (gap >= *gap_size) {
> +				*gap_size = gap;
> +				*gap_start = end;
>   				found = 1;
>   			}
>   		}
> @@ -719,29 +719,29 @@ static int __init e820_search_gap(unsigned long *gapstart, unsigned long *gapsiz
>    */
>   __init void e820__setup_pci_gap(void)
>   {
> -	unsigned long gapstart, gapsize;
> +	unsigned long gap_start, gap_size;
>   	int found;
>   
> -	gapsize = 0x400000;
> -	found  = e820_search_gap(&gapstart, &gapsize);
> +	gap_size = SZ_4M;
> +	found  = e820_search_gap(&gap_start, &gap_size);
>   
>   	if (!found) {
>   #ifdef CONFIG_X86_64
> -		gapstart = (max_pfn << PAGE_SHIFT) + 1024*1024;
> +		gap_start = (max_pfn << PAGE_SHIFT) + SZ_1M;
>   		pr_err("Cannot find an available gap in the 32-bit address range\n");
>   		pr_err("PCI devices with unassigned 32-bit BARs may not work!\n");
>   #else
> -		gapstart = 0x10000000;
> +		gap_start = 0x10000000;

nit: gap_start = SZ_256M;

>   #endif
>   	}
>   
>   	/*
>   	 * e820__reserve_resources_late() protects stolen RAM already:
>   	 */
> -	pci_mem_start = gapstart;
> +	pci_mem_start = gap_start;
>   
>   	pr_info("[gap %#010lx-%#010lx] available for PCI devices\n",
> -		gapstart, gapstart + gapsize - 1);
> +		gap_start, gap_start + gap_size - 1);
>   }
>   
>   /*

[PATCH 21/32] x86/boot/e820: Change e820_search_gap() to search for the highest-address PCI gap

[Ingo Molnar]

Right now the main x86 function that determines the position and size
of the 'PCI gap', e820_search_gap(), has this curious property:

        while (--idx >= 0) {
	...
			if (gap >= *gap_size) {

I.e. it will iterate the E820 table backwards, from its end to the beginning,
and will search for larger and larger gaps in the memory map below 4GB,
until it finishes with the table.

This logic will, should there be two gaps with the same size, pick the
one with the lower physical address - which is contrary to usual
practice that the PCI gap is just below 4GB.

Furthermore, the commit that introduced this weird logic 16 years ago:

  3381959da5a0 ("x86: cleanup e820_setup_gap(), add e820_search_gap(), v2")

  -                       if (gap > gapsize) {
  +                       if (gap >= *gapsize) {

didn't even declare this change, the title says it's a cleanup,
and the changelog declares it as a preparatory refactoring
for a later bugfix:

  809d9a8f93bd ("x86/PCI: ACPI based PCI gap calculation")

which bugfix was reverted only 1 day later without much of an
explanation, and was never reintroduced:

  58b6e5538460 ("Revert "x86/PCI: ACPI based PCI gap calculation"")

So based on the Git archeology and by the plain reading of the
code I declare this '>=' change an unintended bug and side effect.

Change it to '>' again.

It should not make much of a difference in practice, as the
likelihood of having *two* largest gaps with exactly the same
size are very low outside of weird user-provided memory maps.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index d00ca2dd20b7..b2e2ada50adb 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -697,7 +697,7 @@ static int __init e820_search_gap(unsigned long *gap_start, unsigned long *gap_s
 		if (last > end) {
 			unsigned long gap = last - end;
 
-			if (gap >= *gap_size) {
+			if (gap > *gap_size) {
 				*gap_size = gap;
 				*gap_start = end;
 				found = 1;
-- 
2.45.2

[PATCH 22/32] x86/boot/e820: Rename gap_start/gap_size to max_gap_start/max_gap_start in e820_search_gap() et al

[Ingo Molnar]

The PCI gap searching functions pass around pointers to the
gap_start/gap_size variables, which refer to the maximum
size gap found so far.

Rename the variables to say so, and disambiguate their namespace
from 'current gap' variables.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index b2e2ada50adb..1eb5afbdd9e6 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -680,7 +680,7 @@ static void __init e820__update_table_kexec(void)
 /*
  * Search for a gap in the E820 memory space from 0 to MAX_GAP_END (4GB).
  */
-static int __init e820_search_gap(unsigned long *gap_start, unsigned long *gap_size)
+static int __init e820_search_gap(unsigned long *max_gap_start, unsigned long *max_gap_size)
 {
 	u64 last = MAX_GAP_END;
 	int idx = e820_table->nr_entries;
@@ -697,9 +697,9 @@ static int __init e820_search_gap(unsigned long *gap_start, unsigned long *gap_s
 		if (last > end) {
 			unsigned long gap = last - end;
 
-			if (gap > *gap_size) {
-				*gap_size = gap;
-				*gap_start = end;
+			if (gap > *max_gap_size) {
+				*max_gap_size = gap;
+				*max_gap_start = end;
 				found = 1;
 			}
 		}
@@ -719,29 +719,29 @@ static int __init e820_search_gap(unsigned long *gap_start, unsigned long *gap_s
  */
 __init void e820__setup_pci_gap(void)
 {
-	unsigned long gap_start, gap_size;
+	unsigned long max_gap_start, max_gap_size;
 	int found;
 
-	gap_size = SZ_4M;
-	found  = e820_search_gap(&gap_start, &gap_size);
+	max_gap_size = SZ_4M;
+	found  = e820_search_gap(&max_gap_start, &max_gap_size);
 
 	if (!found) {
 #ifdef CONFIG_X86_64
-		gap_start = (max_pfn << PAGE_SHIFT) + SZ_1M;
+		max_gap_start = (max_pfn << PAGE_SHIFT) + SZ_1M;
 		pr_err("Cannot find an available gap in the 32-bit address range\n");
 		pr_err("PCI devices with unassigned 32-bit BARs may not work!\n");
 #else
-		gap_start = 0x10000000;
+		max_gap_start = 0x10000000;
 #endif
 	}
 
 	/*
 	 * e820__reserve_resources_late() protects stolen RAM already:
 	 */
-	pci_mem_start = gap_start;
+	pci_mem_start = max_gap_start;
 
 	pr_info("[gap %#010lx-%#010lx] available for PCI devices\n",
-		gap_start, gap_start + gap_size - 1);
+		max_gap_start, max_gap_start + max_gap_size - 1);
 }
 
 /*
-- 
2.45.2

[PATCH 23/32] x86/boot/e820: Simplify & clarify __e820__range_add() a bit

[Ingo Molnar]

Use 'entry_new' to make clear we are allocating a new entry.

Change the table-full message to say that the table is full.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 1eb5afbdd9e6..7c27661d0e41 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -164,16 +164,19 @@ int e820__get_entry_type(u64 start, u64 end)
 static void __init __e820__range_add(struct e820_table *table, u64 start, u64 size, enum e820_type type)
 {
 	u32 idx = table->nr_entries;
+	struct e820_entry *entry_new;
 
 	if (idx >= ARRAY_SIZE(table->entries)) {
-		pr_err("too many E820 table entries; ignoring [mem %#010llx-%#010llx]\n",
+		pr_err("E820 table full; ignoring [mem %#010llx-%#010llx]\n",
 		       start, start + size-1);
 		return;
 	}
 
-	table->entries[idx].addr = start;
-	table->entries[idx].size = size;
-	table->entries[idx].type = type;
+	entry_new = table->entries + idx;
+
+	entry_new->addr = start;
+	entry_new->size = size;
+	entry_new->type = type;
 
 	table->nr_entries++;
 }
-- 
2.45.2

[PATCH 24/32] x86/boot/e820: Standardize __init/__initdata tag placement

[Ingo Molnar]

So the e820.c file has a hodgepodge of __init and __initdata tag
placements:

    static int __init e820_search_gap(unsigned long *max_gap_start, unsigned long *max_gap_size)
    __init void e820__setup_pci_gap(void)
    __init void e820__reallocate_tables(void)
    void __init e820__memory_setup_extended(u64 phys_addr, u32 data_len)
    void __init e820__register_nosave_regions(unsigned long limit_pfn)
    static int __init e820__register_nvs_regions(void)
    u64 __init e820__memblock_alloc_reserved(u64 size, u64 align)

Standardize on the style used by e820__setup_pci_gap() and place
them before the storage class.

In addition to the consistency, as a bonus this makes the grep output
rather clean looking:

    __init void e820__range_remove(u64 start, u64 size, enum e820_type filter_type)
    __init void e820__update_table_print(void)
    __init static void e820__update_table_kexec(void)
    __init static int e820_search_gap(unsigned long *max_gap_start, unsigned long *max_gap_size)
    __init void e820__setup_pci_gap(void)
    __init void e820__reallocate_tables(void)
    __init void e820__memory_setup_extended(u64 phys_addr, u32 data_len)
    __init void e820__register_nosave_regions(unsigned long limit_pfn)
    __init static int e820__register_nvs_regions(void)

... and if one learns to just ignore the leftmost '__init' noise then
the rest of the line looks just like a regular C function definition.

With the 'mixed' tag placement style the __init tag breaks up the function's
prototype for no good reason.

Do the same for __initdata.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 92 +++++++++++++++++++++++++-------------------------
 1 file changed, 46 insertions(+), 46 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 7c27661d0e41..447f8bbb77b8 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -54,9 +54,9 @@
  * re-propagated. So its main role is a temporary bootstrap storage of firmware
  * specific memory layout data during early bootup.
  */
-static struct e820_table e820_table_init		__initdata;
-static struct e820_table e820_table_kexec_init		__initdata;
-static struct e820_table e820_table_firmware_init	__initdata;
+__initdata static struct e820_table e820_table_init;
+__initdata static struct e820_table e820_table_kexec_init;
+__initdata static struct e820_table e820_table_firmware_init;
 
 __refdata struct e820_table *e820_table			= &e820_table_init;
 __refdata struct e820_table *e820_table_kexec		= &e820_table_kexec_init;
@@ -143,7 +143,7 @@ static struct e820_entry *__e820__mapped_all(u64 start, u64 end,
 /*
  * This function checks if the entire range <start,end> is mapped with type.
  */
-bool __init e820__mapped_all(u64 start, u64 end, enum e820_type type)
+__init bool e820__mapped_all(u64 start, u64 end, enum e820_type type)
 {
 	return __e820__mapped_all(start, end, type);
 }
@@ -161,7 +161,7 @@ int e820__get_entry_type(u64 start, u64 end)
 /*
  * Add a memory region to the kernel E820 map.
  */
-static void __init __e820__range_add(struct e820_table *table, u64 start, u64 size, enum e820_type type)
+__init static void __e820__range_add(struct e820_table *table, u64 start, u64 size, enum e820_type type)
 {
 	u32 idx = table->nr_entries;
 	struct e820_entry *entry_new;
@@ -181,12 +181,12 @@ static void __init __e820__range_add(struct e820_table *table, u64 start, u64 si
 	table->nr_entries++;
 }
 
-void __init e820__range_add(u64 start, u64 size, enum e820_type type)
+__init void e820__range_add(u64 start, u64 size, enum e820_type type)
 {
 	__e820__range_add(e820_table, start, size, type);
 }
 
-static void __init e820_print_type(enum e820_type type)
+__init static void e820_print_type(enum e820_type type)
 {
 	switch (type) {
 	case E820_TYPE_RAM:		pr_cont(" System RAM");				break;
@@ -236,7 +236,7 @@ static void e820_print_size(u64 size)
 		pr_cont(" %4llu   TB", size/SZ_1T);
 }
 
-static void __init e820__print_table(const char *who)
+__init static void e820__print_table(const char *who)
 {
 	u64 range_end_prev = 0;
 	u32 idx;
@@ -343,12 +343,12 @@ struct change_member {
 	u64			addr;
 };
 
-static struct change_member	change_point_list[2*E820_MAX_ENTRIES]	__initdata;
-static struct change_member	*change_point[2*E820_MAX_ENTRIES]	__initdata;
-static struct e820_entry	*overlap_list[E820_MAX_ENTRIES]		__initdata;
-static struct e820_entry	new_entries[E820_MAX_ENTRIES]		__initdata;
+__initdata static struct change_member	change_point_list[2*E820_MAX_ENTRIES];
+__initdata static struct change_member	*change_point[2*E820_MAX_ENTRIES];
+__initdata static struct e820_entry	*overlap_list[E820_MAX_ENTRIES];
+__initdata static struct e820_entry	new_entries[E820_MAX_ENTRIES];
 
-static int __init cpcompare(const void *a, const void *b)
+__init static int cpcompare(const void *a, const void *b)
 {
 	struct change_member * const *app = a, * const *bpp = b;
 	const struct change_member *ap = *app, *bp = *bpp;
@@ -383,7 +383,7 @@ static bool e820_type_mergeable(enum e820_type type)
 	return true;
 }
 
-int __init e820__update_table(struct e820_table *table)
+__init int e820__update_table(struct e820_table *table)
 {
 	struct e820_entry *entries = table->entries;
 	u32 max_nr_entries = ARRAY_SIZE(table->entries);
@@ -483,7 +483,7 @@ int __init e820__update_table(struct e820_table *table)
 	return 0;
 }
 
-static int __init __append_e820_table(struct boot_e820_entry *entries, u32 nr_entries)
+__init static int __append_e820_table(struct boot_e820_entry *entries, u32 nr_entries)
 {
 	struct boot_e820_entry *entry = entries;
 
@@ -514,7 +514,7 @@ static int __init __append_e820_table(struct boot_e820_entry *entries, u32 nr_en
  * will have given us a memory map that we can use to properly
  * set up memory.  If we aren't, we'll fake a memory map.
  */
-static int __init append_e820_table(struct boot_e820_entry *entries, u32 nr_entries)
+__init static int append_e820_table(struct boot_e820_entry *entries, u32 nr_entries)
 {
 	/* Only one memory region (or negative)? Ignore it */
 	if (nr_entries < 2)
@@ -523,7 +523,7 @@ static int __init append_e820_table(struct boot_e820_entry *entries, u32 nr_entr
 	return __append_e820_table(entries, nr_entries);
 }
 
-static u64 __init
+__init static u64
 __e820__range_update(struct e820_table *table, u64 start, u64 size, enum e820_type old_type, enum e820_type new_type)
 {
 	u64 end;
@@ -591,19 +591,19 @@ __e820__range_update(struct e820_table *table, u64 start, u64 size, enum e820_ty
 	return real_updated_size;
 }
 
-u64 __init e820__range_update(u64 start, u64 size, enum e820_type old_type, enum e820_type new_type)
+__init u64 e820__range_update(u64 start, u64 size, enum e820_type old_type, enum e820_type new_type)
 {
 	return __e820__range_update(e820_table, start, size, old_type, new_type);
 }
 
-u64 __init e820__range_update_table(struct e820_table *t, u64 start, u64 size,
+__init u64 e820__range_update_table(struct e820_table *t, u64 start, u64 size,
 				    enum e820_type old_type, enum e820_type new_type)
 {
 	return __e820__range_update(t, start, size, old_type, new_type);
 }
 
 /* Remove a range of memory from the E820 table: */
-u64 __init e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type)
+__init u64 e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type)
 {
 	u32 idx;
 	u64 end;
@@ -664,7 +664,7 @@ u64 __init e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool
 	return real_removed_size;
 }
 
-void __init e820__update_table_print(void)
+__init void e820__update_table_print(void)
 {
 	if (e820__update_table(e820_table))
 		return;
@@ -673,7 +673,7 @@ void __init e820__update_table_print(void)
 	e820__print_table("modified");
 }
 
-static void __init e820__update_table_kexec(void)
+__init static void e820__update_table_kexec(void)
 {
 	e820__update_table(e820_table_kexec);
 }
@@ -683,7 +683,7 @@ static void __init e820__update_table_kexec(void)
 /*
  * Search for a gap in the E820 memory space from 0 to MAX_GAP_END (4GB).
  */
-static int __init e820_search_gap(unsigned long *max_gap_start, unsigned long *max_gap_size)
+__init static int e820_search_gap(unsigned long *max_gap_start, unsigned long *max_gap_size)
 {
 	u64 last = MAX_GAP_END;
 	int idx = e820_table->nr_entries;
@@ -786,7 +786,7 @@ __init void e820__reallocate_tables(void)
  * the remaining (if any) entries are passed via the SETUP_E820_EXT node of
  * struct setup_data, which is parsed here.
  */
-void __init e820__memory_setup_extended(u64 phys_addr, u32 data_len)
+__init void e820__memory_setup_extended(u64 phys_addr, u32 data_len)
 {
 	int entries;
 	struct boot_e820_entry *extmap;
@@ -815,7 +815,7 @@ void __init e820__memory_setup_extended(u64 phys_addr, u32 data_len)
  * This function requires the E820 map to be sorted and without any
  * overlapping entries.
  */
-void __init e820__register_nosave_regions(unsigned long limit_pfn)
+__init void e820__register_nosave_regions(unsigned long limit_pfn)
 {
 	u32 idx;
 	u64 last_addr = 0;
@@ -840,7 +840,7 @@ void __init e820__register_nosave_regions(unsigned long limit_pfn)
  * Register ACPI NVS memory regions, so that we can save/restore them during
  * hibernation and the subsequent resume:
  */
-static int __init e820__register_nvs_regions(void)
+__init static int e820__register_nvs_regions(void)
 {
 	u32 idx;
 
@@ -864,7 +864,7 @@ core_initcall(e820__register_nvs_regions);
  * This allows kexec to fake a new mptable, as if it came from the real
  * system.
  */
-u64 __init e820__memblock_alloc_reserved(u64 size, u64 align)
+__init u64 e820__memblock_alloc_reserved(u64 size, u64 align)
 {
 	u64 addr;
 
@@ -891,7 +891,7 @@ u64 __init e820__memblock_alloc_reserved(u64 size, u64 align)
 /*
  * Find the highest page frame number we have available
  */
-static unsigned long __init e820__end_ram_pfn(unsigned long limit_pfn)
+__init static unsigned long e820__end_ram_pfn(unsigned long limit_pfn)
 {
 	u32 idx;
 	unsigned long last_pfn = 0;
@@ -927,20 +927,20 @@ static unsigned long __init e820__end_ram_pfn(unsigned long limit_pfn)
 	return last_pfn;
 }
 
-unsigned long __init e820__end_of_ram_pfn(void)
+__init unsigned long e820__end_of_ram_pfn(void)
 {
 	return e820__end_ram_pfn(MAX_ARCH_PFN);
 }
 
-unsigned long __init e820__end_of_low_ram_pfn(void)
+__init unsigned long e820__end_of_low_ram_pfn(void)
 {
 	return e820__end_ram_pfn(1UL << (32 - PAGE_SHIFT));
 }
 
-static int userdef __initdata;
+__initdata static int userdef;
 
 /* The "mem=nopentium" boot option disables 4MB page tables on 32-bit kernels: */
-static int __init parse_memopt(char *p)
+__init static int parse_memopt(char *p)
 {
 	u64 mem_size;
 
@@ -974,7 +974,7 @@ static int __init parse_memopt(char *p)
 }
 early_param("mem", parse_memopt);
 
-static int __init parse_memmap_one(char *p)
+__init static int parse_memmap_one(char *p)
 {
 	char *oldp;
 	u64 start_at, mem_size;
@@ -1031,7 +1031,7 @@ static int __init parse_memmap_one(char *p)
 	return *p == '\0' ? 0 : -EINVAL;
 }
 
-static int __init parse_memmap_opt(char *str)
+__init static int parse_memmap_opt(char *str)
 {
 	while (str) {
 		char *k = strchr(str, ',');
@@ -1052,7 +1052,7 @@ early_param("memmap", parse_memmap_opt);
  * have been processed, in which case we already have an E820 table filled in
  * via the parameter callback function(s), but it's not sorted and printed yet:
  */
-void __init e820__finish_early_params(void)
+__init void e820__finish_early_params(void)
 {
 	if (userdef) {
 		if (e820__update_table(e820_table) < 0)
@@ -1063,7 +1063,7 @@ void __init e820__finish_early_params(void)
 	}
 }
 
-static const char *__init e820_type_to_string(struct e820_entry *entry)
+__init static const char * e820_type_to_string(struct e820_entry *entry)
 {
 	switch (entry->type) {
 	case E820_TYPE_RAM:		return "System RAM";
@@ -1078,7 +1078,7 @@ static const char *__init e820_type_to_string(struct e820_entry *entry)
 	}
 }
 
-static unsigned long __init e820_type_to_iomem_type(struct e820_entry *entry)
+__init static unsigned long e820_type_to_iomem_type(struct e820_entry *entry)
 {
 	switch (entry->type) {
 	case E820_TYPE_RAM:		return IORESOURCE_SYSTEM_RAM;
@@ -1093,7 +1093,7 @@ static unsigned long __init e820_type_to_iomem_type(struct e820_entry *entry)
 	}
 }
 
-static unsigned long __init e820_type_to_iores_desc(struct e820_entry *entry)
+__init static unsigned long e820_type_to_iores_desc(struct e820_entry *entry)
 {
 	switch (entry->type) {
 	case E820_TYPE_ACPI:		return IORES_DESC_ACPI_TABLES;
@@ -1111,13 +1111,13 @@ static unsigned long __init e820_type_to_iores_desc(struct e820_entry *entry)
 /*
  * We assign one resource entry for each E820 map entry:
  */
-static struct resource __initdata *e820_res;
+__initdata static struct resource *e820_res;
 
 /*
  * Is this a device address region that should not be marked busy?
  * (Versus system address regions that we register & lock early.)
  */
-static bool __init e820_device_region(enum e820_type type, struct resource *res)
+__init static bool e820_device_region(enum e820_type type, struct resource *res)
 {
 	/* This is the legacy BIOS/DOS ROM-shadow + MMIO region: */
 	if (res->start < (1ULL<<20))
@@ -1146,7 +1146,7 @@ static bool __init e820_device_region(enum e820_type type, struct resource *res)
 /*
  * Mark E820 system regions as busy for the resource manager:
  */
-void __init e820__reserve_resources(void)
+__init void e820__reserve_resources(void)
 {
 	u32 idx;
 	struct resource *res;
@@ -1193,7 +1193,7 @@ void __init e820__reserve_resources(void)
 /*
  * How much should we pad the end of RAM, depending on where it is?
  */
-static unsigned long __init ram_alignment(resource_size_t pos)
+__init static unsigned long ram_alignment(resource_size_t pos)
 {
 	unsigned long mb = pos >> 20;
 
@@ -1211,7 +1211,7 @@ static unsigned long __init ram_alignment(resource_size_t pos)
 
 #define MAX_RESOURCE_SIZE ((resource_size_t)-1)
 
-void __init e820__reserve_resources_late(void)
+__init void e820__reserve_resources_late(void)
 {
 	u32 idx;
 	struct resource *res;
@@ -1261,7 +1261,7 @@ void __init e820__reserve_resources_late(void)
 /*
  * Pass the firmware (bootloader) E820 map to the kernel and process it:
  */
-char *__init e820__memory_setup_default(void)
+__init char * e820__memory_setup_default(void)
 {
 	char *who = "BIOS-e820";
 
@@ -1299,7 +1299,7 @@ char *__init e820__memory_setup_default(void)
  * E820 map - with an optional platform quirk available for virtual platforms
  * to override this method of boot environment processing:
  */
-void __init e820__memory_setup(void)
+__init void e820__memory_setup(void)
 {
 	char *who;
 
@@ -1315,7 +1315,7 @@ void __init e820__memory_setup(void)
 	e820__print_table(who);
 }
 
-void __init e820__memblock_setup(void)
+__init void e820__memblock_setup(void)
 {
 	u32 idx;
 	u64 end;
-- 
2.45.2

[PATCH 25/32] x86/boot/e820: Simplify append_e820_table() and remove restriction on single-entry tables

[Ingo Molnar]

So append_e820_table() begins with this weird condition that checks 'nr_entries':

    static int __init append_e820_table(struct boot_e820_entry *entries, u32 nr_entries)
    {
            /* Only one memory region (or negative)? Ignore it */
            if (nr_entries < 2)
                    return -1;

Firstly, 'nr_entries' has been an u32 since 2017 and cannot be negative.

Secondly, there's nothing inherently wrong with single-entry E820 maps,
especially in virtualized environments.

So remove this restriction and remove the __append_e820_table()
indirection.

Also:

 - fix/update comments
 - remove obsolete comments

This shrinks the generated code a bit as well:

   text       data        bss        dec        hex    filename
   7549      44072          0      51621       c9a5    e820.o.before
   7533      44072          0      51605       c995    e820.o.after

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 35 +++++++++++------------------------
 1 file changed, 11 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 447f8bbb77b8..22bfcad7b723 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -483,17 +483,22 @@ __init int e820__update_table(struct e820_table *table)
 	return 0;
 }
 
-__init static int __append_e820_table(struct boot_e820_entry *entries, u32 nr_entries)
+/*
+ * Copy the BIOS E820 map into the kernel's e820_table.
+ *
+ * Sanity-check it while we're at it..
+ */
+__init static int append_e820_table(struct boot_e820_entry *entries, u32 nr_entries)
 {
 	struct boot_e820_entry *entry = entries;
 
 	while (nr_entries) {
 		u64 start = entry->addr;
-		u64 size = entry->size;
-		u64 end = start + size - 1;
-		u32 type = entry->type;
+		u64 size  = entry->size;
+		u64 end   = start + size-1;
+		u32 type  = entry->type;
 
-		/* Ignore the entry on 64-bit overflow: */
+		/* Ignore the remaining entries on 64-bit overflow: */
 		if (start > end && likely(size))
 			return -1;
 
@@ -505,24 +510,6 @@ __init static int __append_e820_table(struct boot_e820_entry *entries, u32 nr_en
 	return 0;
 }
 
-/*
- * Copy the BIOS E820 map into a safe place.
- *
- * Sanity-check it while we're at it..
- *
- * If we're lucky and live on a modern system, the setup code
- * will have given us a memory map that we can use to properly
- * set up memory.  If we aren't, we'll fake a memory map.
- */
-__init static int append_e820_table(struct boot_e820_entry *entries, u32 nr_entries)
-{
-	/* Only one memory region (or negative)? Ignore it */
-	if (nr_entries < 2)
-		return -1;
-
-	return __append_e820_table(entries, nr_entries);
-}
-
 __init static u64
 __e820__range_update(struct e820_table *table, u64 start, u64 size, enum e820_type old_type, enum e820_type new_type)
 {
@@ -796,7 +783,7 @@ __init void e820__memory_setup_extended(u64 phys_addr, u32 data_len)
 	entries = sdata->len / sizeof(*extmap);
 	extmap = (struct boot_e820_entry *)(sdata->data);
 
-	__append_e820_table(extmap, entries);
+	append_e820_table(extmap, entries);
 	e820__update_table(e820_table);
 
 	memcpy(e820_table_kexec, e820_table, sizeof(*e820_table_kexec));
-- 
2.45.2

Re: [PATCH 25/32] x86/boot/e820: Simplify append_e820_table() and remove restriction on single-entry tables

[Nikolay Borisov]

On 5/15/25 15:05, Ingo Molnar wrote:
> So append_e820_table() begins with this weird condition that checks 'nr_entries':
> 
>      static int __init append_e820_table(struct boot_e820_entry *entries, u32 nr_entries)
>      {
>              /* Only one memory region (or negative)? Ignore it */
>              if (nr_entries < 2)
>                      return -1;
> 
> Firstly, 'nr_entries' has been an u32 since 2017 and cannot be negative.
> 
> Secondly, there's nothing inherently wrong with single-entry E820 maps,
> especially in virtualized environments.
> 
> So remove this restriction and remove the __append_e820_table()
> indirection.
> 
> Also:
> 
>   - fix/update comments
>   - remove obsolete comments
> 
> This shrinks the generated code a bit as well:
> 
>     text       data        bss        dec        hex    filename
>     7549      44072          0      51621       c9a5    e820.o.before
>     7533      44072          0      51605       c995    e820.o.after
> 
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: Arnd Bergmann <arnd@kernel.org>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>   arch/x86/kernel/e820.c | 35 +++++++++++------------------------
>   1 file changed, 11 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 447f8bbb77b8..22bfcad7b723 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -483,17 +483,22 @@ __init int e820__update_table(struct e820_table *table)
>   	return 0;
>   }
>   
> -__init static int __append_e820_table(struct boot_e820_entry *entries, u32 nr_entries)
> +/*
> + * Copy the BIOS E820 map into the kernel's e820_table.
> + *
> + * Sanity-check it while we're at it..
> + */
> +__init static int append_e820_table(struct boot_e820_entry *entries, u32 nr_entries)
>   {
>   	struct boot_e820_entry *entry = entries;
>   
>   	while (nr_entries) {
>   		u64 start = entry->addr;
> -		u64 size = entry->size;
> -		u64 end = start + size - 1;
> -		u32 type = entry->type;
> +		u64 size  = entry->size;
> +		u64 end   = start + size-1;
> +		u32 type  = entry->type;
>   
> -		/* Ignore the entry on 64-bit overflow: */
> +		/* Ignore the remaining entries on 64-bit overflow: */
>   		if (start > end && likely(size))
>   			return -1;

nit: All this function wants to do is simply iterate an array, I'd say
the standard way to iterate an array is to use a 'for' loop.

How about doing this instead (on top of this patch):

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 22bfcad7b723..41aa26eae594 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -490,9 +490,9 @@ __init int e820__update_table(struct e820_table *table)
   */
  __init static int append_e820_table(struct boot_e820_entry *entries, u32 nr_entries)
  {
-       struct boot_e820_entry *entry = entries;
  
-       while (nr_entries) {
+       for (int i = 0; i < nr_entries; i++) {
+               struct boot_e820_entry *entry = entries + i;
                 u64 start = entry->addr;
                 u64 size  = entry->size;
                 u64 end   = start + size-1;
@@ -503,9 +503,6 @@ __init static int append_e820_table(struct boot_e820_entry *entries, u32 nr_entr
                         return -1;
  
                 e820__range_add(start, size, type);
-
-               entry++;
-               nr_entries--;
         }
         return 0;
  }




>   
> @@ -505,24 +510,6 @@ __init static int __append_e820_table(struct boot_e820_entry *entries, u32 nr_en
>   	return 0;
>   }
>   
> -/*
> - * Copy the BIOS E820 map into a safe place.
> - *
> - * Sanity-check it while we're at it..
> - *
> - * If we're lucky and live on a modern system, the setup code
> - * will have given us a memory map that we can use to properly
> - * set up memory.  If we aren't, we'll fake a memory map.
> - */
> -__init static int append_e820_table(struct boot_e820_entry *entries, u32 nr_entries)
> -{
> -	/* Only one memory region (or negative)? Ignore it */
> -	if (nr_entries < 2)
> -		return -1;
> -
> -	return __append_e820_table(entries, nr_entries);
> -}
> -
>   __init static u64
>   __e820__range_update(struct e820_table *table, u64 start, u64 size, enum e820_type old_type, enum e820_type new_type)
>   {
> @@ -796,7 +783,7 @@ __init void e820__memory_setup_extended(u64 phys_addr, u32 data_len)
>   	entries = sdata->len / sizeof(*extmap);
>   	extmap = (struct boot_e820_entry *)(sdata->data);
>   
> -	__append_e820_table(extmap, entries);
> +	append_e820_table(extmap, entries);
>   	e820__update_table(e820_table);
>   
>   	memcpy(e820_table_kexec, e820_table, sizeof(*e820_table_kexec));

Re: [PATCH 25/32] x86/boot/e820: Simplify append_e820_table() and remove restriction on single-entry tables

[Andy Shevchenko]

On Mon, Jun 02, 2025 at 03:50:24PM +0300, Nikolay Borisov wrote:
> On 5/15/25 15:05, Ingo Molnar wrote:

...

> How about doing this instead (on top of this patch):

> +       for (int i = 0; i < nr_entries; i++) {

This would be nice when defined as u32.

>         }

-- 
With Best Regards,
Andy Shevchenko

[PATCH 26/32] x86/boot/e820: Remove e820__range_remove()'s unused return parameter

[Ingo Molnar]

None of the usage sites make use of the 'real_removed_size'
return parameter of e820__range_remove(), and it's hard
to contemplate much constructive use: E820 maps can have
holes, and removing a fixed range may result in removal
of any number of bytes from 0 to the requested size.

So remove this pointless calculation. This simplifies
the function a bit:

   text       data        bss        dec        hex    filename
   7645      44072          0      51717       ca05    e820.o.before
   7597      44072          0      51669       c9d5    e820.o.after

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/include/asm/e820/api.h | 2 +-
 arch/x86/kernel/e820.c          | 8 +-------
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/e820/api.h b/arch/x86/include/asm/e820/api.h
index 54427b77bc19..9cf416f7a84f 100644
--- a/arch/x86/include/asm/e820/api.h
+++ b/arch/x86/include/asm/e820/api.h
@@ -16,7 +16,7 @@ extern bool e820__mapped_all(u64 start, u64 end, enum e820_type type);
 
 extern void e820__range_add   (u64 start, u64 size, enum e820_type type);
 extern u64  e820__range_update(u64 start, u64 size, enum e820_type old_type, enum e820_type new_type);
-extern u64  e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type);
+extern void e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type);
 extern u64  e820__range_update_table(struct e820_table *t, u64 start, u64 size, enum e820_type old_type, enum e820_type new_type);
 
 extern int  e820__update_table(struct e820_table *table);
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 22bfcad7b723..0fc77ab72c5f 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -590,11 +590,10 @@ __init u64 e820__range_update_table(struct e820_table *t, u64 start, u64 size,
 }
 
 /* Remove a range of memory from the E820 table: */
-__init u64 e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type)
+__init void e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type)
 {
 	u32 idx;
 	u64 end;
-	u64 real_removed_size = 0;
 
 	if (size > (ULLONG_MAX - start))
 		size = ULLONG_MAX - start;
@@ -617,7 +616,6 @@ __init u64 e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool
 
 		/* Completely covered? */
 		if (entry->addr >= start && entry_end <= end) {
-			real_removed_size += entry->size;
 			memset(entry, 0, sizeof(*entry));
 			continue;
 		}
@@ -626,7 +624,6 @@ __init u64 e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool
 		if (entry->addr < start && entry_end > end) {
 			e820__range_add(end, entry_end - end, entry->type);
 			entry->size = start - entry->addr;
-			real_removed_size += size;
 			continue;
 		}
 
@@ -636,8 +633,6 @@ __init u64 e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool
 		if (final_start >= final_end)
 			continue;
 
-		real_removed_size += final_end - final_start;
-
 		/*
 		 * Left range could be head or tail, so need to update
 		 * the size first:
@@ -648,7 +643,6 @@ __init u64 e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool
 
 		entry->addr = final_end;
 	}
-	return real_removed_size;
 }
 
 __init void e820__update_table_print(void)
-- 
2.45.2

[PATCH 27/32] x86/boot/e820: Simplify the e820__range_remove() API

[Ingo Molnar]

Right now e820__range_remove() has two parameters to control the
E820 type of the range removed:

	extern void e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type);

Since E820 types start at 1, zero has a natural meaning of 'no type.

Consolidate the (old_type,check_type) parameters into a single (filter_type)
parameter:

	extern void e820__range_remove(u64 start, u64 size, enum e820_type filter_type);

Note that both e820__mapped_raw_any() and e820__mapped_any()
already have such semantics for their 'type' parameter, although
it's currently not used with '0' by in-kernel code.

Also, the __e820__mapped_all() internal helper already has such
semantics implemented as well, and the e820__get_entry_type() API
uses the '0' type to such effect.

This simplifies not just e820__range_remove(), and synchronizes its
use of type filters with other E820 API functions, but simplifies
usage sites as well, such as parse_memmap_one(), beyond the reduction
of the number of parameters:

  -               else if (from)
  -                       e820__range_remove(start_at, mem_size, from, 1);
                  else
  -                       e820__range_remove(start_at, mem_size, 0, 0);
  +                       e820__range_remove(start_at, mem_size, from);

The generated code gets smaller as well:

	add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-66 (-66)

	Function                                     old     new   delta
	parse_memopt                                 112     107      -5
	efi_init                                    1048    1039      -9
	setup_arch                                  2719    2709     -10
	e820__range_remove                           283     273     -10
	parse_memmap_opt                             559     527     -32

	Total: Before=22,675,600, After=22,675,534, chg -0.00%

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/include/asm/e820/api.h |  2 +-
 arch/x86/kernel/e820.c          | 16 +++++++---------
 arch/x86/kernel/setup.c         |  4 ++--
 arch/x86/platform/efi/efi.c     |  3 +--
 4 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/e820/api.h b/arch/x86/include/asm/e820/api.h
index 9cf416f7a84f..bbe0c8de976c 100644
--- a/arch/x86/include/asm/e820/api.h
+++ b/arch/x86/include/asm/e820/api.h
@@ -16,7 +16,7 @@ extern bool e820__mapped_all(u64 start, u64 end, enum e820_type type);
 
 extern void e820__range_add   (u64 start, u64 size, enum e820_type type);
 extern u64  e820__range_update(u64 start, u64 size, enum e820_type old_type, enum e820_type new_type);
-extern void e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type);
+extern void e820__range_remove(u64 start, u64 size, enum e820_type filter_type);
 extern u64  e820__range_update_table(struct e820_table *t, u64 start, u64 size, enum e820_type old_type, enum e820_type new_type);
 
 extern int  e820__update_table(struct e820_table *table);
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 0fc77ab72c5f..0d7e9794cd52 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -590,7 +590,7 @@ __init u64 e820__range_update_table(struct e820_table *t, u64 start, u64 size,
 }
 
 /* Remove a range of memory from the E820 table: */
-__init void e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type)
+__init void e820__range_remove(u64 start, u64 size, enum e820_type filter_type)
 {
 	u32 idx;
 	u64 end;
@@ -600,8 +600,8 @@ __init void e820__range_remove(u64 start, u64 size, enum e820_type old_type, boo
 
 	end = start + size;
 	printk(KERN_DEBUG "e820: remove [mem %#010Lx-%#010Lx]", start, end - 1);
-	if (check_type)
-		e820_print_type(old_type);
+	if (filter_type)
+		e820_print_type(filter_type);
 	pr_cont("\n");
 
 	for (idx = 0; idx < e820_table->nr_entries; idx++) {
@@ -609,7 +609,7 @@ __init void e820__range_remove(u64 start, u64 size, enum e820_type old_type, boo
 		u64 final_start, final_end;
 		u64 entry_end;
 
-		if (check_type && entry->type != old_type)
+		if (filter_type && entry->type != filter_type)
 			continue;
 
 		entry_end = entry->addr + entry->size;
@@ -945,7 +945,7 @@ __init static int parse_memopt(char *p)
 	if (mem_size == 0)
 		return -EINVAL;
 
-	e820__range_remove(mem_size, ULLONG_MAX - mem_size, E820_TYPE_RAM, 1);
+	e820__range_remove(mem_size, ULLONG_MAX - mem_size, E820_TYPE_RAM);
 
 #ifdef CONFIG_MEMORY_HOTPLUG
 	max_mem_size = mem_size;
@@ -1001,12 +1001,10 @@ __init static int parse_memmap_one(char *p)
 			e820__range_update(start_at, mem_size, from, to);
 		else if (to)
 			e820__range_add(start_at, mem_size, to);
-		else if (from)
-			e820__range_remove(start_at, mem_size, from, 1);
 		else
-			e820__range_remove(start_at, mem_size, 0, 0);
+			e820__range_remove(start_at, mem_size, from);
 	} else {
-		e820__range_remove(mem_size, ULLONG_MAX - mem_size, E820_TYPE_RAM, 1);
+		e820__range_remove(mem_size, ULLONG_MAX - mem_size, E820_TYPE_RAM);
 	}
 
 	return *p == '\0' ? 0 : -EINVAL;
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f40dffc3e014..72ac61c58ec8 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -732,7 +732,7 @@ static void __init trim_bios_range(void)
 	 * area (640Kb -> 1Mb) as RAM even though it is not.
 	 * take them out.
 	 */
-	e820__range_remove(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_TYPE_RAM, 1);
+	e820__range_remove(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_TYPE_RAM);
 
 	e820__update_table(e820_table);
 }
@@ -754,7 +754,7 @@ static void __init e820_add_kernel_range(void)
 		return;
 
 	pr_warn(".text .data .bss are not marked as E820_TYPE_RAM!\n");
-	e820__range_remove(start, size, E820_TYPE_RAM, 0);
+	e820__range_remove(start, size, 0);
 	e820__range_add(start, size, E820_TYPE_RAM);
 }
 
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 463b784499a8..d00c6de7f3b7 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -333,8 +333,7 @@ static void __init efi_remove_e820_mmio(void)
 			if (size >= 256*1024) {
 				pr_info("Remove mem%02u: MMIO range=[0x%08llx-0x%08llx] (%lluMB) from e820 map\n",
 					i, start, end, size >> 20);
-				e820__range_remove(start, size,
-						   E820_TYPE_RESERVED, 1);
+				e820__range_remove(start, size, E820_TYPE_RESERVED);
 			} else {
 				pr_info("Not removing mem%02u: MMIO range=[0x%08llx-0x%08llx] (%lluKB) from e820 map\n",
 					i, start, end, size >> 10);
-- 
2.45.2

[PATCH 28/32] x86/boot/e820: Make sure e820_search_gap() finds all gaps

[Ingo Molnar]

The current implementation of e820_search_gap() searches gaps
in a reverse search from MAX_GAP_END back to 0, contrary to
what its main comment claims:

    * Search for a gap in the E820 memory space from 0 to MAX_GAP_END (4GB).

But gaps can not only be beyond E820 RAM ranges, they can be below
them as well. For example this function will not find the proper
PCI gap for simplified memory map layouts that have a single RAM
range that crosses the 4GB boundary.

Rework the function to have a proper forward search of
E820 table entries.

This makes the code somewhat bigger:

   text       data        bss        dec        hex    filename
   7613      44072          0      51685       c9e5    e820.o.before
   7645      44072          0      51717       ca05    e820.o.after

but it now both implements what it claims to do, and is more
straightforward to read.

( This also allows 'idx' to be the regular u32 again, not an 'int'
  underflowing to -1. )

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 59 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 41 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 0d7e9794cd52..5260ce6ad466 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -666,30 +666,52 @@ __init static void e820__update_table_kexec(void)
  */
 __init static int e820_search_gap(unsigned long *max_gap_start, unsigned long *max_gap_size)
 {
-	u64 last = MAX_GAP_END;
-	int idx = e820_table->nr_entries;
+	struct e820_entry *entry;
+	u64 range_end_prev = 0;
 	int found = 0;
+	u32 idx;
 
-	while (--idx >= 0) {
-		u64 start = e820_table->entries[idx].addr;
-		u64 end = start + e820_table->entries[idx].size;
+	for (idx = 0; idx < e820_table->nr_entries; idx++) {
+		u64 range_start, range_end;
 
-		/*
-		 * Since "last" is at most 4GB, we know we'll
-		 * fit in 32 bits if this condition is true:
-		 */
-		if (last > end) {
-			unsigned long gap = last - end;
+		entry = e820_table->entries + idx;
+		range_start = entry->addr;
+		range_end   = entry->addr + entry->size;
 
-			if (gap > *max_gap_size) {
-				*max_gap_size = gap;
-				*max_gap_start = end;
-				found = 1;
+		/* Process any gap before this entry: */
+		if (range_start > range_end_prev) {
+			u64 gap_start = range_end_prev;
+			u64 gap_end = range_start;
+			u64 gap_size;
+
+			if (gap_start < MAX_GAP_END) {
+				/* Make sure the entirety of the gap is below MAX_GAP_END: */
+				gap_end = min(gap_end, MAX_GAP_END);
+				gap_size = gap_end-gap_start;
+
+				if (gap_size >= *max_gap_size) {
+					*max_gap_start = gap_start;
+					*max_gap_size = gap_size;
+					found = 1;
+				}
 			}
 		}
-		if (start < last)
-			last = start;
+
+		range_end_prev = range_end;
+	}
+
+	/* Is there a usable gap beyond the last entry: */
+	if (entry->addr + entry->size < MAX_GAP_END) {
+		u64 gap_start = entry->addr + entry->size;
+		u64 gap_size = MAX_GAP_END-gap_start;
+
+		if (gap_size >= *max_gap_size) {
+			*max_gap_start = gap_start;
+			*max_gap_size = gap_size;
+			found = 1;
+		}
 	}
+
 	return found;
 }
 
@@ -706,6 +728,7 @@ __init void e820__setup_pci_gap(void)
 	unsigned long max_gap_start, max_gap_size;
 	int found;
 
+	/* The minimum eligible gap size is 4MB: */
 	max_gap_size = SZ_4M;
 	found  = e820_search_gap(&max_gap_start, &max_gap_size);
 
@@ -725,7 +748,7 @@ __init void e820__setup_pci_gap(void)
 	pci_mem_start = max_gap_start;
 
 	pr_info("[gap %#010lx-%#010lx] available for PCI devices\n",
-		max_gap_start, max_gap_start + max_gap_size - 1);
+		max_gap_start, max_gap_start + max_gap_size-1);
 }
 
 /*
-- 
2.45.2

Re: [PATCH 28/32] x86/boot/e820: Make sure e820_search_gap() finds all gaps

[Nikolay Borisov]

On 5/15/25 15:05, Ingo Molnar wrote:
> The current implementation of e820_search_gap() searches gaps
> in a reverse search from MAX_GAP_END back to 0, contrary to
> what its main comment claims:
> 
>      * Search for a gap in the E820 memory space from 0 to MAX_GAP_END (4GB).
> 
> But gaps can not only be beyond E820 RAM ranges, they can be below
> them as well. For example this function will not find the proper
> PCI gap for simplified memory map layouts that have a single RAM
> range that crosses the 4GB boundary.
> 
> Rework the function to have a proper forward search of
> E820 table entries.
> 
> This makes the code somewhat bigger:
> 
>     text       data        bss        dec        hex    filename
>     7613      44072          0      51685       c9e5    e820.o.before
>     7645      44072          0      51717       ca05    e820.o.after
> 
> but it now both implements what it claims to do, and is more
> straightforward to read.
> 
> ( This also allows 'idx' to be the regular u32 again, not an 'int'
>    underflowing to -1. )
> 
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: Arnd Bergmann <arnd@kernel.org>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>   arch/x86/kernel/e820.c | 59 +++++++++++++++++++++++++++++++++++---------------
>   1 file changed, 41 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 0d7e9794cd52..5260ce6ad466 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -666,30 +666,52 @@ __init static void e820__update_table_kexec(void)
>    */
>   __init static int e820_search_gap(unsigned long *max_gap_start, unsigned long *max_gap_size)
>   {


Also it's not really searching for 'a gap' but rather it searched for a 
specific gap - one larger than the passed max_gap_size. So I wonder if 
find_gap would be a more apt name, given that you have a specific 
criterion you are searching against, the gap size. If you think I'm 
reading too much into it then it's fine to disregard this.


nit: Also this function ought to return boolean.

<snip>

[PATCH 29/32] x86/boot/e820: Introduce E820_TYPE_13 and treat it as a device region

[Ingo Molnar]

Paul Menzel pointed out that ACPI specification 6.3 defines 'reserved'
E820 region types as E820_TYPE_RESERVED (type 2):

 > Table 15-374 *Address Range Types* in the ACPI specification 6.3 says:
 >
 > > Reserved for future use. OSPM must treat any range of this type as if
 > > the type returned was AddressRangeReserved.

This has relevance for device address regions, which on some firmware such
as CoreBoot, get passed to Linux as type-13 - which the kernel
treats as system regions and registers them as unavailable to drivers:

	static bool __init e820_device_region(enum e820_type type, struct resource *res)

	...

        case E820_TYPE_ACPI:
        case E820_TYPE_NVS:
        case E820_TYPE_UNUSABLE:
        default:
                return false;

Users of such systems will see device breakage on Linux, which they
have to work around with iomem=relaxed kind of boot time hacks to
turn off resource conflict checking.

Partially follow the ACPI spec and add a limited quirk for the
E820_TYPE_13 type, and allow it to be claimed by device drivers
(similarly to E820_TYPE_RESERVED).

Don't change behavior for other unknown types.

Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
Suggested-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/include/asm/e820/types.h |  4 ++++
 arch/x86/kernel/e820.c            | 11 +++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/e820/types.h b/arch/x86/include/asm/e820/types.h
index df12f7ee75d3..2430120c2528 100644
--- a/arch/x86/include/asm/e820/types.h
+++ b/arch/x86/include/asm/e820/types.h
@@ -27,6 +27,10 @@ enum e820_type {
 	 *   6 was assigned differently. Some time they will learn... )
 	 */
 	E820_TYPE_PRAM		= 12,
+	/*
+	 * Certain firmware such as CoreBoot uses this type:
+	 */
+	E820_TYPE_13		= 13,
 
 	/*
 	 * Special-purpose memory is indicated to the system via the
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 5260ce6ad466..6c9c00ce8db9 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1075,7 +1075,7 @@ __init static const char * e820_type_to_string(struct e820_entry *entry)
 	case E820_TYPE_PRAM:		return "Persistent Memory (legacy)";
 	case E820_TYPE_PMEM:		return "Persistent Memory";
 	case E820_TYPE_RESERVED:	return "Reserved";
-	case E820_TYPE_SOFT_RESERVED:	return "Soft Reserved";
+	case E820_TYPE_13:		return "Type 13";
 	default:			return "Unknown E820 type";
 	}
 }
@@ -1090,6 +1090,7 @@ __init static unsigned long e820_type_to_iomem_type(struct e820_entry *entry)
 	case E820_TYPE_PRAM:		/* Fall-through: */
 	case E820_TYPE_PMEM:		/* Fall-through: */
 	case E820_TYPE_RESERVED:	/* Fall-through: */
+	case E820_TYPE_13:		/* Fall-through: */
 	case E820_TYPE_SOFT_RESERVED:	/* Fall-through: */
 	default:			return IORESOURCE_MEM;
 	}
@@ -1102,7 +1103,8 @@ __init static unsigned long e820_type_to_iores_desc(struct e820_entry *entry)
 	case E820_TYPE_NVS:		return IORES_DESC_ACPI_NV_STORAGE;
 	case E820_TYPE_PMEM:		return IORES_DESC_PERSISTENT_MEMORY;
 	case E820_TYPE_PRAM:		return IORES_DESC_PERSISTENT_MEMORY_LEGACY;
-	case E820_TYPE_RESERVED:	return IORES_DESC_RESERVED;
+	case E820_TYPE_RESERVED:	/* Fall-through: */
+	case E820_TYPE_13:		return IORES_DESC_RESERVED;
 	case E820_TYPE_SOFT_RESERVED:	return IORES_DESC_SOFT_RESERVED;
 	case E820_TYPE_RAM:		/* Fall-through: */
 	case E820_TYPE_UNUSABLE:	/* Fall-through: */
@@ -1132,6 +1134,7 @@ __init static bool e820_device_region(enum e820_type type, struct resource *res)
 	 */
 	switch (type) {
 	case E820_TYPE_RESERVED:
+	case E820_TYPE_13:
 	case E820_TYPE_SOFT_RESERVED:
 	case E820_TYPE_PRAM:
 	case E820_TYPE_PMEM:
@@ -1140,6 +1143,10 @@ __init static bool e820_device_region(enum e820_type type, struct resource *res)
 	case E820_TYPE_ACPI:
 	case E820_TYPE_NVS:
 	case E820_TYPE_UNUSABLE:
+	/*
+	 * Unknown E820 types should be treated passively, here we
+	 * don't allow them to be claimed by device drivers:
+	 */
 	default:
 		return false;
 	}
-- 
2.45.2

[PATCH 30/32] x86/boot/e820: Change e820_type_to_string() to take a 'type' parameter

[Ingo Molnar]

Simplify the e820_type_to_string() interface by changing it
to take a 'enum e820_type type' parameter.

This is going to allow the unification of the e820_type_to_string()
and e820_print_type() functions.

No change in functionality intended.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 6c9c00ce8db9..fe3e078a4064 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1065,9 +1065,9 @@ __init void e820__finish_early_params(void)
 	}
 }
 
-__init static const char * e820_type_to_string(struct e820_entry *entry)
+__init static const char * e820_type_to_string(enum e820_type type)
 {
-	switch (entry->type) {
+	switch (type) {
 	case E820_TYPE_RAM:		return "System RAM";
 	case E820_TYPE_ACPI:		return "ACPI Tables";
 	case E820_TYPE_NVS:		return "ACPI Non-volatile Storage";
@@ -1175,7 +1175,7 @@ __init void e820__reserve_resources(void)
 		}
 		res->start = entry->addr;
 		res->end   = end;
-		res->name  = e820_type_to_string(entry);
+		res->name  = e820_type_to_string(entry->type);
 		res->flags = e820_type_to_iomem_type(entry);
 		res->desc  = e820_type_to_iores_desc(entry);
 
@@ -1195,7 +1195,7 @@ __init void e820__reserve_resources(void)
 	for (idx = 0; idx < e820_table_kexec->nr_entries; idx++) {
 		struct e820_entry *entry = e820_table_kexec->entries + idx;
 
-		firmware_map_add_early(entry->addr, entry->addr + entry->size, e820_type_to_string(entry));
+		firmware_map_add_early(entry->addr, entry->addr + entry->size, e820_type_to_string(entry->type));
 	}
 }
 
-- 
2.45.2

[PATCH 31/32] x86/boot/e820: Unify e820_print_type() and e820_type_to_string()

[Ingo Molnar]

Use e820_type_to_string() to derive e820_print_type(),
and unify the messages:

 - Don't Capitalize Words Within Sentences Randomly

 - Use 'Device reserved' instead of 'Reserved'

Suggested-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 arch/x86/kernel/e820.c | 50 ++++++++++++++++++++------------------------------
 1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index fe3e078a4064..10c6e7dc72d7 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -68,6 +68,26 @@ unsigned long pci_mem_start = 0xaeedbabe;
 EXPORT_SYMBOL(pci_mem_start);
 #endif
 
+__init static const char * e820_type_to_string(enum e820_type type)
+{
+	switch (type) {
+	case E820_TYPE_RAM:		return "System RAM";
+	case E820_TYPE_ACPI:		return "ACPI tables";
+	case E820_TYPE_NVS:		return "ACPI non-volatile storage";
+	case E820_TYPE_UNUSABLE:	return "Unusable memory";
+	case E820_TYPE_PRAM:		return "Persistent memory (legacy)";
+	case E820_TYPE_PMEM:		return "Persistent memory";
+	case E820_TYPE_RESERVED:	return "Device reserved";
+	case E820_TYPE_13:		return "Type 13";
+	default:			return "Unknown E820 type";
+	}
+}
+
+__init static void e820_print_type(enum e820_type type)
+{
+	pr_cont(" %s", e820_type_to_string(type));
+}
+
 /*
  * This function checks if any part of the range <start,end> is mapped
  * with type.
@@ -186,21 +206,6 @@ __init void e820__range_add(u64 start, u64 size, enum e820_type type)
 	__e820__range_add(e820_table, start, size, type);
 }
 
-__init static void e820_print_type(enum e820_type type)
-{
-	switch (type) {
-	case E820_TYPE_RAM:		pr_cont(" System RAM");				break;
-	case E820_TYPE_RESERVED:	pr_cont(" device reserved");			break;
-	case E820_TYPE_SOFT_RESERVED:	pr_cont(" soft reserved");			break;
-	case E820_TYPE_ACPI:		pr_cont(" ACPI data");				break;
-	case E820_TYPE_NVS:		pr_cont(" ACPI NVS");				break;
-	case E820_TYPE_UNUSABLE:	pr_cont(" unusable");				break;
-	case E820_TYPE_PMEM:		/* Fall through: */
-	case E820_TYPE_PRAM:		pr_cont(" persistent RAM (type %u)", type);	break;
-	default:			pr_cont(" type %u", type);			break;
-	}
-}
-
 /*
  * Print out the size of a E820 region, in human-readable
  * fashion, going from KB, MB, GB to TB units.
@@ -1065,21 +1070,6 @@ __init void e820__finish_early_params(void)
 	}
 }
 
-__init static const char * e820_type_to_string(enum e820_type type)
-{
-	switch (type) {
-	case E820_TYPE_RAM:		return "System RAM";
-	case E820_TYPE_ACPI:		return "ACPI Tables";
-	case E820_TYPE_NVS:		return "ACPI Non-volatile Storage";
-	case E820_TYPE_UNUSABLE:	return "Unusable memory";
-	case E820_TYPE_PRAM:		return "Persistent Memory (legacy)";
-	case E820_TYPE_PMEM:		return "Persistent Memory";
-	case E820_TYPE_RESERVED:	return "Reserved";
-	case E820_TYPE_13:		return "Type 13";
-	default:			return "Unknown E820 type";
-	}
-}
-
 __init static unsigned long e820_type_to_iomem_type(struct e820_entry *entry)
 {
 	switch (entry->type) {
-- 
2.45.2

[PATCH 32/32] x86/boot/e820: Move index increments outside accessors in e820__update_table()

[Ingo Molnar]

This kind of code:

	change_point[chg_idx++]->entry  = &entries[idx];

Can be a bit confusing to human readers, and GCC-15 started
warning about these patterns.

Move the index increment outside the accessor.

Suggested-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/kernel/e820.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 10c6e7dc72d7..afb312620c82 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -421,9 +421,11 @@ __init int e820__update_table(struct e820_table *table)
 	for (idx = 0; idx < table->nr_entries; idx++)	{
 		if (entries[idx].size != 0) {
 			change_point[chg_idx]->addr	= entries[idx].addr;
-			change_point[chg_idx++]->entry	= &entries[idx];
+			change_point[chg_idx]->entry	= &entries[idx];
+			chg_idx++;
 			change_point[chg_idx]->addr	= entries[idx].addr + entries[idx].size;
-			change_point[chg_idx++]->entry	= &entries[idx];
+			change_point[chg_idx]->entry	= &entries[idx];
+			chg_idx++;
 		}
 	}
 	chg_nr = chg_idx;
-- 
2.45.2

Re: [PATCH 32/32] x86/boot/e820: Move index increments outside accessors in e820__update_table()

[H. Peter Anvin]

On May 15, 2025 5:05:48 AM PDT, Ingo Molnar <mingo@kernel.org> wrote:
>This kind of code:
>
>	change_point[chg_idx++]->entry  = &entries[idx];
>
>Can be a bit confusing to human readers, and GCC-15 started
>warning about these patterns.
>
>Move the index increment outside the accessor.
>
>Suggested-by: Andy Shevchenko <andy@kernel.org>
>Signed-off-by: Ingo Molnar <mingo@kernel.org>
>Cc: Arnd Bergmann <arnd@kernel.org>
>Cc: David Woodhouse <dwmw@amazon.co.uk>
>Cc: H. Peter Anvin <hpa@zytor.com>
>Cc: Kees Cook <keescook@chromium.org>
>Cc: Linus Torvalds <torvalds@linux-foundation.org>
>Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
>---
> arch/x86/kernel/e820.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
>index 10c6e7dc72d7..afb312620c82 100644
>--- a/arch/x86/kernel/e820.c
>+++ b/arch/x86/kernel/e820.c
>@@ -421,9 +421,11 @@ __init int e820__update_table(struct e820_table *table)
> 	for (idx = 0; idx < table->nr_entries; idx++)	{
> 		if (entries[idx].size != 0) {
> 			change_point[chg_idx]->addr	= entries[idx].addr;
>-			change_point[chg_idx++]->entry	= &entries[idx];
>+			change_point[chg_idx]->entry	= &entries[idx];
>+			chg_idx++;
> 			change_point[chg_idx]->addr	= entries[idx].addr + entries[idx].size;
>-			change_point[chg_idx++]->entry	= &entries[idx];
>+			change_point[chg_idx]->entry	= &entries[idx];
>+			chg_idx++;
> 		}
> 	}
> 	chg_nr = chg_idx;

Really? That seems easier to miss to me.

Re: [PATCH 32/32] x86/boot/e820: Move index increments outside accessors in e820__update_table()

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

> On May 15, 2025 5:05:48 AM PDT, Ingo Molnar <mingo@kernel.org> wrote:
> >This kind of code:
> >
> >	change_point[chg_idx++]->entry  = &entries[idx];
> >
> >Can be a bit confusing to human readers, and GCC-15 started
> >warning about these patterns.
> >
> >Move the index increment outside the accessor.
> >
> >Suggested-by: Andy Shevchenko <andy@kernel.org>
> >Signed-off-by: Ingo Molnar <mingo@kernel.org>
> >Cc: Arnd Bergmann <arnd@kernel.org>
> >Cc: David Woodhouse <dwmw@amazon.co.uk>
> >Cc: H. Peter Anvin <hpa@zytor.com>
> >Cc: Kees Cook <keescook@chromium.org>
> >Cc: Linus Torvalds <torvalds@linux-foundation.org>
> >Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
> >---
> > arch/x86/kernel/e820.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> >diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> >index 10c6e7dc72d7..afb312620c82 100644
> >--- a/arch/x86/kernel/e820.c
> >+++ b/arch/x86/kernel/e820.c
> >@@ -421,9 +421,11 @@ __init int e820__update_table(struct e820_table *table)
> > 	for (idx = 0; idx < table->nr_entries; idx++)	{
> > 		if (entries[idx].size != 0) {
> > 			change_point[chg_idx]->addr	= entries[idx].addr;
> >-			change_point[chg_idx++]->entry	= &entries[idx];
> >+			change_point[chg_idx]->entry	= &entries[idx];
> >+			chg_idx++;
> > 			change_point[chg_idx]->addr	= entries[idx].addr + entries[idx].size;
> >-			change_point[chg_idx++]->entry	= &entries[idx];
> >+			change_point[chg_idx]->entry	= &entries[idx];
> >+			chg_idx++;
> > 		}
> > 	}
> > 	chg_nr = chg_idx;
> 
> Really? That seems easier to miss to me.

Maybe writing it in two groups:

			change_point[chg_idx]->addr	= entries[idx].addr;
			change_point[chg_idx]->entry	= &entries[idx];
			chg_idx++;

			change_point[chg_idx]->addr	= entries[idx].addr + entries[idx].size;
			change_point[chg_idx]->entry	= &entries[idx];
			chg_idx++;

makes it a bit easier to read? The chg_idx++ are pretty prominent in 
that form, while it's easier to miss when it's embedded.

Thanks,

	Ingo

Re: [PATCH 32/32] x86/boot/e820: Move index increments outside accessors in e820__update_table()

[Nikolay Borisov]

On 5/15/25 15:05, Ingo Molnar wrote:
> This kind of code:
> 
> 	change_point[chg_idx++]->entry  = &entries[idx];
> 
> Can be a bit confusing to human readers, and GCC-15 started
> warning about these patterns.
> 
> Move the index increment outside the accessor.
> 
> Suggested-by: Andy Shevchenko <andy@kernel.org>
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> Cc: Arnd Bergmann <arnd@kernel.org>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>   arch/x86/kernel/e820.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 10c6e7dc72d7..afb312620c82 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -421,9 +421,11 @@ __init int e820__update_table(struct e820_table *table)
>   	for (idx = 0; idx < table->nr_entries; idx++)	{
>   		if (entries[idx].size != 0) {
nit: The level of nesting can easily be reduced by doing
if (entries[idx].size == 0)
	continue;
>   			change_point[chg_idx]->addr	= entries[idx].addr;
> -			change_point[chg_idx++]->entry	= &entries[idx];
> +			change_point[chg_idx]->entry	= &entries[idx];
> +			chg_idx++;

nit: I have to agree with H. Peter Anvin that this seems somewhat odd to 
me as well.

<snip>