LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Darren Hart <dvhart@infradead.org>,
	Andy Shevchenko <andy@infradead.org>,
	"Luis R . Rodriguez" <mcgrof@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H . Peter Anvin" <hpa@zytor.com>,
	platform-driver-x86@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Peter Jones <pjones@redhat.com>, Dave Olsthoorn <dave@bewaar.me>,
	Will Deacon <will.deacon@arm.com>,
	Andy Lutomirski <luto@kernel.org>,
	Matt Fleming <matt@codeblueprint.co.uk>,
	David Howells <dhowells@redhat.com>,
	Mimi Zohar <zohar@linux.vnet.ibm.com>,
	Josh Triplett <josh@joshtriplett.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Martin Fuzzey <mfuzzey@parkeon.com>,
	Kees Cook <keescook@chromium.org>,
	Kalle Valo <kvalo@codeaurora.org>,
	Arend Van Spriel <arend.vanspriel@broadcom.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Nicolas Broeking <nbroeking@me.com>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Torsten Duwe <duwe@suse.de>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	linux-efi@vger.kernel.org
Subject: Re: [PATCH v3 1/5] efi: Export boot-services code and data as debugfs-blobs
Date: Mon, 16 Apr 2018 10:23:17 +0200	[thread overview]
Message-ID: <CAKv+Gu-DZ86y5pmQ+-gQWLHAcs47H-cixCM1pBKmKZXA=+jRmg@mail.gmail.com> (raw)
In-Reply-To: <20180408174014.21908-2-hdegoede@redhat.com>

Hallo Hans,

On 8 April 2018 at 19:40, Hans de Goede <hdegoede@redhat.com> 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@redhat.com>
> ---
> Changes in v2:
> -Do not call pr_err on debugfs call failures
> ---
>  arch/x86/platform/efi/quirks.c |  4 +++
>  drivers/firmware/efi/efi.c     | 53 ++++++++++++++++++++++++++++++++++
>  2 files changed, 57 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..10c896e8b82b 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,55 @@ 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
> +
> +static 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))
> +               return;
> +
> +       for_each_efi_memory_desc(md) {
> +               switch (md->type) {
> +               case EFI_BOOT_SERVICES_CODE:
> +                       snprintf(name, sizeof(name), "boot_services_code%d",
> +                                type_count[md->type]++);
> +                       break;
> +               case EFI_BOOT_SERVICES_DATA:
> +                       snprintf(name, sizeof(name), "boot_services_data%d",
> +                                type_count[md->type]++);
> +                       break;
> +               default:
> +                       continue;
> +               }
> +
> +               debugfs_blob[i].size = md->num_pages << EFI_PAGE_SHIFT;
> +               debugfs_blob[i].data = memremap(md->phys_addr,
> +                                               debugfs_blob[i].size,
> +                                               MEMREMAP_WB);
> +               if (!debugfs_blob[i].data)
> +                       continue;
> +
> +               debugfs_create_blob(name, 0400, efi_debugfs, &debugfs_blob[i]);
> +               i++;
> +               if (i == EFI_DEBUGFS_MAX_BLOBS)
> +                       break;
> +       }
> +}
> +#else
> +static inline void efi_debugfs_init(void) {}
> +#endif
> +
>  /*
>   * 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 +410,9 @@ static int __init efisubsys_init(void)
>                 goto err_remove_group;
>         }
>
> +       if (efi_enabled(EFI_DBG))
> +               efi_debugfs_init();
> +

This doesn't really make any sense on non-x86. The boot services
regions are released to the kernel for general allocation, and so
exposing them this way only makes sense if you keep them as you do for
x86.

Could you please try to make this call specific to situations where it
makes sense? I don't mind allocating a new EFI_xxx flag for preserving
the boot services regions so we could decide to set it for ARM/arm64
as well in certain cases in the future.

>         return 0;
>
>  err_remove_group:
> --
> 2.17.0
>

  reply	other threads:[~2018-04-16  8:23 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-08 17:40 [PATCH v3 0/5] efi/firmware/platform-x86: Add EFI embedded fw support Hans de Goede
2018-04-08 17:40 ` [PATCH v3 1/5] efi: Export boot-services code and data as debugfs-blobs Hans de Goede
2018-04-16  8:23   ` Ard Biesheuvel [this message]
2018-04-24 13:11     ` Hans de Goede
2018-04-23 11:55   ` Greg Kroah-Hartman
2018-04-08 17:40 ` [PATCH v3 2/5] efi: Add embedded peripheral firmware support Hans de Goede
2018-04-16  8:28   ` Ard Biesheuvel
2018-04-24 13:17     ` Hans de Goede
2018-04-17  0:17   ` Luis R. Rodriguez
2018-04-17  8:58     ` Hans de Goede
2018-04-17  9:19     ` Hans de Goede
2018-04-23 21:11   ` Luis R. Rodriguez
2018-04-24 15:09     ` Hans de Goede
2018-04-24 16:07       ` Mimi Zohar
2018-04-24 18:33         ` Hans de Goede
2018-04-24 23:42         ` Luis R. Rodriguez
2018-04-25  5:00           ` Mimi Zohar
2018-04-25 17:55             ` Luis R. Rodriguez
2018-05-04  0:21               ` Luis R. Rodriguez
2018-05-04 15:26                 ` Martijn Coenen
2018-05-04 19:44               ` Martijn Coenen
2018-05-08 15:38                 ` Luis R. Rodriguez
2018-05-08 16:10                   ` Luis R. Rodriguez
2018-06-07 16:49                     ` Bjorn Andersson
2018-06-07 18:22                       ` Luis R. Rodriguez
2018-06-01 19:23                   ` Luis R. Rodriguez
2018-06-06 20:32                     ` Do Qualcomm drivers use DMA buffers for request_firmware_into_buf()? Luis R. Rodriguez
2018-06-07 16:18                       ` Bjorn Andersson
     [not found]                         ` <CAKv+Gu8+Fq7BD4XD-YCyXzZh0mg6Z2k-0styj0cw6_uZfaqy4Q@mail.gmail.com>
     [not found]                           ` <20180607163308.GA18834@kroah.com>
2018-06-08  6:41                             ` Vlastimil Babka
2018-06-07 16:33             ` [PATCH v3 2/5] efi: Add embedded peripheral firmware support Bjorn Andersson
2018-04-08 17:40 ` [PATCH v3 3/5] platform/x86: Rename silead_dmi to touchscreen_dmi Hans de Goede
2018-04-09  8:07   ` Andy Shevchenko
2018-04-20  0:20     ` Darren Hart
2018-04-08 17:40 ` [PATCH v3 4/5] platform/x86: touchscreen_dmi: Add EFI embedded firmware info support Hans de Goede
2018-04-08 17:40 ` [PATCH v3 5/5] platform/x86: touchscreen_dmi: Add info for the Chuwi Vi8 Plus tablet Hans de Goede
2018-04-09  8:10   ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKv+Gu-DZ86y5pmQ+-gQWLHAcs47H-cixCM1pBKmKZXA=+jRmg@mail.gmail.com' \
    --to=ard.biesheuvel@linaro.org \
    --cc=andy@infradead.org \
    --cc=arend.vanspriel@broadcom.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=dave@bewaar.me \
    --cc=dhowells@redhat.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=duwe@suse.de \
    --cc=dvhart@infradead.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdegoede@redhat.com \
    --cc=hpa@zytor.com \
    --cc=josh@joshtriplett.org \
    --cc=keescook@chromium.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=matt@codeblueprint.co.uk \
    --cc=mcgrof@kernel.org \
    --cc=mfuzzey@parkeon.com \
    --cc=mingo@redhat.com \
    --cc=nbroeking@me.com \
    --cc=pjones@redhat.com \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=will.deacon@arm.com \
    --cc=x86@kernel.org \
    --cc=zohar@linux.vnet.ibm.com \
    --subject='Re: [PATCH v3 1/5] efi: Export boot-services code and data as debugfs-blobs' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).