[PATCH v6 12/12] test: efi: boot: Add a test for the efi bootmeth

[Heinrich Schuchardt]

Am 14. Oktober 2024 05:51:22 MESZ schrieb Tom Rini <trini at konsulko.com>:
>On Sun, Oct 13, 2024 at 01:33:23PM -0600, Simon Glass wrote:
>> Hi Tom,
>> 
>> On Fri, 11 Oct 2024 at 16:32, Tom Rini <trini at konsulko.com> wrote:
>> >
>> > On Fri, Sep 27, 2024 at 12:02:23AM +0200, Simon Glass wrote:
>> >
>> > > Add a simple test of booting with the EFI bootmeth, which runs the app
>> > > and checks that it can call 'exit boot-services' (to check that all the
>> > > device-removal code doesn't break anything) and then exit back to
>> > > U-Boot.
>> > >
>> > > This uses a disk image containing the testapp, ready for execution by
>> > > sandbox when needed.
>> > >
>> > > Signed-off-by: Simon Glass <sjg at chromium.org>
>> >
>> > So lets go to the problem point. What asserts here fail due to ANSI
>> > escape codes being in the output and needing to be filtered out?
>> >
>> > [snip]
>> > > diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
>> > > index e05103188b4..da2ee1ab345 100644
>> > > --- a/test/boot/bootflow.c
>> > > +++ b/test/boot/bootflow.c
>> > > @@ -13,6 +13,7 @@
>> > >  #include <cli.h>
>> > >  #include <dm.h>
>> > >  #include <efi_default_filename.h>
>> > > +#include <efi_loader.h>
>> > >  #include <expo.h>
>> > >  #ifdef CONFIG_SANDBOX
>> > >  #include <asm/test.h>
>> > > @@ -31,6 +32,9 @@ extern U_BOOT_DRIVER(bootmeth_android);
>> > >  extern U_BOOT_DRIVER(bootmeth_cros);
>> > >  extern U_BOOT_DRIVER(bootmeth_2script);
>> > >
>> > > +/* Use this as the vendor for EFI to tell the app to exit boot services */
>> > > +static u16 __efi_runtime_data test_vendor[] = u"U-Boot testing";
>> > > +
>> > >  static int inject_response(struct unit_test_state *uts)
>> > >  {
>> > >       /*
>> > > @@ -1205,3 +1209,62 @@ static int bootflow_android(struct unit_test_state *uts)
>> > >       return 0;
>> > >  }
>> > >  BOOTSTD_TEST(bootflow_android, UTF_CONSOLE);
>> > > +
>> > > +/* Test EFI bootmeth */
>> > > +static int bootflow_efi(struct unit_test_state *uts)
>> > > +{
>> > > +     /* disable ethernet since the hunter will run dhcp */
>> > > +     test_set_eth_enable(false);
>> > > +
>> > > +     /* make USB scan without delays */
>> > > +     test_set_skip_delays(true);
>> > > +
>> > > +     bootstd_reset_usb();
>> > > +
>> > > +     /* Avoid outputting ANSI characters which mess with our asserts */
>> > > +     efi_console_set_ansi(false);
>> > > +
>> > > +     ut_assertok(bootstd_test_drop_bootdev_order(uts));
>> > > +     ut_assertok(run_command("bootflow scan", 0));
>> > > +     ut_assert_skip_to_line(
>> > > +             "Bus usb at 1: scanning bus usb at 1 for devices... 5 USB Device(s) found");
>> > > +
>> > > +     ut_assertok(run_command("bootflow list", 0));
>> > > +
>> > > +     ut_assert_nextlinen("Showing all");
>> > > +     ut_assert_nextlinen("Seq");
>> > > +     ut_assert_nextlinen("---");
>> > > +     ut_assert_nextlinen("  0  extlinux");
>> > > +     ut_assert_nextlinen(
>> > > +             "  1  efi          ready   usb_mass_    1  usb_mass_storage.lun0.boo /EFI/BOOT/BOOTSBOX.EFI");
>> > > +     ut_assert_nextlinen("---");
>> > > +     ut_assert_skip_to_line("(2 bootflows, 2 valid)");
>> > > +     ut_assert_console_end();
>> > > +
>> > > +     ut_assertok(run_command("bootflow select 1", 0));
>> > > +     ut_assert_console_end();
>> > > +
>> > > +     systab.fw_vendor = test_vendor;
>> > > +
>> > > +     ut_asserteq(1, run_command("bootflow boot", 0));
>> > > +     ut_assert_nextline(
>> > > +             "** Booting bootflow 'usb_mass_storage.lun0.bootdev.part_1' with efi");
>> > > +     if (IS_ENABLED(CONFIG_LOGF_FUNC))
>> > > +             ut_assert_skip_to_line("       efi_run_image() Booting /\\EFI\\BOOT\\BOOTSBOX.EFI");
>> > > +     else
>> > > +             ut_assert_skip_to_line("Booting /\\EFI\\BOOT\\BOOTSBOX.EFI");
>> > > +
>> > > +     /* TODO: Why the \r ? */
>> > > +     ut_assert_nextline("U-Boot test app for EFI_LOADER\r");
>> > > +     ut_assert_nextline("Exiting boot sevices");
>> > > +     if (IS_ENABLED(CONFIG_LOGF_FUNC))
>> > > +             ut_assert_nextline("     do_bootefi_exec() ## Application failed, r = 5");
>> > > +     else
>> > > +             ut_assert_nextline("## Application failed, r = 5");
>> > > +     ut_assert_nextline("Boot failed (err=-22)");
>> > > +
>> > > +     ut_assert_console_end();
>> > > +
>> > > +     return 0;
>> > > +}
>> > > +BOOTSTD_TEST(bootflow_efi, UTF_CONSOLE);
>> 
>> Thanks for taking an interest in this.
>> 
>> Nothing at present, as I added ut_assert_skip_to_line() to skip it
>> all. But it really is just wrong...I get gibberish showing up in the
>> terminal after I run tests! This is what I see with this series:
>> 
>> ** Booting bootflow 'usb_mass_storage.lun0.bootdev.part_1' with efi
>>  7 [r [999;999H [6n 8No EFI system partition
>> 
>> ^^^ strange output there but I'm not sure if it will come through on email

It seems you are not providing a video console. So the EFI sub-system is asking the serial console for its size in query_console_serial().

Set CONFIG_VIDEO=y and let vidconsole be your first output device to avoid query_console_serial() being called.

Best regards

Heinrich

>> 
>> No EFI system partition
>> Failed to persist EFI variables
>> No EFI system partition
>> Failed to persist EFI variables
>> No EFI system partition
>> Failed to persist EFI variables
>> Booting /\EFI\BOOT\BOOTSBOX.EFI
>> U-Boot test app for EFI_LOADER
>> 
>> Exiting boot sevices
>> ## Application failed, r = 5
>> Boot failed (err=-22)
>> Failures: 0
>
>So we're finally making progress I think to see what the problem you're
>trying to solve is. I think the next thing to figure out is if you use
>picocom or screen or whatever on console, the codes are handled
>correctly, or no? Specifically, picocom? Next, is this perhaps just
>another strange artifact of how oddly (and as you've noted before,
>slowly) pytest consumes the console input? It feels like maybe we're
>doing several layers of wrong there? Because to be clear, this is not a
>sandbox problem. Looking back at the log I had posted about the watchdog
>reset not counting banners correctly I see that same escape sequence,
>now that I know what I'm looking for/at.
>
>All of that said, if it's not a pytest-consuming-the-console problem
>(and it's not a picocom problem, like I was just asking about since the
>board failure in question was via labgrid-client), maybe it's just a
>thing to note about making sure tests understand. And perhaps document
>somewhere why it matters (and I'm not saying it doesn't!) that we know
>what the dimensions of the console are, for EFI, and so that's why we
>init them when/where we do.
>