Re: [PATCH v11] Boot var automatic management for removable medias
Hi Ilias,
On Wed, 27 Sept 2023 at 03:52, Ilias Apalodimas
wrote:
>
> On Wed, 20 Sept 2023 at 11:41, Masahisa Kojima
> wrote:
> >
> > From: Raymond Mao
> >
> > 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
> >
> > The original efi_secboot tests expect that BootOrder EFI variable
> > is not defined. With this commit, the BootOrder EFI variable is
> > automatically added when the disk is detected. The original efi_secboot
> > tests end up with unexpected failure.
> > The efi_secboot tests need to be modified to clear the BootOrder
> > EFI variable at the beginning of each test.
> >
> > Signed-off-by: Raymond Mao
> > Signed-off-by: Masahisa Kojima
> > Reviewed-by: Heinrich Schuchardt
> > ---
> > lib/efi_loader/efi_disk.c | 18 ++
> > lib/efi_loader/efi_setup.c | 7 +++
> > test/py/tests/test_efi_secboot/test_signed.py | 9 +
> > .../test_efi_secboot/test_signed_intca.py | 3 +++
> > .../py/tests/test_efi_secboot/test_unsigned.py | 3 +++
> > 5 files changed, 40 insertions(+)
> >
> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > index f0d76113b0..b808a7fe62 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -690,6 +690,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;
> > }
> >
> > @@ -742,6 +749,17 @@ int efi_disk_remove(void *ctx, struct event *event)
> > dev_tag_del(dev, DM_TAG_EFI);
> >
> > 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.
> > +*/
> > +
> > }
> >
> > /**
> > 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)
> > diff --git a/test/py/tests/test_efi_secboot/test_signed.py
> > b/test/py/tests/test_efi_secboot/test_signed.py
> > index ca52e853d8..b77b60e223 100644
> > --- a/test/py/tests/test_efi_secboot/test_signed.py
> > +++ b/test/py/tests/test_efi_secboot/test_signed.py
> > @@ -28,6 +28,7 @@ class TestEfiSignedImage(object):
> > # Test Case 1a, run signed image if no PK
> > output = u_boot_console.run_command_list([
> > 'host bind 0 %s' % disk_img,
> > +'setenv -e -nv -bs -rt BootOrder',
> > 'efidebug boot add -b 1 HELLO1 host 0:1
> > /helloworld.efi.signed -s ""',
> > 'efidebug boot next 1',
> > 'bootefi bootmgr'])
> > @@ -52,6 +53,7 @@ class TestEfiSignedImage(object):
> > # Test Case 2a, db is not yet installed
> > output = u_boot_console.run_command_list([
> > 'host bind 0 %s' % disk_img,
> > +'setenv -e -nv -bs -rt BootOrder',
> > 'fatload host 0:1 400 KEK.auth',
> > 'setenv -e -nv -bs -rt -at -i
Re: [PATCH v11] Boot var automatic management for removable medias
On Wed, 20 Sept 2023 at 11:41, Masahisa Kojima
wrote:
>
> From: Raymond Mao
>
> 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
>
> The original efi_secboot tests expect that BootOrder EFI variable
> is not defined. With this commit, the BootOrder EFI variable is
> automatically added when the disk is detected. The original efi_secboot
> tests end up with unexpected failure.
> The efi_secboot tests need to be modified to clear the BootOrder
> EFI variable at the beginning of each test.
>
> Signed-off-by: Raymond Mao
> Signed-off-by: Masahisa Kojima
> Reviewed-by: Heinrich Schuchardt
> ---
> lib/efi_loader/efi_disk.c | 18 ++
> lib/efi_loader/efi_setup.c | 7 +++
> test/py/tests/test_efi_secboot/test_signed.py | 9 +
> .../test_efi_secboot/test_signed_intca.py | 3 +++
> .../py/tests/test_efi_secboot/test_unsigned.py | 3 +++
> 5 files changed, 40 insertions(+)
>
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index f0d76113b0..b808a7fe62 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -690,6 +690,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;
> }
>
> @@ -742,6 +749,17 @@ int efi_disk_remove(void *ctx, struct event *event)
> dev_tag_del(dev, DM_TAG_EFI);
>
> 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.
> +*/
> +
> }
>
> /**
> 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)
> diff --git a/test/py/tests/test_efi_secboot/test_signed.py
> b/test/py/tests/test_efi_secboot/test_signed.py
> index ca52e853d8..b77b60e223 100644
> --- a/test/py/tests/test_efi_secboot/test_signed.py
> +++ b/test/py/tests/test_efi_secboot/test_signed.py
> @@ -28,6 +28,7 @@ class TestEfiSignedImage(object):
> # Test Case 1a, run signed image if no PK
> output = u_boot_console.run_command_list([
> 'host bind 0 %s' % disk_img,
> +'setenv -e -nv -bs -rt BootOrder',
> 'efidebug boot add -b 1 HELLO1 host 0:1
> /helloworld.efi.signed -s ""',
> 'efidebug boot next 1',
> 'bootefi bootmgr'])
> @@ -52,6 +53,7 @@ class TestEfiSignedImage(object):
> # Test Case 2a, db is not yet installed
> output = u_boot_console.run_command_list([
> 'host bind 0 %s' % disk_img,
> +'setenv -e -nv -bs -rt BootOrder',
> 'fatload host 0:1 400 KEK.auth',
> 'setenv -e -nv -bs -rt -at -i 400:$filesize KEK',
> 'fatload host 0:1 400 PK.auth',
> @@ -96,6 +98,7 @@ class TestEfiSignedImage(object):
> # Test Case 3a, rejected by dbx
> output = u_boot_console.run_command_list([
> 'host bind 0 %s' % disk_img,
> +
Re: [PATCH v11] Boot var automatic management for removable medias
Hi Masahisa, Thanks for spending time on this test failure! I am not sure why this cannot be reproduced in my end, but I totally agree with the rationale you analyzed - my previous patch will just add boot options whenever a media is probed. Thanks for adding the changes into the pytest. Regards, Raymond
