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

[Masahisa Kojima]

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