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

From: Tom Rini <trini@konsulko.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v5 00/19] efi_loader: non-volatile variables support
Date: Mon, 4 Nov 2019 11:16:35 -0500	[thread overview]
Message-ID: <20191104161635.GW11173@bill-the-cat> (raw)
In-Reply-To: <20191104160003.1B04C24009F@gemini.denx.de>

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.
> 
> > 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.
> 
> 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 ?
> 
> 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.
> 
> > 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.

I too am very concerned that whatever we come up with here needs to have
between zero and near-zero impact on the rest of the U-Boot world.
We've once again hit the case where some boards don't build due to
bugfixes increasing their binary size.  And they aren't "no one uses
these, we can drop them" boards either.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191104/23848b1b/attachment.sig>