[PATCH v4 0/2] efi_loader: console improvements

[PATCH v4 0/2] efi_loader: console improvements

[Jan Kiszka]

Changes to earlier versions:

 - dropped merged "Let networking support depend on NETDEVICES"
 - made scrolling on clear-screen a Kconfig option

Jan

Jan Kiszka (2):
  efi_loader: Avoid overwriting previous outputs on console screen
    clearing
  efi_loader: Set default console colors on efi_cout_clear_screen if
    needed

 lib/efi_loader/Kconfig       |  9 +++++++++
 lib/efi_loader/efi_console.c | 16 +++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

-- 
2.35.3

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

[Jan Kiszka]

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. Still, there
were remaining concerns about side effects, so this is provided as an
opt-in feature.

Signed-off-by: Jan Kiszka <jan.kiszka@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 */
+		for (row = 1; row < efi_cout_modes[efi_con_mode.mode].rows; row++)
+			printf("\n");
+	}
+
 	/*
 	 * The Linux console wants both a clear and a home command. The video
 	 * uclass does not support <ESC>[H without coordinates, yet.
-- 
2.35.3

[PATCH v4 2/2] efi_loader: Set default console colors on efi_cout_clear_screen if needed

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Ensures a consistent background color of the whole screen for succeeding
outputs as both demanded by the spec and implemented in EDK2 as well.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 lib/efi_loader/efi_console.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
index e40c129d1b2..84b5f60b321 100644
--- a/lib/efi_loader/efi_console.c
+++ b/lib/efi_loader/efi_console.c
@@ -497,6 +497,12 @@ static efi_status_t EFIAPI efi_cout_clear_screen(
 {
 	EFI_ENTRY("%p", this);
 
+	/* Set default colors if not done yet */
+	if (efi_con_mode.attribute == 0) {
+		efi_con_mode.attribute = 0x07;
+		printf(ESC "[0;37;40m");
+	}
+
 	efi_clear_screen();
 
 	return EFI_EXIT(EFI_SUCCESS);
-- 
2.35.3

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

[Heinrich Schuchardt]

Am 18. Januar 2023 21:44:58 MEZ schrieb Jan Kiszka <jan.kiszka@siemens.com>:
>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. Still, there
>were remaining concerns about side effects, so this is provided as an
>opt-in feature.
>
>Signed-off-by: Jan Kiszka <jan.kiszka@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.

Best regards

Heinrich 

>+		for (row = 1; row < efi_cout_modes[efi_con_mode.mode].rows; row++)
>+			printf("\n");
>+	}
>+
> 	/*
> 	 * The Linux console wants both a clear and a home command. The video
> 	 * uclass does not support <ESC>[H without coordinates, yet.

Re: [PATCH v4 2/2] efi_loader: Set default console colors on efi_cout_clear_screen if needed

[Heinrich Schuchardt]

Am 18. Januar 2023 21:44:59 MEZ schrieb Jan Kiszka <jan.kiszka@siemens.com>:
>From: Jan Kiszka <jan.kiszka@siemens.com>
>
>Ensures a consistent background color of the whole screen for succeeding
>outputs as both demanded by the spec and implemented in EDK2 as well.
>
>Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>


Reviewed-by: Heinrich Schuchard <xypron.glpk@gmx.de>

>---
> lib/efi_loader/efi_console.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>index e40c129d1b2..84b5f60b321 100644
>--- a/lib/efi_loader/efi_console.c
>+++ b/lib/efi_loader/efi_console.c
>@@ -497,6 +497,12 @@ static efi_status_t EFIAPI efi_cout_clear_screen(
> {
> 	EFI_ENTRY("%p", this);
> 
>+	/* Set default colors if not done yet */
>+	if (efi_con_mode.attribute == 0) {
>+		efi_con_mode.attribute = 0x07;
>+		printf(ESC "[0;37;40m");
>+	}
>+
> 	efi_clear_screen();
> 
> 	return EFI_EXIT(EFI_SUCCESS);

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

[Jan Kiszka]

On 18.01.23 21:56, Heinrich Schuchardt wrote:
> 
> 
> Am 18. Januar 2023 21:44:58 MEZ schrieb Jan Kiszka <jan.kiszka@siemens.com>:
>> 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. Still, there
>> were remaining concerns about side effects, so this is provided as an
>> opt-in feature.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@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

-- 
Siemens AG, Technology
Competence Center Embedded Linux

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@siemens.com>:
>On 18.01.23 21:56, Heinrich Schuchardt wrote:
>> 
>> 
>> Am 18. Januar 2023 21:44:58 MEZ schrieb Jan Kiszka <jan.kiszka@siemens.com>:
>>> 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. Still, there
>>> were remaining concerns about side effects, so this is provided as an
>>> opt-in feature.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@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.

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

[Jan Kiszka]

On 18.01.23 22:02, Heinrich Schuchardt wrote:
> 
> 
> Am 18. Januar 2023 21:58:42 MEZ schrieb Jan Kiszka <jan.kiszka@siemens.com>:
>> On 18.01.23 21:56, Heinrich Schuchardt wrote:
>>>
>>>
>>> Am 18. Januar 2023 21:44:58 MEZ schrieb Jan Kiszka <jan.kiszka@siemens.com>:
>>>> 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. Still, there
>>>> were remaining concerns about side effects, so this is provided as an
>>>> opt-in feature.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@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.
> 

Yes, that special case could be caught (not very relevant in embedded
practice, though), but any change you did before that is likely not
readably logged. That's why I don't consider interactive menus in scope
here.

Nevertheless, patch update is under test now.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux