| Message ID | 20231121113557.800353-5-sjg@chromium.org |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | efi: Partial attempt at a test for EFI bootmeth | expand |
On 11/21/23 12:35, Simon Glass wrote: > We don't want ANSI characters written in tests since it is a pain to > check the output. I never experienced such pain. The Gitlab output just renders things as expected. > > For now, don't bother checking the console size. > > A better solution is need, which allows ANSI output to be controlled > within EFI. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > include/efi_loader.h | 14 ++++++++++++-- > lib/efi_loader/efi_console.c | 17 +++++++---------- > lib/efi_loader/efi_setup.c | 2 +- > 3 files changed, 20 insertions(+), 13 deletions(-) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index 247be060e1c0..ebe951922791 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -527,8 +527,18 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void); > efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index); > /* search the boot option index in BootOrder */ > bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index); > -/* Set up console modes */ > -void efi_setup_console_size(void); > + > +/** > + * efi_setup_console_size() - update the mode table. > + * > + * @allow_ansi: Allow emitting ANSI characters > + * > + * By default the only mode available is 80x25. If the console has at least 50 > + * lines, enable mode 80x50. If we can query the console size and it is neither > + * 80x25 nor 80x50, set it as an additional mode. > + */ > +void efi_setup_console_size(bool allow_ansi); > + > /* Install device tree */ > efi_status_t efi_install_fdt(void *fdt); > /* Run loaded UEFI image */ > diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c > index 28087582e8d6..ea1608f8e6ec 100644 > --- a/lib/efi_loader/efi_console.c > +++ b/lib/efi_loader/efi_console.c > @@ -349,22 +349,19 @@ static int __maybe_unused query_vidconsole(int *rows, int *cols) > return 0; > } > > -/** > - * efi_setup_console_size() - update the mode table. > - * > - * By default the only mode available is 80x25. If the console has at least 50 > - * lines, enable mode 80x50. If we can query the console size and it is neither > - * 80x25 nor 80x50, set it as an additional mode. > - */ > -void efi_setup_console_size(void) > +void efi_setup_console_size(bool allow_ansi) > { > int rows = 25, cols = 80; > int ret = -ENODEV; > > if (IS_ENABLED(CONFIG_VIDEO)) > ret = query_vidconsole(&rows, &cols); > - if (ret) > - ret = query_console_serial(&rows, &cols); > + if (ret) { > + if (allow_ansi) > + ret = query_console_serial(&rows, &cols); > + else > + ret = 0; > + } > if (ret) > return; > > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c > index e6de685e8795..d4226a3011ac 100644 > --- a/lib/efi_loader/efi_setup.c > +++ b/lib/efi_loader/efi_setup.c > @@ -230,7 +230,7 @@ efi_status_t efi_init_obj_list(void) > return efi_obj_list_initialized; > > /* Set up console modes */ > - efi_setup_console_size(); > + efi_setup_console_size(false); This would render our console non-compliant. Please, keep it as is. Testing is not the primary task of U-Boot. If you need to filter test output, please, add a filter method to your test setup. Best regards Heinrich > > /* > * Probe block devices to find the ESP.
Hi Heinrich, On Tue, 21 Nov 2023 at 10:32, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 11/21/23 12:35, Simon Glass wrote: > > We don't want ANSI characters written in tests since it is a pain to > > check the output. > > I never experienced such pain. The Gitlab output just renders things as > expected. > > > > > For now, don't bother checking the console size. > > > > A better solution is need, which allows ANSI output to be controlled > > within EFI. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > include/efi_loader.h | 14 ++++++++++++-- > > lib/efi_loader/efi_console.c | 17 +++++++---------- > > lib/efi_loader/efi_setup.c | 2 +- > > 3 files changed, 20 insertions(+), 13 deletions(-) > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > index 247be060e1c0..ebe951922791 100644 > > --- a/include/efi_loader.h > > +++ b/include/efi_loader.h > > @@ -527,8 +527,18 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void); > > efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index); > > /* search the boot option index in BootOrder */ > > bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index); > > -/* Set up console modes */ > > -void efi_setup_console_size(void); > > + > > +/** > > + * efi_setup_console_size() - update the mode table. > > + * > > + * @allow_ansi: Allow emitting ANSI characters > > + * > > + * By default the only mode available is 80x25. If the console has at least 50 > > + * lines, enable mode 80x50. If we can query the console size and it is neither > > + * 80x25 nor 80x50, set it as an additional mode. > > + */ > > +void efi_setup_console_size(bool allow_ansi); > > + > > /* Install device tree */ > > efi_status_t efi_install_fdt(void *fdt); > > /* Run loaded UEFI image */ > > diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c > > index 28087582e8d6..ea1608f8e6ec 100644 > > --- a/lib/efi_loader/efi_console.c > > +++ b/lib/efi_loader/efi_console.c > > @@ -349,22 +349,19 @@ static int __maybe_unused query_vidconsole(int *rows, int *cols) > > return 0; > > } > > > > -/** > > - * efi_setup_console_size() - update the mode table. > > - * > > - * By default the only mode available is 80x25. If the console has at least 50 > > - * lines, enable mode 80x50. If we can query the console size and it is neither > > - * 80x25 nor 80x50, set it as an additional mode. > > - */ > > -void efi_setup_console_size(void) > > +void efi_setup_console_size(bool allow_ansi) > > { > > int rows = 25, cols = 80; > > int ret = -ENODEV; > > > > if (IS_ENABLED(CONFIG_VIDEO)) > > ret = query_vidconsole(&rows, &cols); > > - if (ret) > > - ret = query_console_serial(&rows, &cols); > > + if (ret) { > > + if (allow_ansi) > > + ret = query_console_serial(&rows, &cols); > > + else > > + ret = 0; > > + } > > if (ret) > > return; > > > > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c > > index e6de685e8795..d4226a3011ac 100644 > > --- a/lib/efi_loader/efi_setup.c > > +++ b/lib/efi_loader/efi_setup.c > > @@ -230,7 +230,7 @@ efi_status_t efi_init_obj_list(void) > > return efi_obj_list_initialized; > > > > /* Set up console modes */ > > - efi_setup_console_size(); > > + efi_setup_console_size(false); > > This would render our console non-compliant. Please, keep it as is. > > Testing is not the primary task of U-Boot. If you need to filter test > output, please, add a filter method to your test setup. This is just a WIP patch, but we do need a way to avoid outputting ANSI output since ut_assert_nextline() needs that. Can you suggest a better way? Perhaps we could have some EFI settings somewhere? Regards, Simon
diff --git a/include/efi_loader.h b/include/efi_loader.h index 247be060e1c0..ebe951922791 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -527,8 +527,18 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void); efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index); /* search the boot option index in BootOrder */ bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index); -/* Set up console modes */ -void efi_setup_console_size(void); + +/** + * efi_setup_console_size() - update the mode table. + * + * @allow_ansi: Allow emitting ANSI characters + * + * By default the only mode available is 80x25. If the console has at least 50 + * lines, enable mode 80x50. If we can query the console size and it is neither + * 80x25 nor 80x50, set it as an additional mode. + */ +void efi_setup_console_size(bool allow_ansi); + /* Install device tree */ efi_status_t efi_install_fdt(void *fdt); /* Run loaded UEFI image */ diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c index 28087582e8d6..ea1608f8e6ec 100644 --- a/lib/efi_loader/efi_console.c +++ b/lib/efi_loader/efi_console.c @@ -349,22 +349,19 @@ static int __maybe_unused query_vidconsole(int *rows, int *cols) return 0; } -/** - * efi_setup_console_size() - update the mode table. - * - * By default the only mode available is 80x25. If the console has at least 50 - * lines, enable mode 80x50. If we can query the console size and it is neither - * 80x25 nor 80x50, set it as an additional mode. - */ -void efi_setup_console_size(void) +void efi_setup_console_size(bool allow_ansi) { int rows = 25, cols = 80; int ret = -ENODEV; if (IS_ENABLED(CONFIG_VIDEO)) ret = query_vidconsole(&rows, &cols); - if (ret) - ret = query_console_serial(&rows, &cols); + if (ret) { + if (allow_ansi) + ret = query_console_serial(&rows, &cols); + else + ret = 0; + } if (ret) return; diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index e6de685e8795..d4226a3011ac 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -230,7 +230,7 @@ efi_status_t efi_init_obj_list(void) return efi_obj_list_initialized; /* Set up console modes */ - efi_setup_console_size(); + efi_setup_console_size(false); /* * Probe block devices to find the ESP.
We don't want ANSI characters written in tests since it is a pain to check the output. For now, don't bother checking the console size. A better solution is need, which allows ANSI output to be controlled within EFI. Signed-off-by: Simon Glass <sjg@chromium.org> --- include/efi_loader.h | 14 ++++++++++++-- lib/efi_loader/efi_console.c | 17 +++++++---------- lib/efi_loader/efi_setup.c | 2 +- 3 files changed, 20 insertions(+), 13 deletions(-)