[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aCW8GQMGg4O7oZci@gmail.com>
Date: Thu, 15 May 2025 12:04:09 +0200
From: Ingo Molnar <mingo@...nel.org>
To: "H. Peter Anvin" <hpa@...or.com>
Cc: linux-kernel@...r.kernel.org, Andy Shevchenko <andy@...nel.org>,
Arnd Bergmann <arnd@...nel.org>, Borislav Petkov <bp@...en8.de>,
Juergen Gross <jgross@...e.com>, Kees Cook <keescook@...omium.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Mike Rapoport <rppt@...nel.org>,
Paul Menzel <pmenzel@...gen.mpg.de>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
David Woodhouse <dwmw@...zon.co.uk>
Subject: [PATCH -v2 29/29] x86/boot/e820: Introduce E820_TYPE_13 and treat it
as a device region
* H. Peter Anvin <hpa@...or.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@...nel.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@...gen.mpg.de>
Suggested-by: H. Peter Anvin <hpa@...or.com>
Signed-off-by: Ingo Molnar <mingo@...nel.org>
Cc: Andy Shevchenko <andy@...nel.org>
Cc: Arnd Bergmann <arnd@...nel.org>
Cc: David Woodhouse <dwmw@...zon.co.uk>
Cc: H. Peter Anvin <hpa@...or.com>
Cc: Kees Cook <keescook@...omium.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@...nel.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;
}
Powered by blists - more mailing lists