Hi Heinrich,
On Mon, 31 Oct 2022 at 15:58, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 10/31/22 20:27, Simon Glass wrote:
Hi Heinrich,
On Sat, 29 Oct 2022 at 20:56, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 10/30/22 02:43, Simon Glass wrote:
Hi Heinrich,
On Thu, 27 Oct 2022 at 10:41, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 10/27/22 17:22, Simon Glass wrote:
Hi Heinrich,
On Wed, 26 Oct 2022 at 00:13, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote: > > > > On 10/26/22 01:35, Simon Glass wrote: >> Hi Heinrich, >> >> On Sat, 22 Oct 2022 at 03:21, Heinrich Schuchardt >> heinrich.schuchardt@canonical.com wrote: >>> >>> We may enter the command line interface in a state where on the remote >>> console the cursor is not shown. Send an escape sequence to enable it. >>> >>> Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com >>> --- >>> common/main.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/common/main.c b/common/main.c >>> index 682f3359ea..4e7e7b17d6 100644 >>> --- a/common/main.c >>> +++ b/common/main.c >>> @@ -7,6 +7,7 @@ >>> /* #define DEBUG */ >>> >>> #include <common.h> >>> +#include <ansi.h> >>> #include <autoboot.h> >>> #include <bootstage.h> >>> #include <cli.h> >>> @@ -66,6 +67,9 @@ void main_loop(void) >>> >>> autoboot_command(s); >>> >>> + if (!CONFIG_IS_ENABLED(DM_VIDEO) || CONFIG_IS_ENABLED(VIDEO_ANSI)) >>> + printf(ANSI_CURSOR_SHOW "\n"); >> >> Could we create a library which emits these codes? Like ansi_cursor_show() ? > > The question is not if we could but if we should. > > Up to now we tried to call printf() only once where ANSI_* is only one > part of the output string. > > E.g. cmd/eficonfig.c:415: > printf(ANSI_CLEAR_CONSOLE > ANSI_CURSOR_POSITION > ANSI_CURSOR_SHOW, 1, 1); > > This minimizes the number of call library calls but may not be very elegant. > > Creating a library function for ANSI_CURSOR_SHOW alone does not make > much sense to me. Should you want to convert our code base from using > ANSI_* to library functions this should cover the whole code base. > > So I think we should let this patch pass as it solves a current problem > and leave creating a ANSI_* function library to a separate exercise.
Then we should add this to the serial API...what we have here is quite bizarre in a way, since it outputs escape characters as the default for U-Boot. Mostly we don't need it.
So add a set_cursor_visible() method to the serial API and allow it to be controlled via platform data in the serial driver.
I have no clue which platform data you might be thinking of.
It is also not my intention to eject the escape sequence whenever a serial console is probed.
Using serial_puts() in this patch instead of printf() would help to avoid the CONFIG dependency.
Here's what should happen:
- ANSI codes should not appear in logs or on CI (this is currently a
problem with EFI tests)
A program outputting different things in continuous integration then in real live does not provide realistic testing.
It is the CI that has to adapt not the program under test.
Which logs are you talking about?
- We cannot show an ANSI code on boot by default, since that puts
ctrl characters into every start-up of U-Boot
Why would that be a problem?
So I think my suggestion makes sense. See drivers/serial/sandbox.c and have a way of calling isatty() to check the terminal type (with a cmdline flag to override it, see -t for example). Then we can get the correct behaviour.
isatty() is not a U-Boot function. So it is irrelevant in this context.
You cannot detect if a serial console understands ANSI sequences without sending any.
NAK
Please try a little harder to understand my POV above.
I don't understand what your point of view as you did not answer my question.
The only case of logs being written in a production U-Boot is given by CONGIG_LOG_SYSLOG=y which is not affected by the patch. So no clue what logs you refer to.
If you run the tests (e.g. make check) and get a failure you can see the output from them on the console. At the moment some tests clear the console, change colour and all sorts of things. That is not good behaviour. Can you see that?
Concerning CI it is clear that the CI has to handle ANSI sequences in the U-Boot output sensibly. The idea of U-Boot producing different output running in CI than outside CI would contradict the purpose of the CI.
Have you actually looked at the CI output? I am attaching it at the end of this email so you can see it.
I further have no clue what problem you face with the output of the EFI unit tests. Could you, please, be more specific.
That is a whole other topic. We definitely need more C tests and fewer of these large Python end-to-end tests.
But for now, all I am saying is that ANSI characters should not be output in CI. For sandbox they can be output but only if sending to a terminal, or specifically enabled with a flag. Tests that clear the console or output thousands of lines of gibberish output are not acceptable in my book.
Regards, Simon
output:
$ pyt eficonfig ================================================ test session starts ================================================= platform linux -- Python 3.10.6, pytest-6.2.5, py-1.10.0, pluggy-0.13.0 rootdir: /scratch/sglass/cosarm/src/third_party/u-boot/files/test/py, configfile: pytest.ini plugins: hypothesis-6.36.0, forked-1.4.0, xdist-2.5.0 collected 1022 items / 1021 deselected / 1 selected
test/py/tests/test_eficonfig/test_eficonfig.py F [100%]
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
====================================================== FAILURES ====================================================== _________________________________________________ test_efi_eficonfig _________________________________________________ test/py/tests/test_eficonfig/test_eficonfig.py:352: in test_efi_eficonfig assert False E assert False ----------------------------------------------- Captured stdout setup ------------------------------------------------ /u-boot Creating new bloblist size 400 at c000 sandbox_serial serial: pinctrl_select_state_full: uclass_get_device_by_phandle_id: err=-19
U-Boot 2022.10-00013-g695b6238212-dirty (Oct 29 2022 - 17:59:55 -0600)
Reset Status: WARM Reset Status: COLD Model: sandbox DRAM: 256 MiB single-pinctrl pinctrl-single-no-width: missing register width Core: 258 devices, 95 uclasses, devicetree: board WDT: Not starting wdt-gpio-toggle wdt_gpio wdt-gpio-level: Request for wdt gpio failed: -16 WDT: Not starting wdt@0 MMC: mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD) Loading Environment from nowhere... OK In: cros-ec-keyb Out: vidconsole Err: vidconsole Model: sandbox Net: eth0: eth@10002000, eth5: eth@10003000, eth3: sbe5, eth6: eth@10004000, eth8: phy-test-eth, eth4: dsa-test-eth, eth2: lan0, eth7: lan1 Hit any key to stop autoboot: 0 => ------------------------------------------------ Captured stdout call ------------------------------------------------ /u-boot Creating new bloblist size 400 at c000 sandbox_serial serial: pinctrl_select_state_full: uclass_get_device_by_phandle_id: err=-19
U-Boot 2022.10-00013-g695b6238212-dirty (Oct 29 2022 - 17:59:55 -0600)
Reset Status: WARM Reset Status: COLD Model: sandbox DRAM: 256 MiB single-pinctrl pinctrl-single-no-width: missing register width Core: 258 devices, 95 uclasses, devicetree: board WDT: Not starting wdt-gpio-toggle wdt_gpio wdt-gpio-level: Request for wdt gpio failed: -16 WDT: Not starting wdt@0 MMC: mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD) Loading Environment from nowhere... OK In: cros-ec-keyb Out: vidconsole Err: vidconsole Model: sandbox Net: eth0: eth@10002000, eth5: eth@10003000, eth3: sbe5, eth6: eth@10004000, eth8: phy-test-eth, eth4: dsa-test-eth, eth2: lan0, eth7: lan1 Hit any key to stop autoboot: 0 => => eficonfig ** Bad device specification host 0 ** Couldn't find partition host 0:0 Cannot read EFI system partition
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Add Boot Option **
Description: File: Initrd File: Optional Data: Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
=> => host bind 0 /tmp/b/sandbox/persistent-data/efi_eficonfig.img => => eficonfig Cannot read EFI system partition Failed to persist EFI variables Cannot read EFI system partition Failed to persist EFI variables
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Change Boot Order **
[*] host 0:1 Save Quit
Press UP/DOWN to move, +/- to change order Press SPACE to activate or deactivate the entry Select [Save] to complete, ESC/CTRL+C to quit
** Change Boot Order **
[ ] host 0:1 Save Quit
Press UP/DOWN to move, +/- to change order Press SPACE to activate or deactivate the entry Select [Save] to complete, ESC/CTRL+C to quit
** Change Boot Order **
[ ] host 0:1 Save QuitCannot read EFI system partition Failed to persist EFI variables Press UP/DOWN to move, +/- to change order Press SPACE to activate or deactivate the entry Select [Save] to complete, ESC/CTRL+C to quit
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Add Boot Option **
Description: File: Initrd File: Optional Data: Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Edit Description **
enter description: test 1
Press ENTER to complete, ESC/CTRL+C to quit
** Add Boot Option **
Description: test 1 File: Initrd File: Optional Data: Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Update File **
Select File Clear Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select Volume **
host 0:1 Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select File **
initrd-1.img initrd-2.img initrddump.efi Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Add Boot Option **
Description: test 1 File: host 0:1/initrddump.efi Initrd File: Optional Data: Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Update File **
Select File Clear Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select Volume **
host 0:1 Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select File **
initrd-1.img initrd-2.img initrddump.efi Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Add Boot Option **
Description: test 1 File: host 0:1/initrddump.efi Initrd File: host 0:1/initrd-1.img Optional Data: Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Edit Optional Data **
enter optional data: nocolor
Press ENTER to complete, ESC/CTRL+C to quit
** Add Boot Option **
Description: test 1 File: host 0:1/initrddump.efi Initrd File: host 0:1/initrd-1.img Optional Data: nocolor Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
Cannot read EFI system partition Failed to persist EFI variables Cannot read EFI system partition Failed to persist EFI variables
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
=> => bootefi bootmgr Booting: test 1
INITRD Dump ===========
=> => load length: 0x8 crc32: 0x181464af => => exit
=> => eficonfig
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Add Boot Option **
Description: File: Initrd File: Optional Data: Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Edit Description **
enter description: test 2
Press ENTER to complete, ESC/CTRL+C to quit
** Add Boot Option **
Description: test 2 File: Initrd File: Optional Data: Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Update File **
Select File Clear Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select Volume **
host 0:1 Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select File **
initrd-1.img initrd-2.img initrddump.efi Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Add Boot Option **
Description: test 2 File: host 0:1/initrddump.efi Initrd File: Optional Data: Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Update File **
Select File Clear Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select Volume **
host 0:1 Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select File **
initrd-1.img initrd-2.img initrddump.efi Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Add Boot Option **
Description: test 2 File: host 0:1/initrddump.efi Initrd File: host 0:1/initrd-2.img Optional Data: Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Edit Optional Data **
enter optional data: nocolor
Press ENTER to complete, ESC/CTRL+C to quit
** Add Boot Option **
Description: test 2 File: host 0:1/initrddump.efi Initrd File: host 0:1/initrd-2.img Optional Data: nocolor Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
Cannot read EFI system partition Failed to persist EFI variables Cannot read EFI system partition Failed to persist EFI variables
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Change Boot Order **
[*] test 1 [*] test 2 [ ] host 0:1 Save Quit
Press UP/DOWN to move, +/- to change order Press SPACE to activate or deactivate the entry Select [Save] to complete, ESC/CTRL+C to quit
** Change Boot Order **
[*] test 1 [*] test 2 [ ] host 0:1 Save Quit
Press UP/DOWN to move, +/- to change order Press SPACE to activate or deactivate the entry Select [Save] to complete, ESC/CTRL+C to quit
** Change Boot Order **
[*] test 2 [*] test 1 [ ] host 0:1 Save Quit
Press UP/DOWN to move, +/- to change order Press SPACE to activate or deactivate the entry Select [Save] to complete, ESC/CTRL+C to quit
** Change Boot Order **
[*] test 2 [*] test 1 [ ] host 0:1 Save Quit
Press UP/DOWN to move, +/- to change order Press SPACE to activate or deactivate the entry Select [Save] to complete, ESC/CTRL+C to quit
** Change Boot Order **
[*] test 2 [*] test 1 [ ] host 0:1 Save Quit
Press UP/DOWN to move, +/- to change order Press SPACE to activate or deactivate the entry Select [Save] to complete, ESC/CTRL+C to quit
** Change Boot Order **
[*] test 2 [*] test 1 [ ] host 0:1 Save QuitCannot read EFI system partition Failed to persist EFI variables Press UP/DOWN to move, +/- to change order Press SPACE to activate or deactivate the entry Select [Save] to complete, ESC/CTRL+C to quit
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
=> => bootefi bootmgr Booting: test 2
INITRD Dump ===========
=> => load length: 0x8 crc32: 0x811d3515 => => exit
=> => eficonfig
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Change Boot Order **
[*] test 2 [*] test 1 [ ] host 0:1 Save Quit
Press UP/DOWN to move, +/- to change order Press SPACE to activate or deactivate the entry Select [Save] to complete, ESC/CTRL+C to quit
** Change Boot Order **
[*] test 1 [*] test 2 [ ] host 0:1 Save Quit
Press UP/DOWN to move, +/- to change order Press SPACE to activate or deactivate the entry Select [Save] to complete, ESC/CTRL+C to quit
** Change Boot Order **
[*] test 1 [*] test 2 [ ] host 0:1 Save Quit
Press UP/DOWN to move, +/- to change order Press SPACE to activate or deactivate the entry Select [Save] to complete, ESC/CTRL+C to quit
** Change Boot Order **
[*] test 1 [*] test 2 [ ] host 0:1 Save QuitCannot read EFI system partition Failed to persist EFI variables Press UP/DOWN to move, +/- to change order Press SPACE to activate or deactivate the entry Select [Save] to complete, ESC/CTRL+C to quit
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
=> => bootefi bootmgr Booting: test 1
INITRD Dump ===========
=> => load length: 0x8 crc32: 0x181464af => => exit
=> => eficonfig
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select Boot Option **
test 1 test 2 Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
Cannot read EFI system partition Failed to persist EFI variables Cannot read EFI system partition Failed to persist EFI variables
** Select Boot Option **
test 1 Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
=> => eficonfig
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select Boot Option **
test 1 Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Edit Boot Option **
Description: test 1 File: host 0:1/initrddump.efi Initrd File: host 0:1/initrd-1.img Optional Data: nocolor Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Edit Description **
enter description: test 3
Press ENTER to complete, ESC/CTRL+C to quit
** Edit Boot Option **
Description: test 3 File: host 0:1/initrddump.efi Initrd File: host 0:1/initrd-1.img Optional Data: nocolor Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Update File **
Select File Clear Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select Volume **
host 0:1 Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select File **
initrd-1.img initrd-2.img initrddump.efi Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Edit Boot Option **
Description: test 3 File: host 0:1/initrddump.efi Initrd File: host 0:1/initrd-1.img Optional Data: nocolor Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Update File **
Select File Clear Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select Volume **
host 0:1 Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select File **
initrd-1.img initrd-2.img initrddump.efi Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Edit Boot Option **
Description: test 3 File: host 0:1/initrddump.efi Initrd File: host 0:1/initrd-2.img Optional Data: nocolor Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Edit Optional Data **
enter optional data:
Press ENTER to complete, ESC/CTRL+C to quit
** Edit Boot Option **
Description: test 3 File: host 0:1/initrddump.efi Initrd File: host 0:1/initrd-2.img Optional Data: Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
Cannot read EFI system partition Failed to persist EFI variables
** Select Boot Option **
test 3 Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
=> => bootefi bootmgr Booting: test 3
INITRD Dump ===========
=> => load length: 0x8 crc32: 0x811d3515 => => exit
=> => eficonfig
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Select Boot Option **
test 3 Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
Cannot read EFI system partition Failed to persist EFI variables Cannot read EFI system partition Failed to persist EFI variables
** Select Boot Option **
Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
=> => host bind -r 0 => => eficonfig
** UEFI Maintenance Menu **
Add Boot Option Edit Boot Option Change Boot Order Delete Boot Option Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Add Boot Option **
Description: File: Initrd File: Optional Data: Save Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
** Update File **
Select File Clear Quit
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
No block device found!
Press any key to continue ============================================== short test summary info =============================================== FAILED test/py/tests/test_eficonfig/test_eficonfig.py::test_efi_eficonfig - assert False ========================================= 1 failed, 1021 deselected in 5.98s ========================================= ^[[33;118R1 (<detached:remotes/us/master=a90af test/ cspell.json u.code-workspace) sglass@ELLESMERE ~/u> ;118R