[U-Boot] [PATCH v5 00/19] efi_loader: non-volatile variables support

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

Wolfgang,

On Mon, Nov 04, 2019 at 05:00:03PM +0100, Wolfgang Denk wrote:
> Dear Takahiro,
> 
> In message <20191101060434.GV10448@linaro.org> you wrote:
> >
> > > This is even worse, as instead of adding a single argument to a
> > > function call you are adding two full fucntion calls.
> > > 
> > > But why would that be needed?
> > > 
> > > U-Boot is strictly single tasking.  You always know what the current
> > > context is, and nobody will change it behind your back.
> > > 
> > > And UEFI functions are not being called in random places, they are
> > > limited to a small set of UEFI commands, right?  So why do you not
> > > just enter UEFI context when you start executing UEFI code, and
> > > restore the rpevious state when returning to non-UEFI U-Boot?
> >
> > Can you elaborate What you mean by "entering UEFI context"?
> 
> You wrote:
> 
> | So do you always admit the following coding?
> | ===8<===
> |   (in some UEFI function)
> |   ...
> |   old_ctx = env_set_context(ctx_uefi);
> |   env_set(var, value);
> |   env_set_context(old_ctx);
> |   ...
> | ===>8===
> 
> In this code, the function call ``env_set_context(ctx_uefi)'' would
> enter the UEFI context, right?  This is what I mean.

My point is not there. See below.

> > What I'm thinking of here is that we would add one more member field, which
> > is a pointer to my "env_context," in GD and change it in env_set_context().
> > This can be defined even as an inline function.
> 
> Yes, this is OK - I think I understood this already.

So we get one step closer.

> What I mean is that such a call of env_set_context() is not needed
> for each and every call of other env_*() functions.
> 
> If I understand correctly, only the UEFI specific commands will
> actually care about UEFI contexts - most likely no code outside
> cmd/nvedit_efi.c ?

No. There are a couple of reasons:
1) Other commands include cmd/bootefi.c (and cmd/efidebug.c).
2) Any EFI application, once kicked off, can directly call EFI APIs.
3) Bunch of EFI features (either services or protocols) fundamentally
   reply on U-Boot native functionality, including devices (disk,
   network, console, ...), file systems and so on.

   So calling such UEFI interfaces may eventually end up with invoking
   env_*() *indirectly*. I don't come up with good examples (probably,
   network is a candidate) now but such a situation will be quite likely
   and we should not exclude such a possibility in the future.

> So you have pretty clear entry and exit points into and from the
> UEFI world, and it should be sufficient to set and restore this
> context only then.

My point is what entry or exit points are.
They cannot be at each command, but *each* API.
So, let me repeat this example:
===8<===
   (in some UEFI function)
   ...
   old_ctx = env_set_context(ctx_uefi);
   env_set(var, value);
   env_set_context(old_ctx);
   ...
===>8===

This can be at most relaxed to:
===8<===
some_efi_api(...)
{
   old_ctx = env_set_context(ctx_uefi);
   ...
   (env_get(var1);)
   ...
   env_set(var2, value);
   ...
   env_set_context(old_ctx);

   return;
}
===>8===
It is merely a matter of optimization.
This is the reason that I want to introduce env_set_context().

Thanks,
-Takahiro Akashi

> > I don't see why you deny such a simple solution with lots of flexibility.
> 
> I am concerned about code size and execution speed.  So far, you did
> not provide any such numbers, even though I repeatedly asked for the
> size impact, especially for non-UEFI systems.
> 
> Please keep in mind that there are many cases where we need access
> to the environment already in SPL, and on all systems where security
> plays any role we must have the same set of features enabled for the
> environment code in SPL and TPL (if these need access to the
> envronment) as in U-Boot proper, and memory is a precious resource
> there.
> 
> 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
> You may call me by my name, Wirth, or by my value, Worth.
> - Nicklaus Wirth