Re: [Qemu-devel] [PATCH v3 0/7] publish etc/acpi/APIC in fw_cfg

[Michael S. Tsirkin]

On Tue, Apr 16, 2013 at 02:24:07AM +0200, Laszlo Ersek wrote:
> v1 blurb:
> 
>   This series exports the MADT (APIC) ACPI table under the new
>   "etc/acpi/APIC" fw_cfg file. I sought to follow the requirements set
>   forth in [1], the new table is only visible in the patched/patched
>   case. I cross-tested { master, patched } qemu with { master, patched }
>   seabios (the APIC, DSDT and RSDT tables) using guest acpidump and
>   dmesg.
> 
>   The -acpitable command line option is purposely ignored based on the
>   last paragraph of [2]; the user isn't supposed to pass APIC with that
>   option.
> 
>   checkpatch.pl complains a little but (as last time) it's a false
>   alarm.
> 
>   The series is bisectable.
> 
>   [1] http://thread.gmane.org/gmane.comp.emulators.qemu/202005/focus=202072
>   [2] http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/5960/focus=6008
> 
> v2:
> - address (1) in
>   <http://thread.gmane.org/gmane.comp.emulators.qemu/206146/focus=206195>:
> 
>   - rebase to Paolo's recent series
>     <http://thread.gmane.org/gmane.comp.emulators.qemu/206196/focus=206278>,
> 
>   - patch 2 is new, fixes style of function parameter references in a
>     comment [mst],
> 
>   - patch 6 is new, moves a macro definition [mst],
> 
>   - patch 7 should be structured more logically now, plus the commit
>     message has grown some bits related to licensing [mst].
>     acpi_table_fill_hdr() is kept distinct from the -acpitable switch's
>     implementation on purpose, similarly to pc_acpi_install() in v1.
> 
> - Tested APIC / DSDT / RSDT against contents saved from v1.
> 
> v3:
> - rebased to current master 24a6e7f4,
> - added Michael's S-o-b to 6/7 [Eric Blake],
> - added Dave Frodin's relicensing ACK to 7/7, originally sent to Michael
>   in private [Michael Tsirkin],
> - slightly reworded 7/7's commit message where it credits Michael's
>   prototype [Eric Blake].
> 
> Laszlo Ersek (6):
>   refer to FWCfgState explicitly
>   acpi_table_install(): fix funcparam formatting in leading comment
>   hw/acpi: extract standard table headers as a standalone structure
>   hw/acpi: export default ACPI headers using the type just introduced
>   hw/acpi: export acpi_checksum()
>   hw/i386: build ACPI MADT (APIC) for fw_cfg clients
> 
> Michael S. Tsirkin (1):
>   hw/i386/pc.c: move IO_APIC_DEFAULT_ADDRESS to include/hw/i386/apic.h

Looks good to me. For patches 1-6:
Acked-by: Michael S. Tsirkin <address@hidden>

For patch 7 - I'm not sure we want to ship qemu
that only supplies the MADT but not the other tables. So maybe we can
merge it disabling the functionality temporarily, until the rest of the
tables are converted?  A config flag will do.


>  hw/i386/acpi.h         |    9 +++
>  hw/i386/multiboot.h    |    4 +-
>  include/hw/acpi/acpi.h |   15 +++++
>  include/hw/i386/apic.h |    2 +
>  include/hw/i386/pc.h   |   19 +++---
>  include/hw/loader.h    |    3 +-
>  hw/acpi/core.c         |   91 ++++++++++++++-------------
>  hw/acpi/piix4.c        |    2 +-
>  hw/core/loader.c       |    2 +-
>  hw/i386/acpi.c         |  159 
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/i386/multiboot.c    |    2 +-
>  hw/i386/pc.c           |   42 +++++++++----
>  hw/i386/pc_piix.c      |    2 +-
>  hw/sparc/sun4m.c       |    6 +-
>  hw/sparc64/sun4u.c     |    2 +-
>  hw/i386/Makefile.objs  |    1 +
>  16 files changed, 284 insertions(+), 77 deletions(-)
>  create mode 100644 hw/i386/acpi.h
>  create mode 100644 hw/i386/acpi.c
>