Hi,
On 09-10-2019 15:35, Ard Biesheuvel wrote:
On Wed, 9 Oct 2019 at 15:18, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
Hi,
On 09-10-2019 15:07, Ard Biesheuvel wrote:
On Fri, 4 Oct 2019 at 16:51, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
Sometimes it is useful to be able to dump the efi boot-services code and
data. This commit adds these as debugfs-blobs to /sys/kernel/debug/efi,
but only if efi=debug is passed on the kernel-commandline as this requires
not freeing those memory-regions, which costs 20+ MB of RAM.
Reviewed-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Acked-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
Changes in v5:
-Rename the EFI_BOOT_SERVICES flag to EFI_PRESERVE_BS_REGIONS
Changes in v4:
-Add new EFI_BOOT_SERVICES flag and use it to determine if the boot-services
memory segments are available (and thus if it makes sense to register the
debugfs bits for them)
Changes in v2:
-Do not call pr_err on debugfs call failures
---
arch/x86/platform/efi/efi.c | 1 +
arch/x86/platform/efi/quirks.c | 4 +++
drivers/firmware/efi/efi.c | 53 ++++++++++++++++++++++++++++++++++
include/linux/efi.h | 1 +
4 files changed, 59 insertions(+)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index c202e1b07e29..847730f7e74b 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -232,6 +232,7 @@ int __init efi_memblock_x86_reserve_range(void)
efi.memmap.desc_version);
memblock_reserve(pmap, efi.memmap.nr_map * efi.memmap.desc_size);
+ set_bit(EFI_PRESERVE_BS_REGIONS, &efi.flags);
Should we add a Kconfig symbol to opt into this behavior [set by the
driver in question], instead of always preserving all boot services
regions on all x86 systems?
This bit does not control anything, it merely signals that the arch early
boot EFI code keeps the boot-services code around, which is something
which the x86 code already does. Where as e.g. on arm / aarch64 this is
freed early on, this ties in with the other bits:
return 0;
}
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 3b9fd679cea9..fab12ebf0ada 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -411,6 +411,10 @@ void __init efi_free_boot_services(void)
int num_entries = 0;
void *new, *new_md;
+ /* Keep all regions for /sys/kernel/debug/efi */
+ if (efi_enabled(EFI_DBG))
+ return;
+
This is the point where normally on x86 we do actually free the boot-services
code which is a lot later then on other arches. And this new code actually
does change things to keep the boot-services code *forever* but only
if EFI debugging is enabled on the kernel commandline.
I get this part. But at some point, your driver is going to expect
this memory to be preserved even if EFI_DBG is not set, right? My
question was whether we should only opt into that if such a driver is
enabled in the first place.
Ah, I see. No even with CONFIG_EFI_EMBEDDED_FIRMWARE selected, the
boot-services code still gets free-ed. The efi_get_embedded_fw()
function from drivers/firmware/efi/embedded-firmware.c runs before
efi_free_boot_services() and it memdup-s any found firmwares, so it
does not cause the EFI boot-services code to stick around longer
then usual.
The only thing which does cause it to stick around is enabling
EFI debugging with efi=debug, so that the various efi segments
(not only the code-services ones) can be inspected as debugfs
blobs.
Basically this first patch of the series is independent of the
rest. It is part of the series, because adding new
efi_embedded_fw_desc structs to the table of firmwares to check
for becomes a lot easier when we can easily inspect the efi
segments and see if they contain the firmware we want.
As for Kconfig options, the compiling of
drivers/firmware/efi/embedded-firmware.c is controlled by
CONFIG_EFI_EMBEDDED_FIRMWARE which is a hidden option, which
can be selected by code which needs this. currently this is
only selected by CONFIG_TOUCHSCREEN_DMI which is defined
in drivers/platform/x86/Kconfig.
Regards,
Hans
