Hello,
While debugging #1103692 on Debian [0], I came across an issue where
error messages generated early in the syslinux boot process through
writestr() didn't end up appearing on-screen. Instead, it appeared
that only the cursor moved.
I discovered that it's because the early code passes a video attribute
of 0 to the efi_write_char() function, which is black-on-black, and
produces invisible output.
The output can actually be read on the serial line. For example, if we
have a serial line dump in the file serial.dump, and we have problems
with loading ldlinux.e32, the following can be used to read the message:
$ ansi2txt < serial.dump > serial.log
$ tail -n4 serial.log
Shell> FS0:\EFI\syslinux\syslinux.efi
Failed to load ldlinux.e32
Shell>
Detailed steps on reproducing this issue are present in [0],
and if the issue reported in that bug gets fixed, you can easily
trigger this error message by deleting the ldlinux.e32 file.
Another issue which might render the error message unreadable is that
the error message doesn't contain a newline marker, which might cause
the EFI shell command prompt to overwrite the message. This happens
with OVMF under Qemu.
Yet another minor issue is that the efi_scroll_up() function generates
a CRLF pair on the reverse order, LFCR, which can confuse some text
editors when viewing the logs, so it would be good to put them into
the correct order.
The efi_scroll_up() function also contains a bug where it ignores
the number of rows to scroll up by and always scrolls once, but this
seems to be intentional, as it's used to mask a different bug in
the ansicon module that causes it to scroll way more than it should
under EFI.
The ansicon module is used by ldlinux, and this masked bug can be seen
if you implement the rows parameter in efi_scroll_up() and try to boot
a kernel with the default boot prompt, the "Loading <xyz>..."
messages
will be spaced apart with a ludicrous amount of spaces. Fixing this
properly is outside of the scope of this task, so for now, I think it's
okay to ignore the "rows" parameter in efi_scroll_up(), as ansicon
appears to be the only place where it's used.
Another related thing to keep in mind is that printf() and friends
don't work in this early mode. That appears to be intentional though,
and it only affects debugging messages. The printf() family tends
to be enabled through openconsole(), and this happens once the ldlinux
module is loaded, but in the bare syslinux.efi environment, the function
is not present (trying to use it gives an undefined symbol error),
though its behavior can thankfully be recreated pretty easily by just
adding the following lines somewhere before you start using debugging
statements with the printf() family of functions:
close(1);
close(2);
opendev(&dev_null_r, &dev_stdcon_w, O_WRONLY);
opendev(&dev_null_r, &dev_stdcon_w, O_WRONLY);
I'm attaching a patch that updates the writechr() implementation
for EFI by including a default color attribute of light gray on black,
which matches what the BIOS implementation of writechr() does.
The patch additionally also updates efi_write_char() to not change
the color attribute unconditionally whenever a character is written,
and fixes the CR LF order in efi_scroll_up().
A newline character is also added at the end of the ldlinux loading
error message. This newline is added in efi_main() and not
in load_env32() to be consistent with how it's implemented under BIOS.
The load_env32() function is shared between EFI and BIOS, and under
BIOS, the newline is also added as part of the following error message
which tells the user that boot failed, e.g. the err_bootfailed string
in core/diskfs.inc.
Only the writechr() update and newline addition is strictly neccessary
to fix the bug this bug report is about, but the other updates make
the screen output process more efficient and improve the usability
of the generated logs with text editors.
With this patch, the error message about ldlinux is dispalyed correctly,
informing the user about a potentially misconfigured setup, e.g.
with the ldlinux.e32 / ldlinux.e64 file not being present.
Best Regards,
Marek
[0] - https://bugs.debian.org/1103692
--- a/efi/console.c
+++ b/efi/console.c
@@ -34,7 +34,7 @@ void efi_console_restore(void)
__export void writechr(char data)
{
- efi_write_char(data, 0);
+ efi_write_char(data, EFI_BACKGROUND_BLACK | EFI_LIGHTGRAY);
}
static inline EFI_STATUS open_protocol(EFI_HANDLE handle, EFI_GUID *protocol,
--- a/efi/main.c
+++ b/efi/main.c
@@ -255,7 +255,10 @@ void efi_write_char(uint8_t ch, uint8_t
SIMPLE_TEXT_OUTPUT_INTERFACE *out = ST->ConOut;
uint16_t c[2];
- uefi_call_wrapper(out->SetAttribute, 2, out, attribute);
+ if (ST->ConOut->Mode->Attribute != attribute)
+ {
+ uefi_call_wrapper(out->SetAttribute, 2, out, attribute);
+ }
/* Lookup primary Unicode encoding in the system codepage */
c[0] = codepage.uni[0][ch];
@@ -281,8 +284,8 @@ static void efi_set_cursor(int x, int y,
static void efi_scroll_up(uint8_t cols, uint8_t rows, uint8_t attribute)
{
- efi_write_char('\n', 0);
- efi_write_char('\r', 0);
+ efi_write_char('\r', attribute);
+ efi_write_char('\n', attribute);
}
static void efi_get_mode(int *cols, int *rows)
@@ -1384,7 +1387,10 @@ EFI_STATUS efi_main(EFI_HANDLE image, EF
} while (status == EFI_SUCCESS);
if (!setjmp(load_error_buf))
+ {
load_env32(NULL);
+ writestr("\r\n");
+ }
/* load_env32() failed.. cancel timer and bailout */
status = cancel_timer(timer_ev);