Halting auto-boot process in the U-Boot shell https://lore.kernel.org/u-boot/20220614060203.33600-1-heinrich.schuchardt at canonical.com/

[Michal Simek]

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 at 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 at gmx.de>
>>>>>> AuthorDate: Tue Jun 14 08:02:03 2022 +0200
>>>>>> Commit:     Heinrich Schuchardt <xypron.glpk at 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 at canonical.com/ <https://lore.kernel.org/u-
>>>>>> boot/20220614060203.33600-1-heinrich.schuchardt at 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 at ff0c0000 (eth0) using random MAC address - ee:df:0a:38:a8:c4
>>>>>>
>>>>>> eth0: ethernet at ff0c0000, eth1: mrmac at a40f0000, eth2: mrmac at a40f1000, eth3:
>>>>>> mrmac at a40f2000, eth4: mrmac at 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/