From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48484) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UNewG-0002tX-Tg for qemu-devel@nongnu.org; Thu, 04 Apr 2013 03:50:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UNewC-0005BE-Vn for qemu-devel@nongnu.org; Thu, 04 Apr 2013 03:50:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35543) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UNewC-0005B0-Nk for qemu-devel@nongnu.org; Thu, 04 Apr 2013 03:50:24 -0400 Message-ID: <515D313F.4010109@redhat.com> Date: Thu, 04 Apr 2013 09:52:31 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1363821803-3380-1-git-send-email-lersek@redhat.com> <515C5C73.1010404@redhat.com> <87hajncrcr.fsf@codemonkey.ws> In-Reply-To: <87hajncrcr.fsf@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 00/11] build ACPI MADT for fw_cfg clients List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori , Kevin O'Connor Cc: kraxel@redhat.com, qemu-devel@nongnu.org, mst@redhat.com (adding Kevin) On 04/03/13 22:05, Anthony Liguori wrote: > Laszlo Ersek 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 . (I'm adding this paragraph because you can argue this better; I yet have to go through your detailed design 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 Can't thank enough for your quick response, Anthony. Laszlo