Re: [Qemu-devel] bundling edk2 platform firmware images with QEMU

[Daniel P . Berrangé]

On Fri, Mar 08, 2019 at 03:48:07PM +0100, Laszlo Ersek wrote:
> On 03/08/19 14:42, Daniel P. Berrangé wrote:
> >>  pc-bios/edk2-aarch64-code.fd                   | Bin 0 -> 67108864 bytes
> >>  pc-bios/edk2-arm-code.fd                       | Bin 0 -> 67108864 bytes
> >>  pc-bios/edk2-arm-vars.fd                       | Bin 0 -> 67108864 bytes
> >>  pc-bios/edk2-i386-code.fd                      | Bin 0 -> 3653632 bytes
> >>  pc-bios/edk2-i386-secure-code.fd               | Bin 0 -> 3653632 bytes
> >>  pc-bios/edk2-i386-vars.fd                      | Bin 0 -> 540672 bytes
> >>  pc-bios/edk2-licenses.txt                      | 209 +++++++++++++++
> >>  pc-bios/edk2-x86_64-code.fd                    | Bin 0 -> 3653632 bytes
> >>  pc-bios/edk2-x86_64-secure-code.fd             | Bin 0 -> 3653632 bytes
> > 
> > 
> > Yikes, am I really reading those sizes right ? The biggest ROMs there
> > are 64 MB, so this is proposing to add ~206 MB of binaries to the
> > pc-bios directory ?
> 
> Sort of.
> 
> This is because of how the "virt" machine type's pflash chips are sized.
> The edk2 build produces a 2MB firmware executable and a 768KB varstore
> template for each of aarch64 and arm, but (a) QEMU doesn't auto-pad
> either [*], and (b) if we used qcow2 with compression, then libvirt
> couldn't deal with the images [**]. Hence we have to pad the files
> manually, after the edk2 build completes.
> 
> [*] We've just been discussing auto-padding in a parallel patch from
> Alex and Markus (the latest version is "[PATCH v7] pflash: Require
> backend size to match device, improve errors", in which the padding has
> already been dropped). But such padding would only work for read-only
> pflash mappings anyway, not for variable store files.
> 
> [**] The root cause for not using qcow2 with pflash images is that qcow2
> would make them candidates for "savevm" to dump live memory contents
> into them, which is Very Bad (TM).

If using  qcow2 compressed images for ROMs is the desired best answer,
then surely we can figure out a way to fix this savevm problem ? Even
if it is just an internal hack there must be a way we can mark a qcow2
file as being used by the ROM so savevm ignores it.

We've not been very motivated to solve this before since "savevm" command
is supposedly deprecated to be replaced by a sane QMP variant, but we've
been waiting 5 years for that to happen, so in the mean time a hack to
resolve the problem with firmware images feels prudent.

> > Consider that we'll need to refresh those ROMs multiple times a year to
> > track updates or security fixes. It will result in our .git repo size
> > growing massively over time.
> 
> Actually, it's not *that* bad. Earlier I investigated how the formatted
> variant of the patch (adding these blobs) would look like.
> 
> First, I compressed all the "edk2-*fd" files (individually) with "gzip
> --best", just to get a byte count. That yielded almost precisely 9MB, in
> total.
> 
> Second, the formatted patch, adding these blobs, is 12.6MB. Taking the
> 4/3 blowup factor from base64 encoding, the "unexplained overhead" is
> not that huge (0.6 MB).

Ok, that's less alarming.

Though we're still basically doubling the size of the pc-bios dir
which is already significant.

I wonder how much of out .git/objects 230 MB size is due to the current
ROMs.

> - "git checkout" is smart enough to punch holes into the files in the
> working directory.
> 
> $ du -csh edk2-{arm,aarch64}-code.fd edk2-arm-vars.fd
> 2.1M    edk2-arm-code.fd
> 2.1M    edk2-aarch64-code.fd
> 772K    edk2-arm-vars.fd
> 4.8M    total

That's nice.

> This happens despite the fact that the Makefile performs the padding
> with "cat /dev/zero" and "head".

You can resize a file using holes with dd if you set count=0 and
tell it to seek in output eg something like this probably works:

  dd if=/dev/zero of=firmware.img seek=64 bs=1M count=0



> I'm 100% fine with not bundling any firmware binaries for end-users;
> however I've been doing this work because Igor needs full platform
> firmware binaries for ACPI unit tests. In particular, in aarch64 guests,
> there is no ACPI without UEFI, and in UEFI guests, finding the ACPI
> entry point (RSD PTR) is convoluted enough that it needs dedicated guest
> support.

Yes tricky. I don't have a particularly good answer for that, especially
if it is a test that you'd expect all developers to run every time. If
it is a rarely run test then it could be justified in having it download
large blobs from a lookaside cache similar to how we download disk images
for *BSD tests.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|