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 <kra...@redhat.com>
> >> Acked-by: Jan Kiszka <jan.kis...@siemens.com>
> >> Acked-by: Jason Baron <jba...@akamai.com>
> >> Acked-by: David Woodhouse <david.woodho...@intel.com>
> >> Acked-by: Gleb Natapov <g...@redhat.com>
> >> Acked-by: Marcelo Tosatti <mtosa...@redhat.com>
> >> Acked-by: Dave Frodin <dave.fro...@se-eng.com>
> >> Acked-by: Paolo Bonzini <pbonz...@redhat.com>
> >> Acked-by: Kevin O'Connor <ke...@koconnor.net>
> >> Acked-by: Laszlo Ersek <ler...@redhat.com>
> >> Acked-by: Kenji Kaneshige <kaneshige.ke...@jp.fujitsu.com>
> >> Acked-by: Isaku Yamahata <yamah...@valinux.co.jp>
> >> Acked-by: Magnus Christensson <magnus.christens...@intel.com>
> >> Acked-by: Hu Tao <hu...@cn.fujitsu.com>
> >> Acked-by: Eduardo Habkost <ehabk...@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 <ler...@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 <ke...@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
> >>
