Re: PATCH: Increase System Firmware Max Size

[Dr. David Alan Gilbert]

* Laszlo Ersek (lersek@redhat.com) wrote:
> +Markus, Dave, Phil
> 
> On 09/11/20 03:45, McMillan, Erich wrote:
> > Hi all,
> > 
> > (this is my first Qemu patch submission, please let me know if my 
> > formatting/content needs to be fixed).
> > We have a need for increased firmware size, currently we are building Qemu 
> > with the following change to test our Uefi Firmware and it works well for 
> > us. Hope that this change can be made to open source. Thank you.
> > -------
> > Increase allowed system firmware size to 16M per comment suggesting up to 
> > 18M is permissible.
> > 
> >  Signed-off-by: Erich McMillan <erich.mcmillan@hp.com>
> > 
> > diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
> > index 
> > b8d8ef59eb17c6ace8194fc69c3d27809becfbc0..f6f7cd744d0690cee0355fbd19ffdcdb71ea75ca
> >  100644
> > --- a/hw/i386/pc_sysfw.c
> > +++ b/hw/i386/pc_sysfw.c
> > @@ -46,7 +46,7 @@
> >   * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB 
> > in
> >   * size.
> >   */
> > -#define FLASH_SIZE_LIMIT (8 * MiB)
> > +#define FLASH_SIZE_LIMIT (16 * MiB)
> > 
> >  #define FLASH_SECTOR_SIZE 4096
> > -------
> > 
> > 
> 
> (1) This is not a trivial change, so qemu-trivial: please ignore.
> 
> (2) The comment has not been updated.
> 
> (3) I'm almost certain that *if* we want to change this, it needs to be
> turned into a machine type (or some device model) property, for
> migration compatibility.

I'm missing what this constant exists for - why is the
check there at all  Is there something else that lives below this
address that we have to protect?
My reading of the code is that increasing that constant doesn't change
the guests view at all, as long as the guest was given the same flash
files - so if the guests view doesn't change, then why would we tie
it to the machine type?

> (4) I feel we need much more justification for this change than just
> "our firmware is larger than upstream OVMF".
> 
> In the upstream 4MB unified OVMF build, there's *plenty* of free room.
> Of course that's not to say that we're willing to *squander* that space
> -- whenever we include anything new in the upstream OVMF platform(s),
> there must be a very good reason for it --, just that, given the
> upstream OVMF status, the proposed pflash increase in QEMU is staggering.
> 
> Considering upstream OVMF and QEMU, it should take *years* to even get
> close to filling the 4MB unified flash image of OVMF (hint: link-time
> optimization, LZMA compression), let alone to exhaust the twice as large
> (8MB) QEMU allowance.
> 
> Unless you are committed to open source your guest firmware too (maybe
> as part of upstream edk2, maybe elsewhere), I seriously doubt we should
> accommodate this use case in *upstream* QEMU. It complicates things
> (minimally with regard to migration), and currently I don't see the
> benefit to the upstream community.
> 
> Clearly, for building your firmware image, you must have minimally
> modified the DSC and FDF files in OVMF too, or created an entirely
> separate firmware platform -- DSC and FDF both.
> 
> If you are using your downstream OVMF build as a testbed for your
> proprietary physical platform firmware, that's generally a use case that
> we're mildly interested in not breaking in upstream OvmfPkg. But in
> order to make me care for real (as an OvmfPkg co-maintainer), you'd need
> to upstream your OVMF platform to edk2 (we already have Xen and --
> recently added -- bhyve firmware platforms under OvmfPkg, not just
> QEMU). You'd also have to offer long-term reviewership and testing for
> that platform in edk2 (like the Xen and bhyve stake-holders do). Then we
> could consider additional complexity in QEMU for booting that firmware
> platform.

Our UEFI firmware is pretty sparse; it doesn't have any pretty graphics
or snazzy stuff, or have to survive configuring lots of hardware; also
I'm aware of other companies who are looking at putting big blobs
of stuff in firmware for open uses;  so I don't see a problem with
changing this limit from the QEMU side of things.

Dave

> 
> Thanks,
> Laszlo
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK