Re: PATCH: Increase System Firmware Max Size

[Laszlo Ersek]

On 09/11/20 10:34, Dr. David Alan Gilbert wrote:
> * 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?

Yes, some MMIOs that I'm at least aware of are IO_APIC_DEFAULT_ADDRESS
(0xfec00000), TPM_PPI_ADDR_BASE (0xFED45000), APIC_DEFAULT_ADDRESS
(0xfee00000).

They are not directly adjacent with pflash; nor should they be.

On one hand, the current FLASH_SIZE_LIMIT is meant to be  sufficient for
a long time ("should be enough for anyone").

On the other hand, if we have a really strong reason to increase the
size limit, the current value is supposed to give us a safety margin, so
that we can satisfy the immediate need at that point *first*, and start
looking into (likely more intrusive) physical address map changes, to
restore the safety margin.

> 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?

If you increase the size limit (without tieing it to a machine type),
then, with an upgraded QEMU and the same (old) machine type, you can
start a guest with a larger-than-earlier (cumulative) flash size. Then,
when you try to migrate this to an old QEMU (but same machine type),
it's a bad surprise. I understand that backwards migration is not
universally supported (or expected), but I don't want this problem to
land on my desk *ever*.

Furthermore, un-enumerable / platform-MMIO devices tend to pop up time
after time. TPM_PPI_ADDR_BASE (0xFED45000) is a somewhat recent
addition, for example. It's not like we're never going to need more
address space up there.

> 
>> (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;

Yes, in part because I strive to keep it that way. I fight to keep out
all cruft that I can (at least by conditionalizing it) so that there is
room for the stuff that I cannot keep out. (And I always strive to set
expectations that flipping all possible build knobs in OVMF to "on" may
very well cause an "out of space" error.)

- I've made sure that PVSCSI_ENABLE and MPT_SCSI_ENABLE be stand-alone
config knobs. The use case behind them is valid, the drivers are open
source, but the use case is still niche, so they must be easy to keep out.

- I've made sure LSI_SCSI_ENABLE is a stand-alone config knob too (and
it even defaults to FALSE). The QEMU device that the driver drives is
obsolete / deprecated.

- If VMBus drivers are ever going to be contributed, they'll need a
config knob.

- Current Xen code in OVMF is supposed to be separated completely to the
new, dedicated XenPVH platform
<https://bugzilla.tianocore.org/show_bug.cgi?id=2122>.

- Bhyve support is a separate platform build.

- Capsule updates are not supported by OVMF, and if they will ever be,
they're going to have to be a separate firmware platform. Datacenter
virtualization has no use for capsule updates.

- The next big thing I expect to have to keep out of OVMF is Redfish
<https://en.wikipedia.org/wiki/Redfish_(specification)>. Libvirt,
OpenStack, Cockpit, Kubernetes already handle that area *underneath* the
guest, I believe. (It's OK to use OVMF for developing / testing Redfish;
it's not OK to expect that the current OVMF firmware platform contain
everything that it contains now *plus* Redfish.)

Sparse is *good*.

> it doesn't have any pretty graphics
> or snazzy stuff,

Which is arguably completely superfluous on every possible platform one
can imagine. On the other hand, if you want a real serial port on
workstation class hardware, you may have to order a separate part (if
you are lucky and you *can* order one). Serial-over-LAN is a complete
non-replacement.

The reason (should I say: excuse) for the firmware to exist is to (a)
boot operating systems, (b) abstract away some ugly platform-specific
hardware for OS runtime (by providing ACPI and SMBIOS descriptions, and
a *small* set of runtime services). We can maybe add (c) "root of trust".

In practice, physical firmware is becoming the hw vendor's OS before
(and under) your OS, one you cannot replace, and one whose updates can
brick your hardware. Permitting the same feature creep on virtual
platforms is wrong.

> 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;

Key term being "open uses". Let us see them.

> so I don't see a problem with
> changing this limit from the QEMU side of things.

I do. Software and data always expand to consume all available space,
and it's going to cause a conflict between UEFI features and platform
MMIO sooner or later. Then I'll get the privilege of shuffling around
the crap in OVMF (all of which is "indispensable" or course).

If we ever go down this route, it needs to benefit open source directly
and significantly.

Laszlo