Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-28 Thread Fabio Estevam
On Tue, Jun 28, 2022 at 11:01 AM Heinrich Schuchardt
 wrote:

> The patch is merged June 19th:
>
> e05bd68ed5fc ("test: work around for EFI terminal size probing")
>
> Why do you ask?

Because I haven't seen an "applied" message in this thread.

I see it in mainline now, thanks.


Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-28 Thread Heinrich Schuchardt

On 6/28/22 15:52, Fabio Estevam wrote:

Hi Heinrich,

On Fri, Jun 17, 2022 at 1:59 AM Heinrich Schuchardt
 wrote:


This causes: https://source.denx.de/u-boot/u-boot/-/jobs/450891#L229
for all instances of sandbox running that test.



This is a only a bug to the test environment:

test/py/u_boot_console_base.py:110:
self.prompt_compiled = re.compile('^' + re.escape(self.prompt),
re.MULTILINE)

There must not be any byte before the prompt on the line for the test
environment to recognize it as a prompt.

I will look into the problem.


Have you had a chance to look into this?

2022.07 is really close and it would be nice to get a fix for this regression.

Thanks


The patch is merged June 19th:

e05bd68ed5fc ("test: work around for EFI terminal size probing")

Why do you ask?

Best regards

Heinrich


Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-28 Thread Fabio Estevam
On Tue, Jun 28, 2022 at 10:55 AM Tom Rini  wrote:

> I thought this was worked around for now?  The non-testing case should
> be fine again, and tests do need to do additional code to work, which is
> not ideal, but good enough for the moment.

I haven't realized that the patch has been applied.

We are good now, thanks!


Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-28 Thread Tom Rini
On Tue, Jun 28, 2022 at 10:52:32AM -0300, Fabio Estevam wrote:
> Hi Heinrich,
> 
> On Fri, Jun 17, 2022 at 1:59 AM Heinrich Schuchardt
>  wrote:
> 
> > > This causes: https://source.denx.de/u-boot/u-boot/-/jobs/450891#L229
> > > for all instances of sandbox running that test.
> > >
> >
> > This is a only a bug to the test environment:
> >
> > test/py/u_boot_console_base.py:110:
> > self.prompt_compiled = re.compile('^' + re.escape(self.prompt),
> > re.MULTILINE)
> >
> > There must not be any byte before the prompt on the line for the test
> > environment to recognize it as a prompt.
> >
> > I will look into the problem.
> 
> Have you had a chance to look into this?
> 
> 2022.07 is really close and it would be nice to get a fix for this regression.

I thought this was worked around for now?  The non-testing case should
be fine again, and tests do need to do additional code to work, which is
not ideal, but good enough for the moment.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-28 Thread Fabio Estevam
Hi Heinrich,

On Fri, Jun 17, 2022 at 1:59 AM Heinrich Schuchardt
 wrote:

> > This causes: https://source.denx.de/u-boot/u-boot/-/jobs/450891#L229
> > for all instances of sandbox running that test.
> >
>
> This is a only a bug to the test environment:
>
> test/py/u_boot_console_base.py:110:
> self.prompt_compiled = re.compile('^' + re.escape(self.prompt),
> re.MULTILINE)
>
> There must not be any byte before the prompt on the line for the test
> environment to recognize it as a prompt.
>
> I will look into the problem.

Have you had a chance to look into this?

2022.07 is really close and it would be nice to get a fix for this regression.

Thanks


Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-16 Thread Heinrich Schuchardt




On 6/17/22 00:50, Tom Rini wrote:

On Tue, Jun 14, 2022 at 08:02:03AM +0200, Heinrich Schuchardt wrote:


From: Heinrich Schuchardt 

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 
Fixes: a9bf024b2933 ("efi_loader: disk: a helper function to create efi_disk objects 
from udevice")
Signed-off-by: Heinrich Schuchardt 
Tested-by: Fabio Estevam 
Tested-by: Heiko Thiery 


This causes: https://source.denx.de/u-boot/u-boot/-/jobs/450891#L229
for all instances of sandbox running that test.



This is a only a bug to the test environment:

test/py/u_boot_console_base.py:110:
self.prompt_compiled = re.compile('^' + re.escape(self.prompt), 
re.MULTILINE)


There must not be any byte before the prompt on the line for the test 
environment to recognize it as a prompt.


I will look into the problem.

Best regards

Heinrich


Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-16 Thread Tom Rini
On Tue, Jun 14, 2022 at 08:02:03AM +0200, Heinrich Schuchardt wrote:

> From: Heinrich Schuchardt 
> 
> 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 
> Fixes: a9bf024b2933 ("efi_loader: disk: a helper function to create efi_disk 
> objects from udevice")
> Signed-off-by: Heinrich Schuchardt 
> Tested-by: Fabio Estevam 
> Tested-by: Heiko Thiery 

This causes: https://source.denx.de/u-boot/u-boot/-/jobs/450891#L229
for all instances of sandbox running that test.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-15 Thread AKASHI Takahiro
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 
> > > > > 
> > > > > 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 
> > > > > 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 
> > > > > ---
> > > > >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 
> > > > >#include 
> > > > >#include 
> > > > > @@ -12,6 +14,7 @@
> > > > >#include 
> > > > >#include 
> > > > >#include 
> > > > > +#include 
> > > > >#include 
> > > > >#include 
> > > > >#include 
> > > > > @@ -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 

Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-15 Thread Heinrich Schuchardt

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 

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 
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.





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?

Best regards

Heinrich



-Takahiro Akashi



Best regards

Heinrich



-Takahiro Akashi


Signed-off-by: Heinrich Schuchardt 
---
   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 
   #include 
   #include 
@@ -12,6 +14,7 @@
   #include 
   #include 
   #include 
+#include 
   #include 
   #include 
   #include 
@@ -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
 (_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 

Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-15 Thread Fabio Estevam
Hi Akashi-san,

On Wed, Jun 15, 2022 at 3:35 AM AKASHI Takahiro
 wrote:

> I think that your patch is a tentative workaround.

Could Heinrich's patch be applied to 2022.07 to fix the regression?

Any suggestions?

Thanks,

Fabio Estevam


Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-15 Thread AKASHI Takahiro
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 
> > > 
> > > 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 
> > > 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.

> > 
> > 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.

-Takahiro Akashi


> Best regards
> 
> Heinrich
> 
> > 
> > -Takahiro Akashi
> > 
> > > Signed-off-by: Heinrich Schuchardt 
> > > ---
> > >   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 
> > >   #include 
> > >   #include 
> > > @@ -12,6 +14,7 @@
> > >   #include 
> > >   #include 
> > >   #include 
> > > +#include 
> > >   #include 
> > >   #include 
> > >   #include 
> > > @@ -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();
> > > -

Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-15 Thread Heinrich Schuchardt




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 

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 
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.



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.

Best regards

Heinrich



-Takahiro Akashi


Signed-off-by: Heinrich Schuchardt 
---
  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 
  #include 
  #include 
@@ -12,6 +14,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -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
 (_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



Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-15 Thread AKASHI Takahiro
On Tue, Jun 14, 2022 at 08:02:03AM +0200, Heinrich Schuchardt wrote:
> From: Heinrich Schuchardt 
> 
> 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 
> Fixes: a9bf024b2933 ("efi_loader: disk: a helper function to create efi_disk 
> objects from udevice")

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.

-Takahiro Akashi

> Signed-off-by: Heinrich Schuchardt 
> ---
>  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 
>  #include 
>  #include 
> @@ -12,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -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
>(_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
> 


Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-14 Thread Heiko Thiery
Hi,

Am Di., 14. Juni 2022 um 08:02 Uhr schrieb Heinrich Schuchardt
:
>
> From: Heinrich Schuchardt 
>
> 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 
> Fixes: a9bf024b2933 ("efi_loader: disk: a helper function to create efi_disk 
> objects from udevice")
> Signed-off-by: Heinrich Schuchardt 

Tested-by: Heiko Thiery 

Tested on an imx8mn-ddr3l-evk board.

Thank you
-- 
Heiko

> ---
>  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 
>  #include 
>  #include 
> @@ -12,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -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
>  (_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
>


Re: [PATCH 1/1] efi_loader: initialize console size late

2022-06-14 Thread Fabio Estevam
Hi Heinrich,

On Tue, Jun 14, 2022 at 3:02 AM Heinrich Schuchardt
 wrote:
>
> From: Heinrich Schuchardt 
>
> 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 
> Fixes: a9bf024b2933 ("efi_loader: disk: a helper function to create efi_disk 
> objects from udevice")
> Signed-off-by: Heinrich Schuchardt 

This fixes the boot on a kontron-sl-mx8mm board accessed remotely.
The autoboot process is no longer interrupted.

Thanks a lot!

Tested-by: Fabio Estevam 


[PATCH 1/1] efi_loader: initialize console size late

2022-06-14 Thread Heinrich Schuchardt
From: Heinrich Schuchardt 

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 
Fixes: a9bf024b2933 ("efi_loader: disk: a helper function to create efi_disk 
objects from udevice")
Signed-off-by: Heinrich Schuchardt 
---
 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 
 #include 
 #include 
@@ -12,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -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
 (_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