Shorten efi regions output
Subsystems (5)
Borislav Petkov
maintainerHi guys,
so this decoded EFI regions output is nice but can we shorten it? It sticks too far out in the terminal more than anything else in dmesg and staring at it needs me to resize window and such. It probably is an even bigger problem if one's monitor hasn't switched to higher res early during boot.
So here's what I'm seeing with the latest EDKII:
[ 0.000000] efi: EFI v2.40 by EDK II [ 0.000000] efi: ACPI=0x7ff2d000 ACPI 2.0=0x7ff2d014 [ 0.000000] efi: mem00: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000000000-0x0000000000001000) (0MB) [ 0.000000] efi: mem01: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x0000000000001000-0x0000000000002000) (0MB) [ 0.000000] efi: mem02: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000002000-0x000000000009f000) (0MB) [ 0.000000] efi: mem03: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000000009f000-0x00000000000a0000) (0MB) [ 0.000000] efi: mem04: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000100000-0x0000000000820000) (7MB) [ 0.000000] efi: mem05: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x0000000000820000-0x0000000001000000) (7MB) [ 0.000000] efi: mem06: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x0000000001000000-0x0000000003cbc000) (44MB) [ 0.000000] efi: mem07: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000003cbc000-0x0000000004000000) (3MB) [ 0.000000] efi: mem08: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x0000000004000000-0x0000000005cbc000) (28MB) [ 0.000000] efi: mem09: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000005cbc000-0x000000003fffc000) (931MB) [ 0.000000] efi: mem10: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000003fffc000-0x0000000040000000) (0MB) [ 0.000000] efi: mem11: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000040000000-0x000000007c000000) (960MB) [ 0.000000] efi: mem12: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007c000000-0x000000007c020000) (0MB) [ 0.000000] efi: mem13: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x000000007c020000-0x000000007ebc5000) (43MB) [ 0.000000] efi: mem14: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000007ebc5000-0x000000007ebfe000) (0MB) [ 0.000000] efi: mem15: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ebfe000-0x000000007ebff000) (0MB) [ 0.000000] efi: mem16: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x000000007ebff000-0x000000007ec03000) (0MB) [ 0.000000] efi: mem17: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec03000-0x000000007ec05000) (0MB) [ 0.000000] efi: mem18: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec05000-0x000000007ec06000) (0MB) [ 0.000000] efi: mem19: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec06000-0x000000007ec07000) (0MB) [ 0.000000] efi: mem20: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec07000-0x000000007ec08000) (0MB) [ 0.000000] efi: mem21: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec08000-0x000000007ece7000) (0MB) [ 0.000000] efi: mem22: [Boot Code | | | | | |WB|WT|WC|UC] range=[0x000000007ece7000-0x000000007ee3b000) (1MB) [ 0.000000] efi: mem23: [Runtime Data |RUN| | | | |WB|WT|WC|UC] range=[0x000000007ee3b000-0x000000007ee4e000) (0MB) [ 0.000000] efi: mem24: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ee4e000-0x000000007fd4e000) (15MB) [ 0.000000] efi: mem25: [Boot Code | | | | | |WB|WT|WC|UC] range=[0x000000007fd4e000-0x000000007fece000) (1MB) [ 0.000000] efi: mem26: [Runtime Code |RUN| | | | |WB|WT|WC|UC] range=[0x000000007fece000-0x000000007fefe000) (0MB) [ 0.000000] efi: mem27: [Runtime Data |RUN| | | | |WB|WT|WC|UC] range=[0x000000007fefe000-0x000000007ff22000) (0MB) [ 0.000000] efi: mem28: [Reserved | | | | | |WB|WT|WC|UC] range=[0x000000007ff22000-0x000000007ff26000) (0MB) [ 0.000000] efi: mem29: [ACPI Reclaim Memory| | | | | |WB|WT|WC|UC] range=[0x000000007ff26000-0x000000007ff2e000) (0MB) [ 0.000000] efi: mem30: [ACPI Memory NVS | | | | | |WB|WT|WC|UC] range=[0x000000007ff2e000-0x000000007ff32000) (0MB) [ 0.000000] efi: mem31: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ff32000-0x000000007ffd0000) (0MB) [ 0.000000] efi: mem32: [Runtime Data |RUN| | | | |WB|WT|WC|UC] range=[0x000000007ffd0000-0x0000000080000000) (0MB) [ 0.000000] efi: mem33: [Runtime Data |RUN| | | | | | | |UC] range=[0x00000000fff00000-0x0000000100000000) (1MB)
and here's with the proposed changes:
* regions get printed first so that vertical alignment gets preserved
* memory types come then, with shorter names but still readable (I hope :))
* efi mem attributes which are not set do not print the empty string. The vertical bar after "UC" is missing on purpose although this is not that totally correct if the region we're printing doesn't have the UC bit set. Then it would issue "|]" at the end but who cares - I'd like to not complicate this function unnecessarily.
This way the output fits much better in dmesg and we shorten it by 30ish columns while preserving it. Diff below.
[ 0.000000] efi: EFI v2.40 by EDK II [ 0.000000] efi: ACPI=0x7ff2d000 ACPI 2.0=0x7ff2d014 [ 0.000000] efi: mem00: [0x0000000000000000-0x0000000000001000) [Mem |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem01: [0x0000000000001000-0x0000000000002000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem02: [0x0000000000002000-0x000000000009f000) [Mem |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem03: [0x000000000009f000-0x00000000000a0000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem04: [0x0000000000100000-0x0000000000820000) [Mem |WB|WT|WC|UC] (7MB) [ 0.000000] efi: mem05: [0x0000000000820000-0x0000000001000000) [BootData |WB|WT|WC|UC] (7MB) [ 0.000000] efi: mem06: [0x0000000001000000-0x0000000003cbc000) [LdrData |WB|WT|WC|UC] (44MB) [ 0.000000] efi: mem07: [0x0000000003cbc000-0x0000000004000000) [Mem |WB|WT|WC|UC] (3MB) [ 0.000000] efi: mem08: [0x0000000004000000-0x0000000005cbc000) [LdrData |WB|WT|WC|UC] (28MB) [ 0.000000] efi: mem09: [0x0000000005cbc000-0x000000003fffc000) [Mem |WB|WT|WC|UC] (931MB) [ 0.000000] efi: mem10: [0x000000003fffc000-0x0000000040000000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem11: [0x0000000040000000-0x000000007c000000) [Mem |WB|WT|WC|UC] (960MB) [ 0.000000] efi: mem12: [0x000000007c000000-0x000000007c020000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem13: [0x000000007c020000-0x000000007ebc5000) [Mem |WB|WT|WC|UC] (43MB) [ 0.000000] efi: mem14: [0x000000007ebc5000-0x000000007ebfe000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem15: [0x000000007ebfe000-0x000000007ebff000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem16: [0x000000007ebff000-0x000000007ec03000) [Mem |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem17: [0x000000007ec03000-0x000000007ec05000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem18: [0x000000007ec05000-0x000000007ec06000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem19: [0x000000007ec06000-0x000000007ec07000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem20: [0x000000007ec07000-0x000000007ec08000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem21: [0x000000007ec08000-0x000000007ece7000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem22: [0x000000007ece7000-0x000000007ee3b000) [BootCode |WB|WT|WC|UC] (1MB) [ 0.000000] efi: mem23: [0x000000007ee3b000-0x000000007ee4e000) [RtData |RT|WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem24: [0x000000007ee4e000-0x000000007fd4e000) [BootData |WB|WT|WC|UC] (15MB) [ 0.000000] efi: mem25: [0x000000007fd4e000-0x000000007fece000) [BootCode |WB|WT|WC|UC] (1MB) [ 0.000000] efi: mem26: [0x000000007fece000-0x000000007fefe000) [RtCode |RT|WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem27: [0x000000007fefe000-0x000000007ff22000) [RtData |RT|WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem28: [0x000000007ff22000-0x000000007ff26000) [Reserved |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem29: [0x000000007ff26000-0x000000007ff2e000) [ACPIRclMem |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem30: [0x000000007ff2e000-0x000000007ff32000) [ACPIMemNVS |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem31: [0x000000007ff32000-0x000000007ffd0000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem32: [0x000000007ffd0000-0x0000000080000000) [RtData |RT|WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem33: [0x00000000fff00000-0x0000000100000000) [RtData |RT|UC] (1MB)
--- arch/x86/platform/efi/efi.c | 5 +++-- drivers/firmware/efi/efi.c | 52 ++++++++++++++++++++++----------------------- 2 files changed, 29 insertions(+), 28 deletions(-)
arch/x86/platform/efi/efi.c +3 -2
@@ -203,10 +203,11 @@ static void __init print_efi_memmap(void)char buf[64 ]; md = p; -pr_info ("mem%02u: %s range=[0x%016llx-0x%016llx) (%lluMB)\n" , - i,efi_md_typeattr_format (buf,sizeof (buf), md), +pr_info ("mem%02u: [0x%016llx-0x%016llx) %s (%lluMB)\n" , + i, md->phys_addr, md->phys_addr + (md->num_pages <<EFI_PAGE_SHIFT ), +efi_md_typeattr_format (buf,sizeof (buf), md), (md->num_pages >> (20 - EFI_PAGE_SHIFT))); }#endif /* EFI_DEBUG */
drivers/firmware/efi/efi.c +26 -26
71 lines not on this page · raw message
Signature · 9 lines
2.0.0
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 51 lines · Borislav Petkov
Hi guys,
so this decoded EFI regions output is nice but can we shorten it? It sticks too far out in the terminal more than anything else in dmesg and staring at it needs me to resize window and such. It probably is an even bigger problem if one's monitor hasn't switched to higher res early during boot.
So here's what I'm seeing with the latest EDKII:
[ 0.000000] efi: EFI v2.40 by EDK II [ 0.000000] efi: ACPI=0x7ff2d000 ACPI 2.0=0x7ff2d014 [ 0.000000] efi: mem00: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000000000-0x0000000000001000) (0MB) [ 0.000000] efi: mem01: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x0000000000001000-0x0000000000002000) (0MB) [ 0.000000] efi: mem02: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000002000-0x000000000009f000) (0MB) [ 0.000000] efi: mem03: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000000009f000-0x00000000000a0000) (0MB) [ 0.000000] efi: mem04: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000100000-0x0000000000820000) (7MB) [ 0.000000] efi: mem05: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x0000000000820000-0x0000000001000000) (7MB) [ 0.000000] efi: mem06: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x0000000001000000-0x0000000003cbc000) (44MB) [ 0.000000] efi: mem07: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000003cbc000-0x0000000004000000) (3MB) [ 0.000000] efi: mem08: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x0000000004000000-0x0000000005cbc000) (28MB) [ 0.000000] efi: mem09: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000005cbc000-0x000000003fffc000) (931MB) [ 0.000000] efi: mem10: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000003fffc000-0x0000000040000000) (0MB) [ 0.000000] efi: mem11: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000040000000-0x000000007c000000) (960MB) [ 0.000000] efi: mem12: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007c000000-0x000000007c020000) (0MB) [ 0.000000] efi: mem13: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x000000007c020000-0x000000007ebc5000) (43MB) [ 0.000000] efi: mem14: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000007ebc5000-0x000000007ebfe000) (0MB) [ 0.000000] efi: mem15: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ebfe000-0x000000007ebff000) (0MB) [ 0.000000] efi: mem16: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x000000007ebff000-0x000000007ec03000) (0MB) [ 0.000000] efi: mem17: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec03000-0x000000007ec05000) (0MB) [ 0.000000] efi: mem18: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec05000-0x000000007ec06000) (0MB) [ 0.000000] efi: mem19: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec06000-0x000000007ec07000) (0MB) [ 0.000000] efi: mem20: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec07000-0x000000007ec08000) (0MB) [ 0.000000] efi: mem21: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec08000-0x000000007ece7000) (0MB) [ 0.000000] efi: mem22: [Boot Code | | | | | |WB|WT|WC|UC] range=[0x000000007ece7000-0x000000007ee3b000) (1MB) [ 0.000000] efi: mem23: [Runtime Data |RUN| | | | |WB|WT|WC|UC] range=[0x000000007ee3b000-0x000000007ee4e000) (0MB) [ 0.000000] efi: mem24: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ee4e000-0x000000007fd4e000) (15MB) [ 0.000000] efi: mem25: [Boot Code | | | | | |WB|WT|WC|UC] range=[0x000000007fd4e000-0x000000007fece000) (1MB) [ 0.000000] efi: mem26: [Runtime Code |RUN| | | | |WB|WT|WC|UC] range=[0x000000007fece000-0x000000007fefe000) (0MB) [ 0.000000] efi: mem27: [Runtime Data |RUN| | | | |WB|WT|WC|UC] range=[0x000000007fefe000-0x000000007ff22000) (0MB) [ 0.000000] efi: mem28: [Reserved | | | | | |WB|WT|WC|UC] range=[0x000000007ff22000-0x000000007ff26000) (0MB) [ 0.000000] efi: mem29: [ACPI Reclaim Memory| | | | | |WB|WT|WC|UC] range=[0x000000007ff26000-0x000000007ff2e000) (0MB) [ 0.000000] efi: mem30: [ACPI Memory NVS | | | | | |WB|WT|WC|UC] range=[0x000000007ff2e000-0x000000007ff32000) (0MB) [ 0.000000] efi: mem31: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ff32000-0x000000007ffd0000) (0MB) [ 0.000000] efi: mem32: [Runtime Data |RUN| | | | |WB|WT|WC|UC] range=[0x000000007ffd0000-0x0000000080000000) (0MB) [ 0.000000] efi: mem33: [Runtime Data |RUN| | | | | | | |UC] range=[0x00000000fff00000-0x0000000100000000) (1MB)and here's with the proposed changes:
* regions get printed first so that vertical alignment gets preserved
Quoted 8 lines
* memory types come then, with shorter names but still readable (I hope :))
* efi mem attributes which are not set do not print the empty string. The vertical bar after "UC" is missing on purpose although this is not that totally correct if the region we're printing doesn't have the UC bit set. Then it would issue "|]" at the end but who cares - I'd like to not complicate this function unnecessarily.
I am fine with this change, but I have no strong opinion either way, to be honest.
The |] thing is easily fixed, though.
[...]
Quoted 4 lines
drivers/firmware/efi/efi.c +0
-0
[...]
Quoted 5 lines
drivers/firmware/efi/efi.c +0
-1@@ -490,15 +490,15 @@ char * __init efi_md_typeattr_format(char *buf, size_t size,snprintf (pos, size,"|attr=0x%016llx]" , (unsigned long long )attr); else - snprintf(pos, size, "|%3s|%2s|%2s|%2s|%3s|%2s|%2s|%2s|%2s]",
drivers/firmware/efi/efi.c char * __init efi_md_typeattr_format(char *buf, size_t size, · line 493
Quoted 10 lines
drivers/firmware/efi/efi.c +1
-9- attr &EFI_MEMORY_RUNTIME ?"RUN" :"" , - attr &EFI_MEMORY_XP ?"XP" :"" , - attr &EFI_MEMORY_RP ?"RP" :"" , - attr &EFI_MEMORY_WP ?"WP" :"" , - attr &EFI_MEMORY_UCE ?"UCE" :"" , - attr &EFI_MEMORY_WB ?"WB" :"" , - attr &EFI_MEMORY_WT ?"WT" :"" , - attr &EFI_MEMORY_WC ?"WC" :"" , - attr & EFI_MEMORY_UC ? "UC" : ""); + snprintf(pos, size, "|%s%s%s%s%s%s%s%s%s]",
Drop the leading | here
drivers/firmware/efi/efi.c char * __init efi_md_typeattr_format(char *buf, size_t size, · line 502
Quoted 9 lines
drivers/firmware/efi/efi.c +9
-0+ attr &EFI_MEMORY_RUNTIME ?"RT|" :"" , + attr &EFI_MEMORY_XP ?"XP|" :"" , + attr &EFI_MEMORY_RP ?"RP|" :"" , + attr &EFI_MEMORY_WP ?"WP|" :"" , + attr &EFI_MEMORY_UCE ?"UCE|" :"" , + attr &EFI_MEMORY_WB ?"WB|" :"" , + attr &EFI_MEMORY_WT ?"WT|" :"" , + attr &EFI_MEMORY_WC ?"WC|" :"" , + attr & EFI_MEMORY_UC ? "UC" : "");
and move all the | to the beginning of the string here, including "UC"
Quoted 12 lines
drivers/firmware/efi/efi.c +0
-1return buf; } --2.0.0
--
Regards/Gruss,
Boris.Sent from a fat crate under my desk. Formatting is fine. --
Borislav Petkov
maintainerQuoted 36 lines · Ard Biesheuvel
The |] thing is easily fixed, though.
[...]
Quoted 4 lines
drivers/firmware/efi/efi.c +0
-0[...]
Quoted 5 lines
drivers/firmware/efi/efi.c +0
-1@@ -490,15 +490,15 @@ char * __init efi_md_typeattr_format(char *buf, size_t size,snprintf (pos, size,"|attr=0x%016llx]" , (unsigned long long )attr); else - snprintf(pos, size, "|%3s|%2s|%2s|%2s|%3s|%2s|%2s|%2s|%2s]",drivers/firmware/efi/efi.c char * __init efi_md_typeattr_format(char *buf, size_t size, · line 493
Quoted 10 lines
drivers/firmware/efi/efi.c +1
-9- attr &EFI_MEMORY_RUNTIME ?"RUN" :"" , - attr &EFI_MEMORY_XP ?"XP" :"" , - attr &EFI_MEMORY_RP ?"RP" :"" , - attr &EFI_MEMORY_WP ?"WP" :"" , - attr &EFI_MEMORY_UCE ?"UCE" :"" , - attr &EFI_MEMORY_WB ?"WB" :"" , - attr &EFI_MEMORY_WT ?"WT" :"" , - attr &EFI_MEMORY_WC ?"WC" :"" , - attr & EFI_MEMORY_UC ? "UC" : ""); + snprintf(pos, size, "|%s%s%s%s%s%s%s%s%s]",Drop the leading | here
drivers/firmware/efi/efi.c char * __init efi_md_typeattr_format(char *buf, size_t size, · line 502
Quoted 9 lines
drivers/firmware/efi/efi.c +9
-0+ attr &EFI_MEMORY_RUNTIME ?"RT|" :"" , + attr &EFI_MEMORY_XP ?"XP|" :"" , + attr &EFI_MEMORY_RP ?"RP|" :"" , + attr &EFI_MEMORY_WP ?"WP|" :"" , + attr &EFI_MEMORY_UCE ?"UCE|" :"" , + attr &EFI_MEMORY_WB ?"WB|" :"" , + attr &EFI_MEMORY_WT ?"WT|" :"" , + attr &EFI_MEMORY_WC ?"WC|" :"" , + attr & EFI_MEMORY_UC ? "UC" : "");
Quoted 1 line
and move all the | to the beginning of the string here, including "UC"
Haha, of course! :-)
/me slaps forehead.
Thanks!
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 47 lines · Borislav Petkov
Hi guys,
so this decoded EFI regions output is nice but can we shorten it? It sticks too far out in the terminal more than anything else in dmesg and staring at it needs me to resize window and such. It probably is an even bigger problem if one's monitor hasn't switched to higher res early during boot.
So here's what I'm seeing with the latest EDKII:
[ 0.000000] efi: EFI v2.40 by EDK II [ 0.000000] efi: ACPI=0x7ff2d000 ACPI 2.0=0x7ff2d014 [ 0.000000] efi: mem00: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000000000-0x0000000000001000) (0MB) [ 0.000000] efi: mem01: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x0000000000001000-0x0000000000002000) (0MB) [ 0.000000] efi: mem02: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000002000-0x000000000009f000) (0MB) [ 0.000000] efi: mem03: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000000009f000-0x00000000000a0000) (0MB) [ 0.000000] efi: mem04: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000100000-0x0000000000820000) (7MB) [ 0.000000] efi: mem05: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x0000000000820000-0x0000000001000000) (7MB) [ 0.000000] efi: mem06: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x0000000001000000-0x0000000003cbc000) (44MB) [ 0.000000] efi: mem07: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000003cbc000-0x0000000004000000) (3MB) [ 0.000000] efi: mem08: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x0000000004000000-0x0000000005cbc000) (28MB) [ 0.000000] efi: mem09: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000005cbc000-0x000000003fffc000) (931MB) [ 0.000000] efi: mem10: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000003fffc000-0x0000000040000000) (0MB) [ 0.000000] efi: mem11: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000040000000-0x000000007c000000) (960MB) [ 0.000000] efi: mem12: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007c000000-0x000000007c020000) (0MB) [ 0.000000] efi: mem13: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x000000007c020000-0x000000007ebc5000) (43MB) [ 0.000000] efi: mem14: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000007ebc5000-0x000000007ebfe000) (0MB) [ 0.000000] efi: mem15: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ebfe000-0x000000007ebff000) (0MB) [ 0.000000] efi: mem16: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x000000007ebff000-0x000000007ec03000) (0MB) [ 0.000000] efi: mem17: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec03000-0x000000007ec05000) (0MB) [ 0.000000] efi: mem18: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec05000-0x000000007ec06000) (0MB) [ 0.000000] efi: mem19: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec06000-0x000000007ec07000) (0MB) [ 0.000000] efi: mem20: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec07000-0x000000007ec08000) (0MB) [ 0.000000] efi: mem21: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec08000-0x000000007ece7000) (0MB) [ 0.000000] efi: mem22: [Boot Code | | | | | |WB|WT|WC|UC] range=[0x000000007ece7000-0x000000007ee3b000) (1MB) [ 0.000000] efi: mem23: [Runtime Data |RUN| | | | |WB|WT|WC|UC] range=[0x000000007ee3b000-0x000000007ee4e000) (0MB) [ 0.000000] efi: mem24: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ee4e000-0x000000007fd4e000) (15MB) [ 0.000000] efi: mem25: [Boot Code | | | | | |WB|WT|WC|UC] range=[0x000000007fd4e000-0x000000007fece000) (1MB) [ 0.000000] efi: mem26: [Runtime Code |RUN| | | | |WB|WT|WC|UC] range=[0x000000007fece000-0x000000007fefe000) (0MB) [ 0.000000] efi: mem27: [Runtime Data |RUN| | | | |WB|WT|WC|UC] range=[0x000000007fefe000-0x000000007ff22000) (0MB) [ 0.000000] efi: mem28: [Reserved | | | | | |WB|WT|WC|UC] range=[0x000000007ff22000-0x000000007ff26000) (0MB) [ 0.000000] efi: mem29: [ACPI Reclaim Memory| | | | | |WB|WT|WC|UC] range=[0x000000007ff26000-0x000000007ff2e000) (0MB) [ 0.000000] efi: mem30: [ACPI Memory NVS | | | | | |WB|WT|WC|UC] range=[0x000000007ff2e000-0x000000007ff32000) (0MB) [ 0.000000] efi: mem31: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ff32000-0x000000007ffd0000) (0MB) [ 0.000000] efi: mem32: [Runtime Data |RUN| | | | |WB|WT|WC|UC] range=[0x000000007ffd0000-0x0000000080000000) (0MB) [ 0.000000] efi: mem33: [Runtime Data |RUN| | | | | | | |UC] range=[0x00000000fff00000-0x0000000100000000) (1MB)
Quoted 7 lines
and here's with the proposed changes:
* regions get printed first so that vertical alignment gets preserved
* memory types come then, with shorter names but still readable (I hope :))
* efi mem attributes which are not set do not print the empty string.
I disagree with the patch in general, and I strongly disagree with this specific change in particular. This part:
- breaks the alignment for the right side of the table
- completely defeats another of my goals with the original patch, which was to be able to get an overview of memory attributes *at a glance*. That only works if each column is strictly associated with one bit, and your gaze can slide over empty stretches.
Please compare how you visually execute a search for the next runtime region (or the next non-WB line) before and after.
Your patch shaves off 30 characters of the width if I measured it right (goes from 131 to 101.) I don't believe that would justify breaking the alignment of the columns. For example, you could remove 15 characters from the right just with "dmesg -t" (timestamps are probably not overly important for printing the memmap).
Also, how the table looks on the character console while booting is of absolutely no consequence (for me at least).
Anyway it's a matter of taste. I know you work with this every day so if it doesn't meet your needs then I won't insist preserving it. I won't NACK the patch it but I certainly won't ACK it either.
Thanks
Laszlo
Quoted 147 lines
The vertical bar after "UC" is missing on purpose although this is not that totally correct if the region we're printing doesn't have the UC bit set. Then it would issue "|]" at the end but who cares - I'd like to not complicate this function unnecessarily.
This way the output fits much better in dmesg and we shorten it by 30ish columns while preserving it. Diff below.
[ 0.000000] efi: EFI v2.40 by EDK II [ 0.000000] efi: ACPI=0x7ff2d000 ACPI 2.0=0x7ff2d014 [ 0.000000] efi: mem00: [0x0000000000000000-0x0000000000001000) [Mem |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem01: [0x0000000000001000-0x0000000000002000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem02: [0x0000000000002000-0x000000000009f000) [Mem |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem03: [0x000000000009f000-0x00000000000a0000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem04: [0x0000000000100000-0x0000000000820000) [Mem |WB|WT|WC|UC] (7MB) [ 0.000000] efi: mem05: [0x0000000000820000-0x0000000001000000) [BootData |WB|WT|WC|UC] (7MB) [ 0.000000] efi: mem06: [0x0000000001000000-0x0000000003cbc000) [LdrData |WB|WT|WC|UC] (44MB) [ 0.000000] efi: mem07: [0x0000000003cbc000-0x0000000004000000) [Mem |WB|WT|WC|UC] (3MB) [ 0.000000] efi: mem08: [0x0000000004000000-0x0000000005cbc000) [LdrData |WB|WT|WC|UC] (28MB) [ 0.000000] efi: mem09: [0x0000000005cbc000-0x000000003fffc000) [Mem |WB|WT|WC|UC] (931MB) [ 0.000000] efi: mem10: [0x000000003fffc000-0x0000000040000000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem11: [0x0000000040000000-0x000000007c000000) [Mem |WB|WT|WC|UC] (960MB) [ 0.000000] efi: mem12: [0x000000007c000000-0x000000007c020000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem13: [0x000000007c020000-0x000000007ebc5000) [Mem |WB|WT|WC|UC] (43MB) [ 0.000000] efi: mem14: [0x000000007ebc5000-0x000000007ebfe000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem15: [0x000000007ebfe000-0x000000007ebff000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem16: [0x000000007ebff000-0x000000007ec03000) [Mem |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem17: [0x000000007ec03000-0x000000007ec05000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem18: [0x000000007ec05000-0x000000007ec06000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem19: [0x000000007ec06000-0x000000007ec07000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem20: [0x000000007ec07000-0x000000007ec08000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem21: [0x000000007ec08000-0x000000007ece7000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem22: [0x000000007ece7000-0x000000007ee3b000) [BootCode |WB|WT|WC|UC] (1MB) [ 0.000000] efi: mem23: [0x000000007ee3b000-0x000000007ee4e000) [RtData |RT|WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem24: [0x000000007ee4e000-0x000000007fd4e000) [BootData |WB|WT|WC|UC] (15MB) [ 0.000000] efi: mem25: [0x000000007fd4e000-0x000000007fece000) [BootCode |WB|WT|WC|UC] (1MB) [ 0.000000] efi: mem26: [0x000000007fece000-0x000000007fefe000) [RtCode |RT|WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem27: [0x000000007fefe000-0x000000007ff22000) [RtData |RT|WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem28: [0x000000007ff22000-0x000000007ff26000) [Reserved |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem29: [0x000000007ff26000-0x000000007ff2e000) [ACPIRclMem |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem30: [0x000000007ff2e000-0x000000007ff32000) [ACPIMemNVS |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem31: [0x000000007ff32000-0x000000007ffd0000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem32: [0x000000007ffd0000-0x0000000080000000) [RtData |RT|WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem33: [0x00000000fff00000-0x0000000100000000) [RtData |RT|UC] (1MB)--- arch/x86/platform/efi/efi.c | 5 +++-- drivers/firmware/efi/efi.c | 52 ++++++++++++++++++++++----------------------- 2 files changed, 29 insertions(+), 28 deletions(-)arch/x86/platform/efi/efi.c +3
-2@@ -203,10 +203,11 @@ static void __init print_efi_memmap(void)char buf[64 ]; md = p; -pr_info ("mem%02u: %s range=[0x%016llx-0x%016llx) (%lluMB)\n" , - i,efi_md_typeattr_format (buf,sizeof (buf), md), +pr_info ("mem%02u: [0x%016llx-0x%016llx) %s (%lluMB)\n" , + i, md->phys_addr, md->phys_addr + (md->num_pages <<EFI_PAGE_SHIFT ), +efi_md_typeattr_format (buf,sizeof (buf), md), (md->num_pages >> (20 - EFI_PAGE_SHIFT))); }#endif /* EFI_DEBUG */ drivers/firmware/efi/efi.c +26
-26@@ -446,21 +446,23 @@ int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose) }#endif /* CONFIG_EFI_PARAMS_FROM_FDT */ +/* Update this if you're adding a longer string */ +#define MEM_TYPE_MAXLEN 11static __initdata char memory_type_name[][20 ] = {"Reserved" , -"Loader Code" , -"Loader Data" , -"Boot Code" , -"Boot Data" , -"Runtime Code" , -"Runtime Data" , -"Conventional Memory" , -"Unusable Memory" , -"ACPI Reclaim Memory" , -"ACPI Memory NVS" , -"Memory Mapped I/O" , -"MMIO Port Space" , -"PAL Code" +"LdrCode" , +"LdrData" , +"BootCode" , +"BootData" , +"RtCode" , +"RtData" , +"Mem" , +"UnusableMem" , +"ACPIRclMem" , +"ACPIMemNVS" , +"MMappedI/O" , +"MMIOPortSpc" , +"PALCode" }; char * __init efi_md_typeattr_format(char *buf, size_t size,@@ -474,9 +476,7 @@ char * __init efi_md_typeattr_format(char *buf, size_t size,if (md->type >=ARRAY_SIZE (memory_type_name)) type_len =snprintf (pos, size,"[type=%u" , md->type);else - type_len =snprintf (pos, size,"[%-*s" , - (int )(sizeof (memory_type_name[0 ]) -1 ), - memory_type_name[md->type]); + type_len =snprintf (pos, size,"[%-*s" ,MEM_TYPE_MAXLEN , memory_type_name[md->type]);if (type_len >= size)return buf;@@ -490,15 +490,15 @@ char * __init efi_md_typeattr_format(char *buf, size_t size,snprintf (pos, size,"|attr=0x%016llx]" , (unsigned long long )attr); else - snprintf(pos, size, "|%3s|%2s|%2s|%2s|%3s|%2s|%2s|%2s|%2s]", - attr &EFI_MEMORY_RUNTIME ?"RUN" :"" , - attr &EFI_MEMORY_XP ?"XP" :"" , - attr &EFI_MEMORY_RP ?"RP" :"" , - attr &EFI_MEMORY_WP ?"WP" :"" , - attr &EFI_MEMORY_UCE ?"UCE" :"" , - attr &EFI_MEMORY_WB ?"WB" :"" , - attr &EFI_MEMORY_WT ?"WT" :"" , - attr &EFI_MEMORY_WC ?"WC" :"" , - attr & EFI_MEMORY_UC ? "UC" : ""); + snprintf(pos, size, "|%s%s%s%s%s%s%s%s%s]", + attr &EFI_MEMORY_RUNTIME ?"RT|" :"" , + attr &EFI_MEMORY_XP ?"XP|" :"" , + attr &EFI_MEMORY_RP ?"RP|" :"" , + attr &EFI_MEMORY_WP ?"WP|" :"" , + attr &EFI_MEMORY_UCE ?"UCE|" :"" , + attr &EFI_MEMORY_WB ?"WB|" :"" , + attr &EFI_MEMORY_WT ?"WT|" :"" , + attr &EFI_MEMORY_WC ?"WC|" :"" , + attr & EFI_MEMORY_UC ? "UC" : "");return buf; }
Borislav Petkov
maintainerQuoted 4 lines · Laszlo Ersek
I disagree with the patch in general, and I strongly disagree with this specific change in particular. This part:
- breaks the alignment for the right side of the table
Well, we can start by removing the sizes - they're useless anyway and can be computed with a simple awk script or whatever. I.e.,
...
[ 0.000000] efi: mem23: [0x000000007ee3b000-0x000000007ee4e000) [RtData |RT|WB|WT|WC|UC]) [ 0.000000] efi: mem24: [0x000000007ee4e000-0x000000007fd4e000) [BootData |WB|WT|WC|UC] [ 0.000000] efi: mem25: [0x000000007fd4e000-0x000000007fece000) [BootCode |WB|WT|WC|UC] [ 0.000000] efi: mem26: [0x000000007fece000-0x000000007fefe000) [RtCode |RT|WB|WT|WC|UC] [ 0.000000] efi: mem27: [0x000000007fefe000-0x000000007ff22000) [RtData |RT|WB|WT|WC|UC]
...
Then we could do some more work like fishing out columns and removing those which are not set in any region from the output completely and then have alignment but with only the relevant fields, ...
Quoted 4 lines
- completely defeats another of my goals with the original patch, which was to be able to get an overview of memory attributes *at a glance*. That only works if each column is strictly associated with one bit, and your gaze can slide over empty stretches.
Well, and empty column doesn't tell me a whole lot, does it?
[ 0.000000] efi: mem00: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000000000-0x0000000000001000) (0MB)
I still have to go and look up what those 5 empty bits mean. Thus my angle was: if it is not set, don't print it.
Quoted 2 lines
Please compare how you visually execute a search for the next runtime region (or the next non-WB line) before and after.
I'd look for region names RtData/RtCode.
Unless this dump is supposed to be used to sanity-check region attribute flags too, then I see your point about the alignment.
Quoted 5 lines
Your patch shaves off 30 characters of the width if I measured it right (goes from 131 to 101.) I don't believe that would justify breaking the alignment of the columns. For example, you could remove 15 characters from the right just with "dmesg -t" (timestamps are probably not overly important for printing the memmap).
Yeah, but you can look at that output with other means, i.e. serial console/netconsole/guest boot log, etc.
Quoted 3 lines
Anyway it's a matter of taste. I know you work with this every day so if it doesn't meet your needs then I won't insist preserving it. I won't NACK the patch it but I certainly won't ACK it either.
Ok, fair enough. Let's see what the others think.
Thanks.
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 7 lines · Borislav Petkov
Hi guys,
so this decoded EFI regions output is nice but can we shorten it? It sticks too far out in the terminal more than anything else in dmesg and staring at it needs me to resize window and such. It probably is an even bigger problem if one's monitor hasn't switched to higher res early during boot.
I have same feeling with you, it is too long for most of people.
Since the printk code are for EFI_DEBUG, they are around the #ifdef so I would like to see a kernel param like efi_debug=on, so only efi_debug is specified then these verbose messages are printed. Without the param kernel can print some basic infomation about the memory ranges.
In arm64 code there's already a uefi_debug param it can be moved to general code so that there will be a goable switch.
Quoted 207 lines
So here's what I'm seeing with the latest EDKII:
[ 0.000000] efi: EFI v2.40 by EDK II [ 0.000000] efi: ACPI=0x7ff2d000 ACPI 2.0=0x7ff2d014 [ 0.000000] efi: mem00: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000000000-0x0000000000001000) (0MB) [ 0.000000] efi: mem01: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x0000000000001000-0x0000000000002000) (0MB) [ 0.000000] efi: mem02: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000002000-0x000000000009f000) (0MB) [ 0.000000] efi: mem03: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000000009f000-0x00000000000a0000) (0MB) [ 0.000000] efi: mem04: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000100000-0x0000000000820000) (7MB) [ 0.000000] efi: mem05: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x0000000000820000-0x0000000001000000) (7MB) [ 0.000000] efi: mem06: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x0000000001000000-0x0000000003cbc000) (44MB) [ 0.000000] efi: mem07: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000003cbc000-0x0000000004000000) (3MB) [ 0.000000] efi: mem08: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x0000000004000000-0x0000000005cbc000) (28MB) [ 0.000000] efi: mem09: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000005cbc000-0x000000003fffc000) (931MB) [ 0.000000] efi: mem10: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000003fffc000-0x0000000040000000) (0MB) [ 0.000000] efi: mem11: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000040000000-0x000000007c000000) (960MB) [ 0.000000] efi: mem12: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007c000000-0x000000007c020000) (0MB) [ 0.000000] efi: mem13: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x000000007c020000-0x000000007ebc5000) (43MB) [ 0.000000] efi: mem14: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000007ebc5000-0x000000007ebfe000) (0MB) [ 0.000000] efi: mem15: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ebfe000-0x000000007ebff000) (0MB) [ 0.000000] efi: mem16: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x000000007ebff000-0x000000007ec03000) (0MB) [ 0.000000] efi: mem17: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec03000-0x000000007ec05000) (0MB) [ 0.000000] efi: mem18: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec05000-0x000000007ec06000) (0MB) [ 0.000000] efi: mem19: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec06000-0x000000007ec07000) (0MB) [ 0.000000] efi: mem20: [Loader Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec07000-0x000000007ec08000) (0MB) [ 0.000000] efi: mem21: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ec08000-0x000000007ece7000) (0MB) [ 0.000000] efi: mem22: [Boot Code | | | | | |WB|WT|WC|UC] range=[0x000000007ece7000-0x000000007ee3b000) (1MB) [ 0.000000] efi: mem23: [Runtime Data |RUN| | | | |WB|WT|WC|UC] range=[0x000000007ee3b000-0x000000007ee4e000) (0MB) [ 0.000000] efi: mem24: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ee4e000-0x000000007fd4e000) (15MB) [ 0.000000] efi: mem25: [Boot Code | | | | | |WB|WT|WC|UC] range=[0x000000007fd4e000-0x000000007fece000) (1MB) [ 0.000000] efi: mem26: [Runtime Code |RUN| | | | |WB|WT|WC|UC] range=[0x000000007fece000-0x000000007fefe000) (0MB) [ 0.000000] efi: mem27: [Runtime Data |RUN| | | | |WB|WT|WC|UC] range=[0x000000007fefe000-0x000000007ff22000) (0MB) [ 0.000000] efi: mem28: [Reserved | | | | | |WB|WT|WC|UC] range=[0x000000007ff22000-0x000000007ff26000) (0MB) [ 0.000000] efi: mem29: [ACPI Reclaim Memory| | | | | |WB|WT|WC|UC] range=[0x000000007ff26000-0x000000007ff2e000) (0MB) [ 0.000000] efi: mem30: [ACPI Memory NVS | | | | | |WB|WT|WC|UC] range=[0x000000007ff2e000-0x000000007ff32000) (0MB) [ 0.000000] efi: mem31: [Boot Data | | | | | |WB|WT|WC|UC] range=[0x000000007ff32000-0x000000007ffd0000) (0MB) [ 0.000000] efi: mem32: [Runtime Data |RUN| | | | |WB|WT|WC|UC] range=[0x000000007ffd0000-0x0000000080000000) (0MB) [ 0.000000] efi: mem33: [Runtime Data |RUN| | | | | | | |UC] range=[0x00000000fff00000-0x0000000100000000) (1MB)and here's with the proposed changes:
* regions get printed first so that vertical alignment gets preserved
* memory types come then, with shorter names but still readable (I hope :))
* efi mem attributes which are not set do not print the empty string. The vertical bar after "UC" is missing on purpose although this is not that totally correct if the region we're printing doesn't have the UC bit set. Then it would issue "|]" at the end but who cares - I'd like to not complicate this function unnecessarily.
This way the output fits much better in dmesg and we shorten it by 30ish columns while preserving it. Diff below.
[ 0.000000] efi: EFI v2.40 by EDK II [ 0.000000] efi: ACPI=0x7ff2d000 ACPI 2.0=0x7ff2d014 [ 0.000000] efi: mem00: [0x0000000000000000-0x0000000000001000) [Mem |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem01: [0x0000000000001000-0x0000000000002000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem02: [0x0000000000002000-0x000000000009f000) [Mem |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem03: [0x000000000009f000-0x00000000000a0000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem04: [0x0000000000100000-0x0000000000820000) [Mem |WB|WT|WC|UC] (7MB) [ 0.000000] efi: mem05: [0x0000000000820000-0x0000000001000000) [BootData |WB|WT|WC|UC] (7MB) [ 0.000000] efi: mem06: [0x0000000001000000-0x0000000003cbc000) [LdrData |WB|WT|WC|UC] (44MB) [ 0.000000] efi: mem07: [0x0000000003cbc000-0x0000000004000000) [Mem |WB|WT|WC|UC] (3MB) [ 0.000000] efi: mem08: [0x0000000004000000-0x0000000005cbc000) [LdrData |WB|WT|WC|UC] (28MB) [ 0.000000] efi: mem09: [0x0000000005cbc000-0x000000003fffc000) [Mem |WB|WT|WC|UC] (931MB) [ 0.000000] efi: mem10: [0x000000003fffc000-0x0000000040000000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem11: [0x0000000040000000-0x000000007c000000) [Mem |WB|WT|WC|UC] (960MB) [ 0.000000] efi: mem12: [0x000000007c000000-0x000000007c020000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem13: [0x000000007c020000-0x000000007ebc5000) [Mem |WB|WT|WC|UC] (43MB) [ 0.000000] efi: mem14: [0x000000007ebc5000-0x000000007ebfe000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem15: [0x000000007ebfe000-0x000000007ebff000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem16: [0x000000007ebff000-0x000000007ec03000) [Mem |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem17: [0x000000007ec03000-0x000000007ec05000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem18: [0x000000007ec05000-0x000000007ec06000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem19: [0x000000007ec06000-0x000000007ec07000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem20: [0x000000007ec07000-0x000000007ec08000) [LdrData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem21: [0x000000007ec08000-0x000000007ece7000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem22: [0x000000007ece7000-0x000000007ee3b000) [BootCode |WB|WT|WC|UC] (1MB) [ 0.000000] efi: mem23: [0x000000007ee3b000-0x000000007ee4e000) [RtData |RT|WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem24: [0x000000007ee4e000-0x000000007fd4e000) [BootData |WB|WT|WC|UC] (15MB) [ 0.000000] efi: mem25: [0x000000007fd4e000-0x000000007fece000) [BootCode |WB|WT|WC|UC] (1MB) [ 0.000000] efi: mem26: [0x000000007fece000-0x000000007fefe000) [RtCode |RT|WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem27: [0x000000007fefe000-0x000000007ff22000) [RtData |RT|WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem28: [0x000000007ff22000-0x000000007ff26000) [Reserved |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem29: [0x000000007ff26000-0x000000007ff2e000) [ACPIRclMem |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem30: [0x000000007ff2e000-0x000000007ff32000) [ACPIMemNVS |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem31: [0x000000007ff32000-0x000000007ffd0000) [BootData |WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem32: [0x000000007ffd0000-0x0000000080000000) [RtData |RT|WB|WT|WC|UC] (0MB) [ 0.000000] efi: mem33: [0x00000000fff00000-0x0000000100000000) [RtData |RT|UC] (1MB)--- arch/x86/platform/efi/efi.c | 5 +++-- drivers/firmware/efi/efi.c | 52 ++++++++++++++++++++++----------------------- 2 files changed, 29 insertions(+), 28 deletions(-)arch/x86/platform/efi/efi.c +3
-2@@ -203,10 +203,11 @@ static void __init print_efi_memmap(void)char buf[64 ]; md = p; -pr_info ("mem%02u: %s range=[0x%016llx-0x%016llx) (%lluMB)\n" , - i,efi_md_typeattr_format (buf,sizeof (buf), md), +pr_info ("mem%02u: [0x%016llx-0x%016llx) %s (%lluMB)\n" , + i, md->phys_addr, md->phys_addr + (md->num_pages <<EFI_PAGE_SHIFT ), +efi_md_typeattr_format (buf,sizeof (buf), md), (md->num_pages >> (20 - EFI_PAGE_SHIFT))); }#endif /* EFI_DEBUG */ drivers/firmware/efi/efi.c +26
-26@@ -446,21 +446,23 @@ int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose) }#endif /* CONFIG_EFI_PARAMS_FROM_FDT */ +/* Update this if you're adding a longer string */ +#define MEM_TYPE_MAXLEN 11static __initdata char memory_type_name[][20 ] = {"Reserved" , -"Loader Code" , -"Loader Data" , -"Boot Code" , -"Boot Data" , -"Runtime Code" , -"Runtime Data" , -"Conventional Memory" , -"Unusable Memory" , -"ACPI Reclaim Memory" , -"ACPI Memory NVS" , -"Memory Mapped I/O" , -"MMIO Port Space" , -"PAL Code" +"LdrCode" , +"LdrData" , +"BootCode" , +"BootData" , +"RtCode" , +"RtData" , +"Mem" , +"UnusableMem" , +"ACPIRclMem" , +"ACPIMemNVS" , +"MMappedI/O" , +"MMIOPortSpc" , +"PALCode" }; char * __init efi_md_typeattr_format(char *buf, size_t size,@@ -474,9 +476,7 @@ char * __init efi_md_typeattr_format(char *buf, size_t size,if (md->type >=ARRAY_SIZE (memory_type_name)) type_len =snprintf (pos, size,"[type=%u" , md->type);else - type_len =snprintf (pos, size,"[%-*s" , - (int )(sizeof (memory_type_name[0 ]) -1 ), - memory_type_name[md->type]); + type_len =snprintf (pos, size,"[%-*s" ,MEM_TYPE_MAXLEN , memory_type_name[md->type]);if (type_len >= size)return buf;@@ -490,15 +490,15 @@ char * __init efi_md_typeattr_format(char *buf, size_t size,snprintf (pos, size,"|attr=0x%016llx]" , (unsigned long long )attr); else - snprintf(pos, size, "|%3s|%2s|%2s|%2s|%3s|%2s|%2s|%2s|%2s]", - attr &EFI_MEMORY_RUNTIME ?"RUN" :"" , - attr &EFI_MEMORY_XP ?"XP" :"" , - attr &EFI_MEMORY_RP ?"RP" :"" , - attr &EFI_MEMORY_WP ?"WP" :"" , - attr &EFI_MEMORY_UCE ?"UCE" :"" , - attr &EFI_MEMORY_WB ?"WB" :"" , - attr &EFI_MEMORY_WT ?"WT" :"" , - attr &EFI_MEMORY_WC ?"WC" :"" , - attr & EFI_MEMORY_UC ? "UC" : ""); + snprintf(pos, size, "|%s%s%s%s%s%s%s%s%s]", + attr &EFI_MEMORY_RUNTIME ?"RT|" :"" , + attr &EFI_MEMORY_XP ?"XP|" :"" , + attr &EFI_MEMORY_RP ?"RP|" :"" , + attr &EFI_MEMORY_WP ?"WP|" :"" , + attr &EFI_MEMORY_UCE ?"UCE|" :"" , + attr &EFI_MEMORY_WB ?"WB|" :"" , + attr &EFI_MEMORY_WT ?"WT|" :"" , + attr &EFI_MEMORY_WC ?"WC|" :"" , + attr & EFI_MEMORY_UC ? "UC" : "");return buf; }Signature · 13 lines
2.0.0 -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Borislav Petkov
maintainerQuoted 10 lines · Dave Young
I have same feeling with you, it is too long for most of people.
Since the printk code are for
EFI_DEBUG, they are around the #ifdef so I would like to see a kernel param like efi_debug=on, so onlyefi_debugis specified then these verbose messages are printed. Without the param kernel can print some basic infomation about the memory ranges.In arm64 code there's already a
uefi_debugparam it can be moved to general code so that there will be a goable switch.
Hmm, makes sense to me. Maybe we should really hide those behind a debug switch, the question is whether asking the user to boot with "efi_debug=on" in order to see the regions is ok. And I think it is ok because we do that when debugging other stuff so I don't see anything different here.
And then when they're disabled by default, we don't really need to shorten them as they're pure debug output then.
Matt?
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 18 lines · Borislav Petkov
Quoted 10 lines · Dave Young
I have same feeling with you, it is too long for most of people.
Since the printk code are for
EFI_DEBUG, they are around the #ifdef so I would like to see a kernel param like efi_debug=on, so onlyefi_debugis specified then these verbose messages are printed. Without the param kernel can print some basic infomation about the memory ranges.In arm64 code there's already a
uefi_debugparam it can be moved to general code so that there will be a goable switch.Hmm, makes sense to me. Maybe we should really hide those behind a debug switch, the question is whether asking the user to boot with "efi_debug=on" in order to see the regions is ok. And I think it is ok because we do that when debugging other stuff so I don't see anything different here.
Quoted 4 lines
And then when they're disabled by default, we don't really need to shorten them as they're pure debug output then.
Matt?
I'm fine with disabling the EFI memory output regions by default.
Printing the regions is still useful for debugging, but like you mention, we frequently ask users to enable other debug options when tracking down issues.
Laszlo, would you be OK with that?
Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
Quoted 24 lines · Matt Fleming
Quoted 18 lines · Borislav Petkov
Quoted 10 lines · Dave Young
I have same feeling with you, it is too long for most of people.
Since the printk code are for
EFI_DEBUG, they are around the #ifdef so I would like to see a kernel param like efi_debug=on, so onlyefi_debugis specified then these verbose messages are printed. Without the param kernel can print some basic infomation about the memory ranges.In arm64 code there's already a
uefi_debugparam it can be moved to general code so that there will be a goable switch.Hmm, makes sense to me. Maybe we should really hide those behind a debug switch, the question is whether asking the user to boot with "efi_debug=on" in order to see the regions is ok. And I think it is ok because we do that when debugging other stuff so I don't see anything different here.
Quoted 4 lines
And then when they're disabled by default, we don't really need to shorten them as they're pure debug output then.
Matt?
Quoted 7 lines
I'm fine with disabling the EFI memory output regions by default.
Printing the regions is still useful for debugging, but like you mention, we frequently ask users to enable other debug options when tracking down issues.
Laszlo, would you be OK with that?
Sure.
Thanks
Laszlo
Quoted 27 lines · Laszlo Ersek
Quoted 26 lines · Matt Fleming
Quoted 18 lines · Borislav Petkov
Quoted 10 lines · Dave Young
I have same feeling with you, it is too long for most of people.
Since the printk code are for
EFI_DEBUG, they are around the #ifdef so I would like to see a kernel param like efi_debug=on, so onlyefi_debugis specified then these verbose messages are printed. Without the param kernel can print some basic infomation about the memory ranges.In arm64 code there's already a
uefi_debugparam it can be moved to general code so that there will be a goable switch.Hmm, makes sense to me. Maybe we should really hide those behind a debug switch, the question is whether asking the user to boot with "efi_debug=on" in order to see the regions is ok. And I think it is ok because we do that when debugging other stuff so I don't see anything different here.
Quoted 4 lines
And then when they're disabled by default, we don't really need to shorten them as they're pure debug output then.
Matt?
I'm fine with disabling the EFI memory output regions by default.
Quoted 7 lines
Quoted 5 lines
Printing the regions is still useful for debugging, but like you mention, we frequently ask users to enable other debug options when tracking down issues.
Laszlo, would you be OK with that?
Sure.
Pardon my intrusion into this thread (just going over the past month's LKML for things I missed). I'd like to see some map output by default in the kernel. We have on a number of occasions found just this output useful in debugging boot issues on ARM servers and I suspect that will remain the case over the coming months. Sure, you can always tell someone to reboot, but then you have to rely on them doing it.
Jon.
Borislav Petkov
maintainerQuoted 2 lines · Jon Masters
We have on a number of occasions found just this output useful in debugging boot issues on ARM servers
We can turn it on by default on ARM and off on x86...
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Quoted 5 lines · Borislav Petkov
Quoted 2 lines · Jon Masters
We have on a number of occasions found just this output useful in debugging boot issues on ARM servers
We can turn it on by default on ARM and off on x86...
Yeah, there's no reason we need to have the same default. That is to say, it can be on for ARM and off for x86, but when enabled the format should be the same for both.
In the future the memory maps are likely to contain even more entries and I suspect we're going to get more requests to disable the voluminous memmap output by default if we don't do so now.
Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
Borislav Petkov
maintainerPATCH 1/1 efi, x86: Add a "debug" option to the efi= cmdline
From: Borislav Petkov <[email protected]>
Date: Mon, 26 Jan 2015 19:49:59 +0100
Subject: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
... and hide the memory regions dump behind it. Make it default-off.
--- arch/x86/platform/efi/efi.c | 5 ++++- include/linux/efi.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)
arch/x86/platform/efi/efi.c +4 -1
@@ -491,7 +491,8 @@ void __init efi_init(void)if (efi_memmap_init ())return ; -print_efi_memmap (); +if (efi_enabled (EFI_DBG )) +print_efi_memmap (); }void __init efi_late_init (void )
@@ -939,6 +940,8 @@ static int __init arch_parse_efi_cmdline(char *str) {if (parse_option_str (str,"old_map" ))set_bit (EFI_OLD_MEMMAP , &efi.flags); +if (parse_option_str (str,"debug" )) +set_bit (EFI_DBG , &efi.flags);return 0 ; }
include/linux/efi.h +1 -0
@@ -940,6 +940,7 @@ extern int __init efi_setup_pcdp_console(char *);#define EFI_64BIT 5/* Is the firmware 64-bit? */ #define EFI_PARAVIRT 6/* Access is via a paravirt interface */ #define EFI_ARCH_1 7/* First arch-specific bit */ +#define EFI_DBG 8/* Print additional debug info at runtime */ #ifdef CONFIG_EFI /*
Signature · 8 lines
2.2.0.33.gc18b867
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
Quoted 42 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
Date: Mon, 26 Jan 2015 19:49:59 +0100
Subject: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline... and hide the memory regions dump behind it. Make it default-off.
Signed-off-by Borislav Petkov <[email protected]>--- arch/x86/platform/efi/efi.c | 5 ++++- include/linux/efi.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)arch/x86/platform/efi/efi.c +4
-1@@ -491,7 +491,8 @@ void __init efi_init(void)if (efi_memmap_init ())return ; -print_efi_memmap (); +if (efi_enabled (EFI_DBG )) +print_efi_memmap (); }void __init efi_late_init (void )@@ -939,6 +940,8 @@ static int __init arch_parse_efi_cmdline(char *str) {if (parse_option_str (str,"old_map" ))set_bit (EFI_OLD_MEMMAP , &efi.flags); +if (parse_option_str (str,"debug" )) +set_bit (EFI_DBG , &efi.flags);return 0 ; }include/linux/efi.h +0
-0
include/linux/efi.h extern int __init efi_setup_pcdp_console(char *); · line 946
Quoted 7 lines
include/linux/efi.h +1
-0@@ -940,6 +940,7 @@ extern int __init efi_setup_pcdp_console(char *);#define EFI_64BIT 5/* Is the firmware 64-bit? */ #define EFI_PARAVIRT 6/* Access is via a paravirt interface */ #define EFI_ARCH_1 7/* First arch-specific bit */ +#define EFI_DBG 8/* Print additional debug info at runtime */ #ifdef CONFIG_EFI /*
I don't know much about the efi flag setting, but it look sane to me.
ACK
Laszlo
Randy Dunlap
reviewerRe: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
In reply to Borislav PetkovQuoted 4 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
Date: Mon, 26 Jan 2015 19:49:59 +0100
Subject: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
Please update Documentation/kernel-parameters.txt also.
Quoted 45 lines
... and hide the memory regions dump behind it. Make it default-off.
Signed-off-by Borislav Petkov <[email protected]>--- arch/x86/platform/efi/efi.c | 5 ++++- include/linux/efi.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)arch/x86/platform/efi/efi.c +4
-1@@ -491,7 +491,8 @@ void __init efi_init(void)if (efi_memmap_init ())return ; -print_efi_memmap (); +if (efi_enabled (EFI_DBG )) +print_efi_memmap (); }void __init efi_late_init (void )@@ -939,6 +940,8 @@ static int __init arch_parse_efi_cmdline(char *str) {if (parse_option_str (str,"old_map" ))set_bit (EFI_OLD_MEMMAP , &efi.flags); +if (parse_option_str (str,"debug" )) +set_bit (EFI_DBG , &efi.flags);return 0 ; }include/linux/efi.h +1
-0@@ -940,6 +940,7 @@ extern int __init efi_setup_pcdp_console(char *);#define EFI_64BIT 5/* Is the firmware 64-bit? */ #define EFI_PARAVIRT 6/* Access is via a paravirt interface */ #define EFI_ARCH_1 7/* First arch-specific bit */ +#define EFI_DBG 8/* Print additional debug info at runtime */ #ifdef CONFIG_EFI /*
Signature · 1 line
~Randy
Borislav Petkov
maintainerRe: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
Quoted 1 line · Randy Dunlap
Please update
Documentation/kernel-parameters.txtalso.
Good idea. Done. Thanks.
:-)
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
In reply to Randy DunlapSome messages from efifb also should be suppressed. It will be better if we can move them under efi=debug kernel parameter.
Please review the following patch.
Quoted 1 line
From
7fbac896ab87f1b37646ac2f49bb8216ec330642Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <[email protected]>
Date: Wed, 4 Feb 2015 06:50:32 -0500
Subject: [PATCH] efi, x86: Add a debug option to the efi= cmdline
--- drivers/video/fbdev/efifb.c | 49 +++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 22 deletions(-)
drivers/video/fbdev/efifb.c +27 -22
@@ -145,7 +145,8 @@ static int efifb_probe(struct platform_device *dev)printk (KERN_DEBUG "efifb: invalid framebuffer address\n" );return -ENODEV ; } -printk (KERN_INFO "efifb: probing for efifb\n" ); +if (efi_enabled (EFI_DBG )) +printk (KERN_INFO "efifb: probing for efifb\n" );/* just assume they're all unset if any are */ if (!screen_info.blue_size) {
@@ -224,20 +225,22 @@ static int efifb_probe(struct platform_device *dev) err = -EIO ; goto err_release_fb; } - -printk (KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, " -"using %dk, total %dk\n" , - efifb_fix.smem_start, info->screen_base, - size_remap/1024 , size_total/1024 ); -printk (KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n" , - efifb_defined.xres, efifb_defined.yres, - efifb_defined.bits_per_pixel, efifb_fix.line_length, - screen_info.pages); +if (efi_enabled (EFI_DBG )){ +printk (KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, " +"using %dk, total %dk\n" , + efifb_fix.smem_start, info->screen_base, + size_remap/1024 , size_total/1024 ); +printk (KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n" , + efifb_defined.xres, efifb_defined.yres, + efifb_defined.bits_per_pixel, efifb_fix.line_length, + screen_info.pages); + } efifb_defined.xres_virtual = efifb_defined.xres; efifb_defined.yres_virtual = efifb_fix.smem_len / efifb_fix.line_length; -printk (KERN_INFO "efifb: scrolling: redraw\n" ); +if (efi_enabled (EFI_DBG )) +printk (KERN_INFO "efifb: scrolling: redraw\n" ); efifb_defined.yres_virtual = efifb_defined.yres;/* some dummy values for timing to make fbset happy */
@@ -255,17 +258,19 @@ static int efifb_probe(struct platform_device *dev) efifb_defined.transp.offset = screen_info.rsvd_pos; efifb_defined.transp.length = screen_info.rsvd_size; -printk (KERN_INFO "efifb: %s: " -"size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n" , -"Truecolor" , - screen_info.rsvd_size, - screen_info.red_size, - screen_info.green_size, - screen_info.blue_size, - screen_info.rsvd_pos, - screen_info.red_pos, - screen_info.green_pos, - screen_info.blue_pos); +if (efi_enabled (EFI_DBG )){ +printk (KERN_INFO "efifb: %s: " +"size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n" , +"Truecolor" , + screen_info.rsvd_size, + screen_info.red_size, + screen_info.green_size, + screen_info.blue_size, + screen_info.rsvd_pos, + screen_info.red_pos, + screen_info.green_pos, + screen_info.blue_pos); + } efifb_fix.ypanstep =0 ; efifb_fix.ywrapstep =0 ;
--
1.9.3
Quoted 61 lines · Randy Dunlap
Quoted 4 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
Date: Mon, 26 Jan 2015 19:49:59 +0100
Subject: [PATCH] efi, x86: Add a "debug" option to the efi= cmdlinePlease update
Documentation/kernel-parameters.txtalso.Quoted 45 lines
... and hide the memory regions dump behind it. Make it default-off.
Signed-off-by Borislav Petkov <[email protected]>--- arch/x86/platform/efi/efi.c | 5 ++++- include/linux/efi.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)arch/x86/platform/efi/efi.c +4
-1@@ -491,7 +491,8 @@ void __init efi_init(void)if (efi_memmap_init ())return ; -print_efi_memmap (); +if (efi_enabled (EFI_DBG )) +print_efi_memmap (); }void __init efi_late_init (void )@@ -939,6 +940,8 @@ static int __init arch_parse_efi_cmdline(char *str) {if (parse_option_str (str,"old_map" ))set_bit (EFI_OLD_MEMMAP , &efi.flags); +if (parse_option_str (str,"debug" )) +set_bit (EFI_DBG , &efi.flags);return 0 ; }include/linux/efi.h +1
-0@@ -940,6 +940,7 @@ extern int __init efi_setup_pcdp_console(char *);#define EFI_64BIT 5/* Is the firmware 64-bit? */ #define EFI_PARAVIRT 6/* Access is via a paravirt interface */ #define EFI_ARCH_1 7/* First arch-specific bit */ +#define EFI_DBG 8/* Print additional debug info at runtime */ #ifdef CONFIG_EFI /* Signature · 5 lines
~Randy -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
In reply to Borislav PetkovQuoted 41 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
Date: Mon, 26 Jan 2015 19:49:59 +0100
Subject: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline... and hide the memory regions dump behind it. Make it default-off.
Signed-off-by Borislav Petkov <[email protected]>--- arch/x86/platform/efi/efi.c | 5 ++++- include/linux/efi.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)arch/x86/platform/efi/efi.c +4
-1@@ -491,7 +491,8 @@ void __init efi_init(void)if (efi_memmap_init ())return ; -print_efi_memmap (); +if (efi_enabled (EFI_DBG )) +print_efi_memmap (); }void __init efi_late_init (void )@@ -939,6 +940,8 @@ static int __init arch_parse_efi_cmdline(char *str) {if (parse_option_str (str,"old_map" ))set_bit (EFI_OLD_MEMMAP , &efi.flags); +if (parse_option_str (str,"debug" )) +set_bit (EFI_DBG , &efi.flags);return 0 ; }include/linux/efi.h +0
-0
include/linux/efi.h extern int __init efi_setup_pcdp_console(char *); · line 943
Quoted 4 lines
include/linux/efi.h +1
-0@@ -940,6 +940,7 @@ extern int __init efi_setup_pcdp_console(char *);#define EFI_64BIT 5/* Is the firmware 64-bit? */ #define EFI_PARAVIRT 6/* Access is via a paravirt interface */ #define EFI_ARCH_1 7/* First arch-specific bit */ +#define EFI_DBG 8/* Print additional debug info at runtime */
Boris, a nickpick about alignment of the second column..
Thanks
Dave
Borislav Petkov
maintainerRe: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
Quoted 11 lines · Dave Young
include/linux/efi.h extern int __init efi_setup_pcdp_console(char *); · line 943
Quoted 9 lines
include/linux/efi.h +1
-0@@ -940,6 +940,7 @@ extern int __init efi_setup_pcdp_console(char *);#define EFI_64BIT 5/* Is the firmware 64-bit? */ #define EFI_PARAVIRT 6/* Access is via a paravirt interface */ #define EFI_ARCH_1 7/* First arch-specific bit */ +#define EFI_DBG 8/* Print additional debug info at runtime */ Boris, a nickpick about alignment of the second column..
This is due to how diffs show tabs. See EFI_PARAVIRT above, it appears misaligned too. If you switch to list mode in vim, for example, it'll show you how many tabs are being used:
#define EFI_PARAVIRT^I^I6^I/* Access is via a paravirt interface */$ #define EFI_ARCH_1^I^I7^I/* First arch-specific bit */$ #define EFI_DBG^I^I^I8^I/* Print additional debug info at runtime */$
and EFI_DBG has an additional tab. However, the vertical alignment is correct. You can apply the patch and see for yourself :-)
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Dave Young
Acked-byRe: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
Quoted 17 lines · Borislav Petkov
Quoted 11 lines · Dave Young
include/linux/efi.h extern int __init efi_setup_pcdp_console(char *); · line 943
Quoted 9 lines
include/linux/efi.h +1
-0@@ -940,6 +940,7 @@ extern int __init efi_setup_pcdp_console(char *);#define EFI_64BIT 5/* Is the firmware 64-bit? */ #define EFI_PARAVIRT 6/* Access is via a paravirt interface */ #define EFI_ARCH_1 7/* First arch-specific bit */ +#define EFI_DBG 8/* Print additional debug info at runtime */ Boris, a nickpick about alignment of the second column..
This is due to how diffs show tabs. See
EFI_PARAVIRTabove, it appears misaligned too. If you switch to list mode in vim, for example, it'll show you how many tabs are being used:
Quoted 6 lines
#define EFI_PARAVIRT^I^I6^I/* Access is via a paravirt interface */$ #define EFI_ARCH_1^I^I7^I/* First arch-specific bit */$ #define EFI_DBG^I^I^I8^I/* Print additional debug info at runtime */$
and
EFI_DBGhas an additional tab. However, the vertical alignment is correct. You can apply the patch and see for yourself :-)
Interesting, you are right :)
Thanks
Dave
Borislav Petkov
maintainerPATCH v2 1/1 efi, x86: Add a "debug" option to the efi= cmdline
From: Borislav Petkov <[email protected]>
Subject: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
... and hide the memory regions dump behind it. Make it default-off.
--- Documentation/kernel-parameters.txt | 3 ++- arch/x86/platform/efi/efi.c | 5 ++++- include/linux/efi.h | 1 + 3 files changed, 7 insertions(+), 2 deletions(-)
Documentation/kernel-parameters.txt +2 -1
@@ -1024,7 +1024,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted. Format: {"off" | "on" | "skip[mbr]"} efi= [EFI] - Format: { "old_map", "nochunk", "noruntime" } + Format: { "old_map", "nochunk", "noruntime", "debug" } old_map [X86-64]: switch to the old ioremap-based EFI runtime services mapping. 32-bit still uses this one by default.
@@ -1032,6 +1032,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted. boot stub, as chunking can cause problems with some firmware implementations. noruntime : disable EFI runtime services support + debug: enable misc debug output efi_no_storage_paranoia [EFI; X86] Using this parameter you can use more than 50% of
arch/x86/platform/efi/efi.c +4 -1
@@ -491,7 +491,8 @@ void __init efi_init(void)if (efi_memmap_init ())return ; -print_efi_memmap (); +if (efi_enabled (EFI_DBG )) +print_efi_memmap (); }void __init efi_late_init (void )
@@ -939,6 +940,8 @@ static int __init arch_parse_efi_cmdline(char *str) {if (parse_option_str (str,"old_map" ))set_bit (EFI_OLD_MEMMAP , &efi.flags); +if (parse_option_str (str,"debug" )) +set_bit (EFI_DBG , &efi.flags);return 0 ; }
include/linux/efi.h +1 -0
@@ -940,6 +940,7 @@ extern int __init efi_setup_pcdp_console(char *);#define EFI_64BIT 5/* Is the firmware 64-bit? */ #define EFI_PARAVIRT 6/* Access is via a paravirt interface */ #define EFI_ARCH_1 7/* First arch-specific bit */ +#define EFI_DBG 8/* Print additional debug info at runtime */ #ifdef CONFIG_EFI /*
Signature · 8 lines
2.2.0.33.gc18b867
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Re: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
It is better if we add some printk from efifb messages. Please review the below patch.
Quoted 1 line
From
7fbac896ab87f1b37646ac2f49bb8216ec330642Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <[email protected]>
Date: Wed, 4 Feb 2015 06:50:32 -0500
Subject: [PATCH] efi, x86: Add a debug option to the efi= cmdline
--- drivers/video/fbdev/efifb.c | 49 +++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 22 deletions(-)
drivers/video/fbdev/efifb.c +27 -22
@@ -145,7 +145,8 @@ static int efifb_probe(struct platform_device *dev)printk (KERN_DEBUG "efifb: invalid framebuffer address\n" );return -ENODEV ; } -printk (KERN_INFO "efifb: probing for efifb\n" ); +if (efi_enabled (EFI_DBG )) +printk (KERN_INFO "efifb: probing for efifb\n" );/* just assume they're all unset if any are */ if (!screen_info.blue_size) {
@@ -224,20 +225,22 @@ static int efifb_probe(struct platform_device *dev) err = -EIO ; goto err_release_fb; } - -printk (KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, " -"using %dk, total %dk\n" , - efifb_fix.smem_start, info->screen_base, - size_remap/1024 , size_total/1024 ); -printk (KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n" , - efifb_defined.xres, efifb_defined.yres, - efifb_defined.bits_per_pixel, efifb_fix.line_length, - screen_info.pages); +if (efi_enabled (EFI_DBG )){ +printk (KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, " +"using %dk, total %dk\n" , + efifb_fix.smem_start, info->screen_base, + size_remap/1024 , size_total/1024 ); +printk (KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n" , + efifb_defined.xres, efifb_defined.yres, + efifb_defined.bits_per_pixel, efifb_fix.line_length, + screen_info.pages); + } efifb_defined.xres_virtual = efifb_defined.xres; efifb_defined.yres_virtual = efifb_fix.smem_len / efifb_fix.line_length; -printk (KERN_INFO "efifb: scrolling: redraw\n" ); +if (efi_enabled (EFI_DBG )) +printk (KERN_INFO "efifb: scrolling: redraw\n" ); efifb_defined.yres_virtual = efifb_defined.yres;/* some dummy values for timing to make fbset happy */
@@ -255,17 +258,19 @@ static int efifb_probe(struct platform_device *dev) efifb_defined.transp.offset = screen_info.rsvd_pos; efifb_defined.transp.length = screen_info.rsvd_size; -printk (KERN_INFO "efifb: %s: " -"size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n" , -"Truecolor" , - screen_info.rsvd_size, - screen_info.red_size, - screen_info.green_size, - screen_info.blue_size, - screen_info.rsvd_pos, - screen_info.red_pos, - screen_info.green_pos, - screen_info.blue_pos); +if (efi_enabled (EFI_DBG )){ +printk (KERN_INFO "efifb: %s: " +"size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n" , +"Truecolor" , + screen_info.rsvd_size, + screen_info.red_size, + screen_info.green_size, + screen_info.blue_size, + screen_info.rsvd_pos, + screen_info.red_pos, + screen_info.green_pos, + screen_info.blue_pos); + } efifb_fix.ypanstep =0 ; efifb_fix.ywrapstep =0 ;
--
1.9.3
Quoted 84 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
Subject: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline... and hide the memory regions dump behind it. Make it default-off.
Signed-off-by Borislav Petkov <[email protected]>Acked-by Laszlo Ersek <[email protected]>Acked-by Dave Young <[email protected]>--- Documentation/kernel-parameters.txt | 3 ++- arch/x86/platform/efi/efi.c | 5 ++++- include/linux/efi.h | 1 + 3 files changed, 7 insertions(+), 2 deletions(-)Documentation/kernel-parameters.txt +2
-1@@ -1024,7 +1024,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted. Format: {"off" | "on" | "skip[mbr]"} efi= [EFI] - Format: { "old_map", "nochunk", "noruntime" } + Format: { "old_map", "nochunk", "noruntime", "debug" } old_map [X86-64]: switch to the old ioremap-based EFI runtime services mapping. 32-bit still uses this one by default.@@ -1032,6 +1032,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted. boot stub, as chunking can cause problems with some firmware implementations. noruntime : disable EFI runtime services support + debug: enable misc debug output efi_no_storage_paranoia [EFI; X86] Using this parameter you can use more than 50% ofarch/x86/platform/efi/efi.c +4
-1@@ -491,7 +491,8 @@ void __init efi_init(void)if (efi_memmap_init ())return ; -print_efi_memmap (); +if (efi_enabled (EFI_DBG )) +print_efi_memmap (); }void __init efi_late_init (void )@@ -939,6 +940,8 @@ static int __init arch_parse_efi_cmdline(char *str) {if (parse_option_str (str,"old_map" ))set_bit (EFI_OLD_MEMMAP , &efi.flags); +if (parse_option_str (str,"debug" )) +set_bit (EFI_DBG , &efi.flags);return 0 ; }include/linux/efi.h +1
-0@@ -940,6 +940,7 @@ extern int __init efi_setup_pcdp_console(char *);#define EFI_64BIT 5/* Is the firmware 64-bit? */ #define EFI_PARAVIRT 6/* Access is via a paravirt interface */ #define EFI_ARCH_1 7/* First arch-specific bit */ +#define EFI_DBG 8/* Print additional debug info at runtime */ #ifdef CONFIG_EFI /* Signature · 12 lines
2.2.0.33.gc18b867 -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Borislav Petkov
maintainerRe: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
Quoted 2 lines · Parmeshwr Prasad
It is better if we add some printk from efifb messages. Please review the below patch.
First of all, we saw you patch.
Then,
Quoted 4 lines
From
7fbac896ab87f1b37646ac2f49bb8216ec330642Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <[email protected]>
Date: Wed, 4 Feb 2015 06:50:32 -0500
Subject: [PATCH] efi, x86: Add a debug option to the efi= cmdline
yours has the same Subject as mine. This is not how we do patches. Consult Documentation/SubmittingPatches about how to do them properly.
What is more, your patch doesn't have a commit message. But it needs one.
Fix all that, send it as a *separate* patch after mine has been applied and people will take a look then.
Quoted 11 lines
Signed-off-by Parmeshwr Prasad <[email protected]>--- drivers/video/fbdev/efifb.c | 49 +++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 22 deletions(-)drivers/video/fbdev/efifb.c +0
-0
drivers/video/fbdev/efifb.c static int efifb_probe(struct platform_device *dev) · line 149
Quoted 6 lines
drivers/video/fbdev/efifb.c +2
-1@@ -145,7 +145,8 @@ static int efifb_probe(struct platform_device *dev)printk (KERN_DEBUG "efifb: invalid framebuffer address\n" );return -ENODEV ; } -printk (KERN_INFO "efifb: probing for efifb\n" ); +if (efi_enabled (EFI_DBG )) +printk (KERN_INFO "efifb: probing for efifb\n" );
If we're going to use the "efifb" prefix, change those to pr_info and define pr_fmt - lotsa examples in the kernel sources.
More importantly, you'd need a consensus from people that the printks in efifb are really not interesting and should be behind efi=debug.
HTH.
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Re: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
Quoted 13 lines · Borislav Petkov
Quoted 2 lines · Parmeshwr Prasad
It is better if we add some printk from efifb messages. Please review the below patch.
First of all, we saw you patch.
Then,
Quoted 4 lines
From
7fbac896ab87f1b37646ac2f49bb8216ec330642Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <[email protected]>
Date: Wed, 4 Feb 2015 06:50:32 -0500
Subject: [PATCH] efi, x86: Add a debug option to the efi= cmdline
Quoted 7 lines
yours has the same Subject as mine. This is not how we do patches. Consult
Documentation/SubmittingPatchesabout how to do them properly.What is more, your patch doesn't have a commit message. But it needs one.
Fix all that, send it as a *separate* patch after mine has been applied and people will take a look then.
I am really sorry that I could not mentioned for what purpose this patch is . I wanted this patch to be included along with your patch. As your patch is To add “debug” cmdline in efi=”<option>”.
There are some messages in efifb which appears while booting. I thought of Putting that messages under efi=”debug” cmdline. When efi=debug is given Then only efifb messages should appear.
Quoted 37 lines
Quoted 12 lines
Signed-off-by Parmeshwr Prasad <[email protected]>--- drivers/video/fbdev/efifb.c | 49 +++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 22 deletions(-)drivers/video/fbdev/efifb.c +0
-0drivers/video/fbdev/efifb.c static int efifb_probe(struct platform_device *dev) · line 149
Quoted 6 lines
drivers/video/fbdev/efifb.c +2
-1@@ -145,7 +145,8 @@ static int efifb_probe(struct platform_device *dev)printk (KERN_DEBUG "efifb: invalid framebuffer address\n" );return -ENODEV ; } -printk (KERN_INFO "efifb: probing for efifb\n" ); +if (efi_enabled (EFI_DBG )) +printk (KERN_INFO "efifb: probing for efifb\n" );If we're going to use the "efifb" prefix, change those to
pr_infoand definepr_fmt- lotsa examples in the kernel sources.More importantly, you'd need a consensus from people that the printks in efifb are really not interesting and should be behind efi=debug.
HTH.
Signature · 9 lines
Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Borislav Petkov
maintainerRe: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
Quoted 7 lines · Parmeshwr Prasad
I am really sorry that I could not mentioned for what purpose this patch is . I wanted this patch to be included along with your patch. As your patch is To add “debug” cmdline in efi=”<option>”.
There are some messages in efifb which appears while booting. I thought of Putting that messages under efi=”debug” cmdline. When efi=debug is given Then only efifb messages should appear.
This is not how we do patches. If you feel that those printks in efifb are too verbose and should be behind a debug switch or, even removed completely, you can do a separate patch as I explained to you and write in that commit message what needs to be done and *why* you think it should be done.
HTH.
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Re: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
In reply to Borislav PetkovQuoted 9 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
Subject: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline... and hide the memory regions dump behind it. Make it default-off.
Signed-off-by Borislav Petkov <[email protected]>Acked-by Laszlo Ersek <[email protected]>Acked-by Dave Young <[email protected]>
Quoted 5 lines
--- Documentation/kernel-parameters.txt | 3 ++- arch/x86/platform/efi/efi.c | 5 ++++- include/linux/efi.h | 1 + 3 files changed, 7 insertions(+), 2 deletions(-)
Thanks Borislav, applied!
Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
tip-bot for Borislav Petkov
merge bot[tip:core/efi] x86/efi: Add a "debug" option to the efi= cmdline
In reply to Borislav Petkovx86/efi: Add a "debug" option to the efi= cmdline
Commit-ID: fed6cefe3b6e862dcc74d07324478caa07e84eaf Gitweb: http://git.kernel.org/tip/fed6cefe3b6e862dcc74d07324478caa07e84eaf Author: Borislav Petkov <[email protected]> AuthorDate: Thu, 5 Feb 2015 11:44:41 +0100 Committer: Matt Fleming <[email protected]> CommitDate: Wed, 1 Apr 2015 12:46:22 +0100
x86/efi: Add a "debug" option to the efi= cmdline
... and hide the memory regions dump behind it. Make it default-off.
--- Documentation/kernel-parameters.txt | 3 ++- arch/x86/platform/efi/efi.c | 5 ++++- include/linux/efi.h | 1 + 3 files changed, 7 insertions(+), 2 deletions(-)
Documentation/kernel-parameters.txt +2 -1
@@ -1036,7 +1036,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted. Format: {"off" | "on" | "skip[mbr]"} efi= [EFI] - Format: { "old_map", "nochunk", "noruntime" } + Format: { "old_map", "nochunk", "noruntime", "debug" } old_map [X86-64]: switch to the old ioremap-based EFI runtime services mapping. 32-bit still uses this one by default.
@@ -1044,6 +1044,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted. boot stub, as chunking can cause problems with some firmware implementations. noruntime : disable EFI runtime services support + debug: enable misc debug output efi_no_storage_paranoia [EFI; X86] Using this parameter you can use more than 50% of
arch/x86/platform/efi/efi.c +4 -1
@@ -491,7 +491,8 @@ void __init efi_init(void)if (efi_memmap_init ())return ; -print_efi_memmap (); +if (efi_enabled (EFI_DBG )) +print_efi_memmap (); }void __init efi_late_init (void )
@@ -939,6 +940,8 @@ static int __init arch_parse_efi_cmdline(char *str) {if (parse_option_str (str,"old_map" ))set_bit (EFI_OLD_MEMMAP , &efi.flags); +if (parse_option_str (str,"debug" )) +set_bit (EFI_DBG , &efi.flags);return 0 ; }
include/linux/efi.h +1 -0
@@ -942,6 +942,7 @@ extern int __init efi_setup_pcdp_console(char *);#define EFI_64BIT 5/* Is the firmware 64-bit? */ #define EFI_PARAVIRT 6/* Access is via a paravirt interface */ #define EFI_ARCH_1 7/* First arch-specific bit */ +#define EFI_DBG 8/* Print additional debug info at runtime */ #ifdef CONFIG_EFI /*