x86/e820: make e820 type string uniform

Message ID 20221020115609.223940-1-yajun.deng@linux.dev
State New
Headers
Series x86/e820: make e820 type string uniform |

Commit Message

Yajun Deng Oct. 20, 2022, 11:56 a.m. UTC
  These e820_print_type() and e820_type_to_string() functions are similar,
the former used in dmesg and the latter used in /proc/iomem.

Make them uniform, so that we can find the correspondence between dmesg
and /proc/iomem.

It may be confusing in dmesg. The e820 type update seems no change, but
the log like that:
...
[    0.000000] e820: update [mem 0x81004018-0x81023c57] usable ==> usable
[    0.000000] e820: update [mem 0x81004018-0x81023c57] usable ==> usable
[    0.000000] e820: update [mem 0x80ff3018-0x81003e57] usable ==> usable
[    0.000000] e820: update [mem 0x80ff3018-0x81003e57] usable ==> usable
...

Another confusion in /proc/iomem, the continuous area may be divided
into multiple parts, but they have same name, like that:

...
00100000-80ff3017 : System RAM
80ff3018-81003e57 : System RAM
81003e58-81004017 : System RAM
81004018-81023c57 : System RAM
81023c58-87672fff : System RAM
...

Separate the string of E820_TYPE_RESERVED_KERN and E820_TYPE_RAM, this
makes more clearer.

After patch:
dmesg:
...
[    0.000000] BIOS-provided physical RAM map:
[    0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000005efff] System RAM
[    0.000000] BIOS-e820: [mem 0x000000000005f000-0x000000000005ffff] Reserved
[    0.000000] BIOS-e820: [mem 0x0000000000060000-0x000000000009ffff] System RAM
[    0.000000] BIOS-e820: [mem 0x00000000000a0000-0x00000000000fffff] Reserved
[    0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000008c7b5fff] System RAM
[    0.000000] BIOS-e820: [mem 0x000000008c7b6000-0x000000008e025fff] Reserved
[    0.000000] BIOS-e820: [mem 0x000000008e026000-0x000000008e200fff] ACPI Tables
[    0.000000] BIOS-e820: [mem 0x000000008e201000-0x000000008e689fff] ACPI Non-volatile Storage
[    0.000000] BIOS-e820: [mem 0x000000008e68a000-0x000000008f40dfff] Reserved
[    0.000000] BIOS-e820: [mem 0x000000008f40e000-0x000000008f40efff] System RAM
[    0.000000] BIOS-e820: [mem 0x000000008f40f000-0x000000008fffffff] Reserved
[    0.000000] BIOS-e820: [mem 0x00000000e0000000-0x00000000efffffff] Reserved
[    0.000000] BIOS-e820: [mem 0x00000000fe000000-0x00000000fe010fff] Reserved
[    0.000000] BIOS-e820: [mem 0x00000000fec00000-0x00000000fec00fff] Reserved
[    0.000000] BIOS-e820: [mem 0x00000000fed00000-0x00000000fed03fff] Reserved
[    0.000000] BIOS-e820: [mem 0x00000000fee00000-0x00000000fee00fff] Reserved
[    0.000000] BIOS-e820: [mem 0x00000000ff000000-0x00000000ffffffff] Reserved
[    0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000086dffffff] System RAM
[    0.000000] NX (Execute Disable) protection: active
[    0.000000] e820: update [mem 0x81004018-0x81023c57] System RAM ==> System RAM (kernel)
[    0.000000] e820: update [mem 0x81004018-0x81023c57] System RAM ==> System RAM (kernel)
[    0.000000] e820: update [mem 0x80ff3018-0x81003e57] System RAM ==> System RAM (kernel)
[    0.000000] e820: update [mem 0x80ff3018-0x81003e57] System RAM ==> System RAM (kernel)
...

/proc/iomem:
...
00000000-00000fff : Reserved
00001000-0005efff : System RAM
0005f000-0005ffff : Reserved
00060000-0009ffff : System RAM
000a0000-000fffff : Reserved
  000a0000-000bffff : PCI Bus 0000:00
  000c0000-000cddff : Video ROM
  000f0000-000fffff : System ROM
00100000-80ff3017 : System RAM
80ff3018-81003e57 : System RAM (kernel)
81003e58-81004017 : System RAM
81004018-81023c57 : System RAM (kernel)
81023c58-87672fff : System RAM
...

Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
 arch/x86/kernel/e820.c | 54 ++++++++++++++----------------------------
 1 file changed, 18 insertions(+), 36 deletions(-)
  

Comments

Dave Hansen Oct. 20, 2022, 3:44 p.m. UTC | #1
On 10/20/22 04:56, Yajun Deng wrote:
> /proc/iomem:
> ...
> 00000000-00000fff : Reserved
> 00001000-0005efff : System RAM
> 0005f000-0005ffff : Reserved
> 00060000-0009ffff : System RAM
> 000a0000-000fffff : Reserved
>   000a0000-000bffff : PCI Bus 0000:00
>   000c0000-000cddff : Video ROM
>   000f0000-000fffff : System ROM
> 00100000-80ff3017 : System RAM
> 80ff3018-81003e57 : System RAM (kernel)
> 81003e58-81004017 : System RAM
> 81004018-81023c57 : System RAM (kernel)
> 81023c58-87672fff : System RAM

I guess this is a pretty minimal change.  It definitely makes
/proc/iomem more human-readable.

Did you consider if this change might break any users of this file?
  
Yajun Deng Oct. 21, 2022, 2:52 a.m. UTC | #2
October 20, 2022 11:44 PM, "Dave Hansen" <dave.hansen@intel.com> wrote:

> On 10/20/22 04:56, Yajun Deng wrote:
> 
>> /proc/iomem:
>> ...
>> 00000000-00000fff : Reserved
>> 00001000-0005efff : System RAM
>> 0005f000-0005ffff : Reserved
>> 00060000-0009ffff : System RAM
>> 000a0000-000fffff : Reserved
>> 000a0000-000bffff : PCI Bus 0000:00
>> 000c0000-000cddff : Video ROM
>> 000f0000-000fffff : System ROM
>> 00100000-80ff3017 : System RAM
>> 80ff3018-81003e57 : System RAM (kernel)
>> 81003e58-81004017 : System RAM
>> 81004018-81023c57 : System RAM (kernel)
>> 81023c58-87672fff : System RAM
> 
> I guess this is a pretty minimal change. It definitely makes
> /proc/iomem more human-readable.
> 
> Did you consider if this change might break any users of this file?

Yes, it may be.
I don't know how many users use this file, but I think the impact of this change is limited.

1st: This is not the first time this file has been changed. In commit 
     ad5fb870c486("e820, efi: add ACPI 6.0 persistent memory types"), the E820_PRAM changed 
     from "Persistent RAM" to "Persistent Memory (legacy)".

2nd: only super users can access the full information from /proc/iomem, which means the users
     of this file are limited.
     In commit 51d7b120418e("/proc/iomem: only expose physical resource addresses to privileged users")

3rd: The E820_TYPE_RESERVED_KERN changed from "System RAM" to "System RAM (kernel)" is a minimal change,
     and it also contains the "System RAM" string.
  

Patch

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 9dac24680ff8..8a0fb22bd740 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -184,19 +184,19 @@  void __init 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)
+static const char *__init e820_type_to_string(enum e820_type type)
 {
 	switch (type) {
-	case E820_TYPE_RAM:		/* Fall through: */
-	case E820_TYPE_RESERVED_KERN:	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_RESERVED_KERN:	return "System RAM (kernel)";
+	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_SOFT_RESERVED:	return "Soft Reserved";
+	default:			return "Unknown E820 type";
 	}
 }
 
@@ -210,8 +210,7 @@  void __init e820__print_table(char *who)
 			e820_table->entries[i].addr,
 			e820_table->entries[i].addr + e820_table->entries[i].size - 1);
 
-		e820_print_type(e820_table->entries[i].type);
-		pr_cont("\n");
+		pr_info("%s\n", e820_type_to_string(e820_table->entries[i].type));
 	}
 }
 
@@ -473,10 +472,8 @@  __e820__range_update(struct e820_table *table, u64 start, u64 size, enum e820_ty
 
 	end = start + size;
 	printk(KERN_DEBUG "e820: update [mem %#010Lx-%#010Lx] ", start, end - 1);
-	e820_print_type(old_type);
-	pr_cont(" ==> ");
-	e820_print_type(new_type);
-	pr_cont("\n");
+	pr_info("%s ==> %s\n", e820_type_to_string(old_type),
+		e820_type_to_string(new_type));
 
 	for (i = 0; i < table->nr_entries; i++) {
 		struct e820_entry *entry = &table->entries[i];
@@ -550,7 +547,7 @@  u64 __init e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool
 	end = start + size;
 	printk(KERN_DEBUG "e820: remove [mem %#010Lx-%#010Lx] ", start, end - 1);
 	if (check_type)
-		e820_print_type(old_type);
+		pr_info("%s", e820_type_to_string(old_type));
 	pr_cont("\n");
 
 	for (i = 0; i < e820_table->nr_entries; i++) {
@@ -1071,22 +1068,6 @@  void __init e820__finish_early_params(void)
 	}
 }
 
-static const char *__init e820_type_to_string(struct e820_entry *entry)
-{
-	switch (entry->type) {
-	case E820_TYPE_RESERVED_KERN:	/* Fall-through: */
-	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_SOFT_RESERVED:	return "Soft Reserved";
-	default:			return "Unknown E820 type";
-	}
-}
-
 static unsigned long __init e820_type_to_iomem_type(struct e820_entry *entry)
 {
 	switch (entry->type) {
@@ -1174,7 +1155,7 @@  void __init 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);
 
@@ -1194,7 +1175,8 @@  void __init e820__reserve_resources(void)
 	for (i = 0; i < e820_table_firmware->nr_entries; i++) {
 		struct e820_entry *entry = e820_table_firmware->entries + i;
 
-		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));
 	}
 }