On Wed, Jun 15, 2022 at 04:02:54PM +0200, Heinrich Schuchardt wrote:
> On 6/15/22 08:34, AKASHI Takahiro wrote:
> > On Wed, Jun 15, 2022 at 08:27:26AM +0200, Heinrich Schuchardt wrote:
> > >
> > >
> > > On 6/15/22 08:16, AKASHI Takahiro wrote:
> > > > On Tue, Jun 14, 2022 at 08:02:03AM +0200, Heinrich Schuchardt wrote:
> > > > > From: Heinrich Schuchardt <xypron.g...@gmx.de>
> > > > >
> > > > > If CONFIG_VIDEO_DM=n we query the display size from the serial
> > > > > console.
> > > > > Especially when using a remote console the response can be so late
> > > > > that
> > > > > it interferes with autoboot.
> > > > >
> > > > > Only query the console size when running an EFI binary.
> > > > >
> > > > > Add debug output showing the determined console size.
> > > > >
> > > > > Reported-by: Fabio Estevam <feste...@gmail.com>
> > > > > Fixes: a9bf024b2933 ("efi_loader: disk: a helper function to create
> > > > > efi_disk objects from udevice")
> > >
> > > Said patch made CONFIG_EFI_SETUP_EARLY=y the default.
> >
> > I don't think so.
> > Any config with this option enabled could cause the issue.
>
> We could additionally blame your patches that created this config option.
That is what I said.
> >
> > > >
> > > > If the key part of this patch is to move query_console_size() from
> > > > efi_init_early() to efi_init_obj_list(), the to-be-fixed patch is not
> > > > the one above but
> > > > commit a57ad20d07e8 ("efi_loader: split efi_init_obj_list()
> > > > into two stages")
> > > >
> > > > Moreover, this is just a warning but once Sughosh's patch,
> > > > https://lists.denx.de/pipermail/u-boot/2022-June/485977.html
> > > > is merged and FWU_MULTI_BANK_UPDATE is enabled, the said phenomenon can
> > > > be triggered again because efi_init_obj_list(), hence
> > > > query_console_size(),
> > > > will be called in board_init_r() before showing the U-Boot prompt.
> > >
> > > Then we should not merge it as is.
> >
> > I think that your patch is a tentative workaround.
>
> What makes you think so?
> Any better proposal that we can get in in time for v2022.07?
I said that it was a "warning" against a foreseeable issue.
-Takahiro Akashi
>
> Best regards
>
> Heinrich
>
> >
> > -Takahiro Akashi
> >
> >
> > > Best regards
> > >
> > > Heinrich
> > >
> > > >
> > > > -Takahiro Akashi
> > > >
> > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
> > > > > ---
> > > > > include/efi_loader.h | 2 ++
> > > > > lib/efi_loader/efi_console.c | 20 +++++++++++++-------
> > > > > lib/efi_loader/efi_setup.c | 4 ++++
> > > > > 3 files changed, 19 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > > > > index f6651e2c60..c1e00ebac3 100644
> > > > > --- a/include/efi_loader.h
> > > > > +++ b/include/efi_loader.h
> > > > > @@ -499,6 +499,8 @@ extern struct list_head
> > > > > efi_register_notify_events;
> > > > > int efi_init_early(void);
> > > > > /* Initialize efi execution environment */
> > > > > efi_status_t efi_init_obj_list(void);
> > > > > +/* Set up console modes */
> > > > > +void efi_setup_console_size(void);
> > > > > /* Install device tree */
> > > > > efi_status_t efi_install_fdt(void *fdt);
> > > > > /* Run loaded UEFI image */
> > > > > diff --git a/lib/efi_loader/efi_console.c
> > > > > b/lib/efi_loader/efi_console.c
> > > > > index 60a3fc85ac..3164fd484e 100644
> > > > > --- a/lib/efi_loader/efi_console.c
> > > > > +++ b/lib/efi_loader/efi_console.c
> > > > > @@ -5,6 +5,8 @@
> > > > > * Copyright (c) 2016 Alexander Graf
> > > > > */
> > > > > +#define LOG_CATEGORY LOGC_EFI
> > > > > +
> > > > > #include <common.h>
> > > > > #include <charset.h>
> > > > > #include <malloc.h>
> > > > > @@ -12,6 +14,7 @@
> > > > > #include <dm/device.h>
> > > > > #include <efi_loader.h>
> > > > > #include <env.h>
> > > > > +#include <log.h>
> > > > > #include <stdio_dev.h>
> > > > > #include <video_console.h>
> > > > > #include <linux/delay.h>
> > > > > @@ -58,7 +61,12 @@ const efi_guid_t efi_guid_text_output_protocol =
> > > > > #define cESC '\x1b'
> > > > > #define ESC "\x1b"
> > > > > -/* Default to mode 0 */
> > > > > +/*
> > > > > + * efi_con_mode - mode information of the Simple Text Output Protocol
> > > > > + *
> > > > > + * Use safe settings before efi_setup_console_size() is called.
> > > > > + * By default enable only the 80x25 mode which must always exist.
> > > > > + */
> > > > > static struct simple_text_output_mode efi_con_mode = {
> > > > > .max_mode = 1,
> > > > > .mode = 0,
> > > > > @@ -333,13 +341,13 @@ static int __maybe_unused query_vidconsole(int
> > > > > *rows, int *cols)
> > > > > }
> > > > > /**
> > > > > - * query_console_size() - update the mode table.
> > > > > + * efi_setup_console_size() - update the mode table.
> > > > > *
> > > > > * By default the only mode available is 80x25. If the console has
> > > > > at least 50
> > > > > * lines, enable mode 80x50. If we can query the console size and
> > > > > it is neither
> > > > > * 80x25 nor 80x50, set it as an additional mode.
> > > > > */
> > > > > -static void query_console_size(void)
> > > > > +void efi_setup_console_size(void)
> > > > > {
> > > > > int rows = 25, cols = 80;
> > > > > int ret = -ENODEV;
> > > > > @@ -351,6 +359,8 @@ static void query_console_size(void)
> > > > > if (ret)
> > > > > return;
> > > > > + log_debug("Console size %dx%d\n", rows, cols);
> > > > > +
> > > > > /* Test if we can have Mode 1 */
> > > > > if (cols >= 80 && rows >= 50) {
> > > > > efi_cout_modes[1].present = 1;
> > > > > @@ -371,7 +381,6 @@ static void query_console_size(void)
> > > > > }
> > > > > }
> > > > > -
> > > > > /**
> > > > > * efi_cout_query_mode() - get terminal size for a text mode
> > > > > *
> > > > > @@ -1262,9 +1271,6 @@ efi_status_t efi_console_register(void)
> > > > > efi_status_t r;
> > > > > struct efi_device_path *dp;
> > > > > - /* Set up mode information */
> > > > > - query_console_size();
> > > > > -
> > > > > /* Install protocols on root node */
> > > > > r = EFI_CALL(efi_install_multiple_protocol_interfaces
> > > > > (&efi_root,
> > > > > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > > > > index 250eeb2fcd..492ecf4cb1 100644
> > > > > --- a/lib/efi_loader/efi_setup.c
> > > > > +++ b/lib/efi_loader/efi_setup.c
> > > > > @@ -243,6 +243,10 @@ efi_status_t efi_init_obj_list(void)
> > > > > goto out;
> > > > > }
> > > > > + /* Set up console modes */
> > > > > + efi_setup_console_size();
> > > > > +
> > > > > + /* Install EFI_RNG_PROTOCOL */
> > > > > if (IS_ENABLED(CONFIG_EFI_RNG_PROTOCOL)) {
> > > > > ret = efi_rng_register();
> > > > > if (ret != EFI_SUCCESS)
> > > > > --
> > > > > 2.36.1
> > > > >
>
