All of lore.kernel.org
 help / color / mirror / Atom feed
* Halting auto-boot process in the U-Boot shell https://lore.kernel.org/u-boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/
@ 2025-02-26  6:48 Kummari, Prasad
  2025-02-26  8:23 ` Heinrich Schuchardt
  0 siblings, 1 reply; 8+ messages in thread
From: Kummari, Prasad @ 2025-02-26  6:48 UTC (permalink / raw)
  To: heinrich.schuchardt@canonical.com, xypron.glpk@gmx.de,
	u-boot@lists.denx.de, Simek, Michal, ilias.apalodimas@linaro.org
  Cc: Abbarapu, Venkatesh, Begari, Padmarao

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Heinrich,

Regarding below commit.

commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85
Author:     Heinrich Schuchardt <xypron.glpk@gmx.de>
AuthorDate: Tue Jun 14 08:02:03 2022 +0200
Commit:     Heinrich Schuchardt <xypron.glpk@gmx.de>
CommitDate: Sun Jun 19 15:53:09 2022 +0200

    efi_loader: initialize console size late

    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.
https://lore.kernel.org/u-boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/

Commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85 modifies the system to query the console size only when running an EFI binary. However, when CONFIG_EFI_CAPSULE_ON_DISK is enabled, the console is still being invoked, which shouldn't happen. This results in issues on our terminal, which cannot handle the resulting characters, leading to incorrect display and halting the auto-boot process in the U-Boot shell.

Log Output:

Warning: ethernet@ff0c0000 (eth0) using random MAC address - ee:df:0a:38:a8:c4
eth0: ethernet@ff0c0000, eth1: mrmac@a40f0000, eth2: mrmac@a40f1000, eth3: mrmac@a40f2000, eth4: mrmac@a40f3000
Cannot persist EFI variables without system partition
Missing TPMv2 device for EFI_TCG_PROTOCOL
Missing RNG device for EFI_RNG_PROTOCOL
Hit any key to stop autoboot:  0
Versal> [42;173R
Unknown command '[42' - try 'help'
Unknown command '173R' - try 'help'
Versal>


Debug logs:

Before entering the U-Boot shell, set a breakpoint at getc, which is invoked by efi_setup_console_size() and display problems and auto-boot interruptions.

 xsdb% bt
    0  0x7ff0d648 pl01x_serial_getc()+2011873280: drivers/serial/serial_pl01x.c, line 339
    1  0x7ff0d134 __serial_getc()+2011873316: drivers/serial/serial-uclass.c, line 310
    2  0x7ff47dcc efi_setup_console_size()+2011873540: lib/efi_loader/efi_console.c, line 291
    3  0x7ff4e40c efi_init_obj_list()+2011873316: lib/efi_loader/efi_setup.c, line 225
    4  0x7fed699c main_loop()+2011873324: common/main.c, line 61
    5  0x7fed69d8 add_mem_region()+2011873280: common/main.c, line 84
xsdb%

code snippet: Disabling the efi_setup_console_size() function prevents the issue from occurring.

diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index aa59bc7779d..ad68ac19526 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -220,9 +220,10 @@ efi_status_t efi_init_obj_list(void)
        /* Initialize once only */
        if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
                return efi_obj_list_initialized;
-
+#if 0
        /* Set up console modes */
        efi_setup_console_size();
+#endif

Regards,
Prasad.








^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: Halting auto-boot process in the U-Boot shell https://lore.kernel.org/u-boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/
  2025-02-26  6:48 Halting auto-boot process in the U-Boot shell https://lore.kernel.org/u-boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/ Kummari, Prasad
@ 2025-02-26  8:23 ` Heinrich Schuchardt
  2025-02-26  8:28   ` Michal Simek
  0 siblings, 1 reply; 8+ messages in thread
From: Heinrich Schuchardt @ 2025-02-26  8:23 UTC (permalink / raw)
  To: Kummari, Prasad
  Cc: Abbarapu, Venkatesh, Begari, Padmarao, xypron.glpk@gmx.de,
	u-boot@lists.denx.de, Simek, Michal, ilias.apalodimas@linaro.org

On 2/26/25 07:48, Kummari, Prasad wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> 
> Hi Heinrich,
> 
> Regarding below commit.
> 
> commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85
> Author:     Heinrich Schuchardt <xypron.glpk@gmx.de>
> AuthorDate: Tue Jun 14 08:02:03 2022 +0200
> Commit:     Heinrich Schuchardt <xypron.glpk@gmx.de>
> CommitDate: Sun Jun 19 15:53:09 2022 +0200
> 
>      efi_loader: initialize console size late
> 
>      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.*
> 
> https://lore.kernel.org/u-boot/20220614060203.33600-1- 
> heinrich.schuchardt@canonical.com/ <https://lore.kernel.org/u- 
> boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/>
> 
> Commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85 modifies the system to 
> query the console size only when running an EFI binary. However, when 
> CONFIG_EFI_CAPSULE_ON_DISK is enabled, the console is still being 
> invoked, which shouldn't happen. This results in issues on our terminal, 
> which cannot handle the resulting characters, leading to incorrect 
> display and halting the auto-boot process in the U-Boot shell.
> 
> *Log Output:*
> 
> Warning: ethernet@ff0c0000 (eth0) using random MAC address - 
> ee:df:0a:38:a8:c4
> 
> eth0: ethernet@ff0c0000, eth1: mrmac@a40f0000, eth2: mrmac@a40f1000, 
> eth3: mrmac@a40f2000, eth4: mrmac@a40f3000
> 
> Cannot persist EFI variables without system partition
> 
> Missing TPMv2 device for EFI_TCG_PROTOCOL
> 
> Missing RNG device for EFI_RNG_PROTOCOL
> 
> Hit any key to stop autoboot:  0
> 
> *Versal> [42;173R*
> 
> *Unknown command '[42' - try 'help'*

Hello Prasad,

The terminal emulation seems to be sending a valid reply indicating 42 
rows and 173 columns.

It could be that in your setup you are hitting the time out in 
term_get_char().

Could you, please, check what happens if you apply

--- a/lib/efi_loader/efi_console.c
+++ b/lib/efi_loader/efi_console.c
@@ -92,8 +92,7 @@ static int term_get_char(s32 *c)
         timeout = timer_get_us() + 100000;

         while (!tstc())
-               if (timer_get_us() > timeout)
-                       return 1;
+               ;

         *c = getchar();
         return 0;

If the failure disappears, next you would have to analyze why the 
timeout is occurring (e.g. timer_get_us() running too fast).

If you still get failures, please, add debug output term_read_reply() to 
understand what is happening.

Best regards

Heinrich

> 
> *Unknown command '173R' - try 'help'*
> 
> *Versal>*
> 
> Debug logs:
> 
> Before entering the U-Boot shell, set a breakpoint at getc, which is 
> invoked by efi_setup_console_size() and display problems and auto-boot 
> interruptions.
> 
>   xsdb% bt
>      0  0x7ff0d648 pl01x_serial_getc()+2011873280: drivers/serial/ 
> serial_pl01x.c, line 339
>      1  0x7ff0d134 __serial_getc()+2011873316: drivers/serial/serial- 
> uclass.c, line 310
> *    2  0x7ff47dcc efi_setup_console_size()+2011873540: lib/efi_loader/ 
> efi_console.c, line 291
> *    3  0x7ff4e40c efi_init_obj_list()+2011873316: lib/efi_loader/ 
> efi_setup.c, line 225
>      4  0x7fed699c main_loop()+2011873324: common/main.c, line 61
>      5  0x7fed69d8 add_mem_region()+2011873280: common/main.c, line 84
> xsdb%
> 
> code snippet: Disabling the efi_setup_console_size() function prevents 
> the issue from occurring.
> 
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index aa59bc7779d..ad68ac19526 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -220,9 +220,10 @@ efi_status_t efi_init_obj_list(void)
>          /* Initialize once only */
>          if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
>                  return efi_obj_list_initialized;
> -
> +#if 0
>          /* Set up console modes */
>          efi_setup_console_size();
> +#endif
> 
> Regards,
> 
> Prasad.
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Halting auto-boot process in the U-Boot shell https://lore.kernel.org/u-boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/
  2025-02-26  8:23 ` Heinrich Schuchardt
@ 2025-02-26  8:28   ` Michal Simek
  2025-02-26  8:59     ` Heinrich Schuchardt
  0 siblings, 1 reply; 8+ messages in thread
From: Michal Simek @ 2025-02-26  8:28 UTC (permalink / raw)
  To: Heinrich Schuchardt, Kummari, Prasad
  Cc: Abbarapu, Venkatesh, Begari, Padmarao, xypron.glpk@gmx.de,
	u-boot@lists.denx.de, ilias.apalodimas@linaro.org



On 2/26/25 09:23, Heinrich Schuchardt wrote:
> On 2/26/25 07:48, Kummari, Prasad wrote:
>> [AMD Official Use Only - AMD Internal Distribution Only]
>>
>>
>> Hi Heinrich,
>>
>> Regarding below commit.
>>
>> commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85
>> Author:     Heinrich Schuchardt <xypron.glpk@gmx.de>
>> AuthorDate: Tue Jun 14 08:02:03 2022 +0200
>> Commit:     Heinrich Schuchardt <xypron.glpk@gmx.de>
>> CommitDate: Sun Jun 19 15:53:09 2022 +0200
>>
>>      efi_loader: initialize console size late
>>
>>      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.*
>>
>> https://lore.kernel.org/u-boot/20220614060203.33600-1- 
>> heinrich.schuchardt@canonical.com/ <https://lore.kernel.org/u- 
>> boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/>
>>
>> Commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85 modifies the system to query 
>> the console size only when running an EFI binary. However, when 
>> CONFIG_EFI_CAPSULE_ON_DISK is enabled, the console is still being invoked, 
>> which shouldn't happen. This results in issues on our terminal, which cannot 
>> handle the resulting characters, leading to incorrect display and halting the 
>> auto-boot process in the U-Boot shell.
>>
>> *Log Output:*
>>
>> Warning: ethernet@ff0c0000 (eth0) using random MAC address - ee:df:0a:38:a8:c4
>>
>> eth0: ethernet@ff0c0000, eth1: mrmac@a40f0000, eth2: mrmac@a40f1000, eth3: 
>> mrmac@a40f2000, eth4: mrmac@a40f3000
>>
>> Cannot persist EFI variables without system partition
>>
>> Missing TPMv2 device for EFI_TCG_PROTOCOL
>>
>> Missing RNG device for EFI_RNG_PROTOCOL
>>
>> Hit any key to stop autoboot:  0
>>
>> *Versal> [42;173R*
>>
>> *Unknown command '[42' - try 'help'*
> 
> Hello Prasad,
> 
> The terminal emulation seems to be sending a valid reply indicating 42 rows and 
> 173 columns.
> 
> It could be that in your setup you are hitting the time out in term_get_char().
> 
> Could you, please, check what happens if you apply
> 
> --- a/lib/efi_loader/efi_console.c
> +++ b/lib/efi_loader/efi_console.c
> @@ -92,8 +92,7 @@ static int term_get_char(s32 *c)
>          timeout = timer_get_us() + 100000;
> 
>          while (!tstc())
> -               if (timer_get_us() > timeout)
> -                       return 1;
> +               ;
> 
>          *c = getchar();
>          return 0;
> 
> If the failure disappears, next you would have to analyze why the timeout is 
> occurring (e.g. timer_get_us() running too fast).
> 
> If you still get failures, please, add debug output term_read_reply() to 
> understand what is happening.

But why is this send at this stage? Why capsule update needs to be aware about 
size of terminal? I understand that you need to know it for efi application but 
what's the reason to know it capsule update?

Thanks,
Michal



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Halting auto-boot process in the U-Boot shell https://lore.kernel.org/u-boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/
  2025-02-26  8:28   ` Michal Simek
@ 2025-02-26  8:59     ` Heinrich Schuchardt
  2025-02-26  9:04       ` Michal Simek
  0 siblings, 1 reply; 8+ messages in thread
From: Heinrich Schuchardt @ 2025-02-26  8:59 UTC (permalink / raw)
  To: Michal Simek
  Cc: Abbarapu, Venkatesh, Begari, Padmarao, xypron.glpk@gmx.de,
	u-boot@lists.denx.de, ilias.apalodimas@linaro.org,
	Kummari, Prasad

On 2/26/25 09:28, Michal Simek wrote:
> 
> 
> On 2/26/25 09:23, Heinrich Schuchardt wrote:
>> On 2/26/25 07:48, Kummari, Prasad wrote:
>>> [AMD Official Use Only - AMD Internal Distribution Only]
>>>
>>>
>>> Hi Heinrich,
>>>
>>> Regarding below commit.
>>>
>>> commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85
>>> Author:     Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> AuthorDate: Tue Jun 14 08:02:03 2022 +0200
>>> Commit:     Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> CommitDate: Sun Jun 19 15:53:09 2022 +0200
>>>
>>>      efi_loader: initialize console size late
>>>
>>>      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.*
>>>
>>> https://lore.kernel.org/u-boot/20220614060203.33600-1- 
>>> heinrich.schuchardt@canonical.com/ <https://lore.kernel.org/u- 
>>> boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/>
>>>
>>> Commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85 modifies the system 
>>> to query the console size only when running an EFI binary. However, 
>>> when CONFIG_EFI_CAPSULE_ON_DISK is enabled, the console is still 
>>> being invoked, which shouldn't happen. This results in issues on our 
>>> terminal, which cannot handle the resulting characters, leading to 
>>> incorrect display and halting the auto-boot process in the U-Boot shell.
>>>
>>> *Log Output:*
>>>
>>> Warning: ethernet@ff0c0000 (eth0) using random MAC address - 
>>> ee:df:0a:38:a8:c4
>>>
>>> eth0: ethernet@ff0c0000, eth1: mrmac@a40f0000, eth2: mrmac@a40f1000, 
>>> eth3: mrmac@a40f2000, eth4: mrmac@a40f3000
>>>
>>> Cannot persist EFI variables without system partition
>>>
>>> Missing TPMv2 device for EFI_TCG_PROTOCOL
>>>
>>> Missing RNG device for EFI_RNG_PROTOCOL
>>>
>>> Hit any key to stop autoboot:  0
>>>
>>> *Versal> [42;173R*
>>>
>>> *Unknown command '[42' - try 'help'*
>>
>> Hello Prasad,
>>
>> The terminal emulation seems to be sending a valid reply indicating 42 
>> rows and 173 columns.
>>
>> It could be that in your setup you are hitting the time out in 
>> term_get_char().
>>
>> Could you, please, check what happens if you apply
>>
>> --- a/lib/efi_loader/efi_console.c
>> +++ b/lib/efi_loader/efi_console.c
>> @@ -92,8 +92,7 @@ static int term_get_char(s32 *c)
>>          timeout = timer_get_us() + 100000;
>>
>>          while (!tstc())
>> -               if (timer_get_us() > timeout)
>> -                       return 1;
>> +               ;
>>
>>          *c = getchar();
>>          return 0;
>>
>> If the failure disappears, next you would have to analyze why the 
>> timeout is occurring (e.g. timer_get_us() running too fast).
>>
>> If you still get failures, please, add debug output term_read_reply() 
>> to understand what is happening.
> 
> But why is this send at this stage? Why capsule update needs to be aware 
> about size of terminal? I understand that you need to know it for efi 
> application but what's the reason to know it capsule update?
> 
> Thanks,
> Michal
> 
> 

Hello Michal,

There are two separate questions to answer:

* Why does determining the console not work?

The problem can only occur EFI enabled systems. If determining the 
console size fails here, we can expect it to fail when running EFI 
applications, too.

* Why is the EFI console initialized here?

We have currently only one entry-point for initializing the whole of the 
EFI sub-system. We are using the console before running any EFI 
application, e.g. in the eficonfig command.

This could be redesigned with some effort.

Best regards

Heinrich



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Halting auto-boot process in the U-Boot shell https://lore.kernel.org/u-boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/
  2025-02-26  8:59     ` Heinrich Schuchardt
@ 2025-02-26  9:04       ` Michal Simek
  2025-02-27 16:25         ` Simon Glass
  0 siblings, 1 reply; 8+ messages in thread
From: Michal Simek @ 2025-02-26  9:04 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Abbarapu, Venkatesh, Begari, Padmarao, xypron.glpk@gmx.de,
	u-boot@lists.denx.de, ilias.apalodimas@linaro.org,
	Kummari, Prasad



On 2/26/25 09:59, Heinrich Schuchardt wrote:
> On 2/26/25 09:28, Michal Simek wrote:
>>
>>
>> On 2/26/25 09:23, Heinrich Schuchardt wrote:
>>> On 2/26/25 07:48, Kummari, Prasad wrote:
>>>> [AMD Official Use Only - AMD Internal Distribution Only]
>>>>
>>>>
>>>> Hi Heinrich,
>>>>
>>>> Regarding below commit.
>>>>
>>>> commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85
>>>> Author:     Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>> AuthorDate: Tue Jun 14 08:02:03 2022 +0200
>>>> Commit:     Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>> CommitDate: Sun Jun 19 15:53:09 2022 +0200
>>>>
>>>>      efi_loader: initialize console size late
>>>>
>>>>      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.*
>>>>
>>>> https://lore.kernel.org/u-boot/20220614060203.33600-1- 
>>>> heinrich.schuchardt@canonical.com/ <https://lore.kernel.org/u- 
>>>> boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/>
>>>>
>>>> Commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85 modifies the system to query 
>>>> the console size only when running an EFI binary. However, when 
>>>> CONFIG_EFI_CAPSULE_ON_DISK is enabled, the console is still being invoked, 
>>>> which shouldn't happen. This results in issues on our terminal, which cannot 
>>>> handle the resulting characters, leading to incorrect display and halting 
>>>> the auto-boot process in the U-Boot shell.
>>>>
>>>> *Log Output:*
>>>>
>>>> Warning: ethernet@ff0c0000 (eth0) using random MAC address - ee:df:0a:38:a8:c4
>>>>
>>>> eth0: ethernet@ff0c0000, eth1: mrmac@a40f0000, eth2: mrmac@a40f1000, eth3: 
>>>> mrmac@a40f2000, eth4: mrmac@a40f3000
>>>>
>>>> Cannot persist EFI variables without system partition
>>>>
>>>> Missing TPMv2 device for EFI_TCG_PROTOCOL
>>>>
>>>> Missing RNG device for EFI_RNG_PROTOCOL
>>>>
>>>> Hit any key to stop autoboot:  0
>>>>
>>>> *Versal> [42;173R*
>>>>
>>>> *Unknown command '[42' - try 'help'*
>>>
>>> Hello Prasad,
>>>
>>> The terminal emulation seems to be sending a valid reply indicating 42 rows 
>>> and 173 columns.
>>>
>>> It could be that in your setup you are hitting the time out in term_get_char().
>>>
>>> Could you, please, check what happens if you apply
>>>
>>> --- a/lib/efi_loader/efi_console.c
>>> +++ b/lib/efi_loader/efi_console.c
>>> @@ -92,8 +92,7 @@ static int term_get_char(s32 *c)
>>>          timeout = timer_get_us() + 100000;
>>>
>>>          while (!tstc())
>>> -               if (timer_get_us() > timeout)
>>> -                       return 1;
>>> +               ;
>>>
>>>          *c = getchar();
>>>          return 0;
>>>
>>> If the failure disappears, next you would have to analyze why the timeout is 
>>> occurring (e.g. timer_get_us() running too fast).
>>>
>>> If you still get failures, please, add debug output term_read_reply() to 
>>> understand what is happening.
>>
>> But why is this send at this stage? Why capsule update needs to be aware about 
>> size of terminal? I understand that you need to know it for efi application 
>> but what's the reason to know it capsule update?
>>
>> Thanks,
>> Michal
>>
>>
> 
> Hello Michal,
> 
> There are two separate questions to answer:
> 
> * Why does determining the console not work?
> 
> The problem can only occur EFI enabled systems. If determining the console size 
> fails here, we can expect it to fail when running EFI applications, too.

yes but you don't know if users are going to boot over EFI or via legacy way.
You can still use capsule update part only and using bootm/booti for booting.

> 
> * Why is the EFI console initialized here?
> 
> We have currently only one entry-point for initializing the whole of the EFI 
> sub-system. We are using the console before running any EFI application, e.g. in 
> the eficonfig command.
> 
> This could be redesigned with some effort.

And I think it should. Because there is no reason to do initialization of 
anything what it is not going to be used. At the end of day it is just delaying 
boot.

Thanks,
Michal


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Halting auto-boot process in the U-Boot shell https://lore.kernel.org/u-boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/
  2025-02-26  9:04       ` Michal Simek
@ 2025-02-27 16:25         ` Simon Glass
  2025-02-28 14:42           ` Michal Simek
  0 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2025-02-27 16:25 UTC (permalink / raw)
  To: Michal Simek
  Cc: Heinrich Schuchardt, Abbarapu, Venkatesh, Begari, Padmarao,
	xypron.glpk@gmx.de, u-boot@lists.denx.de,
	ilias.apalodimas@linaro.org, Kummari, Prasad

Hi Michal,

On Wed, 26 Feb 2025 at 02:04, Michal Simek <michal.simek@amd.com> wrote:
>
>
>
> On 2/26/25 09:59, Heinrich Schuchardt wrote:
> > On 2/26/25 09:28, Michal Simek wrote:
> >>
> >>
> >> On 2/26/25 09:23, Heinrich Schuchardt wrote:
> >>> On 2/26/25 07:48, Kummari, Prasad wrote:
> >>>> [AMD Official Use Only - AMD Internal Distribution Only]
> >>>>
> >>>>
> >>>> Hi Heinrich,
> >>>>
> >>>> Regarding below commit.
> >>>>
> >>>> commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85
> >>>> Author:     Heinrich Schuchardt <xypron.glpk@gmx.de>
> >>>> AuthorDate: Tue Jun 14 08:02:03 2022 +0200
> >>>> Commit:     Heinrich Schuchardt <xypron.glpk@gmx.de>
> >>>> CommitDate: Sun Jun 19 15:53:09 2022 +0200
> >>>>
> >>>>      efi_loader: initialize console size late
> >>>>
> >>>>      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.*
> >>>>
> >>>> https://lore.kernel.org/u-boot/20220614060203.33600-1-
> >>>> heinrich.schuchardt@canonical.com/ <https://lore.kernel.org/u-
> >>>> boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/>
> >>>>
> >>>> Commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85 modifies the system to query
> >>>> the console size only when running an EFI binary. However, when
> >>>> CONFIG_EFI_CAPSULE_ON_DISK is enabled, the console is still being invoked,
> >>>> which shouldn't happen. This results in issues on our terminal, which cannot
> >>>> handle the resulting characters, leading to incorrect display and halting
> >>>> the auto-boot process in the U-Boot shell.
> >>>>
> >>>> *Log Output:*
> >>>>
> >>>> Warning: ethernet@ff0c0000 (eth0) using random MAC address - ee:df:0a:38:a8:c4
> >>>>
> >>>> eth0: ethernet@ff0c0000, eth1: mrmac@a40f0000, eth2: mrmac@a40f1000, eth3:
> >>>> mrmac@a40f2000, eth4: mrmac@a40f3000
> >>>>
> >>>> Cannot persist EFI variables without system partition
> >>>>
> >>>> Missing TPMv2 device for EFI_TCG_PROTOCOL
> >>>>
> >>>> Missing RNG device for EFI_RNG_PROTOCOL
> >>>>
> >>>> Hit any key to stop autoboot:  0
> >>>>
> >>>> *Versal> [42;173R*
> >>>>
> >>>> *Unknown command '[42' - try 'help'*
> >>>
> >>> Hello Prasad,
> >>>
> >>> The terminal emulation seems to be sending a valid reply indicating 42 rows
> >>> and 173 columns.
> >>>
> >>> It could be that in your setup you are hitting the time out in term_get_char().
> >>>
> >>> Could you, please, check what happens if you apply
> >>>
> >>> --- a/lib/efi_loader/efi_console.c
> >>> +++ b/lib/efi_loader/efi_console.c
> >>> @@ -92,8 +92,7 @@ static int term_get_char(s32 *c)
> >>>          timeout = timer_get_us() + 100000;
> >>>
> >>>          while (!tstc())
> >>> -               if (timer_get_us() > timeout)
> >>> -                       return 1;
> >>> +               ;
> >>>
> >>>          *c = getchar();
> >>>          return 0;
> >>>
> >>> If the failure disappears, next you would have to analyze why the timeout is
> >>> occurring (e.g. timer_get_us() running too fast).
> >>>
> >>> If you still get failures, please, add debug output term_read_reply() to
> >>> understand what is happening.
> >>
> >> But why is this send at this stage? Why capsule update needs to be aware about
> >> size of terminal? I understand that you need to know it for efi application
> >> but what's the reason to know it capsule update?
> >>
> >> Thanks,
> >> Michal
> >>
> >>
> >
> > Hello Michal,
> >
> > There are two separate questions to answer:
> >
> > * Why does determining the console not work?
> >
> > The problem can only occur EFI enabled systems. If determining the console size
> > fails here, we can expect it to fail when running EFI applications, too.
>
> yes but you don't know if users are going to boot over EFI or via legacy way.
> You can still use capsule update part only and using bootm/booti for booting.
>
> >
> > * Why is the EFI console initialized here?
> >
> > We have currently only one entry-point for initializing the whole of the EFI
> > sub-system. We are using the console before running any EFI application, e.g. in
> > the eficonfig command.
> >
> > This could be redesigned with some effort.
>
> And I think it should. Because there is no reason to do initialization of
> anything what it is not going to be used. At the end of day it is just delaying
> boot.

Agreed. My attempt to resolve this for a similar situation just
resulted in my case being a bad example[1]. But this is just bad
design.

Regards,
SImon

[1] https://lore.kernel.org/u-boot/20240926215950.1265143-9-sjg@chromium.org/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Halting auto-boot process in the U-Boot shell https://lore.kernel.org/u-boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/
  2025-02-27 16:25         ` Simon Glass
@ 2025-02-28 14:42           ` Michal Simek
  2025-03-06 13:56             ` Simon Glass
  0 siblings, 1 reply; 8+ messages in thread
From: Michal Simek @ 2025-02-28 14:42 UTC (permalink / raw)
  To: Simon Glass, Tom Rini
  Cc: Heinrich Schuchardt, Abbarapu, Venkatesh, Begari, Padmarao,
	xypron.glpk@gmx.de, u-boot@lists.denx.de,
	ilias.apalodimas@linaro.org, Kummari, Prasad

Hi Simon,

On 2/27/25 17:25, Simon Glass wrote:
> Hi Michal,
> 
> On Wed, 26 Feb 2025 at 02:04, Michal Simek <michal.simek@amd.com> wrote:
>>
>>
>>
>> On 2/26/25 09:59, Heinrich Schuchardt wrote:
>>> On 2/26/25 09:28, Michal Simek wrote:
>>>>
>>>>
>>>> On 2/26/25 09:23, Heinrich Schuchardt wrote:
>>>>> On 2/26/25 07:48, Kummari, Prasad wrote:
>>>>>> [AMD Official Use Only - AMD Internal Distribution Only]
>>>>>>
>>>>>>
>>>>>> Hi Heinrich,
>>>>>>
>>>>>> Regarding below commit.
>>>>>>
>>>>>> commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85
>>>>>> Author:     Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>>>> AuthorDate: Tue Jun 14 08:02:03 2022 +0200
>>>>>> Commit:     Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>>>> CommitDate: Sun Jun 19 15:53:09 2022 +0200
>>>>>>
>>>>>>       efi_loader: initialize console size late
>>>>>>
>>>>>>       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.*
>>>>>>
>>>>>> https://lore.kernel.org/u-boot/20220614060203.33600-1-
>>>>>> heinrich.schuchardt@canonical.com/ <https://lore.kernel.org/u-
>>>>>> boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/>
>>>>>>
>>>>>> Commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85 modifies the system to query
>>>>>> the console size only when running an EFI binary. However, when
>>>>>> CONFIG_EFI_CAPSULE_ON_DISK is enabled, the console is still being invoked,
>>>>>> which shouldn't happen. This results in issues on our terminal, which cannot
>>>>>> handle the resulting characters, leading to incorrect display and halting
>>>>>> the auto-boot process in the U-Boot shell.
>>>>>>
>>>>>> *Log Output:*
>>>>>>
>>>>>> Warning: ethernet@ff0c0000 (eth0) using random MAC address - ee:df:0a:38:a8:c4
>>>>>>
>>>>>> eth0: ethernet@ff0c0000, eth1: mrmac@a40f0000, eth2: mrmac@a40f1000, eth3:
>>>>>> mrmac@a40f2000, eth4: mrmac@a40f3000
>>>>>>
>>>>>> Cannot persist EFI variables without system partition
>>>>>>
>>>>>> Missing TPMv2 device for EFI_TCG_PROTOCOL
>>>>>>
>>>>>> Missing RNG device for EFI_RNG_PROTOCOL
>>>>>>
>>>>>> Hit any key to stop autoboot:  0
>>>>>>
>>>>>> *Versal> [42;173R*
>>>>>>
>>>>>> *Unknown command '[42' - try 'help'*
>>>>>
>>>>> Hello Prasad,
>>>>>
>>>>> The terminal emulation seems to be sending a valid reply indicating 42 rows
>>>>> and 173 columns.
>>>>>
>>>>> It could be that in your setup you are hitting the time out in term_get_char().
>>>>>
>>>>> Could you, please, check what happens if you apply
>>>>>
>>>>> --- a/lib/efi_loader/efi_console.c
>>>>> +++ b/lib/efi_loader/efi_console.c
>>>>> @@ -92,8 +92,7 @@ static int term_get_char(s32 *c)
>>>>>           timeout = timer_get_us() + 100000;
>>>>>
>>>>>           while (!tstc())
>>>>> -               if (timer_get_us() > timeout)
>>>>> -                       return 1;
>>>>> +               ;
>>>>>
>>>>>           *c = getchar();
>>>>>           return 0;
>>>>>
>>>>> If the failure disappears, next you would have to analyze why the timeout is
>>>>> occurring (e.g. timer_get_us() running too fast).
>>>>>
>>>>> If you still get failures, please, add debug output term_read_reply() to
>>>>> understand what is happening.
>>>>
>>>> But why is this send at this stage? Why capsule update needs to be aware about
>>>> size of terminal? I understand that you need to know it for efi application
>>>> but what's the reason to know it capsule update?
>>>>
>>>> Thanks,
>>>> Michal
>>>>
>>>>
>>>
>>> Hello Michal,
>>>
>>> There are two separate questions to answer:
>>>
>>> * Why does determining the console not work?
>>>
>>> The problem can only occur EFI enabled systems. If determining the console size
>>> fails here, we can expect it to fail when running EFI applications, too.
>>
>> yes but you don't know if users are going to boot over EFI or via legacy way.
>> You can still use capsule update part only and using bootm/booti for booting.
>>
>>>
>>> * Why is the EFI console initialized here?
>>>
>>> We have currently only one entry-point for initializing the whole of the EFI
>>> sub-system. We are using the console before running any EFI application, e.g. in
>>> the eficonfig command.
>>>
>>> This could be redesigned with some effort.
>>
>> And I think it should. Because there is no reason to do initialization of
>> anything what it is not going to be used. At the end of day it is just delaying
>> boot.
> 
> Agreed. My attempt to resolve this for a similar situation just
> resulted in my case being a bad example[1]. But this is just bad
> design.

Glad to see your patches.
Not sure if implementation is fitting our needs but it is showing exactly the 
problem.
I pretty much think that nothing should be done unless you explicitly asks for 
it. And I can't see any variable/Kconfig option which is enabled to start to 
call these chars.

I am considering this as bug which should be fixed in this release because it is 
unintentionally breaking boot. I also expect that this is pretty much nice to 
have feature to use bigger screen instead of basic 80x25.

Thanks,
Michal


> 
> Regards,
> SImon
> 
> [1] https://lore.kernel.org/u-boot/20240926215950.1265143-9-sjg@chromium.org/


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Halting auto-boot process in the U-Boot shell https://lore.kernel.org/u-boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/
  2025-02-28 14:42           ` Michal Simek
@ 2025-03-06 13:56             ` Simon Glass
  0 siblings, 0 replies; 8+ messages in thread
From: Simon Glass @ 2025-03-06 13:56 UTC (permalink / raw)
  To: Michal Simek
  Cc: Tom Rini, Heinrich Schuchardt, Abbarapu, Venkatesh,
	Begari, Padmarao, xypron.glpk@gmx.de, u-boot@lists.denx.de,
	ilias.apalodimas@linaro.org, Kummari, Prasad

Hi Michal,

On Fri, 28 Feb 2025 at 07:42, Michal Simek <michal.simek@amd.com> wrote:
>
> Hi Simon,
>
> On 2/27/25 17:25, Simon Glass wrote:
> > Hi Michal,
> >
> > On Wed, 26 Feb 2025 at 02:04, Michal Simek <michal.simek@amd.com> wrote:
> >>
> >>
> >>
> >> On 2/26/25 09:59, Heinrich Schuchardt wrote:
> >>> On 2/26/25 09:28, Michal Simek wrote:
> >>>>
> >>>>
> >>>> On 2/26/25 09:23, Heinrich Schuchardt wrote:
> >>>>> On 2/26/25 07:48, Kummari, Prasad wrote:
> >>>>>> [AMD Official Use Only - AMD Internal Distribution Only]
> >>>>>>
> >>>>>>
> >>>>>> Hi Heinrich,
> >>>>>>
> >>>>>> Regarding below commit.
> >>>>>>
> >>>>>> commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85
> >>>>>> Author:     Heinrich Schuchardt <xypron.glpk@gmx.de>
> >>>>>> AuthorDate: Tue Jun 14 08:02:03 2022 +0200
> >>>>>> Commit:     Heinrich Schuchardt <xypron.glpk@gmx.de>
> >>>>>> CommitDate: Sun Jun 19 15:53:09 2022 +0200
> >>>>>>
> >>>>>>       efi_loader: initialize console size late
> >>>>>>
> >>>>>>       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.*
> >>>>>>
> >>>>>> https://lore.kernel.org/u-boot/20220614060203.33600-1-
> >>>>>> heinrich.schuchardt@canonical.com/ <https://lore.kernel.org/u-
> >>>>>> boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/>
> >>>>>>
> >>>>>> Commit 68edbed454b863dbcd197e19e1ab26a0a05c7d85 modifies the system to query
> >>>>>> the console size only when running an EFI binary. However, when
> >>>>>> CONFIG_EFI_CAPSULE_ON_DISK is enabled, the console is still being invoked,
> >>>>>> which shouldn't happen. This results in issues on our terminal, which cannot
> >>>>>> handle the resulting characters, leading to incorrect display and halting
> >>>>>> the auto-boot process in the U-Boot shell.
> >>>>>>
> >>>>>> *Log Output:*
> >>>>>>
> >>>>>> Warning: ethernet@ff0c0000 (eth0) using random MAC address - ee:df:0a:38:a8:c4
> >>>>>>
> >>>>>> eth0: ethernet@ff0c0000, eth1: mrmac@a40f0000, eth2: mrmac@a40f1000, eth3:
> >>>>>> mrmac@a40f2000, eth4: mrmac@a40f3000
> >>>>>>
> >>>>>> Cannot persist EFI variables without system partition
> >>>>>>
> >>>>>> Missing TPMv2 device for EFI_TCG_PROTOCOL
> >>>>>>
> >>>>>> Missing RNG device for EFI_RNG_PROTOCOL
> >>>>>>
> >>>>>> Hit any key to stop autoboot:  0
> >>>>>>
> >>>>>> *Versal> [42;173R*
> >>>>>>
> >>>>>> *Unknown command '[42' - try 'help'*
> >>>>>
> >>>>> Hello Prasad,
> >>>>>
> >>>>> The terminal emulation seems to be sending a valid reply indicating 42 rows
> >>>>> and 173 columns.
> >>>>>
> >>>>> It could be that in your setup you are hitting the time out in term_get_char().
> >>>>>
> >>>>> Could you, please, check what happens if you apply
> >>>>>
> >>>>> --- a/lib/efi_loader/efi_console.c
> >>>>> +++ b/lib/efi_loader/efi_console.c
> >>>>> @@ -92,8 +92,7 @@ static int term_get_char(s32 *c)
> >>>>>           timeout = timer_get_us() + 100000;
> >>>>>
> >>>>>           while (!tstc())
> >>>>> -               if (timer_get_us() > timeout)
> >>>>> -                       return 1;
> >>>>> +               ;
> >>>>>
> >>>>>           *c = getchar();
> >>>>>           return 0;
> >>>>>
> >>>>> If the failure disappears, next you would have to analyze why the timeout is
> >>>>> occurring (e.g. timer_get_us() running too fast).
> >>>>>
> >>>>> If you still get failures, please, add debug output term_read_reply() to
> >>>>> understand what is happening.
> >>>>
> >>>> But why is this send at this stage? Why capsule update needs to be aware about
> >>>> size of terminal? I understand that you need to know it for efi application
> >>>> but what's the reason to know it capsule update?
> >>>>
> >>>> Thanks,
> >>>> Michal
> >>>>
> >>>>
> >>>
> >>> Hello Michal,
> >>>
> >>> There are two separate questions to answer:
> >>>
> >>> * Why does determining the console not work?
> >>>
> >>> The problem can only occur EFI enabled systems. If determining the console size
> >>> fails here, we can expect it to fail when running EFI applications, too.
> >>
> >> yes but you don't know if users are going to boot over EFI or via legacy way.
> >> You can still use capsule update part only and using bootm/booti for booting.
> >>
> >>>
> >>> * Why is the EFI console initialized here?
> >>>
> >>> We have currently only one entry-point for initializing the whole of the EFI
> >>> sub-system. We are using the console before running any EFI application, e.g. in
> >>> the eficonfig command.
> >>>
> >>> This could be redesigned with some effort.
> >>
> >> And I think it should. Because there is no reason to do initialization of
> >> anything what it is not going to be used. At the end of day it is just delaying
> >> boot.
> >
> > Agreed. My attempt to resolve this for a similar situation just
> > resulted in my case being a bad example[1]. But this is just bad
> > design.
>
> Glad to see your patches.
> Not sure if implementation is fitting our needs but it is showing exactly the
> problem.
> I pretty much think that nothing should be done unless you explicitly asks for
> it. And I can't see any variable/Kconfig option which is enabled to start to
> call these chars.
>
> I am considering this as bug which should be fixed in this release because it is
> unintentionally breaking boot. I also expect that this is pretty much nice to
> have feature to use bigger screen instead of basic 80x25.

From my side I'm unable to get patches into EFI_LOADER and have tried
repeatedly to adjust the ANSI logic[2]. So I'm going to leave it to
others to figure out.

Regards,
Simon

>
> Thanks,
> Michal
>
>
> >
> > Regards,
> > SImon
> >
> > [1] https://lore.kernel.org/u-boot/20240926215950.1265143-9-sjg@chromium.org/
>

[2] https://patchwork.ozlabs.org/project/uboot/patch/20231121113557.800353-5-sjg@chromium.org/
https://patchwork.ozlabs.org/project/uboot/patch/20240811145209.4191404-37-sjg@chromium.org/
https://patchwork.ozlabs.org/project/uboot/patch/20240815202424.766778-11-sjg@chromium.org/
https://patchwork.ozlabs.org/project/uboot/patch/20240902011825.746421-11-sjg@chromium.org/
https://patchwork.ozlabs.org/project/uboot/patch/20240926215950.1265143-9-sjg@chromium.org/
https://patchwork.ozlabs.org/project/uboot/patch/20240926220226.1265965-9-sjg@chromium.org/

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-03-06 13:57 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-26  6:48 Halting auto-boot process in the U-Boot shell https://lore.kernel.org/u-boot/20220614060203.33600-1-heinrich.schuchardt@canonical.com/ Kummari, Prasad
2025-02-26  8:23 ` Heinrich Schuchardt
2025-02-26  8:28   ` Michal Simek
2025-02-26  8:59     ` Heinrich Schuchardt
2025-02-26  9:04       ` Michal Simek
2025-02-27 16:25         ` Simon Glass
2025-02-28 14:42           ` Michal Simek
2025-03-06 13:56             ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.