Re: [PATCH 00/16] hw/uefi: add uefi variable service

[Gerd Hoffmann]

On Mon, Nov 20, 2023 at 12:53:45PM +0100, Alexander Graf wrote:
> Hey Gerd!
> 
> On 15.11.23 16:12, Gerd Hoffmann wrote:
> > This patch adds a virtual device to qemu which the uefi firmware can use
> > to store variables.  This moves the UEFI variable management from
> > privileged guest code (managing vars in pflash) to the host.  Main
> > advantage is that the need to have privilege separation in the guest
> > goes away.
> > 
> > On x86 privileged guest code runs in SMM.  It's supported by kvm, but
> > not liked much by various stakeholders in cloud space due to the
> > complexity SMM emulation brings.
> > 
> > On arm privileged guest code runs in el3 (aka secure world).  This is
> > not supported by kvm, which is unlikely to change anytime soon given
> > that even el2 support (nested virt) is being worked on for years and is
> > not yet in mainline.
> > 
> > The design idea is to reuse the request serialization protocol edk2 uses
> > for communication between SMM and non-SMM code, so large chunks of the
> > edk2 variable driver stack can be used unmodified.  Only the driver
> > which traps into SMM mode must be replaced by a driver which talks to
> > qemu instead.
> 
> 
> I'm not sure I like the split :). If we cut things off at the SMM
> communication layer, we still have a lot of code inside the Runtime Services
> (RTS) code that is edk2 specific which means we're tying ourselves tightly
> to the edk2 data format.

Which data format?

Request serialization format?  Yes.  I can't see what is wrong with
that.  We need one anyway, and I don't see why inventing a new one is
any better than the one we already have in edk2.

Variable storage format?  Nope, that is not the case.  The variable
driver supports a cache, which I think is a read-only mapping of the
variable store, so using that might imply we have to use edk2 storage
format.  Didn't check in detail through.  The cache is optional, can be
disabled at compile time using PcdEnableVariableRuntimeCache=FALSE, and
I intentionally do not use the cache feature, exactly to avoid unwanted
constrains to the host side implementation.

> It also means we can not easily expose UEFI
> variables that QEMU owns,

Qemu owning variables should be no problem.  Adding monitor commands to
read/write UEFI variables should be possible too.

> which can come in very handy when implementing MOR
> - another feature that depends on SMM today.

Have a pointer for me?  Google finds me
https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/device-guard-requirements,
which describes the variable behavior (which I think should be no
problem to implement), but doesn't say a word about what exactly gets
locked when enabled ...

> In EC2, we are simply serializing all variable RTS calls to the hypervisor,

The edk2 code effectively does the same (with 
PcdEnableVariableRuntimeCache=FALSE).

> similar to the Xen implementation
> (https://www.youtube.com/watch?v=jiR8khaECEk).

Is the Xen implementation upstream?  Can't see a xen variable driver in
OvmfPkg.  The video is from 2019.  What is the state of this?

> The edk2 side code we have built is here:
> https://github.com/aws/uefi/tree/main/edk2-stable202211 (see anything with
> VarStore in the name).

Ok, so it's just the variables.  The edk2 variant also sends variable
policy requests (see Edk2VariablePolicyProtocol,
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/VariablePolicyLib/ReadMe.md).
And it can easily be extended should the need arise in the future (all
requests are tagged with a protocol/event guid).

> Given that we deal with untrusted input, I would strongly prefer if we could
> move it to a Rust implementation instead though.

Valid point.

I've started in C because I have next no to experience with rust (yet),
so getting a test-able / demo-able implementation done was much easier
for me.  Also I think we don't have any rust infrastructure in qemu
(yet?).  Being able to use the qapi / qobject support to read/write the
variable store in json format is nice too.

But I'm open to discuss other options.

> We started a Rust
> reimplementation of it that interface that can build as a library with C
> bindings which QEMU could then link against:
> 
>   https://github.com/Mimoja/rs-uefi-varstore/tree/for_main
> 
> The code never went beyond the initial stages,

Hmm.  Why not?

> but if we're pulling the variable store to QEMU, this would be the
> best path forward IMHO.

Ok, so you are trying to sell me a prototype as solution?
/me looks a bit skeptical ...

take care,
  Gerd