Re: [PATCH 1/2] efi: Export boot-services code and data as debugfs-blobs

From: Hans de Goede
Date: Sat Mar 31 2018 - 12:57:56 EST


Hi,

On 03/31/2018 04:10 PM, Greg Kroah-Hartman wrote:
On Sat, Mar 31, 2018 at 02:19:43PM +0200, Hans de Goede 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.

Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
arch/x86/platform/efi/quirks.c | 4 +++
drivers/firmware/efi/efi.c | 57 ++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+)

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 5b513ccffde4..0f968c7bcfec 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -374,6 +374,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;
+
for_each_efi_memory_desc(md) {
unsigned long long start = md->phys_addr;
unsigned long long size = md->num_pages << EFI_PAGE_SHIFT;
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index cd42f66a7c85..fddc5f706fd2 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -18,6 +18,7 @@
#include <linux/kobject.h>
#include <linux/module.h>
#include <linux/init.h>
+#include <linux/debugfs.h>
#include <linux/device.h>
#include <linux/efi.h>
#include <linux/of.h>
@@ -316,6 +317,59 @@ static __init int efivar_ssdt_load(void)
static inline int efivar_ssdt_load(void) { return 0; }
#endif
+#ifdef CONFIG_DEBUG_FS
+
+#define EFI_DEBUGFS_MAX_BLOBS 32
+
+struct debugfs_blob_wrapper debugfs_blob[EFI_DEBUGFS_MAX_BLOBS];
+
+static void __init efi_debugfs_init(void)
+{
+ struct dentry *efi_debugfs;
+ efi_memory_desc_t *md;
+ char name[32];
+ int type_count[EFI_BOOT_SERVICES_DATA + 1] = {};
+ int i = 0;
+
+ efi_debugfs = debugfs_create_dir("efi", NULL);
+ if (IS_ERR_OR_NULL(efi_debugfs)) {
+ pr_warn("Could not create efi debugfs entry\n");
+ return;
+ }

{sigh}

No, don't warn, or complain, or do anything else if a debugfs call
fails. Just keep on moving, you can always use the return value
properly in any future call if you need it, and no code flow should ever
care if a debugfs call succeeded or failed.

Ok.

/*
* We register the efi subsystem with the firmware subsystem and the
* efivars subsystem with the efi subsystem, if the system was booted with
@@ -360,6 +414,9 @@ static int __init efisubsys_init(void)
goto err_remove_group;
}
+ if (efi_enabled(EFI_DBG))
+ efi_debugfs_init();

You never remove the directory?

Correct, this is happening from a subsys_initcall as such there is no
efi_cleanup() counterpart.

Note the "if (efi_enabled(EFI_DBG))" check checks for efi=debug on the
kernel cmdline, so this only happens if the user asked for it.

Regards,

Hans