Re: [PATCH] efi_loader: adjust config options for capsule updates

2024-06-30 Thread Ilias Apalodimas
Hi Heinrich,

On Sun, 30 Jun 2024 at 15:23, Heinrich Schuchardt  wrote:
>
> On 6/22/24 18:38, Ilias Apalodimas wrote:
> > On Sat, 22 Jun 2024 at 19:36, Heinrich Schuchardt  
> > wrote:
> >>
> >> On 20.06.24 22:15, Ilias Apalodimas wrote:
> >>> EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable
> >>> at runtime is not supported and allow the platform to perform capsule
> >>> updates on disk. With the recent changes boards can conditionally enable
> >>> setvariable at runtime using EFI_RT_VOLATILE_STORE.
> >>>
> >>> Let's make that visible in our Kconfigs and enable 
> >>> EFI_IGNORE_OSINDICATIONS
> >>> when set variable at runtime is disabled.
> >>>
> >>> Since EFI_RT_VOLATILE_STORE needs help from the OS to persist the
> >>> variables, allow users to ignore OsIndications even if setvariable at
> >>> runtime is enabled.
> >>>
> >>> Signed-off-by: Ilias Apalodimas 
> >>
> >> So this v2:
> >
> > Yes sorry, forgot to add the tile and log...
>
> With this patch I get a failure on the sandbox in the CI:
>
> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/21382

Yes, this test is trying to test updates with OsIndications not set
and it obviously fails, because it expects the update to stop.

I'll send a v3 and adjust the tests.

Cheers
/Ilias
>
> Without the patch the sandbox runs fine:
>
> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/21383
>
> Best regards
>
> Heinrich
>
> >
> >>
> >> v2:
> >>  allow EFI_IGNORE_OSINDICATIONS if EFI_RT_VOLATILE_STORE=y
> >>
> >> Reviewed-by: Heinrich Schuchardt 
> >
> > Thanks Heinrich
> >
> >>
> >>> ---
> >>>lib/efi_loader/Kconfig | 1 +
> >>>1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> >>> index ee71f417147a..6006e845cb1f 100644
> >>> --- a/lib/efi_loader/Kconfig
> >>> +++ b/lib/efi_loader/Kconfig
> >>> @@ -220,6 +220,7 @@ config EFI_CAPSULE_ON_DISK
> >>>config EFI_IGNORE_OSINDICATIONS
> >>>bool "Ignore OsIndications for CapsuleUpdate on-disk"
> >>>depends on EFI_CAPSULE_ON_DISK
> >>> + default y if !EFI_RT_VOLATILE_STORE
> >>>help
> >>>  There are boards where U-Boot does not support SetVariable at 
> >>> runtime.
> >>>  Select this option if you want to use the capsule-on-disk feature
> >>> --
> >>> 2.43.0
> >>>
> >>
>


Re: [PATCH] efi_loader: adjust config options for capsule updates

2024-06-30 Thread Heinrich Schuchardt

On 6/22/24 18:38, Ilias Apalodimas wrote:

On Sat, 22 Jun 2024 at 19:36, Heinrich Schuchardt  wrote:


On 20.06.24 22:15, Ilias Apalodimas wrote:

EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable
at runtime is not supported and allow the platform to perform capsule
updates on disk. With the recent changes boards can conditionally enable
setvariable at runtime using EFI_RT_VOLATILE_STORE.

Let's make that visible in our Kconfigs and enable EFI_IGNORE_OSINDICATIONS
when set variable at runtime is disabled.

Since EFI_RT_VOLATILE_STORE needs help from the OS to persist the
variables, allow users to ignore OsIndications even if setvariable at
runtime is enabled.

Signed-off-by: Ilias Apalodimas 


So this v2:


Yes sorry, forgot to add the tile and log...


With this patch I get a failure on the sandbox in the CI:

https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/21382

Without the patch the sandbox runs fine:

https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/21383

Best regards

Heinrich





v2:
 allow EFI_IGNORE_OSINDICATIONS if EFI_RT_VOLATILE_STORE=y

Reviewed-by: Heinrich Schuchardt 


Thanks Heinrich




---
   lib/efi_loader/Kconfig | 1 +
   1 file changed, 1 insertion(+)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index ee71f417147a..6006e845cb1f 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -220,6 +220,7 @@ config EFI_CAPSULE_ON_DISK
   config EFI_IGNORE_OSINDICATIONS
   bool "Ignore OsIndications for CapsuleUpdate on-disk"
   depends on EFI_CAPSULE_ON_DISK
+ default y if !EFI_RT_VOLATILE_STORE
   help
 There are boards where U-Boot does not support SetVariable at runtime.
 Select this option if you want to use the capsule-on-disk feature
--
2.43.0







Re: [PATCH] efi_loader: adjust config options for capsule updates

2024-06-22 Thread Ilias Apalodimas
On Sat, 22 Jun 2024 at 19:36, Heinrich Schuchardt  wrote:
>
> On 20.06.24 22:15, Ilias Apalodimas wrote:
> > EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable
> > at runtime is not supported and allow the platform to perform capsule
> > updates on disk. With the recent changes boards can conditionally enable
> > setvariable at runtime using EFI_RT_VOLATILE_STORE.
> >
> > Let's make that visible in our Kconfigs and enable EFI_IGNORE_OSINDICATIONS
> > when set variable at runtime is disabled.
> >
> > Since EFI_RT_VOLATILE_STORE needs help from the OS to persist the
> > variables, allow users to ignore OsIndications even if setvariable at
> > runtime is enabled.
> >
> > Signed-off-by: Ilias Apalodimas 
>
> So this v2:

Yes sorry, forgot to add the tile and log...

>
> v2:
> allow EFI_IGNORE_OSINDICATIONS if EFI_RT_VOLATILE_STORE=y
>
> Reviewed-by: Heinrich Schuchardt 

Thanks Heinrich

>
> > ---
> >   lib/efi_loader/Kconfig | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index ee71f417147a..6006e845cb1f 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -220,6 +220,7 @@ config EFI_CAPSULE_ON_DISK
> >   config EFI_IGNORE_OSINDICATIONS
> >   bool "Ignore OsIndications for CapsuleUpdate on-disk"
> >   depends on EFI_CAPSULE_ON_DISK
> > + default y if !EFI_RT_VOLATILE_STORE
> >   help
> > There are boards where U-Boot does not support SetVariable at 
> > runtime.
> > Select this option if you want to use the capsule-on-disk feature
> > --
> > 2.43.0
> >
>


Re: [PATCH] efi_loader: adjust config options for capsule updates

2024-06-22 Thread Heinrich Schuchardt

On 20.06.24 22:15, Ilias Apalodimas wrote:

EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable
at runtime is not supported and allow the platform to perform capsule
updates on disk. With the recent changes boards can conditionally enable
setvariable at runtime using EFI_RT_VOLATILE_STORE.

Let's make that visible in our Kconfigs and enable EFI_IGNORE_OSINDICATIONS
when set variable at runtime is disabled.

Since EFI_RT_VOLATILE_STORE needs help from the OS to persist the
variables, allow users to ignore OsIndications even if setvariable at
runtime is enabled.

Signed-off-by: Ilias Apalodimas 


So this v2:

v2:
allow EFI_IGNORE_OSINDICATIONS if EFI_RT_VOLATILE_STORE=y

Reviewed-by: Heinrich Schuchardt 


---
  lib/efi_loader/Kconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index ee71f417147a..6006e845cb1f 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -220,6 +220,7 @@ config EFI_CAPSULE_ON_DISK
  config EFI_IGNORE_OSINDICATIONS
bool "Ignore OsIndications for CapsuleUpdate on-disk"
depends on EFI_CAPSULE_ON_DISK
+   default y if !EFI_RT_VOLATILE_STORE
help
  There are boards where U-Boot does not support SetVariable at runtime.
  Select this option if you want to use the capsule-on-disk feature
--
2.43.0





Re: [PATCH] efi_loader: adjust config options for capsule updates

2024-06-20 Thread Ilias Apalodimas
Hi Heinrich,

On Thu, 20 Jun 2024 at 18:23, Heinrich Schuchardt  wrote:
>
> On 18.06.24 17:49, Ilias Apalodimas wrote:
> > EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable
> > at runtime is not supported and allow the platform to perform capsule
> > updates on disk. With the recent changes boards can conditionally enable
> > setvariable at runtime using EFI_RT_VOLATILE_STORE.
> >
> > So let's make the options depend on each other and clarify their
> > functionality. When EFI_RT_VOLATILE_STORE, setvariable at runtime is
> > supported and EFI_IGNORE_OSINDICATIONS, which also breaks the EFI spec, is
> > not needed anymore.
> >
> > Signed-off-by: Ilias Apalodimas 
> > ---
> >   lib/efi_loader/Kconfig | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 430bb7f0f7dc..c84064de1366 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -220,6 +220,8 @@ config EFI_CAPSULE_ON_DISK
> >   config EFI_IGNORE_OSINDICATIONS
> >   bool "Ignore OsIndications for CapsuleUpdate on-disk"
> >   depends on EFI_CAPSULE_ON_DISK
> > + depends on !EFI_RT_VOLATILE_STORE
>
> EFI_RT_VOLATILE_STORE does not imply that the OS that you are running
> supports writing variables to ubootefi.var or eMMC.
>
> How about
>
> default y if !EFI_RT_VOLATILE_STORE

Sure, that works for me. I'll send a v2

Cheers

>
> Best regards
>
> Heinrich
>
> > + default y
> >   help
> > There are boards where U-Boot does not support SetVariable at 
> > runtime.
> > Select this option if you want to use the capsule-on-disk feature
>


Re: [PATCH] efi_loader: adjust config options for capsule updates

2024-06-20 Thread Heinrich Schuchardt

On 18.06.24 17:49, Ilias Apalodimas wrote:

EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable
at runtime is not supported and allow the platform to perform capsule
updates on disk. With the recent changes boards can conditionally enable
setvariable at runtime using EFI_RT_VOLATILE_STORE.

So let's make the options depend on each other and clarify their
functionality. When EFI_RT_VOLATILE_STORE, setvariable at runtime is
supported and EFI_IGNORE_OSINDICATIONS, which also breaks the EFI spec, is
not needed anymore.

Signed-off-by: Ilias Apalodimas 
---
  lib/efi_loader/Kconfig | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 430bb7f0f7dc..c84064de1366 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -220,6 +220,8 @@ config EFI_CAPSULE_ON_DISK
  config EFI_IGNORE_OSINDICATIONS
bool "Ignore OsIndications for CapsuleUpdate on-disk"
depends on EFI_CAPSULE_ON_DISK
+   depends on !EFI_RT_VOLATILE_STORE


EFI_RT_VOLATILE_STORE does not imply that the OS that you are running
supports writing variables to ubootefi.var or eMMC.

How about

default y if !EFI_RT_VOLATILE_STORE

Best regards

Heinrich


+   default y
help
  There are boards where U-Boot does not support SetVariable at runtime.
  Select this option if you want to use the capsule-on-disk feature




Re: [PATCH] efi_loader: adjust config options for capsule updates

2024-06-18 Thread Jon Humphreys
Ilias Apalodimas  writes:

> Hi Jon,
>
> On Tue, 18 Jun 2024 at 19:49, Jon Humphreys  wrote:
>>
>> Ilias Apalodimas  writes:
>>
>> > EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable
>> > at runtime is not supported and allow the platform to perform capsule
>> > updates on disk. With the recent changes boards can conditionally enable
>> > setvariable at runtime using EFI_RT_VOLATILE_STORE.
>> >
>> > So let's make the options depend on each other and clarify their
>> > functionality. When EFI_RT_VOLATILE_STORE, setvariable at runtime is
>> > supported and EFI_IGNORE_OSINDICATIONS, which also breaks the EFI spec, is
>> > not needed anymore.
>>
>> Hi Ilias,
>>
>> Is there a corresponding effort to update fwupdmgr to set OSIndications
>> (and set it correctly so the change persists a reboot)?
>>
>
> We are not trying to fix anything atm, we probably will in the future.
>
>> Otherwise, fwupdmgr provided capsules will now get ignored for boards that
>> enable setvariable at runtime.
>
> That goes beyond fwupd, EFI variable changes will be ignored in
> general unless you sync the file manually -- e.g. dd
> if=/sys/firmware/efi/efivars/VarToFile-b2ac5fc9-92b7-4acd-aeac-11e818c3130c
> of=/boot/efi/ubootefi.var skip=4 bs=1
> after every variable change. Unless you do that, enabling
> EFI_RT_VOLATILE_STORE makes little sense (and that's why it's not
> enabled by default).
> So, I prefer making EFI_IGNORE_OSINDICATIONS, which was a hack,
> mutually exclusive and start going in the right direction.
>

I agree with the principle.  My concern is that we want to encourage people
to move to setvariable at runtime, but until LVFS is updated, people who
care about LVFS will be deincentivized.

Jon

> Regards
> /Ilias
>>
>> Jon
>>
>> >
>> > Signed-off-by: Ilias Apalodimas 
>> > ---
>> >  lib/efi_loader/Kconfig | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>> > index 430bb7f0f7dc..c84064de1366 100644
>> > --- a/lib/efi_loader/Kconfig
>> > +++ b/lib/efi_loader/Kconfig
>> > @@ -220,6 +220,8 @@ config EFI_CAPSULE_ON_DISK
>> >  config EFI_IGNORE_OSINDICATIONS
>> >   bool "Ignore OsIndications for CapsuleUpdate on-disk"
>> >   depends on EFI_CAPSULE_ON_DISK
>> > + depends on !EFI_RT_VOLATILE_STORE
>> > + default y
>> >   help
>> > There are boards where U-Boot does not support SetVariable at 
>> > runtime.
>> > Select this option if you want to use the capsule-on-disk feature
>> > --
>> > 2.45.2


Re: [PATCH] efi_loader: adjust config options for capsule updates

2024-06-18 Thread Jon Humphreys
Ilias Apalodimas  writes:

> EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable
> at runtime is not supported and allow the platform to perform capsule
> updates on disk. With the recent changes boards can conditionally enable
> setvariable at runtime using EFI_RT_VOLATILE_STORE.
>
> So let's make the options depend on each other and clarify their
> functionality. When EFI_RT_VOLATILE_STORE, setvariable at runtime is
> supported and EFI_IGNORE_OSINDICATIONS, which also breaks the EFI spec, is
> not needed anymore.

Hi Ilias,

Is there a corresponding effort to update fwupdmgr to set OSIndications
(and set it correctly so the change persists a reboot)?

Otherwise, fwupdmgr provided capsules will now get ignored for boards that
enable setvariable at runtime.

Jon

>
> Signed-off-by: Ilias Apalodimas 
> ---
>  lib/efi_loader/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 430bb7f0f7dc..c84064de1366 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -220,6 +220,8 @@ config EFI_CAPSULE_ON_DISK
>  config EFI_IGNORE_OSINDICATIONS
>   bool "Ignore OsIndications for CapsuleUpdate on-disk"
>   depends on EFI_CAPSULE_ON_DISK
> + depends on !EFI_RT_VOLATILE_STORE
> + default y
>   help
> There are boards where U-Boot does not support SetVariable at runtime.
> Select this option if you want to use the capsule-on-disk feature
> -- 
> 2.45.2


Re: [PATCH] efi_loader: adjust config options for capsule updates

2024-06-18 Thread Ilias Apalodimas
Hi Jon,

On Tue, 18 Jun 2024 at 19:49, Jon Humphreys  wrote:
>
> Ilias Apalodimas  writes:
>
> > EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable
> > at runtime is not supported and allow the platform to perform capsule
> > updates on disk. With the recent changes boards can conditionally enable
> > setvariable at runtime using EFI_RT_VOLATILE_STORE.
> >
> > So let's make the options depend on each other and clarify their
> > functionality. When EFI_RT_VOLATILE_STORE, setvariable at runtime is
> > supported and EFI_IGNORE_OSINDICATIONS, which also breaks the EFI spec, is
> > not needed anymore.
>
> Hi Ilias,
>
> Is there a corresponding effort to update fwupdmgr to set OSIndications
> (and set it correctly so the change persists a reboot)?
>

We are not trying to fix anything atm, we probably will in the future.

> Otherwise, fwupdmgr provided capsules will now get ignored for boards that
> enable setvariable at runtime.

That goes beyond fwupd, EFI variable changes will be ignored in
general unless you sync the file manually -- e.g. dd
if=/sys/firmware/efi/efivars/VarToFile-b2ac5fc9-92b7-4acd-aeac-11e818c3130c
of=/boot/efi/ubootefi.var skip=4 bs=1
after every variable change. Unless you do that, enabling
EFI_RT_VOLATILE_STORE makes little sense (and that's why it's not
enabled by default).
So, I prefer making EFI_IGNORE_OSINDICATIONS, which was a hack,
mutually exclusive and start going in the right direction.

Regards
/Ilias
>
> Jon
>
> >
> > Signed-off-by: Ilias Apalodimas 
> > ---
> >  lib/efi_loader/Kconfig | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 430bb7f0f7dc..c84064de1366 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -220,6 +220,8 @@ config EFI_CAPSULE_ON_DISK
> >  config EFI_IGNORE_OSINDICATIONS
> >   bool "Ignore OsIndications for CapsuleUpdate on-disk"
> >   depends on EFI_CAPSULE_ON_DISK
> > + depends on !EFI_RT_VOLATILE_STORE
> > + default y
> >   help
> > There are boards where U-Boot does not support SetVariable at 
> > runtime.
> > Select this option if you want to use the capsule-on-disk feature
> > --
> > 2.45.2