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

[Heinrich Schuchardt]

Am 18. Januar 2023 21:58:42 MEZ schrieb Jan Kiszka <jan.kiszka at siemens.com>:
>On 18.01.23 21:56, Heinrich Schuchardt wrote:
>> 
>> 
>> Am 18. Januar 2023 21:44:58 MEZ schrieb Jan Kiszka <jan.kiszka at siemens.com>:
>>> From: Jan Kiszka <jan.kiszka at 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. Still, there
>>> were remaining concerns about side effects, so this is provided as an
>>> opt-in feature.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka at siemens.com>
>>> ---
>>> lib/efi_loader/Kconfig       |  9 +++++++++
>>> lib/efi_loader/efi_console.c | 10 +++++++++-
>>> 2 files changed, 18 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>>> index b498c72206f..b2c3250b4ad 100644
>>> --- a/lib/efi_loader/Kconfig
>>> +++ b/lib/efi_loader/Kconfig
>>> @@ -124,6 +124,15 @@ config EFI_SET_TIME
>>> 	  Provide the SetTime() runtime service at boottime. This service
>>> 	  can be used by an EFI application to adjust the real time clock.
>>>
>>> +config EFI_SCROLL_ON_CLEAR_SCREEN
>>> +	bool "Avoid overwriting previous output on clear screen"
>>> +	help
>>> +	  Instead of moving the cursor home when the console screen should be
>>> +	  cleared, emit blank new lines so that previous output is scrolled
>>> +	  out of sight rather than overwritten. This allows on serial consoles
>>> +	  to capture complete boot logs (except for interactive menus etc.)
>>> +	  and can ease debugging related issues.
>>> +
>>> config EFI_HAVE_CAPSULE_SUPPORT
>>> 	bool
>>>
>>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>>> index 4d08dd3763a..e40c129d1b2 100644
>>> --- a/lib/efi_loader/efi_console.c
>>> +++ b/lib/efi_loader/efi_console.c
>>> @@ -461,10 +461,18 @@ 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)
>>> {
>>> +	if (CONFIG_IS_ENABLED(EFI_SCROLL_ON_CLEAR_SCREEN)) {
>>> +		unsigned int row;
>>> +
>>> +		/* Avoid overwriting previous outputs on streaming consoles */
>> 
>> Thanks for making the behavior configurable.
>> 
>> We cannot assume that the cursor is on the last line. 
>> 
>> Assume that the cursor is on the first line. Then the screen will still be deleted.
>> 
>> Especially in GRUB the cursor often is in the upper half of the screen
>> 
>> The cursor should be moved to the right lower corner before scrolling.
>
>Can do, though logging interactive menus was not in scope (and can't be
>solved this way anyway).
>
>Jan
>


E.g. you can edit a menu entry in GRUB and press F10 and want to know what the entry looked like.