[U-Boot] [RFC 6/6] efi_loader: variable: support runtime variable access via cache

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

Heinrich,

On Tue, Jun 18, 2019 at 10:45:26PM +0200, Heinrich Schuchardt wrote:
> On 6/18/19 12:34 PM, Ilias Apalodimas wrote:
> >Hi all,
> >
> >[...]
> >>>>>I think one version of the functions serving at runtime and boottime is
> >>>>>enough.
> >>>>>
> >>>>>The cache should be used both at runtime and at boottime.
> >>>>
> >>>>So do you mean that we should replace the existing "boottime" version
> >>>>of get/set_variable with my code (algorithm)?
> >>>>
> >>>>This is a bit complicated work because we should be able to *udpate*
> >>>>UEFI variables at boottime, but my version of hsearch_runtime() is
> >>>>a stripped (and modified) version and doesn't support it.
> >>>
> >>>Do we really need a multilevel hash table? I would not expect hundreds
> >>>of variables.
> >>
> >>Please don't change your point suddenly.
> >>Here we are discussing whether "The cache should be used both at runtime
> >>and at boottime" or not.
> >>
> >Heinrich, the idea here is to present a copy of the variables on the OS
> >and totally disable RT variable updating from the OS.
> >If someone wants to update UEFI variables, we can pack them to a Capsule
> >(using FIT image format) and apply them on next reboot.
> >Given the fact that UEFI variables are not updated that often, isn't this a
> >viable option? If it is, then we need to keep the access separated
> >(as Akashi-san suggests) allowing bootime to change the variables.
> >
> >>>>
> >>>>Making the existing hsearch_r() executable at UEFI runtime is,
> >>>>as I said before, quite painful.
> >>>
> >>>You could start the cache implementation with a less complicated data
> >>>structure like a linked list.
> >>
> >>This is totally a different issue. I listed this issue
> >>in my cover letter.
> >>
> >>>>
> >>>>>Essentially I
> >>>>>expect three modules working together:
> >>>>>
> >>>>>UEFI API implementation <-> Cache <-> Persistence driver
> >>>>>
> >>>>>I would suggest to put each of these into a separate file.
> >>>>>
> >>>>>Both the API implementation and the Cache have to be available at
> >>>>>Boottime and at Runtime. A first version of the persistence driver may
> >>>>>only be working at boottime.
> >>>>
> >>>>Unfortunately, this is not practical right now because there is
> >>>>already some sort of assumption (and consensus) that we would re-use
> >>>>"Standalone MM services", which is already there in EDK2, as
> >>>>secure storage for UEFI variables.
> >>>>In the case, all the cache would be bypassed.
> >>>>In my old prototype, I utilized the cache but dropped that feature
> >>>>for several reasons.
> >>>
> >>>What has EDK2 code to do with it?
> >>
> >>Did you follow my comment below?
> >>>>Unfortunately, this is not practical right now because there is
> >>>>already some sort of assumption (and consensus) that we would re-use
> >>>>"Standalone MM services", which is already there in EDK2, as
> >>>>secure storage for UEFI variables.
> >We are already working towards having StandAloneMM as an early OP-TEE TA.
> >This will provide us with a secure variable storage for armv7/v8.
> 
> What would this OP-TEE binary do? - This seems to be a source of
> misunderstanding when reviewing this patch.

I and Ilias will give you more details offline, here's a short(?)
answer:

Standalone MM services here means a SPD entity which provides
[Get|Set]Variable APIs to non-secure side firmware, that is
currently EDK2. So the source code of Standalone MM services
is included in EDK2 repository as a matter of fact.

Here is one drawback: It won't allow for other entities running
concurrently on secure side. One example of useful secure feature
is (software-based) TPM. So Linaro is working on modifying/transforming 
Standalone MM to one OP-TEE application, which Ilias mentioned above.

> My guess is that OP-TEE is used to read non-volatile variables only once
> when starting U-Boot and to write non-volatile variables whenever they
> are changed.

So OP-TEE version of StMM is still on-going project and I assume
that this OP-TEE app will support the same set of functionality/APIs
as StMM does.

> All further reading of non-volatile variables and all access to volatile
> variables will be handled by the U-Boot internal variable cache.
> 
> For volatile variables I would assume OP-TEE to never see them. This
> requires that the U-Boot variable cachek supports reading from and
> writing to the cache at runtime.

No. As far as I correctly understand, StMM handles volatile
variables as well as non-volatile variables.
EDK2 on non-secure side will redirect user's request directly
to secure side even without *caching* variable's values.

> StandaloneMmPkg seems to be the hardware independent part of the
> solution. Where will the hardware driver reside in your OP-TEE solution?
> 
> Is the EDK2 hardware store for variables of the MACCHIATObin here:
> edk2-platforms/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c?
> 
> Which hardware platform will you use for testing the U-Boot development
> of you OP-TEE driver?

Ilias will be able to answer those questions.

Thanks,
-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> >
> >>
> >>>In case of write you could do a write-through in your cache if needed.
> >>>
> >>>>
> >>>>>The NV-cache content should be written to non-volatile memory on Reset()
> >>>>>and on ExitBootServices() and if possible when updating variables at
> >>>>>runtime.
> >>>>
> >>>>I'm not sure your intent here, but are you going to write back
> >>>>the cache only once?
> >>>>It won't work as every change of UEFI variable must be flushed
> >>>>to persistent storage instantly.
> >>>
> >>>>
> >
> >
> >Thanks
> >/Ilias
> >
>