2014-12-09 09:58:51

by Borislav Petkov

[permalink] [raw]
Subject: Shorten efi regions output

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

* 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(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 6a1ffd777db0..4b76256dcb31 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -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 */
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 8590099ac148..6734072980ee 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -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 11
static __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;
}
--
2.0.0


--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--


2014-12-09 12:42:48

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: Shorten efi regions output

On 9 December 2014 at 10:58, Borislav Petkov <[email protected]> wrote:
> 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
>
> * 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.

[...]
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 8590099ac148..6734072980ee 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
[...]
> @@ -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]",

Drop the leading | here

> + 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"


> return buf;
> }
> --
> 2.0.0
>
>
> --
> Regards/Gruss,
> Boris.
>
> Sent from a fat crate under my desk. Formatting is fine.
> --

2014-12-09 12:48:53

by Borislav Petkov

[permalink] [raw]
Subject: Re: Shorten efi regions output

On Tue, Dec 09, 2014 at 01:42:44PM +0100, Ard Biesheuvel wrote:
> The |] thing is easily fixed, though.
>
> [...]
> > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> > index 8590099ac148..6734072980ee 100644
> > --- a/drivers/firmware/efi/efi.c
> > +++ b/drivers/firmware/efi/efi.c
> [...]
> > @@ -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]",
>
> Drop the leading | here
>
> > + 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"

Haha, of course! :-)

/me slaps forehead.

Thanks!

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

2014-12-09 15:36:06

by Laszlo Ersek

[permalink] [raw]
Subject: Re: Shorten efi regions output

On 12/09/14 10:58, Borislav Petkov wrote:
> 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
>
> * 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

> 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(-)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index 6a1ffd777db0..4b76256dcb31 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -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 */
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 8590099ac148..6734072980ee 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -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 11
> static __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;
> }
>

2014-12-09 16:45:11

by Borislav Petkov

[permalink] [raw]
Subject: Re: Shorten efi regions output

On Tue, Dec 09, 2014 at 04:35:33PM +0100, Laszlo Ersek wrote:
> 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, ...

> - 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.

> 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.

> 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.

> 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.

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

2014-12-10 02:18:09

by Dave Young

[permalink] [raw]
Subject: Re: Shorten efi regions output

On 12/09/14 at 10:58am, Borislav Petkov wrote:
> 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.


>
> 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(-)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index 6a1ffd777db0..4b76256dcb31 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -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 */
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 8590099ac148..6734072980ee 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -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 11
> static __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;
> }
> --
> 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

2014-12-10 10:46:32

by Borislav Petkov

[permalink] [raw]
Subject: Re: Shorten efi regions output

On Wed, Dec 10, 2014 at 10:17:41AM +0800, Dave Young wrote:
> 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.

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?

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Subject: [tip:core/efi] x86/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.

Signed-off-by: Borislav Petkov <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Acked-by: Laszlo Ersek <[email protected]>
Acked-by: Dave Young <[email protected]>
Signed-off-by: Matt Fleming <[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(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index bfcb1a6..01aa47d 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -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
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index dbc8627..e859d56 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -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;
}
diff --git a/include/linux/efi.h b/include/linux/efi.h
index cf7e431..af5be03 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -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
/*