[RFC PATCH 0/3] efi: MMC proxy support for the UEFI varstore

[Mark Rutland]

Hi Ard,

On Thu, Sep 22, 2016 at 12:30:03PM +0100, Ard Biesheuvel wrote:
> ================================================================================
> NOTE: this is a work in progress, and not fully functional yet. In particular,
> the actual MMC host protocol methods are stubbed out at the moment, and need to
> be wired up to the Linux device drivers.
> ================================================================================
> 
> On mobile and embedded systems, there is usually only a single MMC device for
> non-volatile storage, which sits behind a controller that is owned by the OS at
> runtime. This makes it difficult to host the UEFI variable store on MMC as well,
> since the UEFI runtime services routines expect ownership of the underlying
> device as well.
> 
> This series proposes an approach to work around this. It implements the UEFI
> MMC host protocol in the kernel, in a way that makes it possible to expose it
> to the firmware. At the same time, the firmware needs be set up for this, i.e.,
> it needs to expose its MMC host protocol pointer via a UEFI configuration table,
> so that the kernel can override it if it decides to expose this functionality
> to the firmware.

At a high level, and assuming a number of details from previous
discussions, I think the general approach of having the kernel mediate
access to the MMC makes sense.

However, I don't think that this series has enough detail for critical
review, even at the interface level. e.g. there is no mention of how
this caters for replay attacks and so on (which the current spec
sidesteps). I'm under the impression that there are mechanisms which
have been discussed for this, and I hope this is simply an oversight.

I also think that this needs to go via the USWG (and to the UEFI spec),
before we can consider using it. I say this because:

* This is critical to correct operation of variable storage as required
  for the standard boot flow. This is a major change to the way variable
  storage works today, with a number of (security) implications.

* There are others in this space trying to use the same class of
  hardware, e.g. FreeBSD. We don't want a Linux-specific interface, nor
  do we want a proliferation of interfaces for this purpose.

I have a few other general concerns:

* Identification of the relevant MMC device(s).
 
  Patch 3 suggests a devicetree property. I don't think that the
  'linux,' prefix makes sense, and it's not clear what we would do with
  ACPI.
 
  Does the MMC device not have some identifier we can query, so that we
  can match this up without requiring additional info in ACPI/DT?

* Lifetime guarantees
 
  When is it valid for EFI to call the MMC proxy? Can other services
  (e.g. ACPI) call this?

  How do we handle kexec/kdump? e.g. how do we teardown the interface
  before branching to a new kernel, how do we safely tear down a crashed
  kernel's interface, what can we call before doing so?

  How do we handle suspend/resume? e.g. is it necessary to re-register
  upon resume?

Thanks,
Mark.

> Note that these patches are based on patches in the EFI tree that are queued
> for v4.9, which replace the runtime wrappers spinlock with a semaphore. This
> allows us to sleep in the firmware callbacks.
> 
> Prerequisites for using these patches:
> 
> * qemu-system-aarch64 built from this branch:
>   https://git.linaro.org/people/ard.biesheuvel/qemu.git/shortlog/refs/heads/mach-virt-pl181
>   which adds a PL181 SD/MMC controller to the mach-virt model, and exposes it
>   via the device tree. It also sets the 'linux,uefi-varstore' property on this
>   node.
> 
> * UEFI firmware built from this branch:
>   https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/refs/heads/mmc-proxy
>   
>   Build using the following command
>   build -a AARCH64 -t GCC5 -p ArmVirtPkg/ArmVirtQemuMmcVars.dsc
> 
>   Run using 
>   qemu-system-aarch64 \
>     -M virt -cpu cortex-a57 -m 2048 \
>     -device virtio-blk-device,drive=boot \
>     -drive if=none,id=boot,file=fat:<path-of-Image>,format=raw \
>     -kernel <edk2-dir>/Build/ArmVirtQemuMmcVars-AARCH64/DEBUG_GCC5/FV/QEMU_EFI.fd \
>     -sd <edk2-dir>/build/edk2/Build/ArmVirtQemuMmcVars-AARCH64/DEBUG_GCC5/FV/QEMU_VARS.fd \
>     -nographic $@
> 
>   This will give you an UEFI environment which keeps its UEFI variables in the
>   emulated MMC volume, and exposes its MMC host protocol in a way that allows
>   these patches to hook into it.
> 
> Patches #1 and #2 implement the arch specific hooks to preserve/restore the NEON
> registers that the firmware may expect to be preserved across function calls.
> 
> Patch #3 implements the plumbing to call back into the kernel from the firmware.
> 
> Please comment on whether this approach seems feasible, and in particular, how
> on earth I should wire this up to the actual MMC code.
> 
> Thanks,
> Ard.
> 
> Ard Biesheuvel (3):
>   efi/arm64: add SIMD stash/unstash operations
>   efi/arm: add SIMD stash/unstash operations
>   efi: implement MMC proxy support for the UEFI variable store
> 
>  arch/arm/include/asm/efi.h       |  11 +
>  arch/arm64/include/asm/efi.h     |  33 +++
>  drivers/firmware/efi/Kconfig     |   9 +
>  drivers/firmware/efi/Makefile    |   1 +
>  drivers/firmware/efi/arm-init.c  |   2 +
>  drivers/firmware/efi/mmc-proxy.c | 222 ++++++++++++++++++++
>  include/linux/efi.h              |   1 +
>  7 files changed, 279 insertions(+)
>  create mode 100644 drivers/firmware/efi/mmc-proxy.c
> 
> -- 
> 2.7.4
>