(adding Kevin)
On 04/03/13 22:05, Anthony Liguori wrote:
> Laszlo Ersek <ler...@redhat.com> writes:
>
>> On 03/21/13 00:23, Laszlo Ersek wrote:
>>> This series reworks the internals of the -acpitable command line option,
>>> and if that option is not specified, produces the APIC (MADT) table
>>> inside qemu, to be consumed over fw_cfg (alongside the DSDT).
>>
>>> Laszlo Ersek (11):
>>> strip some whitespace
>>> change element type from "char" to "unsigned char" in ACPI table data
>>> acpi_table_add(): report fatal errors through an internal Error
>>> object
>>> qapi schema: add AcpiTableOptions
>>> acpi_table_add(): accept QemuOpts and parse it with OptsVisitor
>>> acpi_table_add(): extract and reimplement internals
>>> like acpi_table_install(), acpi_table_add() should propagate Errors
>>> extract/unify the constant 0xfee00000 as APIC_DEFAULT_ADDRESS
>>> Introduce IO_APIC_DEFAULT_ADDRESS for 0xfec00000
>>> pc_acpi_init(): don't bail as soon as failing to find default DSDT
>>> i386/pc: build ACPI MADT for fw_cfg clients
>>
>> Any chance patches 01 to 09 could be considered? Esp. 06 which removes
>> an out-of-bounds access (an innocent-looking one, admittedly).
>>
>> I'm OK too if the series is dropped (patch 11 was the main motivation,
>> but the interface that it extends was deemed unsuitable going forward on
>> the seabios list). I'd just like to hear the maintainer with
>> jurisdiction say the NAK. ("Too expensive even to review for too little
>> gain" is a good reason.)
>
> The whole thing looks pretty nice to me.
That's awesome, thank you very much!
> I'll merge the full series in
> a day or so unless anyone objects.
For transparency's sake: Kevin, this is where you'd object to patch 11:
it adds an MADT to the existing fw_cfg blob, which, combined with an
older (=current) SeaBIOS, leads to a duplicated MADT; see also the blurb
in 00/11 which quotes that from
<http://www.seabios.org/pipermail/seabios/2013-March/005960.html>. (I'm
adding this paragraph because you can argue this better; I yet have to
go through your detailed design
<http://www.seabios.org/pipermail/seabios/2013-March/006020.html> once
more.)
Anyway, if you could tolerate such a duplicate MADT for a very short
time, I'd send a qemu patch that reworks pc_acpi_madt() (in 11/11), such
that it's final call is not made to acpi_table_install() -- ie.
appending the MADT to the existing fw_cfg ACPI blob --, but installs it
as a separate fw_cfg file as you're suggesting (/etc/acpi/APIC or so) --
the existing "-acpitable" switch would still benefit from 01-10, and I'd
salvage the meat of 11/11 as well. I'd then attempt to write a SeaBIOS
patch too for /etc/acpi/APIC. Would that be acceptable for you?
> This is an under maintained area of QEMU so it's very nice to see
> improvements!
>
> Reviewed-by: Anthony Liguori <aligu...@us.ibm.com>
Can't thank enough for your quick response, Anthony.
Laszlo
