[PATCH v10 3/4] Boot var automatic management for removable medias
Masahisa Kojima
masahisa.kojima at linaro.org
Wed Sep 20 10:14:19 CEST 2023
Hi Raymond, Heinrich, Ilias,
On Wed, 19 Jul 2023 at 07:11, Raymond Mao <raymond.mao at linaro.org> wrote:
>
> Hi Heinrich,
>
> I checkouted the 'origin/removable_medias' branch from the 'EFI U-Boot Custodian Tree'.
> Run the secureboot test suite on the sandbox locally by './test/py/test.py --bd sandbox --build -k efi_secboot'.
> But there are no errors observed for 'test_signed.py'.
> Below is the output logs:
> ```
> ./test/py/test.py --bd sandbox --build -k efi_secboot
> +make O=/home/raymond/upstream/U-Boot-custodian/u-boot-efi/build-sandbox -s sandbox_defconfig
> +make O=/home/raymond/upstream/U-Boot-custodian/u-boot-efi/build-sandbox -s -j2
> =========================================================================================== test session starts ===========================================================================================
> platform linux -- Python 3.10.9, pytest-6.2.5, py-1.10.0, pluggy-0.13.0
> rootdir: /home/raymond/upstream/U-Boot-custodian/u-boot-efi/test/py, configfile: pytest.ini
> plugins: xdist-2.5.0, forked-1.6.0
> collected 1147 items / 1128 deselected / 19 selected
>
> test/py/tests/test_efi_secboot/test_authvar.py ..... [ 26%]
> test/py/tests/test_efi_secboot/test_signed.py ........ [ 68%]
> test/py/tests/test_efi_secboot/test_signed_intca.py ... [ 84%]
> test/py/tests/test_efi_secboot/test_unsigned.py ... [100%]
>
> ============================================================================= 19 passed, 1128 deselected in 78.28s (0:01:18) ==============================================================================
> ```
> Is it a pipeline issue? I didn't find a button to retrigger the failed jobs. Do I have the permissions?
>
>
> The HEAD of my testing workspace is on:
> ```
> commit c88c134c29934654a696aef61ba63ff2c9a8481e (HEAD -> removable_medias, origin/removable_medias)
> Author: Raymond Mao <raymond.mao at linaro.org>
> Date: Mon Jun 19 14:23:00 2023 -0700
>
> Boot var automatic management for removable medias
> ```
> Same as the one for pipeline: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/16948
I could reproduce the error on efi_secboot tests and find the cause.
The efi_secboot tests expect that BootOrder is not defined.
WIth this "Boot var automatic management for removable medias" patch,
BootOrder is added when the disk is detected.
In my testing, the BootOrder variable has two entries and the test ends up with
an unexpected failure since efibootmgr falls back to BootOrder when
loading from BootNext failed.
===
Loading from BootNext failed, falling back to BootOrder
Loading Boot0000 'mmc 1:1' failed
Loading Boot0001 'host 0:1' failed
efi_bootmgr_load() returned: 14
efi_start_image() returned: 2
===
I think we need to clear the BootOrder variable at the beginning of tests.
I will resend this patch with efi_secboot pytest modification.
Thanks,
Masahisa Kojima
>
> Thanks!
> Regards,
> Raymond
>
> On Tue, 18 Jul 2023 at 11:35, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> On 18.07.23 11:03, Masahisa Kojima 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
>>
>> To get more output describing why tests are skipped or fail use
>>
>> export PYTEST_ADDOPTS="-ra"
>>
>> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/16948
>> shows the errors with the suggested patch.
>>
>> Best regards
>>
>> Heinrich
>>
>> >
>> >
>> >>
>> >> 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
>>
More information about the U-Boot
mailing list