On Mon, Apr 15, 2013 at 03:24:00PM +0200, Laszlo Ersek wrote: > On 04/15/13 13:25, Michael S. Tsirkin wrote: > > On Mon, Apr 15, 2013 at 12:37:37PM +0300, Michael S. Tsirkin wrote: > >> I wanted to help Laszlo completely move ACPI tables out of seabios in > >> time for 1.5, but had to put out some fires so didn't manage to do this > >> in time, and going offline for now. > >> > >> Meanwhile, here is some refactoring that I did complete. This does not > >> do much by itself, but works fine for me and will be helpful down the > >> road. > >> > >> This includes a couple of patches I posted previously > >> and another one on top. > >> > >> Pls consider for 1.5. > > > > Just two clarifications > > 1. This conflicts with Laszlo's MADT change. As such if Laszlo's patch is > > applied, > > this will need to be rebased on top. > > 2. This was lightly tested but there's a holiday here so I run out of > > time for testing. Will report in a couple of days. > > First, thank you very much for the help! > > Second, 2/3 breaks the binary format exported under the > FW_CFG_ACPI_TABLES key. The format is as follows: > > * number of tables in entire fw_cfg blob : uint16_t > for each table: > * ACPI payload size belonging to this table : uint16_t > * standard ACPI header > * ACPI table contents > > whereas 2/3 changes the format to: > > * zero-filled uint16_t (i) > for each table: > * uint16_t storing value 1 in LE order (ii) > * number of bytes in blob segment belonging to this table : uint16_t > * standard ACPI header > * ACPI table contents > > (ii) shouldn't exist in per-table storage, the increments should target > (i) actually. > > > Another note: 2/3 introduces two ways to spell "has_header" in the > leading comments. The first use is "&&!@has_header" (correct with @, but > whitespace missing). The second use is "&& !has_header" (from my last > series, also see "bloblen"), which has the whitespace OK, but misses the > at-sign @ customary in comments when referring to function paramteres. > > > Finally, regarding the purpose of this patch. It seems to be to prepend > a standard ACPI header to a "headerles" table. > > I think we can do without the reallocation. When preparing any ACPI > table, we know in advance the size of the ACPI header we're going to > need. So we can take it into account when allocating the buffer: fill in > the table body, and then call a function that puts the header at front, > into existing storage. See pc_acpi_install() in "[Qemu-devel] [qemu > PATCH 5/5] i386/pc: build ACPI MADT (APIC) for fw_cfg clients". > > I propose the following: > > (1) I'll resend my "[qemu PATCH 0/5] publish etc/acpi/APIC in fw_cfg" > series from last Monday, rebased to current (or then) master, with the > following changes suggested by Michael in private: > (1a) separating some stuff out into different files, > (1b) adding licensing bits related to SeaBIOS, > (1c) I'll try to consider 1/3 and 3/3 in this series that Paolo has > already picked up (may not match or be useful any longer), > > (2) Then, please review / apply that updated series of mine, > > (3) then please rebase 2/3 from here on top of (2). Again, I suggest > that acpi_table_add_hdr(), if any, fill in preallocated space; this is > consistent with updating the checksum too. > > (4) I *do* anticipate that my code in (2) is not / will not be flexible > enough for all tables. However, please split up functions / extract bits > / reorganize them only in a series that puts those new extracted > functions to use *at once* (probably near the end of said series). I > can't reason about "librarization" without actual use. > > I'll wait for Michael's answer before doing anything. > > Thanks > Laszlo
I'm fine with this. FYI, I'm offline for the rest of today and tomorrow. -- MST
