* [PATCH] efi_loader: Allow capsule update on-disk without checking OsIndications
@ 2021-06-29 4:55 Ilias Apalodimas
2021-06-29 7:43 ` Heinrich Schuchardt
0 siblings, 1 reply; 7+ messages in thread
From: Ilias Apalodimas @ 2021-06-29 4:55 UTC (permalink / raw)
To: xypron.glpk
Cc: masami.hiramatsu, takahiro.akashi, pbrobinson, richard, apalos,
Alexander Graf, u-boot
From: apalos <ilias.apalodimas@linaro.org>
Although U-Boot supports capsule update on-disk, it's lack of support for
SetVariable at runtime prevents applications like fwupd from using it.
In order to perform the capsule update on-disk the spec says that the OS
must copy the capsule to the \EFI\UpdateCapsule directory and set a bit in
the OsIndications variable. The firmware then checks for the
EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED bit in OsIndications
variable, which is set by submitter to trigger processing of the capsule
on next reboot.
Let's add a config option which ignores the bit checking in OsIndications
and just rely on the capsule being present. Since U-Boot deletes the
capsule while processing it, we won't end up running it multiple times.
Note that this is allowed for all capsules. In the future once,
authenticated capsules is fully supported, we can limit the functionality
to those only.
Signed-off-by: apalos <ilias.apalodimas@linaro.org>
---
lib/efi_loader/Kconfig | 9 +++++++++
lib/efi_loader/efi_capsule.c | 36 ++++++++++++++++++++++++++++--------
2 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 684adfb62379..5a3820e76122 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -137,6 +137,15 @@ config EFI_CAPSULE_ON_DISK
under a specific directory on UEFI system partition instead of
via UpdateCapsule API.
+config EFI_IGNORE_OSINDICATIONS
+ bool "Ignore OsIndications for CapsuleUpdate on-disk"
+ depends on EFI_CAPSULE_ON_DISK
+ default n
+ help
+ There are boards were we can't support SetVariable at runtime.
+ Select this option if you want to use capsule-on-disk feature,
+ without setting the OsIndications bit.
+
config EFI_CAPSULE_ON_DISK_EARLY
bool "Initiate capsule-on-disk at U-Boot boottime"
depends on EFI_CAPSULE_ON_DISK
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index d7136035d8f9..50bed32bfb3b 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -948,6 +948,33 @@ efi_status_t __weak efi_load_capsule_drivers(void)
return ret;
}
+/**
+ * check_run_capsules - Check whether capsule update should run
+ *
+ * The spec says OsIndications must be set in order to run the capsule update
+ * on-disk. Since U-Boot doesn't support runtime SetVariable, allow capsules to
+ * run explicitly if CONFIG_EFI_IGNORE_OSINDICATIONS is selected
+ */
+static bool check_run_capsules(void)
+{
+ u64 os_indications;
+ efi_uintn_t size;
+ efi_status_t ret;
+
+ if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS))
+ return true;
+
+ size = sizeof(os_indications);
+ ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
+ NULL, &size, &os_indications, NULL);
+ if (ret == EFI_SUCCESS &&
+ (os_indications
+ & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
+ return true;
+
+ return false;
+}
+
/**
* efi_launch_capsule - launch capsules
*
@@ -958,20 +985,13 @@ efi_status_t __weak efi_load_capsule_drivers(void)
*/
efi_status_t efi_launch_capsules(void)
{
- u64 os_indications;
- efi_uintn_t size;
struct efi_capsule_header *capsule = NULL;
u16 **files;
unsigned int nfiles, index, i;
u16 variable_name16[12];
efi_status_t ret;
- size = sizeof(os_indications);
- ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
- NULL, &size, &os_indications, NULL);
- if (ret != EFI_SUCCESS ||
- !(os_indications
- & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
+ if (!check_run_capsules())
return EFI_SUCCESS;
index = get_last_capsule();
--
2.32.0.rc0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] efi_loader: Allow capsule update on-disk without checking OsIndications
2021-06-29 4:55 [PATCH] efi_loader: Allow capsule update on-disk without checking OsIndications Ilias Apalodimas
@ 2021-06-29 7:43 ` Heinrich Schuchardt
2021-06-29 7:56 ` Ilias Apalodimas
2021-06-29 12:41 ` AKASHI Takahiro
0 siblings, 2 replies; 7+ messages in thread
From: Heinrich Schuchardt @ 2021-06-29 7:43 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: masami.hiramatsu, takahiro.akashi, pbrobinson, richard,
Alexander Graf, u-boot
On 6/29/21 6:55 AM, Ilias Apalodimas wrote:
> From: apalos <ilias.apalodimas@linaro.org>
>
> Although U-Boot supports capsule update on-disk, it's lack of support for
> SetVariable at runtime prevents applications like fwupd from using it.
>
> In order to perform the capsule update on-disk the spec says that the OS
> must copy the capsule to the \EFI\UpdateCapsule directory and set a bit in
> the OsIndications variable. The firmware then checks for the
> EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED bit in OsIndications
> variable, which is set by submitter to trigger processing of the capsule
> on next reboot.
>
> Let's add a config option which ignores the bit checking in OsIndications
> and just rely on the capsule being present. Since U-Boot deletes the
> capsule while processing it, we won't end up running it multiple times.
>
> Note that this is allowed for all capsules. In the future once,
> authenticated capsules is fully supported, we can limit the functionality
> to those only.
>
> Signed-off-by: apalos <ilias.apalodimas@linaro.org>
> ---
> lib/efi_loader/Kconfig | 9 +++++++++
> lib/efi_loader/efi_capsule.c | 36 ++++++++++++++++++++++++++++--------
> 2 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 684adfb62379..5a3820e76122 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -137,6 +137,15 @@ config EFI_CAPSULE_ON_DISK
> under a specific directory on UEFI system partition instead of
> via UpdateCapsule API.
>
> +config EFI_IGNORE_OSINDICATIONS
> + bool "Ignore OsIndications for CapsuleUpdate on-disk"
> + depends on EFI_CAPSULE_ON_DISK
> + default n
> + help
> + There are boards were we can't support SetVariable at runtime.
> + Select this option if you want to use capsule-on-disk feature,
> + without setting the OsIndications bit.
> +
> config EFI_CAPSULE_ON_DISK_EARLY
> bool "Initiate capsule-on-disk at U-Boot boottime"
> depends on EFI_CAPSULE_ON_DISK
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index d7136035d8f9..50bed32bfb3b 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -948,6 +948,33 @@ efi_status_t __weak efi_load_capsule_drivers(void)
> return ret;
> }
>
> +/**
> + * check_run_capsules - Check whether capsule update should run
> + *
> + * The spec says OsIndications must be set in order to run the capsule update
> + * on-disk. Since U-Boot doesn't support runtime SetVariable, allow capsules to
> + * run explicitly if CONFIG_EFI_IGNORE_OSINDICATIONS is selected
> + */
> +static bool check_run_capsules(void)
> +{
> + u64 os_indications;
> + efi_uintn_t size;
> + efi_status_t ret;
> +
> + if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS))
> + return true;
> +
> + size = sizeof(os_indications);
> + ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
> + NULL, &size, &os_indications, NULL);
> + if (ret == EFI_SUCCESS &&
> + (os_indications
> + & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
The bit EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED must be
cleared in OsIndications after successfully applying the capsule. See
related patch
[PATCH 1/1] efi_loader: fix set_capsule_result()
https://lists.denx.de/pipermail/u-boot/2021-June/453111.html
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> + return true;
> +
> + return false;
> +}
> +
> /**
> * efi_launch_capsule - launch capsules
> *
> @@ -958,20 +985,13 @@ efi_status_t __weak efi_load_capsule_drivers(void)
> */
> efi_status_t efi_launch_capsules(void)
> {
> - u64 os_indications;
> - efi_uintn_t size;
> struct efi_capsule_header *capsule = NULL;
> u16 **files;
> unsigned int nfiles, index, i;
> u16 variable_name16[12];
> efi_status_t ret;
>
> - size = sizeof(os_indications);
> - ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
> - NULL, &size, &os_indications, NULL);
> - if (ret != EFI_SUCCESS ||
> - !(os_indications
> - & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
> + if (!check_run_capsules())
> return EFI_SUCCESS;
>
> index = get_last_capsule();
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi_loader: Allow capsule update on-disk without checking OsIndications
2021-06-29 7:43 ` Heinrich Schuchardt
@ 2021-06-29 7:56 ` Ilias Apalodimas
2021-06-29 12:41 ` AKASHI Takahiro
1 sibling, 0 replies; 7+ messages in thread
From: Ilias Apalodimas @ 2021-06-29 7:56 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: masami.hiramatsu, takahiro.akashi, pbrobinson, richard,
Alexander Graf, u-boot
> > + if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS))
[...]
> > + return true;
> > +
> > + size = sizeof(os_indications);
> > + ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
> > + NULL, &size, &os_indications, NULL);
> > + if (ret == EFI_SUCCESS &&
> > + (os_indications
> > + & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
>
> The bit EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED must be
> cleared in OsIndications after successfully applying the capsule. See
> related patch
Yea I noticed that as well while I was coding this and meant to sent a patch
afterwards. You've already done that,so let me go have a look!
>
> [PATCH 1/1] efi_loader: fix set_capsule_result()
> https://lists.denx.de/pipermail/u-boot/2021-June/453111.html
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> > + return true;
> > +
> > + return false;
> > +}
> > +
> > /**
> > * efi_launch_capsule - launch capsules
> > *
> > @@ -958,20 +985,13 @@ efi_status_t __weak efi_load_capsule_drivers(void)
> > */
> > efi_status_t efi_launch_capsules(void)
> > {
> > - u64 os_indications;
> > - efi_uintn_t size;
> > struct efi_capsule_header *capsule = NULL;
> > u16 **files;
> > unsigned int nfiles, index, i;
> > u16 variable_name16[12];
> > efi_status_t ret;
> >
> > - size = sizeof(os_indications);
> > - ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
> > - NULL, &size, &os_indications, NULL);
> > - if (ret != EFI_SUCCESS ||
> > - !(os_indications
> > - & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
> > + if (!check_run_capsules())
> > return EFI_SUCCESS;
> >
> > index = get_last_capsule();
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi_loader: Allow capsule update on-disk without checking OsIndications
2021-06-29 7:43 ` Heinrich Schuchardt
2021-06-29 7:56 ` Ilias Apalodimas
@ 2021-06-29 12:41 ` AKASHI Takahiro
2021-06-29 12:45 ` Ilias Apalodimas
1 sibling, 1 reply; 7+ messages in thread
From: AKASHI Takahiro @ 2021-06-29 12:41 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Ilias Apalodimas, masami.hiramatsu, pbrobinson, richard,
Alexander Graf, u-boot
On Tue, Jun 29, 2021 at 09:43:40AM +0200, Heinrich Schuchardt wrote:
> On 6/29/21 6:55 AM, Ilias Apalodimas wrote:
> > From: apalos <ilias.apalodimas@linaro.org>
> >
> > Although U-Boot supports capsule update on-disk, it's lack of support for
> > SetVariable at runtime prevents applications like fwupd from using it.
> >
> > In order to perform the capsule update on-disk the spec says that the OS
> > must copy the capsule to the \EFI\UpdateCapsule directory and set a bit in
> > the OsIndications variable. The firmware then checks for the
> > EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED bit in OsIndications
> > variable, which is set by submitter to trigger processing of the capsule
> > on next reboot.
> >
> > Let's add a config option which ignores the bit checking in OsIndications
> > and just rely on the capsule being present. Since U-Boot deletes the
> > capsule while processing it, we won't end up running it multiple times.
> >
> > Note that this is allowed for all capsules. In the future once,
> > authenticated capsules is fully supported, we can limit the functionality
> > to those only.
> >
> > Signed-off-by: apalos <ilias.apalodimas@linaro.org>
> > ---
> > lib/efi_loader/Kconfig | 9 +++++++++
> > lib/efi_loader/efi_capsule.c | 36 ++++++++++++++++++++++++++++--------
> > 2 files changed, 37 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 684adfb62379..5a3820e76122 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -137,6 +137,15 @@ config EFI_CAPSULE_ON_DISK
> > under a specific directory on UEFI system partition instead of
> > via UpdateCapsule API.
> >
> > +config EFI_IGNORE_OSINDICATIONS
> > + bool "Ignore OsIndications for CapsuleUpdate on-disk"
> > + depends on EFI_CAPSULE_ON_DISK
> > + default n
> > + help
> > + There are boards were we can't support SetVariable at runtime.
> > + Select this option if you want to use capsule-on-disk feature,
> > + without setting the OsIndications bit.
Obviously, this option breaks the conformance to UEFI specification
and must be turned on carefully in the limited use cases.
You should describe that here explicitly.
Additionally, you may add
depends on !EFI_MM_COMM_TEE (or better config?)
-Takahiro Akashi
> > config EFI_CAPSULE_ON_DISK_EARLY
> > bool "Initiate capsule-on-disk at U-Boot boottime"
> > depends on EFI_CAPSULE_ON_DISK
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index d7136035d8f9..50bed32bfb3b 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -948,6 +948,33 @@ efi_status_t __weak efi_load_capsule_drivers(void)
> > return ret;
> > }
> >
> > +/**
> > + * check_run_capsules - Check whether capsule update should run
> > + *
> > + * The spec says OsIndications must be set in order to run the capsule update
> > + * on-disk. Since U-Boot doesn't support runtime SetVariable, allow capsules to
> > + * run explicitly if CONFIG_EFI_IGNORE_OSINDICATIONS is selected
> > + */
> > +static bool check_run_capsules(void)
> > +{
> > + u64 os_indications;
> > + efi_uintn_t size;
> > + efi_status_t ret;
> > +
> > + if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS))
> > + return true;
> > +
> > + size = sizeof(os_indications);
> > + ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
> > + NULL, &size, &os_indications, NULL);
> > + if (ret == EFI_SUCCESS &&
> > + (os_indications
> > + & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
>
> The bit EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED must be
> cleared in OsIndications after successfully applying the capsule. See
> related patch
>
> [PATCH 1/1] efi_loader: fix set_capsule_result()
> https://lists.denx.de/pipermail/u-boot/2021-June/453111.html
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> > + return true;
> > +
> > + return false;
> > +}
> > +
> > /**
> > * efi_launch_capsule - launch capsules
> > *
> > @@ -958,20 +985,13 @@ efi_status_t __weak efi_load_capsule_drivers(void)
> > */
> > efi_status_t efi_launch_capsules(void)
> > {
> > - u64 os_indications;
> > - efi_uintn_t size;
> > struct efi_capsule_header *capsule = NULL;
> > u16 **files;
> > unsigned int nfiles, index, i;
> > u16 variable_name16[12];
> > efi_status_t ret;
> >
> > - size = sizeof(os_indications);
> > - ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
> > - NULL, &size, &os_indications, NULL);
> > - if (ret != EFI_SUCCESS ||
> > - !(os_indications
> > - & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
> > + if (!check_run_capsules())
> > return EFI_SUCCESS;
> >
> > index = get_last_capsule();
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi_loader: Allow capsule update on-disk without checking OsIndications
2021-06-29 12:41 ` AKASHI Takahiro
@ 2021-06-29 12:45 ` Ilias Apalodimas
2021-06-29 13:04 ` AKASHI Takahiro
0 siblings, 1 reply; 7+ messages in thread
From: Ilias Apalodimas @ 2021-06-29 12:45 UTC (permalink / raw)
To: AKASHI Takahiro, Heinrich Schuchardt, masami.hiramatsu,
pbrobinson, richard, Alexander Graf, u-boot
> > >
[...]
> > > +config EFI_IGNORE_OSINDICATIONS
> > > + bool "Ignore OsIndications for CapsuleUpdate on-disk"
> > > + depends on EFI_CAPSULE_ON_DISK
> > > + default n
> > > + help
> > > + There are boards were we can't support SetVariable at runtime.
> > > + Select this option if you want to use capsule-on-disk feature,
> > > + without setting the OsIndications bit.
>
> Obviously, this option breaks the conformance to UEFI specification
> and must be turned on carefully in the limited use cases.
> You should describe that here explicitly.
Fair enough, I'll send a v2
>
> Additionally, you may add
> depends on !EFI_MM_COMM_TEE (or better config?)
That's not the case (yet). I do have a kernel branch were SetVariable at
runtime is supported, but that's not merged yet. So until this gets accepted
*all* the boards that want to perform a capsuleupdate on-disk without setting
the variable need this.
Cheers
/Ilias
>
> -Takahiro Akashi
>
>
> > > config EFI_CAPSULE_ON_DISK_EARLY
> > > bool "Initiate capsule-on-disk at U-Boot boottime"
> > > depends on EFI_CAPSULE_ON_DISK
> > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > index d7136035d8f9..50bed32bfb3b 100644
> > > --- a/lib/efi_loader/efi_capsule.c
> > > +++ b/lib/efi_loader/efi_capsule.c
> > > @@ -948,6 +948,33 @@ efi_status_t __weak efi_load_capsule_drivers(void)
> > > return ret;
> > > }
> > >
> > > +/**
> > > + * check_run_capsules - Check whether capsule update should run
> > > + *
> > > + * The spec says OsIndications must be set in order to run the capsule update
> > > + * on-disk. Since U-Boot doesn't support runtime SetVariable, allow capsules to
> > > + * run explicitly if CONFIG_EFI_IGNORE_OSINDICATIONS is selected
> > > + */
> > > +static bool check_run_capsules(void)
> > > +{
> > > + u64 os_indications;
> > > + efi_uintn_t size;
> > > + efi_status_t ret;
> > > +
> > > + if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS))
> > > + return true;
> > > +
> > > + size = sizeof(os_indications);
> > > + ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
> > > + NULL, &size, &os_indications, NULL);
> > > + if (ret == EFI_SUCCESS &&
> > > + (os_indications
> > > + & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
> >
> > The bit EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED must be
> > cleared in OsIndications after successfully applying the capsule. See
> > related patch
> >
> > [PATCH 1/1] efi_loader: fix set_capsule_result()
> > https://lists.denx.de/pipermail/u-boot/2021-June/453111.html
> >
> > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >
> > > + return true;
> > > +
> > > + return false;
> > > +}
> > > +
> > > /**
> > > * efi_launch_capsule - launch capsules
> > > *
> > > @@ -958,20 +985,13 @@ efi_status_t __weak efi_load_capsule_drivers(void)
> > > */
> > > efi_status_t efi_launch_capsules(void)
> > > {
> > > - u64 os_indications;
> > > - efi_uintn_t size;
> > > struct efi_capsule_header *capsule = NULL;
> > > u16 **files;
> > > unsigned int nfiles, index, i;
> > > u16 variable_name16[12];
> > > efi_status_t ret;
> > >
> > > - size = sizeof(os_indications);
> > > - ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
> > > - NULL, &size, &os_indications, NULL);
> > > - if (ret != EFI_SUCCESS ||
> > > - !(os_indications
> > > - & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
> > > + if (!check_run_capsules())
> > > return EFI_SUCCESS;
> > >
> > > index = get_last_capsule();
> > >
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi_loader: Allow capsule update on-disk without checking OsIndications
2021-06-29 12:45 ` Ilias Apalodimas
@ 2021-06-29 13:04 ` AKASHI Takahiro
2021-06-29 13:40 ` Ilias Apalodimas
0 siblings, 1 reply; 7+ messages in thread
From: AKASHI Takahiro @ 2021-06-29 13:04 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: Heinrich Schuchardt, masami.hiramatsu, pbrobinson, richard,
Alexander Graf, u-boot
On Tue, Jun 29, 2021 at 03:45:29PM +0300, Ilias Apalodimas wrote:
> > > >
> [...]
> > > > +config EFI_IGNORE_OSINDICATIONS
> > > > + bool "Ignore OsIndications for CapsuleUpdate on-disk"
> > > > + depends on EFI_CAPSULE_ON_DISK
> > > > + default n
> > > > + help
> > > > + There are boards were we can't support SetVariable at runtime.
> > > > + Select this option if you want to use capsule-on-disk feature,
> > > > + without setting the OsIndications bit.
> >
> > Obviously, this option breaks the conformance to UEFI specification
> > and must be turned on carefully in the limited use cases.
> > You should describe that here explicitly.
>
> Fair enough, I'll send a v2
>
> >
> > Additionally, you may add
> > depends on !EFI_MM_COMM_TEE (or better config?)
>
> That's not the case (yet). I do have a kernel branch were SetVariable at
> runtime is supported, but that's not merged yet. So until this gets accepted
> *all* the boards that want to perform a capsuleupdate on-disk without setting
> the variable need this.
If so, I would suggest that another Kconfig option be added
for the availability of SetVariable at runtime.
This will allow us to keep "depends on" unchanged even if
yet another implementation of SetVariable is introduced in the future.
-Takahiro Akashi
> Cheers
> /Ilias
> >
> > -Takahiro Akashi
> >
> >
> > > > config EFI_CAPSULE_ON_DISK_EARLY
> > > > bool "Initiate capsule-on-disk at U-Boot boottime"
> > > > depends on EFI_CAPSULE_ON_DISK
> > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > > index d7136035d8f9..50bed32bfb3b 100644
> > > > --- a/lib/efi_loader/efi_capsule.c
> > > > +++ b/lib/efi_loader/efi_capsule.c
> > > > @@ -948,6 +948,33 @@ efi_status_t __weak efi_load_capsule_drivers(void)
> > > > return ret;
> > > > }
> > > >
> > > > +/**
> > > > + * check_run_capsules - Check whether capsule update should run
> > > > + *
> > > > + * The spec says OsIndications must be set in order to run the capsule update
> > > > + * on-disk. Since U-Boot doesn't support runtime SetVariable, allow capsules to
> > > > + * run explicitly if CONFIG_EFI_IGNORE_OSINDICATIONS is selected
> > > > + */
> > > > +static bool check_run_capsules(void)
> > > > +{
> > > > + u64 os_indications;
> > > > + efi_uintn_t size;
> > > > + efi_status_t ret;
> > > > +
> > > > + if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS))
> > > > + return true;
> > > > +
> > > > + size = sizeof(os_indications);
> > > > + ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
> > > > + NULL, &size, &os_indications, NULL);
> > > > + if (ret == EFI_SUCCESS &&
> > > > + (os_indications
> > > > + & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
> > >
> > > The bit EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED must be
> > > cleared in OsIndications after successfully applying the capsule. See
> > > related patch
> > >
> > > [PATCH 1/1] efi_loader: fix set_capsule_result()
> > > https://lists.denx.de/pipermail/u-boot/2021-June/453111.html
> > >
> > > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > >
> > > > + return true;
> > > > +
> > > > + return false;
> > > > +}
> > > > +
> > > > /**
> > > > * efi_launch_capsule - launch capsules
> > > > *
> > > > @@ -958,20 +985,13 @@ efi_status_t __weak efi_load_capsule_drivers(void)
> > > > */
> > > > efi_status_t efi_launch_capsules(void)
> > > > {
> > > > - u64 os_indications;
> > > > - efi_uintn_t size;
> > > > struct efi_capsule_header *capsule = NULL;
> > > > u16 **files;
> > > > unsigned int nfiles, index, i;
> > > > u16 variable_name16[12];
> > > > efi_status_t ret;
> > > >
> > > > - size = sizeof(os_indications);
> > > > - ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
> > > > - NULL, &size, &os_indications, NULL);
> > > > - if (ret != EFI_SUCCESS ||
> > > > - !(os_indications
> > > > - & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
> > > > + if (!check_run_capsules())
> > > > return EFI_SUCCESS;
> > > >
> > > > index = get_last_capsule();
> > > >
> > >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi_loader: Allow capsule update on-disk without checking OsIndications
2021-06-29 13:04 ` AKASHI Takahiro
@ 2021-06-29 13:40 ` Ilias Apalodimas
0 siblings, 0 replies; 7+ messages in thread
From: Ilias Apalodimas @ 2021-06-29 13:40 UTC (permalink / raw)
To: AKASHI Takahiro, Ilias Apalodimas, Heinrich Schuchardt,
Masami Hiramatsu, Peter Robinson, Richard Hughes, Alexander Graf,
U-Boot Mailing List
On Tue, 29 Jun 2021 at 16:04, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> On Tue, Jun 29, 2021 at 03:45:29PM +0300, Ilias Apalodimas wrote:
> > > > >
> > [...]
> > > > > +config EFI_IGNORE_OSINDICATIONS
> > > > > + bool "Ignore OsIndications for CapsuleUpdate on-disk"
> > > > > + depends on EFI_CAPSULE_ON_DISK
> > > > > + default n
> > > > > + help
> > > > > + There are boards were we can't support SetVariable at runtime.
> > > > > + Select this option if you want to use capsule-on-disk feature,
> > > > > + without setting the OsIndications bit.
> > >
> > > Obviously, this option breaks the conformance to UEFI specification
> > > and must be turned on carefully in the limited use cases.
> > > You should describe that here explicitly.
> >
> > Fair enough, I'll send a v2
> >
> > >
> > > Additionally, you may add
> > > depends on !EFI_MM_COMM_TEE (or better config?)
> >
> > That's not the case (yet). I do have a kernel branch were SetVariable at
> > runtime is supported, but that's not merged yet. So until this gets accepted
> > *all* the boards that want to perform a capsuleupdate on-disk without setting
> > the variable need this.
>
> If so, I would suggest that another Kconfig option be added
> for the availability of SetVariable at runtime.
>
> This will allow us to keep "depends on" unchanged even if
> yet another implementation of SetVariable is introduced in the future.
Let's not fill in so many Kconfig options for now. We might need to
do that in the future, but on the other hand, I would much prefer
checking the RTPROP which has that info.
>
> -Takahiro Akashi
>
>
> > Cheers
> > /Ilias
> > >
> > > -Takahiro Akashi
> > >
> > >
> > > > > config EFI_CAPSULE_ON_DISK_EARLY
> > > > > bool "Initiate capsule-on-disk at U-Boot boottime"
> > > > > depends on EFI_CAPSULE_ON_DISK
> > > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > > > index d7136035d8f9..50bed32bfb3b 100644
> > > > > --- a/lib/efi_loader/efi_capsule.c
> > > > > +++ b/lib/efi_loader/efi_capsule.c
> > > > > @@ -948,6 +948,33 @@ efi_status_t __weak efi_load_capsule_drivers(void)
> > > > > return ret;
> > > > > }
> > > > >
> > > > > +/**
> > > > > + * check_run_capsules - Check whether capsule update should run
> > > > > + *
> > > > > + * The spec says OsIndications must be set in order to run the capsule update
> > > > > + * on-disk. Since U-Boot doesn't support runtime SetVariable, allow capsules to
> > > > > + * run explicitly if CONFIG_EFI_IGNORE_OSINDICATIONS is selected
> > > > > + */
> > > > > +static bool check_run_capsules(void)
> > > > > +{
> > > > > + u64 os_indications;
> > > > > + efi_uintn_t size;
> > > > > + efi_status_t ret;
> > > > > +
> > > > > + if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS))
> > > > > + return true;
> > > > > +
> > > > > + size = sizeof(os_indications);
> > > > > + ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
> > > > > + NULL, &size, &os_indications, NULL);
> > > > > + if (ret == EFI_SUCCESS &&
> > > > > + (os_indications
> > > > > + & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
> > > >
> > > > The bit EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED must be
> > > > cleared in OsIndications after successfully applying the capsule. See
> > > > related patch
> > > >
> > > > [PATCH 1/1] efi_loader: fix set_capsule_result()
> > > > https://lists.denx.de/pipermail/u-boot/2021-June/453111.html
> > > >
> > > > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > >
> > > > > + return true;
> > > > > +
> > > > > + return false;
> > > > > +}
> > > > > +
> > > > > /**
> > > > > * efi_launch_capsule - launch capsules
> > > > > *
> > > > > @@ -958,20 +985,13 @@ efi_status_t __weak efi_load_capsule_drivers(void)
> > > > > */
> > > > > efi_status_t efi_launch_capsules(void)
> > > > > {
> > > > > - u64 os_indications;
> > > > > - efi_uintn_t size;
> > > > > struct efi_capsule_header *capsule = NULL;
> > > > > u16 **files;
> > > > > unsigned int nfiles, index, i;
> > > > > u16 variable_name16[12];
> > > > > efi_status_t ret;
> > > > >
> > > > > - size = sizeof(os_indications);
> > > > > - ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
> > > > > - NULL, &size, &os_indications, NULL);
> > > > > - if (ret != EFI_SUCCESS ||
> > > > > - !(os_indications
> > > > > - & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
> > > > > + if (!check_run_capsules())
> > > > > return EFI_SUCCESS;
> > > > >
> > > > > index = get_last_capsule();
> > > > >
> > > >
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-06-29 13:41 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-29 4:55 [PATCH] efi_loader: Allow capsule update on-disk without checking OsIndications Ilias Apalodimas
2021-06-29 7:43 ` Heinrich Schuchardt
2021-06-29 7:56 ` Ilias Apalodimas
2021-06-29 12:41 ` AKASHI Takahiro
2021-06-29 12:45 ` Ilias Apalodimas
2021-06-29 13:04 ` AKASHI Takahiro
2021-06-29 13:40 ` Ilias Apalodimas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.