[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aCXN2mcspYiXJIu0@gmail.com>
Date: Thu, 15 May 2025 13:19:54 +0200
From: Ingo Molnar <mingo@...nel.org>
To: Mike Rapoport <rppt@...nel.org>
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>, "H . Peter Anvin" <hpa@...or.com>,
Kees Cook <keescook@...omium.org>,
Linus Torvalds <torvalds@...ux-foundation.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 30/29] x86/boot/e820: Unify e820_print_type() and
e820_type_to_string()
* Mike Rapoport <rppt@...nel.org> wrote:
> On Mon, Apr 21, 2025 at 08:51:48PM +0200, Ingo Molnar wrote:
> > 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 'kernel usable RAM':
> >
> > BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] 1.9 GB kernel usable RAM
> >
> > 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/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 8ee89962fcbf..99f997ae88dc 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(" kernel usable 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;
>
> We have e820_type_to_string(), IMO the whole switch here can be replaced by
>
> pr_cont(" %s", e820_type_to_string(type));
Yeah, agreed, but there's a few additional details:
- Your suggestion doesn't work as-is, because e820_type_to_string()
takes an 'entry' parameter, not 'type'.
- There's some difference in the messages, so I think this should be a
separate patch.
- Also, I think unified messages with the best of both sets of
messages is the best outcome, instead of just picking one side.
See these commits in the WIP.x86/e820 tree:
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()
With the more interesting one attached below.
Thanks,
Ingo
=================================>
From: Ingo Molnar <mingo@...nel.org>
Date: Thu, 15 May 2025 13:17:45 +0200
Subject: [PATCH] x86/boot/e820: Unify e820_print_type() and e820_type_to_string()
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@...nel.org>
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>
---
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 3a86216ee05f..aadc46f3d074 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) {
Powered by blists - more mailing lists