Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Anthony Liguori <anthony@codemonkey.ws>]

Laszlo Ersek <lersek@redhat.com> writes:

> On 06/10/13 21:57, Anthony Liguori wrote:
>> Laszlo Ersek <lersek@redhat.com> writes:
>
>> 
>> I don't understand this piece.
>> 
>> Other than TianoCore being a weird environment, what made this more
>> difficult than it is to generate the tables in QEMU?
>
> QEMU can pass down two kinds of SMBIOS config items over fw_cfg:
> verbatim tables and individual fields.
>
> The verbatim table kind means "install this table as-is". I'm not sure
> how SeaBIOS solves it, in OVMF you call Smbios->Add().
>
> Regarding the "field kind" config item: for each table type required by
> the SMBIOS spec, if qemu doesn't provide such a table in whole / in
> verbatim, then the firmware must install a default table for that type,
> *and* patch it with any referring field kind config items.
>
> Given the current format of the "field kind" config item, it is
> currently not possible to just iterate over the "field kind" config
> items, and patch whatever field in whatever table they refer to. The
> information contained in the "field kind" config item is insufficient to
> do this.
>
> The "field kind" config item designates a (table, field offset) pair. If
> that offset points at a string-typed SMBIOS field (basically string
> serial number into the unformatted area), one must update the
> unformatted string area after the actual table fields. (In OVMF one does
> this with a different call, Smbios->UpdateStrings(), and at that point
> the table even must have been installed already.)
>
> If the offset points at a non-string-typed (= in-formatted-area) field,
> the field must be overwritten in-place. (In OVMF this must happen
> *before* table installation, ie. before Smbios->Add().)

Okay, I understand the problem here.  Thanks for the explanation.  I had
assumed that the smbios_field structure was the binary representation of
the table.  I didn't realize it was our own format used to pass
information over.

> So, the current fw_cfg representation is insufficient, and the firmware
> must extend (or qemu should have extended) the info contents with this
> distinction between formatted and unformatted. Again this differs from
> field to field and the origin of that classification is the SMBIOS spec.
>
> Until this point in my email there has been no clear indication whether
> qemu or the firmware should do this. *Some* side will have to suffer
> shoveling the field/offset info, and a table template for each type,
> from the SMBIOS spec into code. I described the above solely as
> background, to set the landscape.
>
> (a) The first thing that tips the scale is: of qemu there is one, and of
> firmware, there exist at least two, *wildly different* implementations.
> (Apologies, I forgot about coreboot; not sure if it's affected.) It's
> not a design purity argument, just what's cheaper.

The only reason there are two implementations is a licensing problem
that must be solved anyway.

> (b) The second thing that further unbalances the scale: suppose you
> decide to add further field-kind overrides to qemu. For example,
> currently Type 3 (chassis information, table required by SMBIOS spec)
> cannot be overridden field-wise.

The reason we expose blobs in SMBIOS is that blobs can be vendor
specific and there was a strong desire to allow vendor specific blobs to
be passed through.  There really is no choice but to expose a blob
interface for that.

> If a user is unhappy with her firmware's default Type 3 table, she's
> allowed to hex-edit a Type 3 blob, and pass it with "-smbios
> file=type3.blob" on the qemu command line. If you as developer want to
> provide her with more flexibility, you need to
> (i) patch qemu (of course), to recognize individual fields on the
> command line, and to populate fw_cfg with them,
> (ii) patch *every single firmware* supported by qemu to *look for* those
> (type=3,offset) field-kind config entries, and to effectuate them.
>
> If you pass down only verbatim tables, then (ii) falls away. You don't
> have to update SeaBIOS, nor OVMF.

OVMF is proprietary.  It is not "supported" by QEMU.  Firmware is a
fundamental part of our stack.  I'm not really convinced that
QEMU<->firmware is a GPL boundary because of how tightly the two are
linked.

Moving large chunks of firmware code into QEMU just to avoid solving
licensing issues is a non-starter with me.

> As a bonus, you don't have to care
> about versions: age-old firmware that only knows how to install verbatim
> tables will immediately benefit from qemu's new-fangled Type 3
> flexibility.

This presume that (1) you only care about exposing this information to
x86 guests (2) that the firmware will never have any need to patch these
blobs.

> Case in point: <https://bugzilla.redhat.com/show_bug.cgi?id=788142>.
> (Non-public bug, sorry about that. I think I can disclose that it is
> about supporting the "field kind" config entry for Type 3 tables.)

This information is *exactly* the kind of information that you want to
be exposed across architectures.  It makes lots of sense to model a
device to expose this type of information in a structure way.

This discussion comes down to two things I think: (a) our existing
firmware interface is pretty poor (b) we are duplicating work because of
firmware licensing.

We can fix (a) and there's lots of value in doing that in terms of
improving support for other architectures.  We've discussed (b) in other
threads and I've stated my opinion on the direction we need to take.

Regards,

Anthony Liguori

>
>
> For ACPI tables, especially for tables containing AML, multiply the cost
> of (ii) by twenty. Many more offsets to patch and vastly more
> dependencies via fw_cfg.
>
> Laszlo