On Thu, 23 Apr 2020 at 23:32, André Przywara <andre.przywara@xxxxxxx> wrote: > > On 23/04/2020 21:43, Ard Biesheuvel wrote: > > Hi Ard, > > > On Thu, 23 Apr 2020 at 19:55, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > >> > >> On Thu, 23 Apr 2020 at 19:39, Andre Przywara <andre.przywara@xxxxxxx> 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@xxxxxxxxxx> > > 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.
