Re: PATCH: Increase System Firmware Max Size

[Laszlo Ersek]

On 09/11/20 17:06, Dr. David Alan Gilbert wrote:
> * Laszlo Ersek (lersek@redhat.com) wrote:
>> On 09/11/20 10:34, Dr. David Alan Gilbert wrote:

>>> 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.
> 
> Hmm those need explicitly checks adding somewhere against
> FLASH_SIZE_LIMIT!

Yes, that would be nice. I don't know how it works. Maybe when adding
the next MemoryRegion there's an error or an assert(). No clue.

>> 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*.
> 
> I support backwards migration; but that migration wouldn't work anyway -
> wouldn't that fail nicely with a mismatched RAMBlock size?

My point wasn't that the guest would be lost or corrupted, only that it
couldn't be migrated. We'd say "for this, you have to upgrade QEMU on
the destination host as well, or use a smaller firmware", and they'd say
"we don't want either of those things".

>>> Our UEFI firmware is pretty sparse;
>>
>> Yes, in part because I strive to keep it that way.
> 
> But that's your choice, on our firmware implementation; that's not a
> requirement of QEMU or q35.

Right; if we can keep regressions out (not just functional regressions,
but workflow / use case regressions too), then it's OK to support more
use cases.

By workflow / use case regressions I mean that it should not become more
difficult to maintain OVMF as a result of the patch. (It should not
imply that now people can stuff even more cruft into OVMF, because "hey
there's more room now".)

>> 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.
> 
> On the firmware you develop that choice is fine; but there's no reason
> to force that restriction onto others.

Technically, I agree. It's fine to run larger UEFI firmware as long as
the default size restriction for the default (or traditional) UEFI
firmware remains the same.

Turn the size limit into a property, keep the same default value,
implement the migration aspects; specify *very clearly* in the commit
message what particular firmware this knob is being introduced for.

... And I'd still be grumpy, because it increases maintenance burden for
QEMU (and possibly OVMF too -- "hey we got more room now!"; see above),
without open source users benefiting from the change. It's not like
we'll ever be able to run, or read the source code of, that 8MB+
firmware image, is it?

>>> 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.
> 
> Well yes, I think we know who we're speaking about here and we're
> working on it.

Sorry, I wasn't clear enough.

I *know* we're going to see *those* "open uses" that you meant.

Precisely because they are "open uses", they have a chance at justifying
the churn.

My intent was to apply your (valid) argument to *this* proposal -- let
us see the "open uses" for *this* particular proposal.

Notice, in the thread starter:

"We have a need for increased firmware size", "our Uefi Firmware",
"change can be made to open source" --> it's obviously for the sake of a
proprietary platform firmware. Do you feel comfortable about taking on
more risks, reviews and maintenance for that?

(Note that I'm not singling out this particular proprietary guest
payload. I feel the exact same way when QEMU is being contorted for
Windows of OSX guests, but at least those guest payloads are universally
available to users, if the users are willing to comply with the
corresponding terms and conditions.)

> Being able to use QEMU to let vendors debug their platform firmware is a
> perfectly reasonable use of QEMU; maybe not of your OVMF build - we
> need to keep the restrictions on the two separate.

You as a QEMU maintainer / reviewer, and I as an OVMF maintainer /
reviewer, are going to pay with our time and effort for a proprietary
guest oriented change that normal QEMU users won't even be able to run,
let alone read, modify, distribute.

But yes, technically speaking, we can replace the size limit constant
with a property, I think.

Laszlo