On Mon, May 13, 2013 at 03:38:51PM -0500, Anthony Liguori wrote:
"Michael S. Tsirkin" email@example.com writes:
git trees, for those interested in testing this:
bios patchset will be posted tomorrow (need to clean up the commit log a bit).
I don't think it's a good idea to move BIOS functionality in QEMU.
We don't frequently add firmware or chipsets so it seems like we're optimizing for an uncommon scenario here.
Your objection has come as a surprise to me. Are you stating that you don't think QEMU should generate the ACPI tables and pass them through to SeaBIOS via fw_cfg?
I do think it would be worthwhile to do the above. If we look at the ACPI tables that are currently generated in SeaBIOS, nearly every field describes the hardware. (I've described this in some detail at: http://lists.gnu.org/archive/html/qemu-devel/2013-02/msg04417.html ) When generating ACPI tables, SeaBIOS can only guess at what the QEMU hardware is or it must receive the hardware details directly from QEMU.
When receiving hardware information from QEMU today, a one-off fw_cfg format is inevitably created. Currently there are seven different fw_cfg entries that control various "bios tables". (These are "irq0-override", "system-states", "numa-nodes", "acpi tables", "smbios tables", "pvpanic-port", and "max cpus".) Of these, only one has any use directly to SeaBIOS (max cpus) - the rest are purely pass through to one or more of the various guest visible bios tables.
The problem with generating custom fw_cfg structures to describe the hardware, is that it inevitably causes the creation of new (poorly documented) structures that serve only to pass the info so another program can create an industry standard structure with the info. A good example of this is the FW_CFG_NUMA entry. If one looks at the definition of this fw_cfg structure (see hw/i386/pc.c:bochs_bios_init) they will find a complex structure (it's actually two dynamically sized arrays of two different values with the size of the first dynamically sized array determined by the FW_CFG_MAX_CPUS fw_cfg entry). This undocumented fw_cfg structure serves no purpose other than to generate an ACPI SRAT table. Had QEMU just populated an SRAT table and put it into a fw_cfg entry then there would be a much better documented and understood interface. Had SeaBIOS wanted to know the numa information (it doesn't) it would be much easier for it to parse out the well documented ACPI SRAT table passed via fw_cfg then it would be for it to handle the cumbersome undocumented fw_cfg entry present today.
There have been several requests recently to add more enhanced and detailed ACPI tables to SeaBIOS that require new custom fw_cfg entries. I've been pushing back on them because I don't wish to invent new custom fw_cfg entries and propagate the code throughout QEMU and SeaBIOS, when the sole goal is to create an industry standard ACPI table. We're better off accepting that the goal is an ACPI table, that only QEMU has the knowledge to generate it, and that QEMU is the best place to generate it.
Hopefully I've misunderstood your objection or you will reconsider.