Re: [PATCH v3 1/3] efi_loader: Avoid overwriting previous outputs on console screen clearing

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 11/7/22 17:41, Jan Kiszka wrote:
> On 07.11.22 17:29, Heinrich Schuchardt wrote:
>> On 11/4/22 09:06, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> Before clearing the screen, ensure that no previous output of firmware
>>> or UEFI programs will be overwritten on serial devices or other
>>> streaming consoles. This helps generating complete boot logs.
>>>
>>> Tested regarding multi-output against qemu-x86_defconfig.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>    lib/efi_loader/efi_console.c | 8 +++++++-
>>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>>> index 4d08dd3763a..6ce0fcc168d 100644
>>> --- a/lib/efi_loader/efi_console.c
>>> +++ b/lib/efi_loader/efi_console.c
>>> @@ -461,10 +461,16 @@ static efi_status_t EFIAPI efi_cout_set_attribute(
>>>    }
>>>
>>>    /**
>>> - * efi_cout_clear_screen() - clear screen
>>> + * efi_clear_screen() - clear screen
>>>     */
>>>    static void efi_clear_screen(void)
>>>    {
>>> +    unsigned int row;
>>> +
>>> +    /* Avoid overwriting previous outputs on streaming consoles */
>>> +    for (row = 1; row < efi_cout_modes[efi_con_mode.mode].rows; row++)
>>> +        printf("\n");
>>> +
>>
>> Scrolling on a framebuffer is a very expensive operations: For each of
>> the 135 lines of an UHD display you have to copy 32 MiB. This would
>> create a delay of multiple seconds on a slow device.
>
> Thanks for explaining this - now. I didn't observe this delay in QEMU,
> but maybe that wasn't representative

My BananaPi is really slow on it.

>
>>
>> So keep the patch for your debugging purposed. But we cannot merge it
>> into upstream.
>
> It's in production - we can't afford shipping without it.
>
> To avoid that this will be the last patch between full upstream and our
> device one day: Do you see a way to confine overwrite protection to real
> streaming device, UARTs and such?

How about putting this behind a Kconfig switch?

You probably should first move the cursor to the lower right corner
before sending carriage returns and then place the cursor in the upper
left corner. Otherwise you cannot be sure that you scrolled all of the
text out of the visible area.

If you use scrolling, why would you still send an ANSI sequence for
clear screen?

Best regards

Heinrich

>
> Thanks,
> Jan
>