Patchwork [v4,7/7] hw/i386: build ACPI MADT (APIC) for fw_cfg clients

login
register
mail settings
Submitter Laszlo Ersek
Date April 18, 2013, 8:22 p.m.
Message ID <1366316544-1428-8-git-send-email-lersek@redhat.com>
Download mbox | patch
Permalink /patch/237730/
State New
Headers show

Comments

Laszlo Ersek - April 18, 2013, 8:22 p.m.
This patch reuses some code from SeaBIOS, which was originally under
LGPLv2 and then relicensed to GPLv3 or LGPLv3, in QEMU under GPLv2+. This
relicensing has been acked by all contributors that had contributed to the
code since the v2->v3 relicense. ACKs approving the v2+ relicensing are
listed below. The list might include ACKs from people not holding
copyright on any parts of the reused code, but it's better to err on the
side of caution and include them.

Affected SeaBIOS files (GPLv2+ license headers added)
<http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/5949>:

 src/acpi-dsdt-cpu-hotplug.dsl    |   15 +++++++++++++++
 src/acpi-dsdt-dbug.dsl           |   15 +++++++++++++++
 src/acpi-dsdt-hpet.dsl           |   15 +++++++++++++++
 src/acpi-dsdt-isa.dsl            |   15 +++++++++++++++
 src/acpi-dsdt-pci-crs.dsl        |   15 +++++++++++++++
 src/acpi.c                       |   14 +++++++++++++-
 src/acpi.h                       |   14 ++++++++++++++
 src/ssdt-misc.dsl                |   15 +++++++++++++++
 src/ssdt-pcihp.dsl               |   15 +++++++++++++++
 src/ssdt-proc.dsl                |   15 +++++++++++++++
 tools/acpi_extract.py            |   13 ++++++++++++-
 tools/acpi_extract_preprocess.py |   13 ++++++++++++-
 12 files changed, 171 insertions(+), 3 deletions(-)

Each one of the listed people agreed to the following:

> If you allow the use of your contribution in QEMU under the
> terms of GPLv2 or later as proposed by this patch,
> please respond to this mail including the line:
>
> Acked-by: Name <email address>

  Acked-by: Gerd Hoffmann <kraxel@redhat.com>
  Acked-by: Jan Kiszka <jan.kiszka@siemens.com>
  Acked-by: Jason Baron <jbaron@akamai.com>
  Acked-by: David Woodhouse <David.Woodhouse@intel.com>
  Acked-by: Gleb Natapov <gleb@redhat.com>
  Acked-by: Marcelo Tosatti <mtosatti@redhat.com>
  Acked-by: Dave Frodin <dave.frodin@se-eng.com>
  Acked-by: Paolo Bonzini <pbonzini@redhat.com>
  Acked-by: Kevin O'Connor <kevin@koconnor.net>
  Acked-by: Laszlo Ersek <lersek@redhat.com>
  Acked-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
  Acked-by: Isaku Yamahata <yamahata@valinux.co.jp>
  Acked-by: Magnus Christensson <magnus.christensson@intel.com>
  Acked-by: Hu Tao <hutao@cn.fujitsu.com>
  Acked-by: Eduardo Habkost <ehabkost@redhat.com>

The patch incorporates ideas/suggestions from Michael Tsirkin's prototype
code:
- "hw/i386/pc.c" is too big, create new file "hw/i386/acpi.c" with
  i386-specific ACPI table stuff,
- separate preparation of individual tables from their installation as
  fw_cfg files,
- install these fw_cfg files inside pc_memory_init(), which is shared by
  piix4/q35,
- add the above licensing-related block to the commit message.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 configure             |   12 ++++
 hw/i386/Makefile.objs |    1 +
 hw/i386/acpi.h        |    9 +++
 hw/i386/acpi.c        |  159 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/i386/pc.c          |   23 +++++++
 5 files changed, 204 insertions(+), 0 deletions(-)
 create mode 100644 hw/i386/acpi.h
 create mode 100644 hw/i386/acpi.c
Michael S. Tsirkin - April 18, 2013, 8:30 p.m.
On Thu, Apr 18, 2013 at 10:22:24PM +0200, Laszlo Ersek wrote:
> This patch reuses some code from SeaBIOS, which was originally under
> LGPLv2 and then relicensed to GPLv3 or LGPLv3, in QEMU under GPLv2+. This
> relicensing has been acked by all contributors that had contributed to the
> code since the v2->v3 relicense. ACKs approving the v2+ relicensing are
> listed below. The list might include ACKs from people not holding
> copyright on any parts of the reused code, but it's better to err on the
> side of caution and include them.
> 
> Affected SeaBIOS files (GPLv2+ license headers added)
> <http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/5949>:
> 
>  src/acpi-dsdt-cpu-hotplug.dsl    |   15 +++++++++++++++
>  src/acpi-dsdt-dbug.dsl           |   15 +++++++++++++++
>  src/acpi-dsdt-hpet.dsl           |   15 +++++++++++++++
>  src/acpi-dsdt-isa.dsl            |   15 +++++++++++++++
>  src/acpi-dsdt-pci-crs.dsl        |   15 +++++++++++++++
>  src/acpi.c                       |   14 +++++++++++++-
>  src/acpi.h                       |   14 ++++++++++++++
>  src/ssdt-misc.dsl                |   15 +++++++++++++++
>  src/ssdt-pcihp.dsl               |   15 +++++++++++++++
>  src/ssdt-proc.dsl                |   15 +++++++++++++++
>  tools/acpi_extract.py            |   13 ++++++++++++-
>  tools/acpi_extract_preprocess.py |   13 ++++++++++++-
>  12 files changed, 171 insertions(+), 3 deletions(-)
> 
> Each one of the listed people agreed to the following:
> 
> > If you allow the use of your contribution in QEMU under the
> > terms of GPLv2 or later as proposed by this patch,
> > please respond to this mail including the line:
> >
> > Acked-by: Name <email address>
> 
>   Acked-by: Gerd Hoffmann <kraxel@redhat.com>
>   Acked-by: Jan Kiszka <jan.kiszka@siemens.com>
>   Acked-by: Jason Baron <jbaron@akamai.com>
>   Acked-by: David Woodhouse <David.Woodhouse@intel.com>
>   Acked-by: Gleb Natapov <gleb@redhat.com>
>   Acked-by: Marcelo Tosatti <mtosatti@redhat.com>
>   Acked-by: Dave Frodin <dave.frodin@se-eng.com>
>   Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>   Acked-by: Kevin O'Connor <kevin@koconnor.net>
>   Acked-by: Laszlo Ersek <lersek@redhat.com>
>   Acked-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
>   Acked-by: Isaku Yamahata <yamahata@valinux.co.jp>
>   Acked-by: Magnus Christensson <magnus.christensson@intel.com>
>   Acked-by: Hu Tao <hutao@cn.fujitsu.com>
>   Acked-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> The patch incorporates ideas/suggestions from Michael Tsirkin's prototype
> code:
> - "hw/i386/pc.c" is too big, create new file "hw/i386/acpi.c" with
>   i386-specific ACPI table stuff,
> - separate preparation of individual tables from their installation as
>   fw_cfg files,
> - install these fw_cfg files inside pc_memory_init(), which is shared by
>   piix4/q35,
> - add the above licensing-related block to the commit message.
> 
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  configure             |   12 ++++
>  hw/i386/Makefile.objs |    1 +
>  hw/i386/acpi.h        |    9 +++
>  hw/i386/acpi.c        |  159 +++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/i386/pc.c          |   23 +++++++
>  5 files changed, 204 insertions(+), 0 deletions(-)
>  create mode 100644 hw/i386/acpi.h
>  create mode 100644 hw/i386/acpi.c
> 
> diff --git a/configure b/configure
> index ed49f91..45a5f55 100755
> --- a/configure
> +++ b/configure
> @@ -241,6 +241,7 @@ gtk=""
>  gtkabi="2.0"
>  tpm="no"
>  libssh2=""
> +dynamic_acpi="no"
>  
>  # parse CC options first
>  for opt do
> @@ -928,6 +929,10 @@ for opt do
>    ;;
>    --enable-libssh2) libssh2="yes"
>    ;;
> +  --disable-dynamic-acpi) dynamic_acpi="no"
> +  ;;
> +  --enable-dynamic-acpi) dynamic_acpi="yes"
> +  ;;
>    *) echo "ERROR: unknown option $opt"; show_help="yes"
>    ;;
>    esac
> @@ -1195,6 +1200,8 @@ echo "  --gcov=GCOV              use specified gcov [$gcov_tool]"
>  echo "  --enable-tpm             enable TPM support"
>  echo "  --disable-libssh2        disable ssh block device support"
>  echo "  --enable-libssh2         enable ssh block device support"
> +echo "  --disable-dynamic-acpi   disable dynamic ACPI table generation (default)"
> +echo "  --enable-dynamic-acpi    enable dynamic ACPI table generation (work in progress)"
>  echo ""
>  echo "NOTE: The object files are built at the place where configure is launched"
>  exit 1
> @@ -3573,6 +3580,7 @@ echo "gcov enabled      $gcov"
>  echo "TPM support       $tpm"
>  echo "libssh2 support   $libssh2"
>  echo "TPM passthrough   $tpm_passthrough"
> +echo "dynamic ACPI tables $dynamic_acpi"
>  
>  if test "$sdl_too_old" = "yes"; then
>  echo "-> Your SDL version is too old - please upgrade to have SDL support"
> @@ -3958,6 +3966,10 @@ if test "$virtio_blk_data_plane" = "yes" ; then
>    echo 'CONFIG_VIRTIO_BLK_DATA_PLANE=$(CONFIG_VIRTIO)' >> $config_host_mak
>  fi
>  
> +if test "$dynamic_acpi" = "yes"; then
> +  echo "CONFIG_DYN_ACPI=y" >> $config_host_mak
> +fi
> +
>  # USB host support
>  case "$usb" in
>  linux)
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index 205d22e..8429d52 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -1,6 +1,7 @@
>  obj-$(CONFIG_KVM) += kvm/
>  obj-y += multiboot.o smbios.o
>  obj-y += pc.o pc_piix.o pc_q35.o
> +obj-$(CONFIG_DYN_ACPI) += acpi.o
>  obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
>  
>  obj-y += kvmvapic.o
> diff --git a/hw/i386/acpi.h b/hw/i386/acpi.h
> new file mode 100644
> index 0000000..94f9ad3
> --- /dev/null
> +++ b/hw/i386/acpi.h
> @@ -0,0 +1,9 @@
> +#ifndef QEMU_HW_I386_ACPI_H
> +#define QEMU_HW_I386_ACPI_H
> +
> +#include <stddef.h>
> +
> +void acpi_build_madt(unsigned char **out_blob, size_t *out_blob_size,
> +                     unsigned num_lapic);
> +
> +#endif
> diff --git a/hw/i386/acpi.c b/hw/i386/acpi.c
> new file mode 100644
> index 0000000..179cdf5
> --- /dev/null
> +++ b/hw/i386/acpi.c
> @@ -0,0 +1,159 @@
> +/*
> + * Copyright (c) 2013 Red Hat Inc.
> + * Copyright (C) 2008-2010  Kevin O'Connor <kevin@koconnor.net>
> + * Copyright (C) 2006 Fabrice Bellard
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "hw/acpi/acpi.h"
> +#include "hw/i386/acpi.h"
> +#include "kvm_i386.h"
> +#include "sysemu/sysemu.h"
> +
> +static void acpi_table_fill_hdr(AcpiTableStdHdr *std_hdr, size_t blob_size,
> +                                const char *sig)
> +{
> +    g_assert(blob_size >= sizeof *std_hdr);
> +
> +    *std_hdr = acpi_dfl_hdr;
> +    strncpy(std_hdr->sig, sig, sizeof std_hdr->sig);
> +    strncpy(std_hdr->oem_id, "QEMU  ", sizeof std_hdr->oem_id);
> +    strncpy(std_hdr->oem_table_id + 4, sig, sizeof std_hdr->oem_table_id - 4);
> +    std_hdr->length = cpu_to_le32(blob_size);
> +    std_hdr->checksum = acpi_checksum((uint8_t *)std_hdr, blob_size);
> +}
> +
> +void acpi_build_madt(unsigned char **out_blob, size_t *out_blob_size,
> +                     unsigned num_lapic)
> +{
> +    typedef struct {
> +        uint8_t    type;
> +        uint8_t    length;
> +    } QEMU_PACKED AcpiSubHdr;
> +
> +    AcpiTableStdHdr *std_hdr;
> +    struct {
> +        uint32_t   lapic_addr; /* Local Interrupt Controller Address */
> +        uint32_t   flags;      /* Multiple APIC flags */
> +    } QEMU_PACKED *madt;
> +    struct {
> +        AcpiSubHdr hdr;
> +        uint8_t    processor_id; /* ACPI Processor ID */
> +        uint8_t    apic_id;      /* APIC ID */
> +        uint32_t   flags;        /* LOcal APIC flags */
> +    } QEMU_PACKED *lapic;
> +    struct {
> +        AcpiSubHdr hdr;
> +        uint8_t    io_apic_id;   /* The I/O APIC's ID */
> +        uint8_t    reserved;     /* constant zero */
> +        uint32_t   io_apic_addr; /* 32-bit physical address to access */
> +        uint32_t   gsi_base;     /* interrupt inputs start here */
> +    } QEMU_PACKED *io_apic;
> +    struct {
> +        AcpiSubHdr hdr;
> +        uint8_t    bus;    /* constant zero: ISA */
> +        uint8_t    source; /* this bus-relative interrupt source... */
> +        uint32_t   gsi;    /* ... will signal this global system interrupt */
> +        uint16_t   flags;  /* MPS INTI Flags: Polarity, Trigger Mode */
> +    } QEMU_PACKED *int_src_ovr;
> +    struct {
> +        AcpiSubHdr hdr;
> +        uint8_t    processor_id; /* ACPI Processor ID */
> +        uint16_t   flags;        /* MPS INTI Flags: Polarity, Trigger Mode */
> +        uint8_t    lint;         /* LAPIC interrupt input for NMI */
> +    } QEMU_PACKED *lapic_nmi;
> +
> +    static const uint8_t pci_isa_irq[] = { 5, 9, 10, 11 };
> +
> +    unsigned num_int_src_ovr, i;
> +    size_t blob_size;
> +    char unsigned *blob;
> +
> +    num_int_src_ovr = sizeof pci_isa_irq + kvm_allows_irq0_override();
> +
> +    blob_size = (sizeof *std_hdr)     * 1               +
> +                (sizeof *madt)        * 1               +
> +                (sizeof *lapic)       * num_lapic       +
> +                (sizeof *io_apic)     * 1               +
> +                (sizeof *int_src_ovr) * num_int_src_ovr +
> +                (sizeof *lapic_nmi)   * 1;
> +    blob      = g_malloc(blob_size);
> +
> +    std_hdr     = (void *)blob;
> +    madt        = (void *)(std_hdr     + 1              );
> +    lapic       = (void *)(madt        + 1              );
> +    io_apic     = (void *)(lapic       + num_lapic      );
> +    int_src_ovr = (void *)(io_apic     + 1              );
> +    lapic_nmi   = (void *)(int_src_ovr + num_int_src_ovr);
> +
> +    madt->lapic_addr = cpu_to_le32(APIC_DEFAULT_ADDRESS);
> +    madt->flags      = cpu_to_le32(1); /* PCAT_COMPAT */
> +
> +    /* create a Local APIC structure for each possible APIC ID */
> +    for (i = 0; i < num_lapic; ++i) {
> +        lapic[i].hdr.type     = 0; /* Processor Local APIC */
> +        lapic[i].hdr.length   = sizeof *lapic;
> +        lapic[i].processor_id = i;
> +        lapic[i].apic_id      = i;
> +        lapic[i].flags        = cpu_to_le32(0); /* disabled */
> +    }
> +    /* enable the CPUs with a CPU index in the [0..smp_cpus-1] range */
> +    for (i = 0; i < smp_cpus; ++i) {
> +        lapic[x86_cpu_apic_id_from_index(i)].flags = cpu_to_le32(1);
> +    }
> +
> +    io_apic->hdr.type     = 1; /* I/O APIC */
> +    io_apic->hdr.length   = sizeof *io_apic;
> +    io_apic->io_apic_id   = 0;
> +    io_apic->reserved     = 0;
> +    io_apic->io_apic_addr = cpu_to_le32(IO_APIC_DEFAULT_ADDRESS);
> +    io_apic->gsi_base     = cpu_to_le32(0);
> +
> +    for (i = 0; i < sizeof pci_isa_irq; ++i) {
> +        int_src_ovr[i].hdr.type   = 2; /* Interrupt Source Override */
> +        int_src_ovr[i].hdr.length = sizeof *int_src_ovr;
> +        int_src_ovr[i].bus        = 0;
> +        int_src_ovr[i].source     = pci_isa_irq[i];
> +        int_src_ovr[i].gsi        = cpu_to_le32(pci_isa_irq[i]);
> +        int_src_ovr[i].flags      = cpu_to_le16(0xd);
> +                                    /* active high, level-triggered */
> +    }
> +    if (kvm_allows_irq0_override()) {
> +        int_src_ovr[i].hdr.type   = 2; /* Interrupt Source Override */
> +        int_src_ovr[i].hdr.length = sizeof *int_src_ovr;
> +        int_src_ovr[i].bus        = 0;
> +        int_src_ovr[i].source     = 0;
> +        int_src_ovr[i].gsi        = cpu_to_le32(2);
> +        int_src_ovr[i].flags      = cpu_to_le16(0); /* conforms to bus spec */
> +    }
> +
> +    lapic_nmi->hdr.type     = 4; /* Local APIC NMI */
> +    lapic_nmi->hdr.length   = sizeof *lapic_nmi;
> +    lapic_nmi->processor_id = 0xff; /* all processors */
> +    lapic_nmi->flags        = cpu_to_le16(0); /* conforms to bus spec */
> +    lapic_nmi->lint         = 1; /* NMI connected to LAPIC input LINT1 */
> +
> +    acpi_table_fill_hdr(std_hdr, blob_size, "APIC");
> +    *out_blob = blob;
> +    *out_blob_size = blob_size;
> +}
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 8727489..15c6284 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -54,6 +54,10 @@
>  #include "qemu/config-file.h"
>  #include "hw/acpi/acpi.h"
>  
> +#ifdef CONFIG_DYN_ACPI
> +#  include "hw/i386/acpi.h"
> +#endif
> +
>  /* debug PC/ISA interrupts */
>  //#define DEBUG_IRQ
>  
> @@ -922,6 +926,21 @@ void pc_acpi_init(const char *default_dsdt)
>      }
>  }
>  
> +#ifdef CONFIG_DYN_ACPI
> +static void pc_acpi_madt(FWCfgState *fw_cfg)
> +{
> +    unsigned num_lapic;
> +    char unsigned *blob;
> +    size_t blob_size;
> +
> +    /* see note on FW_CFG_MAX_CPUS in bochs_bios_init() */
> +    num_lapic = pc_apic_id_limit(max_cpus);
> +
> +    acpi_build_madt(&blob, &blob_size, num_lapic);
> +    fw_cfg_add_file(fw_cfg, "etc/acpi/APIC", blob, blob_size);
> +}
> +#endif
> +
>  FWCfgState *pc_memory_init(MemoryRegion *system_memory,
>                             const char *kernel_filename,
>                             const char *kernel_cmdline,
> @@ -974,6 +993,10 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
>      fw_cfg = bochs_bios_init();
>      rom_set_fw(fw_cfg);

Let's cut out the ifdefs, just use an if below. Compiler is smart enough
to do it right.

> +#ifdef CONFIG_DYN_ACPI
> +    pc_acpi_madt(fw_cfg);
> +#endif
> +
>      if (linux_boot) {
>          load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size);
>      }
> -- 
> 1.7.1
>
Laszlo Ersek - April 19, 2013, 10:58 a.m.
On 04/18/13 22:30, Michael S. Tsirkin wrote:
> On Thu, Apr 18, 2013 at 10:22:24PM +0200, Laszlo Ersek wrote:
>> This patch reuses some code from SeaBIOS, which was originally under
>> LGPLv2 and then relicensed to GPLv3 or LGPLv3, in QEMU under GPLv2+. This
>> relicensing has been acked by all contributors that had contributed to the
>> code since the v2->v3 relicense. ACKs approving the v2+ relicensing are
>> listed below. The list might include ACKs from people not holding
>> copyright on any parts of the reused code, but it's better to err on the
>> side of caution and include them.
>>
>> Affected SeaBIOS files (GPLv2+ license headers added)
>> <http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/5949>:
>>
>>  src/acpi-dsdt-cpu-hotplug.dsl    |   15 +++++++++++++++
>>  src/acpi-dsdt-dbug.dsl           |   15 +++++++++++++++
>>  src/acpi-dsdt-hpet.dsl           |   15 +++++++++++++++
>>  src/acpi-dsdt-isa.dsl            |   15 +++++++++++++++
>>  src/acpi-dsdt-pci-crs.dsl        |   15 +++++++++++++++
>>  src/acpi.c                       |   14 +++++++++++++-
>>  src/acpi.h                       |   14 ++++++++++++++
>>  src/ssdt-misc.dsl                |   15 +++++++++++++++
>>  src/ssdt-pcihp.dsl               |   15 +++++++++++++++
>>  src/ssdt-proc.dsl                |   15 +++++++++++++++
>>  tools/acpi_extract.py            |   13 ++++++++++++-
>>  tools/acpi_extract_preprocess.py |   13 ++++++++++++-
>>  12 files changed, 171 insertions(+), 3 deletions(-)
>>
>> Each one of the listed people agreed to the following:
>>
>>> If you allow the use of your contribution in QEMU under the
>>> terms of GPLv2 or later as proposed by this patch,
>>> please respond to this mail including the line:
>>>
>>> Acked-by: Name <email address>
>>
>>   Acked-by: Gerd Hoffmann <kraxel@redhat.com>
>>   Acked-by: Jan Kiszka <jan.kiszka@siemens.com>
>>   Acked-by: Jason Baron <jbaron@akamai.com>
>>   Acked-by: David Woodhouse <David.Woodhouse@intel.com>
>>   Acked-by: Gleb Natapov <gleb@redhat.com>
>>   Acked-by: Marcelo Tosatti <mtosatti@redhat.com>
>>   Acked-by: Dave Frodin <dave.frodin@se-eng.com>
>>   Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>>   Acked-by: Kevin O'Connor <kevin@koconnor.net>
>>   Acked-by: Laszlo Ersek <lersek@redhat.com>
>>   Acked-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
>>   Acked-by: Isaku Yamahata <yamahata@valinux.co.jp>
>>   Acked-by: Magnus Christensson <magnus.christensson@intel.com>
>>   Acked-by: Hu Tao <hutao@cn.fujitsu.com>
>>   Acked-by: Eduardo Habkost <ehabkost@redhat.com>
>>
>> The patch incorporates ideas/suggestions from Michael Tsirkin's prototype
>> code:
>> - "hw/i386/pc.c" is too big, create new file "hw/i386/acpi.c" with
>>   i386-specific ACPI table stuff,
>> - separate preparation of individual tables from their installation as
>>   fw_cfg files,
>> - install these fw_cfg files inside pc_memory_init(), which is shared by
>>   piix4/q35,
>> - add the above licensing-related block to the commit message.
>>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  configure             |   12 ++++
>>  hw/i386/Makefile.objs |    1 +
>>  hw/i386/acpi.h        |    9 +++
>>  hw/i386/acpi.c        |  159 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  hw/i386/pc.c          |   23 +++++++
>>  5 files changed, 204 insertions(+), 0 deletions(-)
>>  create mode 100644 hw/i386/acpi.h
>>  create mode 100644 hw/i386/acpi.c
>>
>> diff --git a/configure b/configure
>> index ed49f91..45a5f55 100755
>> --- a/configure
>> +++ b/configure
>> @@ -241,6 +241,7 @@ gtk=""
>>  gtkabi="2.0"
>>  tpm="no"
>>  libssh2=""
>> +dynamic_acpi="no"
>>  
>>  # parse CC options first
>>  for opt do
>> @@ -928,6 +929,10 @@ for opt do
>>    ;;
>>    --enable-libssh2) libssh2="yes"
>>    ;;
>> +  --disable-dynamic-acpi) dynamic_acpi="no"
>> +  ;;
>> +  --enable-dynamic-acpi) dynamic_acpi="yes"
>> +  ;;
>>    *) echo "ERROR: unknown option $opt"; show_help="yes"
>>    ;;
>>    esac
>> @@ -1195,6 +1200,8 @@ echo "  --gcov=GCOV              use specified gcov [$gcov_tool]"
>>  echo "  --enable-tpm             enable TPM support"
>>  echo "  --disable-libssh2        disable ssh block device support"
>>  echo "  --enable-libssh2         enable ssh block device support"
>> +echo "  --disable-dynamic-acpi   disable dynamic ACPI table generation (default)"
>> +echo "  --enable-dynamic-acpi    enable dynamic ACPI table generation (work in progress)"
>>  echo ""
>>  echo "NOTE: The object files are built at the place where configure is launched"
>>  exit 1
>> @@ -3573,6 +3580,7 @@ echo "gcov enabled      $gcov"
>>  echo "TPM support       $tpm"
>>  echo "libssh2 support   $libssh2"
>>  echo "TPM passthrough   $tpm_passthrough"
>> +echo "dynamic ACPI tables $dynamic_acpi"
>>  
>>  if test "$sdl_too_old" = "yes"; then
>>  echo "-> Your SDL version is too old - please upgrade to have SDL support"
>> @@ -3958,6 +3966,10 @@ if test "$virtio_blk_data_plane" = "yes" ; then
>>    echo 'CONFIG_VIRTIO_BLK_DATA_PLANE=$(CONFIG_VIRTIO)' >> $config_host_mak
>>  fi
>>  
>> +if test "$dynamic_acpi" = "yes"; then
>> +  echo "CONFIG_DYN_ACPI=y" >> $config_host_mak
>> +fi
>> +
>>  # USB host support
>>  case "$usb" in
>>  linux)
>> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
>> index 205d22e..8429d52 100644
>> --- a/hw/i386/Makefile.objs
>> +++ b/hw/i386/Makefile.objs
>> @@ -1,6 +1,7 @@
>>  obj-$(CONFIG_KVM) += kvm/
>>  obj-y += multiboot.o smbios.o
>>  obj-y += pc.o pc_piix.o pc_q35.o
>> +obj-$(CONFIG_DYN_ACPI) += acpi.o
>>  obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
>>  
>>  obj-y += kvmvapic.o
>> diff --git a/hw/i386/acpi.h b/hw/i386/acpi.h
>> new file mode 100644
>> index 0000000..94f9ad3
>> --- /dev/null
>> +++ b/hw/i386/acpi.h
>> @@ -0,0 +1,9 @@
>> +#ifndef QEMU_HW_I386_ACPI_H
>> +#define QEMU_HW_I386_ACPI_H
>> +
>> +#include <stddef.h>
>> +
>> +void acpi_build_madt(unsigned char **out_blob, size_t *out_blob_size,
>> +                     unsigned num_lapic);
>> +
>> +#endif
>> diff --git a/hw/i386/acpi.c b/hw/i386/acpi.c
>> new file mode 100644
>> index 0000000..179cdf5
>> --- /dev/null
>> +++ b/hw/i386/acpi.c
>> @@ -0,0 +1,159 @@
>> +/*
>> + * Copyright (c) 2013 Red Hat Inc.
>> + * Copyright (C) 2008-2010  Kevin O'Connor <kevin@koconnor.net>
>> + * Copyright (C) 2006 Fabrice Bellard
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along
>> + * with this program; if not, see <http://www.gnu.org/licenses/>.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "hw/acpi/acpi.h"
>> +#include "hw/i386/acpi.h"
>> +#include "kvm_i386.h"
>> +#include "sysemu/sysemu.h"
>> +
>> +static void acpi_table_fill_hdr(AcpiTableStdHdr *std_hdr, size_t blob_size,
>> +                                const char *sig)
>> +{
>> +    g_assert(blob_size >= sizeof *std_hdr);
>> +
>> +    *std_hdr = acpi_dfl_hdr;
>> +    strncpy(std_hdr->sig, sig, sizeof std_hdr->sig);
>> +    strncpy(std_hdr->oem_id, "QEMU  ", sizeof std_hdr->oem_id);
>> +    strncpy(std_hdr->oem_table_id + 4, sig, sizeof std_hdr->oem_table_id - 4);
>> +    std_hdr->length = cpu_to_le32(blob_size);
>> +    std_hdr->checksum = acpi_checksum((uint8_t *)std_hdr, blob_size);
>> +}
>> +
>> +void acpi_build_madt(unsigned char **out_blob, size_t *out_blob_size,
>> +                     unsigned num_lapic)
>> +{
>> +    typedef struct {
>> +        uint8_t    type;
>> +        uint8_t    length;
>> +    } QEMU_PACKED AcpiSubHdr;
>> +
>> +    AcpiTableStdHdr *std_hdr;
>> +    struct {
>> +        uint32_t   lapic_addr; /* Local Interrupt Controller Address */
>> +        uint32_t   flags;      /* Multiple APIC flags */
>> +    } QEMU_PACKED *madt;
>> +    struct {
>> +        AcpiSubHdr hdr;
>> +        uint8_t    processor_id; /* ACPI Processor ID */
>> +        uint8_t    apic_id;      /* APIC ID */
>> +        uint32_t   flags;        /* LOcal APIC flags */
>> +    } QEMU_PACKED *lapic;
>> +    struct {
>> +        AcpiSubHdr hdr;
>> +        uint8_t    io_apic_id;   /* The I/O APIC's ID */
>> +        uint8_t    reserved;     /* constant zero */
>> +        uint32_t   io_apic_addr; /* 32-bit physical address to access */
>> +        uint32_t   gsi_base;     /* interrupt inputs start here */
>> +    } QEMU_PACKED *io_apic;
>> +    struct {
>> +        AcpiSubHdr hdr;
>> +        uint8_t    bus;    /* constant zero: ISA */
>> +        uint8_t    source; /* this bus-relative interrupt source... */
>> +        uint32_t   gsi;    /* ... will signal this global system interrupt */
>> +        uint16_t   flags;  /* MPS INTI Flags: Polarity, Trigger Mode */
>> +    } QEMU_PACKED *int_src_ovr;
>> +    struct {
>> +        AcpiSubHdr hdr;
>> +        uint8_t    processor_id; /* ACPI Processor ID */
>> +        uint16_t   flags;        /* MPS INTI Flags: Polarity, Trigger Mode */
>> +        uint8_t    lint;         /* LAPIC interrupt input for NMI */
>> +    } QEMU_PACKED *lapic_nmi;
>> +
>> +    static const uint8_t pci_isa_irq[] = { 5, 9, 10, 11 };
>> +
>> +    unsigned num_int_src_ovr, i;
>> +    size_t blob_size;
>> +    char unsigned *blob;
>> +
>> +    num_int_src_ovr = sizeof pci_isa_irq + kvm_allows_irq0_override();
>> +
>> +    blob_size = (sizeof *std_hdr)     * 1               +
>> +                (sizeof *madt)        * 1               +
>> +                (sizeof *lapic)       * num_lapic       +
>> +                (sizeof *io_apic)     * 1               +
>> +                (sizeof *int_src_ovr) * num_int_src_ovr +
>> +                (sizeof *lapic_nmi)   * 1;
>> +    blob      = g_malloc(blob_size);
>> +
>> +    std_hdr     = (void *)blob;
>> +    madt        = (void *)(std_hdr     + 1              );
>> +    lapic       = (void *)(madt        + 1              );
>> +    io_apic     = (void *)(lapic       + num_lapic      );
>> +    int_src_ovr = (void *)(io_apic     + 1              );
>> +    lapic_nmi   = (void *)(int_src_ovr + num_int_src_ovr);
>> +
>> +    madt->lapic_addr = cpu_to_le32(APIC_DEFAULT_ADDRESS);
>> +    madt->flags      = cpu_to_le32(1); /* PCAT_COMPAT */
>> +
>> +    /* create a Local APIC structure for each possible APIC ID */
>> +    for (i = 0; i < num_lapic; ++i) {
>> +        lapic[i].hdr.type     = 0; /* Processor Local APIC */
>> +        lapic[i].hdr.length   = sizeof *lapic;
>> +        lapic[i].processor_id = i;
>> +        lapic[i].apic_id      = i;
>> +        lapic[i].flags        = cpu_to_le32(0); /* disabled */
>> +    }
>> +    /* enable the CPUs with a CPU index in the [0..smp_cpus-1] range */
>> +    for (i = 0; i < smp_cpus; ++i) {
>> +        lapic[x86_cpu_apic_id_from_index(i)].flags = cpu_to_le32(1);
>> +    }
>> +
>> +    io_apic->hdr.type     = 1; /* I/O APIC */
>> +    io_apic->hdr.length   = sizeof *io_apic;
>> +    io_apic->io_apic_id   = 0;
>> +    io_apic->reserved     = 0;
>> +    io_apic->io_apic_addr = cpu_to_le32(IO_APIC_DEFAULT_ADDRESS);
>> +    io_apic->gsi_base     = cpu_to_le32(0);
>> +
>> +    for (i = 0; i < sizeof pci_isa_irq; ++i) {
>> +        int_src_ovr[i].hdr.type   = 2; /* Interrupt Source Override */
>> +        int_src_ovr[i].hdr.length = sizeof *int_src_ovr;
>> +        int_src_ovr[i].bus        = 0;
>> +        int_src_ovr[i].source     = pci_isa_irq[i];
>> +        int_src_ovr[i].gsi        = cpu_to_le32(pci_isa_irq[i]);
>> +        int_src_ovr[i].flags      = cpu_to_le16(0xd);
>> +                                    /* active high, level-triggered */
>> +    }
>> +    if (kvm_allows_irq0_override()) {
>> +        int_src_ovr[i].hdr.type   = 2; /* Interrupt Source Override */
>> +        int_src_ovr[i].hdr.length = sizeof *int_src_ovr;
>> +        int_src_ovr[i].bus        = 0;
>> +        int_src_ovr[i].source     = 0;
>> +        int_src_ovr[i].gsi        = cpu_to_le32(2);
>> +        int_src_ovr[i].flags      = cpu_to_le16(0); /* conforms to bus spec */
>> +    }
>> +
>> +    lapic_nmi->hdr.type     = 4; /* Local APIC NMI */
>> +    lapic_nmi->hdr.length   = sizeof *lapic_nmi;
>> +    lapic_nmi->processor_id = 0xff; /* all processors */
>> +    lapic_nmi->flags        = cpu_to_le16(0); /* conforms to bus spec */
>> +    lapic_nmi->lint         = 1; /* NMI connected to LAPIC input LINT1 */
>> +
>> +    acpi_table_fill_hdr(std_hdr, blob_size, "APIC");
>> +    *out_blob = blob;
>> +    *out_blob_size = blob_size;
>> +}
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 8727489..15c6284 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -54,6 +54,10 @@
>>  #include "qemu/config-file.h"
>>  #include "hw/acpi/acpi.h"
>>  
>> +#ifdef CONFIG_DYN_ACPI
>> +#  include "hw/i386/acpi.h"
>> +#endif
>> +
>>  /* debug PC/ISA interrupts */
>>  //#define DEBUG_IRQ
>>  
>> @@ -922,6 +926,21 @@ void pc_acpi_init(const char *default_dsdt)
>>      }
>>  }
>>  
>> +#ifdef CONFIG_DYN_ACPI
>> +static void pc_acpi_madt(FWCfgState *fw_cfg)
>> +{
>> +    unsigned num_lapic;
>> +    char unsigned *blob;
>> +    size_t blob_size;
>> +
>> +    /* see note on FW_CFG_MAX_CPUS in bochs_bios_init() */
>> +    num_lapic = pc_apic_id_limit(max_cpus);
>> +
>> +    acpi_build_madt(&blob, &blob_size, num_lapic);
>> +    fw_cfg_add_file(fw_cfg, "etc/acpi/APIC", blob, blob_size);
>> +}
>> +#endif
>> +
>>  FWCfgState *pc_memory_init(MemoryRegion *system_memory,
>>                             const char *kernel_filename,
>>                             const char *kernel_cmdline,
>> @@ -974,6 +993,10 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
>>      fw_cfg = bochs_bios_init();
>>      rom_set_fw(fw_cfg);
> 
> Let's cut out the ifdefs, just use an if below. Compiler is smart enough
> to do it right.

I considered that, because it's how SeaBIOS does it.

There are several "depths" to do this; for example, I could even build
hw/i386/acpi.o with the switch being off, and pass the object file to
the linker (ie. no variable substitution in Makefile.objs) and let the
linker omit it when it finds no references.

However, I obviously checked how existent parts of the code do it,
specifically CONFIG_XEN, and I followed that manner. See
"hw/i386/pc_piix.c" and "arch_init.c". There's not one CONFIG_XXX
feature test macro in the qemu source (that I can find) that is examined
with the "if" statement.

IOW with all due respect I disagree.

Thanks,
Laszlo

> 
>> +#ifdef CONFIG_DYN_ACPI
>> +    pc_acpi_madt(fw_cfg);
>> +#endif
>> +
>>      if (linux_boot) {
>>          load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size);
>>      }
>> -- 
>> 1.7.1
>>
Michael S. Tsirkin - April 24, 2013, 9:42 a.m.
On Fri, Apr 19, 2013 at 12:58:03PM +0200, Laszlo Ersek wrote:
> On 04/18/13 22:30, Michael S. Tsirkin wrote:
> > On Thu, Apr 18, 2013 at 10:22:24PM +0200, Laszlo Ersek wrote:
> >> This patch reuses some code from SeaBIOS, which was originally under
> >> LGPLv2 and then relicensed to GPLv3 or LGPLv3, in QEMU under GPLv2+. This
> >> relicensing has been acked by all contributors that had contributed to the
> >> code since the v2->v3 relicense. ACKs approving the v2+ relicensing are
> >> listed below. The list might include ACKs from people not holding
> >> copyright on any parts of the reused code, but it's better to err on the
> >> side of caution and include them.
> >>
> >> Affected SeaBIOS files (GPLv2+ license headers added)
> >> <http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/5949>:
> >>
> >>  src/acpi-dsdt-cpu-hotplug.dsl    |   15 +++++++++++++++
> >>  src/acpi-dsdt-dbug.dsl           |   15 +++++++++++++++
> >>  src/acpi-dsdt-hpet.dsl           |   15 +++++++++++++++
> >>  src/acpi-dsdt-isa.dsl            |   15 +++++++++++++++
> >>  src/acpi-dsdt-pci-crs.dsl        |   15 +++++++++++++++
> >>  src/acpi.c                       |   14 +++++++++++++-
> >>  src/acpi.h                       |   14 ++++++++++++++
> >>  src/ssdt-misc.dsl                |   15 +++++++++++++++
> >>  src/ssdt-pcihp.dsl               |   15 +++++++++++++++
> >>  src/ssdt-proc.dsl                |   15 +++++++++++++++
> >>  tools/acpi_extract.py            |   13 ++++++++++++-
> >>  tools/acpi_extract_preprocess.py |   13 ++++++++++++-
> >>  12 files changed, 171 insertions(+), 3 deletions(-)
> >>
> >> Each one of the listed people agreed to the following:
> >>
> >>> If you allow the use of your contribution in QEMU under the
> >>> terms of GPLv2 or later as proposed by this patch,
> >>> please respond to this mail including the line:
> >>>
> >>> Acked-by: Name <email address>
> >>
> >>   Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> >>   Acked-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>   Acked-by: Jason Baron <jbaron@akamai.com>
> >>   Acked-by: David Woodhouse <David.Woodhouse@intel.com>
> >>   Acked-by: Gleb Natapov <gleb@redhat.com>
> >>   Acked-by: Marcelo Tosatti <mtosatti@redhat.com>
> >>   Acked-by: Dave Frodin <dave.frodin@se-eng.com>
> >>   Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> >>   Acked-by: Kevin O'Connor <kevin@koconnor.net>
> >>   Acked-by: Laszlo Ersek <lersek@redhat.com>
> >>   Acked-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> >>   Acked-by: Isaku Yamahata <yamahata@valinux.co.jp>
> >>   Acked-by: Magnus Christensson <magnus.christensson@intel.com>
> >>   Acked-by: Hu Tao <hutao@cn.fujitsu.com>
> >>   Acked-by: Eduardo Habkost <ehabkost@redhat.com>
> >>
> >> The patch incorporates ideas/suggestions from Michael Tsirkin's prototype
> >> code:
> >> - "hw/i386/pc.c" is too big, create new file "hw/i386/acpi.c" with
> >>   i386-specific ACPI table stuff,
> >> - separate preparation of individual tables from their installation as
> >>   fw_cfg files,
> >> - install these fw_cfg files inside pc_memory_init(), which is shared by
> >>   piix4/q35,
> >> - add the above licensing-related block to the commit message.
> >>
> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> >> ---
> >>  configure             |   12 ++++
> >>  hw/i386/Makefile.objs |    1 +
> >>  hw/i386/acpi.h        |    9 +++
> >>  hw/i386/acpi.c        |  159 +++++++++++++++++++++++++++++++++++++++++++++++++
> >>  hw/i386/pc.c          |   23 +++++++
> >>  5 files changed, 204 insertions(+), 0 deletions(-)
> >>  create mode 100644 hw/i386/acpi.h
> >>  create mode 100644 hw/i386/acpi.c
> >>
> >> diff --git a/configure b/configure
> >> index ed49f91..45a5f55 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -241,6 +241,7 @@ gtk=""
> >>  gtkabi="2.0"
> >>  tpm="no"
> >>  libssh2=""
> >> +dynamic_acpi="no"
> >>  
> >>  # parse CC options first
> >>  for opt do
> >> @@ -928,6 +929,10 @@ for opt do
> >>    ;;
> >>    --enable-libssh2) libssh2="yes"
> >>    ;;
> >> +  --disable-dynamic-acpi) dynamic_acpi="no"
> >> +  ;;
> >> +  --enable-dynamic-acpi) dynamic_acpi="yes"
> >> +  ;;
> >>    *) echo "ERROR: unknown option $opt"; show_help="yes"
> >>    ;;
> >>    esac
> >> @@ -1195,6 +1200,8 @@ echo "  --gcov=GCOV              use specified gcov [$gcov_tool]"
> >>  echo "  --enable-tpm             enable TPM support"
> >>  echo "  --disable-libssh2        disable ssh block device support"
> >>  echo "  --enable-libssh2         enable ssh block device support"
> >> +echo "  --disable-dynamic-acpi   disable dynamic ACPI table generation (default)"
> >> +echo "  --enable-dynamic-acpi    enable dynamic ACPI table generation (work in progress)"
> >>  echo ""
> >>  echo "NOTE: The object files are built at the place where configure is launched"
> >>  exit 1
> >> @@ -3573,6 +3580,7 @@ echo "gcov enabled      $gcov"
> >>  echo "TPM support       $tpm"
> >>  echo "libssh2 support   $libssh2"
> >>  echo "TPM passthrough   $tpm_passthrough"
> >> +echo "dynamic ACPI tables $dynamic_acpi"
> >>  
> >>  if test "$sdl_too_old" = "yes"; then
> >>  echo "-> Your SDL version is too old - please upgrade to have SDL support"
> >> @@ -3958,6 +3966,10 @@ if test "$virtio_blk_data_plane" = "yes" ; then
> >>    echo 'CONFIG_VIRTIO_BLK_DATA_PLANE=$(CONFIG_VIRTIO)' >> $config_host_mak
> >>  fi
> >>  
> >> +if test "$dynamic_acpi" = "yes"; then
> >> +  echo "CONFIG_DYN_ACPI=y" >> $config_host_mak
> >> +fi
> >> +
> >>  # USB host support
> >>  case "$usb" in
> >>  linux)
> >> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> >> index 205d22e..8429d52 100644
> >> --- a/hw/i386/Makefile.objs
> >> +++ b/hw/i386/Makefile.objs
> >> @@ -1,6 +1,7 @@
> >>  obj-$(CONFIG_KVM) += kvm/
> >>  obj-y += multiboot.o smbios.o
> >>  obj-y += pc.o pc_piix.o pc_q35.o
> >> +obj-$(CONFIG_DYN_ACPI) += acpi.o
> >>  obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
> >>  
> >>  obj-y += kvmvapic.o
> >> diff --git a/hw/i386/acpi.h b/hw/i386/acpi.h
> >> new file mode 100644
> >> index 0000000..94f9ad3
> >> --- /dev/null
> >> +++ b/hw/i386/acpi.h
> >> @@ -0,0 +1,9 @@
> >> +#ifndef QEMU_HW_I386_ACPI_H
> >> +#define QEMU_HW_I386_ACPI_H
> >> +
> >> +#include <stddef.h>
> >> +
> >> +void acpi_build_madt(unsigned char **out_blob, size_t *out_blob_size,
> >> +                     unsigned num_lapic);
> >> +
> >> +#endif
> >> diff --git a/hw/i386/acpi.c b/hw/i386/acpi.c
> >> new file mode 100644
> >> index 0000000..179cdf5
> >> --- /dev/null
> >> +++ b/hw/i386/acpi.c
> >> @@ -0,0 +1,159 @@
> >> +/*
> >> + * Copyright (c) 2013 Red Hat Inc.
> >> + * Copyright (C) 2008-2010  Kevin O'Connor <kevin@koconnor.net>
> >> + * Copyright (C) 2006 Fabrice Bellard
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License as published by
> >> + * the Free Software Foundation; either version 2 of the License, or
> >> + * (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License along
> >> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> >> + *
> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> >> + * THE SOFTWARE.
> >> + */
> >> +
> >> +#include "hw/acpi/acpi.h"
> >> +#include "hw/i386/acpi.h"
> >> +#include "kvm_i386.h"
> >> +#include "sysemu/sysemu.h"
> >> +
> >> +static void acpi_table_fill_hdr(AcpiTableStdHdr *std_hdr, size_t blob_size,
> >> +                                const char *sig)
> >> +{
> >> +    g_assert(blob_size >= sizeof *std_hdr);
> >> +
> >> +    *std_hdr = acpi_dfl_hdr;
> >> +    strncpy(std_hdr->sig, sig, sizeof std_hdr->sig);
> >> +    strncpy(std_hdr->oem_id, "QEMU  ", sizeof std_hdr->oem_id);
> >> +    strncpy(std_hdr->oem_table_id + 4, sig, sizeof std_hdr->oem_table_id - 4);
> >> +    std_hdr->length = cpu_to_le32(blob_size);
> >> +    std_hdr->checksum = acpi_checksum((uint8_t *)std_hdr, blob_size);
> >> +}
> >> +
> >> +void acpi_build_madt(unsigned char **out_blob, size_t *out_blob_size,
> >> +                     unsigned num_lapic)
> >> +{
> >> +    typedef struct {
> >> +        uint8_t    type;
> >> +        uint8_t    length;
> >> +    } QEMU_PACKED AcpiSubHdr;
> >> +
> >> +    AcpiTableStdHdr *std_hdr;
> >> +    struct {
> >> +        uint32_t   lapic_addr; /* Local Interrupt Controller Address */
> >> +        uint32_t   flags;      /* Multiple APIC flags */
> >> +    } QEMU_PACKED *madt;
> >> +    struct {
> >> +        AcpiSubHdr hdr;
> >> +        uint8_t    processor_id; /* ACPI Processor ID */
> >> +        uint8_t    apic_id;      /* APIC ID */
> >> +        uint32_t   flags;        /* LOcal APIC flags */
> >> +    } QEMU_PACKED *lapic;
> >> +    struct {
> >> +        AcpiSubHdr hdr;
> >> +        uint8_t    io_apic_id;   /* The I/O APIC's ID */
> >> +        uint8_t    reserved;     /* constant zero */
> >> +        uint32_t   io_apic_addr; /* 32-bit physical address to access */
> >> +        uint32_t   gsi_base;     /* interrupt inputs start here */
> >> +    } QEMU_PACKED *io_apic;
> >> +    struct {
> >> +        AcpiSubHdr hdr;
> >> +        uint8_t    bus;    /* constant zero: ISA */
> >> +        uint8_t    source; /* this bus-relative interrupt source... */
> >> +        uint32_t   gsi;    /* ... will signal this global system interrupt */
> >> +        uint16_t   flags;  /* MPS INTI Flags: Polarity, Trigger Mode */
> >> +    } QEMU_PACKED *int_src_ovr;
> >> +    struct {
> >> +        AcpiSubHdr hdr;
> >> +        uint8_t    processor_id; /* ACPI Processor ID */
> >> +        uint16_t   flags;        /* MPS INTI Flags: Polarity, Trigger Mode */
> >> +        uint8_t    lint;         /* LAPIC interrupt input for NMI */
> >> +    } QEMU_PACKED *lapic_nmi;
> >> +
> >> +    static const uint8_t pci_isa_irq[] = { 5, 9, 10, 11 };
> >> +
> >> +    unsigned num_int_src_ovr, i;
> >> +    size_t blob_size;
> >> +    char unsigned *blob;
> >> +
> >> +    num_int_src_ovr = sizeof pci_isa_irq + kvm_allows_irq0_override();
> >> +
> >> +    blob_size = (sizeof *std_hdr)     * 1               +
> >> +                (sizeof *madt)        * 1               +
> >> +                (sizeof *lapic)       * num_lapic       +
> >> +                (sizeof *io_apic)     * 1               +
> >> +                (sizeof *int_src_ovr) * num_int_src_ovr +
> >> +                (sizeof *lapic_nmi)   * 1;
> >> +    blob      = g_malloc(blob_size);
> >> +
> >> +    std_hdr     = (void *)blob;
> >> +    madt        = (void *)(std_hdr     + 1              );
> >> +    lapic       = (void *)(madt        + 1              );
> >> +    io_apic     = (void *)(lapic       + num_lapic      );
> >> +    int_src_ovr = (void *)(io_apic     + 1              );
> >> +    lapic_nmi   = (void *)(int_src_ovr + num_int_src_ovr);
> >> +
> >> +    madt->lapic_addr = cpu_to_le32(APIC_DEFAULT_ADDRESS);
> >> +    madt->flags      = cpu_to_le32(1); /* PCAT_COMPAT */
> >> +
> >> +    /* create a Local APIC structure for each possible APIC ID */
> >> +    for (i = 0; i < num_lapic; ++i) {
> >> +        lapic[i].hdr.type     = 0; /* Processor Local APIC */
> >> +        lapic[i].hdr.length   = sizeof *lapic;
> >> +        lapic[i].processor_id = i;
> >> +        lapic[i].apic_id      = i;
> >> +        lapic[i].flags        = cpu_to_le32(0); /* disabled */
> >> +    }
> >> +    /* enable the CPUs with a CPU index in the [0..smp_cpus-1] range */
> >> +    for (i = 0; i < smp_cpus; ++i) {
> >> +        lapic[x86_cpu_apic_id_from_index(i)].flags = cpu_to_le32(1);
> >> +    }
> >> +
> >> +    io_apic->hdr.type     = 1; /* I/O APIC */
> >> +    io_apic->hdr.length   = sizeof *io_apic;
> >> +    io_apic->io_apic_id   = 0;
> >> +    io_apic->reserved     = 0;
> >> +    io_apic->io_apic_addr = cpu_to_le32(IO_APIC_DEFAULT_ADDRESS);
> >> +    io_apic->gsi_base     = cpu_to_le32(0);
> >> +
> >> +    for (i = 0; i < sizeof pci_isa_irq; ++i) {
> >> +        int_src_ovr[i].hdr.type   = 2; /* Interrupt Source Override */
> >> +        int_src_ovr[i].hdr.length = sizeof *int_src_ovr;
> >> +        int_src_ovr[i].bus        = 0;
> >> +        int_src_ovr[i].source     = pci_isa_irq[i];
> >> +        int_src_ovr[i].gsi        = cpu_to_le32(pci_isa_irq[i]);
> >> +        int_src_ovr[i].flags      = cpu_to_le16(0xd);
> >> +                                    /* active high, level-triggered */
> >> +    }
> >> +    if (kvm_allows_irq0_override()) {
> >> +        int_src_ovr[i].hdr.type   = 2; /* Interrupt Source Override */
> >> +        int_src_ovr[i].hdr.length = sizeof *int_src_ovr;
> >> +        int_src_ovr[i].bus        = 0;
> >> +        int_src_ovr[i].source     = 0;
> >> +        int_src_ovr[i].gsi        = cpu_to_le32(2);
> >> +        int_src_ovr[i].flags      = cpu_to_le16(0); /* conforms to bus spec */
> >> +    }
> >> +
> >> +    lapic_nmi->hdr.type     = 4; /* Local APIC NMI */
> >> +    lapic_nmi->hdr.length   = sizeof *lapic_nmi;
> >> +    lapic_nmi->processor_id = 0xff; /* all processors */
> >> +    lapic_nmi->flags        = cpu_to_le16(0); /* conforms to bus spec */
> >> +    lapic_nmi->lint         = 1; /* NMI connected to LAPIC input LINT1 */
> >> +
> >> +    acpi_table_fill_hdr(std_hdr, blob_size, "APIC");
> >> +    *out_blob = blob;
> >> +    *out_blob_size = blob_size;
> >> +}
> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> index 8727489..15c6284 100644
> >> --- a/hw/i386/pc.c
> >> +++ b/hw/i386/pc.c
> >> @@ -54,6 +54,10 @@
> >>  #include "qemu/config-file.h"
> >>  #include "hw/acpi/acpi.h"
> >>  
> >> +#ifdef CONFIG_DYN_ACPI
> >> +#  include "hw/i386/acpi.h"
> >> +#endif
> >> +
> >>  /* debug PC/ISA interrupts */
> >>  //#define DEBUG_IRQ
> >>  
> >> @@ -922,6 +926,21 @@ void pc_acpi_init(const char *default_dsdt)
> >>      }
> >>  }
> >>  
> >> +#ifdef CONFIG_DYN_ACPI
> >> +static void pc_acpi_madt(FWCfgState *fw_cfg)
> >> +{
> >> +    unsigned num_lapic;
> >> +    char unsigned *blob;
> >> +    size_t blob_size;
> >> +
> >> +    /* see note on FW_CFG_MAX_CPUS in bochs_bios_init() */
> >> +    num_lapic = pc_apic_id_limit(max_cpus);
> >> +
> >> +    acpi_build_madt(&blob, &blob_size, num_lapic);
> >> +    fw_cfg_add_file(fw_cfg, "etc/acpi/APIC", blob, blob_size);
> >> +}
> >> +#endif
> >> +
> >>  FWCfgState *pc_memory_init(MemoryRegion *system_memory,
> >>                             const char *kernel_filename,
> >>                             const char *kernel_cmdline,
> >> @@ -974,6 +993,10 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
> >>      fw_cfg = bochs_bios_init();
> >>      rom_set_fw(fw_cfg);
> > 
> > Let's cut out the ifdefs, just use an if below. Compiler is smart enough
> > to do it right.
> 
> I considered that, because it's how SeaBIOS does it.
> 
> There are several "depths" to do this; for example, I could even build
> hw/i386/acpi.o with the switch being off, and pass the object file to
> the linker (ie. no variable substitution in Makefile.objs) and let the
> linker omit it when it finds no references.
> 
> However, I obviously checked how existent parts of the code do it,
> specifically CONFIG_XEN, and I followed that manner. See
> "hw/i386/pc_piix.c" and "arch_init.c". There's not one CONFIG_XXX
> feature test macro in the qemu source (that I can find) that is examined
> with the "if" statement.

Good point.

> IOW with all due respect I disagree.
> 
> Thanks,
> Laszlo

Fine, it's not a big deal.

> > 
> >> +#ifdef CONFIG_DYN_ACPI
> >> +    pc_acpi_madt(fw_cfg);
> >> +#endif
> >> +
> >>      if (linux_boot) {
> >>          load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size);
> >>      }
> >> -- 
> >> 1.7.1
> >>
Anthony Liguori - April 25, 2013, 7:03 p.m.
Laszlo Ersek <lersek@redhat.com> writes:

> This patch reuses some code from SeaBIOS, which was originally under
> LGPLv2 and then relicensed to GPLv3 or LGPLv3, in QEMU under GPLv2+. This
> relicensing has been acked by all contributors that had contributed to the
> code since the v2->v3 relicense. ACKs approving the v2+ relicensing are
> listed below. The list might include ACKs from people not holding
> copyright on any parts of the reused code, but it's better to err on the
> side of caution and include them.
>
> Affected SeaBIOS files (GPLv2+ license headers added)
> <http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/5949>:
>
>  src/acpi-dsdt-cpu-hotplug.dsl    |   15 +++++++++++++++
>  src/acpi-dsdt-dbug.dsl           |   15 +++++++++++++++
>  src/acpi-dsdt-hpet.dsl           |   15 +++++++++++++++
>  src/acpi-dsdt-isa.dsl            |   15 +++++++++++++++
>  src/acpi-dsdt-pci-crs.dsl        |   15 +++++++++++++++
>  src/acpi.c                       |   14 +++++++++++++-
>  src/acpi.h                       |   14 ++++++++++++++
>  src/ssdt-misc.dsl                |   15 +++++++++++++++
>  src/ssdt-pcihp.dsl               |   15 +++++++++++++++
>  src/ssdt-proc.dsl                |   15 +++++++++++++++
>  tools/acpi_extract.py            |   13 ++++++++++++-
>  tools/acpi_extract_preprocess.py |   13 ++++++++++++-
>  12 files changed, 171 insertions(+), 3 deletions(-)
>
> Each one of the listed people agreed to the following:
>
>> If you allow the use of your contribution in QEMU under the
>> terms of GPLv2 or later as proposed by this patch,
>> please respond to this mail including the line:
>>
>> Acked-by: Name <email address>
>
>   Acked-by: Gerd Hoffmann <kraxel@redhat.com>
>   Acked-by: Jan Kiszka <jan.kiszka@siemens.com>
>   Acked-by: Jason Baron <jbaron@akamai.com>
>   Acked-by: David Woodhouse <David.Woodhouse@intel.com>
>   Acked-by: Gleb Natapov <gleb@redhat.com>
>   Acked-by: Marcelo Tosatti <mtosatti@redhat.com>
>   Acked-by: Dave Frodin <dave.frodin@se-eng.com>
>   Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>   Acked-by: Kevin O'Connor <kevin@koconnor.net>
>   Acked-by: Laszlo Ersek <lersek@redhat.com>
>   Acked-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
>   Acked-by: Isaku Yamahata <yamahata@valinux.co.jp>
>   Acked-by: Magnus Christensson <magnus.christensson@intel.com>
>   Acked-by: Hu Tao <hutao@cn.fujitsu.com>
>   Acked-by: Eduardo Habkost <ehabkost@redhat.com>
>
> The patch incorporates ideas/suggestions from Michael Tsirkin's prototype
> code:
> - "hw/i386/pc.c" is too big, create new file "hw/i386/acpi.c" with
>   i386-specific ACPI table stuff,
> - separate preparation of individual tables from their installation as
>   fw_cfg files,
> - install these fw_cfg files inside pc_memory_init(), which is shared by
>   piix4/q35,
> - add the above licensing-related block to the commit message.
>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  configure             |   12 ++++
>  hw/i386/Makefile.objs |    1 +
>  hw/i386/acpi.h        |    9 +++
>  hw/i386/acpi.c        |  159 +++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/i386/pc.c          |   23 +++++++
>  5 files changed, 204 insertions(+), 0 deletions(-)
>  create mode 100644 hw/i386/acpi.h
>  create mode 100644 hw/i386/acpi.c
>
> diff --git a/configure b/configure
> index ed49f91..45a5f55 100755
> --- a/configure
> +++ b/configure
> @@ -241,6 +241,7 @@ gtk=""
>  gtkabi="2.0"
>  tpm="no"
>  libssh2=""
> +dynamic_acpi="no"
>  
>  # parse CC options first
>  for opt do
> @@ -928,6 +929,10 @@ for opt do
>    ;;
>    --enable-libssh2) libssh2="yes"
>    ;;
> +  --disable-dynamic-acpi) dynamic_acpi="no"
> +  ;;
> +  --enable-dynamic-acpi) dynamic_acpi="yes"
> +  ;;
>    *) echo "ERROR: unknown option $opt"; show_help="yes"
>    ;;
>    esac
> @@ -1195,6 +1200,8 @@ echo "  --gcov=GCOV              use specified gcov [$gcov_tool]"
>  echo "  --enable-tpm             enable TPM support"
>  echo "  --disable-libssh2        disable ssh block device support"
>  echo "  --enable-libssh2         enable ssh block device support"
> +echo "  --disable-dynamic-acpi   disable dynamic ACPI table generation (default)"
> +echo "  --enable-dynamic-acpi    enable dynamic ACPI table generation (work in progress)"
>  echo ""
>  echo "NOTE: The object files are built at the place where configure is launched"
>  exit 1
> @@ -3573,6 +3580,7 @@ echo "gcov enabled      $gcov"
>  echo "TPM support       $tpm"
>  echo "libssh2 support   $libssh2"
>  echo "TPM passthrough   $tpm_passthrough"
> +echo "dynamic ACPI tables $dynamic_acpi"
>  
>  if test "$sdl_too_old" = "yes"; then
>  echo "-> Your SDL version is too old - please upgrade to have SDL support"
> @@ -3958,6 +3966,10 @@ if test "$virtio_blk_data_plane" = "yes" ; then
>    echo 'CONFIG_VIRTIO_BLK_DATA_PLANE=$(CONFIG_VIRTIO)' >> $config_host_mak
>  fi
>  
> +if test "$dynamic_acpi" = "yes"; then
> +  echo "CONFIG_DYN_ACPI=y" >> $config_host_mak
> +fi
> +
>  # USB host support
>  case "$usb" in
>  linux)

No reason for this to be a configure option.

> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index 205d22e..8429d52 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -1,6 +1,7 @@
>  obj-$(CONFIG_KVM) += kvm/
>  obj-y += multiboot.o smbios.o
>  obj-y += pc.o pc_piix.o pc_q35.o
> +obj-$(CONFIG_DYN_ACPI) += acpi.o
>  obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
>  
>  obj-y += kvmvapic.o
> diff --git a/hw/i386/acpi.h b/hw/i386/acpi.h
> new file mode 100644
> index 0000000..94f9ad3
> --- /dev/null
> +++ b/hw/i386/acpi.h
> @@ -0,0 +1,9 @@
> +#ifndef QEMU_HW_I386_ACPI_H
> +#define QEMU_HW_I386_ACPI_H

Missing copyright.

> +
> +#include <stddef.h>

QEMU style would normally be to use qemu-common.h here but honestly I
prefer using system headers when it's possible.  Just FYI.

> +
> +void acpi_build_madt(unsigned char **out_blob, size_t *out_blob_size,
> +                     unsigned num_lapic);
> +
> +#endif
> diff --git a/hw/i386/acpi.c b/hw/i386/acpi.c
> new file mode 100644
> index 0000000..179cdf5
> --- /dev/null
> +++ b/hw/i386/acpi.c
> @@ -0,0 +1,159 @@
> +/*
> + * Copyright (c) 2013 Red Hat Inc.
> + * Copyright (C) 2008-2010  Kevin O'Connor <kevin@koconnor.net>
> + * Copyright (C) 2006 Fabrice Bellard
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "hw/acpi/acpi.h"
> +#include "hw/i386/acpi.h"
> +#include "kvm_i386.h"
> +#include "sysemu/sysemu.h"
> +
> +static void acpi_table_fill_hdr(AcpiTableStdHdr *std_hdr, size_t blob_size,
> +                                const char *sig)
> +{
> +    g_assert(blob_size >= sizeof *std_hdr);
> +
> +    *std_hdr = acpi_dfl_hdr;
> +    strncpy(std_hdr->sig, sig, sizeof std_hdr->sig);

Should use () with sizeof and avoid strncpy.  It almost never has the
behavior you want with respect to NULL termination (unless you
explicitly want to not NULL terminate when hitting bufsize).

> +    strncpy(std_hdr->oem_id, "QEMU  ", sizeof std_hdr->oem_id);
> +    strncpy(std_hdr->oem_table_id + 4, sig, sizeof std_hdr->oem_table_id - 4);
> +    std_hdr->length = cpu_to_le32(blob_size);
> +    std_hdr->checksum = acpi_checksum((uint8_t *)std_hdr, blob_size);
> +}
> +
> +void acpi_build_madt(unsigned char **out_blob, size_t *out_blob_size,
> +                     unsigned num_lapic)
> +{
> +    typedef struct {
> +        uint8_t    type;
> +        uint8_t    length;
> +    } QEMU_PACKED AcpiSubHdr;
> +
> +    AcpiTableStdHdr *std_hdr;
> +    struct {
> +        uint32_t   lapic_addr; /* Local Interrupt Controller Address */
> +        uint32_t   flags;      /* Multiple APIC flags */
> +    } QEMU_PACKED *madt;
> +    struct {
> +        AcpiSubHdr hdr;
> +        uint8_t    processor_id; /* ACPI Processor ID */
> +        uint8_t    apic_id;      /* APIC ID */
> +        uint32_t   flags;        /* LOcal APIC flags */
> +    } QEMU_PACKED *lapic;
> +    struct {
> +        AcpiSubHdr hdr;
> +        uint8_t    io_apic_id;   /* The I/O APIC's ID */
> +        uint8_t    reserved;     /* constant zero */
> +        uint32_t   io_apic_addr; /* 32-bit physical address to access */
> +        uint32_t   gsi_base;     /* interrupt inputs start here */
> +    } QEMU_PACKED *io_apic;
> +    struct {
> +        AcpiSubHdr hdr;
> +        uint8_t    bus;    /* constant zero: ISA */
> +        uint8_t    source; /* this bus-relative interrupt source... */
> +        uint32_t   gsi;    /* ... will signal this global system interrupt */
> +        uint16_t   flags;  /* MPS INTI Flags: Polarity, Trigger Mode */
> +    } QEMU_PACKED *int_src_ovr;
> +    struct {
> +        AcpiSubHdr hdr;
> +        uint8_t    processor_id; /* ACPI Processor ID */
> +        uint16_t   flags;        /* MPS INTI Flags: Polarity, Trigger Mode */
> +        uint8_t    lint;         /* LAPIC interrupt input for NMI */
> +    } QEMU_PACKED *lapic_nmi;
> +
> +    static const uint8_t pci_isa_irq[] = { 5, 9, 10, 11 };

I understand why it's hardcoded but I wish it wasn't.

> +    unsigned num_int_src_ovr, i;
> +    size_t blob_size;
> +    char unsigned *blob;
> +
> +    num_int_src_ovr = sizeof pci_isa_irq + kvm_allows_irq0_override();
> +
> +    blob_size = (sizeof *std_hdr)     * 1               +
> +                (sizeof *madt)        * 1               +
> +                (sizeof *lapic)       * num_lapic       +
> +                (sizeof *io_apic)     * 1               +
> +                (sizeof *int_src_ovr) * num_int_src_ovr +
> +                (sizeof *lapic_nmi)   * 1;
> +    blob      = g_malloc(blob_size);
> +
> +    std_hdr     = (void *)blob;
> +    madt        = (void *)(std_hdr     + 1              );
> +    lapic       = (void *)(madt        + 1              );
> +    io_apic     = (void *)(lapic       + num_lapic      );
> +    int_src_ovr = (void *)(io_apic     + 1              );
> +    lapic_nmi   = (void *)(int_src_ovr + num_int_src_ovr);
> +
> +    madt->lapic_addr = cpu_to_le32(APIC_DEFAULT_ADDRESS);
> +    madt->flags      = cpu_to_le32(1); /* PCAT_COMPAT */
> +
> +    /* create a Local APIC structure for each possible APIC ID */
> +    for (i = 0; i < num_lapic; ++i) {
> +        lapic[i].hdr.type     = 0; /* Processor Local APIC */
> +        lapic[i].hdr.length   = sizeof *lapic;
> +        lapic[i].processor_id = i;
> +        lapic[i].apic_id      = i;
> +        lapic[i].flags        = cpu_to_le32(0); /* disabled */
> +    }
> +    /* enable the CPUs with a CPU index in the [0..smp_cpus-1] range */
> +    for (i = 0; i < smp_cpus; ++i) {
> +        lapic[x86_cpu_apic_id_from_index(i)].flags = cpu_to_le32(1);
> +    }
> +
> +    io_apic->hdr.type     = 1; /* I/O APIC */
> +    io_apic->hdr.length   = sizeof *io_apic;
> +    io_apic->io_apic_id   = 0;
> +    io_apic->reserved     = 0;
> +    io_apic->io_apic_addr = cpu_to_le32(IO_APIC_DEFAULT_ADDRESS);
> +    io_apic->gsi_base     = cpu_to_le32(0);
> +
> +    for (i = 0; i < sizeof pci_isa_irq; ++i) {
> +        int_src_ovr[i].hdr.type   = 2; /* Interrupt Source Override */
> +        int_src_ovr[i].hdr.length = sizeof *int_src_ovr;
> +        int_src_ovr[i].bus        = 0;
> +        int_src_ovr[i].source     = pci_isa_irq[i];
> +        int_src_ovr[i].gsi        = cpu_to_le32(pci_isa_irq[i]);
> +        int_src_ovr[i].flags      = cpu_to_le16(0xd);
> +                                    /* active high, level-triggered */
> +    }
> +    if (kvm_allows_irq0_override()) {
> +        int_src_ovr[i].hdr.type   = 2; /* Interrupt Source Override */
> +        int_src_ovr[i].hdr.length = sizeof *int_src_ovr;
> +        int_src_ovr[i].bus        = 0;
> +        int_src_ovr[i].source     = 0;
> +        int_src_ovr[i].gsi        = cpu_to_le32(2);
> +        int_src_ovr[i].flags      = cpu_to_le16(0); /* conforms to bus spec */
> +    }
> +
> +    lapic_nmi->hdr.type     = 4; /* Local APIC NMI */
> +    lapic_nmi->hdr.length   = sizeof *lapic_nmi;
> +    lapic_nmi->processor_id = 0xff; /* all processors */
> +    lapic_nmi->flags        = cpu_to_le16(0); /* conforms to bus spec */
> +    lapic_nmi->lint         = 1; /* NMI connected to LAPIC input LINT1 */
> +
> +    acpi_table_fill_hdr(std_hdr, blob_size, "APIC");
> +    *out_blob = blob;
> +    *out_blob_size = blob_size;
> +}
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 8727489..15c6284 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -54,6 +54,10 @@
>  #include "qemu/config-file.h"
>  #include "hw/acpi/acpi.h"
>  
> +#ifdef CONFIG_DYN_ACPI
> +#  include "hw/i386/acpi.h"
> +#endif
> +
>  /* debug PC/ISA interrupts */
>  //#define DEBUG_IRQ
>  
> @@ -922,6 +926,21 @@ void pc_acpi_init(const char *default_dsdt)
>      }
>  }
>  
> +#ifdef CONFIG_DYN_ACPI
> +static void pc_acpi_madt(FWCfgState *fw_cfg)
> +{
> +    unsigned num_lapic;
> +    char unsigned *blob;
> +    size_t blob_size;
> +
> +    /* see note on FW_CFG_MAX_CPUS in bochs_bios_init() */
> +    num_lapic = pc_apic_id_limit(max_cpus);
> +
> +    acpi_build_madt(&blob, &blob_size, num_lapic);

I'd be a lot happier if we were passing more information to this routine
and not hard coding it.  For instance, the PCI interrupt assignments,
the APIC ids, the number of available CPUs, etc.

The commit message also doesn't provide any reason about why we would
want this.  The cover letter provides a reference at least but cover
letters don't end up in git history.

Regards,

Anthony Liguori

> +    fw_cfg_add_file(fw_cfg, "etc/acpi/APIC", blob, blob_size);
> +}
> +#endif
> +
>  FWCfgState *pc_memory_init(MemoryRegion *system_memory,
>                             const char *kernel_filename,
>                             const char *kernel_cmdline,
> @@ -974,6 +993,10 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
>      fw_cfg = bochs_bios_init();
>      rom_set_fw(fw_cfg);
>  
> +#ifdef CONFIG_DYN_ACPI
> +    pc_acpi_madt(fw_cfg);
> +#endif
> +
>      if (linux_boot) {
>          load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size);
>      }
> -- 
> 1.7.1
Eduardo Habkost - April 25, 2013, 8:11 p.m.
On Thu, Apr 25, 2013 at 02:03:05PM -0500, Anthony Liguori wrote:
> Laszlo Ersek <lersek@redhat.com> writes:
> > --- /dev/null
> > +++ b/hw/i386/acpi.h
> > +
> > +#include <stddef.h>
> 
> QEMU style would normally be to use qemu-common.h here but honestly I
> prefer using system headers when it's possible.  Just FYI.

I thought we were actively trying to stop including qemu-common.h from
other header files, because it easily leads to unexpected (and hard to
fix) circular header dependencies.
Anthony Liguori - April 25, 2013, 8:45 p.m.
Eduardo Habkost <ehabkost@redhat.com> writes:

> On Thu, Apr 25, 2013 at 02:03:05PM -0500, Anthony Liguori wrote:
>> Laszlo Ersek <lersek@redhat.com> writes:
>> > --- /dev/null
>> > +++ b/hw/i386/acpi.h
>> > +
>> > +#include <stddef.h>
>> 
>> QEMU style would normally be to use qemu-common.h here but honestly I
>> prefer using system headers when it's possible.  Just FYI.
>
> I thought we were actively trying to stop including qemu-common.h from
> other header files, because it easily leads to unexpected (and hard to
> fix) circular header dependencies.

The problem is qemu-common including other headers, not other headers
including qemu-common...

But like I said in my original note, I don't like using qemu-common in
headers anyway.

Regards,

Anthony Liguori

>
> -- 
> Eduardo
Eduardo Habkost - April 25, 2013, 8:57 p.m.
On Thu, Apr 25, 2013 at 03:45:04PM -0500, Anthony Liguori wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Thu, Apr 25, 2013 at 02:03:05PM -0500, Anthony Liguori wrote:
> >> Laszlo Ersek <lersek@redhat.com> writes:
> >> > --- /dev/null
> >> > +++ b/hw/i386/acpi.h
> >> > +
> >> > +#include <stddef.h>
> >> 
> >> QEMU style would normally be to use qemu-common.h here but honestly I
> >> prefer using system headers when it's possible.  Just FYI.
> >
> > I thought we were actively trying to stop including qemu-common.h from
> > other header files, because it easily leads to unexpected (and hard to
> > fix) circular header dependencies.
> 
> The problem is qemu-common including other headers, not other headers
> including qemu-common...

Well, it depends on what's the stated purpose/rules of qemu-common.h. If
its purpose if to allow .c files to have many commonly-used definitions
available without including commonly-used header files one by one,
qemu-common.h will inevitably include other QEMU header files.

> 
> But like I said in my original note, I don't like using qemu-common in
> headers anyway.

Agreed on this specific case. But I would really like to clarify the
purpose of qemu-common.h, because I always believed that it was supposed
to be a "includes lots of other stuff" header, not a "is included by
lots of other stuff" header (and it can't be both).


BTW, qemu-common.h already have a comment stating the following:

"""
This file is supposed to be included only by .c files. No header file should
depend on qemu-common.h, as this would easily lead to circular header
dependencies.
"""

If this is not true, we must correct it.
Michael S. Tsirkin - April 25, 2013, 9:33 p.m.
On Thu, Apr 25, 2013 at 05:57:41PM -0300, Eduardo Habkost wrote:
> On Thu, Apr 25, 2013 at 03:45:04PM -0500, Anthony Liguori wrote:
> > Eduardo Habkost <ehabkost@redhat.com> writes:
> > 
> > > On Thu, Apr 25, 2013 at 02:03:05PM -0500, Anthony Liguori wrote:
> > >> Laszlo Ersek <lersek@redhat.com> writes:
> > >> > --- /dev/null
> > >> > +++ b/hw/i386/acpi.h
> > >> > +
> > >> > +#include <stddef.h>
> > >> 
> > >> QEMU style would normally be to use qemu-common.h here but honestly I
> > >> prefer using system headers when it's possible.  Just FYI.
> > >
> > > I thought we were actively trying to stop including qemu-common.h from
> > > other header files, because it easily leads to unexpected (and hard to
> > > fix) circular header dependencies.
> > 
> > The problem is qemu-common including other headers, not other headers
> > including qemu-common...
> 
> Well, it depends on what's the stated purpose/rules of qemu-common.h. If
> its purpose if to allow .c files to have many commonly-used definitions
> available without including commonly-used header files one by one,
> qemu-common.h will inevitably include other QEMU header files.
> 
> > 
> > But like I said in my original note, I don't like using qemu-common in
> > headers anyway.
> 
> Agreed on this specific case. But I would really like to clarify the
> purpose of qemu-common.h, because I always believed that it was supposed
> to be a "includes lots of other stuff" header, not a "is included by
> lots of other stuff" header (and it can't be both).
> 
> 
> BTW, qemu-common.h already have a comment stating the following:
> 
> """
> This file is supposed to be included only by .c files. No header file should
> depend on qemu-common.h, as this would easily lead to circular header
> dependencies.
> """
> 
> If this is not true, we must correct it.

I think most of these should be switched to include
qemu/typedefs.h. qemu-common used to have all typedefs
a while ago.


> -- 
> Eduardo
Laszlo Ersek - April 26, 2013, 11:13 a.m.
On 04/25/13 21:03, Anthony Liguori wrote:
> Laszlo Ersek <lersek@redhat.com> writes:
> 
>> This patch reuses some code from SeaBIOS, which was originally under
>> LGPLv2 and then relicensed to GPLv3 or LGPLv3, in QEMU under GPLv2+. This
>> relicensing has been acked by all contributors that had contributed to the
>> code since the v2->v3 relicense. ACKs approving the v2+ relicensing are
>> listed below. The list might include ACKs from people not holding
>> copyright on any parts of the reused code, but it's better to err on the
>> side of caution and include them.
>>
>> Affected SeaBIOS files (GPLv2+ license headers added)
>> <http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/5949>:
>>
>>  src/acpi-dsdt-cpu-hotplug.dsl    |   15 +++++++++++++++
>>  src/acpi-dsdt-dbug.dsl           |   15 +++++++++++++++
>>  src/acpi-dsdt-hpet.dsl           |   15 +++++++++++++++
>>  src/acpi-dsdt-isa.dsl            |   15 +++++++++++++++
>>  src/acpi-dsdt-pci-crs.dsl        |   15 +++++++++++++++
>>  src/acpi.c                       |   14 +++++++++++++-
>>  src/acpi.h                       |   14 ++++++++++++++
>>  src/ssdt-misc.dsl                |   15 +++++++++++++++
>>  src/ssdt-pcihp.dsl               |   15 +++++++++++++++
>>  src/ssdt-proc.dsl                |   15 +++++++++++++++
>>  tools/acpi_extract.py            |   13 ++++++++++++-
>>  tools/acpi_extract_preprocess.py |   13 ++++++++++++-
>>  12 files changed, 171 insertions(+), 3 deletions(-)
>>
>> Each one of the listed people agreed to the following:
>>
>>> If you allow the use of your contribution in QEMU under the
>>> terms of GPLv2 or later as proposed by this patch,
>>> please respond to this mail including the line:
>>>
>>> Acked-by: Name <email address>
>>
>>   Acked-by: Gerd Hoffmann <kraxel@redhat.com>
>>   Acked-by: Jan Kiszka <jan.kiszka@siemens.com>
>>   Acked-by: Jason Baron <jbaron@akamai.com>
>>   Acked-by: David Woodhouse <David.Woodhouse@intel.com>
>>   Acked-by: Gleb Natapov <gleb@redhat.com>
>>   Acked-by: Marcelo Tosatti <mtosatti@redhat.com>
>>   Acked-by: Dave Frodin <dave.frodin@se-eng.com>
>>   Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>>   Acked-by: Kevin O'Connor <kevin@koconnor.net>
>>   Acked-by: Laszlo Ersek <lersek@redhat.com>
>>   Acked-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
>>   Acked-by: Isaku Yamahata <yamahata@valinux.co.jp>
>>   Acked-by: Magnus Christensson <magnus.christensson@intel.com>
>>   Acked-by: Hu Tao <hutao@cn.fujitsu.com>
>>   Acked-by: Eduardo Habkost <ehabkost@redhat.com>
>>
>> The patch incorporates ideas/suggestions from Michael Tsirkin's prototype
>> code:
>> - "hw/i386/pc.c" is too big, create new file "hw/i386/acpi.c" with
>>   i386-specific ACPI table stuff,
>> - separate preparation of individual tables from their installation as
>>   fw_cfg files,
>> - install these fw_cfg files inside pc_memory_init(), which is shared by
>>   piix4/q35,
>> - add the above licensing-related block to the commit message.
>>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  configure             |   12 ++++
>>  hw/i386/Makefile.objs |    1 +
>>  hw/i386/acpi.h        |    9 +++
>>  hw/i386/acpi.c        |  159 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  hw/i386/pc.c          |   23 +++++++
>>  5 files changed, 204 insertions(+), 0 deletions(-)
>>  create mode 100644 hw/i386/acpi.h
>>  create mode 100644 hw/i386/acpi.c
>>
>> diff --git a/configure b/configure
>> index ed49f91..45a5f55 100755
>> --- a/configure
>> +++ b/configure
>> @@ -241,6 +241,7 @@ gtk=""
>>  gtkabi="2.0"
>>  tpm="no"
>>  libssh2=""
>> +dynamic_acpi="no"
>>  
>>  # parse CC options first
>>  for opt do
>> @@ -928,6 +929,10 @@ for opt do
>>    ;;
>>    --enable-libssh2) libssh2="yes"
>>    ;;
>> +  --disable-dynamic-acpi) dynamic_acpi="no"
>> +  ;;
>> +  --enable-dynamic-acpi) dynamic_acpi="yes"
>> +  ;;
>>    *) echo "ERROR: unknown option $opt"; show_help="yes"
>>    ;;
>>    esac
>> @@ -1195,6 +1200,8 @@ echo "  --gcov=GCOV              use specified gcov [$gcov_tool]"
>>  echo "  --enable-tpm             enable TPM support"
>>  echo "  --disable-libssh2        disable ssh block device support"
>>  echo "  --enable-libssh2         enable ssh block device support"
>> +echo "  --disable-dynamic-acpi   disable dynamic ACPI table generation (default)"
>> +echo "  --enable-dynamic-acpi    enable dynamic ACPI table generation (work in progress)"
>>  echo ""
>>  echo "NOTE: The object files are built at the place where configure is launched"
>>  exit 1
>> @@ -3573,6 +3580,7 @@ echo "gcov enabled      $gcov"
>>  echo "TPM support       $tpm"
>>  echo "libssh2 support   $libssh2"
>>  echo "TPM passthrough   $tpm_passthrough"
>> +echo "dynamic ACPI tables $dynamic_acpi"
>>  
>>  if test "$sdl_too_old" = "yes"; then
>>  echo "-> Your SDL version is too old - please upgrade to have SDL support"
>> @@ -3958,6 +3966,10 @@ if test "$virtio_blk_data_plane" = "yes" ; then
>>    echo 'CONFIG_VIRTIO_BLK_DATA_PLANE=$(CONFIG_VIRTIO)' >> $config_host_mak
>>  fi
>>  
>> +if test "$dynamic_acpi" = "yes"; then
>> +  echo "CONFIG_DYN_ACPI=y" >> $config_host_mak
>> +fi
>> +
>>  # USB host support
>>  case "$usb" in
>>  linux)
> 
> No reason for this to be a configure option.

:-/

I added this from v3 to v4 because Michael asked me for it
<http://thread.gmane.org/gmane.comp.emulators.qemu/206435/focus=207146>.

From the SeaBIOS side, I believe Kevin O'Connor also wants to keep out
related code from SeaBIOS until a full set of tables is passed. I
disagree with flipping a big switch in the end (ie. I agree a config
option (let alone a separate SeaBIOS branch) is unwarranted, which is
why I didn't add the former in v3), but I have no say in it.

Do you want me to rip out these hunks (and adapt the dependencies)?

>> +static void acpi_table_fill_hdr(AcpiTableStdHdr *std_hdr, size_t blob_size,
>> +                                const char *sig)
>> +{
>> +    g_assert(blob_size >= sizeof *std_hdr);
>> +
>> +    *std_hdr = acpi_dfl_hdr;
>> +    strncpy(std_hdr->sig, sig, sizeof std_hdr->sig);
> 
> Should use () with sizeof and avoid strncpy.  It almost never has the
> behavior you want with respect to NULL termination (unless you
> explicitly want to not NULL terminate when hitting bufsize).

Correct, I explicitly wanted to avoid NUL-termination here. The
destination ACPI fields are not NUL-terminated but fixed size. The
caller specifies a C string. The bytes not covered by that string in the
ACPI field, ie. coming from the default header, should be overwritten by
NULs.


>> +#ifdef CONFIG_DYN_ACPI
>> +static void pc_acpi_madt(FWCfgState *fw_cfg)
>> +{
>> +    unsigned num_lapic;
>> +    char unsigned *blob;
>> +    size_t blob_size;
>> +
>> +    /* see note on FW_CFG_MAX_CPUS in bochs_bios_init() */
>> +    num_lapic = pc_apic_id_limit(max_cpus);
>> +
>> +    acpi_build_madt(&blob, &blob_size, num_lapic);
> 
> I'd be a lot happier if we were passing more information to this routine
> and not hard coding it.  For instance, the PCI interrupt assignments,
> the APIC ids, the number of available CPUs, etc.

I can try to extract all dependencies as parameters, but I think it will
lead us down an unpleasant path. In my understanding the migration of
ACPI tables from boot firmware(s) to qemu is *also* motivated that ACPI
tables are very quirky ^W flexible, and passing down every bit of info
to build them is (a) a chore, because there are many parameters erecting
the whole bunch of tables, (b) the set of parameters is a constantly
moving target, as tables are added or their ACPI versions are upgraded
(eg. from ACPI 1.0 to ACPI 2.0).

Hence my thinking about this went as the polar opposite of yours. In
these ACPI preparation functions, where we're composing a "portable"
description of the machine, we're fishing from a global pool of
information. Nothing should be off limits. Just as well as I can call
kvm_allows_irq0_override(), I should be able to access whatever global
data and functions, without explicitly specifying in a function
prototype what I need. I need *everything* -- that's (also) why we're
doing the tables in QEMU. Because everything is accessible there without
jumping through hoops.

There's nothing regular in the kinds of information stored in various
ACPI tables; they are arbitrary. A function prototype according to your
suggestion would be justified by nothing more than "well, that's what's
required for the MADT", and if we bump an ACPI version (maybe not for
the MADT but for another table), we'll have to adapt the prototype and
the caller too.

(I already dislike the "num_lapic" parameter; IMO the MADT function
should directly call x86_cpu_apic_id_from_index() with both max_cpus and
smp_cpus, or *maybe* export and call pc_apic_id_limit() for the former.)

Of course this is just my two cents.

> The commit message also doesn't provide any reason about why we would
> want this.  The cover letter provides a reference at least but cover
> letters don't end up in git history.

Well I'll need help wording that. From my perspective there are two goals:
- primary goal: stop OVMF chasing SeaBIOS's tail in ACPI features --
prepare the ACPI tables in QEMU and let whatever boot firmware use the
same set of tables,
- secondary goal: stop bumping into fw_cfg boundaries when needing new
ACPI dependencies in boot firmware (see above).

I believe that Michael is interested in the ACPI move because of a new
chipset he's introducing (or some such, please excuse my ignorance).

Thanks!
Laszlo
Michael S. Tsirkin - April 29, 2013, 8:20 a.m.
On Fri, Apr 26, 2013 at 01:13:28PM +0200, Laszlo Ersek wrote:
> On 04/25/13 21:03, Anthony Liguori wrote:
> > Laszlo Ersek <lersek@redhat.com> writes:
> > 
> >> This patch reuses some code from SeaBIOS, which was originally under
> >> LGPLv2 and then relicensed to GPLv3 or LGPLv3, in QEMU under GPLv2+. This
> >> relicensing has been acked by all contributors that had contributed to the
> >> code since the v2->v3 relicense. ACKs approving the v2+ relicensing are
> >> listed below. The list might include ACKs from people not holding
> >> copyright on any parts of the reused code, but it's better to err on the
> >> side of caution and include them.
> >>
> >> Affected SeaBIOS files (GPLv2+ license headers added)
> >> <http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/5949>:
> >>
> >>  src/acpi-dsdt-cpu-hotplug.dsl    |   15 +++++++++++++++
> >>  src/acpi-dsdt-dbug.dsl           |   15 +++++++++++++++
> >>  src/acpi-dsdt-hpet.dsl           |   15 +++++++++++++++
> >>  src/acpi-dsdt-isa.dsl            |   15 +++++++++++++++
> >>  src/acpi-dsdt-pci-crs.dsl        |   15 +++++++++++++++
> >>  src/acpi.c                       |   14 +++++++++++++-
> >>  src/acpi.h                       |   14 ++++++++++++++
> >>  src/ssdt-misc.dsl                |   15 +++++++++++++++
> >>  src/ssdt-pcihp.dsl               |   15 +++++++++++++++
> >>  src/ssdt-proc.dsl                |   15 +++++++++++++++
> >>  tools/acpi_extract.py            |   13 ++++++++++++-
> >>  tools/acpi_extract_preprocess.py |   13 ++++++++++++-
> >>  12 files changed, 171 insertions(+), 3 deletions(-)
> >>
> >> Each one of the listed people agreed to the following:
> >>
> >>> If you allow the use of your contribution in QEMU under the
> >>> terms of GPLv2 or later as proposed by this patch,
> >>> please respond to this mail including the line:
> >>>
> >>> Acked-by: Name <email address>
> >>
> >>   Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> >>   Acked-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>   Acked-by: Jason Baron <jbaron@akamai.com>
> >>   Acked-by: David Woodhouse <David.Woodhouse@intel.com>
> >>   Acked-by: Gleb Natapov <gleb@redhat.com>
> >>   Acked-by: Marcelo Tosatti <mtosatti@redhat.com>
> >>   Acked-by: Dave Frodin <dave.frodin@se-eng.com>
> >>   Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> >>   Acked-by: Kevin O'Connor <kevin@koconnor.net>
> >>   Acked-by: Laszlo Ersek <lersek@redhat.com>
> >>   Acked-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> >>   Acked-by: Isaku Yamahata <yamahata@valinux.co.jp>
> >>   Acked-by: Magnus Christensson <magnus.christensson@intel.com>
> >>   Acked-by: Hu Tao <hutao@cn.fujitsu.com>
> >>   Acked-by: Eduardo Habkost <ehabkost@redhat.com>
> >>
> >> The patch incorporates ideas/suggestions from Michael Tsirkin's prototype
> >> code:
> >> - "hw/i386/pc.c" is too big, create new file "hw/i386/acpi.c" with
> >>   i386-specific ACPI table stuff,
> >> - separate preparation of individual tables from their installation as
> >>   fw_cfg files,
> >> - install these fw_cfg files inside pc_memory_init(), which is shared by
> >>   piix4/q35,
> >> - add the above licensing-related block to the commit message.
> >>
> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> >> ---
> >>  configure             |   12 ++++
> >>  hw/i386/Makefile.objs |    1 +
> >>  hw/i386/acpi.h        |    9 +++
> >>  hw/i386/acpi.c        |  159 +++++++++++++++++++++++++++++++++++++++++++++++++
> >>  hw/i386/pc.c          |   23 +++++++
> >>  5 files changed, 204 insertions(+), 0 deletions(-)
> >>  create mode 100644 hw/i386/acpi.h
> >>  create mode 100644 hw/i386/acpi.c
> >>
> >> diff --git a/configure b/configure
> >> index ed49f91..45a5f55 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -241,6 +241,7 @@ gtk=""
> >>  gtkabi="2.0"
> >>  tpm="no"
> >>  libssh2=""
> >> +dynamic_acpi="no"
> >>  
> >>  # parse CC options first
> >>  for opt do
> >> @@ -928,6 +929,10 @@ for opt do
> >>    ;;
> >>    --enable-libssh2) libssh2="yes"
> >>    ;;
> >> +  --disable-dynamic-acpi) dynamic_acpi="no"
> >> +  ;;
> >> +  --enable-dynamic-acpi) dynamic_acpi="yes"
> >> +  ;;
> >>    *) echo "ERROR: unknown option $opt"; show_help="yes"
> >>    ;;
> >>    esac
> >> @@ -1195,6 +1200,8 @@ echo "  --gcov=GCOV              use specified gcov [$gcov_tool]"
> >>  echo "  --enable-tpm             enable TPM support"
> >>  echo "  --disable-libssh2        disable ssh block device support"
> >>  echo "  --enable-libssh2         enable ssh block device support"
> >> +echo "  --disable-dynamic-acpi   disable dynamic ACPI table generation (default)"
> >> +echo "  --enable-dynamic-acpi    enable dynamic ACPI table generation (work in progress)"
> >>  echo ""
> >>  echo "NOTE: The object files are built at the place where configure is launched"
> >>  exit 1
> >> @@ -3573,6 +3580,7 @@ echo "gcov enabled      $gcov"
> >>  echo "TPM support       $tpm"
> >>  echo "libssh2 support   $libssh2"
> >>  echo "TPM passthrough   $tpm_passthrough"
> >> +echo "dynamic ACPI tables $dynamic_acpi"
> >>  
> >>  if test "$sdl_too_old" = "yes"; then
> >>  echo "-> Your SDL version is too old - please upgrade to have SDL support"
> >> @@ -3958,6 +3966,10 @@ if test "$virtio_blk_data_plane" = "yes" ; then
> >>    echo 'CONFIG_VIRTIO_BLK_DATA_PLANE=$(CONFIG_VIRTIO)' >> $config_host_mak
> >>  fi
> >>  
> >> +if test "$dynamic_acpi" = "yes"; then
> >> +  echo "CONFIG_DYN_ACPI=y" >> $config_host_mak
> >> +fi
> >> +
> >>  # USB host support
> >>  case "$usb" in
> >>  linux)
> > 
> > No reason for this to be a configure option.
> 
> :-/
> 
> I added this from v3 to v4 because Michael asked me for it
> <http://thread.gmane.org/gmane.comp.emulators.qemu/206435/focus=207146>.
> 
> >From the SeaBIOS side, I believe Kevin O'Connor also wants to keep out
> related code from SeaBIOS until a full set of tables is passed. I
> disagree with flipping a big switch in the end (ie. I agree a config
> option (let alone a separate SeaBIOS branch) is unwarranted, which is
> why I didn't add the former in v3), but I have no say in it.
> 
> Do you want me to rip out these hunks (and adapt the dependencies)?

Essentially, what seabios wants to do is operate in two
modes:
    - (mostly) use builtin acpi tables
    - use acpi tables from hypervisor

in particular, seabios wants to interpret presence
of any file in etc/acpi as a signal not to generate
its own tables.

So merging this patch but without the config option will break
this plan. The only two ways I see are:
- merge this last patch with the config option, disabled by default
  the idea being we can improve it in-tree, gradually.
- keep this patch out of tree until we have a complete
  set of tables.

Both are fine with me.

> >> +static void acpi_table_fill_hdr(AcpiTableStdHdr *std_hdr, size_t blob_size,
> >> +                                const char *sig)
> >> +{
> >> +    g_assert(blob_size >= sizeof *std_hdr);
> >> +
> >> +    *std_hdr = acpi_dfl_hdr;
> >> +    strncpy(std_hdr->sig, sig, sizeof std_hdr->sig);
> > 
> > Should use () with sizeof and avoid strncpy.  It almost never has the
> > behavior you want with respect to NULL termination (unless you
> > explicitly want to not NULL terminate when hitting bufsize).
> 
> Correct, I explicitly wanted to avoid NUL-termination here. The
> destination ACPI fields are not NUL-terminated but fixed size. The
> caller specifies a C string. The bytes not covered by that string in the
> ACPI field, ie. coming from the default header, should be overwritten by
> NULs.
> 
> 
> >> +#ifdef CONFIG_DYN_ACPI
> >> +static void pc_acpi_madt(FWCfgState *fw_cfg)
> >> +{
> >> +    unsigned num_lapic;
> >> +    char unsigned *blob;
> >> +    size_t blob_size;
> >> +
> >> +    /* see note on FW_CFG_MAX_CPUS in bochs_bios_init() */
> >> +    num_lapic = pc_apic_id_limit(max_cpus);
> >> +
> >> +    acpi_build_madt(&blob, &blob_size, num_lapic);
> > 
> > I'd be a lot happier if we were passing more information to this routine
> > and not hard coding it.  For instance, the PCI interrupt assignments,
> > the APIC ids, the number of available CPUs, etc.
> 
> I can try to extract all dependencies as parameters, but I think it will
> lead us down an unpleasant path. In my understanding the migration of
> ACPI tables from boot firmware(s) to qemu is *also* motivated that ACPI
> tables are very quirky ^W flexible, and passing down every bit of info
> to build them is (a) a chore, because there are many parameters erecting
> the whole bunch of tables, (b) the set of parameters is a constantly
> moving target, as tables are added or their ACPI versions are upgraded
> (eg. from ACPI 1.0 to ACPI 2.0).
> 
> Hence my thinking about this went as the polar opposite of yours. In
> these ACPI preparation functions, where we're composing a "portable"
> description of the machine, we're fishing from a global pool of
> information. Nothing should be off limits. Just as well as I can call
> kvm_allows_irq0_override(), I should be able to access whatever global
> data and functions, without explicitly specifying in a function
> prototype what I need. I need *everything* -- that's (also) why we're
> doing the tables in QEMU. Because everything is accessible there without
> jumping through hoops.
> 
> There's nothing regular in the kinds of information stored in various
> ACPI tables; they are arbitrary. A function prototype according to your
> suggestion would be justified by nothing more than "well, that's what's
> required for the MADT", and if we bump an ACPI version (maybe not for
> the MADT but for another table), we'll have to adapt the prototype and
> the caller too.
> 
> (I already dislike the "num_lapic" parameter; IMO the MADT function
> should directly call x86_cpu_apic_id_from_index() with both max_cpus and
> smp_cpus, or *maybe* export and call pc_apic_id_limit() for the former.)
> 
> Of course this is just my two cents.
> 
> > The commit message also doesn't provide any reason about why we would
> > want this.  The cover letter provides a reference at least but cover
> > letters don't end up in git history.
> 
> Well I'll need help wording that. From my perspective there are two goals:
> - primary goal: stop OVMF chasing SeaBIOS's tail in ACPI features --
> prepare the ACPI tables in QEMU and let whatever boot firmware use the
> same set of tables,
> - secondary goal: stop bumping into fw_cfg boundaries when needing new
> ACPI dependencies in boot firmware (see above).
> 
> I believe that Michael is interested in the ACPI move because of a new
> chipset he's introducing (or some such, please excuse my ignorance).
> 
> Thanks!
> Laszlo

Yes, and hotplug for PCI bridge as well.
Kevin O'Connor - April 29, 2013, 12:39 p.m.
On Mon, Apr 29, 2013 at 11:20:15AM +0300, Michael S. Tsirkin wrote:
> On Fri, Apr 26, 2013 at 01:13:28PM +0200, Laszlo Ersek wrote:
> > 
> > I added this from v3 to v4 because Michael asked me for it
> > <http://thread.gmane.org/gmane.comp.emulators.qemu/206435/focus=207146>.
> > 
> > >From the SeaBIOS side, I believe Kevin O'Connor also wants to keep out
> > related code from SeaBIOS until a full set of tables is passed. I
> > disagree with flipping a big switch in the end (ie. I agree a config
> > option (let alone a separate SeaBIOS branch) is unwarranted, which is
> > why I didn't add the former in v3), but I have no say in it.
> > 
> > Do you want me to rip out these hunks (and adapt the dependencies)?
> 
> Essentially, what seabios wants to do is operate in two
> modes:
>     - (mostly) use builtin acpi tables
>     - use acpi tables from hypervisor
> 
> in particular, seabios wants to interpret presence
> of any file in etc/acpi as a signal not to generate
> its own tables.

Right.

> So merging this patch but without the config option will break
> this plan. The only two ways I see are:
> - merge this last patch with the config option, disabled by default
>   the idea being we can improve it in-tree, gradually.
> - keep this patch out of tree until we have a complete
>   set of tables.
> 
> Both are fine with me.

Why?  As long as QEMU places the new tables under new fwcfg entries,
old seabios will totally ignore the new tables.  I don't see why a
QEMU config option is needed - it's safe for QEMU to always create
both old and new fwcfg entries.

-Kevin
Michael S. Tsirkin - April 29, 2013, 1:21 p.m.
On Mon, Apr 29, 2013 at 08:39:26AM -0400, Kevin O'Connor wrote:
> On Mon, Apr 29, 2013 at 11:20:15AM +0300, Michael S. Tsirkin wrote:
> > On Fri, Apr 26, 2013 at 01:13:28PM +0200, Laszlo Ersek wrote:
> > > 
> > > I added this from v3 to v4 because Michael asked me for it
> > > <http://thread.gmane.org/gmane.comp.emulators.qemu/206435/focus=207146>.
> > > 
> > > >From the SeaBIOS side, I believe Kevin O'Connor also wants to keep out
> > > related code from SeaBIOS until a full set of tables is passed. I
> > > disagree with flipping a big switch in the end (ie. I agree a config
> > > option (let alone a separate SeaBIOS branch) is unwarranted, which is
> > > why I didn't add the former in v3), but I have no say in it.
> > > 
> > > Do you want me to rip out these hunks (and adapt the dependencies)?
> > 
> > Essentially, what seabios wants to do is operate in two
> > modes:
> >     - (mostly) use builtin acpi tables
> >     - use acpi tables from hypervisor
> > 
> > in particular, seabios wants to interpret presence
> > of any file in etc/acpi as a signal not to generate
> > its own tables.
> 
> Right.
> 
> > So merging this patch but without the config option will break
> > this plan. The only two ways I see are:
> > - merge this last patch with the config option, disabled by default
> >   the idea being we can improve it in-tree, gradually.
> > - keep this patch out of tree until we have a complete
> >   set of tables.
> > 
> > Both are fine with me.
> 
> Why?  As long as QEMU places the new tables under new fwcfg entries,
> old seabios will totally ignore the new tables.  I don't see why a
> QEMU config option is needed - it's safe for QEMU to always create
> both old and new fwcfg entries.
> 
> -Kevin


Yes backwards compatibility is fine, but the problem here is forwards
compatibility.
Future BIOS will say:
"there's something in etc/acpi/ therefore I won't generate any tables"
so we should not release QEMU that puts only MADT under etc/acpi/madt.
Laszlo Ersek - April 29, 2013, 1:21 p.m.
On 04/29/13 14:39, Kevin O'Connor wrote:
> On Mon, Apr 29, 2013 at 11:20:15AM +0300, Michael S. Tsirkin wrote:

>> in particular, seabios wants to interpret presence
>> of any file in etc/acpi as a signal not to generate
>> its own tables.
> 
> Right.

In that case,

>> So merging this patch but without the config option will break
>> this plan. The only two ways I see are:
>> - merge this last patch with the config option, disabled by default
>>   the idea being we can improve it in-tree, gradually.
>> - keep this patch out of tree until we have a complete
>>   set of tables.
>>
>> Both are fine with me.
> 
> Why?  As long as QEMU places the new tables under new fwcfg entries,
> old seabios will totally ignore the new tables.  I don't see why a
> QEMU config option is needed - it's safe for QEMU to always create
> both old and new fwcfg entries.

the new-style fw_cfg entry for the MADT (= "etc/acpi/APIC") would
prevent the generation of all other (yet unexported by qemu) ACPI tables
in *new* seabios.

[SeaBIOS] [PATCH RFC 2/3] acpi: load and link tables from /etc/acpi/
<http://thread.gmane.org/gmane.comp.emulators.qemu/208457/focus=208458>

Thanks,
Laszlo

Patch

diff --git a/configure b/configure
index ed49f91..45a5f55 100755
--- a/configure
+++ b/configure
@@ -241,6 +241,7 @@  gtk=""
 gtkabi="2.0"
 tpm="no"
 libssh2=""
+dynamic_acpi="no"
 
 # parse CC options first
 for opt do
@@ -928,6 +929,10 @@  for opt do
   ;;
   --enable-libssh2) libssh2="yes"
   ;;
+  --disable-dynamic-acpi) dynamic_acpi="no"
+  ;;
+  --enable-dynamic-acpi) dynamic_acpi="yes"
+  ;;
   *) echo "ERROR: unknown option $opt"; show_help="yes"
   ;;
   esac
@@ -1195,6 +1200,8 @@  echo "  --gcov=GCOV              use specified gcov [$gcov_tool]"
 echo "  --enable-tpm             enable TPM support"
 echo "  --disable-libssh2        disable ssh block device support"
 echo "  --enable-libssh2         enable ssh block device support"
+echo "  --disable-dynamic-acpi   disable dynamic ACPI table generation (default)"
+echo "  --enable-dynamic-acpi    enable dynamic ACPI table generation (work in progress)"
 echo ""
 echo "NOTE: The object files are built at the place where configure is launched"
 exit 1
@@ -3573,6 +3580,7 @@  echo "gcov enabled      $gcov"
 echo "TPM support       $tpm"
 echo "libssh2 support   $libssh2"
 echo "TPM passthrough   $tpm_passthrough"
+echo "dynamic ACPI tables $dynamic_acpi"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -3958,6 +3966,10 @@  if test "$virtio_blk_data_plane" = "yes" ; then
   echo 'CONFIG_VIRTIO_BLK_DATA_PLANE=$(CONFIG_VIRTIO)' >> $config_host_mak
 fi
 
+if test "$dynamic_acpi" = "yes"; then
+  echo "CONFIG_DYN_ACPI=y" >> $config_host_mak
+fi
+
 # USB host support
 case "$usb" in
 linux)
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 205d22e..8429d52 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -1,6 +1,7 @@ 
 obj-$(CONFIG_KVM) += kvm/
 obj-y += multiboot.o smbios.o
 obj-y += pc.o pc_piix.o pc_q35.o
+obj-$(CONFIG_DYN_ACPI) += acpi.o
 obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
 
 obj-y += kvmvapic.o
diff --git a/hw/i386/acpi.h b/hw/i386/acpi.h
new file mode 100644
index 0000000..94f9ad3
--- /dev/null
+++ b/hw/i386/acpi.h
@@ -0,0 +1,9 @@ 
+#ifndef QEMU_HW_I386_ACPI_H
+#define QEMU_HW_I386_ACPI_H
+
+#include <stddef.h>
+
+void acpi_build_madt(unsigned char **out_blob, size_t *out_blob_size,
+                     unsigned num_lapic);
+
+#endif
diff --git a/hw/i386/acpi.c b/hw/i386/acpi.c
new file mode 100644
index 0000000..179cdf5
--- /dev/null
+++ b/hw/i386/acpi.c
@@ -0,0 +1,159 @@ 
+/*
+ * Copyright (c) 2013 Red Hat Inc.
+ * Copyright (C) 2008-2010  Kevin O'Connor <kevin@koconnor.net>
+ * Copyright (C) 2006 Fabrice Bellard
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/acpi/acpi.h"
+#include "hw/i386/acpi.h"
+#include "kvm_i386.h"
+#include "sysemu/sysemu.h"
+
+static void acpi_table_fill_hdr(AcpiTableStdHdr *std_hdr, size_t blob_size,
+                                const char *sig)
+{
+    g_assert(blob_size >= sizeof *std_hdr);
+
+    *std_hdr = acpi_dfl_hdr;
+    strncpy(std_hdr->sig, sig, sizeof std_hdr->sig);
+    strncpy(std_hdr->oem_id, "QEMU  ", sizeof std_hdr->oem_id);
+    strncpy(std_hdr->oem_table_id + 4, sig, sizeof std_hdr->oem_table_id - 4);
+    std_hdr->length = cpu_to_le32(blob_size);
+    std_hdr->checksum = acpi_checksum((uint8_t *)std_hdr, blob_size);
+}
+
+void acpi_build_madt(unsigned char **out_blob, size_t *out_blob_size,
+                     unsigned num_lapic)
+{
+    typedef struct {
+        uint8_t    type;
+        uint8_t    length;
+    } QEMU_PACKED AcpiSubHdr;
+
+    AcpiTableStdHdr *std_hdr;
+    struct {
+        uint32_t   lapic_addr; /* Local Interrupt Controller Address */
+        uint32_t   flags;      /* Multiple APIC flags */
+    } QEMU_PACKED *madt;
+    struct {
+        AcpiSubHdr hdr;
+        uint8_t    processor_id; /* ACPI Processor ID */
+        uint8_t    apic_id;      /* APIC ID */
+        uint32_t   flags;        /* LOcal APIC flags */
+    } QEMU_PACKED *lapic;
+    struct {
+        AcpiSubHdr hdr;
+        uint8_t    io_apic_id;   /* The I/O APIC's ID */
+        uint8_t    reserved;     /* constant zero */
+        uint32_t   io_apic_addr; /* 32-bit physical address to access */
+        uint32_t   gsi_base;     /* interrupt inputs start here */
+    } QEMU_PACKED *io_apic;
+    struct {
+        AcpiSubHdr hdr;
+        uint8_t    bus;    /* constant zero: ISA */
+        uint8_t    source; /* this bus-relative interrupt source... */
+        uint32_t   gsi;    /* ... will signal this global system interrupt */
+        uint16_t   flags;  /* MPS INTI Flags: Polarity, Trigger Mode */
+    } QEMU_PACKED *int_src_ovr;
+    struct {
+        AcpiSubHdr hdr;
+        uint8_t    processor_id; /* ACPI Processor ID */
+        uint16_t   flags;        /* MPS INTI Flags: Polarity, Trigger Mode */
+        uint8_t    lint;         /* LAPIC interrupt input for NMI */
+    } QEMU_PACKED *lapic_nmi;
+
+    static const uint8_t pci_isa_irq[] = { 5, 9, 10, 11 };
+
+    unsigned num_int_src_ovr, i;
+    size_t blob_size;
+    char unsigned *blob;
+
+    num_int_src_ovr = sizeof pci_isa_irq + kvm_allows_irq0_override();
+
+    blob_size = (sizeof *std_hdr)     * 1               +
+                (sizeof *madt)        * 1               +
+                (sizeof *lapic)       * num_lapic       +
+                (sizeof *io_apic)     * 1               +
+                (sizeof *int_src_ovr) * num_int_src_ovr +
+                (sizeof *lapic_nmi)   * 1;
+    blob      = g_malloc(blob_size);
+
+    std_hdr     = (void *)blob;
+    madt        = (void *)(std_hdr     + 1              );
+    lapic       = (void *)(madt        + 1              );
+    io_apic     = (void *)(lapic       + num_lapic      );
+    int_src_ovr = (void *)(io_apic     + 1              );
+    lapic_nmi   = (void *)(int_src_ovr + num_int_src_ovr);
+
+    madt->lapic_addr = cpu_to_le32(APIC_DEFAULT_ADDRESS);
+    madt->flags      = cpu_to_le32(1); /* PCAT_COMPAT */
+
+    /* create a Local APIC structure for each possible APIC ID */
+    for (i = 0; i < num_lapic; ++i) {
+        lapic[i].hdr.type     = 0; /* Processor Local APIC */
+        lapic[i].hdr.length   = sizeof *lapic;
+        lapic[i].processor_id = i;
+        lapic[i].apic_id      = i;
+        lapic[i].flags        = cpu_to_le32(0); /* disabled */
+    }
+    /* enable the CPUs with a CPU index in the [0..smp_cpus-1] range */
+    for (i = 0; i < smp_cpus; ++i) {
+        lapic[x86_cpu_apic_id_from_index(i)].flags = cpu_to_le32(1);
+    }
+
+    io_apic->hdr.type     = 1; /* I/O APIC */
+    io_apic->hdr.length   = sizeof *io_apic;
+    io_apic->io_apic_id   = 0;
+    io_apic->reserved     = 0;
+    io_apic->io_apic_addr = cpu_to_le32(IO_APIC_DEFAULT_ADDRESS);
+    io_apic->gsi_base     = cpu_to_le32(0);
+
+    for (i = 0; i < sizeof pci_isa_irq; ++i) {
+        int_src_ovr[i].hdr.type   = 2; /* Interrupt Source Override */
+        int_src_ovr[i].hdr.length = sizeof *int_src_ovr;
+        int_src_ovr[i].bus        = 0;
+        int_src_ovr[i].source     = pci_isa_irq[i];
+        int_src_ovr[i].gsi        = cpu_to_le32(pci_isa_irq[i]);
+        int_src_ovr[i].flags      = cpu_to_le16(0xd);
+                                    /* active high, level-triggered */
+    }
+    if (kvm_allows_irq0_override()) {
+        int_src_ovr[i].hdr.type   = 2; /* Interrupt Source Override */
+        int_src_ovr[i].hdr.length = sizeof *int_src_ovr;
+        int_src_ovr[i].bus        = 0;
+        int_src_ovr[i].source     = 0;
+        int_src_ovr[i].gsi        = cpu_to_le32(2);
+        int_src_ovr[i].flags      = cpu_to_le16(0); /* conforms to bus spec */
+    }
+
+    lapic_nmi->hdr.type     = 4; /* Local APIC NMI */
+    lapic_nmi->hdr.length   = sizeof *lapic_nmi;
+    lapic_nmi->processor_id = 0xff; /* all processors */
+    lapic_nmi->flags        = cpu_to_le16(0); /* conforms to bus spec */
+    lapic_nmi->lint         = 1; /* NMI connected to LAPIC input LINT1 */
+
+    acpi_table_fill_hdr(std_hdr, blob_size, "APIC");
+    *out_blob = blob;
+    *out_blob_size = blob_size;
+}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8727489..15c6284 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -54,6 +54,10 @@ 
 #include "qemu/config-file.h"
 #include "hw/acpi/acpi.h"
 
+#ifdef CONFIG_DYN_ACPI
+#  include "hw/i386/acpi.h"
+#endif
+
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
 
@@ -922,6 +926,21 @@  void pc_acpi_init(const char *default_dsdt)
     }
 }
 
+#ifdef CONFIG_DYN_ACPI
+static void pc_acpi_madt(FWCfgState *fw_cfg)
+{
+    unsigned num_lapic;
+    char unsigned *blob;
+    size_t blob_size;
+
+    /* see note on FW_CFG_MAX_CPUS in bochs_bios_init() */
+    num_lapic = pc_apic_id_limit(max_cpus);
+
+    acpi_build_madt(&blob, &blob_size, num_lapic);
+    fw_cfg_add_file(fw_cfg, "etc/acpi/APIC", blob, blob_size);
+}
+#endif
+
 FWCfgState *pc_memory_init(MemoryRegion *system_memory,
                            const char *kernel_filename,
                            const char *kernel_cmdline,
@@ -974,6 +993,10 @@  FWCfgState *pc_memory_init(MemoryRegion *system_memory,
     fw_cfg = bochs_bios_init();
     rom_set_fw(fw_cfg);
 
+#ifdef CONFIG_DYN_ACPI
+    pc_acpi_madt(fw_cfg);
+#endif
+
     if (linux_boot) {
         load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size);
     }