On Fri, 8 Mar 2019 15:06:12 +0000
Daniel P. Berrangé <berra...@redhat.com> wrote:
> 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.
I'd say it's intended to run on every 'make check' for bios-tables-test
(arm/x86 variants) like we do now with Seabios. The goal is to catch
regressions in ACPI table (as we have zero coverage there).
Secondary motivation is to help with merging NEMU fork. Intel generalizes
a bunch ACPI code across x86 and ARM boards and I'm not ready to manually
test every iteration posted on list.
The last time we talked where to put firmware, we've got to consensus
to bundle UEFI within qemu like we do with other firmware.
Question of moving firmwares somewhere else is still open but it's
a separate topic and shouldn't block us from properly testing arm board
now (and other related projects that depend on it).
As for soft-freeze, maybe it's acceptable to merge this series during it
as its primary(the only) user would be 'make check'. I'll respin tests
after rebasing previous iteration on top of this series.
> Regards,
> Daniel
