[Qemu-devel] [PATCH v2 0/7] publish etc/acpi/APIC in fw_cfg

[Qemu-devel] [PATCH v2 0/7] publish etc/acpi/APIC in fw_cfg

[Laszlo Ersek]

v1 blurb:

  This series exports the MADT (APIC) ACPI table under the new
  "etc/acpi/APIC" fw_cfg file. I sought to follow the requirements set
  forth in [1], the new table is only visible in the patched/patched
  case. I cross-tested { master, patched } qemu with { master, patched }
  seabios (the APIC, DSDT and RSDT tables) using guest acpidump and
  dmesg.

  The -acpitable command line option is purposely ignored based on the
  last paragraph of [2]; the user isn't supposed to pass APIC with that
  option.

  checkpatch.pl complains a little but (as last time) it's a false
  alarm.

  The series is bisectable.

  [1] http://thread.gmane.org/gmane.comp.emulators.qemu/202005/focus=202072
  [2] http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/5960/focus=6008

v2:
- address (1) in
  <http://thread.gmane.org/gmane.comp.emulators.qemu/206146/focus=206195>:

  - rebase to Paolo's recent series
    <http://thread.gmane.org/gmane.comp.emulators.qemu/206196/focus=206278>,

  - patch 2 is new, fixes style of function parameter references in a
    comment [mst],

  - patch 6 is new, moves a macro definition [mst],

  - patch 7 should be structured more logically now, plus the commit
    message has grown some bits related to licensing [mst].
    acpi_table_fill_hdr() is kept distinct from the -acpitable switch's
    implementation on purpose, similarly to pc_acpi_install() in v1.

- Tested APIC / DSDT / RSDT against contents saved from v1.

Laszlo Ersek (6):
  refer to FWCfgState explicitly
  acpi_table_install(): fix funcparam formatting in leading comment
  hw/acpi: extract standard table headers as a standalone structure
  hw/acpi: export default ACPI headers using the type just introduced
  hw/acpi: export acpi_checksum()
  hw/i386: build ACPI MADT (APIC) for fw_cfg clients

Michael S. Tsirkin (1):
  hw/i386/pc.c: move IO_APIC_DEFAULT_ADDRESS to include/hw/i386/apic.h

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

[Qemu-devel] [PATCH v2 1/7] refer to FWCfgState explicitly

[Laszlo Ersek]

Currently some places use pointer-to-void even though they mean
pointer-to-FWCfgState. Clean them up.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/i386/multiboot.h  |    4 +++-
 include/hw/i386/pc.h |   19 ++++++++++---------
 include/hw/loader.h  |    3 ++-
 hw/acpi/piix4.c      |    2 +-
 hw/core/loader.c     |    2 +-
 hw/i386/multiboot.c  |    2 +-
 hw/i386/pc.c         |   24 ++++++++++++------------
 hw/i386/pc_piix.c    |    2 +-
 hw/sparc/sun4m.c     |    6 +++---
 hw/sparc64/sun4u.c   |    2 +-
 10 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/hw/i386/multiboot.h b/hw/i386/multiboot.h
index 98fb1b7..60de309 100644
--- a/hw/i386/multiboot.h
+++ b/hw/i386/multiboot.h
@@ -1,7 +1,9 @@
 #ifndef QEMU_MULTIBOOT_H
 #define QEMU_MULTIBOOT_H
 
-int load_multiboot(void *fw_cfg,
+#include "hw/nvram/fw_cfg.h"
+
+int load_multiboot(FWCfgState *fw_cfg,
                    FILE *f,
                    const char *kernel_filename,
                    const char *initrd_filename,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9bcc819..fa4b5ce 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -9,6 +9,7 @@
 #include "net/net.h"
 #include "exec/memory.h"
 #include "hw/i386/ioapic.h"
+#include "hw/nvram/fw_cfg.h"
 
 /* PC-style peripherals (also used by other machines).  */
 
@@ -80,14 +81,14 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
 void pc_cpus_init(const char *cpu_model);
 void pc_acpi_init(const char *default_dsdt);
-void *pc_memory_init(MemoryRegion *system_memory,
-                    const char *kernel_filename,
-                    const char *kernel_cmdline,
-                    const char *initrd_filename,
-                    ram_addr_t below_4g_mem_size,
-                    ram_addr_t above_4g_mem_size,
-                    MemoryRegion *rom_memory,
-                    MemoryRegion **ram_memory);
+FWCfgState *pc_memory_init(MemoryRegion *system_memory,
+                           const char *kernel_filename,
+                           const char *kernel_cmdline,
+                           const char *initrd_filename,
+                           ram_addr_t below_4g_mem_size,
+                           ram_addr_t above_4g_mem_size,
+                           MemoryRegion *rom_memory,
+                           MemoryRegion **ram_memory);
 qemu_irq *pc_allocate_cpu_irq(void);
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
 void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
@@ -111,7 +112,7 @@ void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
 
 i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
                        qemu_irq sci_irq, qemu_irq smi_irq,
-                       int kvm_enabled, void *fw_cfg);
+                       int kvm_enabled, FWCfgState *fw_cfg);
 void piix4_smbus_register_device(SMBusDevice *dev, uint8_t addr);
 
 /* hpet.c */
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 0958f06..15d4cc9 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -1,6 +1,7 @@
 #ifndef LOADER_H
 #define LOADER_H
 #include "qapi/qmp/qdict.h"
+#include "hw/nvram/fw_cfg.h"
 
 /* loader.c */
 int get_image_size(const char *filename);
@@ -30,7 +31,7 @@ int rom_add_blob(const char *name, const void *blob, size_t len,
 int rom_add_elf_program(const char *name, void *data, size_t datasize,
                         size_t romsize, hwaddr addr);
 int rom_load_all(void);
-void rom_set_fw(void *f);
+void rom_set_fw(FWCfgState *f);
 int rom_copy(uint8_t *dest, hwaddr addr, size_t size);
 void *rom_ptr(hwaddr addr);
 void do_info_roms(Monitor *mon, const QDict *qdict);
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 88386d7..200c297 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -435,7 +435,7 @@ static int piix4_pm_initfn(PCIDevice *dev)
 
 i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
                        qemu_irq sci_irq, qemu_irq smi_irq,
-                       int kvm_enabled, void *fw_cfg)
+                       int kvm_enabled, FWCfgState *fw_cfg)
 {
     PCIDevice *dev;
     PIIX4PMState *s;
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 7507914..a711145 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -733,7 +733,7 @@ int rom_load_all(void)
     return 0;
 }
 
-void rom_set_fw(void *f)
+void rom_set_fw(FWCfgState *f)
 {
     fw_cfg = f;
 }
diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index d696507..09211e0 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -124,7 +124,7 @@ static void mb_add_mod(MultibootState *s,
     s->mb_mods_count++;
 }
 
-int load_multiboot(void *fw_cfg,
+int load_multiboot(FWCfgState *fw_cfg,
                    FILE *f,
                    const char *kernel_filename,
                    const char *initrd_filename,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0d6e72b..3c12b55 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -569,9 +569,9 @@ static unsigned int pc_apic_id_limit(unsigned int max_cpus)
     return x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
 }
 
-static void *bochs_bios_init(void)
+static FWCfgState *bochs_bios_init(void)
 {
-    void *fw_cfg;
+    FWCfgState *fw_cfg;
     uint8_t *smbios_table;
     size_t smbios_len;
     uint64_t *numa_fw_cfg;
@@ -648,7 +648,7 @@ static long get_file_size(FILE *f)
     return size;
 }
 
-static void load_linux(void *fw_cfg,
+static void load_linux(FWCfgState *fw_cfg,
                        const char *kernel_filename,
                        const char *initrd_filename,
                        const char *kernel_cmdline,
@@ -924,19 +924,19 @@ void pc_acpi_init(const char *default_dsdt)
     }
 }
 
-void *pc_memory_init(MemoryRegion *system_memory,
-                    const char *kernel_filename,
-                    const char *kernel_cmdline,
-                    const char *initrd_filename,
-                    ram_addr_t below_4g_mem_size,
-                    ram_addr_t above_4g_mem_size,
-                    MemoryRegion *rom_memory,
-                    MemoryRegion **ram_memory)
+FWCfgState *pc_memory_init(MemoryRegion *system_memory,
+                           const char *kernel_filename,
+                           const char *kernel_cmdline,
+                           const char *initrd_filename,
+                           ram_addr_t below_4g_mem_size,
+                           ram_addr_t above_4g_mem_size,
+                           MemoryRegion *rom_memory,
+                           MemoryRegion **ram_memory)
 {
     int linux_boot, i;
     MemoryRegion *ram, *option_rom_mr;
     MemoryRegion *ram_below_4g, *ram_above_4g;
-    void *fw_cfg;
+    FWCfgState *fw_cfg;
 
     linux_boot = (kernel_filename != NULL);
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 943758a..da10e6d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -85,7 +85,7 @@ static void pc_init1(MemoryRegion *system_memory,
     MemoryRegion *ram_memory;
     MemoryRegion *pci_memory;
     MemoryRegion *rom_memory;
-    void *fw_cfg = NULL;
+    FWCfgState *fw_cfg = NULL;
 
     pc_cpus_init(cpu_model);
     pc_acpi_init("acpi-dsdt.aml");
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 31beb32..0b1b620 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -874,7 +874,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
     qemu_irq *cpu_halt;
     unsigned long kernel_size;
     DriveInfo *fd[MAX_FD];
-    void *fw_cfg;
+    FWCfgState *fw_cfg;
     unsigned int num_vsimms;
 
     /* init CPUs */
@@ -1592,7 +1592,7 @@ static void sun4d_hw_init(const struct sun4d_hwdef *hwdef, ram_addr_t RAM_size,
         espdma_irq, ledma_irq;
     qemu_irq esp_reset, dma_enable;
     unsigned long kernel_size;
-    void *fw_cfg;
+    FWCfgState *fw_cfg;
     DeviceState *dev;
 
     /* init CPUs */
@@ -1793,7 +1793,7 @@ static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size,
     qemu_irq fdc_tc;
     unsigned long kernel_size;
     DriveInfo *fd[MAX_FD];
-    void *fw_cfg;
+    FWCfgState *fw_cfg;
     DeviceState *dev;
     unsigned int i;
 
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 0d29620..0138252 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -818,7 +818,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     qemu_irq *ivec_irqs, *pbm_irqs;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     DriveInfo *fd[MAX_FD];
-    void *fw_cfg;
+    FWCfgState *fw_cfg;
 
     /* init CPUs */
     cpu = cpu_devinit(cpu_model, hwdef);
-- 
1.7.1

[Qemu-devel] [PATCH v2 2/7] acpi_table_install(): fix funcparam formatting in leading comment

[Laszlo Ersek]

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/acpi/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 64b8718..69cadb0 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -86,7 +86,7 @@ static int acpi_checksum(const uint8_t *data, int len)
  * is optionally overwritten from @hdrs.
  *
  * It is valid to call this function with
- * (@blob == NULL && bloblen == 0 && !has_header).
+ * (@blob == NULL && @bloblen == 0 && !@has_header).
  *
  * @hdrs->file and @hdrs->data are ignored.
  *
-- 
1.7.1

[Qemu-devel] [PATCH v2 3/7] hw/acpi: extract standard table headers as a standalone structure

[Laszlo Ersek]

This enables reuse when preparing per-table fw_cfg blobs later.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 include/hw/acpi/acpi.h |   11 +++++++++++
 hw/acpi/core.c         |   48 +++++++++++++++++++++---------------------------
 2 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index 635be7b..0e26f63 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -167,4 +167,15 @@ extern size_t acpi_tables_len;
 
 void acpi_table_add(const QemuOpts *opts, Error **errp);
 
+typedef struct acpi_table_std_header {
+    char sig[4];              /* ACPI signature (4 ASCII characters) */
+    uint32_t length;          /* Length of table, in bytes, including header */
+    uint8_t revision;         /* ACPI Specification minor version # */
+    uint8_t checksum;         /* To make sum of entire table == 0 */
+    char oem_id[6];           /* OEM identification */
+    char oem_table_id[8];     /* OEM table identification */
+    uint32_t oem_revision;    /* OEM revision number */
+    char asl_compiler_id[4];  /* ASL compiler vendor ID */
+    uint32_t asl_compiler_revision; /* ASL compiler revision number */
+} QEMU_PACKED AcpiTableStdHdr;
 #endif /* !QEMU_HW_ACPI_H */
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 69cadb0..d348f81 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -31,21 +31,13 @@
 struct acpi_table_header {
     uint16_t _length;         /* our length, not actual part of the hdr */
                               /* allows easier parsing for fw_cfg clients */
-    char sig[4];              /* ACPI signature (4 ASCII characters) */
-    uint32_t length;          /* Length of table, in bytes, including header */
-    uint8_t revision;         /* ACPI Specification minor version # */
-    uint8_t checksum;         /* To make sum of entire table == 0 */
-    char oem_id[6];           /* OEM identification */
-    char oem_table_id[8];     /* OEM table identification */
-    uint32_t oem_revision;    /* OEM revision number */
-    char asl_compiler_id[4];  /* ASL compiler vendor ID */
-    uint32_t asl_compiler_revision; /* ASL compiler revision number */
+    AcpiTableStdHdr std;
 } QEMU_PACKED;
 
-#define ACPI_TABLE_HDR_SIZE sizeof(struct acpi_table_header)
-#define ACPI_TABLE_PFX_SIZE sizeof(uint16_t)  /* size of the extra prefix */
+/* size of the extra prefix */
+#define ACPI_TABLE_PFX_SIZE offsetof(struct acpi_table_header, std)
 
-static const char unsigned dfl_hdr[ACPI_TABLE_HDR_SIZE - ACPI_TABLE_PFX_SIZE] =
+static const char unsigned dfl_hdr[sizeof(AcpiTableStdHdr)] =
     "QEMU\0\0\0\0\1\0"       /* sig (4), len(4), revno (1), csum (1) */
     "QEMUQEQEMUQEMU\1\0\0\0" /* OEM id (6), table (8), revno (4) */
     "QEMU\1\0\0\0"           /* ASL compiler ID (4), version (4) */
@@ -105,6 +97,7 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
     size_t body_size, acpi_payload_size;
     struct acpi_table_header *ext_hdr;
     unsigned changed_fields;
+    AcpiTableStdHdr *std;
 
     /* Calculate where the ACPI table body starts within the blob, plus where
      * to copy the ACPI table header from.
@@ -177,46 +170,47 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
     changed_fields = 0;
     ext_hdr->_length = cpu_to_le16(acpi_payload_size);
 
+    std = &ext_hdr->std;
     if (hdrs->has_sig) {
-        strncpy(ext_hdr->sig, hdrs->sig, sizeof ext_hdr->sig);
+        strncpy(std->sig, hdrs->sig, sizeof std->sig);
         ++changed_fields;
     }
 
-    if (has_header && le32_to_cpu(ext_hdr->length) != acpi_payload_size) {
+    if (has_header && le32_to_cpu(std->length) != acpi_payload_size) {
         fprintf(stderr,
                 "warning: ACPI table has wrong length, header says "
                 "%" PRIu32 ", actual size %zu bytes\n",
-                le32_to_cpu(ext_hdr->length), acpi_payload_size);
+                le32_to_cpu(std->length), acpi_payload_size);
     }
-    ext_hdr->length = cpu_to_le32(acpi_payload_size);
+    std->length = cpu_to_le32(acpi_payload_size);
 
     if (hdrs->has_rev) {
-        ext_hdr->revision = hdrs->rev;
+        std->revision = hdrs->rev;
         ++changed_fields;
     }
 
-    ext_hdr->checksum = 0;
+    std->checksum = 0;
 
     if (hdrs->has_oem_id) {
-        strncpy(ext_hdr->oem_id, hdrs->oem_id, sizeof ext_hdr->oem_id);
+        strncpy(std->oem_id, hdrs->oem_id, sizeof std->oem_id);
         ++changed_fields;
     }
     if (hdrs->has_oem_table_id) {
-        strncpy(ext_hdr->oem_table_id, hdrs->oem_table_id,
-                sizeof ext_hdr->oem_table_id);
+        strncpy(std->oem_table_id, hdrs->oem_table_id,
+                sizeof std->oem_table_id);
         ++changed_fields;
     }
     if (hdrs->has_oem_rev) {
-        ext_hdr->oem_revision = cpu_to_le32(hdrs->oem_rev);
+        std->oem_revision = cpu_to_le32(hdrs->oem_rev);
         ++changed_fields;
     }
     if (hdrs->has_asl_compiler_id) {
-        strncpy(ext_hdr->asl_compiler_id, hdrs->asl_compiler_id,
-                sizeof ext_hdr->asl_compiler_id);
+        strncpy(std->asl_compiler_id, hdrs->asl_compiler_id,
+                sizeof std->asl_compiler_id);
         ++changed_fields;
     }
     if (hdrs->has_asl_compiler_rev) {
-        ext_hdr->asl_compiler_revision = cpu_to_le32(hdrs->asl_compiler_rev);
+        std->asl_compiler_revision = cpu_to_le32(hdrs->asl_compiler_rev);
         ++changed_fields;
     }
 
@@ -225,8 +219,8 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
     }
 
     /* recalculate checksum */
-    ext_hdr->checksum = acpi_checksum((const char unsigned *)ext_hdr +
-                                      ACPI_TABLE_PFX_SIZE, acpi_payload_size);
+    std->checksum = acpi_checksum((const char unsigned *)std,
+                                  acpi_payload_size);
 }
 
 void acpi_table_add(const QemuOpts *opts, Error **errp)
-- 
1.7.1

[Qemu-devel] [PATCH v2 4/7] hw/acpi: export default ACPI headers using the type just introduced

[Laszlo Ersek]

This enables reuse when preparing per-table fw_cfg blobs later.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 include/hw/acpi/acpi.h |    2 ++
 hw/acpi/core.c         |   39 ++++++++++++++++++++++++---------------
 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index 0e26f63..bc7e107 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -178,4 +178,6 @@ typedef struct acpi_table_std_header {
     char asl_compiler_id[4];  /* ASL compiler vendor ID */
     uint32_t asl_compiler_revision; /* ASL compiler revision number */
 } QEMU_PACKED AcpiTableStdHdr;
+
+extern const AcpiTableStdHdr acpi_dfl_hdr;
 #endif /* !QEMU_HW_ACPI_H */
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index d348f81..36a4d03 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -37,11 +37,20 @@ struct acpi_table_header {
 /* size of the extra prefix */
 #define ACPI_TABLE_PFX_SIZE offsetof(struct acpi_table_header, std)
 
-static const char unsigned dfl_hdr[sizeof(AcpiTableStdHdr)] =
-    "QEMU\0\0\0\0\1\0"       /* sig (4), len(4), revno (1), csum (1) */
-    "QEMUQEQEMUQEMU\1\0\0\0" /* OEM id (6), table (8), revno (4) */
-    "QEMU\1\0\0\0"           /* ASL compiler ID (4), version (4) */
-    ;
+const AcpiTableStdHdr acpi_dfl_hdr = {
+    .sig = "QEMU",
+    .revision = 1,
+    .oem_id = "QEMUQE",
+    .oem_table_id = "QEMUQEMU",
+    .asl_compiler_id = "QEMU",
+#ifdef HOST_WORDS_BIGENDIAN
+    .oem_revision = 0x01000000,
+    .asl_compiler_revision = 0x01000000
+#else
+    .oem_revision = 1,
+    .asl_compiler_revision = 1
+#endif
+};
 
 char unsigned *acpi_tables;
 size_t acpi_tables_len;
@@ -74,8 +83,8 @@ static int acpi_checksum(const uint8_t *data, int len)
 /* Install a copy of the ACPI table specified in @blob.
  *
  * If @has_header is set, @blob starts with the System Description Table Header
- * structure. Otherwise, "dfl_hdr" is prepended. In any case, each header field
- * is optionally overwritten from @hdrs.
+ * structure. Otherwise, "acpi_dfl_hdr" is prepended. In any case, each header
+ * field is optionally overwritten from @hdrs.
  *
  * It is valid to call this function with
  * (@blob == NULL && @bloblen == 0 && !@has_header).
@@ -105,13 +114,13 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
     if (has_header) {
         /*   _length             | ACPI header in blob | blob body
          *   ^^^^^^^^^^^^^^^^^^^   ^^^^^^^^^^^^^^^^^^^   ^^^^^^^^^
-         *   ACPI_TABLE_PFX_SIZE     sizeof dfl_hdr      body_size
+         *   ACPI_TABLE_PFX_SIZE   sizeof acpi_dfl_hdr   body_size
          *                           == body_start
          *
          *                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
          *                           acpi_payload_size == bloblen
          */
-        body_start = sizeof dfl_hdr;
+        body_start = sizeof acpi_dfl_hdr;
 
         if (bloblen < body_start) {
             error_setg(errp, "ACPI table claiming to have header is too "
@@ -123,17 +132,17 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
     } else {
         /*   _length             | ACPI header in template | blob body
          *   ^^^^^^^^^^^^^^^^^^^   ^^^^^^^^^^^^^^^^^^^^^^^   ^^^^^^^^^^
-         *   ACPI_TABLE_PFX_SIZE       sizeof dfl_hdr        body_size
+         *   ACPI_TABLE_PFX_SIZE     sizeof acpi_dfl_hdr     body_size
          *                                                   == bloblen
          *
          *                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
          *                                  acpi_payload_size
          */
         body_start = 0;
-        hdr_src = dfl_hdr;
+        hdr_src = (const char unsigned *)&acpi_dfl_hdr;
     }
     body_size = bloblen - body_start;
-    acpi_payload_size = sizeof dfl_hdr + body_size;
+    acpi_payload_size = sizeof acpi_dfl_hdr + body_size;
 
     if (acpi_payload_size > UINT16_MAX) {
         error_setg(errp, "ACPI table too big, requested: %zu, max: %u",
@@ -149,13 +158,13 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
 
     acpi_tables = g_realloc(acpi_tables, acpi_tables_len +
                                          ACPI_TABLE_PFX_SIZE +
-                                         sizeof dfl_hdr + body_size);
+                                         sizeof acpi_dfl_hdr + body_size);
 
     ext_hdr = (struct acpi_table_header *)(acpi_tables + acpi_tables_len);
     acpi_tables_len += ACPI_TABLE_PFX_SIZE;
 
-    memcpy(acpi_tables + acpi_tables_len, hdr_src, sizeof dfl_hdr);
-    acpi_tables_len += sizeof dfl_hdr;
+    memcpy(acpi_tables + acpi_tables_len, hdr_src, sizeof acpi_dfl_hdr);
+    acpi_tables_len += sizeof acpi_dfl_hdr;
 
     if (blob != NULL) {
         memcpy(acpi_tables + acpi_tables_len, blob + body_start, body_size);
-- 
1.7.1

[Qemu-devel] [PATCH v2 5/7] hw/acpi: export acpi_checksum()

[Laszlo Ersek]

Again, this enables reuse when preparing per-table fw_cfg blobs later.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 include/hw/acpi/acpi.h |    2 ++
 hw/acpi/core.c         |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index bc7e107..7e51110 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -180,4 +180,6 @@ typedef struct acpi_table_std_header {
 } QEMU_PACKED AcpiTableStdHdr;
 
 extern const AcpiTableStdHdr acpi_dfl_hdr;
+
+int acpi_checksum(const uint8_t *data, int len);
 #endif /* !QEMU_HW_ACPI_H */
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 36a4d03..3be7e09 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -69,7 +69,7 @@ static void acpi_register_config(void)
 
 machine_init(acpi_register_config);
 
-static int acpi_checksum(const uint8_t *data, int len)
+int acpi_checksum(const uint8_t *data, int len)
 {
     int sum, i;
     sum = 0;
-- 
1.7.1

[Qemu-devel] [PATCH v2 6/7] hw/i386/pc.c: move IO_APIC_DEFAULT_ADDRESS to include/hw/i386/apic.h

[Laszlo Ersek]

From: Michael S. Tsirkin <mst@redhat.com>


Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 include/hw/i386/apic.h |    2 ++
 hw/i386/pc.c           |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/hw/i386/apic.h b/include/hw/i386/apic.h
index 1d48e02..edbb37f 100644
--- a/include/hw/i386/apic.h
+++ b/include/hw/i386/apic.h
@@ -26,6 +26,8 @@ void apic_designate_bsp(DeviceState *d);
 /* pc.c */
 DeviceState *cpu_get_current_apic(void);
 
+#define IO_APIC_DEFAULT_ADDRESS 0xfec00000
+
 /* cpu.c */
 bool cpu_is_bsp(X86CPU *cpu);
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 3c12b55..8727489 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -73,8 +73,6 @@
 #define FW_CFG_E820_TABLE (FW_CFG_ARCH_LOCAL + 3)
 #define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4)
 
-#define IO_APIC_DEFAULT_ADDRESS 0xfec00000
-
 #define E820_NR_ENTRIES		16
 
 struct e820_entry {
-- 
1.7.1

[Qemu-devel] [PATCH v2 7/7] hw/i386: build ACPI MADT (APIC) for fw_cfg clients

[Laszlo Ersek]

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: 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>

Changes since v1, based on prototype code from Michael Tsirkin:
- "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>
---
 hw/i386/acpi.h        |    9 +++
 hw/i386/acpi.c        |  159 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/i386/pc.c          |   16 +++++
 hw/i386/Makefile.objs |    1 +
 4 files changed, 185 insertions(+), 0 deletions(-)
 create mode 100644 hw/i386/acpi.h
 create mode 100644 hw/i386/acpi.c

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..f91ed21 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -53,6 +53,7 @@
 #include "qemu/bitmap.h"
 #include "qemu/config-file.h"
 #include "hw/acpi/acpi.h"
+#include "hw/i386/acpi.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
@@ -922,6 +923,19 @@ void pc_acpi_init(const char *default_dsdt)
     }
 }
 
+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);
+}
+
 FWCfgState *pc_memory_init(MemoryRegion *system_memory,
                            const char *kernel_filename,
                            const char *kernel_cmdline,
@@ -974,6 +988,8 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
     fw_cfg = bochs_bios_init();
     rom_set_fw(fw_cfg);
 
+    pc_acpi_madt(fw_cfg);
+
     if (linux_boot) {
         load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size);
     }
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 205d22e..a55da29 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-y += acpi.o
 obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
 
 obj-y += kvmvapic.o
-- 
1.7.1

Re: [Qemu-devel] [PATCH v2 6/7] hw/i386/pc.c: move IO_APIC_DEFAULT_ADDRESS to include/hw/i386/apic.h

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 323 bytes --]

On 04/15/2013 02:47 PM, Laszlo Ersek wrote:
> From: Michael S. Tsirkin <mst@redhat.com>
> 
> 
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

If Michael is the original author, don't we need his S-o-b?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH v2 7/7] hw/i386: build ACPI MADT (APIC) for fw_cfg clients

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1982 bytes --]

On 04/15/2013 02:47 PM, 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.
> 
...

Thanks for tracking that down.

> 
> Changes since v1, based on prototype code from Michael Tsirkin:
> - "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.

This paragraph of information usually appears...

> 
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---

...here, so someone doing 'git am' on the message doesn't include
information that is essential to the review process, but worthless to
the ultimate git history (readers of 'git log' don't care how many
iterations a patch took before it was ready to be included).

>  hw/i386/acpi.h        |    9 +++
>  hw/i386/acpi.c        |  159 +++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/i386/pc.c          |   16 +++++
>  hw/i386/Makefile.objs |    1 +
>  4 files changed, 185 insertions(+), 0 deletions(-)
>  create mode 100644 hw/i386/acpi.h
>  create mode 100644 hw/i386/acpi.c

Sorry - I just did a superficial review of the commit message, but this
is in code that I'm not familiar enough with to do any sort of formal
review.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH v2 6/7] hw/i386/pc.c: move IO_APIC_DEFAULT_ADDRESS to include/hw/i386/apic.h

[Laszlo Ersek]

On 04/15/13 23:06, Eric Blake wrote:
> On 04/15/2013 02:47 PM, Laszlo Ersek wrote:
>> From: Michael S. Tsirkin <mst@redhat.com>
>>
>>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> 
> If Michael is the original author, don't we need his S-o-b?

Yeah, probably. Sorry about that. Should I resend just for this?
Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2 7/7] hw/i386: build ACPI MADT (APIC) for fw_cfg clients

[Laszlo Ersek]

On 04/15/13 23:11, Eric Blake wrote:
> On 04/15/2013 02:47 PM, 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.
>>
> ...
> 
> Thanks for tracking that down.

The credits belong to Michael.


>> Changes since v1, based on prototype code from Michael Tsirkin:
>> - "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.
> 
> This paragraph of information usually appears...
> 
>>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
> 
> ...here, so someone doing 'git am' on the message doesn't include
> information that is essential to the review process, but worthless to
> the ultimate git history (readers of 'git log' don't care how many
> iterations a patch took before it was ready to be included).

I'm aware and agree in general; on this occasion I left it in on
purpose. The v1->v2 changes are somewhat relevant on their own IMHO,
plus I wanted to save for posterity that v2 of this patch has been
greatly influenced by a patch Michael sent me in private.

If you think it's important I can respin and mayhap reword.

> Sorry - I just did a superficial review of the commit message, but this
> is in code that I'm not familiar enough with to do any sort of formal
> review.

Thank you very much for the review! I can't imagine where you got the
energy from to look at random patches on the list.

Laszlo