linuxboot / heads

A minimal Linux that runs as a coreboot or LinuxBoot ROM payload to provide a secure, flexible boot environment for laptops, workstations and servers.
https://osresearch.net/
GNU General Public License v2.0
1.42k stars 186 forks source link

Some issues discovered with whiptail output (BMC tested: smaller screen) #1237

Closed tlaurion closed 1 year ago

tlaurion commented 1 year ago

Some tests conducted on https://github.com/osresearch/heads/tree/1cb0324a12aa4aec01287142c76b5a41ed51a2cb

Whiptail can fixate (optional) the size of its windows with calls of Height x Width. As of now on master, it's 30 x 90, where 90 is too wide for BMC output. This causes some issues. Also, some output is considered to be of a static height, causing issues as well.

It is understandable to want to have fixated Height x Width to have consistent UX through menus, but that should not cause bogus output.


Main menu (gui-init's show_main_menu function):

30x90 2022-11-08-133047

30x80 2022-11-08-150634

20x80: 2022-11-08-144920

0x80: 2022-11-08-150908

0x0 2022-11-08-150808


Some menus just don't show properly without proper selections, as seen above, but also can hide option boxes:

Here, tampered with a single kexec_hashes.txt entree and selecting default boot option. This happens under gui-init's verify_global_hashes:

30x90: 2022-11-08-151802

30x80: 2022-11-08-151445

20x80: 2022-11-08-151940

0x80 2022-11-08-152047

tlaurion commented 1 year ago

Attempted to pass vga=795 to kernel, but without change in behavior. As of now setting 20x80 instead of 30x90 everywhere fixes it.

tlaurion commented 1 year ago

Here, tampered with 9 entrees under kexec_hashes.txt entries and selecting default boot option. This happens under gui-init's verify_global_hashes

30x90: 2022-11-08-164801

20x80 2022-11-08-162752

tlaurion commented 1 year ago

But unfortunately, not everything fits under 20x90.

One of those is oem-factory-reset script: 2022-11-08-164927

I'll continue to try to find a way to increase ast2500 console settings. @JonathonHall-Purism @SergiiDmytruk any idea? passing vga=795 to kernel was actually passed but didn't change a thing.

tlaurion commented 1 year ago

But then, if I pass 0x90: 2022-11-08-170859

If I pass 0x0: 2022-11-08-170935

So @JonathonHall-Purism I understand fixating 80 (the width to minimal display size) or maybe configure it under board configs. But I think the height maybe should be dynamic?

JonathonHall-Purism commented 1 year ago

Definitely agree with dynamic height. It doesn't make sense to leave those small prompts swimming in empty space, especially for short yes/no where the prompt and selections are separated by a lot of blank lines.

Re: width, just from having looked at the above, I actually like the dynamic width. It looks like all our prompts have hard line breaks anyway to accommodate fbwhiptail, so I think the dynamic width looks best. The main boot menu in particular looks better without a bunch of extra whitespace, IMO. I don't think there is a UX consistency issue (on the contrary, I think it is easier to read at a glance).

But otherwise, I'd say to fix it globally to 80, not make it board-specific. IMO being able to raise the width per board is not that useful right now since the prompts need hard line breaks for fbwhiptail anyway.

tlaurion commented 1 year ago

@JonathonHall-Purism there is some place in code that are using busybox's "fold" for long strings, without having to fuss around into manipulating strings to match constant width. From my succing tests, I haven't seen any regression in GUIs setting the width to 80.

Will push PR soon to set width to 80 and height to 0 (dynamic).

tlaurion commented 1 year ago

Ok, testing those on talos II where artifacts under https://app.circleci.com/pipelines/github/tlaurion/heads/1253/workflows/08e0593b-1726-4af7-a7f5-059c740bdef2/jobs/11935/artifacts were flashed.

Which are linked to talos II specific : https://github.com/tlaurion/heads/tree/SergiiDmytruk_flashrom_fff474a_9bb6be887457a3cb28ac831c1547a33752429184

And branch on top of master https://github.com/tlaurion/heads/tree/whiptail_dynamic_height_fixated_width_to_80


Some output with 0x80 over BMC:

2022-11-09-133801 2022-11-09-133816 2022-11-09-133832 2022-11-09-133850 2022-11-09-133947 2022-11-09-134007 2022-11-09-134027