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

Ingo Molnar posted 29 patches 7 months, 4 weeks ago
There is a newer version of this series
[PATCH 29/29] x86/boot/e820: Treat non-type-2 'reserved' E820 region types as E820_TYPE_RESERVED
Posted by Ingo Molnar 7 months, 4 weeks ago
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.

Follow the ACPI spec and change the Linux E820 code to treat unknown/reserved
E820 region types as E820_TYPE_RESERVED device memory regions.

Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index f9caf4c922ea..c36261d78109 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1140,8 +1140,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:
-	default:
 		return false;
+	/* Reserved E820 types should be treated as device memory regions: */
+	default:
+		return true;
 	}
 }
 
-- 
2.45.2
Re: [PATCH 29/29] x86/boot/e820: Treat non-type-2 'reserved' E820 region types as E820_TYPE_RESERVED
Posted by H. Peter Anvin 7 months, 3 weeks ago
On 4/21/25 11:52, Ingo Molnar wrote:
> 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:
> 

... so we should handle 13 accordingly (and probably request that the 
ACPI committee permanently reserve it.  It would have been better to use 
negative numbers for OS-specific things.)

However, if we run into a value that we have never seen, say something 
like 84, we shouldn't assume that it is safe to do anything at all to 
it; in particular we really don't want to assume that it is safe to 
place I/O devices there.

Note that devices may be a priori set up in type 2 memory; it pretty 
much means "this device is treated specially by firmware, don't move it 
around or bad things will happen."

	-hpa
[PATCH -v2 29/29] x86/boot/e820: Introduce E820_TYPE_13 and treat it as a device region
Posted by Ingo Molnar 7 months ago

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

> On 4/21/25 11:52, Ingo Molnar wrote:
> > 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:
> > 
> 
> ... so we should handle 13 accordingly (and probably request that the 
> ACPI committee permanently reserve it.  It would have been better to 
> use negative numbers for OS-specific things.)
> 
> However, if we run into a value that we have never seen, say 
> something like 84, we shouldn't assume that it is safe to do anything 
> at all to it; in particular we really don't want to assume that it is 
> safe to place I/O devices there.
> 
> Note that devices may be a priori set up in type 2 memory; it pretty 
> much means "this device is treated specially by firmware, don't move 
> it around or bad things will happen."

Okay, agreed, this approach makes a lot of sense.

How about the replacement patch below? It basically implements your 
recommendation: E820_TYPE_13 follows E820_TYPE_RESERVED behavior 
(device region), while other unknown types are still treated 
conservatively: system region, don't touch, don't merge.

Thanks,

	Ingo

====================================>
From: Ingo Molnar <mingo@kernel.org>
Date: Sat, 19 Apr 2025 21:50:24 +0200
Subject: [PATCH] x86/boot/e820: Introduce E820_TYPE_13 and treat it as a device region

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 6649d49c9c0f..e2579e385181 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;
 	}