[PATCH v10 3/4] Boot var automatic management for removable medias

[Raymond Mao]

Hi Masahisa,

Thanks for the hints. Found that I was missing 'guestfs-tools' for
'virt-make-fs'.
Now I am able to reproduce the failure logs that Heinrich pointed out.

Thanks!
Regards,
Raymond

On Tue, 18 Jul 2023 at 05:03, Masahisa Kojima <masahisa.kojima at linaro.org>
wrote:

> Hi Raymond,
>
> On Tue, 18 Jul 2023 at 05:23, Raymond Mao <raymond.mao at linaro.org> wrote:
> >
> > Hi Heinrich,
> >
> > I run 'make tests' with the patches locally but no errors observed from
> 'test_signed.py', below are few lines from the output console:
> > ```
> > test/py/tests/test_efi_secboot/test_signed.py ssssssss
>
>                                                 [      90%]
> > ```
>
> I think the test cases are just skipped, not executed.
>
> I guess that some required packages are missing on the host machine,
> such as sbsigntool, efitools and libguestfs-tools.
> build-sandbox/test-log.html will help to analyze why the tests are skipped.
>
> The following document describes prerequisites to run python tests.
> https://u-boot.readthedocs.io/en/latest/develop/py_testing.html
>
> 'make tests' runs many test cases and takes time.
> './test/py/test.py --bd sandbox --build -k efi_secboot' will run only
> the efi_secboot test
> and it will be useful for debugging purposes.
>
> Thanks,
> Masahisa Kojima
>
>
> >
> > Regards,
> > Raymond
> >
> > On Sat, 15 Jul 2023 at 05:19, Heinrich Schuchardt <xypron.glpk at gmx.de>
> wrote:
> >>
> >> On 7/9/23 10:56, Heinrich Schuchardt wrote:
> >> > On 6/19/23 23:23, Raymond Mao wrote:
> >> >> Changes for complying to EFI spec §3.5.1.1
> >> >> 'Removable Media Boot Behavior'.
> >> >> Boot variables can be automatically generated during a removable
> >> >> media is probed. At the same time, unused boot variables will be
> >> >> detected and removed.
> >> >>
> >> >> Please note that currently the function 'efi_disk_remove' has no
> >> >> ability to distinguish below two scenarios
> >> >> a) Unplugging of a removable media under U-Boot
> >> >> b) U-Boot exiting and booting an OS
> >> >> Thus currently the boot variables management is not added into
> >> >> 'efi_disk_remove' to avoid boot options being added/erased
> >> >> repeatedly under scenario b) during power cycles
> >> >> See TODO comments under function 'efi_disk_remove' for more details
> >> >>
> >> >> Signed-off-by: Raymond Mao <raymond.mao at linaro.org>
> >> >> ---
> >> >> Changes in v3
> >> >> - Split the patch into moving and renaming functions and
> >> >>    individual patches for each changed functionality
> >> >> Changes in v5
> >> >> - Move function call of efi_bootmgr_update_media_device_boot_option()
> >> >>    from efi_init_variables() to efi_init_obj_list()
> >> >> Changes in v6
> >> >> - Revert unrelated changes
> >> >> Changes in v7
> >> >> - adapt the return code of function
> >> >>    efi_bootmgr_update_media_device_boot_option()
> >> >> Changes in v8
> >> >> - add a note in the commit message for future reference
> >> >> Changes in v9
> >> >> - amend the note text in the commit message
> >> >> - add a TODO comment in 'efi_disk_remove'
> >> >> Changes in v10
> >> >> - fix typo and build failures with 'CONFIG_CMD_BOOTEFI_BOOTMGR=n'
> >> >>
> >> >>   lib/efi_loader/efi_disk.c  | 18 ++++++++++++++++++
> >> >>   lib/efi_loader/efi_setup.c |  7 +++++++
> >> >>   2 files changed, 25 insertions(+)
> >> >>
> >> >> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> >> >> index d2256713a8..911a4adfb1 100644
> >> >> --- a/lib/efi_loader/efi_disk.c
> >> >> +++ b/lib/efi_loader/efi_disk.c
> >> >> @@ -687,6 +687,13 @@ int efi_disk_probe(void *ctx, struct event
> *event)
> >> >>               return -1;
> >> >>       }
> >> >> +    /* only do the boot option management when UEFI sub-system is
> >> >> initialized */
> >> >> +    if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
> >> >> efi_obj_list_initialized == EFI_SUCCESS) {
> >> >> +        ret = efi_bootmgr_update_media_device_boot_option();
> >> >> +        if (ret != EFI_SUCCESS)
> >> >> +            return -1;
> >> >> +    }
> >> >> +
> >> >>       return 0;
> >> >>   }
> >> >> @@ -773,6 +780,17 @@ int efi_disk_remove(void *ctx, struct event
> *event)
> >> >>           return efi_disk_delete_part(dev);
> >> >>       else
> >> >>           return 0;
> >> >> +
> >> >> +    /*
> >> >> +     * TODO A flag to distinguish below 2 different scenarios of
> this
> >> >> +     * function call is needed:
> >> >> +     * a) Unplugging of a removable media under U-Boot
> >> >> +     * b) U-Boot exiting and booting an OS
> >> >> +     * In case of scenario a),
> >> >> efi_bootmgr_update_media_device_boot_option()
> >> >> +     * needs to be invoked here to update the boot options and
> remove
> >> >> the
> >> >> +     * unnecessary ones.
> >> >> +     */
> >> >
> >> > As in future we want to integrate more EFI drivers with the driver
> model
> >> > such a status should be in a global variable. It will have to be set
> in
> >> > ExitBootServices() before invoking dm_remove_devices_flags().
> >> >
> >> > Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> >> >
> >> >> +
> >> >>   }
> >> >>   /**
> >> >> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> >> >> index 58d4e13402..69c8b27730 100644
> >> >> --- a/lib/efi_loader/efi_setup.c
> >> >> +++ b/lib/efi_loader/efi_setup.c
> >> >> @@ -245,6 +245,13 @@ efi_status_t efi_init_obj_list(void)
> >> >>       if (ret != EFI_SUCCESS)
> >> >>           goto out;
> >> >> +    if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> >> >> +        /* update boot option after variable service initialized */
> >> >> +        ret = efi_bootmgr_update_media_device_boot_option();
> >> >> +        if (ret != EFI_SUCCESS)
> >> >> +            goto out;
> >> >> +    }
> >> >> +
> >> >>       /* Define supported languages */
> >> >>       ret = efi_init_platform_lang();
> >> >>       if (ret != EFI_SUCCESS)
> >> >
> >>
> >> This patch leads to a test failure in
> >>
> >>
> test/py/tests/test_efi_secboot/test_signed.py::TestEfiSignedImage::test_efi_signed_image_auth2
> >>
> >> No EFI system partition
> >> Failed to persist EFI variables
> >>
> >> Please, run 'make tests' before resubmitting.
> >>
> >> Best regards
> >>
> >> Heinrich
>