From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: ARC-Seal: i=1; a=rsa-sha256; t=1522515452; cv=none; d=google.com; s=arc-20160816; b=VBJLkZXf4auaH+OQNDRs38ltKc3Yoo9M3pdIPDwJ6WDP6ebmSRljPm9Gj+RE1rHF1m OdOrh4GOWWtAAKhT4Mtu7BjWwSfrM36F1asL4orLbMWXBpEH8+vN3jE4elqsiIunkCZe wNMfWDtMzIYS0Oj8KjjBmyGa85m3FUr/vfmvGMRf8By8r9dDxxRd4boDklHN4jiuoEmG 7WgHzBWnZZ3IbsC8Owvne8jpSNPi2xmI+yYnpO+4ZFuHl120+sXvRYgZMWPoD1Yhxfob Yu3fLRJPjnsQPSeaIdtVvzEHfSo+Ss6yVlW6SH1WBBTfigxQpgaZYwWLRE/WMBCuiBTC +0xA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :arc-authentication-results; bh=Jo2ZxghcO1oN5+XmUA8x1OdnQb5agB+P3VCpHDn06Rg=; b=iCxmPvFkFiZCOK2wUOHHOxncDERlyTm1ilS5diop3c8eJYnhN+HKY+A6ZXVkLFsVD2 4OSzt3uhlF/iidUGCzkYQ3Fyrbbr8rKDl+9I/zPkRSEClq3t/DJEPgYfY7LynhH9n/Ii X/RMCJ4DB1iWTuAoAWiHqNfvB4dNiL1T1OOATYxA87HAhTf/qVsPowDO3arKYuP+Si/r YpukmfF+y/vtdswNx5VHZbY6otw3ZISl+VNVp1qGGX47XyFNpds3lTAiuKs7fsJRaQ5+ yyB83haeYTzY4W88/x2YTqDolbW8UyWtXg/Dszc22XE/TsT9O9v0qt0QMgfo4vfeaU2s jVNA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of hdegoede@redhat.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=hdegoede@redhat.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Authentication-Results: mx.google.com; spf=pass (google.com: domain of hdegoede@redhat.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=hdegoede@redhat.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com X-Google-Smtp-Source: AIpwx4/4J9yNrmsUbkTN1HCyy+WnacVwaSw4tedVJGwojTHAZ+I4PBQmUekAUu2uGC14YSrRVzCqvg== Subject: Re: [PATCH 1/2] efi: Export boot-services code and data as debugfs-blobs To: Greg Kroah-Hartman Cc: Ard Biesheuvel , "Luis R . Rodriguez" , Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , linux-kernel@vger.kernel.org, Peter Jones , Dave Olsthoorn , x86@kernel.org, linux-efi@vger.kernel.org References: <20180331121944.8618-1-hdegoede@redhat.com> <20180331141030.GB1074@kroah.com> From: Hans de Goede Message-ID: <646d64dc-0c14-a96a-f91b-787a74f7ca35@redhat.com> Date: Sat, 31 Mar 2018 18:57:29 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180331141030.GB1074@kroah.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1596455689779545050?= X-GMAIL-MSGID: =?utf-8?q?1596473163048531745?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: 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 >> --- >> 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 >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -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