[U-Boot] efi_loader: implementing non-volatile UEFI variables

[AKASHI Takahiro]

Wolfgang,

On Tue, Jun 25, 2019 at 11:11:18AM +0200, Wolfgang Denk wrote:
> Dear Takahiro,
> 
> In message <20190625075931.GP6610 at linaro.org> you wrote:
> >
> > > It does not have to be ugly, and I think it is also not so
> > > complicatred.  In any case it seems more attractive to me than
> > > adding a completly separate, new implementation for variable
> > > storage.
> >
> > Really? Take an example:
> > U-Boot (non-volatile) variable a: value: A
> > U-Boot (non-volatile) variable b: value: B
> > UEFI non-volatile variable c: value: C
> >
> > Then,
> > 1) user changed b's value to B', but didn't want to save it
> > 2) user changed c's value to C'
> >
> > How should U-Boot behave here?
> 
> You did not complete the scenario, but I guess you mean that the
> user _wants_ to save 'c' to persistent storage?

That is the behavior that UEFI specification requires.

> Yes, the current implementation does not allow this.
> 
> But if you extend the existing environment variable flags by some
> new properties you can have all this easily:
> 
> a) add a "volatile" flag.
> 
>    In the above setup you should then mark 'b' as volatile, so it
>    does not get saved to the environment.
> 
> b) add a "UEFI" flag (*) and extend "env save" and "env export"
>    to respect such flags (ideally both for a "include only these"
>    and an "exclude these" operation).
> 
>    Then you can do a _lot_ of new things.
> 
>    You can for example save _only_ the UEFI variable to persistent
>    storage - which would implement the feature you requested before
>    where you said you would like to set CONFIG_ENV_IS_NOWHERE.
> 
>    Alternatively, you could use "env save" to only save the "normal"
>    U-Boot environment, and "env export" to extract only the UEFI
>    variables and then use a file write to store these elsewhere.
> 
>    You can even go a step further and define separate storage areas
>    so "env save" will do this automatically for you (more cosing
>    effort, easier to use).
> 
>    (*) For a start and just to fulfil your requirement, such a
>    simple "UEFI" flag would be sufficient.  If you think further,
>    you could as well implement a flag that takes a string value,
>    so you can define any number of different variable contexts that
>    can all be managed using the existing code, but kept strictly
>    separate else.  There is a zillion of potential use cases.
> 
> > 3)
> > - hold B as well as B' and marks b as "modified, but not saved"
> > - search the entire U-Boot environment, create a temporary buffer
> >   of {a=A, b=B, c=C'} and save it to backing storage
> > If user does "env save" later,
> > - discard B (and save {a=A, b=B', c=C'})
> >
> > I said this is ugly and complicated.
> 
> What you describe in 3) is indeed ugly, but it is also not
> necessary.  There are other, more intelligent ways to implement
> that.  See above for a proposal.
> 
> > > Contexts, or flags.  But this does not require much refactoring.  It
> > > should be a straighforward extension.
> >
> > Contexts != flags, I mean, by Context, a separate "env_t" data
> > with interfaces like env_save(&env_efi)/env_load(&env_efi).
> > I don't think that it is a "different implementation."
> 
> See above.  If you think small (and go for minimal coding efforts),
> just add a UEFI flag now to provide a UEFI context.  If tomorrow
> someone needs a FOO context, add a FOO flag.  We did not really need
> such context based variable handlint for the last 2 decades, so we
> can probably go on like this for a couple of years.

In the decades, there came out no new FOO context other than UEFI,
so it is unlikely that you will see yet another context for next
couple of years.

> If you think bit and are willing to pay a little more efforts for a
> more generic, runtime extensible solution, than add a context flag
> that takes a string as value, and you can define as many contexts as
> you have memory for.  And you can use them all through the same
> existing tools, and all this is independent of the storage of the
> persistent copies.  and yes, of course you can have separate storage
> for each of the "context" groups.
> 
> > I don't know why you stick to a "single" storage, but if you don't
> > see the implementation above(3) as ugly, that's fine. I can take it
> > since it's is doable anyway.
> 
> I do not stick to a single storage, on contrary.

It is good news to me. I thought that a "single" storage
was the point.

Depending on the nature of "context," we may want to have
different storage devices for different contexts.

> And yes, 3) _is_
> ugly.  But there should be better ways to implement this.
> 
> My concern is to widen the view and not only think of UEFI needs
> here, but to make such feature generally useful for others, too.
> And at the same time minimize the amount oif new code we need and
> keeping it agnostic of the storage media used.

So overall, I basically agree with your proposal, but still need
some more thinkings.

* two new terms
  - interface (I prefer this over context now): uboot or uefi for now
  - variable attribute: volatile(/non-volatile) (and autosave if appropriate)
    some attributes may be interface-specific and expandable in future
* extend existing env-related interfaces
  - accept one or two additional parameters, interface and/or attribute
* (not sure yet) extend hash functions(hsearch_r ...)
  - to allow arbitrary type of key instead of a string of name
  - to allow arbitrary type of value instead of a printable string
  (assuming GUID for UEFI variable)
* add CONFIG_ENV_xxx's to specify dedicated storage device for each interface

Does this meet your requirements?

-Takahiro Akashi

> Please feel free to contact me directly if the outline above is not
> clear enough and you want more details how I think this could be
> implemented.
> 
> 
> Best regards,
> 
> Wolfgang Denk
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> Viele Probleme erscheinen uns nur deshalb so groß, weil wir sie mit zu
> wenig Abstand betrachten.