[Qemu-devel] [PULL v2 01/25] tests: acpi: report names of expected files in verbose mode

[Qemu-devel] [PULL v2 01/25] tests: acpi: report names of expected files in verbose mode

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

print expected file name if it doesn't exists if
verbose mode is enabled*. It helps to avoid running
bios-tables-test under debugger to figure out missing
file name.

*)
verbose mode is enabled if "V" env. variable is set

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 tests/bios-tables-test.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 0352814..f0493f8 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -464,7 +464,6 @@ static GArray *load_expected_aml(test_data *data)
 {
     int i;
     AcpiSdtTable *sdt;
-    gchar *aml_file = NULL;
     GError *error = NULL;
     gboolean ret;
 
@@ -472,6 +471,7 @@ static GArray *load_expected_aml(test_data *data)
     for (i = 0; i < data->tables->len; ++i) {
         AcpiSdtTable exp_sdt;
         uint32_t signature;
+        gchar *aml_file = NULL;
         const char *ext = data->variant ? data->variant : "";
 
         sdt = &g_array_index(data->tables, AcpiSdtTable, i);
@@ -484,13 +484,21 @@ static GArray *load_expected_aml(test_data *data)
 try_again:
         aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
                                    (gchar *)&signature, ext);
-        if (data->variant && !g_file_test(aml_file, G_FILE_TEST_EXISTS)) {
-            g_free(aml_file);
+        if (getenv("V")) {
+            fprintf(stderr, "\nLooking for expected file '%s'\n", aml_file);
+        }
+        if (g_file_test(aml_file, G_FILE_TEST_EXISTS)) {
+            exp_sdt.aml_file = aml_file;
+        } else if (*ext != '\0') {
+            /* try fallback to generic (extention less) expected file */
             ext = "";
+            g_free(aml_file);
             goto try_again;
         }
-        exp_sdt.aml_file = aml_file;
-        g_assert(g_file_test(aml_file, G_FILE_TEST_EXISTS));
+        g_assert(exp_sdt.aml_file);
+        if (getenv("V")) {
+            fprintf(stderr, "\nUsing expected file '%s'\n", aml_file);
+        }
         ret = g_file_get_contents(aml_file, &exp_sdt.aml,
                                   &exp_sdt.aml_len, &error);
         g_assert(ret);
-- 
MST

[Qemu-devel] [PULL v2 02/25] acpi: add aml_debug()

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 include/hw/acpi/aml-build.h | 1 +
 hw/acpi/aml-build.c         | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 7eb51c7..70a4b14 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -252,6 +252,7 @@ void aml_append(Aml *parent_ctx, Aml *child);
 /* non block AML object primitives */
 Aml *aml_name(const char *name_format, ...) GCC_FMT_ATTR(1, 2);
 Aml *aml_name_decl(const char *name, Aml *val);
+Aml *aml_debug(void);
 Aml *aml_return(Aml *val);
 Aml *aml_int(const uint64_t val);
 Aml *aml_arg(int pos);
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index cedb74e..f7d99be 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -406,6 +406,15 @@ Aml *aml_return(Aml *val)
     return var;
 }
 
+/* ACPI 1.0b: 16.2.6.3 Debug Objects Encoding: DebugObj */
+Aml *aml_debug(void)
+{
+    Aml *var = aml_alloc();
+    build_append_byte(var->buf, 0x5B); /* ExtOpPrefix */
+    build_append_byte(var->buf, 0x31); /* DebugOp */
+    return var;
+}
+
 /*
  * ACPI 1.0b: 16.2.3 Data Objects Encoding:
  * encodes: ByteConst, WordConst, DWordConst, QWordConst, ZeroOp, OneOp
-- 
MST

[Qemu-devel] [PULL v2 03/25] acpi: add aml_refof()

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 include/hw/acpi/aml-build.h | 1 +
 hw/acpi/aml-build.c         | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 70a4b14..dd432d2 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -359,6 +359,7 @@ Aml *aml_create_qword_field(Aml *srcbuf, Aml *index, const char *name);
 Aml *aml_varpackage(uint32_t num_elements);
 Aml *aml_touuid(const char *uuid);
 Aml *aml_unicode(const char *str);
+Aml *aml_refof(Aml *arg);
 Aml *aml_derefof(Aml *arg);
 Aml *aml_sizeof(Aml *arg);
 Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target);
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index f7d99be..95f6021 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1416,6 +1416,14 @@ Aml *aml_unicode(const char *str)
     return var;
 }
 
+/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefRefOf */
+Aml *aml_refof(Aml *arg)
+{
+    Aml *var = aml_opcode(0x71 /* RefOfOp */);
+    aml_append(var, arg);
+    return var;
+}
+
 /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefDerefOf */
 Aml *aml_derefof(Aml *arg)
 {
-- 
MST

[Qemu-devel] [PULL v2 04/25] pc: acpi: remove AML for empty/not used GPE handlers

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

ACPI spec requires GPE handlers only for GPE events
that hardware implements.
So remove AML for not supported by QEMU device model
events.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/i386/acpi-build.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 279f0d7..ff98ad8 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2051,8 +2051,6 @@ build_dsdt(GArray *table_data, GArray *linker,
     {
         aml_append(scope, aml_name_decl("_HID", aml_string("ACPI0006")));
 
-        aml_append(scope, aml_method("_L00", 0, AML_NOTSERIALIZED));
-
         if (misc->is_piix4) {
             method = aml_method("_E01", 0, AML_NOTSERIALIZED);
             aml_append(method,
@@ -2060,8 +2058,6 @@ build_dsdt(GArray *table_data, GArray *linker,
             aml_append(method, aml_call0("\\_SB.PCI0.PCNT"));
             aml_append(method, aml_release(aml_name("\\_SB.PCI0.BLCK")));
             aml_append(scope, method);
-        } else {
-            aml_append(scope, aml_method("_L01", 0, AML_NOTSERIALIZED));
         }
 
         method = aml_method("_E02", 0, AML_NOTSERIALIZED);
@@ -2071,19 +2067,6 @@ build_dsdt(GArray *table_data, GArray *linker,
         method = aml_method("_E03", 0, AML_NOTSERIALIZED);
         aml_append(method, aml_call0(MEMORY_HOTPLUG_HANDLER_PATH));
         aml_append(scope, method);
-
-        aml_append(scope, aml_method("_L04", 0, AML_NOTSERIALIZED));
-        aml_append(scope, aml_method("_L05", 0, AML_NOTSERIALIZED));
-        aml_append(scope, aml_method("_L06", 0, AML_NOTSERIALIZED));
-        aml_append(scope, aml_method("_L07", 0, AML_NOTSERIALIZED));
-        aml_append(scope, aml_method("_L08", 0, AML_NOTSERIALIZED));
-        aml_append(scope, aml_method("_L09", 0, AML_NOTSERIALIZED));
-        aml_append(scope, aml_method("_L0A", 0, AML_NOTSERIALIZED));
-        aml_append(scope, aml_method("_L0B", 0, AML_NOTSERIALIZED));
-        aml_append(scope, aml_method("_L0C", 0, AML_NOTSERIALIZED));
-        aml_append(scope, aml_method("_L0D", 0, AML_NOTSERIALIZED));
-        aml_append(scope, aml_method("_L0E", 0, AML_NOTSERIALIZED));
-        aml_append(scope, aml_method("_L0F", 0, AML_NOTSERIALIZED));
     }
     aml_append(dsdt, scope);
 
-- 
MST

[Qemu-devel] [PULL v2 05/25] pc: acpi: consolidate CPU hotplug AML

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

move the former SSDT part of CPU hoplug close to DSDT part.
AML is only moved but there isn't any functional change.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/cpu_hotplug.h    |   3 +-
 hw/acpi/cpu_hotplug_acpi_table.c | 104 +++++++++++++++++++++++++++++++++++-
 hw/i386/acpi-build.c             | 112 +--------------------------------------
 3 files changed, 106 insertions(+), 113 deletions(-)

diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index f22640e..9b1d0cf 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -34,5 +34,6 @@ void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
 #define CPU_STATUS_MAP "PRS"
 #define CPU_SCAN_METHOD "PRSC"
 
-void build_cpu_hotplug_aml(Aml *ctx);
+void build_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
+                           uint16_t io_base, uint16_t io_len);
 #endif
diff --git a/hw/acpi/cpu_hotplug_acpi_table.c b/hw/acpi/cpu_hotplug_acpi_table.c
index 97bb109..730f44c 100644
--- a/hw/acpi/cpu_hotplug_acpi_table.c
+++ b/hw/acpi/cpu_hotplug_acpi_table.c
@@ -15,12 +15,19 @@
 
 #include "qemu/osdep.h"
 #include "hw/acpi/cpu_hotplug.h"
+#include "hw/i386/pc.h"
 
-void build_cpu_hotplug_aml(Aml *ctx)
+void build_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
+                           uint16_t io_base, uint16_t io_len)
 {
+    Aml *dev;
+    Aml *crs;
+    Aml *pkg;
+    Aml *field;
     Aml *method;
     Aml *if_ctx;
     Aml *else_ctx;
+    int i, apic_idx;
     Aml *sb_scope = aml_scope("_SB");
     uint8_t madt_tmpl[8] = {0x00, 0x08, 0x00, 0x00, 0x00, 0, 0, 0};
     Aml *cpu_id = aml_arg(0);
@@ -29,6 +36,9 @@ void build_cpu_hotplug_aml(Aml *ctx)
     Aml *cpus_map = aml_name(CPU_ON_BITMAP);
     Aml *zero = aml_int(0);
     Aml *one = aml_int(1);
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
+    CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
+    PCMachineState *pcms = PC_MACHINE(machine);
 
     /*
      * _MAT method - creates an madt apic buffer
@@ -132,5 +142,97 @@ void build_cpu_hotplug_aml(Aml *ctx)
     }
     aml_append(sb_scope, method);
 
+    /* The current AML generator can cover the APIC ID range [0..255],
+     * inclusive, for VCPU hotplug. */
+    QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
+    g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
+
+    /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
+    dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A06")));
+    aml_append(dev,
+        aml_name_decl("_UID", aml_string("CPU Hotplug resources"))
+    );
+    /* device present, functioning, decoding, not shown in UI */
+    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
+    crs = aml_resource_template();
+    aml_append(crs,
+        aml_io(AML_DECODE16, io_base, io_base, 1, io_len)
+    );
+    aml_append(dev, aml_name_decl("_CRS", crs));
+    aml_append(sb_scope, dev);
+    /* declare CPU hotplug MMIO region and PRS field to access it */
+    aml_append(sb_scope, aml_operation_region(
+        "PRST", AML_SYSTEM_IO, aml_int(io_base), io_len));
+    field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
+    aml_append(field, aml_named_field("PRS", 256));
+    aml_append(sb_scope, field);
+
+    /* build Processor object for each processor */
+    for (i = 0; i < apic_ids->len; i++) {
+        int apic_id = apic_ids->cpus[i].arch_id;
+
+        assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
+
+        dev = aml_processor(apic_id, 0, 0, "CP%.02X", apic_id);
+
+        method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
+        aml_append(method,
+            aml_return(aml_call1(CPU_MAT_METHOD, aml_int(apic_id))));
+        aml_append(dev, method);
+
+        method = aml_method("_STA", 0, AML_NOTSERIALIZED);
+        aml_append(method,
+            aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(apic_id))));
+        aml_append(dev, method);
+
+        method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
+        aml_append(method,
+            aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(apic_id),
+                aml_arg(0)))
+        );
+        aml_append(dev, method);
+
+        aml_append(sb_scope, dev);
+    }
+
+    /* build this code:
+     *   Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} ...}
+     */
+    /* Arg0 = Processor ID = APIC ID */
+    method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED);
+    for (i = 0; i < apic_ids->len; i++) {
+        int apic_id = apic_ids->cpus[i].arch_id;
+
+        if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(apic_id)));
+        aml_append(if_ctx,
+            aml_notify(aml_name("CP%.02X", apic_id), aml_arg(1))
+        );
+        aml_append(method, if_ctx);
+    }
+    aml_append(sb_scope, method);
+
+    /* build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })"
+     *
+     * Note: The ability to create variable-sized packages was first
+     * introduced in ACPI 2.0. ACPI 1.0 only allowed fixed-size packages
+     * ith up to 255 elements. Windows guests up to win2k8 fail when
+     * VarPackageOp is used.
+     */
+    pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) :
+                                       aml_varpackage(pcms->apic_id_limit);
+
+    for (i = 0, apic_idx = 0; i < apic_ids->len; i++) {
+        int apic_id = apic_ids->cpus[i].arch_id;
+
+        for (; apic_idx < apic_id; apic_idx++) {
+            aml_append(pkg, aml_int(0));
+        }
+        aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0));
+        apic_idx = apic_id + 1;
+    }
+    aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
+    g_free(apic_ids);
+
     aml_append(ctx, sb_scope);
 }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ff98ad8..822230f 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -943,114 +943,6 @@ static Aml *build_crs(PCIHostState *host,
     return crs;
 }
 
-static void build_processor_devices(Aml *sb_scope, MachineState *machine,
-                                    AcpiPmInfo *pm)
-{
-    int i, apic_idx;
-    Aml *dev;
-    Aml *crs;
-    Aml *pkg;
-    Aml *field;
-    Aml *ifctx;
-    Aml *method;
-    MachineClass *mc = MACHINE_GET_CLASS(machine);
-    CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
-    PCMachineState *pcms = PC_MACHINE(machine);
-
-    /* The current AML generator can cover the APIC ID range [0..255],
-     * inclusive, for VCPU hotplug. */
-    QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
-    g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
-
-    /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
-    dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
-    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A06")));
-    aml_append(dev,
-        aml_name_decl("_UID", aml_string("CPU Hotplug resources"))
-    );
-    /* device present, functioning, decoding, not shown in UI */
-    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
-    crs = aml_resource_template();
-    aml_append(crs,
-        aml_io(AML_DECODE16, pm->cpu_hp_io_base, pm->cpu_hp_io_base, 1,
-               pm->cpu_hp_io_len)
-    );
-    aml_append(dev, aml_name_decl("_CRS", crs));
-    aml_append(sb_scope, dev);
-    /* declare CPU hotplug MMIO region and PRS field to access it */
-    aml_append(sb_scope, aml_operation_region(
-        "PRST", AML_SYSTEM_IO, aml_int(pm->cpu_hp_io_base), pm->cpu_hp_io_len));
-    field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
-    aml_append(field, aml_named_field("PRS", 256));
-    aml_append(sb_scope, field);
-
-    /* build Processor object for each processor */
-    for (i = 0; i < apic_ids->len; i++) {
-        int apic_id = apic_ids->cpus[i].arch_id;
-
-        assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
-
-        dev = aml_processor(apic_id, 0, 0, "CP%.02X", apic_id);
-
-        method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
-        aml_append(method,
-            aml_return(aml_call1(CPU_MAT_METHOD, aml_int(apic_id))));
-        aml_append(dev, method);
-
-        method = aml_method("_STA", 0, AML_NOTSERIALIZED);
-        aml_append(method,
-            aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(apic_id))));
-        aml_append(dev, method);
-
-        method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
-        aml_append(method,
-            aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(apic_id),
-                aml_arg(0)))
-        );
-        aml_append(dev, method);
-
-        aml_append(sb_scope, dev);
-    }
-
-    /* build this code:
-     *   Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} ...}
-     */
-    /* Arg0 = Processor ID = APIC ID */
-    method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED);
-    for (i = 0; i < apic_ids->len; i++) {
-        int apic_id = apic_ids->cpus[i].arch_id;
-
-        ifctx = aml_if(aml_equal(aml_arg(0), aml_int(apic_id)));
-        aml_append(ifctx,
-            aml_notify(aml_name("CP%.02X", apic_id), aml_arg(1))
-        );
-        aml_append(method, ifctx);
-    }
-    aml_append(sb_scope, method);
-
-    /* build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })"
-     *
-     * Note: The ability to create variable-sized packages was first
-     * introduced in ACPI 2.0. ACPI 1.0 only allowed fixed-size packages
-     * ith up to 255 elements. Windows guests up to win2k8 fail when
-     * VarPackageOp is used.
-     */
-    pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) :
-                                       aml_varpackage(pcms->apic_id_limit);
-
-    for (i = 0, apic_idx = 0; i < apic_ids->len; i++) {
-        int apic_id = apic_ids->cpus[i].arch_id;
-
-        for (; apic_idx < apic_id; apic_idx++) {
-            aml_append(pkg, aml_int(0));
-        }
-        aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0));
-        apic_idx = apic_id + 1;
-    }
-    aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
-    g_free(apic_ids);
-}
-
 static void build_memory_devices(Aml *sb_scope, int nr_mem,
                                  uint16_t io_base, uint16_t io_len)
 {
@@ -2043,7 +1935,7 @@ build_dsdt(GArray *table_data, GArray *linker,
         build_q35_pci0_int(dsdt);
     }
 
-    build_cpu_hotplug_aml(dsdt);
+    build_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base, pm->cpu_hp_io_len);
     build_memory_hotplug_aml(dsdt, nr_mem, pm->mem_hp_io_base,
                              pm->mem_hp_io_len);
 
@@ -2305,8 +2197,6 @@ build_dsdt(GArray *table_data, GArray *linker,
 
     sb_scope = aml_scope("\\_SB");
     {
-        build_processor_devices(sb_scope, machine, pm);
-
         build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base,
                              pm->mem_hp_io_len);
 
-- 
MST

[Qemu-devel] [PULL v2 06/25] pc: acpi: consolidate \GPE._E02 with the rest of CPU hotplug AML

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/acpi/cpu_hotplug_acpi_table.c | 4 ++++
 hw/i386/acpi-build.c             | 4 ----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/acpi/cpu_hotplug_acpi_table.c b/hw/acpi/cpu_hotplug_acpi_table.c
index 730f44c..c31f346 100644
--- a/hw/acpi/cpu_hotplug_acpi_table.c
+++ b/hw/acpi/cpu_hotplug_acpi_table.c
@@ -235,4 +235,8 @@ void build_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
     g_free(apic_ids);
 
     aml_append(ctx, sb_scope);
+
+    method = aml_method("\\_GPE._E02", 0, AML_NOTSERIALIZED);
+    aml_append(method, aml_call0("\\_SB." CPU_SCAN_METHOD));
+    aml_append(ctx, method);
 }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 822230f..63e2723 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1952,10 +1952,6 @@ build_dsdt(GArray *table_data, GArray *linker,
             aml_append(scope, method);
         }
 
-        method = aml_method("_E02", 0, AML_NOTSERIALIZED);
-        aml_append(method, aml_call0("\\_SB." CPU_SCAN_METHOD));
-        aml_append(scope, method);
-
         method = aml_method("_E03", 0, AML_NOTSERIALIZED);
         aml_append(method, aml_call0(MEMORY_HOTPLUG_HANDLER_PATH));
         aml_append(scope, method);
-- 
MST

[Qemu-devel] [PULL v2 07/25] pc: acpi: cpu-hotplug: make AML CPU_foo defines local to cpu_hotplug_acpi_table.c

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

now as those defines are used only locally inside of
cpu_hotplug_acpi_table.c, move them out of header file.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 include/hw/acpi/cpu_hotplug.h    | 7 -------
 hw/acpi/cpu_hotplug_acpi_table.c | 7 +++++++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 9b1d0cf..565f96c 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -27,13 +27,6 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
 void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
                            AcpiCpuHotplug *gpe_cpu, uint16_t base);
 
-#define CPU_EJECT_METHOD "CPEJ"
-#define CPU_MAT_METHOD "CPMA"
-#define CPU_ON_BITMAP "CPON"
-#define CPU_STATUS_METHOD "CPST"
-#define CPU_STATUS_MAP "PRS"
-#define CPU_SCAN_METHOD "PRSC"
-
 void build_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
                            uint16_t io_base, uint16_t io_len);
 #endif
diff --git a/hw/acpi/cpu_hotplug_acpi_table.c b/hw/acpi/cpu_hotplug_acpi_table.c
index c31f346..9fdde6d 100644
--- a/hw/acpi/cpu_hotplug_acpi_table.c
+++ b/hw/acpi/cpu_hotplug_acpi_table.c
@@ -17,6 +17,13 @@
 #include "hw/acpi/cpu_hotplug.h"
 #include "hw/i386/pc.h"
 
+#define CPU_EJECT_METHOD "CPEJ"
+#define CPU_MAT_METHOD "CPMA"
+#define CPU_ON_BITMAP "CPON"
+#define CPU_STATUS_METHOD "CPST"
+#define CPU_STATUS_MAP "PRS"
+#define CPU_SCAN_METHOD "PRSC"
+
 void build_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
                            uint16_t io_base, uint16_t io_len)
 {
-- 
MST

[Qemu-devel] [PULL v2 08/25] pc: acpi: mark current CPU hotplug functions as legacy

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 include/hw/acpi/cpu_hotplug.h    | 12 ++++++------
 hw/acpi/cpu_hotplug.c            |  8 ++++----
 hw/acpi/cpu_hotplug_acpi_table.c |  4 ++--
 hw/acpi/ich9.c                   |  7 ++++---
 hw/acpi/piix4.c                  |  6 +++---
 hw/i386/acpi-build.c             |  3 ++-
 6 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 565f96c..241b50f 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -21,12 +21,12 @@ typedef struct AcpiCpuHotplug {
     uint8_t sts[ACPI_GPE_PROC_LEN];
 } AcpiCpuHotplug;
 
-void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
-                      AcpiCpuHotplug *g, DeviceState *dev, Error **errp);
+void legacy_acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
+                             AcpiCpuHotplug *g, DeviceState *dev, Error **errp);
 
-void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
-                           AcpiCpuHotplug *gpe_cpu, uint16_t base);
+void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
+                                  AcpiCpuHotplug *gpe_cpu, uint16_t base);
 
-void build_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
-                           uint16_t io_base, uint16_t io_len);
+void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
+                                  uint16_t io_base, uint16_t io_len);
 #endif
diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 4d86743..ba9d903 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -54,8 +54,8 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
     g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
 }
 
-void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
-                      AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
+void legacy_acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
+                             AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
 {
     acpi_set_cpu_present_bit(g, CPU(dev), errp);
     if (*errp != NULL) {
@@ -65,8 +65,8 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
     acpi_send_gpe_event(ar, irq, ACPI_CPU_HOTPLUG_STATUS);
 }
 
-void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
-                           AcpiCpuHotplug *gpe_cpu, uint16_t base)
+void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
+                                  AcpiCpuHotplug *gpe_cpu, uint16_t base)
 {
     CPUState *cpu;
 
diff --git a/hw/acpi/cpu_hotplug_acpi_table.c b/hw/acpi/cpu_hotplug_acpi_table.c
index 9fdde6d..fc79c54 100644
--- a/hw/acpi/cpu_hotplug_acpi_table.c
+++ b/hw/acpi/cpu_hotplug_acpi_table.c
@@ -24,8 +24,8 @@
 #define CPU_STATUS_MAP "PRS"
 #define CPU_SCAN_METHOD "PRSC"
 
-void build_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
-                           uint16_t io_base, uint16_t io_len)
+void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
+                                  uint16_t io_base, uint16_t io_len)
 {
     Aml *dev;
     Aml *crs;
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 27e978f..af340d0 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -273,8 +273,8 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
     pm->powerdown_notifier.notify = pm_powerdown_req;
     qemu_register_powerdown_notifier(&pm->powerdown_notifier);
 
-    acpi_cpu_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci),
-                          &pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE);
+    legacy_acpi_cpu_hotplug_init(pci_address_space_io(lpc_pci),
+        OBJECT(lpc_pci), &pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE);
 
     if (pm->acpi_memory_hotplug.is_enabled) {
         acpi_memory_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci),
@@ -437,7 +437,8 @@ void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, Error **errp)
         acpi_memory_plug_cb(&pm->acpi_regs, pm->irq, &pm->acpi_memory_hotplug,
                             dev, errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
-        acpi_cpu_plug_cb(&pm->acpi_regs, pm->irq, &pm->gpe_cpu, dev, errp);
+        legacy_acpi_cpu_plug_cb(&pm->acpi_regs, pm->irq,
+                                &pm->gpe_cpu, dev, errp);
     } else {
         error_setg(errp, "acpi: device plug request for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b3e3bb3..522c9a8 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -353,7 +353,7 @@ static void piix4_device_plug_cb(HotplugHandler *hotplug_dev,
         acpi_pcihp_device_plug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug, dev,
                                   errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
-        acpi_cpu_plug_cb(&s->ar, s->irq, &s->gpe_cpu, dev, errp);
+        legacy_acpi_cpu_plug_cb(&s->ar, s->irq, &s->gpe_cpu, dev, errp);
     } else {
         error_setg(errp, "acpi: device plug request for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
@@ -571,8 +571,8 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
     acpi_pcihp_init(OBJECT(s), &s->acpi_pci_hotplug, bus, parent,
                     s->use_acpi_pci_hotplug);
 
-    acpi_cpu_hotplug_init(parent, OBJECT(s), &s->gpe_cpu,
-                          PIIX4_CPU_HOTPLUG_IO_BASE);
+    legacy_acpi_cpu_hotplug_init(parent, OBJECT(s), &s->gpe_cpu,
+                                 PIIX4_CPU_HOTPLUG_IO_BASE);
 
     if (s->acpi_memory_hotplug.is_enabled) {
         acpi_memory_hotplug_init(parent, OBJECT(s), &s->acpi_memory_hotplug);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 63e2723..2f6de43 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1935,7 +1935,8 @@ build_dsdt(GArray *table_data, GArray *linker,
         build_q35_pci0_int(dsdt);
     }
 
-    build_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base, pm->cpu_hp_io_len);
+    build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base,
+                                 pm->cpu_hp_io_len);
     build_memory_hotplug_aml(dsdt, nr_mem, pm->mem_hp_io_base,
                              pm->mem_hp_io_len);
 
-- 
MST

[Qemu-devel] [PULL v2 09/25] pc: acpi: consolidate legacy CPU hotplug in one file

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

Since AML part of CPU hotplug is tightly coupled with
its hardware part (IO port layout/protocol), move
build_legacy_cpu_hotplug_aml() to cpu_hotplug.c
and remove empty cpu_hotplug_acpi_table.c

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/acpi/cpu_hotplug.c            | 232 ++++++++++++++++++++++++++++++++++++
 hw/acpi/cpu_hotplug_acpi_table.c | 249 ---------------------------------------
 hw/acpi/Makefile.objs            |   2 +-
 3 files changed, 233 insertions(+), 250 deletions(-)
 delete mode 100644 hw/acpi/cpu_hotplug_acpi_table.c

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index ba9d903..2d4e034 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -14,6 +14,14 @@
 #include "hw/acpi/cpu_hotplug.h"
 #include "qapi/error.h"
 #include "qom/cpu.h"
+#include "hw/i386/pc.h"
+
+#define CPU_EJECT_METHOD "CPEJ"
+#define CPU_MAT_METHOD "CPMA"
+#define CPU_ON_BITMAP "CPON"
+#define CPU_STATUS_METHOD "CPST"
+#define CPU_STATUS_MAP "PRS"
+#define CPU_SCAN_METHOD "PRSC"
 
 static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
 {
@@ -77,3 +85,227 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
                           gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
     memory_region_add_subregion(parent, base, &gpe_cpu->io);
 }
+
+void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
+                                  uint16_t io_base, uint16_t io_len)
+{
+    Aml *dev;
+    Aml *crs;
+    Aml *pkg;
+    Aml *field;
+    Aml *method;
+    Aml *if_ctx;
+    Aml *else_ctx;
+    int i, apic_idx;
+    Aml *sb_scope = aml_scope("_SB");
+    uint8_t madt_tmpl[8] = {0x00, 0x08, 0x00, 0x00, 0x00, 0, 0, 0};
+    Aml *cpu_id = aml_arg(0);
+    Aml *cpu_on = aml_local(0);
+    Aml *madt = aml_local(1);
+    Aml *cpus_map = aml_name(CPU_ON_BITMAP);
+    Aml *zero = aml_int(0);
+    Aml *one = aml_int(1);
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
+    CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
+    PCMachineState *pcms = PC_MACHINE(machine);
+
+    /*
+     * _MAT method - creates an madt apic buffer
+     * cpu_id = Arg0 = Processor ID = Local APIC ID
+     * cpu_on = Local0 = CPON flag for this cpu
+     * madt = Local1 = Buffer (in madt apic form) to return
+     */
+    method = aml_method(CPU_MAT_METHOD, 1, AML_NOTSERIALIZED);
+    aml_append(method,
+        aml_store(aml_derefof(aml_index(cpus_map, cpu_id)), cpu_on));
+    aml_append(method,
+        aml_store(aml_buffer(sizeof(madt_tmpl), madt_tmpl), madt));
+    /* Update the processor id, lapic id, and enable/disable status */
+    aml_append(method, aml_store(cpu_id, aml_index(madt, aml_int(2))));
+    aml_append(method, aml_store(cpu_id, aml_index(madt, aml_int(3))));
+    aml_append(method, aml_store(cpu_on, aml_index(madt, aml_int(4))));
+    aml_append(method, aml_return(madt));
+    aml_append(sb_scope, method);
+
+    /*
+     * _STA method - return ON status of cpu
+     * cpu_id = Arg0 = Processor ID = Local APIC ID
+     * cpu_on = Local0 = CPON flag for this cpu
+     */
+    method = aml_method(CPU_STATUS_METHOD, 1, AML_NOTSERIALIZED);
+    aml_append(method,
+        aml_store(aml_derefof(aml_index(cpus_map, cpu_id)), cpu_on));
+    if_ctx = aml_if(cpu_on);
+    {
+        aml_append(if_ctx, aml_return(aml_int(0xF)));
+    }
+    aml_append(method, if_ctx);
+    else_ctx = aml_else();
+    {
+        aml_append(else_ctx, aml_return(zero));
+    }
+    aml_append(method, else_ctx);
+    aml_append(sb_scope, method);
+
+    method = aml_method(CPU_EJECT_METHOD, 2, AML_NOTSERIALIZED);
+    aml_append(method, aml_sleep(200));
+    aml_append(sb_scope, method);
+
+    method = aml_method(CPU_SCAN_METHOD, 0, AML_NOTSERIALIZED);
+    {
+        Aml *while_ctx, *if_ctx2, *else_ctx2;
+        Aml *bus_check_evt = aml_int(1);
+        Aml *remove_evt = aml_int(3);
+        Aml *status_map = aml_local(5); /* Local5 = active cpu bitmap */
+        Aml *byte = aml_local(2); /* Local2 = last read byte from bitmap */
+        Aml *idx = aml_local(0); /* Processor ID / APIC ID iterator */
+        Aml *is_cpu_on = aml_local(1); /* Local1 = CPON flag for cpu */
+        Aml *status = aml_local(3); /* Local3 = active state for cpu */
+
+        aml_append(method, aml_store(aml_name(CPU_STATUS_MAP), status_map));
+        aml_append(method, aml_store(zero, byte));
+        aml_append(method, aml_store(zero, idx));
+
+        /* While (idx < SizeOf(CPON)) */
+        while_ctx = aml_while(aml_lless(idx, aml_sizeof(cpus_map)));
+        aml_append(while_ctx,
+            aml_store(aml_derefof(aml_index(cpus_map, idx)), is_cpu_on));
+
+        if_ctx = aml_if(aml_and(idx, aml_int(0x07), NULL));
+        {
+            /* Shift down previously read bitmap byte */
+            aml_append(if_ctx, aml_shiftright(byte, one, byte));
+        }
+        aml_append(while_ctx, if_ctx);
+
+        else_ctx = aml_else();
+        {
+            /* Read next byte from cpu bitmap */
+            aml_append(else_ctx, aml_store(aml_derefof(aml_index(status_map,
+                       aml_shiftright(idx, aml_int(3), NULL))), byte));
+        }
+        aml_append(while_ctx, else_ctx);
+
+        aml_append(while_ctx, aml_store(aml_and(byte, one, NULL), status));
+        if_ctx = aml_if(aml_lnot(aml_equal(is_cpu_on, status)));
+        {
+            /* State change - update CPON with new state */
+            aml_append(if_ctx, aml_store(status, aml_index(cpus_map, idx)));
+            if_ctx2 = aml_if(aml_equal(status, one));
+            {
+                aml_append(if_ctx2,
+                    aml_call2(AML_NOTIFY_METHOD, idx, bus_check_evt));
+            }
+            aml_append(if_ctx, if_ctx2);
+            else_ctx2 = aml_else();
+            {
+                aml_append(else_ctx2,
+                    aml_call2(AML_NOTIFY_METHOD, idx, remove_evt));
+            }
+        }
+        aml_append(if_ctx, else_ctx2);
+        aml_append(while_ctx, if_ctx);
+
+        aml_append(while_ctx, aml_increment(idx)); /* go to next cpu */
+        aml_append(method, while_ctx);
+    }
+    aml_append(sb_scope, method);
+
+    /* The current AML generator can cover the APIC ID range [0..255],
+     * inclusive, for VCPU hotplug. */
+    QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
+    g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
+
+    /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
+    dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A06")));
+    aml_append(dev,
+        aml_name_decl("_UID", aml_string("CPU Hotplug resources"))
+    );
+    /* device present, functioning, decoding, not shown in UI */
+    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
+    crs = aml_resource_template();
+    aml_append(crs,
+        aml_io(AML_DECODE16, io_base, io_base, 1, io_len)
+    );
+    aml_append(dev, aml_name_decl("_CRS", crs));
+    aml_append(sb_scope, dev);
+    /* declare CPU hotplug MMIO region and PRS field to access it */
+    aml_append(sb_scope, aml_operation_region(
+        "PRST", AML_SYSTEM_IO, aml_int(io_base), io_len));
+    field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
+    aml_append(field, aml_named_field("PRS", 256));
+    aml_append(sb_scope, field);
+
+    /* build Processor object for each processor */
+    for (i = 0; i < apic_ids->len; i++) {
+        int apic_id = apic_ids->cpus[i].arch_id;
+
+        assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
+
+        dev = aml_processor(apic_id, 0, 0, "CP%.02X", apic_id);
+
+        method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
+        aml_append(method,
+            aml_return(aml_call1(CPU_MAT_METHOD, aml_int(apic_id))));
+        aml_append(dev, method);
+
+        method = aml_method("_STA", 0, AML_NOTSERIALIZED);
+        aml_append(method,
+            aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(apic_id))));
+        aml_append(dev, method);
+
+        method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
+        aml_append(method,
+            aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(apic_id),
+                aml_arg(0)))
+        );
+        aml_append(dev, method);
+
+        aml_append(sb_scope, dev);
+    }
+
+    /* build this code:
+     *   Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} ...}
+     */
+    /* Arg0 = Processor ID = APIC ID */
+    method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED);
+    for (i = 0; i < apic_ids->len; i++) {
+        int apic_id = apic_ids->cpus[i].arch_id;
+
+        if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(apic_id)));
+        aml_append(if_ctx,
+            aml_notify(aml_name("CP%.02X", apic_id), aml_arg(1))
+        );
+        aml_append(method, if_ctx);
+    }
+    aml_append(sb_scope, method);
+
+    /* build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })"
+     *
+     * Note: The ability to create variable-sized packages was first
+     * introduced in ACPI 2.0. ACPI 1.0 only allowed fixed-size packages
+     * ith up to 255 elements. Windows guests up to win2k8 fail when
+     * VarPackageOp is used.
+     */
+    pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) :
+                                       aml_varpackage(pcms->apic_id_limit);
+
+    for (i = 0, apic_idx = 0; i < apic_ids->len; i++) {
+        int apic_id = apic_ids->cpus[i].arch_id;
+
+        for (; apic_idx < apic_id; apic_idx++) {
+            aml_append(pkg, aml_int(0));
+        }
+        aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0));
+        apic_idx = apic_id + 1;
+    }
+    aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
+    g_free(apic_ids);
+
+    aml_append(ctx, sb_scope);
+
+    method = aml_method("\\_GPE._E02", 0, AML_NOTSERIALIZED);
+    aml_append(method, aml_call0("\\_SB." CPU_SCAN_METHOD));
+    aml_append(ctx, method);
+}
diff --git a/hw/acpi/cpu_hotplug_acpi_table.c b/hw/acpi/cpu_hotplug_acpi_table.c
deleted file mode 100644
index fc79c54..0000000
--- a/hw/acpi/cpu_hotplug_acpi_table.c
+++ /dev/null
@@ -1,249 +0,0 @@
-/*
- * 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/>.
- */
-
-#include "qemu/osdep.h"
-#include "hw/acpi/cpu_hotplug.h"
-#include "hw/i386/pc.h"
-
-#define CPU_EJECT_METHOD "CPEJ"
-#define CPU_MAT_METHOD "CPMA"
-#define CPU_ON_BITMAP "CPON"
-#define CPU_STATUS_METHOD "CPST"
-#define CPU_STATUS_MAP "PRS"
-#define CPU_SCAN_METHOD "PRSC"
-
-void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
-                                  uint16_t io_base, uint16_t io_len)
-{
-    Aml *dev;
-    Aml *crs;
-    Aml *pkg;
-    Aml *field;
-    Aml *method;
-    Aml *if_ctx;
-    Aml *else_ctx;
-    int i, apic_idx;
-    Aml *sb_scope = aml_scope("_SB");
-    uint8_t madt_tmpl[8] = {0x00, 0x08, 0x00, 0x00, 0x00, 0, 0, 0};
-    Aml *cpu_id = aml_arg(0);
-    Aml *cpu_on = aml_local(0);
-    Aml *madt = aml_local(1);
-    Aml *cpus_map = aml_name(CPU_ON_BITMAP);
-    Aml *zero = aml_int(0);
-    Aml *one = aml_int(1);
-    MachineClass *mc = MACHINE_GET_CLASS(machine);
-    CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
-    PCMachineState *pcms = PC_MACHINE(machine);
-
-    /*
-     * _MAT method - creates an madt apic buffer
-     * cpu_id = Arg0 = Processor ID = Local APIC ID
-     * cpu_on = Local0 = CPON flag for this cpu
-     * madt = Local1 = Buffer (in madt apic form) to return
-     */
-    method = aml_method(CPU_MAT_METHOD, 1, AML_NOTSERIALIZED);
-    aml_append(method,
-        aml_store(aml_derefof(aml_index(cpus_map, cpu_id)), cpu_on));
-    aml_append(method,
-        aml_store(aml_buffer(sizeof(madt_tmpl), madt_tmpl), madt));
-    /* Update the processor id, lapic id, and enable/disable status */
-    aml_append(method, aml_store(cpu_id, aml_index(madt, aml_int(2))));
-    aml_append(method, aml_store(cpu_id, aml_index(madt, aml_int(3))));
-    aml_append(method, aml_store(cpu_on, aml_index(madt, aml_int(4))));
-    aml_append(method, aml_return(madt));
-    aml_append(sb_scope, method);
-
-    /*
-     * _STA method - return ON status of cpu
-     * cpu_id = Arg0 = Processor ID = Local APIC ID
-     * cpu_on = Local0 = CPON flag for this cpu
-     */
-    method = aml_method(CPU_STATUS_METHOD, 1, AML_NOTSERIALIZED);
-    aml_append(method,
-        aml_store(aml_derefof(aml_index(cpus_map, cpu_id)), cpu_on));
-    if_ctx = aml_if(cpu_on);
-    {
-        aml_append(if_ctx, aml_return(aml_int(0xF)));
-    }
-    aml_append(method, if_ctx);
-    else_ctx = aml_else();
-    {
-        aml_append(else_ctx, aml_return(zero));
-    }
-    aml_append(method, else_ctx);
-    aml_append(sb_scope, method);
-
-    method = aml_method(CPU_EJECT_METHOD, 2, AML_NOTSERIALIZED);
-    aml_append(method, aml_sleep(200));
-    aml_append(sb_scope, method);
-
-    method = aml_method(CPU_SCAN_METHOD, 0, AML_NOTSERIALIZED);
-    {
-        Aml *while_ctx, *if_ctx2, *else_ctx2;
-        Aml *bus_check_evt = aml_int(1);
-        Aml *remove_evt = aml_int(3);
-        Aml *status_map = aml_local(5); /* Local5 = active cpu bitmap */
-        Aml *byte = aml_local(2); /* Local2 = last read byte from bitmap */
-        Aml *idx = aml_local(0); /* Processor ID / APIC ID iterator */
-        Aml *is_cpu_on = aml_local(1); /* Local1 = CPON flag for cpu */
-        Aml *status = aml_local(3); /* Local3 = active state for cpu */
-
-        aml_append(method, aml_store(aml_name(CPU_STATUS_MAP), status_map));
-        aml_append(method, aml_store(zero, byte));
-        aml_append(method, aml_store(zero, idx));
-
-        /* While (idx < SizeOf(CPON)) */
-        while_ctx = aml_while(aml_lless(idx, aml_sizeof(cpus_map)));
-        aml_append(while_ctx,
-            aml_store(aml_derefof(aml_index(cpus_map, idx)), is_cpu_on));
-
-        if_ctx = aml_if(aml_and(idx, aml_int(0x07), NULL));
-        {
-            /* Shift down previously read bitmap byte */
-            aml_append(if_ctx, aml_shiftright(byte, one, byte));
-        }
-        aml_append(while_ctx, if_ctx);
-
-        else_ctx = aml_else();
-        {
-            /* Read next byte from cpu bitmap */
-            aml_append(else_ctx, aml_store(aml_derefof(aml_index(status_map,
-                       aml_shiftright(idx, aml_int(3), NULL))), byte));
-        }
-        aml_append(while_ctx, else_ctx);
-
-        aml_append(while_ctx, aml_store(aml_and(byte, one, NULL), status));
-        if_ctx = aml_if(aml_lnot(aml_equal(is_cpu_on, status)));
-        {
-            /* State change - update CPON with new state */
-            aml_append(if_ctx, aml_store(status, aml_index(cpus_map, idx)));
-            if_ctx2 = aml_if(aml_equal(status, one));
-            {
-                aml_append(if_ctx2,
-                    aml_call2(AML_NOTIFY_METHOD, idx, bus_check_evt));
-            }
-            aml_append(if_ctx, if_ctx2);
-            else_ctx2 = aml_else();
-            {
-                aml_append(else_ctx2,
-                    aml_call2(AML_NOTIFY_METHOD, idx, remove_evt));
-            }
-        }
-        aml_append(if_ctx, else_ctx2);
-        aml_append(while_ctx, if_ctx);
-
-        aml_append(while_ctx, aml_increment(idx)); /* go to next cpu */
-        aml_append(method, while_ctx);
-    }
-    aml_append(sb_scope, method);
-
-    /* The current AML generator can cover the APIC ID range [0..255],
-     * inclusive, for VCPU hotplug. */
-    QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
-    g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
-
-    /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
-    dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
-    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A06")));
-    aml_append(dev,
-        aml_name_decl("_UID", aml_string("CPU Hotplug resources"))
-    );
-    /* device present, functioning, decoding, not shown in UI */
-    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
-    crs = aml_resource_template();
-    aml_append(crs,
-        aml_io(AML_DECODE16, io_base, io_base, 1, io_len)
-    );
-    aml_append(dev, aml_name_decl("_CRS", crs));
-    aml_append(sb_scope, dev);
-    /* declare CPU hotplug MMIO region and PRS field to access it */
-    aml_append(sb_scope, aml_operation_region(
-        "PRST", AML_SYSTEM_IO, aml_int(io_base), io_len));
-    field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
-    aml_append(field, aml_named_field("PRS", 256));
-    aml_append(sb_scope, field);
-
-    /* build Processor object for each processor */
-    for (i = 0; i < apic_ids->len; i++) {
-        int apic_id = apic_ids->cpus[i].arch_id;
-
-        assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
-
-        dev = aml_processor(apic_id, 0, 0, "CP%.02X", apic_id);
-
-        method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
-        aml_append(method,
-            aml_return(aml_call1(CPU_MAT_METHOD, aml_int(apic_id))));
-        aml_append(dev, method);
-
-        method = aml_method("_STA", 0, AML_NOTSERIALIZED);
-        aml_append(method,
-            aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(apic_id))));
-        aml_append(dev, method);
-
-        method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
-        aml_append(method,
-            aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(apic_id),
-                aml_arg(0)))
-        );
-        aml_append(dev, method);
-
-        aml_append(sb_scope, dev);
-    }
-
-    /* build this code:
-     *   Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} ...}
-     */
-    /* Arg0 = Processor ID = APIC ID */
-    method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED);
-    for (i = 0; i < apic_ids->len; i++) {
-        int apic_id = apic_ids->cpus[i].arch_id;
-
-        if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(apic_id)));
-        aml_append(if_ctx,
-            aml_notify(aml_name("CP%.02X", apic_id), aml_arg(1))
-        );
-        aml_append(method, if_ctx);
-    }
-    aml_append(sb_scope, method);
-
-    /* build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })"
-     *
-     * Note: The ability to create variable-sized packages was first
-     * introduced in ACPI 2.0. ACPI 1.0 only allowed fixed-size packages
-     * ith up to 255 elements. Windows guests up to win2k8 fail when
-     * VarPackageOp is used.
-     */
-    pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) :
-                                       aml_varpackage(pcms->apic_id_limit);
-
-    for (i = 0, apic_idx = 0; i < apic_ids->len; i++) {
-        int apic_id = apic_ids->cpus[i].arch_id;
-
-        for (; apic_idx < apic_id; apic_idx++) {
-            aml_append(pkg, aml_int(0));
-        }
-        aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0));
-        apic_idx = apic_id + 1;
-    }
-    aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
-    g_free(apic_ids);
-
-    aml_append(ctx, sb_scope);
-
-    method = aml_method("\\_GPE._E02", 0, AML_NOTSERIALIZED);
-    aml_append(method, aml_call0("\\_SB." CPU_SCAN_METHOD));
-    aml_append(ctx, method);
-}
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index faee86c..66bd727 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -1,6 +1,6 @@
 common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
 common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
-common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o cpu_hotplug_acpi_table.o
+common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
 common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
 obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
 common-obj-$(CONFIG_ACPI) += acpi_interface.o
-- 
MST

[Qemu-devel] [PULL v2 10/25] pc: acpi: simplify build_legacy_cpu_hotplug_aml() signature

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

since IO block used by CPU hotplug is fixed size and
initialized it the same file as build_legacy_cpu_hotplug_aml()
just use ACPI_GPE_PROC_LEN directly instead of passing
it around in several files.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 include/hw/acpi/cpu_hotplug.h | 2 +-
 hw/acpi/cpu_hotplug.c         | 6 +++---
 hw/i386/acpi-build.c          | 5 +----
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 241b50f..6d729d8 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -28,5 +28,5 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
                                   AcpiCpuHotplug *gpe_cpu, uint16_t base);
 
 void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
-                                  uint16_t io_base, uint16_t io_len);
+                                  uint16_t io_base);
 #endif
diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 2d4e034..36ea6c2 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -87,7 +87,7 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
 }
 
 void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
-                                  uint16_t io_base, uint16_t io_len)
+                                  uint16_t io_base)
 {
     Aml *dev;
     Aml *crs;
@@ -226,13 +226,13 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
     aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
     crs = aml_resource_template();
     aml_append(crs,
-        aml_io(AML_DECODE16, io_base, io_base, 1, io_len)
+        aml_io(AML_DECODE16, io_base, io_base, 1, ACPI_GPE_PROC_LEN)
     );
     aml_append(dev, aml_name_decl("_CRS", crs));
     aml_append(sb_scope, dev);
     /* declare CPU hotplug MMIO region and PRS field to access it */
     aml_append(sb_scope, aml_operation_region(
-        "PRST", AML_SYSTEM_IO, aml_int(io_base), io_len));
+        "PRST", AML_SYSTEM_IO, aml_int(io_base), ACPI_GPE_PROC_LEN));
     field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
     aml_append(field, aml_named_field("PRS", 256));
     aml_append(sb_scope, field);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 2f6de43..b33fec9 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -94,7 +94,6 @@ typedef struct AcpiPmInfo {
     uint32_t gpe0_blk_len;
     uint32_t io_base;
     uint16_t cpu_hp_io_base;
-    uint16_t cpu_hp_io_len;
     uint16_t mem_hp_io_base;
     uint16_t mem_hp_io_len;
     uint16_t pcihp_io_base;
@@ -142,7 +141,6 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
     }
     assert(obj);
 
-    pm->cpu_hp_io_len = ACPI_GPE_PROC_LEN;
     pm->mem_hp_io_base = ACPI_MEMORY_HOTPLUG_BASE;
     pm->mem_hp_io_len = ACPI_MEMORY_HOTPLUG_IO_LEN;
 
@@ -1935,8 +1933,7 @@ build_dsdt(GArray *table_data, GArray *linker,
         build_q35_pci0_int(dsdt);
     }
 
-    build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base,
-                                 pm->cpu_hp_io_len);
+    build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base);
     build_memory_hotplug_aml(dsdt, nr_mem, pm->mem_hp_io_base,
                              pm->mem_hp_io_len);
 
-- 
MST

[Qemu-devel] [PULL v2 11/25] pc: acpi: cpuhp-legacy: switch ProcessorID to possible_cpus idx

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

In legacy cpu-hotplug ProcessorID == APIC ID is used
in MADT and cpu-hotplug AML. It was fine as both
are 8bit and unique. Spec depricated Processor()
with corresponding ProcessorID and advises to use
Device() and UID instead of it.

However UID is just 32bit and it can't fit ARM's
arch_id(MPIDR) which is 64bit. Also in case of
sparse arch_id() distribution, managment/lookup
of maps by arch_id(APIC ID/MPIDR) becomes complex
and expensive.

In preparation to common CPU hotplug with ARM
and to simplify lookup in possible_cpus[] map
switch ProcessorID to possible_cpus index in
MADT.

Legacy cpu-hotplug considerations:
HW interface of it is APIC ID based bitmask so
it's impossible to change, also CPON package in
AML also APIC ID based as well all the methods.

To avoid massive rewrite of AML keep is so and
just break assumption that ProcessorID == APIC ID,
ammending CPU_MAT_METHOD to accept APIC ID and
possible_cpus index, it needs them both to patch
MADT entry template. Also switch to possible_cpus
index Processor(ProcessorID) AML.
That way changes to MADT/AML are minimal and kept
inside AML/MADT not affecting external interfaces.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/cpu_hotplug.c | 23 +++++++++++++----------
 hw/i386/acpi-build.c  |  2 +-
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 36ea6c2..9d71d2f 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -99,7 +99,8 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
     int i, apic_idx;
     Aml *sb_scope = aml_scope("_SB");
     uint8_t madt_tmpl[8] = {0x00, 0x08, 0x00, 0x00, 0x00, 0, 0, 0};
-    Aml *cpu_id = aml_arg(0);
+    Aml *cpu_id = aml_arg(1);
+    Aml *apic_id = aml_arg(0);
     Aml *cpu_on = aml_local(0);
     Aml *madt = aml_local(1);
     Aml *cpus_map = aml_name(CPU_ON_BITMAP);
@@ -111,30 +112,31 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
 
     /*
      * _MAT method - creates an madt apic buffer
-     * cpu_id = Arg0 = Processor ID = Local APIC ID
+     * apic_id = Arg0 = Local APIC ID
+     * cpu_id  = Arg1 = Processor ID
      * cpu_on = Local0 = CPON flag for this cpu
      * madt = Local1 = Buffer (in madt apic form) to return
      */
-    method = aml_method(CPU_MAT_METHOD, 1, AML_NOTSERIALIZED);
+    method = aml_method(CPU_MAT_METHOD, 2, AML_NOTSERIALIZED);
     aml_append(method,
-        aml_store(aml_derefof(aml_index(cpus_map, cpu_id)), cpu_on));
+        aml_store(aml_derefof(aml_index(cpus_map, apic_id)), cpu_on));
     aml_append(method,
         aml_store(aml_buffer(sizeof(madt_tmpl), madt_tmpl), madt));
     /* Update the processor id, lapic id, and enable/disable status */
     aml_append(method, aml_store(cpu_id, aml_index(madt, aml_int(2))));
-    aml_append(method, aml_store(cpu_id, aml_index(madt, aml_int(3))));
+    aml_append(method, aml_store(apic_id, aml_index(madt, aml_int(3))));
     aml_append(method, aml_store(cpu_on, aml_index(madt, aml_int(4))));
     aml_append(method, aml_return(madt));
     aml_append(sb_scope, method);
 
     /*
      * _STA method - return ON status of cpu
-     * cpu_id = Arg0 = Processor ID = Local APIC ID
+     * apic_id = Arg0 = Local APIC ID
      * cpu_on = Local0 = CPON flag for this cpu
      */
     method = aml_method(CPU_STATUS_METHOD, 1, AML_NOTSERIALIZED);
     aml_append(method,
-        aml_store(aml_derefof(aml_index(cpus_map, cpu_id)), cpu_on));
+        aml_store(aml_derefof(aml_index(cpus_map, apic_id)), cpu_on));
     if_ctx = aml_if(cpu_on);
     {
         aml_append(if_ctx, aml_return(aml_int(0xF)));
@@ -243,11 +245,12 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
 
         assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
 
-        dev = aml_processor(apic_id, 0, 0, "CP%.02X", apic_id);
+        dev = aml_processor(i, 0, 0, "CP%.02X", apic_id);
 
         method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
         aml_append(method,
-            aml_return(aml_call1(CPU_MAT_METHOD, aml_int(apic_id))));
+            aml_return(aml_call2(CPU_MAT_METHOD, aml_int(apic_id), aml_int(i))
+        ));
         aml_append(dev, method);
 
         method = aml_method("_STA", 0, AML_NOTSERIALIZED);
@@ -268,7 +271,7 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
     /* build this code:
      *   Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} ...}
      */
-    /* Arg0 = Processor ID = APIC ID */
+    /* Arg0 = APIC ID */
     method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED);
     for (i = 0; i < apic_ids->len; i++) {
         int apic_id = apic_ids->cpus[i].arch_id;
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b33fec9..768918f 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -354,7 +354,7 @@ build_madt(GArray *table_data, GArray *linker, PCMachineState *pcms)
 
         apic->type = ACPI_APIC_PROCESSOR;
         apic->length = sizeof(*apic);
-        apic->processor_id = apic_id;
+        apic->processor_id = i;
         apic->local_apic_id = apic_id;
         if (apic_ids->cpus[i].cpu != NULL) {
             apic->flags = cpu_to_le32(1);
-- 
MST

[Qemu-devel] [PULL v2 12/25] tests: acpi: update tables with consolidated legacy cpu-hotplug AML

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/acpi-test-data/pc/DSDT         | Bin 5587 -> 5503 bytes
 tests/acpi-test-data/pc/DSDT.bridge  | Bin 7446 -> 7362 bytes
 tests/acpi-test-data/q35/DSDT        | Bin 8357 -> 8265 bytes
 tests/acpi-test-data/q35/DSDT.bridge | Bin 8374 -> 8282 bytes
 4 files changed, 0 insertions(+), 0 deletions(-)

diff --git a/tests/acpi-test-data/pc/DSDT b/tests/acpi-test-data/pc/DSDT
index 9d1274d3c2e2b7a316d5133d013b0550024ee413..8b4f1a09b87f8361fb572022f69d304ddeeace99 100644
GIT binary patch
delta 174
zcmcbt{a=gACD<jTUX+1>(Rw3SB|ERHM0~JQyojlDfUhId<O%GOjG2>Hu`iq4$(t!*
z;v8ULzyJm!yz#z{Aq-1E$`~ed@|lZ^h{fo|y9c<&yBZiV0GU8j0)m2_HwSP$U=s0U
z14(hjdw9C=Iywh<8W<RuP4?!tk(2}~H)e>@X925s_B8PI2ypQYcJ|xM%2Uh8?o}BB
LlGyCPH<=v(JBlpX

delta 221
zcmeybby=IsCD<k8vM2)sqxnXzN_Jjfp7>y=co9?Q0AEMO$rIQm88arYVqdmdp7Q|{
zlPm9JMjjDXws;={gUP}?HUa|it_DU7F?t|90YSmell^$4LPS9FCJbQ86hfInD02v9
z0ii4*lp}<4f>6#7$^}BXLMS(e%^5tkjO>oBF(8{J2XJJH8#@OW7%+f=2v5APV+g|%
Qkeer8<S^g-hIcYM0Eja(?f?J)

diff --git a/tests/acpi-test-data/pc/DSDT.bridge b/tests/acpi-test-data/pc/DSDT.bridge
index cf48c62aa71a7dd7d816fd4ef8ad626e39d4965c..0d09b5cc61114b68fee0f14729732786854b19fe 100644
GIT binary patch
delta 174
zcmbPcb;y#-CD<k8kPHI@W6MUaN_JjXiTGfrco9?Q0AEL@$rIQm88auZVqZ47lQ&bs
z#5usgfB_6dc;kH?Ll~BTlrc=^<TDo+5sT4_cMoulcQr6#05XB51Ox>;Zw}yiz$D_y
z29n~4_waP#b#xB!G%zqQo9xYPBPj_~Zp;v)&jMEO>}lZZ5#Zt*?CiIhm8X`G-S=$_
LNMf@C-!(n}U^p$&

delta 221
zcmX?PIn9d8CD<iIOqPLxv3?_0B|EP#PkgXbyojlDfUhIt<O%GOj2V+xu`k;!&-s9f
z$(469Baa9xTfC2f!DL|`8v%iMR|6x47(I}lfS_RK$$mUiAtE4o69zD43Zcv(lsSa5
zfKZkY$`L|2K`3Vk<pQBxA(R`#<_w-%Ms}B<F(8{J2XJJH8#@OW7%+f=2v5APV+g|%
Qkeer8<S^g-hW8pD0P)W<<^TWy

diff --git a/tests/acpi-test-data/q35/DSDT b/tests/acpi-test-data/q35/DSDT
index 1c089c34b06c9f2ea9fe67abb45498021319303c..67445428d935bd6ea5957526089ba7e719a1783a 100644
GIT binary patch
delta 167
zcmZ4Lc+!E(CD<jzQ-Ohjv1cP!qolg4M0~JQyojlDfUhG{L34+5fWKb`LqdTdHx~y3
z2Ll)+7GzG|B)M#|kbI_uiF1H~0Yq4YH{RDVgkcFtE5qbT^5)_qVljI0?g6gxt_DU7
zKqk=SfS_RK%@I-$m_)!b9Pu8WF1(J;0iFg124)PCb7ZA9-;y<CWOwF^0V$t+QvMDA
Dr{gPK

delta 259
zcmX@<u+)*uCD<iosR9E7qrpb5MoD#Fp7>y=co9?Q0AEMOg60nA0Dr#>hJ*q^ZY~Z6
z4hAqtEXbI=Npjg{b?FC8CQe-O?g6eG@gANoypGNRo(2X6W(*>1@jeCy5Xz82L?GVP
zz=$D64`g&eP_Xl4X*sD7uo@GH8dC@b)o2dkSwJXD2;~T&oFJ4lgmQsUt`N$NVY8{6
rAtSq&WDLlr$q`bS;>OMaK!<>V2v5APV+g|%kk=;Plro>3D}M(70tPv}

diff --git a/tests/acpi-test-data/q35/DSDT.bridge b/tests/acpi-test-data/q35/DSDT.bridge
index b29fcda0bb1717ff708668c6e98f3ded3f34a96c..e85f5b1af9fcd36c9522b05e0085af84d6c010cb 100644
GIT binary patch
delta 167
zcmdnyc*}vyCD<h-N`ZlaQD-Apqolg4M0~JQyojlDfUhG{L34+5fWKb`LqdTdHx~y3
z2Ll)+7GzG|B)M#|kbI_uiF1H~0Yq4YH{RDVgkcFtE5qbT^5)_qVljI0?g6gxt_DU7
zKqk=SfS_RK%@I-$m_)!b9Pu8WF1(J;0iFg124)PCb7ZA9-;y<CWOw0@0V$t+QvNLf
Dk25Pn

delta 259
zcmccRu+5RnCD<ion*sv^<Ase}jgsoVJn_L!@gk<q0ltom1<f7K0sej&3<(8-+*}+C
z91LKPSdcM!ljO3^>e3IGOq{sl-2+@X;ypZFcpaSsJPiyC%os%2;(ZJZAe144h(Nrn
zfe}ND9?0l`pkU|8(sEKEU^ONXHKq^>s?i+6vw%>R5XuokIYB6A2;~ByTp^Sj!)8-C
rLq>LQsThz=lOv=u#f_Z<fDQoz5uSKo#}I}kAg@inDP=x6SN<&k&T2ZM

-- 
MST

[Qemu-devel] [PULL v2 13/25] ipmi: rework the fwinfo to be fetched from the interface

[Michael S. Tsirkin]

From: Corey Minyard <cminyard@mvista.com>

Instead of scanning IPMI devices from a fwinfo list, allow
the fwinfo to be fetched from the IPMI interface class.
Then the code looking for IPMI fwinfo can scan devices on a
bus and look for ones that implement the IPMI class.

This will let the ACPI scope be defined by the calling
code so the IPMI code doesn't have to know the scope.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/ipmi/ipmi.h | 74 ++++++++++++++++++++++++++------------------------
 hw/ipmi/ipmi.c         | 34 +++++------------------
 hw/ipmi/isa_ipmi_bt.c  | 57 +++++++++++++++++++++-----------------
 hw/ipmi/isa_ipmi_kcs.c | 56 +++++++++++++++++++++-----------------
 4 files changed, 109 insertions(+), 112 deletions(-)

diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
index 74a2b5a..91b83b5 100644
--- a/include/hw/ipmi/ipmi.h
+++ b/include/hw/ipmi/ipmi.h
@@ -65,6 +65,40 @@ enum ipmi_op {
 #define IPMI_SMBIOS_BT          0x03
 #define IPMI_SMBIOS_SSIF        0x04
 
+/*
+ * Used for transferring information to interfaces that add
+ * entries to firmware tables.
+ */
+typedef struct IPMIFwInfo {
+    const char *interface_name;
+    int interface_type;
+    uint8_t ipmi_spec_major_revision;
+    uint8_t ipmi_spec_minor_revision;
+    uint8_t i2c_slave_address;
+    uint32_t uuid;
+
+    uint64_t base_address;
+    uint64_t register_length;
+    uint8_t register_spacing;
+    enum {
+        IPMI_MEMSPACE_IO,
+        IPMI_MEMSPACE_MEM32,
+        IPMI_MEMSPACE_MEM64,
+        IPMI_MEMSPACE_SMBUS
+    } memspace;
+
+    int interrupt_number;
+    enum {
+        IPMI_LEVEL_IRQ,
+        IPMI_EDGE_IRQ
+    } irq_type;
+} IPMIFwInfo;
+
+/*
+ * Called by each instantiated IPMI interface device to get it's uuid.
+ */
+uint32_t ipmi_next_uuid(void);
+
 /* IPMI Interface types (KCS, SMIC, BT) are prefixed with this */
 #define TYPE_IPMI_INTERFACE_PREFIX "ipmi-interface-"
 
@@ -127,6 +161,11 @@ typedef struct IPMIInterfaceClass {
      * Set by the owner to hold the backend data for the interface.
      */
     void *(*get_backend_data)(struct IPMIInterface *s);
+
+    /*
+     * Return the firmware info for a device.
+     */
+    void (*get_fwinfo)(struct IPMIInterface *s, IPMIFwInfo *info);
 } IPMIInterfaceClass;
 
 /*
@@ -168,41 +207,6 @@ typedef struct IPMIBmcClass {
  */
 void ipmi_bmc_find_and_link(Object *obj, Object **bmc);
 
-/*
- * Used for transferring information to interfaces that add
- * entries to firmware tables.
- */
-typedef struct IPMIFwInfo {
-    const char *interface_name;
-    int interface_type;
-    uint8_t ipmi_spec_major_revision;
-    uint8_t ipmi_spec_minor_revision;
-    uint8_t i2c_slave_address;
-    uint32_t uuid;
-
-    uint64_t base_address;
-    uint64_t register_length;
-    uint8_t register_spacing;
-    enum {
-        IPMI_MEMSPACE_IO,
-        IPMI_MEMSPACE_MEM32,
-        IPMI_MEMSPACE_MEM64,
-        IPMI_MEMSPACE_SMBUS
-    } memspace;
-
-    int interrupt_number;
-    enum {
-        IPMI_LEVEL_IRQ,
-        IPMI_EDGE_IRQ
-    } irq_type;
-
-    const char *acpi_parent;
-} IPMIFwInfo;
-
-void ipmi_add_fwinfo(IPMIFwInfo *info, Error **errp);
-IPMIFwInfo *ipmi_first_fwinfo(void);
-IPMIFwInfo *ipmi_next_fwinfo(IPMIFwInfo *current);
-
 #ifdef IPMI_DEBUG
 #define ipmi_debug(fs, ...) \
     fprintf(stderr, "IPMI (%s): " fs, __func__, ##__VA_ARGS__)
diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
index 6adec1e..f09f217 100644
--- a/hw/ipmi/ipmi.c
+++ b/hw/ipmi/ipmi.c
@@ -30,6 +30,13 @@
 #include "qom/object_interfaces.h"
 #include "qapi/visitor.h"
 
+static uint32_t ipmi_current_uuid = 1;
+
+uint32_t ipmi_next_uuid(void)
+{
+    return ipmi_current_uuid++;
+}
+
 static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
 {
     switch (op) {
@@ -122,30 +129,3 @@ static void ipmi_register_types(void)
 }
 
 type_init(ipmi_register_types)
-
-static IPMIFwInfo *ipmi_fw_info;
-static unsigned int ipmi_fw_info_len;
-
-static uint32_t current_uuid = 1;
-
-void ipmi_add_fwinfo(IPMIFwInfo *info, Error **errp)
-{
-    info->uuid = current_uuid++;
-    ipmi_fw_info = g_realloc(ipmi_fw_info,
-                             sizeof(*ipmi_fw_info) * (ipmi_fw_info_len + 1));
-    ipmi_fw_info[ipmi_fw_info_len] = *info;
-}
-
-IPMIFwInfo *ipmi_first_fwinfo(void)
-{
-    return ipmi_fw_info;
-}
-
-IPMIFwInfo *ipmi_next_fwinfo(IPMIFwInfo *current)
-{
-    current++;
-    if (current >= &ipmi_fw_info[ipmi_fw_info_len]) {
-        return NULL;
-    }
-    return current;
-}
diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
index aaea12e..f036617 100644
--- a/hw/ipmi/isa_ipmi_bt.c
+++ b/hw/ipmi/isa_ipmi_bt.c
@@ -390,16 +390,6 @@ static void ipmi_bt_init(IPMIInterface *ii, Error **errp)
     memory_region_init_io(&ib->io, NULL, &ipmi_bt_io_ops, ii, "ipmi-bt", 3);
 }
 
-static void ipmi_bt_class_init(IPMIInterfaceClass *iic)
-{
-    iic->init = ipmi_bt_init;
-    iic->set_atn = ipmi_bt_set_atn;
-    iic->handle_rsp = ipmi_bt_handle_rsp;
-    iic->handle_if_event = ipmi_bt_handle_event;
-    iic->set_irq_enable = ipmi_bt_set_irq_enable;
-    iic->reset = ipmi_bt_handle_reset;
-}
-
 
 #define TYPE_ISA_IPMI_BT "isa-ipmi-bt"
 #define ISA_IPMI_BT(obj) OBJECT_CHECK(ISAIPMIBTDevice, (obj), \
@@ -409,9 +399,38 @@ typedef struct ISAIPMIBTDevice {
     ISADevice dev;
     int32_t isairq;
     IPMIBT bt;
-    IPMIFwInfo fwinfo;
+    uint32_t uuid;
 } ISAIPMIBTDevice;
 
+static void ipmi_bt_get_fwinfo(struct IPMIInterface *ii, IPMIFwInfo *info)
+{
+    ISAIPMIBTDevice *iib = ISA_IPMI_BT(ii);
+
+    info->interface_name = "bt";
+    info->interface_type = IPMI_SMBIOS_BT;
+    info->ipmi_spec_major_revision = 2;
+    info->ipmi_spec_minor_revision = 0;
+    info->base_address = iib->bt.io_base;
+    info->register_length = iib->bt.io_length;
+    info->register_spacing = 1;
+    info->memspace = IPMI_MEMSPACE_IO;
+    info->irq_type = IPMI_LEVEL_IRQ;
+    info->interrupt_number = iib->isairq;
+    info->i2c_slave_address = iib->bt.bmc->slave_addr;
+    info->uuid = iib->uuid;
+}
+
+static void ipmi_bt_class_init(IPMIInterfaceClass *iic)
+{
+    iic->init = ipmi_bt_init;
+    iic->set_atn = ipmi_bt_set_atn;
+    iic->handle_rsp = ipmi_bt_handle_rsp;
+    iic->handle_if_event = ipmi_bt_handle_event;
+    iic->set_irq_enable = ipmi_bt_set_irq_enable;
+    iic->reset = ipmi_bt_handle_reset;
+    iic->get_fwinfo = ipmi_bt_get_fwinfo;
+}
+
 static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
 {
     ISADevice *isadev = ISA_DEVICE(dev);
@@ -424,6 +443,8 @@ static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    iib->uuid = ipmi_next_uuid();
+
     iib->bt.bmc->intf = ii;
 
     iic->init(ii, errp);
@@ -438,20 +459,6 @@ static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
     qdev_set_legacy_instance_id(dev, iib->bt.io_base, iib->bt.io_length);
 
     isa_register_ioport(isadev, &iib->bt.io, iib->bt.io_base);
-
-    iib->fwinfo.interface_name = "bt";
-    iib->fwinfo.interface_type = IPMI_SMBIOS_BT;
-    iib->fwinfo.ipmi_spec_major_revision = 2;
-    iib->fwinfo.ipmi_spec_minor_revision = 0;
-    iib->fwinfo.base_address = iib->bt.io_base;
-    iib->fwinfo.register_length = iib->bt.io_length;
-    iib->fwinfo.register_spacing = 1;
-    iib->fwinfo.memspace = IPMI_MEMSPACE_IO;
-    iib->fwinfo.irq_type = IPMI_LEVEL_IRQ;
-    iib->fwinfo.interrupt_number = iib->isairq;
-    iib->fwinfo.acpi_parent = "\\_SB.PCI0.ISA";
-    iib->fwinfo.i2c_slave_address = iib->bt.bmc->slave_addr;
-    ipmi_add_fwinfo(&iib->fwinfo, errp);
 }
 
 static const VMStateDescription vmstate_ISAIPMIBTDevice = {
diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
index 2742ce0..9a38f8a 100644
--- a/hw/ipmi/isa_ipmi_kcs.c
+++ b/hw/ipmi/isa_ipmi_kcs.c
@@ -354,16 +354,6 @@ static void ipmi_kcs_init(IPMIInterface *ii, Error **errp)
     memory_region_init_io(&ik->io, NULL, &ipmi_kcs_io_ops, ii, "ipmi-kcs", 2);
 }
 
-static void ipmi_kcs_class_init(IPMIInterfaceClass *iic)
-{
-    iic->init = ipmi_kcs_init;
-    iic->set_atn = ipmi_kcs_set_atn;
-    iic->handle_rsp = ipmi_kcs_handle_rsp;
-    iic->handle_if_event = ipmi_kcs_handle_event;
-    iic->set_irq_enable = ipmi_kcs_set_irq_enable;
-}
-
-
 #define TYPE_ISA_IPMI_KCS "isa-ipmi-kcs"
 #define ISA_IPMI_KCS(obj) OBJECT_CHECK(ISAIPMIKCSDevice, (obj), \
                                        TYPE_ISA_IPMI_KCS)
@@ -372,9 +362,37 @@ typedef struct ISAIPMIKCSDevice {
     ISADevice dev;
     int32_t isairq;
     IPMIKCS kcs;
-    IPMIFwInfo fwinfo;
+    uint32_t uuid;
 } ISAIPMIKCSDevice;
 
+static void ipmi_kcs_get_fwinfo(IPMIInterface *ii, IPMIFwInfo *info)
+{
+    ISAIPMIKCSDevice *iik = ISA_IPMI_KCS(ii);
+
+    info->interface_name = "kcs";
+    info->interface_type = IPMI_SMBIOS_KCS;
+    info->ipmi_spec_major_revision = 2;
+    info->ipmi_spec_minor_revision = 0;
+    info->base_address = iik->kcs.io_base;
+    info->i2c_slave_address = iik->kcs.bmc->slave_addr;
+    info->register_length = iik->kcs.io_length;
+    info->register_spacing = 1;
+    info->memspace = IPMI_MEMSPACE_IO;
+    info->irq_type = IPMI_LEVEL_IRQ;
+    info->interrupt_number = iik->isairq;
+    info->uuid = iik->uuid;
+}
+
+static void ipmi_kcs_class_init(IPMIInterfaceClass *iic)
+{
+    iic->init = ipmi_kcs_init;
+    iic->set_atn = ipmi_kcs_set_atn;
+    iic->handle_rsp = ipmi_kcs_handle_rsp;
+    iic->handle_if_event = ipmi_kcs_handle_event;
+    iic->set_irq_enable = ipmi_kcs_set_irq_enable;
+    iic->get_fwinfo = ipmi_kcs_get_fwinfo;
+}
+
 static void ipmi_isa_realize(DeviceState *dev, Error **errp)
 {
     ISADevice *isadev = ISA_DEVICE(dev);
@@ -387,6 +405,8 @@ static void ipmi_isa_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    iik->uuid = ipmi_next_uuid();
+
     iik->kcs.bmc->intf = ii;
 
     iic->init(ii, errp);
@@ -401,20 +421,6 @@ static void ipmi_isa_realize(DeviceState *dev, Error **errp)
     qdev_set_legacy_instance_id(dev, iik->kcs.io_base, iik->kcs.io_length);
 
     isa_register_ioport(isadev, &iik->kcs.io, iik->kcs.io_base);
-
-    iik->fwinfo.interface_name = "kcs";
-    iik->fwinfo.interface_type = IPMI_SMBIOS_KCS;
-    iik->fwinfo.ipmi_spec_major_revision = 2;
-    iik->fwinfo.ipmi_spec_minor_revision = 0;
-    iik->fwinfo.base_address = iik->kcs.io_base;
-    iik->fwinfo.i2c_slave_address = iik->kcs.bmc->slave_addr;
-    iik->fwinfo.register_length = iik->kcs.io_length;
-    iik->fwinfo.register_spacing = 1;
-    iik->fwinfo.memspace = IPMI_MEMSPACE_IO;
-    iik->fwinfo.irq_type = IPMI_LEVEL_IRQ;
-    iik->fwinfo.interrupt_number = iik->isairq;
-    iik->fwinfo.acpi_parent = "\\_SB.PCI0.ISA";
-    ipmi_add_fwinfo(&iik->fwinfo, errp);
 }
 
 const VMStateDescription vmstate_ISAIPMIKCSDevice = {
-- 
MST

[Qemu-devel] [PULL v2 14/25] pc: Postpone SMBIOS table installation to post machine init

[Michael S. Tsirkin]

From: Corey Minyard <cminyard@mvista.com>

This is the same place that the ACPI SSDT table gets added, so that
devices can add themselves to the SMBIOS table.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/pc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e29ccc8..92125a8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -765,8 +765,6 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
                      acpi_tables, acpi_tables_len);
     fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
 
-    pc_build_smbios(fw_cfg);
-
     fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
                      &e820_reserve, sizeof(e820_reserve));
     fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
@@ -1182,6 +1180,9 @@ void pc_machine_done(Notifier *notifier, void *data)
     }
 
     acpi_setup();
+    if (pcms->fw_cfg) {
+        pc_build_smbios(pcms->fw_cfg);
+    }
 }
 
 void pc_guest_info_init(PCMachineState *pcms)
-- 
MST

[Qemu-devel] [PULL v2 15/25] acpi: extend ACPI interface to provide send_event hook

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

send_event() hook will allow to send ACPI event in
a target specific way (GPE or GPIO based impl.)
it will also simplify proxy wrappers in piix4pm/ich9
that access ACPI regs and SCI which are part of
piix4pm/lcp_ich9 devices and call acpi_foo() API directly.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 include/hw/acpi/acpi.h               | 10 ++--------
 include/hw/acpi/acpi_dev_interface.h | 11 +++++++++++
 hw/acpi/acpi_interface.c             |  9 +++++++++
 hw/acpi/core.c                       |  2 +-
 hw/acpi/piix4.c                      |  8 ++++++++
 hw/isa/lpc_ich9.c                    |  8 ++++++++
 6 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index dc6ee00..c717f15 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -23,6 +23,7 @@
 #include "qemu/option.h"
 #include "exec/memory.h"
 #include "hw/irq.h"
+#include "hw/acpi/acpi_dev_interface.h"
 
 /*
  * current device naming scheme supports up to 256 memory devices
@@ -89,13 +90,6 @@
 /* PM2_CNT */
 #define ACPI_BITMASK_ARB_DISABLE                0x0001
 
-/* These values are part of guest ABI, and can not be changed */
-typedef enum {
-    ACPI_PCI_HOTPLUG_STATUS = 2,
-    ACPI_CPU_HOTPLUG_STATUS = 4,
-    ACPI_MEMORY_HOTPLUG_STATUS = 8,
-} AcpiGPEStatusBits;
-
 /* structs */
 typedef struct ACPIPMTimer ACPIPMTimer;
 typedef struct ACPIPM1EVT ACPIPM1EVT;
@@ -172,7 +166,7 @@ void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val);
 uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr);
 
 void acpi_send_gpe_event(ACPIREGS *ar, qemu_irq irq,
-                         AcpiGPEStatusBits status);
+                         AcpiEventStatusBits status);
 
 void acpi_update_sci(ACPIREGS *acpi_regs, qemu_irq irq);
 
diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h
index f245f8d..a0c4a33 100644
--- a/include/hw/acpi/acpi_dev_interface.h
+++ b/include/hw/acpi/acpi_dev_interface.h
@@ -4,6 +4,13 @@
 #include "qom/object.h"
 #include "qapi-types.h"
 
+/* These values are part of guest ABI, and can not be changed */
+typedef enum {
+    ACPI_PCI_HOTPLUG_STATUS = 2,
+    ACPI_CPU_HOTPLUG_STATUS = 4,
+    ACPI_MEMORY_HOTPLUG_STATUS = 8,
+} AcpiEventStatusBits;
+
 #define TYPE_ACPI_DEVICE_IF "acpi-device-interface"
 
 #define ACPI_DEVICE_IF_CLASS(klass) \
@@ -22,11 +29,14 @@ typedef struct AcpiDeviceIf {
     Object Parent;
 } AcpiDeviceIf;
 
+void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event);
+
 /**
  * AcpiDeviceIfClass:
  *
  * ospm_status: returns status of ACPI device objects, reported
  *              via _OST method if device supports it.
+ * send_event: inject a specified event into guest
  *
  * Interface is designed for providing unified interface
  * to generic ACPI functionality that could be used without
@@ -39,5 +49,6 @@ typedef struct AcpiDeviceIfClass {
 
     /* <public> */
     void (*ospm_status)(AcpiDeviceIf *adev, ACPIOSTInfoList ***list);
+    void (*send_event)(AcpiDeviceIf *adev, AcpiEventStatusBits ev);
 } AcpiDeviceIfClass;
 #endif
diff --git a/hw/acpi/acpi_interface.c b/hw/acpi/acpi_interface.c
index d821313..6583917 100644
--- a/hw/acpi/acpi_interface.c
+++ b/hw/acpi/acpi_interface.c
@@ -2,6 +2,15 @@
 #include "hw/acpi/acpi_dev_interface.h"
 #include "qemu/module.h"
 
+void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event)
+{
+    AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(dev);
+    if (adevc->send_event) {
+        AcpiDeviceIf *adev = ACPI_DEVICE_IF(dev);
+        adevc->send_event(adev, event);
+    }
+}
+
 static void register_types(void)
 {
     static const TypeInfo acpi_dev_if_info = {
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 1ffd155..d24b9a9 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -698,7 +698,7 @@ uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr)
 }
 
 void acpi_send_gpe_event(ACPIREGS *ar, qemu_irq irq,
-                         AcpiGPEStatusBits status)
+                         AcpiEventStatusBits status)
 {
     ar->gpe.sts[0] |= status;
     acpi_update_sci(ar, irq);
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 522c9a8..5b4fcb5 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -586,6 +586,13 @@ static void piix4_ospm_status(AcpiDeviceIf *adev, ACPIOSTInfoList ***list)
     acpi_memory_ospm_status(&s->acpi_memory_hotplug, list);
 }
 
+static void piix4_send_gpe(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
+{
+    PIIX4PMState *s = PIIX4_PM(adev);
+
+    acpi_send_gpe_event(&s->ar, s->irq, ev);
+}
+
 static Property piix4_pm_properties[] = {
     DEFINE_PROP_UINT32("smb_io_base", PIIX4PMState, smb_io_base, 0),
     DEFINE_PROP_UINT8(ACPI_PM_PROP_S3_DISABLED, PIIX4PMState, disable_s3, 0),
@@ -624,6 +631,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
     hc->unplug_request = piix4_device_unplug_request_cb;
     hc->unplug = piix4_device_unplug_cb;
     adevc->ospm_status = piix4_ospm_status;
+    adevc->send_event = piix4_send_gpe;
 }
 
 static const TypeInfo piix4_pm_info = {
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 4f8ca45..72d0781 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -703,6 +703,13 @@ static Property ich9_lpc_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static void ich9_send_gpe(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
+{
+    ICH9LPCState *s = ICH9_LPC_DEVICE(adev);
+
+    acpi_send_gpe_event(&s->pm.acpi_regs, s->pm.irq, ev);
+}
+
 static void ich9_lpc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -730,6 +737,7 @@ static void ich9_lpc_class_init(ObjectClass *klass, void *data)
     hc->unplug_request = ich9_device_unplug_request_cb;
     hc->unplug = ich9_device_unplug_cb;
     adevc->ospm_status = ich9_pm_ospm_status;
+    adevc->send_event = ich9_send_gpe;
 }
 
 static const TypeInfo ich9_lpc_info = {
-- 
MST

[Qemu-devel] [PULL v2 16/25] pc: use AcpiDeviceIfClass.send_event to issue GPE events

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

it reduces number of args passed in handlers by 1 and
a number of used proxy wrappers saving ~20LOC.
Also it allows to make cpu/mem hotplug code more
universal as it would allow ARM to reuse it without
rewrite by providing its own send_event callback
to trigger events usiong GPIO instead of GPE
as fixed hadrware ACPI model doen't have GPE at all.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/cpu_hotplug.h    |  3 ++-
 include/hw/acpi/ich9.h           |  9 +++++----
 include/hw/acpi/memory_hotplug.h |  4 ++--
 include/hw/acpi/pcihp.h          |  5 +++--
 hw/acpi/cpu_hotplug.c            |  5 ++---
 hw/acpi/ich9.c                   | 33 ++++++++++++++++++++-------------
 hw/acpi/memory_hotplug.c         | 12 ++++--------
 hw/acpi/pcihp.c                  | 10 ++++------
 hw/acpi/piix4.c                  | 11 +++++------
 hw/isa/lpc_ich9.c                | 30 +++---------------------------
 10 files changed, 50 insertions(+), 72 deletions(-)

diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 6d729d8..6fef67e 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -15,13 +15,14 @@
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/pc-hotplug.h"
 #include "hw/acpi/aml-build.h"
+#include "hw/hotplug.h"
 
 typedef struct AcpiCpuHotplug {
     MemoryRegion io;
     uint8_t sts[ACPI_GPE_PROC_LEN];
 } AcpiCpuHotplug;
 
-void legacy_acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
+void legacy_acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
                              AcpiCpuHotplug *g, DeviceState *dev, Error **errp);
 
 void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
index 63fa198..bbd657c 100644
--- a/include/hw/acpi/ich9.h
+++ b/include/hw/acpi/ich9.h
@@ -69,10 +69,11 @@ extern const VMStateDescription vmstate_ich9_pm;
 
 void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm, Error **errp);
 
-void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, Error **errp);
-void ich9_pm_device_unplug_request_cb(ICH9LPCPMRegs *pm, DeviceState *dev,
-                                      Error **errp);
-void ich9_pm_device_unplug_cb(ICH9LPCPMRegs *pm, DeviceState *dev,
+void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+                            Error **errp);
+void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                      DeviceState *dev, Error **errp);
+void ich9_pm_device_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
                               Error **errp);
 
 void ich9_pm_ospm_status(AcpiDeviceIf *adev, ACPIOSTInfoList ***list);
diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
index 3a646b1..d2c7452 100644
--- a/include/hw/acpi/memory_hotplug.h
+++ b/include/hw/acpi/memory_hotplug.h
@@ -32,9 +32,9 @@ typedef struct MemHotplugState {
 void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
                               MemHotplugState *state);
 
-void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
+void acpi_memory_plug_cb(HotplugHandler *hotplug_dev, MemHotplugState *mem_st,
                          DeviceState *dev, Error **errp);
-void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
+void acpi_memory_unplug_request_cb(HotplugHandler *hotplug_dev,
                                    MemHotplugState *mem_st,
                                    DeviceState *dev, Error **errp);
 void acpi_memory_unplug_cb(MemHotplugState *mem_st,
diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
index 79a4392..04528b7 100644
--- a/include/hw/acpi/pcihp.h
+++ b/include/hw/acpi/pcihp.h
@@ -29,6 +29,7 @@
 
 #include "hw/acpi/acpi.h"
 #include "migration/vmstate.h"
+#include "hw/hotplug.h"
 
 #define ACPI_PCIHP_IO_BASE_PROP "acpi-pcihp-io-base"
 #define ACPI_PCIHP_IO_LEN_PROP "acpi-pcihp-io-len"
@@ -56,9 +57,9 @@ typedef struct AcpiPciHpState {
 void acpi_pcihp_init(Object *owner, AcpiPciHpState *, PCIBus *root,
                      MemoryRegion *address_space_io, bool bridges_enabled);
 
-void acpi_pcihp_device_plug_cb(ACPIREGS *ar, qemu_irq irq, AcpiPciHpState *s,
+void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
                                DeviceState *dev, Error **errp);
-void acpi_pcihp_device_unplug_cb(ACPIREGS *ar, qemu_irq irq, AcpiPciHpState *s,
+void acpi_pcihp_device_unplug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
                                  DeviceState *dev, Error **errp);
 
 /* Called on reset */
diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 9d71d2f..fe75bd9 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -62,15 +62,14 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
     g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
 }
 
-void legacy_acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
+void legacy_acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
                              AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
 {
     acpi_set_cpu_present_bit(g, CPU(dev), errp);
     if (*errp != NULL) {
         return;
     }
-
-    acpi_send_gpe_event(ar, irq, ACPI_CPU_HOTPLUG_STATUS);
+    acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS);
 }
 
 void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index af340d0..853c9c4 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -430,40 +430,47 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm, Error **errp)
                              NULL);
 }
 
-void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, Error **errp)
+void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+                            Error **errp)
 {
-    if (pm->acpi_memory_hotplug.is_enabled &&
+    ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev);
+
+    if (lpc->pm.acpi_memory_hotplug.is_enabled &&
         object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
-        acpi_memory_plug_cb(&pm->acpi_regs, pm->irq, &pm->acpi_memory_hotplug,
+        acpi_memory_plug_cb(hotplug_dev, &lpc->pm.acpi_memory_hotplug,
                             dev, errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
-        legacy_acpi_cpu_plug_cb(&pm->acpi_regs, pm->irq,
-                                &pm->gpe_cpu, dev, errp);
+        legacy_acpi_cpu_plug_cb(hotplug_dev, &lpc->pm.gpe_cpu, dev, errp);
     } else {
         error_setg(errp, "acpi: device plug request for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
     }
 }
 
-void ich9_pm_device_unplug_request_cb(ICH9LPCPMRegs *pm, DeviceState *dev,
-                                      Error **errp)
+void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                      DeviceState *dev, Error **errp)
 {
-    if (pm->acpi_memory_hotplug.is_enabled &&
+    ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev);
+
+    if (lpc->pm.acpi_memory_hotplug.is_enabled &&
         object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
-        acpi_memory_unplug_request_cb(&pm->acpi_regs, pm->irq,
-                                      &pm->acpi_memory_hotplug, dev, errp);
+        acpi_memory_unplug_request_cb(hotplug_dev,
+                                      &lpc->pm.acpi_memory_hotplug, dev,
+                                      errp);
     } else {
         error_setg(errp, "acpi: device unplug request for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
     }
 }
 
-void ich9_pm_device_unplug_cb(ICH9LPCPMRegs *pm, DeviceState *dev,
+void ich9_pm_device_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
                               Error **errp)
 {
-    if (pm->acpi_memory_hotplug.is_enabled &&
+    ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev);
+
+    if (lpc->pm.acpi_memory_hotplug.is_enabled &&
         object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
-        acpi_memory_unplug_cb(&pm->acpi_memory_hotplug, dev, errp);
+        acpi_memory_unplug_cb(&lpc->pm.acpi_memory_hotplug, dev, errp);
     } else {
         error_setg(errp, "acpi: device unplug for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index f65a3a2..ec4e64b 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -228,7 +228,7 @@ acpi_memory_slot_status(MemHotplugState *mem_st,
     return &mem_st->devs[slot];
 }
 
-void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
+void acpi_memory_plug_cb(HotplugHandler *hotplug_dev, MemHotplugState *mem_st,
                          DeviceState *dev, Error **errp)
 {
     MemStatus *mdev;
@@ -247,13 +247,11 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
     mdev->is_enabled = true;
     if (dev->hotplugged) {
         mdev->is_inserting = true;
-
-        /* do ACPI magic */
-        acpi_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS);
+        acpi_send_event(DEVICE(hotplug_dev), ACPI_MEMORY_HOTPLUG_STATUS);
     }
 }
 
-void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
+void acpi_memory_unplug_request_cb(HotplugHandler *hotplug_dev,
                                    MemHotplugState *mem_st,
                                    DeviceState *dev, Error **errp)
 {
@@ -265,9 +263,7 @@ void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
     }
 
     mdev->is_removing = true;
-
-    /* Do ACPI magic */
-    acpi_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS);
+    acpi_send_event(DEVICE(hotplug_dev), ACPI_MEMORY_HOTPLUG_STATUS);
 }
 
 void acpi_memory_unplug_cb(MemHotplugState *mem_st,
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 71f4c4e..d957d1e 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -182,7 +182,7 @@ void acpi_pcihp_reset(AcpiPciHpState *s)
     acpi_pcihp_update(s);
 }
 
-void acpi_pcihp_device_plug_cb(ACPIREGS *ar, qemu_irq irq, AcpiPciHpState *s,
+void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
                                DeviceState *dev, Error **errp)
 {
     PCIDevice *pdev = PCI_DEVICE(dev);
@@ -202,11 +202,10 @@ void acpi_pcihp_device_plug_cb(ACPIREGS *ar, qemu_irq irq, AcpiPciHpState *s,
     }
 
     s->acpi_pcihp_pci_status[bsel].up |= (1U << slot);
-
-    acpi_send_gpe_event(ar, irq, ACPI_PCI_HOTPLUG_STATUS);
+    acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS);
 }
 
-void acpi_pcihp_device_unplug_cb(ACPIREGS *ar, qemu_irq irq, AcpiPciHpState *s,
+void acpi_pcihp_device_unplug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
                                  DeviceState *dev, Error **errp)
 {
     PCIDevice *pdev = PCI_DEVICE(dev);
@@ -219,8 +218,7 @@ void acpi_pcihp_device_unplug_cb(ACPIREGS *ar, qemu_irq irq, AcpiPciHpState *s,
     }
 
     s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
-
-    acpi_send_gpe_event(ar, irq, ACPI_PCI_HOTPLUG_STATUS);
+    acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS);
 }
 
 static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 5b4fcb5..c48cb1b 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -348,12 +348,11 @@ static void piix4_device_plug_cb(HotplugHandler *hotplug_dev,
 
     if (s->acpi_memory_hotplug.is_enabled &&
         object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
-        acpi_memory_plug_cb(&s->ar, s->irq, &s->acpi_memory_hotplug, dev, errp);
+        acpi_memory_plug_cb(hotplug_dev, &s->acpi_memory_hotplug, dev, errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
-        acpi_pcihp_device_plug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug, dev,
-                                  errp);
+        acpi_pcihp_device_plug_cb(hotplug_dev, &s->acpi_pci_hotplug, dev, errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
-        legacy_acpi_cpu_plug_cb(&s->ar, s->irq, &s->gpe_cpu, dev, errp);
+        legacy_acpi_cpu_plug_cb(hotplug_dev, &s->gpe_cpu, dev, errp);
     } else {
         error_setg(errp, "acpi: device plug request for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
@@ -367,10 +366,10 @@ static void piix4_device_unplug_request_cb(HotplugHandler *hotplug_dev,
 
     if (s->acpi_memory_hotplug.is_enabled &&
         object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
-        acpi_memory_unplug_request_cb(&s->ar, s->irq, &s->acpi_memory_hotplug,
+        acpi_memory_unplug_request_cb(hotplug_dev, &s->acpi_memory_hotplug,
                                       dev, errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
-        acpi_pcihp_device_unplug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug, dev,
+        acpi_pcihp_device_unplug_cb(hotplug_dev, &s->acpi_pci_hotplug, dev,
                                     errp);
     } else {
         error_setg(errp, "acpi: device unplug request for not supported device"
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 72d0781..2a2d52e 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -637,30 +637,6 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
                                         1);
 }
 
-static void ich9_device_plug_cb(HotplugHandler *hotplug_dev,
-                                DeviceState *dev, Error **errp)
-{
-    ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev);
-
-    ich9_pm_device_plug_cb(&lpc->pm, dev, errp);
-}
-
-static void ich9_device_unplug_request_cb(HotplugHandler *hotplug_dev,
-                                          DeviceState *dev, Error **errp)
-{
-    ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev);
-
-    ich9_pm_device_unplug_request_cb(&lpc->pm, dev, errp);
-}
-
-static void ich9_device_unplug_cb(HotplugHandler *hotplug_dev,
-                                  DeviceState *dev, Error **errp)
-{
-    ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev);
-
-    ich9_pm_device_unplug_cb(&lpc->pm, dev, errp);
-}
-
 static bool ich9_rst_cnt_needed(void *opaque)
 {
     ICH9LPCState *lpc = opaque;
@@ -733,9 +709,9 @@ static void ich9_lpc_class_init(ObjectClass *klass, void *data)
      * pc_q35_init()
      */
     dc->cannot_instantiate_with_device_add_yet = true;
-    hc->plug = ich9_device_plug_cb;
-    hc->unplug_request = ich9_device_unplug_request_cb;
-    hc->unplug = ich9_device_unplug_cb;
+    hc->plug = ich9_pm_device_plug_cb;
+    hc->unplug_request = ich9_pm_device_unplug_request_cb;
+    hc->unplug = ich9_pm_device_unplug_cb;
     adevc->ospm_status = ich9_pm_ospm_status;
     adevc->send_event = ich9_send_gpe;
 }
-- 
MST

[Qemu-devel] [PULL v2 17/25] acpi: convert linker from GArray to BIOSLinker structure

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

Patch just changes type of of linker variables to
a structure, there aren't any functional changes.

Converting linker to a structure will allow to extend
it functionality in follow up patch adding sanity blob
checks.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/aml-build.h          |  7 ++++---
 include/hw/acpi/bios-linker-loader.h | 14 ++++++++-----
 include/hw/mem/nvdimm.h              |  3 ++-
 hw/acpi/aml-build.c                  |  5 ++---
 hw/acpi/bios-linker-loader.c         | 40 +++++++++++++++++++++---------------
 hw/acpi/nvdimm.c                     |  6 +++---
 hw/arm/virt-acpi-build.c             | 24 +++++++++++-----------
 hw/i386/acpi-build.c                 | 29 +++++++++++++-------------
 8 files changed, 70 insertions(+), 58 deletions(-)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index dd432d2..3952f85 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -3,6 +3,7 @@
 
 #include <glib.h>
 #include "hw/acpi/acpi-defs.h"
+#include "hw/acpi/bios-linker-loader.h"
 
 /* Reserve RAM space for tables: add another order of magnitude. */
 #define ACPI_BUILD_TABLE_MAX_SIZE         0x200000
@@ -210,7 +211,7 @@ struct AcpiBuildTables {
     GArray *table_data;
     GArray *rsdp;
     GArray *tcpalog;
-    GArray *linker;
+    BIOSLinker *linker;
 } AcpiBuildTables;
 
 /**
@@ -365,7 +366,7 @@ Aml *aml_sizeof(Aml *arg);
 Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target);
 
 void
-build_header(GArray *linker, GArray *table_data,
+build_header(BIOSLinker *linker, GArray *table_data,
              AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
              const char *oem_id, const char *oem_table_id);
 void *acpi_data_push(GArray *table_data, unsigned size);
@@ -374,7 +375,7 @@ void acpi_add_table(GArray *table_offsets, GArray *table_data);
 void acpi_build_tables_init(AcpiBuildTables *tables);
 void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre);
 void
-build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
+build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
            const char *oem_id, const char *oem_table_id);
 
 int
diff --git a/include/hw/acpi/bios-linker-loader.h b/include/hw/acpi/bios-linker-loader.h
index 82f1af6..4145c56 100644
--- a/include/hw/acpi/bios-linker-loader.h
+++ b/include/hw/acpi/bios-linker-loader.h
@@ -3,23 +3,27 @@
 
 #include <glib.h>
 
-GArray *bios_linker_loader_init(void);
+typedef struct BIOSLinker {
+    GArray *cmd_blob;
+} BIOSLinker;
 
-void bios_linker_loader_alloc(GArray *linker,
+BIOSLinker *bios_linker_loader_init(void);
+
+void bios_linker_loader_alloc(BIOSLinker *linker,
                               const char *file,
                               uint32_t alloc_align,
                               bool alloc_fseg);
 
-void bios_linker_loader_add_checksum(GArray *linker, const char *file,
+void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
                                      GArray *table,
                                      void *start, unsigned size,
                                      uint8_t *checksum);
 
-void bios_linker_loader_add_pointer(GArray *linker,
+void bios_linker_loader_add_pointer(BIOSLinker *linker,
                                     const char *dest_file,
                                     const char *src_file,
                                     GArray *table, void *pointer,
                                     uint8_t pointer_size);
 
-void *bios_linker_loader_cleanup(GArray *linker);
+void *bios_linker_loader_cleanup(BIOSLinker *linker);
 #endif
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index 517de9c..32e1445 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -24,6 +24,7 @@
 #define QEMU_NVDIMM_H
 
 #include "hw/mem/pc-dimm.h"
+#include "hw/acpi/bios-linker-loader.h"
 
 #define NVDIMM_DEBUG 0
 #define nvdimm_debug(fmt, ...)                                \
@@ -58,5 +59,5 @@ typedef struct AcpiNVDIMMState AcpiNVDIMMState;
 void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
                             FWCfgState *fw_cfg, Object *owner);
 void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
-                       GArray *linker);
+                       BIOSLinker *linker);
 #endif
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 95f6021..a3fc93a 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -24,7 +24,6 @@
 #include "hw/acpi/aml-build.h"
 #include "qemu/bswap.h"
 #include "qemu/bitops.h"
-#include "hw/acpi/bios-linker-loader.h"
 
 static GArray *build_alloc_array(void)
 {
@@ -1490,7 +1489,7 @@ Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
 }
 
 void
-build_header(GArray *linker, GArray *table_data,
+build_header(BIOSLinker *linker, GArray *table_data,
              AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
              const char *oem_id, const char *oem_table_id)
 {
@@ -1558,7 +1557,7 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre)
 
 /* Build rsdt table */
 void
-build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
+build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
            const char *oem_id, const char *oem_table_id)
 {
     AcpiRsdtDescriptorRev1 *rsdt;
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index 5153ab1..6b15a85 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -96,33 +96,39 @@ enum {
 };
 
 /*
- * bios_linker_loader_init: allocate a new linker file blob array.
+ * bios_linker_loader_init: allocate a new linker object instance.
  *
  * After initialization, linker commands can be added, and will
- * be stored in the array.
+ * be stored in the linker.cmd_blob array.
  */
-GArray *bios_linker_loader_init(void)
+BIOSLinker *bios_linker_loader_init(void)
 {
-    return g_array_new(false, true /* clear */, 1);
+    BIOSLinker *linker = g_new(BIOSLinker, 1);
+
+    linker->cmd_blob = g_array_new(false, true /* clear */, 1);
+    return linker;
 }
 
-/* Free linker wrapper and return the linker array. */
-void *bios_linker_loader_cleanup(GArray *linker)
+/* Free linker wrapper and return the linker commands array. */
+void *bios_linker_loader_cleanup(BIOSLinker *linker)
 {
-    return g_array_free(linker, false);
+    void *cmd_blob = g_array_free(linker->cmd_blob, false);
+
+    g_free(linker);
+    return cmd_blob;
 }
 
 /*
  * bios_linker_loader_alloc: ask guest to load file into guest memory.
  *
- * @linker: linker file blob array
- * @file: file to be loaded
+ * @linker: linker object instance
+ * @file: name of the file blob to be loaded
  * @alloc_align: required minimal alignment in bytes. Must be a power of 2.
  * @alloc_fseg: request allocation in FSEG zone (useful for the RSDP ACPI table)
  *
  * Note: this command must precede any other linker command using this file.
  */
-void bios_linker_loader_alloc(GArray *linker,
+void bios_linker_loader_alloc(BIOSLinker *linker,
                               const char *file,
                               uint32_t alloc_align,
                               bool alloc_fseg)
@@ -139,7 +145,7 @@ void bios_linker_loader_alloc(GArray *linker,
                                     BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH;
 
     /* Alloc entries must come first, so prepend them */
-    g_array_prepend_vals(linker, &entry, sizeof entry);
+    g_array_prepend_vals(linker->cmd_blob, &entry, sizeof entry);
 }
 
 /*
@@ -149,7 +155,7 @@ void bios_linker_loader_alloc(GArray *linker,
  * Checksum calculation simply sums -X for each byte X in the range
  * using 8-bit math (i.e. ACPI checksum).
  *
- * @linker: linker file blob array
+ * @linker: linker object instance
  * @file: file that includes the checksum to be calculated
  *        and the data to be checksummed
  * @table: @file blob contents
@@ -167,7 +173,7 @@ void bios_linker_loader_alloc(GArray *linker,
  * - To avoid confusion, caller must always put 0x0 at @checksum.
  * - @file must be loaded into Guest memory using bios_linker_loader_alloc
  */
-void bios_linker_loader_add_checksum(GArray *linker, const char *file,
+void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
                                      GArray *table,
                                      void *start, unsigned size,
                                      uint8_t *checksum)
@@ -189,14 +195,14 @@ void bios_linker_loader_add_checksum(GArray *linker, const char *file,
     entry.cksum.start = cpu_to_le32(start_offset);
     entry.cksum.length = cpu_to_le32(size);
 
-    g_array_append_vals(linker, &entry, sizeof entry);
+    g_array_append_vals(linker->cmd_blob, &entry, sizeof entry);
 }
 
 /*
  * bios_linker_loader_add_pointer: ask guest to add address of source file
  * into destination file at the specified pointer.
  *
- * @linker: linker file blob array
+ * @linker: linker object instance
  * @dest_file: destination file that must be changed
  * @src_file: source file who's address must be taken
  * @table: @dest_file blob contents array
@@ -213,7 +219,7 @@ void bios_linker_loader_add_checksum(GArray *linker, const char *file,
  * - Both @dest_file and @src_file must be
  *   loaded into Guest memory using bios_linker_loader_alloc
  */
-void bios_linker_loader_add_pointer(GArray *linker,
+void bios_linker_loader_add_pointer(BIOSLinker *linker,
                                     const char *dest_file,
                                     const char *src_file,
                                     GArray *table, void *pointer,
@@ -236,5 +242,5 @@ void bios_linker_loader_add_pointer(GArray *linker,
     assert(pointer_size == 1 || pointer_size == 2 ||
            pointer_size == 4 || pointer_size == 8);
 
-    g_array_append_vals(linker, &entry, sizeof entry);
+    g_array_append_vals(linker->cmd_blob, &entry, sizeof entry);
 }
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index fb925dc..a2d20ea 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -353,7 +353,7 @@ static GArray *nvdimm_build_device_structure(GSList *device_list)
 }
 
 static void nvdimm_build_nfit(GSList *device_list, GArray *table_offsets,
-                              GArray *table_data, GArray *linker)
+                              GArray *table_data, BIOSLinker *linker)
 {
     GArray *structures = nvdimm_build_device_structure(device_list);
     unsigned int header;
@@ -579,7 +579,7 @@ static void nvdimm_build_nvdimm_devices(GSList *device_list, Aml *root_dev)
 }
 
 static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
-                              GArray *table_data, GArray *linker)
+                              GArray *table_data, BIOSLinker *linker)
 {
     Aml *ssdt, *sb_scope, *dev, *field;
     int mem_addr_offset, nvdimm_ssdt;
@@ -691,7 +691,7 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
 }
 
 void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
-                       GArray *linker)
+                       BIOSLinker *linker)
 {
     GSList *device_list;
 
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 83a5420..fc130ff 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -354,7 +354,7 @@ static void acpi_dsdt_add_power_button(Aml *scope)
 
 /* RSDP */
 static GArray *
-build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
+build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
 {
     AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
 
@@ -383,7 +383,7 @@ build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
 }
 
 static void
-build_spcr(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
+build_spcr(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
 {
     AcpiSerialPortConsoleRedirection *spcr;
     const MemMapEntry *uart_memmap = &guest_info->memmap[VIRT_UART];
@@ -416,7 +416,7 @@ build_spcr(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
 }
 
 static void
-build_srat(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
+build_srat(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
 {
     AcpiSystemResourceAffinityTable *srat;
     AcpiSratProcessorGiccAffinity *core;
@@ -456,13 +456,12 @@ build_srat(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
         mem_base += numa_info[i].node_mem;
     }
 
-    build_header(linker, table_data,
-                 (void *)(table_data->data + srat_start), "SRAT",
+    build_header(linker, table_data, (void *)srat, "SRAT",
                  table_data->len - srat_start, 3, NULL, NULL);
 }
 
 static void
-build_mcfg(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
+build_mcfg(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
 {
     AcpiTableMcfg *mcfg;
     const MemMapEntry *memmap = guest_info->memmap;
@@ -482,7 +481,7 @@ build_mcfg(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
 
 /* GTDT */
 static void
-build_gtdt(GArray *table_data, GArray *linker)
+build_gtdt(GArray *table_data, BIOSLinker *linker)
 {
     int gtdt_start = table_data->len;
     AcpiGenericTimerTable *gtdt;
@@ -508,7 +507,7 @@ build_gtdt(GArray *table_data, GArray *linker)
 
 /* MADT */
 static void
-build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
+build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
 {
     int madt_start = table_data->len;
     const MemMapEntry *memmap = guest_info->memmap;
@@ -567,7 +566,7 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
 
 /* FADT */
 static void
-build_fadt(GArray *table_data, GArray *linker, unsigned dsdt)
+build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt)
 {
     AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
 
@@ -592,7 +591,7 @@ build_fadt(GArray *table_data, GArray *linker, unsigned dsdt)
 
 /* DSDT */
 static void
-build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
+build_dsdt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
 {
     Aml *scope, *dsdt;
     const MemMapEntry *memmap = guest_info->memmap;
@@ -731,7 +730,7 @@ static void virt_acpi_build_update(void *build_opaque)
 
     acpi_ram_update(build_state->table_mr, tables.table_data);
     acpi_ram_update(build_state->rsdp_mr, tables.rsdp);
-    acpi_ram_update(build_state->linker_mr, tables.linker);
+    acpi_ram_update(build_state->linker_mr, tables.linker->cmd_blob);
 
 
     acpi_build_tables_cleanup(&tables, true);
@@ -789,7 +788,8 @@ void virt_acpi_setup(VirtGuestInfo *guest_info)
     assert(build_state->table_mr != NULL);
 
     build_state->linker_mr =
-        acpi_add_rom_blob(build_state, tables.linker, "etc/table-loader", 0);
+        acpi_add_rom_blob(build_state, tables.linker->cmd_blob,
+                          "etc/table-loader", 0);
 
     fw_cfg_add_file(guest_info->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
                     tables.tcpalog->data, acpi_data_len(tables.tcpalog));
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 768918f..e6c390c 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -260,7 +260,7 @@ static void acpi_align_size(GArray *blob, unsigned align)
 
 /* FACS */
 static void
-build_facs(GArray *table_data, GArray *linker)
+build_facs(GArray *table_data, BIOSLinker *linker)
 {
     AcpiFacsDescriptorRev1 *facs = acpi_data_push(table_data, sizeof *facs);
     memcpy(&facs->signature, "FACS", 4);
@@ -305,7 +305,7 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
 
 /* FADT */
 static void
-build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
+build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
            unsigned facs, unsigned dsdt,
            const char *oem_id, const char *oem_table_id)
 {
@@ -332,7 +332,7 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
 }
 
 static void
-build_madt(GArray *table_data, GArray *linker, PCMachineState *pcms)
+build_madt(GArray *table_data, BIOSLinker *linker, PCMachineState *pcms)
 {
     MachineClass *mc = MACHINE_GET_CLASS(pcms);
     CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(pcms));
@@ -1869,7 +1869,7 @@ static Aml *build_q35_osc_method(void)
 }
 
 static void
-build_dsdt(GArray *table_data, GArray *linker,
+build_dsdt(GArray *table_data, BIOSLinker *linker,
            AcpiPmInfo *pm, AcpiMiscInfo *misc,
            PcPciInfo *pci, MachineState *machine)
 {
@@ -2240,7 +2240,7 @@ build_dsdt(GArray *table_data, GArray *linker,
 }
 
 static void
-build_hpet(GArray *table_data, GArray *linker)
+build_hpet(GArray *table_data, BIOSLinker *linker)
 {
     Acpi20Hpet *hpet;
 
@@ -2255,7 +2255,7 @@ build_hpet(GArray *table_data, GArray *linker)
 }
 
 static void
-build_tpm_tcpa(GArray *table_data, GArray *linker, GArray *tcpalog)
+build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
 {
     Acpi20Tcpa *tcpa = acpi_data_push(table_data, sizeof *tcpa);
     uint64_t log_area_start_address = acpi_data_len(tcpalog);
@@ -2280,7 +2280,7 @@ build_tpm_tcpa(GArray *table_data, GArray *linker, GArray *tcpalog)
 }
 
 static void
-build_tpm2(GArray *table_data, GArray *linker)
+build_tpm2(GArray *table_data, BIOSLinker *linker)
 {
     Acpi20TPM2 *tpm2_ptr;
 
@@ -2295,7 +2295,7 @@ build_tpm2(GArray *table_data, GArray *linker)
 }
 
 static void
-build_srat(GArray *table_data, GArray *linker, MachineState *machine)
+build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 {
     AcpiSystemResourceAffinityTable *srat;
     AcpiSratProcessorAffinity *core;
@@ -2392,7 +2392,7 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
 }
 
 static void
-build_mcfg_q35(GArray *table_data, GArray *linker, AcpiMcfgInfo *info)
+build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
 {
     AcpiTableMcfg *mcfg;
     const char *sig;
@@ -2421,7 +2421,7 @@ build_mcfg_q35(GArray *table_data, GArray *linker, AcpiMcfgInfo *info)
 }
 
 static void
-build_dmar_q35(GArray *table_data, GArray *linker)
+build_dmar_q35(GArray *table_data, BIOSLinker *linker)
 {
     int dmar_start = table_data->len;
 
@@ -2445,7 +2445,7 @@ build_dmar_q35(GArray *table_data, GArray *linker)
 }
 
 static GArray *
-build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
+build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
 {
     AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
 
@@ -2657,7 +2657,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
         acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
     }
 
-    acpi_align_size(tables->linker, ACPI_BUILD_ALIGN_SIZE);
+    acpi_align_size(tables->linker->cmd_blob, ACPI_BUILD_ALIGN_SIZE);
 
     /* Cleanup memory that's no longer used. */
     g_array_free(table_offsets, true);
@@ -2697,7 +2697,7 @@ static void acpi_build_update(void *build_opaque)
         acpi_ram_update(build_state->rsdp_mr, tables.rsdp);
     }
 
-    acpi_ram_update(build_state->linker_mr, tables.linker);
+    acpi_ram_update(build_state->linker_mr, tables.linker->cmd_blob);
     acpi_build_tables_cleanup(&tables, true);
 }
 
@@ -2761,7 +2761,8 @@ void acpi_setup(void)
     assert(build_state->table_mr != NULL);
 
     build_state->linker_mr =
-        acpi_add_rom_blob(build_state, tables.linker, "etc/table-loader", 0);
+        acpi_add_rom_blob(build_state, tables.linker->cmd_blob,
+                          "etc/table-loader", 0);
 
     fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
                     tables.tcpalog->data, acpi_data_len(tables.tcpalog));
-- 
MST

[Qemu-devel] [PULL v2 18/25] acpi: simplify bios_linker API by removing redundant 'table' argument

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

'table' argument in bios_linker_add_foo() commands is
a data blob of one of files also passed to the same API.
So instead of passing blob in every API call, add and keep
file name association with related blob at bios_linker_loader_alloc()
time.

And find blob by name looking up allocated file entries
inside of bios_linker_add_foo() commands.

It will:
 - make API less confusing,
 - enforce calling bios_linker_loader_alloc() before
   calling any bios_linker_add_foo()
 - make sure that blob is the correct one, i.e.
   associated with the right file name

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/bios-linker-loader.h |  7 ++--
 include/hw/mem/nvdimm.h              |  2 +-
 hw/acpi/aml-build.c                  |  4 +-
 hw/acpi/bios-linker-loader.c         | 81 ++++++++++++++++++++++++++----------
 hw/acpi/nvdimm.c                     | 15 ++++---
 hw/arm/virt-acpi-build.c             | 11 ++---
 hw/i386/acpi-build.c                 | 20 +++++----
 7 files changed, 93 insertions(+), 47 deletions(-)

diff --git a/include/hw/acpi/bios-linker-loader.h b/include/hw/acpi/bios-linker-loader.h
index 4145c56..bee6dee 100644
--- a/include/hw/acpi/bios-linker-loader.h
+++ b/include/hw/acpi/bios-linker-loader.h
@@ -5,24 +5,25 @@
 
 typedef struct BIOSLinker {
     GArray *cmd_blob;
+    GArray *file_list;
 } BIOSLinker;
 
 BIOSLinker *bios_linker_loader_init(void);
 
 void bios_linker_loader_alloc(BIOSLinker *linker,
-                              const char *file,
+                              const char *file_name,
+                              GArray *file_blob,
                               uint32_t alloc_align,
                               bool alloc_fseg);
 
 void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
-                                     GArray *table,
                                      void *start, unsigned size,
                                      uint8_t *checksum);
 
 void bios_linker_loader_add_pointer(BIOSLinker *linker,
                                     const char *dest_file,
                                     const char *src_file,
-                                    GArray *table, void *pointer,
+                                    void *pointer,
                                     uint8_t pointer_size);
 
 void *bios_linker_loader_cleanup(BIOSLinker *linker);
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index 32e1445..60ee92b 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -59,5 +59,5 @@ typedef struct AcpiNVDIMMState AcpiNVDIMMState;
 void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
                             FWCfgState *fw_cfg, Object *owner);
 void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
-                       BIOSLinker *linker);
+                       BIOSLinker *linker, GArray *dsm_dma_arrea);
 #endif
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index a3fc93a..bff2599 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1516,7 +1516,7 @@ build_header(BIOSLinker *linker, GArray *table_data,
     h->checksum = 0;
     /* Checksum to be filled in by Guest linker */
     bios_linker_loader_add_checksum(linker, ACPI_BUILD_TABLE_FILE,
-                                    table_data, h, len, &h->checksum);
+                                    h, len, &h->checksum);
 }
 
 void *acpi_data_push(GArray *table_data, unsigned size)
@@ -1573,7 +1573,7 @@ build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
         bios_linker_loader_add_pointer(linker,
                                        ACPI_BUILD_TABLE_FILE,
                                        ACPI_BUILD_TABLE_FILE,
-                                       table_data, &rsdt->table_offset_entry[i],
+                                       &rsdt->table_offset_entry[i],
                                        sizeof(uint32_t));
     }
     build_header(linker, table_data,
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index 6b15a85..8e3f30d 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -96,6 +96,16 @@ enum {
 };
 
 /*
+ * BiosLinkerFileEntry:
+ *
+ * An internal type used for book-keeping file entries
+ */
+typedef struct BiosLinkerFileEntry {
+    char *name; /* file name */
+    GArray *blob; /* data accosiated with @name */
+} BiosLinkerFileEntry;
+
+/*
  * bios_linker_loader_init: allocate a new linker object instance.
  *
  * After initialization, linker commands can be added, and will
@@ -106,39 +116,69 @@ BIOSLinker *bios_linker_loader_init(void)
     BIOSLinker *linker = g_new(BIOSLinker, 1);
 
     linker->cmd_blob = g_array_new(false, true /* clear */, 1);
+    linker->file_list = g_array_new(false, true /* clear */,
+                                    sizeof(BiosLinkerFileEntry));
     return linker;
 }
 
 /* Free linker wrapper and return the linker commands array. */
 void *bios_linker_loader_cleanup(BIOSLinker *linker)
 {
+    int i;
+    BiosLinkerFileEntry *entry;
     void *cmd_blob = g_array_free(linker->cmd_blob, false);
 
+    for (i = 0; i < linker->file_list->len; i++) {
+        entry = &g_array_index(linker->file_list, BiosLinkerFileEntry, i);
+        g_free(entry->name);
+    }
+    g_array_free(linker->file_list, true);
     g_free(linker);
     return cmd_blob;
 }
 
+static const BiosLinkerFileEntry *
+bios_linker_find_file(const BIOSLinker *linker, const char *name)
+{
+    int i;
+    BiosLinkerFileEntry *entry;
+
+    for (i = 0; i < linker->file_list->len; i++) {
+        entry = &g_array_index(linker->file_list, BiosLinkerFileEntry, i);
+        if (!strcmp(entry->name, name)) {
+            return entry;
+        }
+    }
+    return NULL;
+}
+
 /*
  * bios_linker_loader_alloc: ask guest to load file into guest memory.
  *
  * @linker: linker object instance
- * @file: name of the file blob to be loaded
+ * @file_name: name of the file blob to be loaded
+ * @file_blob: pointer to blob corresponding to @file_name
  * @alloc_align: required minimal alignment in bytes. Must be a power of 2.
  * @alloc_fseg: request allocation in FSEG zone (useful for the RSDP ACPI table)
  *
  * Note: this command must precede any other linker command using this file.
  */
 void bios_linker_loader_alloc(BIOSLinker *linker,
-                              const char *file,
+                              const char *file_name,
+                              GArray *file_blob,
                               uint32_t alloc_align,
                               bool alloc_fseg)
 {
     BiosLinkerLoaderEntry entry;
+    BiosLinkerFileEntry file = { g_strdup(file_name), file_blob};
 
     assert(!(alloc_align & (alloc_align - 1)));
 
+    assert(!bios_linker_find_file(linker, file_name));
+    g_array_append_val(linker->file_list, file);
+
     memset(&entry, 0, sizeof entry);
-    strncpy(entry.alloc.file, file, sizeof entry.alloc.file - 1);
+    strncpy(entry.alloc.file, file_name, sizeof entry.alloc.file - 1);
     entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ALLOCATE);
     entry.alloc.align = cpu_to_le32(alloc_align);
     entry.alloc.zone = alloc_fseg ? BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG :
@@ -158,38 +198,37 @@ void bios_linker_loader_alloc(BIOSLinker *linker,
  * @linker: linker object instance
  * @file: file that includes the checksum to be calculated
  *        and the data to be checksummed
- * @table: @file blob contents
  * @start, @size: range of data to checksum
  * @checksum: location of the checksum to be patched within file blob
  *
  * Notes:
- * - checksum byte initial value must have been pushed into @table
- *   and reside at address @checksum.
- * - @size bytes must have been pushed into @table and reside at address
- *   @start.
+ * - checksum byte initial value must have been pushed into blob
+ *   associated with @file and reside at address @checksum.
+ * - @size bytes must have been pushed into blob associated wtih @file
+ *   and reside at address @start.
  * - Guest calculates checksum of specified range of data, result is added to
  *   initial value at @checksum into copy of @file in Guest memory.
  * - Range might include the checksum itself.
  * - To avoid confusion, caller must always put 0x0 at @checksum.
  * - @file must be loaded into Guest memory using bios_linker_loader_alloc
  */
-void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
-                                     GArray *table,
+void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file_name,
                                      void *start, unsigned size,
                                      uint8_t *checksum)
 {
     BiosLinkerLoaderEntry entry;
-    ptrdiff_t checksum_offset = (gchar *)checksum - table->data;
-    ptrdiff_t start_offset = (gchar *)start - table->data;
+    const BiosLinkerFileEntry *file = bios_linker_find_file(linker, file_name);
+    ptrdiff_t checksum_offset = (gchar *)checksum - file->blob->data;
+    ptrdiff_t start_offset = (gchar *)start - file->blob->data;
 
     assert(checksum_offset >= 0);
     assert(start_offset >= 0);
-    assert(checksum_offset + 1 <= table->len);
-    assert(start_offset + size <= table->len);
+    assert(checksum_offset + 1 <= file->blob->len);
+    assert(start_offset + size <= file->blob->len);
     assert(*checksum == 0x0);
 
     memset(&entry, 0, sizeof entry);
-    strncpy(entry.cksum.file, file, sizeof entry.cksum.file - 1);
+    strncpy(entry.cksum.file, file_name, sizeof entry.cksum.file - 1);
     entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM);
     entry.cksum.offset = cpu_to_le32(checksum_offset);
     entry.cksum.start = cpu_to_le32(start_offset);
@@ -205,13 +244,12 @@ void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
  * @linker: linker object instance
  * @dest_file: destination file that must be changed
  * @src_file: source file who's address must be taken
- * @table: @dest_file blob contents array
  * @pointer: location of the pointer to be patched within destination file blob
  * @pointer_size: size of pointer to be patched, in bytes
  *
  * Notes:
- * - @pointer_size bytes must have been pushed into @table
- *   and reside at address @pointer.
+ * - @pointer_size bytes must have been pushed into blob associated with
+ *   @dest_file and reside at address @pointer.
  * - Guest address is added to initial value at @pointer
  *   into copy of @dest_file in Guest memory.
  *   e.g. to get start of src_file in guest memory, put 0x0 there
@@ -222,14 +260,15 @@ void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
 void bios_linker_loader_add_pointer(BIOSLinker *linker,
                                     const char *dest_file,
                                     const char *src_file,
-                                    GArray *table, void *pointer,
+                                    void *pointer,
                                     uint8_t pointer_size)
 {
     BiosLinkerLoaderEntry entry;
-    ptrdiff_t offset = (gchar *)pointer - table->data;
+    const BiosLinkerFileEntry *file = bios_linker_find_file(linker, dest_file);
+    ptrdiff_t offset = (gchar *)pointer - file->blob->data;
 
     assert(offset >= 0);
-    assert(offset + pointer_size <= table->len);
+    assert(offset + pointer_size <= file->blob->len);
 
     memset(&entry, 0, sizeof entry);
     strncpy(entry.pointer.dest_file, dest_file,
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index a2d20ea..9d95b1d 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -579,7 +579,8 @@ static void nvdimm_build_nvdimm_devices(GSList *device_list, Aml *root_dev)
 }
 
 static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
-                              GArray *table_data, BIOSLinker *linker)
+                              GArray *table_data, BIOSLinker *linker,
+                              GArray *dsm_dma_arrea)
 {
     Aml *ssdt, *sb_scope, *dev, *field;
     int mem_addr_offset, nvdimm_ssdt;
@@ -678,10 +679,11 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
     mem_addr_offset = build_append_named_dword(table_data,
                                                NVDIMM_ACPI_MEM_ADDR);
 
-    bios_linker_loader_alloc(linker, NVDIMM_DSM_MEM_FILE, sizeof(NvdimmDsmIn),
-                             false /* high memory */);
+    bios_linker_loader_alloc(linker,
+                             NVDIMM_DSM_MEM_FILE, dsm_dma_arrea,
+                             sizeof(NvdimmDsmIn), false /* high memory */);
     bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
-                                   NVDIMM_DSM_MEM_FILE, table_data,
+                                   NVDIMM_DSM_MEM_FILE,
                                    table_data->data + mem_addr_offset,
                                    sizeof(uint32_t));
     build_header(linker, table_data,
@@ -691,7 +693,7 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
 }
 
 void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
-                       BIOSLinker *linker)
+                       BIOSLinker *linker, GArray *dsm_dma_arrea)
 {
     GSList *device_list;
 
@@ -701,6 +703,7 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
         return;
     }
     nvdimm_build_nfit(device_list, table_offsets, table_data, linker);
-    nvdimm_build_ssdt(device_list, table_offsets, table_data, linker);
+    nvdimm_build_ssdt(device_list, table_offsets, table_data, linker,
+                      dsm_dma_arrea);
     g_slist_free(device_list);
 }
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index fc130ff..2b23cef 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -358,7 +358,7 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
 {
     AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
 
-    bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, 16,
+    bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
                              true /* fseg memory */);
 
     memcpy(&rsdp->signature, "RSD PTR ", sizeof(rsdp->signature));
@@ -371,12 +371,12 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
     /* Address to be filled by Guest linker */
     bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE,
                                    ACPI_BUILD_TABLE_FILE,
-                                   rsdp_table, &rsdp->rsdt_physical_address,
+                                   &rsdp->rsdt_physical_address,
                                    sizeof rsdp->rsdt_physical_address);
     rsdp->checksum = 0;
     /* Checksum to be filled by Guest linker */
     bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
-                                    rsdp_table, rsdp, sizeof *rsdp,
+                                    rsdp, sizeof *rsdp,
                                     &rsdp->checksum);
 
     return rsdp_table;
@@ -582,7 +582,7 @@ build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt)
     /* DSDT address to be filled by Guest linker */
     bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
                                    ACPI_BUILD_TABLE_FILE,
-                                   table_data, &fadt->dsdt,
+                                   &fadt->dsdt,
                                    sizeof fadt->dsdt);
 
     build_header(linker, table_data,
@@ -651,7 +651,8 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
     table_offsets = g_array_new(false, true /* clear */,
                                         sizeof(uint32_t));
 
-    bios_linker_loader_alloc(tables->linker, ACPI_BUILD_TABLE_FILE,
+    bios_linker_loader_alloc(tables->linker,
+                             ACPI_BUILD_TABLE_FILE, tables_blob,
                              64, false /* high memory */);
 
     /*
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index e6c390c..5aa70c1 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -315,14 +315,14 @@ build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
     /* FACS address to be filled by Guest linker */
     bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
                                    ACPI_BUILD_TABLE_FILE,
-                                   table_data, &fadt->firmware_ctrl,
+                                   &fadt->firmware_ctrl,
                                    sizeof fadt->firmware_ctrl);
 
     fadt->dsdt = cpu_to_le32(dsdt);
     /* DSDT address to be filled by Guest linker */
     bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
                                    ACPI_BUILD_TABLE_FILE,
-                                   table_data, &fadt->dsdt,
+                                   &fadt->dsdt,
                                    sizeof fadt->dsdt);
 
     fadt_setup(fadt, pm);
@@ -2264,13 +2264,13 @@ build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
     tcpa->log_area_minimum_length = cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE);
     tcpa->log_area_start_address = cpu_to_le64(log_area_start_address);
 
-    bios_linker_loader_alloc(linker, ACPI_BUILD_TPMLOG_FILE, 1,
+    bios_linker_loader_alloc(linker, ACPI_BUILD_TPMLOG_FILE, tcpalog, 1,
                              false /* high memory */);
 
     /* log area start address to be filled by Guest linker */
     bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
                                    ACPI_BUILD_TPMLOG_FILE,
-                                   table_data, &tcpa->log_area_start_address,
+                                   &tcpa->log_area_start_address,
                                    sizeof(tcpa->log_area_start_address));
 
     build_header(linker, table_data,
@@ -2449,7 +2449,7 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
 {
     AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
 
-    bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, 16,
+    bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
                              true /* fseg memory */);
 
     memcpy(&rsdp->signature, "RSD PTR ", 8);
@@ -2458,12 +2458,12 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
     /* Address to be filled by Guest linker */
     bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE,
                                    ACPI_BUILD_TABLE_FILE,
-                                   rsdp_table, &rsdp->rsdt_physical_address,
+                                   &rsdp->rsdt_physical_address,
                                    sizeof rsdp->rsdt_physical_address);
     rsdp->checksum = 0;
     /* Checksum to be filled by Guest linker */
     bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
-                                    rsdp_table, rsdp, sizeof *rsdp,
+                                    rsdp, sizeof *rsdp,
                                     &rsdp->checksum);
 
     return rsdp_table;
@@ -2537,7 +2537,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
                                         sizeof(uint32_t));
     ACPI_BUILD_DPRINTF("init ACPI tables\n");
 
-    bios_linker_loader_alloc(tables->linker, ACPI_BUILD_TABLE_FILE,
+    bios_linker_loader_alloc(tables->linker,
+                             ACPI_BUILD_TABLE_FILE, tables_blob,
                              64 /* Ensure FACS is aligned */,
                              false /* high memory */);
 
@@ -2594,7 +2595,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
         build_dmar_q35(tables_blob, tables->linker);
     }
     if (pcms->acpi_nvdimm_state.is_enabled) {
-        nvdimm_build_acpi(table_offsets, tables_blob, tables->linker);
+        nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
+                          pcms->acpi_nvdimm_state.dsm_mem);
     }
 
     /* Add tables supplied by user (if any) */
-- 
MST

[Qemu-devel] [PULL v2 19/25] acpi: cleanup bios_linker_loader_cleanup()

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

bios_linker_loader_cleanup() is called only from one place
and returned value is immediately freed wich makes returning
pointer from bios_linker_loader_cleanup() useless.

Cleanup bios_linker_loader_cleanup() by freeing
data there so that caller won't have to free it.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/bios-linker-loader.h | 2 +-
 hw/acpi/aml-build.c                  | 3 +--
 hw/acpi/bios-linker-loader.c         | 8 ++++----
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/hw/acpi/bios-linker-loader.h b/include/hw/acpi/bios-linker-loader.h
index bee6dee..564e192 100644
--- a/include/hw/acpi/bios-linker-loader.h
+++ b/include/hw/acpi/bios-linker-loader.h
@@ -26,5 +26,5 @@ void bios_linker_loader_add_pointer(BIOSLinker *linker,
                                     void *pointer,
                                     uint8_t pointer_size);
 
-void *bios_linker_loader_cleanup(BIOSLinker *linker);
+void bios_linker_loader_cleanup(BIOSLinker *linker);
 #endif
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index bff2599..66e9bf8 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1548,8 +1548,7 @@ void acpi_build_tables_init(AcpiBuildTables *tables)
 
 void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre)
 {
-    void *linker_data = bios_linker_loader_cleanup(tables->linker);
-    g_free(linker_data);
+    bios_linker_loader_cleanup(tables->linker);
     g_array_free(tables->rsdp, true);
     g_array_free(tables->table_data, true);
     g_array_free(tables->tcpalog, mfre);
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index 8e3f30d..b67da68 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -121,12 +121,13 @@ BIOSLinker *bios_linker_loader_init(void)
     return linker;
 }
 
-/* Free linker wrapper and return the linker commands array. */
-void *bios_linker_loader_cleanup(BIOSLinker *linker)
+/* Free linker wrapper */
+void bios_linker_loader_cleanup(BIOSLinker *linker)
 {
     int i;
     BiosLinkerFileEntry *entry;
-    void *cmd_blob = g_array_free(linker->cmd_blob, false);
+
+    g_array_free(linker->cmd_blob, true);
 
     for (i = 0; i < linker->file_list->len; i++) {
         entry = &g_array_index(linker->file_list, BiosLinkerFileEntry, i);
@@ -134,7 +135,6 @@ void *bios_linker_loader_cleanup(BIOSLinker *linker)
     }
     g_array_free(linker->file_list, true);
     g_free(linker);
-    return cmd_blob;
 }
 
 static const BiosLinkerFileEntry *
-- 
MST

[Qemu-devel] [PULL v2 20/25] tpm: apci: cleanup TCPA table initialization

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

At the time build_tpm_tcpa() is called the tcpalog size is
always 0, so log_area_start_address which is actually offset
from the start of ACPI_BUILD_TPMLOG_FILE is always 0.

Also as 'TCPA' is allocated 0 filled, there is no point
in calculating always 0 log_area_start_address and set
tcpa->log_area_start_address to it since the field should
always point to start of ACPI_BUILD_TPMLOG_FILE.
Make code easier to read dropping not needed offset
calculations.
While at that move tcpalog allocation closer to the code
that defines its size.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5aa70c1..1e8364a 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2258,11 +2258,10 @@ static void
 build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
 {
     Acpi20Tcpa *tcpa = acpi_data_push(table_data, sizeof *tcpa);
-    uint64_t log_area_start_address = acpi_data_len(tcpalog);
 
     tcpa->platform_class = cpu_to_le16(TPM_TCPA_ACPI_CLASS_CLIENT);
     tcpa->log_area_minimum_length = cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE);
-    tcpa->log_area_start_address = cpu_to_le64(log_area_start_address);
+    acpi_data_push(tcpalog, le32_to_cpu(tcpa->log_area_minimum_length));
 
     bios_linker_loader_alloc(linker, ACPI_BUILD_TPMLOG_FILE, tcpalog, 1,
                              false /* high memory */);
@@ -2275,8 +2274,6 @@ build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
 
     build_header(linker, table_data,
                  (void *)tcpa, "TCPA", sizeof(*tcpa), 2, NULL, NULL);
-
-    acpi_data_push(tcpalog, TPM_LOG_AREA_MINIMUM_SIZE);
 }
 
 static void
-- 
MST

[Qemu-devel] [PULL v2 21/25] acpi: make bios_linker_loader_add_pointer() API offset based

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

cleanup bios_linker_loader_add_pointer() API by switching
arguments to taking offsets relative to corresponding files
instead of doing pointer arithmetic on behalf of user which
were confusing.

Also make offset inside of source file explicit in API
so that user won't have to manually set it in
destination file blob and while at it add additional
boundary checks.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/bios-linker-loader.h |  5 ++--
 hw/acpi/aml-build.c                  | 22 ++++++++-------
 hw/acpi/bios-linker-loader.c         | 53 ++++++++++++++++++++----------------
 hw/acpi/nvdimm.c                     |  7 ++---
 hw/arm/virt-acpi-build.c             | 26 +++++++++---------
 hw/i386/acpi-build.c                 | 45 +++++++++++++++---------------
 6 files changed, 83 insertions(+), 75 deletions(-)

diff --git a/include/hw/acpi/bios-linker-loader.h b/include/hw/acpi/bios-linker-loader.h
index 564e192..f666b7c 100644
--- a/include/hw/acpi/bios-linker-loader.h
+++ b/include/hw/acpi/bios-linker-loader.h
@@ -22,9 +22,10 @@ void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
 
 void bios_linker_loader_add_pointer(BIOSLinker *linker,
                                     const char *dest_file,
+                                    uint32_t dst_patched_offset,
+                                    uint8_t dst_patched_size,
                                     const char *src_file,
-                                    void *pointer,
-                                    uint8_t pointer_size);
+                                    uint32_t src_offset);
 
 void bios_linker_loader_cleanup(BIOSLinker *linker);
 #endif
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 66e9bf8..d025837 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1534,7 +1534,7 @@ unsigned acpi_data_len(GArray *table)
 
 void acpi_add_table(GArray *table_offsets, GArray *table_data)
 {
-    uint32_t offset = cpu_to_le32(table_data->len);
+    uint32_t offset = table_data->len;
     g_array_append_val(table_offsets, offset);
 }
 
@@ -1559,21 +1559,23 @@ void
 build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
            const char *oem_id, const char *oem_table_id)
 {
-    AcpiRsdtDescriptorRev1 *rsdt;
-    size_t rsdt_len;
     int i;
-    const int table_data_len = (sizeof(uint32_t) * table_offsets->len);
+    unsigned rsdt_entries_offset;
+    AcpiRsdtDescriptorRev1 *rsdt;
+    const unsigned table_data_len = (sizeof(uint32_t) * table_offsets->len);
+    const unsigned rsdt_entry_size = sizeof(rsdt->table_offset_entry[0]);
+    const size_t rsdt_len = sizeof(*rsdt) + table_data_len;
 
-    rsdt_len = sizeof(*rsdt) + table_data_len;
     rsdt = acpi_data_push(table_data, rsdt_len);
-    memcpy(rsdt->table_offset_entry, table_offsets->data, table_data_len);
+    rsdt_entries_offset = (char *)rsdt->table_offset_entry - table_data->data;
     for (i = 0; i < table_offsets->len; ++i) {
+        uint32_t ref_tbl_offset = g_array_index(table_offsets, uint32_t, i);
+        uint32_t rsdt_entry_offset = rsdt_entries_offset + rsdt_entry_size * i;
+
         /* rsdt->table_offset_entry to be filled by Guest linker */
         bios_linker_loader_add_pointer(linker,
-                                       ACPI_BUILD_TABLE_FILE,
-                                       ACPI_BUILD_TABLE_FILE,
-                                       &rsdt->table_offset_entry[i],
-                                       sizeof(uint32_t));
+            ACPI_BUILD_TABLE_FILE, rsdt_entry_offset, rsdt_entry_size,
+            ACPI_BUILD_TABLE_FILE, ref_tbl_offset);
     }
     build_header(linker, table_data,
                  (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id);
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index b67da68..3fb54dc 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -238,37 +238,38 @@ void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file_name,
 }
 
 /*
- * bios_linker_loader_add_pointer: ask guest to add address of source file
- * into destination file at the specified pointer.
+ * bios_linker_loader_add_pointer: ask guest to patch address in
+ * destination file with a pointer to source file
  *
  * @linker: linker object instance
  * @dest_file: destination file that must be changed
+ * @dst_patched_offset: location within destination file blob to be patched
+ *                      with the pointer to @src_file+@src_offset (i.e. source
+ *                      blob allocated in guest memory + @src_offset), in bytes
+ * @dst_patched_offset_size: size of the pointer to be patched
+ *                      at @dst_patched_offset in @dest_file blob, in bytes
  * @src_file: source file who's address must be taken
- * @pointer: location of the pointer to be patched within destination file blob
- * @pointer_size: size of pointer to be patched, in bytes
- *
- * Notes:
- * - @pointer_size bytes must have been pushed into blob associated with
- *   @dest_file and reside at address @pointer.
- * - Guest address is added to initial value at @pointer
- *   into copy of @dest_file in Guest memory.
- *   e.g. to get start of src_file in guest memory, put 0x0 there
- *   to get address of a field at offset 0x10 in src_file, put 0x10 there
- * - Both @dest_file and @src_file must be
- *   loaded into Guest memory using bios_linker_loader_alloc
+ * @src_offset: location within source file blob to which
+ *              @dest_file+@dst_patched_offset will point to after
+ *              firmware's executed ADD_POINTER command
  */
 void bios_linker_loader_add_pointer(BIOSLinker *linker,
                                     const char *dest_file,
+                                    uint32_t dst_patched_offset,
+                                    uint8_t dst_patched_size,
                                     const char *src_file,
-                                    void *pointer,
-                                    uint8_t pointer_size)
+                                    uint32_t src_offset)
 {
+    uint64_t le_src_offset;
     BiosLinkerLoaderEntry entry;
-    const BiosLinkerFileEntry *file = bios_linker_find_file(linker, dest_file);
-    ptrdiff_t offset = (gchar *)pointer - file->blob->data;
+    const BiosLinkerFileEntry *dst_file =
+        bios_linker_find_file(linker, dest_file);
+    const BiosLinkerFileEntry *source_file =
+        bios_linker_find_file(linker, src_file);
 
-    assert(offset >= 0);
-    assert(offset + pointer_size <= file->blob->len);
+    assert(dst_patched_offset < dst_file->blob->len);
+    assert(dst_patched_offset + dst_patched_size <= dst_file->blob->len);
+    assert(src_offset < source_file->blob->len);
 
     memset(&entry, 0, sizeof entry);
     strncpy(entry.pointer.dest_file, dest_file,
@@ -276,10 +277,14 @@ void bios_linker_loader_add_pointer(BIOSLinker *linker,
     strncpy(entry.pointer.src_file, src_file,
             sizeof entry.pointer.src_file - 1);
     entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ADD_POINTER);
-    entry.pointer.offset = cpu_to_le32(offset);
-    entry.pointer.size = pointer_size;
-    assert(pointer_size == 1 || pointer_size == 2 ||
-           pointer_size == 4 || pointer_size == 8);
+    entry.pointer.offset = cpu_to_le32(dst_patched_offset);
+    entry.pointer.size = dst_patched_size;
+    assert(dst_patched_size == 1 || dst_patched_size == 2 ||
+           dst_patched_size == 4 || dst_patched_size == 8);
+
+    le_src_offset = cpu_to_le64(src_offset);
+    memcpy(dst_file->blob->data + dst_patched_offset,
+           &le_src_offset, dst_patched_size);
 
     g_array_append_vals(linker->cmd_blob, &entry, sizeof entry);
 }
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 9d95b1d..b4c2262 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -682,10 +682,9 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
     bios_linker_loader_alloc(linker,
                              NVDIMM_DSM_MEM_FILE, dsm_dma_arrea,
                              sizeof(NvdimmDsmIn), false /* high memory */);
-    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
-                                   NVDIMM_DSM_MEM_FILE,
-                                   table_data->data + mem_addr_offset,
-                                   sizeof(uint32_t));
+    bios_linker_loader_add_pointer(linker,
+        ACPI_BUILD_TABLE_FILE, mem_addr_offset, sizeof(uint32_t),
+        NVDIMM_DSM_MEM_FILE, 0);
     build_header(linker, table_data,
         (void *)(table_data->data + nvdimm_ssdt),
         "SSDT", table_data->len - nvdimm_ssdt, 1, NULL, "NVDIMM");
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 2b23cef..1cbb496 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -354,9 +354,12 @@ static void acpi_dsdt_add_power_button(Aml *scope)
 
 /* RSDP */
 static GArray *
-build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
+build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset)
 {
     AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
+    unsigned rsdt_pa_size = sizeof(rsdp->rsdt_physical_address);
+    unsigned rsdt_pa_offset =
+        (char *)&rsdp->rsdt_physical_address - rsdp_table->data;
 
     bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
                              true /* fseg memory */);
@@ -366,13 +369,11 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
     rsdp->length = cpu_to_le32(sizeof(*rsdp));
     rsdp->revision = 0x02;
 
-    /* Point to RSDT */
-    rsdp->rsdt_physical_address = cpu_to_le32(rsdt);
     /* Address to be filled by Guest linker */
-    bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE,
-                                   ACPI_BUILD_TABLE_FILE,
-                                   &rsdp->rsdt_physical_address,
-                                   sizeof rsdp->rsdt_physical_address);
+    bios_linker_loader_add_pointer(linker,
+        ACPI_BUILD_RSDP_FILE, rsdt_pa_offset, rsdt_pa_size,
+        ACPI_BUILD_TABLE_FILE, rsdt_tbl_offset);
+
     rsdp->checksum = 0;
     /* Checksum to be filled by Guest linker */
     bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
@@ -566,9 +567,10 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
 
 /* FADT */
 static void
-build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt)
+build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt_tbl_offset)
 {
     AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
+    unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
 
     /* Hardware Reduced = 1 and use PSCI 0.2+ and with HVC */
     fadt->flags = cpu_to_le32(1 << ACPI_FADT_F_HW_REDUCED_ACPI);
@@ -578,12 +580,10 @@ build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt)
     /* ACPI v5.1 (fadt->revision.fadt->minor_revision) */
     fadt->minor_revision = 0x1;
 
-    fadt->dsdt = cpu_to_le32(dsdt);
     /* DSDT address to be filled by Guest linker */
-    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
-                                   ACPI_BUILD_TABLE_FILE,
-                                   &fadt->dsdt,
-                                   sizeof fadt->dsdt);
+    bios_linker_loader_add_pointer(linker,
+        ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
+        ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
 
     build_header(linker, table_data,
                  (void *)fadt, "FACP", sizeof(*fadt), 5, NULL, NULL);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1e8364a..b3ce5be 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -306,26 +306,23 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
 /* FADT */
 static void
 build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
-           unsigned facs, unsigned dsdt,
+           unsigned facs_tbl_offset, unsigned dsdt_tbl_offset,
            const char *oem_id, const char *oem_table_id)
 {
     AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
+    unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data->data;
+    unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
 
-    fadt->firmware_ctrl = cpu_to_le32(facs);
     /* FACS address to be filled by Guest linker */
-    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
-                                   ACPI_BUILD_TABLE_FILE,
-                                   &fadt->firmware_ctrl,
-                                   sizeof fadt->firmware_ctrl);
+    bios_linker_loader_add_pointer(linker,
+        ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->firmware_ctrl),
+        ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
 
-    fadt->dsdt = cpu_to_le32(dsdt);
     /* DSDT address to be filled by Guest linker */
-    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
-                                   ACPI_BUILD_TABLE_FILE,
-                                   &fadt->dsdt,
-                                   sizeof fadt->dsdt);
-
     fadt_setup(fadt, pm);
+    bios_linker_loader_add_pointer(linker,
+        ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
+        ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
 
     build_header(linker, table_data,
                  (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id);
@@ -2258,6 +2255,9 @@ static void
 build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
 {
     Acpi20Tcpa *tcpa = acpi_data_push(table_data, sizeof *tcpa);
+    unsigned log_addr_size = sizeof(tcpa->log_area_start_address);
+    unsigned log_addr_offset =
+        (char *)&tcpa->log_area_start_address - table_data->data;
 
     tcpa->platform_class = cpu_to_le16(TPM_TCPA_ACPI_CLASS_CLIENT);
     tcpa->log_area_minimum_length = cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE);
@@ -2267,10 +2267,9 @@ build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
                              false /* high memory */);
 
     /* log area start address to be filled by Guest linker */
-    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
-                                   ACPI_BUILD_TPMLOG_FILE,
-                                   &tcpa->log_area_start_address,
-                                   sizeof(tcpa->log_area_start_address));
+    bios_linker_loader_add_pointer(linker,
+        ACPI_BUILD_TABLE_FILE, log_addr_offset, log_addr_size,
+        ACPI_BUILD_TPMLOG_FILE, 0);
 
     build_header(linker, table_data,
                  (void *)tcpa, "TCPA", sizeof(*tcpa), 2, NULL, NULL);
@@ -2442,21 +2441,23 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
 }
 
 static GArray *
-build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt)
+build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset)
 {
     AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
+    unsigned rsdt_pa_size = sizeof(rsdp->rsdt_physical_address);
+    unsigned rsdt_pa_offset =
+        (char *)&rsdp->rsdt_physical_address - rsdp_table->data;
 
     bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
                              true /* fseg memory */);
 
     memcpy(&rsdp->signature, "RSD PTR ", 8);
     memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6);
-    rsdp->rsdt_physical_address = cpu_to_le32(rsdt);
     /* Address to be filled by Guest linker */
-    bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE,
-                                   ACPI_BUILD_TABLE_FILE,
-                                   &rsdp->rsdt_physical_address,
-                                   sizeof rsdp->rsdt_physical_address);
+    bios_linker_loader_add_pointer(linker,
+        ACPI_BUILD_RSDP_FILE, rsdt_pa_offset, rsdt_pa_size,
+        ACPI_BUILD_TABLE_FILE, rsdt_tbl_offset);
+
     rsdp->checksum = 0;
     /* Checksum to be filled by Guest linker */
     bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
-- 
MST

[Qemu-devel] [PULL v2 22/25] acpi: make bios_linker_loader_add_checksum() API offset based

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

It should help to make clear that bios_linker works in terms
of offsets within a file. Also it should prevent mistakes
where user passes as arguments pointers to unrelated to file blobs.

While at it, considering that it's a ACPI checksum and
it's initial value must be 0, move checksum field zeroing
into bios_linker_loader_add_checksum() instead of doing it
at every call site manually before bios_linker_loader_add_checksum()
is called.

In addition add extra boundary checks.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/bios-linker-loader.h |  4 ++--
 hw/acpi/aml-build.c                  |  5 +++--
 hw/acpi/bios-linker-loader.c         | 36 +++++++++++++-----------------------
 hw/arm/virt-acpi-build.c             |  5 ++---
 hw/i386/acpi-build.c                 |  5 ++---
 5 files changed, 22 insertions(+), 33 deletions(-)

diff --git a/include/hw/acpi/bios-linker-loader.h b/include/hw/acpi/bios-linker-loader.h
index f666b7c..a05227e 100644
--- a/include/hw/acpi/bios-linker-loader.h
+++ b/include/hw/acpi/bios-linker-loader.h
@@ -17,8 +17,8 @@ void bios_linker_loader_alloc(BIOSLinker *linker,
                               bool alloc_fseg);
 
 void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
-                                     void *start, unsigned size,
-                                     uint8_t *checksum);
+                                     unsigned start_offset, unsigned size,
+                                     unsigned checksum_offset);
 
 void bios_linker_loader_add_pointer(BIOSLinker *linker,
                                     const char *dest_file,
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index d025837..123160a 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1493,6 +1493,8 @@ build_header(BIOSLinker *linker, GArray *table_data,
              AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
              const char *oem_id, const char *oem_table_id)
 {
+    unsigned tbl_offset = (char *)h - table_data->data;
+    unsigned checksum_offset = (char *)&h->checksum - table_data->data;
     memcpy(&h->signature, sig, 4);
     h->length = cpu_to_le32(len);
     h->revision = rev;
@@ -1513,10 +1515,9 @@ build_header(BIOSLinker *linker, GArray *table_data,
     h->oem_revision = cpu_to_le32(1);
     memcpy(h->asl_compiler_id, ACPI_BUILD_APPNAME4, 4);
     h->asl_compiler_revision = cpu_to_le32(1);
-    h->checksum = 0;
     /* Checksum to be filled in by Guest linker */
     bios_linker_loader_add_checksum(linker, ACPI_BUILD_TABLE_FILE,
-                                    h, len, &h->checksum);
+        tbl_offset, len, checksum_offset);
 }
 
 void *acpi_data_push(GArray *table_data, unsigned size)
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index 3fb54dc..d963ebe 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -189,8 +189,8 @@ void bios_linker_loader_alloc(BIOSLinker *linker,
 }
 
 /*
- * bios_linker_loader_add_checksum: ask guest to add checksum of file data
- * into (same) file at the specified pointer.
+ * bios_linker_loader_add_checksum: ask guest to add checksum of ACPI
+ * table in the specified file at the specified offset.
  *
  * Checksum calculation simply sums -X for each byte X in the range
  * using 8-bit math (i.e. ACPI checksum).
@@ -198,35 +198,25 @@ void bios_linker_loader_alloc(BIOSLinker *linker,
  * @linker: linker object instance
  * @file: file that includes the checksum to be calculated
  *        and the data to be checksummed
- * @start, @size: range of data to checksum
- * @checksum: location of the checksum to be patched within file blob
- *
- * Notes:
- * - checksum byte initial value must have been pushed into blob
- *   associated with @file and reside at address @checksum.
- * - @size bytes must have been pushed into blob associated wtih @file
- *   and reside at address @start.
- * - Guest calculates checksum of specified range of data, result is added to
- *   initial value at @checksum into copy of @file in Guest memory.
- * - Range might include the checksum itself.
- * - To avoid confusion, caller must always put 0x0 at @checksum.
- * - @file must be loaded into Guest memory using bios_linker_loader_alloc
+ * @start_offset, @size: range of data in the file to checksum,
+ *                       relative to the start of file blob
+ * @checksum_offset: location of the checksum to be patched within file blob,
+ *                   relative to the start of file blob
  */
 void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file_name,
-                                     void *start, unsigned size,
-                                     uint8_t *checksum)
+                                     unsigned start_offset, unsigned size,
+                                     unsigned checksum_offset)
 {
     BiosLinkerLoaderEntry entry;
     const BiosLinkerFileEntry *file = bios_linker_find_file(linker, file_name);
-    ptrdiff_t checksum_offset = (gchar *)checksum - file->blob->data;
-    ptrdiff_t start_offset = (gchar *)start - file->blob->data;
 
-    assert(checksum_offset >= 0);
-    assert(start_offset >= 0);
-    assert(checksum_offset + 1 <= file->blob->len);
+    assert(file);
+    assert(start_offset < file->blob->len);
     assert(start_offset + size <= file->blob->len);
-    assert(*checksum == 0x0);
+    assert(checksum_offset >= start_offset);
+    assert(checksum_offset + 1 <= start_offset + size);
 
+    *(file->blob->data + checksum_offset) = 0;
     memset(&entry, 0, sizeof entry);
     strncpy(entry.cksum.file, file_name, sizeof entry.cksum.file - 1);
     entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM);
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 1cbb496..735ab86 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -374,11 +374,10 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset)
         ACPI_BUILD_RSDP_FILE, rsdt_pa_offset, rsdt_pa_size,
         ACPI_BUILD_TABLE_FILE, rsdt_tbl_offset);
 
-    rsdp->checksum = 0;
     /* Checksum to be filled by Guest linker */
     bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
-                                    rsdp, sizeof *rsdp,
-                                    &rsdp->checksum);
+        (char *)rsdp - rsdp_table->data, sizeof *rsdp,
+        (char *)&rsdp->checksum - rsdp_table->data);
 
     return rsdp_table;
 }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b3ce5be..06d6204 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2458,11 +2458,10 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset)
         ACPI_BUILD_RSDP_FILE, rsdt_pa_offset, rsdt_pa_size,
         ACPI_BUILD_TABLE_FILE, rsdt_tbl_offset);
 
-    rsdp->checksum = 0;
     /* Checksum to be filled by Guest linker */
     bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
-                                    rsdp, sizeof *rsdp,
-                                    &rsdp->checksum);
+        (char *)rsdp - rsdp_table->data, sizeof *rsdp,
+        (char *)&rsdp->checksum - rsdp_table->data);
 
     return rsdp_table;
 }
-- 
MST

[Qemu-devel] [PULL v2 23/25] pc-dimm: get memory region from ->get_memory_region()

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Curretly, the memory region of backed memory is all directly
mapped to guest's address space, however, it will be not true
for nvdimm device if we introduce nvdimm label which only can
be indirectly accessed by ACPI DSM method

Also it improves the comments a bit to reflect this fact

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/mem/pc-dimm.h | 3 ++-
 hw/mem/pc-dimm.c         | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index 8cdc326..6024627 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -58,7 +58,8 @@ typedef struct PCDIMMDevice {
 
 /**
  * PCDIMMDeviceClass:
- * @get_memory_region: returns #MemoryRegion associated with @dimm
+ * @get_memory_region: returns #MemoryRegion associated with @dimm which
+ * is directly mapped into the physical address space of guest
  */
 typedef struct PCDIMMDeviceClass {
     /* private */
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 9e7de56..70b9451 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -354,8 +354,9 @@ static void pc_dimm_get_size(Object *obj, Visitor *v, const char *name,
     int64_t value;
     MemoryRegion *mr;
     PCDIMMDevice *dimm = PC_DIMM(obj);
+    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(obj);
 
-    mr = host_memory_backend_get_memory(dimm->hostmem, errp);
+    mr = ddc->get_memory_region(dimm);
     value = memory_region_size(mr);
 
     visit_type_int(v, name, &value, errp);
-- 
MST

[Qemu-devel] [PULL v2 24/25] pc-dimm: introduce realize callback

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

nvdimm needs to  check if the backend memory is large enough to contain
label data and init its memory region when the device is realized, so
introduce realize callback which is called after common dimm has been
realize

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/mem/pc-dimm.h | 3 +++
 hw/mem/pc-dimm.c         | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index 6024627..67e92d8 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -58,6 +58,8 @@ typedef struct PCDIMMDevice {
 
 /**
  * PCDIMMDeviceClass:
+ * @realize: called after common dimm is realized so that the dimm based
+ * devices get the chance to do specified operations.
  * @get_memory_region: returns #MemoryRegion associated with @dimm which
  * is directly mapped into the physical address space of guest
  */
@@ -66,6 +68,7 @@ typedef struct PCDIMMDeviceClass {
     DeviceClass parent_class;
 
     /* public */
+    void (*realize)(PCDIMMDevice *dimm, Error **errp);
     MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm);
 } PCDIMMDeviceClass;
 
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 70b9451..6de2275 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -400,6 +400,7 @@ static void pc_dimm_init(Object *obj)
 static void pc_dimm_realize(DeviceState *dev, Error **errp)
 {
     PCDIMMDevice *dimm = PC_DIMM(dev);
+    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
 
     if (!dimm->hostmem) {
         error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property is not set");
@@ -412,6 +413,10 @@ static void pc_dimm_realize(DeviceState *dev, Error **errp)
                    dimm->node, nb_numa_nodes ? nb_numa_nodes : 1);
         return;
     }
+
+    if (ddc->realize) {
+        ddc->realize(dimm, errp);
+    }
 }
 
 static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm)
-- 
MST

[Qemu-devel] [PULL v2 25/25] virtio: move bi-endian target support to a single location

[Michael S. Tsirkin]

From: Greg Kurz <gkurz@linux.vnet.ibm.com>

Paolo's recent cpu.h cleanups broke legacy virtio for ppc64 LE guests (and
arm BE guests as well, even if I have not verified that). Especially, commit
"33c11879fd42 qemu-common: push cpu.h inclusion out of qemu-common.h" has
the side-effect of silently hiding the TARGET_IS_BIENDIAN macro from the
virtio memory accessors, and thus fully disabling support of endian changing
targets.

To be sure this cannot happen again, let's gather all the bi-endian bits
where they belong in include/hw/virtio/virtio-access.h.

The changes in hw/virtio/vhost.c are safe because vhost_needs_vring_endian()
is not called on a hot path and non bi-endian targets will return false
anyway.

While here, also rename TARGET_IS_BIENDIAN to be more precise: it is only for
legacy virtio and bi-endian guests.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/hw/virtio/virtio-access.h | 6 +++++-
 target-arm/cpu.h                  | 2 --
 target-ppc/cpu.h                  | 2 --
 hw/virtio/vhost.c                 | 4 ----
 4 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index 8dc84f5..4b28038 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -17,9 +17,13 @@
 #include "hw/virtio/virtio.h"
 #include "exec/address-spaces.h"
 
+#if defined(TARGET_PPC64) || defined(TARGET_ARM)
+#define LEGACY_VIRTIO_IS_BIENDIAN 1
+#endif
+
 static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
 {
-#if defined(TARGET_IS_BIENDIAN)
+#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
     return virtio_is_big_endian(vdev);
 #elif defined(TARGET_WORDS_BIGENDIAN)
     if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 401955f..17d8051 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -29,8 +29,6 @@
 #  define TARGET_LONG_BITS 32
 #endif
 
-#define TARGET_IS_BIENDIAN 1
-
 #define CPUArchState struct CPUARMState
 
 #include "qemu-common.h"
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 98a24a5..db7ee0c 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -28,8 +28,6 @@
 #define TARGET_LONG_BITS 64
 #define TARGET_PAGE_BITS 12
 
-#define TARGET_IS_BIENDIAN 1
-
 /* Note that the official physical address space bits is 62-M where M
    is implementation dependent.  I've not looked up M for the set of
    cpus we emulate at the system level.  */
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 4400718..81cc5b0 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -767,15 +767,11 @@ static inline bool vhost_needs_vring_endian(VirtIODevice *vdev)
     if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
         return false;
     }
-#ifdef TARGET_IS_BIENDIAN
 #ifdef HOST_WORDS_BIGENDIAN
     return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_LITTLE;
 #else
     return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG;
 #endif
-#else
-    return false;
-#endif
 }
 
 static int vhost_virtqueue_set_vring_endian_legacy(struct vhost_dev *dev,
-- 
MST