public inbox for linux-efi@vger.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: matt@codeblueprint.co.uk, rfranz@cavium.com,
	linux-efi@vger.kernel.org, leif.lindholm@linaro.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] efi/libstub: arm/arm64: make debug prints dependent on efi=debug
Date: Tue, 21 Mar 2017 11:58:05 +0000	[thread overview]
Message-ID: <20170321115805.GC22188@leverpostej> (raw)
In-Reply-To: <1490043425-20118-1-git-send-email-ard.biesheuvel@linaro.org>

On Mon, Mar 20, 2017 at 08:57:05PM +0000, Ard Biesheuvel wrote:
> The EFI stub currently prints a number of diagnostic messages that do
> not carry a lot of information. Since these prints are not controlled
> by 'loglevel' or other command line parameters, and since they appear on
> the EFI framebuffer as well (if enabled), it would be nice if we could
> turn them off.
> 
> So let's only print them if 'efi=debug' is passed on the command line,
> or when CONFIG_DEBUG_EFI is set in the kernel build configuration.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/firmware/efi/libstub/arm32-stub.c      |  2 ++
>  drivers/firmware/efi/libstub/efi-stub-helper.c | 23 ++++++++++++++---------
>  drivers/firmware/efi/libstub/efistub.h         |  9 +++++++++
>  drivers/firmware/efi/libstub/secureboot.c      |  2 ++
>  include/linux/efi.h                            |  3 ---
>  5 files changed, 27 insertions(+), 12 deletions(-)

> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index 71c4d0e3c4ed..5ed16e6e166f 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -24,6 +24,15 @@
>  #define EFI_ALLOC_ALIGN		EFI_PAGE_SIZE
>  #endif
>  
> +extern int __pure debug_enabled(void);
> +
> +#define pr_efi(sys_table, msg)		do {				\
> +	if (debug_enabled()) efi_printk(sys_table, "EFI stub: "msg);	\
> +} while (0)

I was going to say that it's somewhat unfortunate to lose some of these
messages by default, but I guess this only adds one additional
round-trip when debugging a remote issue, and gives us the freedom to
add more comprehensive debug messages in future.

Is the idea is to make the EFI stub completely silent in the successful
case, or is the aim just to minimise the number of messages?

If it's only the latter, we could log a message regarding the existence
of efi=debug in the default case, which might help with bug reports.

No strong feelings from me.

Thanks,
Mark.

  reply	other threads:[~2017-03-21 11:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-20 20:57 [PATCH] efi/libstub: arm/arm64: make debug prints dependent on efi=debug Ard Biesheuvel
2017-03-21 11:58 ` Mark Rutland [this message]
2017-03-21 12:12   ` Ard Biesheuvel

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=20170321115805.GC22188@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=leif.lindholm@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=matt@codeblueprint.co.uk \
    --cc=rfranz@cavium.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox