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

Ingo Molnar posted 29 patches 7 months, 4 weeks ago
There is a newer version of this series
[PATCH 08/29] x86/boot/e820: Print E820_TYPE_RAM entries as ... RAM entries
Posted by Ingo Molnar 7 months, 4 weeks ago
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@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 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;
-- 
2.45.2
Re: [PATCH 08/29] x86/boot/e820: Print E820_TYPE_RAM entries as ... RAM entries
Posted by Mike Rapoport 7 months, 4 weeks ago
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@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 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));

> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.
[PATCH 30/29] x86/boot/e820: Unify e820_print_type() and e820_type_to_string()
Posted by Ingo Molnar 7 months ago

* Mike Rapoport <rppt@kernel.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@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 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@kernel.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@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 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) {
Re: [PATCH 08/29] x86/boot/e820: Print E820_TYPE_RAM entries as ... RAM entries
Posted by Andy Shevchenko 7 months, 4 weeks ago
Mon, Apr 21, 2025 at 08:51:48PM +0200, Ingo Molnar kirjoitti:
> 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

Can't user space use that RAM?

Shouldn't we rather refer to "OS usable RAM"?


-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 08/29] x86/boot/e820: Print E820_TYPE_RAM entries as ... RAM entries
Posted by Mike Rapoport 7 months, 4 weeks ago
On Tue, Apr 22, 2025 at 09:31:31AM +0300, Andy Shevchenko wrote:
> Mon, Apr 21, 2025 at 08:51:48PM +0200, Ingo Molnar kirjoitti:
> > 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
> 
> Can't user space use that RAM?
> 
> Shouldn't we rather refer to "OS usable RAM"?

Or "System RAM", just like in /proc/iomem
  
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

-- 
Sincerely yours,
Mike.
Re: [PATCH 08/29] x86/boot/e820: Print E820_TYPE_RAM entries as ... RAM entries
Posted by Ingo Molnar 7 months ago
* Mike Rapoport <rppt@kernel.org> wrote:

> On Tue, Apr 22, 2025 at 09:31:31AM +0300, Andy Shevchenko wrote:
> > Mon, Apr 21, 2025 at 08:51:48PM +0200, Ingo Molnar kirjoitti:
> > > 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
> > 
> > Can't user space use that RAM?
> > 
> > Shouldn't we rather refer to "OS usable RAM"?
> 
> Or "System RAM", just like in /proc/iomem

Agreed - I have folded in the delta patch below.

Thanks,

	Ingo

=============================>
 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 8b84261173cc..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(" kernel usable RAM");		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;