* Shorten efi regions output
@ 2014-12-09 9:58 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2014-12-09 9:58 UTC (permalink / raw)
To: linux-efi; +Cc: Laszlo Ersek, Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
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.
--
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Shorten efi regions output
@ 2014-12-09 9:58 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2014-12-09 9:58 UTC (permalink / raw)
To: linux-efi; +Cc: Laszlo Ersek, Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
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.
--
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2014-12-09 12:42 ` Ard Biesheuvel
0 siblings, 0 replies; 49+ messages in thread
From: Ard Biesheuvel @ 2014-12-09 12:42 UTC (permalink / raw)
To: Borislav Petkov; +Cc: linux-efi, Laszlo Ersek, Matt Fleming, Ricardo Neri, lkml
On 9 December 2014 at 10:58, Borislav Petkov <bp@alien8.de> 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.
> --
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2014-12-09 12:42 ` Ard Biesheuvel
0 siblings, 0 replies; 49+ messages in thread
From: Ard Biesheuvel @ 2014-12-09 12:42 UTC (permalink / raw)
To: Borislav Petkov; +Cc: linux-efi, Laszlo Ersek, Matt Fleming, Ricardo Neri, lkml
On 9 December 2014 at 10:58, Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org> 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.
> --
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2014-12-09 12:48 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2014-12-09 12:48 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, Laszlo Ersek, Matt Fleming, Ricardo Neri, lkml
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.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2014-12-09 12:48 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2014-12-09 12:48 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, Laszlo Ersek, Matt Fleming, Ricardo Neri, lkml
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.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2014-12-09 15:35 ` Laszlo Ersek
0 siblings, 0 replies; 49+ messages in thread
From: Laszlo Ersek @ 2014-12-09 15:35 UTC (permalink / raw)
To: Borislav Petkov, linux-efi
Cc: Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
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;
> }
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2014-12-09 15:35 ` Laszlo Ersek
0 siblings, 0 replies; 49+ messages in thread
From: Laszlo Ersek @ 2014-12-09 15:35 UTC (permalink / raw)
To: Borislav Petkov, linux-efi
Cc: Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
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;
> }
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2014-12-09 16:45 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2014-12-09 16:45 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: linux-efi, Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
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.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2014-12-09 16:45 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2014-12-09 16:45 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: linux-efi, Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
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.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2014-12-10 2:17 ` Dave Young
0 siblings, 0 replies; 49+ messages in thread
From: Dave Young @ 2014-12-10 2:17 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-efi, Laszlo Ersek, Ard Biesheuvel, Matt Fleming,
Ricardo Neri, lkml
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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2014-12-10 2:17 ` Dave Young
0 siblings, 0 replies; 49+ messages in thread
From: Dave Young @ 2014-12-10 2:17 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-efi, Laszlo Ersek, Ard Biesheuvel, Matt Fleming,
Ricardo Neri, lkml
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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2014-12-10 10:46 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2014-12-10 10:46 UTC (permalink / raw)
To: Dave Young
Cc: linux-efi, Laszlo Ersek, Ard Biesheuvel, Matt Fleming,
Ricardo Neri, lkml
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.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2014-12-10 10:46 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2014-12-10 10:46 UTC (permalink / raw)
To: Dave Young
Cc: linux-efi, Laszlo Ersek, Ard Biesheuvel, Matt Fleming,
Ricardo Neri, lkml
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.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2015-01-05 14:03 ` Matt Fleming
0 siblings, 0 replies; 49+ messages in thread
From: Matt Fleming @ 2015-01-05 14:03 UTC (permalink / raw)
To: Borislav Petkov
Cc: Dave Young, linux-efi, Laszlo Ersek, Ard Biesheuvel,
Matt Fleming, Ricardo Neri, lkml
On Wed, 10 Dec, at 11:46:28AM, Borislav Petkov wrote:
> 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?
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?
--
Matt Fleming, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2015-01-05 14:03 ` Matt Fleming
0 siblings, 0 replies; 49+ messages in thread
From: Matt Fleming @ 2015-01-05 14:03 UTC (permalink / raw)
To: Borislav Petkov
Cc: Dave Young, linux-efi, Laszlo Ersek, Ard Biesheuvel,
Matt Fleming, Ricardo Neri, lkml
On Wed, 10 Dec, at 11:46:28AM, Borislav Petkov wrote:
> 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?
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?
--
Matt Fleming, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
2015-01-05 14:03 ` Matt Fleming
(?)
@ 2015-01-05 15:00 ` Laszlo Ersek
2015-01-21 5:48 ` Jon Masters
-1 siblings, 1 reply; 49+ messages in thread
From: Laszlo Ersek @ 2015-01-05 15:00 UTC (permalink / raw)
To: Matt Fleming, Borislav Petkov
Cc: Dave Young, linux-efi, Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On 01/05/15 15:03, Matt Fleming wrote:
> On Wed, 10 Dec, at 11:46:28AM, Borislav Petkov wrote:
>> 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?
>
> 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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
2015-01-05 15:00 ` Laszlo Ersek
@ 2015-01-21 5:48 ` Jon Masters
2015-01-21 10:06 ` Borislav Petkov
0 siblings, 1 reply; 49+ messages in thread
From: Jon Masters @ 2015-01-21 5:48 UTC (permalink / raw)
To: Laszlo Ersek, Matt Fleming, Borislav Petkov
Cc: Dave Young, linux-efi, Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On 1/5/15, 10:00 AM, Laszlo Ersek wrote:
> On 01/05/15 15:03, Matt Fleming wrote:
>> On Wed, 10 Dec, at 11:46:28AM, Borislav Petkov wrote:
>>> 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?
>>
>> 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.
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.
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2015-01-21 10:06 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2015-01-21 10:06 UTC (permalink / raw)
To: Jon Masters
Cc: Laszlo Ersek, Matt Fleming, Dave Young, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On Wed, Jan 21, 2015 at 12:48:58AM -0500, Jon Masters wrote:
> 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...
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
@ 2015-01-21 10:06 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2015-01-21 10:06 UTC (permalink / raw)
To: Jon Masters
Cc: Laszlo Ersek, Matt Fleming, Dave Young, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On Wed, Jan 21, 2015 at 12:48:58AM -0500, Jon Masters wrote:
> 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...
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Shorten efi regions output
2015-01-21 10:06 ` Borislav Petkov
(?)
@ 2015-01-26 10:49 ` Matt Fleming
2015-01-30 16:43 ` Borislav Petkov
-1 siblings, 1 reply; 49+ messages in thread
From: Matt Fleming @ 2015-01-26 10:49 UTC (permalink / raw)
To: Borislav Petkov
Cc: Jon Masters, Laszlo Ersek, Dave Young, linux-efi, Ard Biesheuvel,
Matt Fleming, Ricardo Neri, lkml
On Wed, 21 Jan, at 11:06:33AM, Borislav Petkov wrote:
> On Wed, Jan 21, 2015 at 12:48:58AM -0500, Jon Masters wrote:
> > 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.
--
Matt Fleming, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-01-30 16:43 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2015-01-30 16:43 UTC (permalink / raw)
To: Matt Fleming
Cc: Jon Masters, Laszlo Ersek, Dave Young, linux-efi, Ard Biesheuvel,
Matt Fleming, Ricardo Neri, lkml
From: Borislav Petkov <bp@suse.de>
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 <bp@suse.de>
Link: http://lkml.kernel.org/r/20141209095843.GA3990@pd.tnic
---
arch/x86/platform/efi/efi.c | 5 ++++-
include/linux/efi.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -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
/*
--
2.2.0.33.gc18b867
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-01-30 16:43 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2015-01-30 16:43 UTC (permalink / raw)
To: Matt Fleming
Cc: Jon Masters, Laszlo Ersek, Dave Young, linux-efi, Ard Biesheuvel,
Matt Fleming, Ricardo Neri, lkml
From: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
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 <bp-l3A5Bk7waGM@public.gmane.org>
Link: http://lkml.kernel.org/r/20141209095843.GA3990-fF5Pk5pvG8Y@public.gmane.org
---
arch/x86/platform/efi/efi.c | 5 ++++-
include/linux/efi.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -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
/*
--
2.2.0.33.gc18b867
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-01-30 16:58 ` Laszlo Ersek
0 siblings, 0 replies; 49+ messages in thread
From: Laszlo Ersek @ 2015-01-30 16:58 UTC (permalink / raw)
To: Borislav Petkov, Matt Fleming
Cc: Jon Masters, Dave Young, linux-efi, Ard Biesheuvel, Matt Fleming,
Ricardo Neri, lkml
On 01/30/15 17:43, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 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 <bp@suse.de>
> Link: http://lkml.kernel.org/r/20141209095843.GA3990@pd.tnic
> ---
> arch/x86/platform/efi/efi.c | 5 ++++-
> include/linux/efi.h | 1 +
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-01-30 16:58 ` Laszlo Ersek
0 siblings, 0 replies; 49+ messages in thread
From: Laszlo Ersek @ 2015-01-30 16:58 UTC (permalink / raw)
To: Borislav Petkov, Matt Fleming
Cc: Jon Masters, Dave Young, linux-efi, Ard Biesheuvel, Matt Fleming,
Ricardo Neri, lkml
On 01/30/15 17:43, Borislav Petkov wrote:
> From: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
> 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 <bp-l3A5Bk7waGM@public.gmane.org>
> Link: http://lkml.kernel.org/r/20141209095843.GA3990-fF5Pk5pvG8Y@public.gmane.org
> ---
> arch/x86/platform/efi/efi.c | 5 ++++-
> include/linux/efi.h | 1 +
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-01-30 18:06 ` Randy Dunlap
0 siblings, 0 replies; 49+ messages in thread
From: Randy Dunlap @ 2015-01-30 18:06 UTC (permalink / raw)
To: Borislav Petkov, Matt Fleming
Cc: Jon Masters, Laszlo Ersek, Dave Young, linux-efi, Ard Biesheuvel,
Matt Fleming, Ricardo Neri, lkml
On 01/30/15 08:43, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 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.
> ... and hide the memory regions dump behind it. Make it default-off.
>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Link: http://lkml.kernel.org/r/20141209095843.GA3990@pd.tnic
> ---
> arch/x86/platform/efi/efi.c | 5 ++++-
> include/linux/efi.h | 1 +
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -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
> /*
>
--
~Randy
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-01-30 18:06 ` Randy Dunlap
0 siblings, 0 replies; 49+ messages in thread
From: Randy Dunlap @ 2015-01-30 18:06 UTC (permalink / raw)
To: Borislav Petkov, Matt Fleming
Cc: Jon Masters, Laszlo Ersek, Dave Young, linux-efi, Ard Biesheuvel,
Matt Fleming, Ricardo Neri, lkml
On 01/30/15 08:43, Borislav Petkov wrote:
> From: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
> 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.
> ... and hide the memory regions dump behind it. Make it default-off.
>
> Signed-off-by: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
> Link: http://lkml.kernel.org/r/20141209095843.GA3990-fF5Pk5pvG8Y@public.gmane.org
> ---
> arch/x86/platform/efi/efi.c | 5 ++++-
> include/linux/efi.h | 1 +
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -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
> /*
>
--
~Randy
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-01-30 21:17 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2015-01-30 21:17 UTC (permalink / raw)
To: Randy Dunlap
Cc: Matt Fleming, Jon Masters, Laszlo Ersek, Dave Young, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On Fri, Jan 30, 2015 at 10:06:13AM -0800, Randy Dunlap wrote:
> Please update Documentation/kernel-parameters.txt also.
Good idea. Done. Thanks.
:-)
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-01-30 21:17 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2015-01-30 21:17 UTC (permalink / raw)
To: Randy Dunlap
Cc: Matt Fleming, Jon Masters, Laszlo Ersek, Dave Young, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On Fri, Jan 30, 2015 at 10:06:13AM -0800, Randy Dunlap wrote:
> Please update Documentation/kernel-parameters.txt also.
Good idea. Done. Thanks.
:-)
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-04 12:18 ` Parmeshwr Prasad
0 siblings, 0 replies; 49+ messages in thread
From: Parmeshwr Prasad @ 2015-02-04 12:18 UTC (permalink / raw)
To: Borislav Petkov, Matt Fleming, Randy Dunlap, Jon Masters,
Laszlo Ersek, Dave Young, linux-efi, Ard Biesheuvel,
Matt Fleming, Ricardo Neri, lkml, parmeshwr_prasad
Some 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.
>From 7fbac896ab87f1b37646ac2f49bb8216ec330642 Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
Date: Wed, 4 Feb 2015 06:50:32 -0500
Subject: [PATCH] efi, x86: Add a debug option to the efi= cmdline
Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
---
drivers/video/fbdev/efifb.c | 49 +++++++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 4bfff34..505bc56 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -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
Fri, Jan 30, 2015 at 12:06:13PM -0600, Randy Dunlap wrote:
> On 01/30/15 08:43, Borislav Petkov wrote:
> > From: Borislav Petkov <bp@suse.de>
> > 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.
>
> > ... and hide the memory regions dump behind it. Make it default-off.
> >
> > Signed-off-by: Borislav Petkov <bp@suse.de>
> > Link: http://lkml.kernel.org/r/20141209095843.GA3990@pd.tnic
> > ---
> > arch/x86/platform/efi/efi.c | 5 ++++-
> > include/linux/efi.h | 1 +
> > 2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> > index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
> > --- a/include/linux/efi.h
> > +++ b/include/linux/efi.h
> > @@ -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
> > /*
> >
>
>
> --
> ~Randy
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-04 12:18 ` Parmeshwr Prasad
0 siblings, 0 replies; 49+ messages in thread
From: Parmeshwr Prasad @ 2015-02-04 12:18 UTC (permalink / raw)
To: Borislav Petkov, Matt Fleming, Randy Dunlap, Jon Masters,
Laszlo Ersek, Dave Young, linux-efi, Ard Biesheuvel,
Matt Fleming, Ricardo Neri, lkml, parmeshwr_prasad-8PEkshWhKlo
Some 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.
>From 7fbac896ab87f1b37646ac2f49bb8216ec330642 Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <parmeshwr_prasad-8PEkshWhKlo@public.gmane.org>
Date: Wed, 4 Feb 2015 06:50:32 -0500
Subject: [PATCH] efi, x86: Add a debug option to the efi= cmdline
Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad-8PEkshWhKlo@public.gmane.org>
---
drivers/video/fbdev/efifb.c | 49 +++++++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 4bfff34..505bc56 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -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
Fri, Jan 30, 2015 at 12:06:13PM -0600, Randy Dunlap wrote:
> On 01/30/15 08:43, Borislav Petkov wrote:
> > From: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
> > 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.
>
> > ... and hide the memory regions dump behind it. Make it default-off.
> >
> > Signed-off-by: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
> > Link: http://lkml.kernel.org/r/20141209095843.GA3990-fF5Pk5pvG8Y@public.gmane.org
> > ---
> > arch/x86/platform/efi/efi.c | 5 ++++-
> > include/linux/efi.h | 1 +
> > 2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> > index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
> > --- a/include/linux/efi.h
> > +++ b/include/linux/efi.h
> > @@ -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
> > /*
> >
>
>
> --
> ~Randy
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-05 3:18 ` Dave Young
0 siblings, 0 replies; 49+ messages in thread
From: Dave Young @ 2015-02-05 3:18 UTC (permalink / raw)
To: Borislav Petkov
Cc: Matt Fleming, Jon Masters, Laszlo Ersek, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On 01/30/15 at 05:43pm, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 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 <bp@suse.de>
> Link: http://lkml.kernel.org/r/20141209095843.GA3990@pd.tnic
> ---
> arch/x86/platform/efi/efi.c | 5 ++++-
> include/linux/efi.h | 1 +
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-05 3:18 ` Dave Young
0 siblings, 0 replies; 49+ messages in thread
From: Dave Young @ 2015-02-05 3:18 UTC (permalink / raw)
To: Borislav Petkov
Cc: Matt Fleming, Jon Masters, Laszlo Ersek, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On 01/30/15 at 05:43pm, Borislav Petkov wrote:
> From: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
> 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 <bp-l3A5Bk7waGM@public.gmane.org>
> Link: http://lkml.kernel.org/r/20141209095843.GA3990-fF5Pk5pvG8Y@public.gmane.org
> ---
> arch/x86/platform/efi/efi.c | 5 ++++-
> include/linux/efi.h | 1 +
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-05 8:11 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2015-02-05 8:11 UTC (permalink / raw)
To: Dave Young
Cc: Matt Fleming, Jon Masters, Laszlo Ersek, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On Thu, Feb 05, 2015 at 11:18:46AM +0800, Dave Young wrote:
> > diff --git a/include/linux/efi.h b/include/linux/efi.h
> > index 0238d612750e..14cec75d7e74 100644
> > --- a/include/linux/efi.h
> > +++ b/include/linux/efi.h
> > @@ -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 :-)
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-05 8:11 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2015-02-05 8:11 UTC (permalink / raw)
To: Dave Young
Cc: Matt Fleming, Jon Masters, Laszlo Ersek, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On Thu, Feb 05, 2015 at 11:18:46AM +0800, Dave Young wrote:
> > diff --git a/include/linux/efi.h b/include/linux/efi.h
> > index 0238d612750e..14cec75d7e74 100644
> > --- a/include/linux/efi.h
> > +++ b/include/linux/efi.h
> > @@ -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 :-)
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-05 8:41 ` Dave Young
0 siblings, 0 replies; 49+ messages in thread
From: Dave Young @ 2015-02-05 8:41 UTC (permalink / raw)
To: Borislav Petkov
Cc: Matt Fleming, Jon Masters, Laszlo Ersek, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On 02/05/15 at 09:11am, Borislav Petkov wrote:
> On Thu, Feb 05, 2015 at 11:18:46AM +0800, Dave Young wrote:
> > > diff --git a/include/linux/efi.h b/include/linux/efi.h
> > > index 0238d612750e..14cec75d7e74 100644
> > > --- a/include/linux/efi.h
> > > +++ b/include/linux/efi.h
> > > @@ -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 :-)
Interesting, you are right :)
Acked-by: Dave Young <dyoung@redhat.com>
Thanks
Dave
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-05 8:41 ` Dave Young
0 siblings, 0 replies; 49+ messages in thread
From: Dave Young @ 2015-02-05 8:41 UTC (permalink / raw)
To: Borislav Petkov
Cc: Matt Fleming, Jon Masters, Laszlo Ersek, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On 02/05/15 at 09:11am, Borislav Petkov wrote:
> On Thu, Feb 05, 2015 at 11:18:46AM +0800, Dave Young wrote:
> > > diff --git a/include/linux/efi.h b/include/linux/efi.h
> > > index 0238d612750e..14cec75d7e74 100644
> > > --- a/include/linux/efi.h
> > > +++ b/include/linux/efi.h
> > > @@ -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 :-)
Interesting, you are right :)
Acked-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Thanks
Dave
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-05 10:44 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2015-02-05 10:44 UTC (permalink / raw)
To: Matt Fleming
Cc: Dave Young, Jon Masters, Laszlo Ersek, linux-efi, Ard Biesheuvel,
Matt Fleming, Ricardo Neri, lkml
From: Borislav Petkov <bp@suse.de>
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 <bp@suse.de>
Link: http://lkml.kernel.org/r/20141209095843.GA3990@pd.tnic
Acked-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Dave Young <dyoung@redhat.com>
---
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 176d4fe4f076..101ab5601c2d 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -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
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -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
/*
--
2.2.0.33.gc18b867
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-05 10:44 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2015-02-05 10:44 UTC (permalink / raw)
To: Matt Fleming
Cc: Dave Young, Jon Masters, Laszlo Ersek, linux-efi, Ard Biesheuvel,
Matt Fleming, Ricardo Neri, lkml
From: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
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 <bp-l3A5Bk7waGM@public.gmane.org>
Link: http://lkml.kernel.org/r/20141209095843.GA3990-fF5Pk5pvG8Y@public.gmane.org
Acked-by: Laszlo Ersek <lersek-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Acked-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
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 176d4fe4f076..101ab5601c2d 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -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
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -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
/*
--
2.2.0.33.gc18b867
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-05 12:45 ` Parmeshwr Prasad
0 siblings, 0 replies; 49+ messages in thread
From: Parmeshwr Prasad @ 2015-02-05 12:45 UTC (permalink / raw)
To: Borislav Petkov
Cc: Matt Fleming, Dave Young, Jon Masters, Laszlo Ersek, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
It is better if we add some printk from efifb messages.
Please review the below patch.
>From 7fbac896ab87f1b37646ac2f49bb8216ec330642 Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
Date: Wed, 4 Feb 2015 06:50:32 -0500
Subject: [PATCH] efi, x86: Add a debug option to the efi= cmdline
Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
---
drivers/video/fbdev/efifb.c | 49 +++++++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 4bfff34..505bc56 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -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
On Thu, Feb 05, 2015 at 04:44:41AM -0600, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 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 <bp@suse.de>
> Link: http://lkml.kernel.org/r/20141209095843.GA3990@pd.tnic
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> Acked-by: Dave Young <dyoung@redhat.com>
> ---
> 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 176d4fe4f076..101ab5601c2d 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -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
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -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
> /*
> --
> 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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-05 12:45 ` Parmeshwr Prasad
0 siblings, 0 replies; 49+ messages in thread
From: Parmeshwr Prasad @ 2015-02-05 12:45 UTC (permalink / raw)
To: Borislav Petkov
Cc: Matt Fleming, Dave Young, Jon Masters, Laszlo Ersek, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
It is better if we add some printk from efifb messages.
Please review the below patch.
>From 7fbac896ab87f1b37646ac2f49bb8216ec330642 Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <parmeshwr_prasad-8PEkshWhKlo@public.gmane.org>
Date: Wed, 4 Feb 2015 06:50:32 -0500
Subject: [PATCH] efi, x86: Add a debug option to the efi= cmdline
Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad-8PEkshWhKlo@public.gmane.org>
---
drivers/video/fbdev/efifb.c | 49 +++++++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 4bfff34..505bc56 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -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
On Thu, Feb 05, 2015 at 04:44:41AM -0600, Borislav Petkov wrote:
> From: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
> 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 <bp-l3A5Bk7waGM@public.gmane.org>
> Link: http://lkml.kernel.org/r/20141209095843.GA3990-fF5Pk5pvG8Y@public.gmane.org
> Acked-by: Laszlo Ersek <lersek-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Acked-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> 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 176d4fe4f076..101ab5601c2d 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -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
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index dbc8627a5cdf..e859d56ce9f8 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 0238d612750e..14cec75d7e74 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -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
> /*
> --
> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
2015-02-05 12:45 ` Parmeshwr Prasad
(?)
@ 2015-02-05 14:28 ` Borislav Petkov
2015-02-06 6:00 ` Parmeshwr Prasad
-1 siblings, 1 reply; 49+ messages in thread
From: Borislav Petkov @ 2015-02-05 14:28 UTC (permalink / raw)
To: Parmeshwr Prasad
Cc: Matt Fleming, Dave Young, Jon Masters, Laszlo Ersek, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On Thu, Feb 05, 2015 at 07:45:10AM -0500, Parmeshwr Prasad wrote:
> It is better if we add some printk from efifb messages.
> Please review the below patch.
First of all, we saw you patch.
Then,
> From 7fbac896ab87f1b37646ac2f49bb8216ec330642 Mon Sep 17 00:00:00 2001
> From: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
> 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.
>
> Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
> ---
> drivers/video/fbdev/efifb.c | 49 +++++++++++++++++++++++++--------------------
> 1 file changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index 4bfff34..505bc56 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -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.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-06 6:00 ` Parmeshwr Prasad
0 siblings, 0 replies; 49+ messages in thread
From: Parmeshwr Prasad @ 2015-02-06 6:00 UTC (permalink / raw)
To: Borislav Petkov
Cc: Matt Fleming, Dave Young, Jon Masters, Laszlo Ersek, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On Thu, Feb 05, 2015 at 08:28:58AM -0600, Borislav Petkov wrote:
> On Thu, Feb 05, 2015 at 07:45:10AM -0500, Parmeshwr Prasad wrote:
> > It is better if we add some printk from efifb messages.
> > Please review the below patch.
>
> First of all, we saw you patch.
>
> Then,
>
> > From 7fbac896ab87f1b37646ac2f49bb8216ec330642 Mon Sep 17 00:00:00 2001
> > From: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
> > 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.
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.
>
> >
> > Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
> > ---
> > drivers/video/fbdev/efifb.c | 49 +++++++++++++++++++++++++--------------------
> > 1 file changed, 27 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> > index 4bfff34..505bc56 100644
> > --- a/drivers/video/fbdev/efifb.c
> > +++ b/drivers/video/fbdev/efifb.c
> > @@ -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.
>
> --
> 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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-06 6:00 ` Parmeshwr Prasad
0 siblings, 0 replies; 49+ messages in thread
From: Parmeshwr Prasad @ 2015-02-06 6:00 UTC (permalink / raw)
To: Borislav Petkov
Cc: Matt Fleming, Dave Young, Jon Masters, Laszlo Ersek, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On Thu, Feb 05, 2015 at 08:28:58AM -0600, Borislav Petkov wrote:
> On Thu, Feb 05, 2015 at 07:45:10AM -0500, Parmeshwr Prasad wrote:
> > It is better if we add some printk from efifb messages.
> > Please review the below patch.
>
> First of all, we saw you patch.
>
> Then,
>
> > From 7fbac896ab87f1b37646ac2f49bb8216ec330642 Mon Sep 17 00:00:00 2001
> > From: Parmeshwr Prasad <parmeshwr_prasad-8PEkshWhKlo@public.gmane.org>
> > 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.
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.
>
> >
> > Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad-8PEkshWhKlo@public.gmane.org>
> > ---
> > drivers/video/fbdev/efifb.c | 49 +++++++++++++++++++++++++--------------------
> > 1 file changed, 27 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> > index 4bfff34..505bc56 100644
> > --- a/drivers/video/fbdev/efifb.c
> > +++ b/drivers/video/fbdev/efifb.c
> > @@ -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.
>
> --
> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-06 10:49 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2015-02-06 10:49 UTC (permalink / raw)
To: Parmeshwr Prasad
Cc: Matt Fleming, Dave Young, Jon Masters, Laszlo Ersek, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On Fri, Feb 06, 2015 at 01:00:12AM -0500, Parmeshwr Prasad wrote:
> 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.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-06 10:49 ` Borislav Petkov
0 siblings, 0 replies; 49+ messages in thread
From: Borislav Petkov @ 2015-02-06 10:49 UTC (permalink / raw)
To: Parmeshwr Prasad
Cc: Matt Fleming, Dave Young, Jon Masters, Laszlo Ersek, linux-efi,
Ard Biesheuvel, Matt Fleming, Ricardo Neri, lkml
On Fri, Feb 06, 2015 at 01:00:12AM -0500, Parmeshwr Prasad wrote:
> 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.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-24 22:33 ` Matt Fleming
0 siblings, 0 replies; 49+ messages in thread
From: Matt Fleming @ 2015-02-24 22:33 UTC (permalink / raw)
To: Borislav Petkov
Cc: Dave Young, Jon Masters, Laszlo Ersek, linux-efi, Ard Biesheuvel,
Matt Fleming, Ricardo Neri, lkml
On Thu, 05 Feb, at 11:44:41AM, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 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 <bp@suse.de>
> Link: http://lkml.kernel.org/r/20141209095843.GA3990@pd.tnic
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> Acked-by: Dave Young <dyoung@redhat.com>
> ---
> 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!
--
Matt Fleming, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2] efi, x86: Add a "debug" option to the efi= cmdline
@ 2015-02-24 22:33 ` Matt Fleming
0 siblings, 0 replies; 49+ messages in thread
From: Matt Fleming @ 2015-02-24 22:33 UTC (permalink / raw)
To: Borislav Petkov
Cc: Dave Young, Jon Masters, Laszlo Ersek, linux-efi, Ard Biesheuvel,
Matt Fleming, Ricardo Neri, lkml
On Thu, 05 Feb, at 11:44:41AM, Borislav Petkov wrote:
> From: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
> 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 <bp-l3A5Bk7waGM@public.gmane.org>
> Link: http://lkml.kernel.org/r/20141209095843.GA3990-fF5Pk5pvG8Y@public.gmane.org
> Acked-by: Laszlo Ersek <lersek-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Acked-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> 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!
--
Matt Fleming, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 49+ messages in thread
* [tip:core/efi] x86/efi: Add a "debug" option to the efi= cmdline
2014-12-09 9:58 ` Borislav Petkov
` (3 preceding siblings ...)
(?)
@ 2015-04-02 12:27 ` tip-bot for Borislav Petkov
-1 siblings, 0 replies; 49+ messages in thread
From: tip-bot for Borislav Petkov @ 2015-04-02 12:27 UTC (permalink / raw)
To: linux-tip-commits
Cc: dyoung, lersek, hpa, bp, matt.fleming, mingo, linux-kernel, tglx
Commit-ID: fed6cefe3b6e862dcc74d07324478caa07e84eaf
Gitweb: http://git.kernel.org/tip/fed6cefe3b6e862dcc74d07324478caa07e84eaf
Author: Borislav Petkov <bp@suse.de>
AuthorDate: Thu, 5 Feb 2015 11:44:41 +0100
Committer: Matt Fleming <matt.fleming@intel.com>
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 <bp@suse.de>
Link: http://lkml.kernel.org/r/20141209095843.GA3990@pd.tnic
Acked-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Dave Young <dyoung@redhat.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
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
/*
^ permalink raw reply related [flat|nested] 49+ messages in thread
end of thread, other threads:[~2015-04-02 12:27 UTC | newest]
Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-09 9:58 Shorten efi regions output Borislav Petkov
2014-12-09 9:58 ` Borislav Petkov
2014-12-09 12:42 ` Ard Biesheuvel
2014-12-09 12:42 ` Ard Biesheuvel
2014-12-09 12:48 ` Borislav Petkov
2014-12-09 12:48 ` Borislav Petkov
2014-12-09 15:35 ` Laszlo Ersek
2014-12-09 15:35 ` Laszlo Ersek
2014-12-09 16:45 ` Borislav Petkov
2014-12-09 16:45 ` Borislav Petkov
2014-12-10 2:17 ` Dave Young
2014-12-10 2:17 ` Dave Young
2014-12-10 10:46 ` Borislav Petkov
2014-12-10 10:46 ` Borislav Petkov
2015-01-05 14:03 ` Matt Fleming
2015-01-05 14:03 ` Matt Fleming
2015-01-05 15:00 ` Laszlo Ersek
2015-01-21 5:48 ` Jon Masters
2015-01-21 10:06 ` Borislav Petkov
2015-01-21 10:06 ` Borislav Petkov
2015-01-26 10:49 ` Matt Fleming
2015-01-30 16:43 ` [PATCH] efi, x86: Add a "debug" option to the efi= cmdline Borislav Petkov
2015-01-30 16:43 ` Borislav Petkov
2015-01-30 16:58 ` Laszlo Ersek
2015-01-30 16:58 ` Laszlo Ersek
2015-01-30 18:06 ` Randy Dunlap
2015-01-30 18:06 ` Randy Dunlap
2015-01-30 21:17 ` Borislav Petkov
2015-01-30 21:17 ` Borislav Petkov
2015-02-04 12:18 ` Parmeshwr Prasad
2015-02-04 12:18 ` Parmeshwr Prasad
2015-02-05 3:18 ` Dave Young
2015-02-05 3:18 ` Dave Young
2015-02-05 8:11 ` Borislav Petkov
2015-02-05 8:11 ` Borislav Petkov
2015-02-05 8:41 ` Dave Young
2015-02-05 8:41 ` Dave Young
2015-02-05 10:44 ` [PATCH v2] " Borislav Petkov
2015-02-05 10:44 ` Borislav Petkov
2015-02-05 12:45 ` Parmeshwr Prasad
2015-02-05 12:45 ` Parmeshwr Prasad
2015-02-05 14:28 ` Borislav Petkov
2015-02-06 6:00 ` Parmeshwr Prasad
2015-02-06 6:00 ` Parmeshwr Prasad
2015-02-06 10:49 ` Borislav Petkov
2015-02-06 10:49 ` Borislav Petkov
2015-02-24 22:33 ` Matt Fleming
2015-02-24 22:33 ` Matt Fleming
2015-04-02 12:27 ` [tip:core/efi] x86/efi: " tip-bot for Borislav Petkov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.