From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6B782C47404 for ; Wed, 9 Oct 2019 14:02:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 441A4218DE for ; Wed, 9 Oct 2019 14:02:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="MV7CRBzo" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731376AbfJIOCI (ORCPT ); Wed, 9 Oct 2019 10:02:08 -0400 Received: from mail-wr1-f66.google.com ([209.85.221.66]:38872 "EHLO mail-wr1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731336AbfJIOCI (ORCPT ); Wed, 9 Oct 2019 10:02:08 -0400 Received: by mail-wr1-f66.google.com with SMTP id w12so3158781wro.5 for ; Wed, 09 Oct 2019 07:02:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ZGDUmf8vQElxXW6jlDEJJ26hKrxEvdzngHjwzVdydCI=; b=MV7CRBzotQ1VCRustpsQe1/OamCkR6/QHzmRMP9C1rqyIGQQBMUsKMaH6R4yksGlFC wTuer6mkSjer8uPDRDhDyq0gopJLuz8qQhBBtJTz+HEUFf8/G8g9qrYQbE45vyTg0Egv FY1DRqmw53YVZvszSbwqeZOBRMNdw637nZNQelAOB/Sc5iW8NHI7/s2YqKT3PJS5vf10 ow1YVlxUleDtCxbb4tR/16B6MlRqkLZ8GjW3G+9u+bBRm6GyWGRuoWWBW2id2cCD9Pxs eTt9WU9BrU+lcsHxEyOk0prD9pvx506E/DGYNaI2JRUpIHXSgYR0F2FPTainkwtfOisC vtkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ZGDUmf8vQElxXW6jlDEJJ26hKrxEvdzngHjwzVdydCI=; b=JVvvTy488kHpnXvP5oEupnerSuDy6wZqPz8cGwuJmBHtmIaGy3yPRWO7BnROEEVL86 9V06JoXDh+ZXAa2ZNXKu62NKx1MU27EDBeyFLGIhHatstRqJP7R/sV0kfWPJL3w89Z5v 2Z83GsMmziIY+yYu2H+wnmMFWrfZa/rlgXiuCfgqPL5wTSkeBd60tl2E7ZsNDAHAqjB+ zcX3dkj2ad4mbb4/LKz3D459Gfof9BcayjjGbliJngmKlb4Qa/s/2upxyb4pOb73fruA G89tmBuZ8Y3OE9dhG0p9qNCOLZflj8uvte4pSGrgDTSA8m8XqbdUCiENOS2/lxtnb+tB FvnA== X-Gm-Message-State: APjAAAVOGYMwVjUOocvWsSgdT/uJhHEv6z+TRtOIGUBMtwrQQOTGIH6t e7NSyqlwSsGTckJKKYiY0YNUf+j+gg7zbADxFeNmiQ== X-Google-Smtp-Source: APXvYqxXsXcxHW2JT3gu+25BnBOo2zJYevT1fUUUiXoZf/pfeoaZAGuBbvOTwiv9z1XVkFvK7IgN305kAzNL96RO0Dg= X-Received: by 2002:adf:f5c2:: with SMTP id k2mr3313430wrp.0.1570629724936; Wed, 09 Oct 2019 07:02:04 -0700 (PDT) MIME-Version: 1.0 References: <20191004145056.43267-1-hdegoede@redhat.com> <20191004145056.43267-2-hdegoede@redhat.com> <81c648d6-428e-d978-246b-9a87d43c5d21@redhat.com> In-Reply-To: <81c648d6-428e-d978-246b-9a87d43c5d21@redhat.com> From: Ard Biesheuvel Date: Wed, 9 Oct 2019 16:01:53 +0200 Message-ID: Subject: Re: [PATCH v7 1/8] efi: Export boot-services code and data as debugfs-blobs To: Hans de Goede Cc: Darren Hart , Andy Shevchenko , Luis Chamberlain , Greg Kroah-Hartman , "Rafael J . Wysocki" , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H . Peter Anvin" , Jonathan Corbet , Dmitry Torokhov , Peter Jones , Dave Olsthoorn , "the arch/x86 maintainers" , platform-driver-x86@vger.kernel.org, linux-efi , Linux Kernel Mailing List , Linux Doc Mailing List , linux-input@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 9 Oct 2019 at 15:59, Hans de Goede wrote: > > Hi, > > On 09-10-2019 15:35, Ard Biesheuvel wrote: > > On Wed, 9 Oct 2019 at 15:18, Hans de Goede wrote: > >> > >> Hi, > >> > >> On 09-10-2019 15:07, Ard Biesheuvel wrote: > >>> On Fri, 4 Oct 2019 at 16:51, 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. > >>>> > >>>> Reviewed-by: Greg Kroah-Hartman > >>>> Acked-by: Ard Biesheuvel > >>>> Signed-off-by: Hans de Goede > >>>> --- > >>>> 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. > OK, thanks for clearing that up.