On Tue, Oct 15, 2024 at 07:25:55AM -0600, Simon Glass wrote:
Hi Heinrich,
On Tue, 15 Oct 2024 at 05:36, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 15.10.24 12:19, Mark Kettenis wrote:
Date: Mon, 14 Oct 2024 15:11:50 -0600 From: Tom Rini trini@konsulko.com
On Mon, Oct 14, 2024 at 01:13:41PM -0600, Simon Glass wrote:
[snip]
Or perhaps just have a way to turn it off? I first sent this patch last November. It is just wrong to generate output like this which we don't want. There isn't even a test for it, so just add a way to disable it, and be done!
I don't know that it's unwanted. As I'm trying to get Heinrich to explain, _why_ does efi_setup_console_size need to exist, and do what it does? This isn't the case of color-coding the output of tests as they happen, there must be a reason we care about knowing the console size. At that point we can figure out if the right answer is:
- Don't generate that check on serial ports, it's somewhere between misleading to wrong.
- text-based tests just need to expect and skip it because there's a good reason to need to know the console size and not just assume 80x24.
- Something else we won't know until it's clearly explained why we do this.
Sadly this is a misfeature of UEFI. The EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL is required if console devices are supported by the UEFI implementation, and is built around the concept of "text mode" with a fixed number of character rows and columns. That pretty much assumes there is some sort of terminal emulator on the other end. And this is pretty much incompatible with anything that just want to log serial output.
I think it is possible to do better though. Instead of calling efi_setup_console_size() in efi_init_obj_list(), we could postpone this until the application makes a call that requires us to know the size. This would mean that a simple EFI application that just uses EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString() (such as the Hello world EFI application or the OpenBSD EFI bootloader) wouldn't have to do the size query.
The implementation of EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString() needs to know the output size to update the cursor position in
EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.MODE.CursorColumn EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.MODE.CursorRow
The EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL is passed to the entry point of any EFI application via the system table.
We could delay the invocation of efi_setup_console_size() to the launch of the first EFI application (efi_start_image()).
This discussion I believe should continue as it does sound like we can be a bit better about when we do this required series of events.
That's a start, but I think the right answer here is to use my flag and in efi_setup_console_size() don't call query_console_serial() if the flag is set. Then the default (80x25) will be returned, which is fine for the test which is the subject of this series.
No, your series, like other tests that already exist and have this escape code printed, should just swallow the escape sequence if it shows up because, sadly, there is reason for it. I wonder if we should have some test _for_ this sequence, or otherwise make sure we see it when we should see it?