2022-10-20 12:21:41

by Yajun Deng

[permalink] [raw]
Subject: [PATCH] x86/e820: make e820 type string uniform

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 <[email protected]>
---
arch/x86/kernel/e820.c | 54 ++++++++++++++----------------------------
1 file changed, 18 insertions(+), 36 deletions(-)

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));
}
}

--
2.25.1


2022-10-20 16:00:42

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH] x86/e820: make e820 type string uniform

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?

2022-10-21 03:48:38

by Yajun Deng

[permalink] [raw]
Subject: Re: [PATCH] x86/e820: make e820 type string uniform

October 20, 2022 11:44 PM, "Dave Hansen" <[email protected]> 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.