Re: [PATCH kvmtool v4 0/5] Add CFI flash emulation

[Ard Biesheuvel <ardb@kernel.org>]

On Thu, 23 Apr 2020 at 23:32, André Przywara <andre.przywara@arm.com> wrote:
>
> On 23/04/2020 21:43, Ard Biesheuvel wrote:
>
> Hi Ard,
>
> > On Thu, 23 Apr 2020 at 19:55, Ard Biesheuvel <ardb@kernel.org> wrote:
> >>
> >> On Thu, 23 Apr 2020 at 19:39, Andre Przywara <andre.przywara@arm.com> wrote:
> >>>
> >>> Hi,
> >>>
> >>> an update for the CFI flash emulation, addressing Alex' comments and
> >>> adding direct mapping support.
> >>> The actual code changes to the flash emulation are minimal, mostly this
> >>> is about renaming and cleanups.
> >>> This versions now adds some patches. 1/5 is a required fix, the last
> >>> three patches add mapping support as an extension. See below.
> >>>
> >>> In addition to a branch with this series[1], I also put a git branch with
> >>> all the changes compared to v3[2] as separate patches on the server, please
> >>> have a look if you want to verify against a previous review.
> >>>
> >>> ===============
> >>> The EDK II UEFI firmware implementation requires some storage for the EFI
> >>> variables, which is typically some flash storage.
> >>> Since this is already supported on the EDK II side, and looks like a
> >>> generic standard, this series adds a CFI flash emulation to kvmtool.
> >>>
> >>> Patch 2/5 is the actual emulation code, patch 1/5 is a bug-fix for
> >>> registering MMIO devices, which is needed for this device.
> >>> Patches 3-5 add support for mapping the flash memory into guest, should
> >>> it be in read-array mode. For this to work, patch 3/5 is cherry-picked
> >>> from Alex' PCIe reassignable BAR series, to support removing a memslot
> >>> mapping. Patch 4/5 adds support for read-only mappings, while patch 5/5
> >>> adds or removes the mapping based on the current state.
> >>> I am happy to squash 5/5 into 2/5, if we agree that patch 3/5 should be
> >>> merged either separately or the PCIe series is actually merged before
> >>> this one.
> >>>
> >>> This is one missing piece towards a working UEFI boot with kvmtool on
> >>> ARM guests, the other is to provide writable PCI BARs, which is WIP.
> >>> This series alone already enables UEFI boot, but only with virtio-mmio.
> >>>
> >>
> >> Excellent! Thanks for taking the time to implement the r/o memslot for
> >> the flash, it really makes the UEFI firmware much more usable.
> >>
> >> I will test this as soon as I get a chance, probably tomorrow.
> >>
> >
> > I tested this on a SynQuacer box as a host, using EFI firmware [0]
> > built from patches provided by Sami.
> >
> > I booted the Debian buster installer, completed the installation, and
> > could boot into the system. The only glitch was the fact that the
> > reboot didn't work, but I suppose we are not preserving the memory the
> > contains the firmware image, so there is nothing to reboot into.
>
> It's even worth, kvmtool does actually not support reset at all:
> https://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git/tree/kvm-cpu.c#n220
>
> And yeah, the UEFI firmware is loaded at the beginning of RAM, so most
> of it is long gone by then.
> kvmtool could reload the image and reset the VCPUs, but every device
> emulation would need to be reset, for which there is no code yet.
>

Fair enough. For my use case, it doesn't really matter anyway.

> > But
> > just restarting kvmtool with the flash image containing the EFI
> > variables got me straight into GRUB and the installed OS.
>
> So, yeah, this is the way to do it ;-)
>
> > Tested-by: Ard Biesheuvel <ardb@kernel.org>
>
> Many thanks for that!
>
> > Thanks again for getting this sorted.
>
> It was actually easier than I thought (see the last patch).
>
> Just curious: the images Sami gave me this morning did not show any
> issues anymore (no no-syndrome fault, no alignment issues), even without
> the mapping [1]. And even though I saw the 800k read traps, I didn't
> notice any real performance difference (on a Juno). The PXE timeout was
> definitely much more noticeable.
>
> So did you see any performance impact with this series?
>

You normally don't PXE boot. There is an issue with the iSCSI driver
as well, which causes a boot delay for some reason, so I disabled that
in my build.

I definitely *feels* faster :-) But in any case, exposing the array
mode as a r/o memslot is definitely the right way to deal with this.
Even if Sami did find a workaround that masks the error, it is no
guarantee that all accesses go through that library.


> > [0] https://people.linaro.org/~ard.biesheuvel/KVMTOOL_EFI.fd
>
> Ah, nice, will this stay there for a while? I can't provide binaries,
> but wanted others to be able to easily test this.
>

Sure, I will leave it up until Linaro decides to take down my account.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm