[PATCH v8 7/8] test: efi: boot: Set up an image suitable for EFI testing

Simon Glass sjg at chromium.org
Mon Oct 28 18:02:25 CET 2024


Hi Heinrich,

On Mon, 28 Oct 2024 at 11:25, Heinrich Schuchardt <xypron.glpk at gmx.de>
wrote:
>
> On 10/28/24 07:08, Heinrich Schuchardt wrote:
> > On 10/22/24 14:00, Simon Glass wrote:
> >> Create a new disk for use with tests, which contains the new 'testapp'
> >> EFI app specifically intended for testing the EFI loader.
> >>
> >> Attach it to the USB device, since most testing is currently done with
> >> mmc.
> >>
> >> Initially this image will be used to test the EFI bootmeth.
> >>
> >> Fix a stale comment in prep_mmc_bootdev() while we are here.
> >>
> >> For now this uses sudo and a compressed fallback file, like all the
> >> other bootstd tests. Once this series is in, the patch which moves
> >> this to use user-space tools will be cleaned up and re-submitted.
> >>
> >> Signed-off-by: Simon Glass <sjg at chromium.org>
> >>
> >> ---
> >> Here is the patch to avoid sudo and CI fallback:
> >>
> >> [1] https://patchwork.ozlabs.org/project/uboot/patch/
> >> 20240802093322.15240-1-richard at nod.at/
> >>
> >> (no changes since v1)
> >>
> >>   arch/sandbox/dts/test.dts           |   2 +-
> >>   test/boot/bootdev.c                 |  18 +++++++++-
> >>   test/boot/bootflow.c                |   2 +-
> >>   test/py/tests/bootstd/flash1.img.xz | Bin 0 -> 5016 bytes
> >>   test/py/tests/test_ut.py            |  52
++++++++++++++++++++++++----
> >>   5 files changed, 65 insertions(+), 9 deletions(-)
> >>   create mode 100644 test/py/tests/bootstd/flash1.img.xz
> >>
> >> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> >> index 9bf44ae3b0b..f40fd5a2b16 100644
> >> --- a/arch/sandbox/dts/test.dts
> >> +++ b/arch/sandbox/dts/test.dts
> >> @@ -1512,7 +1512,7 @@
> >>                   flash-stick at 1 {
> >>                       reg = <1>;
> >>                       compatible = "sandbox,usb-flash";
> >> -                    sandbox,filepath = "testflash1.bin";
> >> +                    sandbox,filepath = "flash1.img";
> >>                   };
> >>
> >>                   flash-stick at 2 {
> >> diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c
> >> index 369105ca4cf..c892854b227 100644
> >> --- a/test/boot/bootdev.c
> >> +++ b/test/boot/bootdev.c
> >> @@ -221,6 +221,10 @@ static int bootdev_test_order(struct
> >> unit_test_state *uts)
> >>       /* Use the environment variable to override it */
> >>       ut_assertok(env_set("boot_targets", "mmc1 mmc2 usb"));
> >>       ut_assertok(bootflow_scan_first(NULL, NULL, &iter, 0, &bflow));
> >> +
> >> +    /* get the usb device which has a backing file (flash1.img) */
> >> +    ut_asserteq(0, bootflow_scan_next(&iter, &bflow));
> >> +
> >>       ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
> >>       ut_asserteq(5, iter.num_devs);
> >>       ut_asserteq_str("mmc1.bootdev", iter.dev_used[0]->name);
> >> @@ -260,7 +264,11 @@ static int bootdev_test_order(struct
> >> unit_test_state *uts)
> >>       ut_assertok(bootflow_scan_first(NULL, NULL, &iter, 0, &bflow));
> >>       ut_asserteq(2, iter.num_devs);
> >>
> >> -    /* Now scan past mmc1 and make sure that the 3 USB devices show
> >> up */
> >> +    /*
> >> +     * Now scan past mmc1 and make sure that the 3 USB devices show
> >> up. The
> >> +     * first one has a backing file so returns success
> >> +     */
> >> +    ut_asserteq(0, bootflow_scan_next(&iter, &bflow));
> >>       ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
> >>       ut_asserteq(6, iter.num_devs);
> >>       ut_asserteq_str("mmc2.bootdev", iter.dev_used[0]->name);
> >> @@ -322,6 +330,10 @@ static int bootdev_test_prio(struct
> >> unit_test_state *uts)
> >>
> >>       /* 3 MMC and 3 USB bootdevs: MMC should come before USB */
> >>       ut_assertok(bootflow_scan_first(NULL, NULL, &iter, 0, &bflow));
> >> +
> >> +    /* get the usb device which has a backing file (flash1.img) */
> >> +    ut_asserteq(0, bootflow_scan_next(&iter, &bflow));
> >> +
> >>       ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
> >>       ut_asserteq(6, iter.num_devs);
> >>       ut_asserteq_str("mmc2.bootdev", iter.dev_used[0]->name);
> >> @@ -339,6 +351,10 @@ static int bootdev_test_prio(struct
> >> unit_test_state *uts)
> >>       bootflow_iter_uninit(&iter);
> >>       ut_assertok(bootflow_scan_first(NULL, NULL, &iter,
BOOTFLOWIF_HUNT,
> >>                       &bflow));
> >> +
> >> +    /* get the usb device which has a backing file (flash1.img) */
> >> +    ut_asserteq(0, bootflow_scan_next(&iter, &bflow));
> >> +
> >>       ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
> >>       ut_asserteq(7, iter.num_devs);
> >>       ut_asserteq_str("usb_mass_storage.lun0.bootdev",
> >> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> >> index 539abe63ebe..d94c07963d5 100644
> >> --- a/test/boot/bootflow.c
> >> +++ b/test/boot/bootflow.c
> >> @@ -533,7 +533,7 @@ static int prep_mmc_bootdev(struct unit_test_state
> >> *uts, const char *mmc_dev,
> >>
> >>       order[2] = mmc_dev;
> >>
> >> -    /* Enable the mmc4 node since we need a second bootflow */
> >> +    /* Enable the requested mmc node since we need a second bootflow
*/
> >>       root = oftree_root(oftree_default());
> >>       node = ofnode_find_subnode(root, mmc_dev);
> >>       ut_assert(ofnode_valid(node));
> >> diff --git a/test/py/tests/bootstd/flash1.img.xz b/test/py/tests/
> >> bootstd/flash1.img.xz
> >> new file mode 100644
> >> index
> >>
0000000000000000000000000000000000000000..9148b91a20c1f0acb54449b124d1408961e92507
> >
> > We should not accept such binaries. This was just how xz was hacked
> > (https://en.wikipedia.org/wiki/XZ_Utils_backdoor).
> >
> > This binary wouldn't conform to the GPL 2.0 license either.

Yes, I've offered to tidy up the patch and drop this binary (and the
others) once this test lands.

[..]

> >> +def setup_efi_image(cons):
> >> +    """Create a 20MB disk image with an EFI app on it"""
> >> +    devnum = 1
> >> +    basename = 'flash'
> >> +    fname, mnt = setup_image(cons, devnum, 0xc, second_part=True,
> >> +                             basename=basename)
> >> +
> >> +    loop = None
> >> +    mounted = False
> >> +    complete = False
> >> +    try:
> >> +        loop = mount_image(cons, fname, mnt, 'ext4')
> >> +        mounted = True
> >> +        efi_dir = os.path.join(mnt, 'EFI')
> >> +        mkdir_cond(efi_dir)
> >> +        bootdir = os.path.join(efi_dir, 'BOOT')
> >> +        mkdir_cond(bootdir)
> >> +        efi_src = os.path.join(cons.config.build_dir,
> >> +                               f'lib/efi_loader/testapp.efi')
> >> +        efi_dst = os.path.join(bootdir, 'BOOTSBOX.EFI')
>
> As said I can live with the -n switch though that standards defying
> bootsbox.efi name provides no benefit whatsoever. But, please, do not
> ignore the switch.

The -N flag is provided for allowing sandbox to use the correct, default
name for the underlying host. It is not intended for use in CI, where
'sandbox is sandbox'. So far, CI calls sandbox without that flag, so the
code above is actually correct.

Regards,
Simon


More information about the U-Boot mailing list