On Tue, Apr 01, 2014 at 05:47:09PM +0200, Laszlo Ersek wrote:
On 04/01/14 16:39, Kevin O'Connor wrote:
On Tue, Apr 01, 2014 at 10:40:00AM +0200, Laszlo Ersek wrote:
On 03/31/14 22:18, Gabriel L. Somlo wrote:
The only sticking point remaining would be who gets to generate the Type 0 (BIOS Information) table and when, which is something QEMU should arguably NOT be doing on behalf of SeaBIOS (or OVMF, or ...).
Sorry I can follow this discussion only intermittently.
- I think OVMF would be fine with the Type 0 table as-is (it has a
default table and adheres to field-level patches). Full tables for other types would be welcome.
When SeaBIOS generates the smbios table, it creates a hardcoded type 0 sub-table. (See SeaBIOS code src/fw/smbios.c:smbios_init_type_0.) If OVMF is okay with the same hardcodes, then I'd suggest QEMU create the table the same way SeaBIOS currently does.
OK, smbios_init_type_0() is called by the add_struct() macro in smbios_setup(), but not if fw_cfg already holds a QEMU-supplied type 0 table.
When it is called, smbios_init_type_0() will look for individual fields supplied via fw_cfg, and only use its own defaults otherwise.
By default, QEMU will not set any type 0 fields, nor generate a type 0 table, which means that each specific BIOS gets to supply its own default type 0 table.
My patch set supplies various full tables via fw_cfg, but by default it still doesn't built a type 0. That only happens if either a binary table or default fields for type 0 are provided on the command line (same as the current behavior, except I'm always sending a full table rather than field-by-field).
bit 2 of the BIOS Characteristics Extension Byte 2 (7.1.2.2) is set, for "Enable Targeted Content Distribution".
In OVMF, the same byte has the following bits set:
Bit 3 -- UEFI Specification is supported. Bit 4 -- The SMBIOS table describes a virtual machine. (If this bit is not set, no inference can be made about the virtuality of the system.)
I have nothing against bit 2 (I didn't include it because I had no clue what it meant, but we can certainly set that bit down the road). Bit 3 would be wrong for SeaBIOS, and it would be wrong to leave clear for OVMF. Bit 4 would be wrong for SeaBIOS (as a static default), but is correct (and very nice, although not necessary) for OVMF.
I can add an extra command line option for type 0 defaults (e.g. "char_ext" but we can pick a better name):
"-smbios type=0,vendor='foo',version='x.y',char_ext=4"
... and make the user responsible for supplying the correct value if/when they wish to override the defaults. I'll do that in the v5 patch set I'm working on right now :)
As an aside, I think in the end it doesn't matter much if we supply individual field defaults or full tables for *optional* types such as type 0. I just like to generate full tables across the board to keep reusing the same code, rather than leave the individual-field logic in just for this one table type...
From the conversation so far, it seems to me that:
- type 0 is best left to the BIOS (user overrides via command line at their own risk)
- therefore, the maximum granularity of QEMU-generated elements should be full tables of a given type, and not the full SMBIOS blob at once (other mechanisms to allow the BIOS to insert its own type 0 welcome, but so far this seems the most straightforward one).
- this means the smbios structure header has to be left up to the BIOS
- the BIOS is then responsible for setting the smbios spec version (2.4 for SeaBIOS, 2.7.1 for OVMF).
On that last point, at least Linux seems to be OK with individual type tables having a higher version than the structure header; i.e., dmidecode works fine when e.g. the structure header says 2.4 but the type 4 cpu record is 2.6. I'll test on Windows and OS X as well, and post my results here.
My one remaining question is: how do we get the BIOS to *not* generate a certain table type that's being left out on purpose by QEMU ?
I'm talking here of type 20, which is no longer required as of spec v2.5, and which would unnecessarily complicate things if/when more than two E820_RAM memory areas are present...
Thanks much, --Gabriel