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

[Wolfgang Denk]

Dear Takahiro,

In message <20191028011435.GP10448 at linaro.org> you wrote:
>
> > Can you please at least comment on the size impact?  How much does
> > the code size grow on everage, especially for SPL?
>
> Well, from the next version as I will make a drastic change as far as
> I follow your suggestion below.

OK.

> Okay, I see but I hope that you should have said so much earlier.
> My patch (v5) has been stranded almost two months without any comments.

I have said so uin lcear and unmistakable words when discussion was
to add me as maintainer for environment code.  I cannot commit to
such a position as I don't have enough time available.

> > > > > > > * Non-volatile feature is not implemented in a general form and must be
> > > > > > >   implemented by users in their sub-systems.
> > > > 
> > > > I don't understand what this means, or why such a decision was made.
> > > > Which sub-systems do you have in mind here?
> > >
> > > UEFI sub-system/library.
> > 
> > What needs to be done to have this - say - for U-Boot context?
> >
> > > > What prevented you from implementing a solution to works for all of
> > > > us?
> > 
> > ?
>
> At least for me or UEFI, nothing more, other than a *context*, is needed
> to meet the requirement.
> If some one really want to have such(?) a feature, he or she can on top
> of my patch.
> I believe that my attitude here is fair enough.

I would appreciate if you could actually answer my questions.  there
are two of them:

You write: "Non-volatile feature is not implemented in a general
form and must be implemented by users in their sub-systems."

Q1: Why did you not implement this feature in a generic way, so it
    is automatically available to all sub-systems that support
    contexts?

Q2: What exactly needs to be done, which code needs to be writtenm,
    to implement this feature - say - for U-Boot context?


> 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===

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?

Or am I missing something?

> > > > You don't seriously expect to have patches which cause
> > > > "incomprehensible memory corruption" to be included into mainline?
> > >
> > > It will be just a matter of time for debugging.
> > 
> > It might be difficult to find willing testers under such
> > circumstances.  I would not want to run code with serious known
> > bugs.
>
> I believe that the issue is independent from the basic approach
> that we are now discussing.

The reason of this problem must be understood and the bug fixed
before it can go into mainline.

> > I think this is a new feature that needs to be tested.  You focus on
> > UEFI + FAT and I don't know if UEFI + non-FAT makes sense, but at
> > least (1) non-UEFI + FAT and (2) non-UEFI + non-FAT are
> > configurations which must be tested - absolute minimum is at least
> > one example implementation. My suggestion would be to use something
> > that is not file system based, but using plain storage, say a board
> > which has the environment in SPI NOR flash.
>
> What do you mean by "non-UEFI"? Disabling UEFI, or testing U-Boot environment
> variables on no-FAT device?
> If the latter, I have tested it with flash (on qemu).

We need to test if the new context code works (and does not cause
problems) on systems where UEFI is not enabled.

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
Don't you know anything? I should have thought anyone knows that  who
knows anything about anything...      - Terry Pratchett, _Soul Music_