On Wed, 20 May 2020 at 19:02, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote:
>
> On Wed, May 20, 2020 at 06:38:53PM +0200, Ard Biesheuvel wrote:
> > On Wed, 20 May 2020 at 18:38, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote:
> > >
> > > On Tue, May 19, 2020 at 11:07:55AM -0400, Arvind Sankar wrote:
> > > > On Tue, May 19, 2020 at 10:22:40AM +0200, Ard Biesheuvel wrote:
> > > > > On Mon, 18 May 2020 at 21:07, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote:
> > > > > > @@ -100,7 +123,9 @@ efi_status_t efi_parse_options(char const *cmdline)
> > > > > > if (!strcmp(param, "nokaslr")) {
> > > > > > efi_nokaslr = true;
> > > > > > } else if (!strcmp(param, "quiet")) {
> > > > > > - efi_quiet = true;
> > > > > > + efi_loglevel = CONSOLE_LOGLEVEL_QUIET;
> > > > > > + } else if (!strcmp(param, "debug")) {
> > > > > > + efi_loglevel = CONSOLE_LOGLEVEL_DEBUG;
> > > > >
> > > > > Should we wire this to 'efi=debug' instead?
> > > > >
> > > >
> > > > Sure.
> > >
> > > Do you prefer it wired up to both or just efi=debug?
> > >
> >
> > Let's stick with efi=debug, that is what all other EFI code uses.
>
> Ok -- you can replace the patch with the attached version. Only change
> is in efi_parse_options.
>
Done - thanks!
> Author: Arvind Sankar <nivedita@xxxxxxxxxxxx>
> Date: Thu May 14 19:33:39 2020 -0400
>
> efi/libstub: Implement printk-style logging
>
> Use the efi_printk function in efi_info/efi_err, and add efi_debug. This
> allows formatted output at different log levels.
>
> Add the notion of a loglevel instead of just quiet/not-quiet, and
> parse the efi=debug kernel parameter in addition to quiet.
>
> Signed-off-by: Arvind Sankar <nivedita@xxxxxxxxxxxx>
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index 5ecafc57619a..1f5a00b4f201 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -11,6 +11,7 @@
>
> #include <linux/efi.h>
> #include <linux/kernel.h>
> +#include <linux/printk.h> /* For CONSOLE_LOGLEVEL_* */
> #include <asm/efi.h>
>
> #include "efistub.h"
> @@ -18,7 +19,7 @@
> bool efi_nochunk;
> bool efi_nokaslr;
> bool efi_noinitrd;
> -bool efi_quiet;
> +int efi_loglevel = CONSOLE_LOGLEVEL_DEFAULT;
> bool efi_novamap;
>
> static bool efi_nosoftreserve;
> @@ -58,6 +59,28 @@ int efi_printk(const char *fmt, ...)
> char printf_buf[256];
> va_list args;
> int printed;
> + int loglevel = printk_get_level(fmt);
> +
> + switch (loglevel) {
> + case '0' ... '9':
> + loglevel -= '0';
> + break;
> + default:
> + /*
> + * Use loglevel -1 for cases where we just want to print to
> + * the screen.
> + */
> + loglevel = -1;
> + break;
> + }
> +
> + if (loglevel >= efi_loglevel)
> + return 0;
> +
> + if (loglevel >= 0)
> + efi_puts("EFI stub: ");
> +
> + fmt = printk_skip_level(fmt);
>
> va_start(args, fmt);
> printed = vsnprintf(printf_buf, sizeof(printf_buf), fmt, args);
> @@ -100,7 +123,7 @@ efi_status_t efi_parse_options(char const *cmdline)
> if (!strcmp(param, "nokaslr")) {
> efi_nokaslr = true;
> } else if (!strcmp(param, "quiet")) {
> - efi_quiet = true;
> + efi_loglevel = CONSOLE_LOGLEVEL_QUIET;
> } else if (!strcmp(param, "noinitrd")) {
> efi_noinitrd = true;
> } else if (!strcmp(param, "efi") && val) {
> @@ -114,6 +137,8 @@ efi_status_t efi_parse_options(char const *cmdline)
> efi_disable_pci_dma = true;
> if (parse_option_str(val, "no_disable_early_pci_dma"))
> efi_disable_pci_dma = false;
> + if (parse_option_str(val, "debug"))
> + efi_loglevel = CONSOLE_LOGLEVEL_DEBUG;
> } else if (!strcmp(param, "video") &&
> val && strstarts(val, "efifb:")) {
> efi_parse_option_graphics(val + strlen("efifb:"));
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index caa7dcc71c69..3a323a009836 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -6,6 +6,7 @@
> #include <linux/compiler.h>
> #include <linux/efi.h>
> #include <linux/kernel.h>
> +#include <linux/kern_levels.h>
> #include <linux/types.h>
> #include <asm/efi.h>
>
> @@ -34,7 +35,7 @@
> extern bool efi_nochunk;
> extern bool efi_nokaslr;
> extern bool efi_noinitrd;
> -extern bool efi_quiet;
> +extern int efi_loglevel;
> extern bool efi_novamap;
>
> extern const efi_system_table_t *efi_system_table;
> @@ -49,11 +50,12 @@ extern const efi_system_table_t *efi_system_table;
>
> #endif
>
> -#define efi_info(msg) do { \
> - if (!efi_quiet) efi_puts("EFI stub: "msg); \
> -} while (0)
> -
> -#define efi_err(msg) efi_puts("EFI stub: ERROR: "msg)
> +#define efi_info(fmt, ...) \
> + efi_printk(KERN_INFO fmt, ##__VA_ARGS__)
> +#define efi_err(fmt, ...) \
> + efi_printk(KERN_ERR "ERROR: " fmt, ##__VA_ARGS__)
> +#define efi_debug(fmt, ...) \
> + efi_printk(KERN_DEBUG "DEBUG: " fmt, ##__VA_ARGS__)
>
> /* Helper macros for the usual case of using simple C variables: */
> #ifndef fdt_setprop_inplace_var
