[PATCH v3 00/15] acpi: i386 tweaks

[PATCH v3 00/15] acpi: i386 tweaks

[Gerd Hoffmann]

First batch of microvm patches, some generic acpi stuff.
Split the acpi-build.c monster, specifically split the
pc and q35 and pci bits into a separate file which we
can skip building at some point in the future.

v2 changes: leave acpi-build.c largely as-is, move useful
bits to other places to allow them being reused, specifically:

 * move isa device generator functions to individual isa devices.
 * move fw_cfg generator function to fw_cfg.c

v3 changes: fix rtc, support multiple lpt devices.

take care,
  Gerd

Gerd Hoffmann (15):
  move 'typedef Aml' to qemu/types.h
  acpi: add aml builder stubs
  qtest: allow DSDT acpi table changes
  acpi: drop pointless _STA method
  acpi: add ISADeviceClass->build_aml()
  rtc: add RTC_ISA_BASE
  acpi: move aml builder code for rtc device
  acpi: serial: don't use _STA method
  acpi: move aml builder code for serial device
  acpi: parallel: don't use _STA method
  acpi: move aml builder code for parallel device
  acpi: move aml builder code for floppy device
  acpi: move aml builder code for i8042 (kbd+mouse) device
  acpi: factor out fw_cfg_add_acpi_dsdt()
  acpi: simplify build_isa_devices_aml()

 hw/i386/fw_cfg.h                            |   1 +
 include/hw/acpi/aml-build.h                 |   1 -
 include/hw/isa/isa.h                        |   2 +
 include/hw/rtc/mc146818rtc.h                |   1 +
 include/qemu/typedefs.h                     |   1 +
 tests/qtest/bios-tables-test-allowed-diff.h |  17 ++
 hw/acpi/aml-build-stub.c                    |  79 ++++++
 hw/block/fdc.c                              |  83 ++++++
 hw/char/parallel.c                          |  32 +++
 hw/char/serial-isa.c                        |  32 +++
 hw/i386/acpi-build.c                        | 271 +-------------------
 hw/i386/fw_cfg.c                            |  28 ++
 hw/input/pckbd.c                            |  31 +++
 hw/isa/isa-bus.c                            |  15 ++
 hw/rtc/mc146818rtc.c                        |  25 +-
 stubs/cmos.c                                |   7 +
 hw/acpi/Makefile.objs                       |   4 +-
 stubs/Makefile.objs                         |   1 +
 18 files changed, 361 insertions(+), 270 deletions(-)
 create mode 100644 hw/acpi/aml-build-stub.c
 create mode 100644 stubs/cmos.c

-- 
2.18.2

[PATCH v3 01/15] move 'typedef Aml' to qemu/types.h

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/acpi/aml-build.h | 1 -
 include/qemu/typedefs.h     | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 0f4ed53d7fbf..1539fe066714 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -33,7 +33,6 @@ struct Aml {
     uint8_t op;
     AmlBlockFlags block_flags;
 };
-typedef struct Aml Aml;
 
 typedef enum {
     AML_COMPATIBILITY = 0,
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 375770a80f06..ecf3cde26c3c 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -24,6 +24,7 @@
 typedef struct AdapterInfo AdapterInfo;
 typedef struct AddressSpace AddressSpace;
 typedef struct AioContext AioContext;
+typedef struct Aml Aml;
 typedef struct AnnounceTimer AnnounceTimer;
 typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
 typedef struct BdrvDirtyBitmapIter BdrvDirtyBitmapIter;
-- 
2.18.2

[PATCH v3 02/15] acpi: add aml builder stubs

[Gerd Hoffmann]

Needed when moving aml builder code to devices.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/acpi/aml-build-stub.c | 79 ++++++++++++++++++++++++++++++++++++++++
 hw/acpi/Makefile.objs    |  4 +-
 2 files changed, 81 insertions(+), 2 deletions(-)
 create mode 100644 hw/acpi/aml-build-stub.c

diff --git a/hw/acpi/aml-build-stub.c b/hw/acpi/aml-build-stub.c
new file mode 100644
index 000000000000..58b2e162277f
--- /dev/null
+++ b/hw/acpi/aml-build-stub.c
@@ -0,0 +1,79 @@
+/*
+ * ACPI aml builder stubs for platforms that don't support ACPI.
+ *
+ * Copyright (c) 2006 Fabrice Bellard
+ * Copyright (c) 2016 Red Hat, Inc.
+ *
+ * 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/acpi.h"
+#include "hw/acpi/aml-build.h"
+
+void aml_append(Aml *parent_ctx, Aml *child)
+{
+}
+
+Aml *aml_resource_template(void)
+{
+    return NULL;
+}
+
+Aml *aml_device(const char *name_format, ...)
+{
+    return NULL;
+}
+
+Aml *aml_eisaid(const char *str)
+{
+    return NULL;
+}
+
+Aml *aml_name_decl(const char *name, Aml *val)
+{
+    return NULL;
+}
+
+Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
+            uint8_t aln, uint8_t len)
+{
+    return NULL;
+}
+
+Aml *aml_irq_no_flags(uint8_t irq)
+{
+    return NULL;
+}
+
+Aml *aml_int(const uint64_t val)
+{
+    return NULL;
+}
+
+Aml *aml_package(uint8_t num_elements)
+{
+    return NULL;
+}
+
+Aml *aml_dma(AmlDmaType typ, AmlDmaBusMaster bm, AmlTransferSize sz,
+             uint8_t channel)
+{
+    return NULL;
+}
+
+Aml *aml_buffer(int buffer_size, uint8_t *byte_list)
+{
+    return NULL;
+}
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index 777da07f4d70..cab9bcd457dc 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -20,6 +20,6 @@ common-obj-$(CONFIG_TPM) += tpm.o
 common-obj-$(CONFIG_IPMI) += ipmi.o
 common-obj-$(call lnot,$(CONFIG_IPMI)) += ipmi-stub.o
 else
-common-obj-y += acpi-stub.o
+common-obj-y += acpi-stub.o aml-build-stub.o
 endif
-common-obj-$(CONFIG_ALL) += acpi-stub.o acpi-x86-stub.o ipmi-stub.o
+common-obj-$(CONFIG_ALL) += acpi-stub.o aml-build-stub.o acpi-x86-stub.o ipmi-stub.o
-- 
2.18.2

[PATCH v3 03/15] qtest: allow DSDT acpi table changes

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8bf4..6a052c50447a 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,18 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/pc/DSDT",
+"tests/data/acpi/pc/DSDT.acpihmat",
+"tests/data/acpi/pc/DSDT.bridge",
+"tests/data/acpi/pc/DSDT.cphp",
+"tests/data/acpi/pc/DSDT.dimmpxm",
+"tests/data/acpi/pc/DSDT.ipmikcs",
+"tests/data/acpi/pc/DSDT.memhp",
+"tests/data/acpi/pc/DSDT.numamem",
+"tests/data/acpi/q35/DSDT",
+"tests/data/acpi/q35/DSDT.acpihmat",
+"tests/data/acpi/q35/DSDT.bridge",
+"tests/data/acpi/q35/DSDT.cphp",
+"tests/data/acpi/q35/DSDT.dimmpxm",
+"tests/data/acpi/q35/DSDT.ipmibt",
+"tests/data/acpi/q35/DSDT.memhp",
+"tests/data/acpi/q35/DSDT.mmio64",
+"tests/data/acpi/q35/DSDT.numamem",
-- 
2.18.2

[PATCH v3 04/15] acpi: drop pointless _STA method

[Gerd Hoffmann]

When returning a constant there is no point in having a method
in the first place, _STA can be a simple integer instead.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/acpi-build.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 23c77eeb95a9..3a046b03e4cd 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1151,14 +1151,11 @@ static Aml *build_kbd_device_aml(void)
 {
     Aml *dev;
     Aml *crs;
-    Aml *method;
 
     dev = aml_device("KBD");
     aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0303")));
 
-    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
-    aml_append(method, aml_return(aml_int(0x0f)));
-    aml_append(dev, method);
+    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
 
     crs = aml_resource_template();
     aml_append(crs, aml_io(AML_DECODE16, 0x0060, 0x0060, 0x01, 0x01));
@@ -1173,14 +1170,11 @@ static Aml *build_mouse_device_aml(void)
 {
     Aml *dev;
     Aml *crs;
-    Aml *method;
 
     dev = aml_device("MOU");
     aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0F13")));
 
-    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
-    aml_append(method, aml_return(aml_int(0x0f)));
-    aml_append(dev, method);
+    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
 
     crs = aml_resource_template();
     aml_append(crs, aml_irq_no_flags(12));
@@ -2238,9 +2232,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
                                            TPM_CRB_ADDR_SIZE, AML_READ_WRITE));
         aml_append(dev, aml_name_decl("_CRS", crs));
 
-        method = aml_method("_STA", 0, AML_NOTSERIALIZED);
-        aml_append(method, aml_return(aml_int(0x0f)));
-        aml_append(dev, method);
+        aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
 
         tpm_build_ppi_acpi(tpm, dev);
 
-- 
2.18.2

[PATCH v3 05/15] acpi: add ISADeviceClass->build_aml()

[Gerd Hoffmann]

Also add isa_aml_build() function which walks all isa devices.
This allows to move aml builder code to isa devices.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/isa/isa.h |  2 ++
 hw/i386/acpi-build.c |  1 +
 hw/isa/isa-bus.c     | 15 +++++++++++++++
 3 files changed, 18 insertions(+)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 59a4d4b50a6d..02c235027484 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -69,6 +69,7 @@ typedef struct IsaDmaClass {
 
 typedef struct ISADeviceClass {
     DeviceClass parent_class;
+    void (*build_aml)(ISADevice *dev, Aml *scope);
 } ISADeviceClass;
 
 struct ISABus {
@@ -107,6 +108,7 @@ ISADevice *isa_try_create(ISABus *bus, const char *name);
 ISADevice *isa_create_simple(ISABus *bus, const char *name);
 
 ISADevice *isa_vga_init(ISABus *bus);
+void isa_build_aml(ISABus *bus, Aml *scope);
 
 /**
  * isa_register_ioport: Install an I/O port region on the ISA bus.
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 3a046b03e4cd..97f3c75cd995 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1288,6 +1288,7 @@ static void build_isa_devices_aml(Aml *table)
         error_report("No ISA bus, unable to define IPMI ACPI data");
     } else {
         build_acpi_ipmi_devices(scope, BUS(obj), "\\_SB.PCI0.ISA");
+        isa_build_aml(ISA_BUS(obj), scope);
     }
 
     aml_append(table, scope);
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 798dd9194e8f..1f2189f4d5db 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -207,6 +207,21 @@ ISADevice *isa_vga_init(ISABus *bus)
     }
 }
 
+void isa_build_aml(ISABus *bus, Aml *scope)
+{
+    BusChild *kid;
+    ISADevice *dev;
+    ISADeviceClass *dc;
+
+    QTAILQ_FOREACH(kid, &bus->parent_obj.children, sibling) {
+        dev = ISA_DEVICE(kid->child);
+        dc = ISA_DEVICE_GET_CLASS(dev);
+        if (dc->build_aml) {
+            dc->build_aml(dev, scope);
+        }
+    }
+}
+
 static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
 {
     ISADevice *d = ISA_DEVICE(dev);
-- 
2.18.2

[PATCH v3 06/15] rtc: add RTC_ISA_BASE

[Gerd Hoffmann]

Add and use RTC_ISA_BASE define instead of hardcoding 0x70.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/rtc/mc146818rtc.h | 1 +
 hw/rtc/mc146818rtc.c         | 5 ++---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h
index 10c93a096a1d..3713181b56fe 100644
--- a/include/hw/rtc/mc146818rtc.h
+++ b/include/hw/rtc/mc146818rtc.h
@@ -47,6 +47,7 @@ typedef struct RTCState {
 } RTCState;
 
 #define RTC_ISA_IRQ 8
+#define RTC_ISA_BASE 0x70
 
 ISADevice *mc146818_rtc_init(ISABus *bus, int base_year,
                              qemu_irq intercept_irq);
diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index dc4269cc55cb..d18c09911be2 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -908,7 +908,6 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
 {
     ISADevice *isadev = ISA_DEVICE(dev);
     RTCState *s = MC146818_RTC(dev);
-    int base = 0x70;
 
     s->cmos_data[RTC_REG_A] = 0x26;
     s->cmos_data[RTC_REG_B] = 0x02;
@@ -951,7 +950,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
     qemu_register_suspend_notifier(&s->suspend_notifier);
 
     memory_region_init_io(&s->io, OBJECT(s), &cmos_ops, s, "rtc", 2);
-    isa_register_ioport(isadev, &s->io, base);
+    isa_register_ioport(isadev, &s->io, RTC_ISA_BASE);
 
     /* register rtc 0x70 port for coalesced_pio */
     memory_region_set_flush_coalesced(&s->io);
@@ -960,7 +959,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
     memory_region_add_subregion(&s->io, 0, &s->coalesced_io);
     memory_region_add_coalescing(&s->coalesced_io, 0, 1);
 
-    qdev_set_legacy_instance_id(dev, base, 3);
+    qdev_set_legacy_instance_id(dev, RTC_ISA_BASE, 3);
     qemu_register_reset(rtc_reset, s);
 
     object_property_add_tm(OBJECT(s), "date", rtc_get_date, NULL);
-- 
2.18.2

[PATCH v3 07/15] acpi: move aml builder code for rtc device

[Gerd Hoffmann]

Also use a single io range instead of two,
following what real hardware does.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/acpi-build.c | 17 -----------------
 hw/rtc/mc146818rtc.c | 20 ++++++++++++++++++++
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 97f3c75cd995..f79e5eaf170d 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1131,22 +1131,6 @@ static Aml *build_fdc_device_aml(ISADevice *fdc)
     return dev;
 }
 
-static Aml *build_rtc_device_aml(void)
-{
-    Aml *dev;
-    Aml *crs;
-
-    dev = aml_device("RTC");
-    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0B00")));
-    crs = aml_resource_template();
-    aml_append(crs, aml_io(AML_DECODE16, 0x0070, 0x0070, 0x10, 0x02));
-    aml_append(crs, aml_irq_no_flags(8));
-    aml_append(crs, aml_io(AML_DECODE16, 0x0072, 0x0072, 0x02, 0x06));
-    aml_append(dev, aml_name_decl("_CRS", crs));
-
-    return dev;
-}
-
 static Aml *build_kbd_device_aml(void)
 {
     Aml *dev;
@@ -1272,7 +1256,6 @@ static void build_isa_devices_aml(Aml *table)
     Aml *scope = aml_scope("_SB.PCI0.ISA");
     Object *obj = object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous);
 
-    aml_append(scope, build_rtc_device_aml());
     aml_append(scope, build_kbd_device_aml());
     aml_append(scope, build_mouse_device_aml());
     if (fdc) {
diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index d18c09911be2..47fafcfb7c1d 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -27,6 +27,7 @@
 #include "qemu/cutils.h"
 #include "qemu/module.h"
 #include "qemu/bcd.h"
+#include "hw/acpi/aml-build.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
 #include "qemu/timer.h"
@@ -1007,13 +1008,32 @@ static void rtc_resetdev(DeviceState *d)
     }
 }
 
+static void rtc_build_aml(ISADevice *isadev, Aml *scope)
+{
+    Aml *dev;
+    Aml *crs;
+
+    crs = aml_resource_template();
+    aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE, RTC_ISA_BASE,
+                           0x10, 0x08));
+    aml_append(crs, aml_irq_no_flags(RTC_ISA_IRQ));
+
+    dev = aml_device("RTC");
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0B00")));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+
+    aml_append(scope, dev);
+}
+
 static void rtc_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    ISADeviceClass *isa = ISA_DEVICE_CLASS(klass);
 
     dc->realize = rtc_realizefn;
     dc->reset = rtc_resetdev;
     dc->vmsd = &vmstate_rtc;
+    isa->build_aml = rtc_build_aml;
     device_class_set_props(dc, mc146818rtc_properties);
 }
 
-- 
2.18.2

[PATCH v3 08/15] acpi: serial: don't use _STA method

[Gerd Hoffmann]

The _STA method dates back to the days where we had a static DSDT.  The
device is listed in the DSDT table unconditionally and the _STA method
checks a bit in the isa bridge pci config space to figure whenever a
given is isa device is present or not, then evaluates to 0x0f (present)
or 0x00 (absent).

These days the DSDT is generated by qemu anyway, so if a device is not
present we can simply drop it from the DSDT instead.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/acpi-build.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index f79e5eaf170d..a99e5bbd1156 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1202,50 +1202,34 @@ static Aml *build_lpt_device_aml(void)
     return dev;
 }
 
-static Aml *build_com_device_aml(uint8_t uid)
+static void build_com_device_aml(Aml *scope, uint8_t uid)
 {
     Aml *dev;
     Aml *crs;
-    Aml *method;
-    Aml *if_ctx;
-    Aml *else_ctx;
-    Aml *zero = aml_int(0);
-    Aml *is_present = aml_local(0);
-    const char *enabled_field = "CAEN";
     uint8_t irq = 4;
     uint16_t io_port = 0x03F8;
 
     assert(uid == 1 || uid == 2);
     if (uid == 2) {
-        enabled_field = "CBEN";
         irq = 3;
         io_port = 0x02F8;
     }
+    if (!memory_region_present(get_system_io(), io_port)) {
+        return;
+    }
 
     dev = aml_device("COM%d", uid);
     aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0501")));
     aml_append(dev, aml_name_decl("_UID", aml_int(uid)));
 
-    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
-    aml_append(method, aml_store(aml_name("%s", enabled_field), is_present));
-    if_ctx = aml_if(aml_equal(is_present, zero));
-    {
-        aml_append(if_ctx, aml_return(aml_int(0x00)));
-    }
-    aml_append(method, if_ctx);
-    else_ctx = aml_else();
-    {
-        aml_append(else_ctx, aml_return(aml_int(0x0f)));
-    }
-    aml_append(method, else_ctx);
-    aml_append(dev, method);
+    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
 
     crs = aml_resource_template();
     aml_append(crs, aml_io(AML_DECODE16, io_port, io_port, 0x00, 0x08));
     aml_append(crs, aml_irq_no_flags(irq));
     aml_append(dev, aml_name_decl("_CRS", crs));
 
-    return dev;
+    aml_append(scope, dev);
 }
 
 static void build_isa_devices_aml(Aml *table)
@@ -1262,8 +1246,8 @@ static void build_isa_devices_aml(Aml *table)
         aml_append(scope, build_fdc_device_aml(fdc));
     }
     aml_append(scope, build_lpt_device_aml());
-    aml_append(scope, build_com_device_aml(1));
-    aml_append(scope, build_com_device_aml(2));
+    build_com_device_aml(scope, 1);
+    build_com_device_aml(scope, 2);
 
     if (ambiguous) {
         error_report("Multiple ISA busses, unable to define IPMI ACPI data");
-- 
2.18.2

[PATCH v3 09/15] acpi: move aml builder code for serial device

[Gerd Hoffmann]

The code uses the isa_serial_io array to figure what the device uid is.
Side effect is that acpi antries are not limited to port 1+2 any more,
we'll also get entries for ports 3+4.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/char/serial-isa.c | 32 ++++++++++++++++++++++++++++++++
 hw/i386/acpi-build.c | 32 --------------------------------
 2 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
index f9b6eed7833d..f7c19a398ced 100644
--- a/hw/char/serial-isa.c
+++ b/hw/char/serial-isa.c
@@ -27,6 +27,7 @@
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "sysemu/sysemu.h"
+#include "hw/acpi/aml-build.h"
 #include "hw/char/serial.h"
 #include "hw/isa/isa.h"
 #include "hw/qdev-properties.h"
@@ -81,6 +82,35 @@ static void serial_isa_realizefn(DeviceState *dev, Error **errp)
     isa_register_ioport(isadev, &s->io, isa->iobase);
 }
 
+static void serial_isa_build_aml(ISADevice *isadev, Aml *scope)
+{
+    ISASerialState *isa = ISA_SERIAL(isadev);
+    int i, uid = 0;
+    Aml *dev;
+    Aml *crs;
+
+    for (i = 0; i < ARRAY_SIZE(isa_serial_io); i++) {
+        if (isa->iobase == isa_serial_io[i]) {
+            uid = i + 1;
+        }
+    }
+    if (!uid) {
+        return;
+    }
+
+    crs = aml_resource_template();
+    aml_append(crs, aml_io(AML_DECODE16, isa->iobase, isa->iobase, 0x00, 0x08));
+    aml_append(crs, aml_irq_no_flags(isa->isairq));
+
+    dev = aml_device("COM%d", uid);
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0501")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(uid)));
+    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+
+    aml_append(scope, dev);
+}
+
 static const VMStateDescription vmstate_isa_serial = {
     .name = "serial",
     .version_id = 3,
@@ -103,9 +133,11 @@ static Property serial_isa_properties[] = {
 static void serial_isa_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    ISADeviceClass *isa = ISA_DEVICE_CLASS(klass);
 
     dc->realize = serial_isa_realizefn;
     dc->vmsd = &vmstate_isa_serial;
+    isa->build_aml = serial_isa_build_aml;
     device_class_set_props(dc, serial_isa_properties);
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a99e5bbd1156..fea83352e6ab 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1202,36 +1202,6 @@ static Aml *build_lpt_device_aml(void)
     return dev;
 }
 
-static void build_com_device_aml(Aml *scope, uint8_t uid)
-{
-    Aml *dev;
-    Aml *crs;
-    uint8_t irq = 4;
-    uint16_t io_port = 0x03F8;
-
-    assert(uid == 1 || uid == 2);
-    if (uid == 2) {
-        irq = 3;
-        io_port = 0x02F8;
-    }
-    if (!memory_region_present(get_system_io(), io_port)) {
-        return;
-    }
-
-    dev = aml_device("COM%d", uid);
-    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0501")));
-    aml_append(dev, aml_name_decl("_UID", aml_int(uid)));
-
-    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
-
-    crs = aml_resource_template();
-    aml_append(crs, aml_io(AML_DECODE16, io_port, io_port, 0x00, 0x08));
-    aml_append(crs, aml_irq_no_flags(irq));
-    aml_append(dev, aml_name_decl("_CRS", crs));
-
-    aml_append(scope, dev);
-}
-
 static void build_isa_devices_aml(Aml *table)
 {
     ISADevice *fdc = pc_find_fdc0();
@@ -1246,8 +1216,6 @@ static void build_isa_devices_aml(Aml *table)
         aml_append(scope, build_fdc_device_aml(fdc));
     }
     aml_append(scope, build_lpt_device_aml());
-    build_com_device_aml(scope, 1);
-    build_com_device_aml(scope, 2);
 
     if (ambiguous) {
         error_report("Multiple ISA busses, unable to define IPMI ACPI data");
-- 
2.18.2

[PATCH v3 10/15] acpi: parallel: don't use _STA method

[Gerd Hoffmann]

The _STA method dates back to the days where we had a static DSDT.  The
device is listed in the DSDT table unconditionally and the _STA method
checks a bit in the isa bridge pci config space to figure whenever a
given is isa device is present or not, then evaluates to 0x0f (present)
or 0x00 (absent).

These days the DSDT is generated by qemu anyway, so if a device is not
present we can simply drop it from the DSDT instead.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/acpi-build.c | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index fea83352e6ab..e01afbd011d9 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1167,39 +1167,26 @@ static Aml *build_mouse_device_aml(void)
     return dev;
 }
 
-static Aml *build_lpt_device_aml(void)
+static void build_lpt_device_aml(Aml *scope)
 {
     Aml *dev;
     Aml *crs;
-    Aml *method;
-    Aml *if_ctx;
-    Aml *else_ctx;
-    Aml *zero = aml_int(0);
-    Aml *is_present = aml_local(0);
+
+    if (!memory_region_present(get_system_io(), 0x0378)) {
+        return;
+    }
 
     dev = aml_device("LPT");
     aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
 
-    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
-    aml_append(method, aml_store(aml_name("LPEN"), is_present));
-    if_ctx = aml_if(aml_equal(is_present, zero));
-    {
-        aml_append(if_ctx, aml_return(aml_int(0x00)));
-    }
-    aml_append(method, if_ctx);
-    else_ctx = aml_else();
-    {
-        aml_append(else_ctx, aml_return(aml_int(0x0f)));
-    }
-    aml_append(method, else_ctx);
-    aml_append(dev, method);
+    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
 
     crs = aml_resource_template();
     aml_append(crs, aml_io(AML_DECODE16, 0x0378, 0x0378, 0x08, 0x08));
     aml_append(crs, aml_irq_no_flags(7));
     aml_append(dev, aml_name_decl("_CRS", crs));
 
-    return dev;
+    aml_append(scope, dev);
 }
 
 static void build_isa_devices_aml(Aml *table)
@@ -1215,7 +1202,7 @@ static void build_isa_devices_aml(Aml *table)
     if (fdc) {
         aml_append(scope, build_fdc_device_aml(fdc));
     }
-    aml_append(scope, build_lpt_device_aml());
+    build_lpt_device_aml(scope);
 
     if (ambiguous) {
         error_report("Multiple ISA busses, unable to define IPMI ACPI data");
-- 
2.18.2

[PATCH v3 11/15] acpi: move aml builder code for parallel device

[Gerd Hoffmann]

Also adds support for multiple LPT devices.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/char/parallel.c   | 32 ++++++++++++++++++++++++++++++++
 hw/i386/acpi-build.c | 23 -----------------------
 2 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index 8dd67d13759b..bc6b55b3b910 100644
--- a/hw/char/parallel.c
+++ b/hw/char/parallel.c
@@ -28,6 +28,7 @@
 #include "qemu/module.h"
 #include "chardev/char-parallel.h"
 #include "chardev/char-fe.h"
+#include "hw/acpi/aml-build.h"
 #include "hw/irq.h"
 #include "hw/isa/isa.h"
 #include "hw/qdev-properties.h"
@@ -568,6 +569,35 @@ static void parallel_isa_realizefn(DeviceState *dev, Error **errp)
                              s, "parallel");
 }
 
+static void parallel_isa_build_aml(ISADevice *isadev, Aml *scope)
+{
+    ISAParallelState *isa = ISA_PARALLEL(isadev);
+    int i, uid = 0;
+    Aml *dev;
+    Aml *crs;
+
+    for (i = 0; i < ARRAY_SIZE(isa_parallel_io); i++) {
+        if (isa->iobase == isa_parallel_io[i]) {
+            uid = i + 1;
+        }
+    }
+    if (!uid) {
+        return;
+    }
+
+    crs = aml_resource_template();
+    aml_append(crs, aml_io(AML_DECODE16, isa->iobase, isa->iobase, 0x08, 0x08));
+    aml_append(crs, aml_irq_no_flags(isa->isairq));
+
+    dev = aml_device("LPT%d", uid);
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(uid)));
+    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+
+    aml_append(scope, dev);
+}
+
 /* Memory mapped interface */
 static uint64_t parallel_mm_readfn(void *opaque, hwaddr addr, unsigned size)
 {
@@ -624,9 +654,11 @@ static Property parallel_isa_properties[] = {
 static void parallel_isa_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    ISADeviceClass *isa = ISA_DEVICE_CLASS(klass);
 
     dc->realize = parallel_isa_realizefn;
     dc->vmsd = &vmstate_parallel_isa;
+    isa->build_aml = parallel_isa_build_aml;
     device_class_set_props(dc, parallel_isa_properties);
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index e01afbd011d9..12a017e1f45b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1167,28 +1167,6 @@ static Aml *build_mouse_device_aml(void)
     return dev;
 }
 
-static void build_lpt_device_aml(Aml *scope)
-{
-    Aml *dev;
-    Aml *crs;
-
-    if (!memory_region_present(get_system_io(), 0x0378)) {
-        return;
-    }
-
-    dev = aml_device("LPT");
-    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
-
-    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
-
-    crs = aml_resource_template();
-    aml_append(crs, aml_io(AML_DECODE16, 0x0378, 0x0378, 0x08, 0x08));
-    aml_append(crs, aml_irq_no_flags(7));
-    aml_append(dev, aml_name_decl("_CRS", crs));
-
-    aml_append(scope, dev);
-}
-
 static void build_isa_devices_aml(Aml *table)
 {
     ISADevice *fdc = pc_find_fdc0();
@@ -1202,7 +1180,6 @@ static void build_isa_devices_aml(Aml *table)
     if (fdc) {
         aml_append(scope, build_fdc_device_aml(fdc));
     }
-    build_lpt_device_aml(scope);
 
     if (ambiguous) {
         error_report("Multiple ISA busses, unable to define IPMI ACPI data");
-- 
2.18.2

[PATCH v3 12/15] acpi: move aml builder code for floppy device

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/block/fdc.c       | 83 ++++++++++++++++++++++++++++++++++++++++++++
 hw/i386/acpi-build.c | 83 --------------------------------------------
 stubs/cmos.c         |  7 ++++
 stubs/Makefile.objs  |  1 +
 4 files changed, 91 insertions(+), 83 deletions(-)
 create mode 100644 stubs/cmos.c

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 33bc9e2f9249..56ddc26c7065 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -32,6 +32,8 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
+#include "hw/i386/pc.h"
+#include "hw/acpi/aml-build.h"
 #include "hw/irq.h"
 #include "hw/isa/isa.h"
 #include "hw/qdev-properties.h"
@@ -2764,6 +2766,85 @@ void isa_fdc_get_drive_max_chs(FloppyDriveType type,
     (*maxc)--;
 }
 
+static Aml *build_fdinfo_aml(int idx, FloppyDriveType type)
+{
+    Aml *dev, *fdi;
+    uint8_t maxc, maxh, maxs;
+
+    isa_fdc_get_drive_max_chs(type, &maxc, &maxh, &maxs);
+
+    dev = aml_device("FLP%c", 'A' + idx);
+
+    aml_append(dev, aml_name_decl("_ADR", aml_int(idx)));
+
+    fdi = aml_package(16);
+    aml_append(fdi, aml_int(idx));  /* Drive Number */
+    aml_append(fdi,
+        aml_int(cmos_get_fd_drive_type(type)));  /* Device Type */
+    /*
+     * the values below are the limits of the drive, and are thus independent
+     * of the inserted media
+     */
+    aml_append(fdi, aml_int(maxc));  /* Maximum Cylinder Number */
+    aml_append(fdi, aml_int(maxs));  /* Maximum Sector Number */
+    aml_append(fdi, aml_int(maxh));  /* Maximum Head Number */
+    /*
+     * SeaBIOS returns the below values for int 0x13 func 0x08 regardless of
+     * the drive type, so shall we
+     */
+    aml_append(fdi, aml_int(0xAF));  /* disk_specify_1 */
+    aml_append(fdi, aml_int(0x02));  /* disk_specify_2 */
+    aml_append(fdi, aml_int(0x25));  /* disk_motor_wait */
+    aml_append(fdi, aml_int(0x02));  /* disk_sector_siz */
+    aml_append(fdi, aml_int(0x12));  /* disk_eot */
+    aml_append(fdi, aml_int(0x1B));  /* disk_rw_gap */
+    aml_append(fdi, aml_int(0xFF));  /* disk_dtl */
+    aml_append(fdi, aml_int(0x6C));  /* disk_formt_gap */
+    aml_append(fdi, aml_int(0xF6));  /* disk_fill */
+    aml_append(fdi, aml_int(0x0F));  /* disk_head_sttl */
+    aml_append(fdi, aml_int(0x08));  /* disk_motor_strt */
+
+    aml_append(dev, aml_name_decl("_FDI", fdi));
+    return dev;
+}
+
+static void fdc_isa_build_aml(ISADevice *isadev, Aml *scope)
+{
+    Aml *dev;
+    Aml *crs;
+    int i;
+
+#define ACPI_FDE_MAX_FD 4
+    uint32_t fde_buf[5] = {
+        0, 0, 0, 0,     /* presence of floppy drives #0 - #3 */
+        cpu_to_le32(2)  /* tape presence (2 == never present) */
+    };
+
+    crs = aml_resource_template();
+    aml_append(crs, aml_io(AML_DECODE16, 0x03F2, 0x03F2, 0x00, 0x04));
+    aml_append(crs, aml_io(AML_DECODE16, 0x03F7, 0x03F7, 0x00, 0x01));
+    aml_append(crs, aml_irq_no_flags(6));
+    aml_append(crs,
+        aml_dma(AML_COMPATIBILITY, AML_NOTBUSMASTER, AML_TRANSFER8, 2));
+
+    dev = aml_device("FDC0");
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0700")));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+
+    for (i = 0; i < MIN(MAX_FD, ACPI_FDE_MAX_FD); i++) {
+        FloppyDriveType type = isa_fdc_get_drive_type(isadev, i);
+
+        if (type < FLOPPY_DRIVE_TYPE_NONE) {
+            fde_buf[i] = cpu_to_le32(1);  /* drive present */
+            aml_append(dev, build_fdinfo_aml(i, type));
+        }
+    }
+    aml_append(dev, aml_name_decl("_FDE",
+               aml_buffer(sizeof(fde_buf), (uint8_t *)fde_buf)));
+
+    aml_append(scope, dev);
+}
+
 static const VMStateDescription vmstate_isa_fdc ={
     .name = "fdc",
     .version_id = 2,
@@ -2797,11 +2878,13 @@ static Property isa_fdc_properties[] = {
 static void isabus_fdc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    ISADeviceClass *isa = ISA_DEVICE_CLASS(klass);
 
     dc->realize = isabus_fdc_realize;
     dc->fw_name = "fdc";
     dc->reset = fdctrl_external_reset_isa;
     dc->vmsd = &vmstate_isa_fdc;
+    isa->build_aml = fdc_isa_build_aml;
     device_class_set_props(dc, isa_fdc_properties);
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 12a017e1f45b..7415bd5fdce0 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1052,85 +1052,6 @@ static void build_hpet_aml(Aml *table)
     aml_append(table, scope);
 }
 
-static Aml *build_fdinfo_aml(int idx, FloppyDriveType type)
-{
-    Aml *dev, *fdi;
-    uint8_t maxc, maxh, maxs;
-
-    isa_fdc_get_drive_max_chs(type, &maxc, &maxh, &maxs);
-
-    dev = aml_device("FLP%c", 'A' + idx);
-
-    aml_append(dev, aml_name_decl("_ADR", aml_int(idx)));
-
-    fdi = aml_package(16);
-    aml_append(fdi, aml_int(idx));  /* Drive Number */
-    aml_append(fdi,
-        aml_int(cmos_get_fd_drive_type(type)));  /* Device Type */
-    /*
-     * the values below are the limits of the drive, and are thus independent
-     * of the inserted media
-     */
-    aml_append(fdi, aml_int(maxc));  /* Maximum Cylinder Number */
-    aml_append(fdi, aml_int(maxs));  /* Maximum Sector Number */
-    aml_append(fdi, aml_int(maxh));  /* Maximum Head Number */
-    /*
-     * SeaBIOS returns the below values for int 0x13 func 0x08 regardless of
-     * the drive type, so shall we
-     */
-    aml_append(fdi, aml_int(0xAF));  /* disk_specify_1 */
-    aml_append(fdi, aml_int(0x02));  /* disk_specify_2 */
-    aml_append(fdi, aml_int(0x25));  /* disk_motor_wait */
-    aml_append(fdi, aml_int(0x02));  /* disk_sector_siz */
-    aml_append(fdi, aml_int(0x12));  /* disk_eot */
-    aml_append(fdi, aml_int(0x1B));  /* disk_rw_gap */
-    aml_append(fdi, aml_int(0xFF));  /* disk_dtl */
-    aml_append(fdi, aml_int(0x6C));  /* disk_formt_gap */
-    aml_append(fdi, aml_int(0xF6));  /* disk_fill */
-    aml_append(fdi, aml_int(0x0F));  /* disk_head_sttl */
-    aml_append(fdi, aml_int(0x08));  /* disk_motor_strt */
-
-    aml_append(dev, aml_name_decl("_FDI", fdi));
-    return dev;
-}
-
-static Aml *build_fdc_device_aml(ISADevice *fdc)
-{
-    int i;
-    Aml *dev;
-    Aml *crs;
-
-#define ACPI_FDE_MAX_FD 4
-    uint32_t fde_buf[5] = {
-        0, 0, 0, 0,     /* presence of floppy drives #0 - #3 */
-        cpu_to_le32(2)  /* tape presence (2 == never present) */
-    };
-
-    dev = aml_device("FDC0");
-    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0700")));
-
-    crs = aml_resource_template();
-    aml_append(crs, aml_io(AML_DECODE16, 0x03F2, 0x03F2, 0x00, 0x04));
-    aml_append(crs, aml_io(AML_DECODE16, 0x03F7, 0x03F7, 0x00, 0x01));
-    aml_append(crs, aml_irq_no_flags(6));
-    aml_append(crs,
-        aml_dma(AML_COMPATIBILITY, AML_NOTBUSMASTER, AML_TRANSFER8, 2));
-    aml_append(dev, aml_name_decl("_CRS", crs));
-
-    for (i = 0; i < MIN(MAX_FD, ACPI_FDE_MAX_FD); i++) {
-        FloppyDriveType type = isa_fdc_get_drive_type(fdc, i);
-
-        if (type < FLOPPY_DRIVE_TYPE_NONE) {
-            fde_buf[i] = cpu_to_le32(1);  /* drive present */
-            aml_append(dev, build_fdinfo_aml(i, type));
-        }
-    }
-    aml_append(dev, aml_name_decl("_FDE",
-               aml_buffer(sizeof(fde_buf), (uint8_t *)fde_buf)));
-
-    return dev;
-}
-
 static Aml *build_kbd_device_aml(void)
 {
     Aml *dev;
@@ -1169,7 +1090,6 @@ static Aml *build_mouse_device_aml(void)
 
 static void build_isa_devices_aml(Aml *table)
 {
-    ISADevice *fdc = pc_find_fdc0();
     bool ambiguous;
 
     Aml *scope = aml_scope("_SB.PCI0.ISA");
@@ -1177,9 +1097,6 @@ static void build_isa_devices_aml(Aml *table)
 
     aml_append(scope, build_kbd_device_aml());
     aml_append(scope, build_mouse_device_aml());
-    if (fdc) {
-        aml_append(scope, build_fdc_device_aml(fdc));
-    }
 
     if (ambiguous) {
         error_report("Multiple ISA busses, unable to define IPMI ACPI data");
diff --git a/stubs/cmos.c b/stubs/cmos.c
new file mode 100644
index 000000000000..416cbe4055ff
--- /dev/null
+++ b/stubs/cmos.c
@@ -0,0 +1,7 @@
+#include "qemu/osdep.h"
+#include "hw/i386/pc.h"
+
+int cmos_get_fd_drive_type(FloppyDriveType fd0)
+{
+    return 0;
+}
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 45be5dc0ed78..3cbe472d1c6c 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -3,6 +3,7 @@ stub-obj-y += bdrv-next-monitor-owned.o
 stub-obj-y += blk-commit-all.o
 stub-obj-y += blockdev-close-all-bdrv-states.o
 stub-obj-y += clock-warp.o
+stub-obj-y += cmos.o
 stub-obj-y += cpu-get-clock.o
 stub-obj-y += cpu-get-icount.o
 stub-obj-y += dump.o
-- 
2.18.2

[PATCH v3 13/15] acpi: move aml builder code for i8042 (kbd+mouse) device

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/acpi-build.c | 39 ---------------------------------------
 hw/input/pckbd.c     | 31 +++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 39 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 7415bd5fdce0..643e875c05a5 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1052,42 +1052,6 @@ static void build_hpet_aml(Aml *table)
     aml_append(table, scope);
 }
 
-static Aml *build_kbd_device_aml(void)
-{
-    Aml *dev;
-    Aml *crs;
-
-    dev = aml_device("KBD");
-    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0303")));
-
-    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
-
-    crs = aml_resource_template();
-    aml_append(crs, aml_io(AML_DECODE16, 0x0060, 0x0060, 0x01, 0x01));
-    aml_append(crs, aml_io(AML_DECODE16, 0x0064, 0x0064, 0x01, 0x01));
-    aml_append(crs, aml_irq_no_flags(1));
-    aml_append(dev, aml_name_decl("_CRS", crs));
-
-    return dev;
-}
-
-static Aml *build_mouse_device_aml(void)
-{
-    Aml *dev;
-    Aml *crs;
-
-    dev = aml_device("MOU");
-    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0F13")));
-
-    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
-
-    crs = aml_resource_template();
-    aml_append(crs, aml_irq_no_flags(12));
-    aml_append(dev, aml_name_decl("_CRS", crs));
-
-    return dev;
-}
-
 static void build_isa_devices_aml(Aml *table)
 {
     bool ambiguous;
@@ -1095,9 +1059,6 @@ static void build_isa_devices_aml(Aml *table)
     Aml *scope = aml_scope("_SB.PCI0.ISA");
     Object *obj = object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous);
 
-    aml_append(scope, build_kbd_device_aml());
-    aml_append(scope, build_mouse_device_aml());
-
     if (ambiguous) {
         error_report("Multiple ISA busses, unable to define IPMI ACPI data");
     } else if (!obj) {
diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index 60a41303203a..29d633ca9478 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -26,6 +26,7 @@
 #include "qemu/log.h"
 #include "hw/isa/isa.h"
 #include "migration/vmstate.h"
+#include "hw/acpi/aml-build.h"
 #include "hw/input/ps2.h"
 #include "hw/irq.h"
 #include "hw/input/i8042.h"
@@ -561,12 +562,42 @@ static void i8042_realizefn(DeviceState *dev, Error **errp)
     qemu_register_reset(kbd_reset, s);
 }
 
+static void i8042_build_aml(ISADevice *isadev, Aml *scope)
+{
+    Aml *kbd;
+    Aml *mou;
+    Aml *crs;
+
+    crs = aml_resource_template();
+    aml_append(crs, aml_io(AML_DECODE16, 0x0060, 0x0060, 0x01, 0x01));
+    aml_append(crs, aml_io(AML_DECODE16, 0x0064, 0x0064, 0x01, 0x01));
+    aml_append(crs, aml_irq_no_flags(1));
+
+    kbd = aml_device("KBD");
+    aml_append(kbd, aml_name_decl("_HID", aml_eisaid("PNP0303")));
+    aml_append(kbd, aml_name_decl("_STA", aml_int(0xf)));
+    aml_append(kbd, aml_name_decl("_CRS", crs));
+
+    crs = aml_resource_template();
+    aml_append(crs, aml_irq_no_flags(12));
+
+    mou = aml_device("MOU");
+    aml_append(mou, aml_name_decl("_HID", aml_eisaid("PNP0F13")));
+    aml_append(mou, aml_name_decl("_STA", aml_int(0xf)));
+    aml_append(mou, aml_name_decl("_CRS", crs));
+
+    aml_append(scope, kbd);
+    aml_append(scope, mou);
+}
+
 static void i8042_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    ISADeviceClass *isa = ISA_DEVICE_CLASS(klass);
 
     dc->realize = i8042_realizefn;
     dc->vmsd = &vmstate_kbd_isa;
+    isa->build_aml = i8042_build_aml;
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 }
 
-- 
2.18.2

[PATCH v3 14/15] acpi: factor out fw_cfg_add_acpi_dsdt()

[Gerd Hoffmann]

Add helper function to add fw_cfg device,
also move code to hw/i386/fw_cfg.c.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/i386/fw_cfg.h     |  1 +
 hw/i386/acpi-build.c | 24 +-----------------------
 hw/i386/fw_cfg.c     | 28 ++++++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/hw/i386/fw_cfg.h b/hw/i386/fw_cfg.h
index 9e742787792b..275f15c1c5e8 100644
--- a/hw/i386/fw_cfg.h
+++ b/hw/i386/fw_cfg.h
@@ -25,5 +25,6 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms,
                                uint16_t apic_id_limit);
 void fw_cfg_build_smbios(MachineState *ms, FWCfgState *fw_cfg);
 void fw_cfg_build_feature_control(MachineState *ms, FWCfgState *fw_cfg);
+void fw_cfg_add_acpi_dsdt(Aml *scope, FWCfgState *fw_cfg);
 
 #endif
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 643e875c05a5..a8b790021974 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1874,30 +1874,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
 
     /* create fw_cfg node, unconditionally */
     {
-        /* when using port i/o, the 8-bit data register *always* overlaps
-         * with half of the 16-bit control register. Hence, the total size
-         * of the i/o region used is FW_CFG_CTL_SIZE; when using DMA, the
-         * DMA control register is located at FW_CFG_DMA_IO_BASE + 4 */
-        uint8_t io_size = object_property_get_bool(OBJECT(x86ms->fw_cfg),
-                                                   "dma_enabled", NULL) ?
-                          ROUND_UP(FW_CFG_CTL_SIZE, 4) + sizeof(dma_addr_t) :
-                          FW_CFG_CTL_SIZE;
-
         scope = aml_scope("\\_SB.PCI0");
-        dev = aml_device("FWCF");
-
-        aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
-
-        /* 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, FW_CFG_IO_BASE, FW_CFG_IO_BASE, 0x01, io_size)
-        );
-        aml_append(dev, aml_name_decl("_CRS", crs));
-
-        aml_append(scope, dev);
+        fw_cfg_add_acpi_dsdt(scope, x86ms->fw_cfg);
         aml_append(dsdt, scope);
     }
 
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index da60ada59462..c55abfb01abb 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -15,6 +15,7 @@
 #include "qemu/osdep.h"
 #include "sysemu/numa.h"
 #include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
 #include "hw/firmware/smbios.h"
 #include "hw/i386/fw_cfg.h"
 #include "hw/timer/hpet.h"
@@ -179,3 +180,30 @@ void fw_cfg_build_feature_control(MachineState *ms, FWCfgState *fw_cfg)
     *val = cpu_to_le64(feature_control_bits | FEATURE_CONTROL_LOCKED);
     fw_cfg_add_file(fw_cfg, "etc/msr_feature_control", val, sizeof(*val));
 }
+
+void fw_cfg_add_acpi_dsdt(Aml *scope, FWCfgState *fw_cfg)
+{
+    /*
+     * when using port i/o, the 8-bit data register *always* overlaps
+     * with half of the 16-bit control register. Hence, the total size
+     * of the i/o region used is FW_CFG_CTL_SIZE; when using DMA, the
+     * DMA control register is located at FW_CFG_DMA_IO_BASE + 4
+     */
+    Object *obj = OBJECT(fw_cfg);
+    uint8_t io_size = object_property_get_bool(obj, "dma_enabled", NULL) ?
+        ROUND_UP(FW_CFG_CTL_SIZE, 4) + sizeof(dma_addr_t) :
+        FW_CFG_CTL_SIZE;
+    Aml *dev = aml_device("FWCF");
+    Aml *crs = aml_resource_template();
+
+    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
+
+    /* device present, functioning, decoding, not shown in UI */
+    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
+
+    aml_append(crs,
+        aml_io(AML_DECODE16, FW_CFG_IO_BASE, FW_CFG_IO_BASE, 0x01, io_size));
+
+    aml_append(dev, aml_name_decl("_CRS", crs));
+    aml_append(scope, dev);
+}
-- 
2.18.2

[PATCH v3 15/15] acpi: simplify build_isa_devices_aml()

[Gerd Hoffmann]

x86 machines can have a single ISA bus only.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/acpi-build.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a8b790021974..98b3fd1cdb14 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1055,19 +1055,14 @@ static void build_hpet_aml(Aml *table)
 static void build_isa_devices_aml(Aml *table)
 {
     bool ambiguous;
-
-    Aml *scope = aml_scope("_SB.PCI0.ISA");
     Object *obj = object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous);
+    Aml *scope;
 
-    if (ambiguous) {
-        error_report("Multiple ISA busses, unable to define IPMI ACPI data");
-    } else if (!obj) {
-        error_report("No ISA bus, unable to define IPMI ACPI data");
-    } else {
-        build_acpi_ipmi_devices(scope, BUS(obj), "\\_SB.PCI0.ISA");
-        isa_build_aml(ISA_BUS(obj), scope);
-    }
+    assert(obj && !ambiguous);
 
+    scope = aml_scope("_SB.PCI0.ISA");
+    build_acpi_ipmi_devices(scope, BUS(obj), "\\_SB.PCI0.ISA");
+    isa_build_aml(ISA_BUS(obj), scope);
     aml_append(table, scope);
 }
 
-- 
2.18.2

Re: [PATCH v3 00/15] acpi: i386 tweaks

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200429140003.7336-1-kraxel@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20200429140003.7336-1-kraxel@redhat.com
Subject: [PATCH v3 00/15] acpi: i386 tweaks
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
e42c6f8 acpi: simplify build_isa_devices_aml()
f55de34 acpi: factor out fw_cfg_add_acpi_dsdt()
e717e9e acpi: move aml builder code for i8042 (kbd+mouse) device
902fe4b acpi: move aml builder code for floppy device
381c640 acpi: move aml builder code for parallel device
7143112 acpi: parallel: don't use _STA method
652bafc acpi: move aml builder code for serial device
a156bbb acpi: serial: don't use _STA method
5b9e269 acpi: move aml builder code for rtc device
aee5c70 rtc: add RTC_ISA_BASE
0b4e292 acpi: add ISADeviceClass->build_aml()
aa50821 acpi: drop pointless _STA method
977bff5 qtest: allow DSDT acpi table changes
af6a412 acpi: add aml builder stubs
c714c16 move 'typedef Aml' to qemu/types.h

=== OUTPUT BEGIN ===
1/15 Checking commit c714c160fe50 (move 'typedef Aml' to qemu/types.h)
2/15 Checking commit af6a4124ef46 (acpi: add aml builder stubs)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#27: 
new file mode 100644

total: 0 errors, 1 warnings, 87 lines checked

Patch 2/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/15 Checking commit 977bff5929a2 (qtest: allow DSDT acpi table changes)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/aml-build-stub.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/aml-build-stub.c found

total: 2 errors, 0 warnings, 18 lines checked

Patch 3/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/15 Checking commit aa508215d7fe (acpi: drop pointless _STA method)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

total: 2 errors, 0 warnings, 40 lines checked

Patch 4/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/15 Checking commit 0b4e292ee2df (acpi: add ISADeviceClass->build_aml())
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/isa/isa-bus.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/isa/isa-bus.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/isa/isa.h found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/isa/isa.h found

total: 6 errors, 0 warnings, 42 lines checked

Patch 5/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

6/15 Checking commit aee5c702ac1b (rtc: add RTC_ISA_BASE)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/rtc/mc146818rtc.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/rtc/mc146818rtc.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/rtc/mc146818rtc.h found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/rtc/mc146818rtc.h found

total: 4 errors, 0 warnings, 30 lines checked

Patch 6/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/15 Checking commit 5b9e269be56d (acpi: move aml builder code for rtc device)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/rtc/mc146818rtc.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/rtc/mc146818rtc.c found

total: 4 errors, 0 warnings, 68 lines checked

Patch 7/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

8/15 Checking commit a156bbb317d7 (acpi: serial: don't use _STA method)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

total: 2 errors, 0 warnings, 66 lines checked

Patch 8/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

9/15 Checking commit 652bafc9af4e (acpi: move aml builder code for serial device)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/char/serial-isa.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/char/serial-isa.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

total: 4 errors, 0 warnings, 97 lines checked

Patch 9/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

10/15 Checking commit 7143112749cd (acpi: parallel: don't use _STA method)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

total: 2 errors, 0 warnings, 54 lines checked

Patch 10/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

11/15 Checking commit 381c64082a32 (acpi: move aml builder code for parallel device)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/char/parallel.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/char/parallel.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

total: 4 errors, 0 warnings, 88 lines checked

Patch 11/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

12/15 Checking commit 902fe4b93a56 (acpi: move aml builder code for floppy device)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/block/fdc.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/block/fdc.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and stubs/Makefile.objs found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and stubs/Makefile.objs found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and stubs/cmos.c found

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#244: 
new file mode 100644

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and stubs/cmos.c found

total: 8 errors, 1 warnings, 221 lines checked

Patch 12/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

13/15 Checking commit e717e9e3bb42 (acpi: move aml builder code for i8042 (kbd+mouse) device)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/input/pckbd.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/input/pckbd.c found

total: 4 errors, 0 warnings, 100 lines checked

Patch 13/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

14/15 Checking commit f55de3428a01 (acpi: factor out fw_cfg_add_acpi_dsdt())
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/fw_cfg.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/fw_cfg.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/fw_cfg.h found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/fw_cfg.h found

total: 6 errors, 0 warnings, 74 lines checked

Patch 14/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

15/15 Checking commit e42c6f818141 (acpi: simplify build_isa_devices_aml())
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found

total: 2 errors, 0 warnings, 24 lines checked

Patch 15/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200429140003.7336-1-kraxel@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v3 04/15] acpi: drop pointless _STA method

[Philippe Mathieu-Daudé]

On 4/29/20 3:59 PM, Gerd Hoffmann wrote:
> When returning a constant there is no point in having a method
> in the first place, _STA can be a simple integer instead.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   hw/i386/acpi-build.c | 14 +++-----------
>   1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 23c77eeb95a9..3a046b03e4cd 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1151,14 +1151,11 @@ static Aml *build_kbd_device_aml(void)
>   {
>       Aml *dev;
>       Aml *crs;
> -    Aml *method;
>   
>       dev = aml_device("KBD");
>       aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0303")));
>   
> -    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> -    aml_append(method, aml_return(aml_int(0x0f)));
> -    aml_append(dev, method);
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
>   
>       crs = aml_resource_template();
>       aml_append(crs, aml_io(AML_DECODE16, 0x0060, 0x0060, 0x01, 0x01));
> @@ -1173,14 +1170,11 @@ static Aml *build_mouse_device_aml(void)
>   {
>       Aml *dev;
>       Aml *crs;
> -    Aml *method;
>   
>       dev = aml_device("MOU");
>       aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0F13")));
>   
> -    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> -    aml_append(method, aml_return(aml_int(0x0f)));
> -    aml_append(dev, method);
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
>   
>       crs = aml_resource_template();
>       aml_append(crs, aml_irq_no_flags(12));
> @@ -2238,9 +2232,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>                                              TPM_CRB_ADDR_SIZE, AML_READ_WRITE));
>           aml_append(dev, aml_name_decl("_CRS", crs));
>   
> -        method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> -        aml_append(method, aml_return(aml_int(0x0f)));
> -        aml_append(dev, method);
> +        aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
>   
>           tpm_build_ppi_acpi(tpm, dev);
>   
> 

Re: [PATCH v3 01/15] move 'typedef Aml' to qemu/types.h

[Philippe Mathieu-Daudé]

On 4/29/20 3:59 PM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   include/hw/acpi/aml-build.h | 1 -
>   include/qemu/typedefs.h     | 1 +
>   2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 0f4ed53d7fbf..1539fe066714 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -33,7 +33,6 @@ struct Aml {
>       uint8_t op;
>       AmlBlockFlags block_flags;
>   };
> -typedef struct Aml Aml;
>   
>   typedef enum {
>       AML_COMPATIBILITY = 0,
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 375770a80f06..ecf3cde26c3c 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -24,6 +24,7 @@
>   typedef struct AdapterInfo AdapterInfo;
>   typedef struct AddressSpace AddressSpace;
>   typedef struct AioContext AioContext;
> +typedef struct Aml Aml;
>   typedef struct AnnounceTimer AnnounceTimer;
>   typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
>   typedef struct BdrvDirtyBitmapIter BdrvDirtyBitmapIter;
> 

Re: [PATCH v3 06/15] rtc: add RTC_ISA_BASE

[Philippe Mathieu-Daudé]

On 4/29/20 3:59 PM, Gerd Hoffmann wrote:
> Add and use RTC_ISA_BASE define instead of hardcoding 0x70.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   include/hw/rtc/mc146818rtc.h | 1 +
>   hw/rtc/mc146818rtc.c         | 5 ++---
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h
> index 10c93a096a1d..3713181b56fe 100644
> --- a/include/hw/rtc/mc146818rtc.h
> +++ b/include/hw/rtc/mc146818rtc.h
> @@ -47,6 +47,7 @@ typedef struct RTCState {
>   } RTCState;
>   
>   #define RTC_ISA_IRQ 8
> +#define RTC_ISA_BASE 0x70

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>   
>   ISADevice *mc146818_rtc_init(ISABus *bus, int base_year,
>                                qemu_irq intercept_irq);
> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
> index dc4269cc55cb..d18c09911be2 100644
> --- a/hw/rtc/mc146818rtc.c
> +++ b/hw/rtc/mc146818rtc.c
> @@ -908,7 +908,6 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
>   {
>       ISADevice *isadev = ISA_DEVICE(dev);
>       RTCState *s = MC146818_RTC(dev);
> -    int base = 0x70;
>   
>       s->cmos_data[RTC_REG_A] = 0x26;
>       s->cmos_data[RTC_REG_B] = 0x02;
> @@ -951,7 +950,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
>       qemu_register_suspend_notifier(&s->suspend_notifier);
>   
>       memory_region_init_io(&s->io, OBJECT(s), &cmos_ops, s, "rtc", 2);
> -    isa_register_ioport(isadev, &s->io, base);
> +    isa_register_ioport(isadev, &s->io, RTC_ISA_BASE);
>   
>       /* register rtc 0x70 port for coalesced_pio */
>       memory_region_set_flush_coalesced(&s->io);
> @@ -960,7 +959,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
>       memory_region_add_subregion(&s->io, 0, &s->coalesced_io);
>       memory_region_add_coalescing(&s->coalesced_io, 0, 1);
>   
> -    qdev_set_legacy_instance_id(dev, base, 3);
> +    qdev_set_legacy_instance_id(dev, RTC_ISA_BASE, 3);
>       qemu_register_reset(rtc_reset, s);
>   
>       object_property_add_tm(OBJECT(s), "date", rtc_get_date, NULL);
> 

Re: [PATCH v3 08/15] acpi: serial: don't use _STA method

[Philippe Mathieu-Daudé]

Hi Gerd,

On 4/29/20 3:59 PM, Gerd Hoffmann wrote:
> The _STA method dates back to the days where we had a static DSDT.  The
> device is listed in the DSDT table unconditionally and the _STA method
> checks a bit in the isa bridge pci config space to figure whenever a
> given is isa device is present or not, then evaluates to 0x0f (present)
> or 0x00 (absent).
> 
> These days the DSDT is generated by qemu anyway, so if a device is not
> present we can simply drop it from the DSDT instead.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
>   hw/i386/acpi-build.c | 32 ++++++++------------------------
>   1 file changed, 8 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index f79e5eaf170d..a99e5bbd1156 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1202,50 +1202,34 @@ static Aml *build_lpt_device_aml(void)
>       return dev;
>   }
>   
> -static Aml *build_com_device_aml(uint8_t uid)
> +static void build_com_device_aml(Aml *scope, uint8_t uid)
>   {
>       Aml *dev;
>       Aml *crs;
> -    Aml *method;
> -    Aml *if_ctx;
> -    Aml *else_ctx;
> -    Aml *zero = aml_int(0);
> -    Aml *is_present = aml_local(0);
> -    const char *enabled_field = "CAEN";
>       uint8_t irq = 4;
>       uint16_t io_port = 0x03F8;
>   
>       assert(uid == 1 || uid == 2);
>       if (uid == 2) {
> -        enabled_field = "CBEN";
>           irq = 3;
>           io_port = 0x02F8;
>       }
> +    if (!memory_region_present(get_system_io(), io_port)) {
> +        return;
> +    }

The patch looks OK, but an you split this check into a separate 
(previous?) patch please?

>   
>       dev = aml_device("COM%d", uid);
>       aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0501")));
>       aml_append(dev, aml_name_decl("_UID", aml_int(uid)));
>   
> -    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> -    aml_append(method, aml_store(aml_name("%s", enabled_field), is_present));
> -    if_ctx = aml_if(aml_equal(is_present, zero));
> -    {
> -        aml_append(if_ctx, aml_return(aml_int(0x00)));
> -    }
> -    aml_append(method, if_ctx);
> -    else_ctx = aml_else();
> -    {
> -        aml_append(else_ctx, aml_return(aml_int(0x0f)));
> -    }
> -    aml_append(method, else_ctx);
> -    aml_append(dev, method);
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
>   
>       crs = aml_resource_template();
>       aml_append(crs, aml_io(AML_DECODE16, io_port, io_port, 0x00, 0x08));
>       aml_append(crs, aml_irq_no_flags(irq));
>       aml_append(dev, aml_name_decl("_CRS", crs));
>   
> -    return dev;
> +    aml_append(scope, dev);
>   }
>   
>   static void build_isa_devices_aml(Aml *table)
> @@ -1262,8 +1246,8 @@ static void build_isa_devices_aml(Aml *table)
>           aml_append(scope, build_fdc_device_aml(fdc));
>       }
>       aml_append(scope, build_lpt_device_aml());
> -    aml_append(scope, build_com_device_aml(1));
> -    aml_append(scope, build_com_device_aml(2));
> +    build_com_device_aml(scope, 1);
> +    build_com_device_aml(scope, 2);
>   
>       if (ambiguous) {
>           error_report("Multiple ISA busses, unable to define IPMI ACPI data");
> 

Re: [PATCH v3 10/15] acpi: parallel: don't use _STA method

[Philippe Mathieu-Daudé]

On 4/29/20 3:59 PM, Gerd Hoffmann wrote:
> The _STA method dates back to the days where we had a static DSDT.  The
> device is listed in the DSDT table unconditionally and the _STA method
> checks a bit in the isa bridge pci config space to figure whenever a
> given is isa device is present or not, then evaluates to 0x0f (present)
> or 0x00 (absent).
> 
> These days the DSDT is generated by qemu anyway, so if a device is not
> present we can simply drop it from the DSDT instead.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
>   hw/i386/acpi-build.c | 29 ++++++++---------------------
>   1 file changed, 8 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index fea83352e6ab..e01afbd011d9 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1167,39 +1167,26 @@ static Aml *build_mouse_device_aml(void)
>       return dev;
>   }
>   
> -static Aml *build_lpt_device_aml(void)
> +static void build_lpt_device_aml(Aml *scope)
>   {
>       Aml *dev;
>       Aml *crs;
> -    Aml *method;
> -    Aml *if_ctx;
> -    Aml *else_ctx;
> -    Aml *zero = aml_int(0);
> -    Aml *is_present = aml_local(0);
> +
> +    if (!memory_region_present(get_system_io(), 0x0378)) {
> +        return;
> +    }

Please move this check in a separate patch.

Also, it may be worth a ISA_PARALLEL_IOBASE 0x378 definition cleanup 
like you did with the RTC.

>   
>       dev = aml_device("LPT");
>       aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
>   
> -    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> -    aml_append(method, aml_store(aml_name("LPEN"), is_present));
> -    if_ctx = aml_if(aml_equal(is_present, zero));
> -    {
> -        aml_append(if_ctx, aml_return(aml_int(0x00)));
> -    }
> -    aml_append(method, if_ctx);
> -    else_ctx = aml_else();
> -    {
> -        aml_append(else_ctx, aml_return(aml_int(0x0f)));
> -    }
> -    aml_append(method, else_ctx);
> -    aml_append(dev, method);
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
>   
>       crs = aml_resource_template();
>       aml_append(crs, aml_io(AML_DECODE16, 0x0378, 0x0378, 0x08, 0x08));
>       aml_append(crs, aml_irq_no_flags(7));
>       aml_append(dev, aml_name_decl("_CRS", crs));
>   
> -    return dev;
> +    aml_append(scope, dev);
>   }
>   
>   static void build_isa_devices_aml(Aml *table)
> @@ -1215,7 +1202,7 @@ static void build_isa_devices_aml(Aml *table)
>       if (fdc) {
>           aml_append(scope, build_fdc_device_aml(fdc));
>       }
> -    aml_append(scope, build_lpt_device_aml());
> +    build_lpt_device_aml(scope);
>   
>       if (ambiguous) {
>           error_report("Multiple ISA busses, unable to define IPMI ACPI data");
> 

Re: [PATCH v3 15/15] acpi: simplify build_isa_devices_aml()

[Philippe Mathieu-Daudé]

Cc'ing IPMI maintainer.

On 4/29/20 4:00 PM, Gerd Hoffmann wrote:
> x86 machines can have a single ISA bus only.

I disagree with the comment.
Machines can have multiple ISA bus.
However the PCI IO space can have a single ISA bus.
This patch is about PCI0.

> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/i386/acpi-build.c | 15 +++++----------
>   1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a8b790021974..98b3fd1cdb14 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1055,19 +1055,14 @@ static void build_hpet_aml(Aml *table)
>   static void build_isa_devices_aml(Aml *table)
>   {
>       bool ambiguous;
> -
> -    Aml *scope = aml_scope("_SB.PCI0.ISA");
>       Object *obj = object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous);
> +    Aml *scope;
>   
> -    if (ambiguous) {
> -        error_report("Multiple ISA busses, unable to define IPMI ACPI data");
> -    } else if (!obj) {
> -        error_report("No ISA bus, unable to define IPMI ACPI data");
> -    } else {
> -        build_acpi_ipmi_devices(scope, BUS(obj), "\\_SB.PCI0.ISA");
> -        isa_build_aml(ISA_BUS(obj), scope);
> -    }
> +    assert(obj && !ambiguous);
>   
> +    scope = aml_scope("_SB.PCI0.ISA");
> +    build_acpi_ipmi_devices(scope, BUS(obj), "\\_SB.PCI0.ISA");
> +    isa_build_aml(ISA_BUS(obj), scope);
>       aml_append(table, scope);
>   }
>   
> 

Re: [PATCH v3 13/15] acpi: move aml builder code for i8042 (kbd+mouse) device

[Philippe Mathieu-Daudé]

On 4/29/20 4:00 PM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   hw/i386/acpi-build.c | 39 ---------------------------------------
>   hw/input/pckbd.c     | 31 +++++++++++++++++++++++++++++++
>   2 files changed, 31 insertions(+), 39 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 7415bd5fdce0..643e875c05a5 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1052,42 +1052,6 @@ static void build_hpet_aml(Aml *table)
>       aml_append(table, scope);
>   }
>   
> -static Aml *build_kbd_device_aml(void)
> -{
> -    Aml *dev;
> -    Aml *crs;
> -
> -    dev = aml_device("KBD");
> -    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0303")));
> -
> -    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
> -
> -    crs = aml_resource_template();
> -    aml_append(crs, aml_io(AML_DECODE16, 0x0060, 0x0060, 0x01, 0x01));
> -    aml_append(crs, aml_io(AML_DECODE16, 0x0064, 0x0064, 0x01, 0x01));
> -    aml_append(crs, aml_irq_no_flags(1));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -
> -    return dev;
> -}
> -
> -static Aml *build_mouse_device_aml(void)
> -{
> -    Aml *dev;
> -    Aml *crs;
> -
> -    dev = aml_device("MOU");
> -    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0F13")));
> -
> -    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
> -
> -    crs = aml_resource_template();
> -    aml_append(crs, aml_irq_no_flags(12));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -
> -    return dev;
> -}
> -
>   static void build_isa_devices_aml(Aml *table)
>   {
>       bool ambiguous;
> @@ -1095,9 +1059,6 @@ static void build_isa_devices_aml(Aml *table)
>       Aml *scope = aml_scope("_SB.PCI0.ISA");
>       Object *obj = object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous);
>   
> -    aml_append(scope, build_kbd_device_aml());
> -    aml_append(scope, build_mouse_device_aml());
> -
>       if (ambiguous) {
>           error_report("Multiple ISA busses, unable to define IPMI ACPI data");
>       } else if (!obj) {
> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
> index 60a41303203a..29d633ca9478 100644
> --- a/hw/input/pckbd.c
> +++ b/hw/input/pckbd.c
> @@ -26,6 +26,7 @@
>   #include "qemu/log.h"
>   #include "hw/isa/isa.h"
>   #include "migration/vmstate.h"
> +#include "hw/acpi/aml-build.h"
>   #include "hw/input/ps2.h"
>   #include "hw/irq.h"
>   #include "hw/input/i8042.h"
> @@ -561,12 +562,42 @@ static void i8042_realizefn(DeviceState *dev, Error **errp)
>       qemu_register_reset(kbd_reset, s);
>   }
>   
> +static void i8042_build_aml(ISADevice *isadev, Aml *scope)
> +{
> +    Aml *kbd;
> +    Aml *mou;
> +    Aml *crs;
> +
> +    crs = aml_resource_template();
> +    aml_append(crs, aml_io(AML_DECODE16, 0x0060, 0x0060, 0x01, 0x01));
> +    aml_append(crs, aml_io(AML_DECODE16, 0x0064, 0x0064, 0x01, 0x01));
> +    aml_append(crs, aml_irq_no_flags(1));
> +
> +    kbd = aml_device("KBD");
> +    aml_append(kbd, aml_name_decl("_HID", aml_eisaid("PNP0303")));
> +    aml_append(kbd, aml_name_decl("_STA", aml_int(0xf)));
> +    aml_append(kbd, aml_name_decl("_CRS", crs));
> +
> +    crs = aml_resource_template();
> +    aml_append(crs, aml_irq_no_flags(12));
> +
> +    mou = aml_device("MOU");
> +    aml_append(mou, aml_name_decl("_HID", aml_eisaid("PNP0F13")));
> +    aml_append(mou, aml_name_decl("_STA", aml_int(0xf)));
> +    aml_append(mou, aml_name_decl("_CRS", crs));
> +
> +    aml_append(scope, kbd);
> +    aml_append(scope, mou);
> +}
> +
>   static void i8042_class_initfn(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
> +    ISADeviceClass *isa = ISA_DEVICE_CLASS(klass);
>   
>       dc->realize = i8042_realizefn;
>       dc->vmsd = &vmstate_kbd_isa;
> +    isa->build_aml = i8042_build_aml;
>       set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
>   }
>   
> 

Re: [PATCH v3 06/15] rtc: add RTC_ISA_BASE

[Igor Mammedov]

On Wed, 29 Apr 2020 15:59:54 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Add and use RTC_ISA_BASE define instead of hardcoding 0x70.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/hw/rtc/mc146818rtc.h | 1 +
>  hw/rtc/mc146818rtc.c         | 5 ++---
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h
> index 10c93a096a1d..3713181b56fe 100644
> --- a/include/hw/rtc/mc146818rtc.h
> +++ b/include/hw/rtc/mc146818rtc.h
> @@ -47,6 +47,7 @@ typedef struct RTCState {
>  } RTCState;
>  
>  #define RTC_ISA_IRQ 8
> +#define RTC_ISA_BASE 0x70
>  
>  ISADevice *mc146818_rtc_init(ISABus *bus, int base_year,
>                               qemu_irq intercept_irq);
> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
> index dc4269cc55cb..d18c09911be2 100644
> --- a/hw/rtc/mc146818rtc.c
> +++ b/hw/rtc/mc146818rtc.c
> @@ -908,7 +908,6 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
>  {
>      ISADevice *isadev = ISA_DEVICE(dev);
>      RTCState *s = MC146818_RTC(dev);
> -    int base = 0x70;
>  
>      s->cmos_data[RTC_REG_A] = 0x26;
>      s->cmos_data[RTC_REG_B] = 0x02;
> @@ -951,7 +950,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
>      qemu_register_suspend_notifier(&s->suspend_notifier);
>  
>      memory_region_init_io(&s->io, OBJECT(s), &cmos_ops, s, "rtc", 2);
> -    isa_register_ioport(isadev, &s->io, base);
> +    isa_register_ioport(isadev, &s->io, RTC_ISA_BASE);
>  
>      /* register rtc 0x70 port for coalesced_pio */
>      memory_region_set_flush_coalesced(&s->io);
> @@ -960,7 +959,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
>      memory_region_add_subregion(&s->io, 0, &s->coalesced_io);
>      memory_region_add_coalescing(&s->coalesced_io, 0, 1);
>  
> -    qdev_set_legacy_instance_id(dev, base, 3);
> +    qdev_set_legacy_instance_id(dev, RTC_ISA_BASE, 3);
>      qemu_register_reset(rtc_reset, s);
>  
>      object_property_add_tm(OBJECT(s), "date", rtc_get_date, NULL);

Re: [PATCH v3 07/15] acpi: move aml builder code for rtc device

[Igor Mammedov]

On Wed, 29 Apr 2020 15:59:55 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Also use a single io range instead of two,
> following what real hardware does.
I'd make a separate patch for this comment.
Also qemu maps only the first range (0x70) if I'm not mistaken,
so we perhaps should drop the second (0x72) instead of merging it into the first

[...]

> -static Aml *build_rtc_device_aml(void)
> -{
> -    Aml *dev;
> -    Aml *crs;
> -
> -    dev = aml_device("RTC");
> -    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0B00")));
> -    crs = aml_resource_template();
> -    aml_append(crs, aml_io(AML_DECODE16, 0x0070, 0x0070, 0x10, 0x02));
> -    aml_append(crs, aml_irq_no_flags(8));
> -    aml_append(crs, aml_io(AML_DECODE16, 0x0072, 0x0072, 0x02, 0x06));
[...]
> +static void rtc_build_aml(ISADevice *isadev, Aml *scope)
> +{
> +    Aml *dev;
> +    Aml *crs;
> +
> +    crs = aml_resource_template();
> +    aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE, RTC_ISA_BASE,
> +                           0x10, 0x08));

[...]

Re: [PATCH v3 11/15] acpi: move aml builder code for parallel device

[Igor Mammedov]

On Wed, 29 Apr 2020 15:59:59 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Also adds support for multiple LPT devices.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/char/parallel.c   | 32 ++++++++++++++++++++++++++++++++
>  hw/i386/acpi-build.c | 23 -----------------------
>  2 files changed, 32 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/char/parallel.c b/hw/char/parallel.c
> index 8dd67d13759b..bc6b55b3b910 100644
> --- a/hw/char/parallel.c
> +++ b/hw/char/parallel.c
> @@ -28,6 +28,7 @@
>  #include "qemu/module.h"
>  #include "chardev/char-parallel.h"
>  #include "chardev/char-fe.h"
> +#include "hw/acpi/aml-build.h"
>  #include "hw/irq.h"
>  #include "hw/isa/isa.h"
>  #include "hw/qdev-properties.h"
> @@ -568,6 +569,35 @@ static void parallel_isa_realizefn(DeviceState *dev, Error **errp)
>                               s, "parallel");
>  }
>  
> +static void parallel_isa_build_aml(ISADevice *isadev, Aml *scope)
> +{
> +    ISAParallelState *isa = ISA_PARALLEL(isadev);
> +    int i, uid = 0;
> +    Aml *dev;
> +    Aml *crs;
> +
> +    for (i = 0; i < ARRAY_SIZE(isa_parallel_io); i++) {
> +        if (isa->iobase == isa_parallel_io[i]) {
> +            uid = i + 1;
> +        }
> +    }
> +    if (!uid) {
> +        return;
> +    }
> +
> +    crs = aml_resource_template();
> +    aml_append(crs, aml_io(AML_DECODE16, isa->iobase, isa->iobase, 0x08, 0x08));
> +    aml_append(crs, aml_irq_no_flags(isa->isairq));
> +
> +    dev = aml_device("LPT%d", uid);
> +    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(uid)));
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +    aml_append(scope, dev);
> +}
> +
>  /* Memory mapped interface */
>  static uint64_t parallel_mm_readfn(void *opaque, hwaddr addr, unsigned size)
>  {
> @@ -624,9 +654,11 @@ static Property parallel_isa_properties[] = {
>  static void parallel_isa_class_initfn(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    ISADeviceClass *isa = ISA_DEVICE_CLASS(klass);
>  
>      dc->realize = parallel_isa_realizefn;
>      dc->vmsd = &vmstate_parallel_isa;
> +    isa->build_aml = parallel_isa_build_aml;
>      device_class_set_props(dc, parallel_isa_properties);
>      set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
>  }
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index e01afbd011d9..12a017e1f45b 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1167,28 +1167,6 @@ static Aml *build_mouse_device_aml(void)
>      return dev;
>  }
>  
> -static void build_lpt_device_aml(Aml *scope)
> -{
> -    Aml *dev;
> -    Aml *crs;
> -
> -    if (!memory_region_present(get_system_io(), 0x0378)) {
> -        return;
> -    }
> -
> -    dev = aml_device("LPT");
> -    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
> -
> -    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
> -
> -    crs = aml_resource_template();
> -    aml_append(crs, aml_io(AML_DECODE16, 0x0378, 0x0378, 0x08, 0x08));
> -    aml_append(crs, aml_irq_no_flags(7));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -
> -    aml_append(scope, dev);
> -}
> -
>  static void build_isa_devices_aml(Aml *table)
>  {
>      ISADevice *fdc = pc_find_fdc0();
> @@ -1202,7 +1180,6 @@ static void build_isa_devices_aml(Aml *table)
>      if (fdc) {
>          aml_append(scope, build_fdc_device_aml(fdc));
>      }
> -    build_lpt_device_aml(scope);
>  
>      if (ambiguous) {
>          error_report("Multiple ISA busses, unable to define IPMI ACPI data");

Re: [PATCH v3 12/15] acpi: move aml builder code for floppy device

[Igor Mammedov]

On Wed, 29 Apr 2020 16:00:00 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

see small nit below

> ---
>  hw/block/fdc.c       | 83 ++++++++++++++++++++++++++++++++++++++++++++
>  hw/i386/acpi-build.c | 83 --------------------------------------------
>  stubs/cmos.c         |  7 ++++
>  stubs/Makefile.objs  |  1 +
>  4 files changed, 91 insertions(+), 83 deletions(-)
>  create mode 100644 stubs/cmos.c
> 
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 33bc9e2f9249..56ddc26c7065 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -32,6 +32,8 @@
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
>  #include "qemu/timer.h"
> +#include "hw/i386/pc.h"
> +#include "hw/acpi/aml-build.h"
>  #include "hw/irq.h"
>  #include "hw/isa/isa.h"
>  #include "hw/qdev-properties.h"
> @@ -2764,6 +2766,85 @@ void isa_fdc_get_drive_max_chs(FloppyDriveType type,
>      (*maxc)--;
>  }
>  
> +static Aml *build_fdinfo_aml(int idx, FloppyDriveType type)
> +{
> +    Aml *dev, *fdi;
> +    uint8_t maxc, maxh, maxs;
> +
> +    isa_fdc_get_drive_max_chs(type, &maxc, &maxh, &maxs);
       ^^^ can be made static now


> +
> +    dev = aml_device("FLP%c", 'A' + idx);
> +
> +    aml_append(dev, aml_name_decl("_ADR", aml_int(idx)));
> +
> +    fdi = aml_package(16);
> +    aml_append(fdi, aml_int(idx));  /* Drive Number */
> +    aml_append(fdi,
> +        aml_int(cmos_get_fd_drive_type(type)));  /* Device Type */
> +    /*
> +     * the values below are the limits of the drive, and are thus independent
> +     * of the inserted media
> +     */
> +    aml_append(fdi, aml_int(maxc));  /* Maximum Cylinder Number */
> +    aml_append(fdi, aml_int(maxs));  /* Maximum Sector Number */
> +    aml_append(fdi, aml_int(maxh));  /* Maximum Head Number */
> +    /*
> +     * SeaBIOS returns the below values for int 0x13 func 0x08 regardless of
> +     * the drive type, so shall we
> +     */
> +    aml_append(fdi, aml_int(0xAF));  /* disk_specify_1 */
> +    aml_append(fdi, aml_int(0x02));  /* disk_specify_2 */
> +    aml_append(fdi, aml_int(0x25));  /* disk_motor_wait */
> +    aml_append(fdi, aml_int(0x02));  /* disk_sector_siz */
> +    aml_append(fdi, aml_int(0x12));  /* disk_eot */
> +    aml_append(fdi, aml_int(0x1B));  /* disk_rw_gap */
> +    aml_append(fdi, aml_int(0xFF));  /* disk_dtl */
> +    aml_append(fdi, aml_int(0x6C));  /* disk_formt_gap */
> +    aml_append(fdi, aml_int(0xF6));  /* disk_fill */
> +    aml_append(fdi, aml_int(0x0F));  /* disk_head_sttl */
> +    aml_append(fdi, aml_int(0x08));  /* disk_motor_strt */
> +
> +    aml_append(dev, aml_name_decl("_FDI", fdi));
> +    return dev;
> +}
> +
> +static void fdc_isa_build_aml(ISADevice *isadev, Aml *scope)
> +{
> +    Aml *dev;
> +    Aml *crs;
> +    int i;
> +
> +#define ACPI_FDE_MAX_FD 4
> +    uint32_t fde_buf[5] = {
> +        0, 0, 0, 0,     /* presence of floppy drives #0 - #3 */
> +        cpu_to_le32(2)  /* tape presence (2 == never present) */
> +    };
> +
> +    crs = aml_resource_template();
> +    aml_append(crs, aml_io(AML_DECODE16, 0x03F2, 0x03F2, 0x00, 0x04));
> +    aml_append(crs, aml_io(AML_DECODE16, 0x03F7, 0x03F7, 0x00, 0x01));
> +    aml_append(crs, aml_irq_no_flags(6));
> +    aml_append(crs,
> +        aml_dma(AML_COMPATIBILITY, AML_NOTBUSMASTER, AML_TRANSFER8, 2));
> +
> +    dev = aml_device("FDC0");
> +    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0700")));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +    for (i = 0; i < MIN(MAX_FD, ACPI_FDE_MAX_FD); i++) {
> +        FloppyDriveType type = isa_fdc_get_drive_type(isadev, i);
> +
> +        if (type < FLOPPY_DRIVE_TYPE_NONE) {
> +            fde_buf[i] = cpu_to_le32(1);  /* drive present */
> +            aml_append(dev, build_fdinfo_aml(i, type));
> +        }
> +    }
> +    aml_append(dev, aml_name_decl("_FDE",
> +               aml_buffer(sizeof(fde_buf), (uint8_t *)fde_buf)));
> +
> +    aml_append(scope, dev);
> +}
> +
>  static const VMStateDescription vmstate_isa_fdc ={
>      .name = "fdc",
>      .version_id = 2,
> @@ -2797,11 +2878,13 @@ static Property isa_fdc_properties[] = {
>  static void isabus_fdc_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    ISADeviceClass *isa = ISA_DEVICE_CLASS(klass);
>  
>      dc->realize = isabus_fdc_realize;
>      dc->fw_name = "fdc";
>      dc->reset = fdctrl_external_reset_isa;
>      dc->vmsd = &vmstate_isa_fdc;
> +    isa->build_aml = fdc_isa_build_aml;
>      device_class_set_props(dc, isa_fdc_properties);
>      set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
>  }
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 12a017e1f45b..7415bd5fdce0 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1052,85 +1052,6 @@ static void build_hpet_aml(Aml *table)
>      aml_append(table, scope);
>  }
>  
> -static Aml *build_fdinfo_aml(int idx, FloppyDriveType type)
> -{
> -    Aml *dev, *fdi;
> -    uint8_t maxc, maxh, maxs;
> -
> -    isa_fdc_get_drive_max_chs(type, &maxc, &maxh, &maxs);
> -
> -    dev = aml_device("FLP%c", 'A' + idx);
> -
> -    aml_append(dev, aml_name_decl("_ADR", aml_int(idx)));
> -
> -    fdi = aml_package(16);
> -    aml_append(fdi, aml_int(idx));  /* Drive Number */
> -    aml_append(fdi,
> -        aml_int(cmos_get_fd_drive_type(type)));  /* Device Type */
> -    /*
> -     * the values below are the limits of the drive, and are thus independent
> -     * of the inserted media
> -     */
> -    aml_append(fdi, aml_int(maxc));  /* Maximum Cylinder Number */
> -    aml_append(fdi, aml_int(maxs));  /* Maximum Sector Number */
> -    aml_append(fdi, aml_int(maxh));  /* Maximum Head Number */
> -    /*
> -     * SeaBIOS returns the below values for int 0x13 func 0x08 regardless of
> -     * the drive type, so shall we
> -     */
> -    aml_append(fdi, aml_int(0xAF));  /* disk_specify_1 */
> -    aml_append(fdi, aml_int(0x02));  /* disk_specify_2 */
> -    aml_append(fdi, aml_int(0x25));  /* disk_motor_wait */
> -    aml_append(fdi, aml_int(0x02));  /* disk_sector_siz */
> -    aml_append(fdi, aml_int(0x12));  /* disk_eot */
> -    aml_append(fdi, aml_int(0x1B));  /* disk_rw_gap */
> -    aml_append(fdi, aml_int(0xFF));  /* disk_dtl */
> -    aml_append(fdi, aml_int(0x6C));  /* disk_formt_gap */
> -    aml_append(fdi, aml_int(0xF6));  /* disk_fill */
> -    aml_append(fdi, aml_int(0x0F));  /* disk_head_sttl */
> -    aml_append(fdi, aml_int(0x08));  /* disk_motor_strt */
> -
> -    aml_append(dev, aml_name_decl("_FDI", fdi));
> -    return dev;
> -}
> -
> -static Aml *build_fdc_device_aml(ISADevice *fdc)
> -{
> -    int i;
> -    Aml *dev;
> -    Aml *crs;
> -
> -#define ACPI_FDE_MAX_FD 4
> -    uint32_t fde_buf[5] = {
> -        0, 0, 0, 0,     /* presence of floppy drives #0 - #3 */
> -        cpu_to_le32(2)  /* tape presence (2 == never present) */
> -    };
> -
> -    dev = aml_device("FDC0");
> -    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0700")));
> -
> -    crs = aml_resource_template();
> -    aml_append(crs, aml_io(AML_DECODE16, 0x03F2, 0x03F2, 0x00, 0x04));
> -    aml_append(crs, aml_io(AML_DECODE16, 0x03F7, 0x03F7, 0x00, 0x01));
> -    aml_append(crs, aml_irq_no_flags(6));
> -    aml_append(crs,
> -        aml_dma(AML_COMPATIBILITY, AML_NOTBUSMASTER, AML_TRANSFER8, 2));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -
> -    for (i = 0; i < MIN(MAX_FD, ACPI_FDE_MAX_FD); i++) {
> -        FloppyDriveType type = isa_fdc_get_drive_type(fdc, i);
> -
> -        if (type < FLOPPY_DRIVE_TYPE_NONE) {
> -            fde_buf[i] = cpu_to_le32(1);  /* drive present */
> -            aml_append(dev, build_fdinfo_aml(i, type));
> -        }
> -    }
> -    aml_append(dev, aml_name_decl("_FDE",
> -               aml_buffer(sizeof(fde_buf), (uint8_t *)fde_buf)));
> -
> -    return dev;
> -}
> -
>  static Aml *build_kbd_device_aml(void)
>  {
>      Aml *dev;
> @@ -1169,7 +1090,6 @@ static Aml *build_mouse_device_aml(void)
>  
>  static void build_isa_devices_aml(Aml *table)
>  {
> -    ISADevice *fdc = pc_find_fdc0();
>      bool ambiguous;
>  
>      Aml *scope = aml_scope("_SB.PCI0.ISA");
> @@ -1177,9 +1097,6 @@ static void build_isa_devices_aml(Aml *table)
>  
>      aml_append(scope, build_kbd_device_aml());
>      aml_append(scope, build_mouse_device_aml());
> -    if (fdc) {
> -        aml_append(scope, build_fdc_device_aml(fdc));
> -    }
>  
>      if (ambiguous) {
>          error_report("Multiple ISA busses, unable to define IPMI ACPI data");
> diff --git a/stubs/cmos.c b/stubs/cmos.c
> new file mode 100644
> index 000000000000..416cbe4055ff
> --- /dev/null
> +++ b/stubs/cmos.c
> @@ -0,0 +1,7 @@
> +#include "qemu/osdep.h"
> +#include "hw/i386/pc.h"
> +
> +int cmos_get_fd_drive_type(FloppyDriveType fd0)
> +{
> +    return 0;
> +}
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index 45be5dc0ed78..3cbe472d1c6c 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -3,6 +3,7 @@ stub-obj-y += bdrv-next-monitor-owned.o
>  stub-obj-y += blk-commit-all.o
>  stub-obj-y += blockdev-close-all-bdrv-states.o
>  stub-obj-y += clock-warp.o
> +stub-obj-y += cmos.o
>  stub-obj-y += cpu-get-clock.o
>  stub-obj-y += cpu-get-icount.o
>  stub-obj-y += dump.o

Re: [PATCH v3 10/15] acpi: parallel: don't use _STA method

[Igor Mammedov]

On Wed, 29 Apr 2020 15:59:58 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> The _STA method dates back to the days where we had a static DSDT.  The
> device is listed in the DSDT table unconditionally and the _STA method
> checks a bit in the isa bridge pci config space to figure whenever a
> given is isa device is present or not, then evaluates to 0x0f (present)
> or 0x00 (absent).
> 
> These days the DSDT is generated by qemu anyway, so if a device is not
> present we can simply drop it from the DSDT instead.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>

looking more at it, we should also cleanup no longer used LPEN field
the same applies to similar fields for serial and ... 


> ---
>  hw/i386/acpi-build.c | 29 ++++++++---------------------
>  1 file changed, 8 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index fea83352e6ab..e01afbd011d9 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1167,39 +1167,26 @@ static Aml *build_mouse_device_aml(void)
>      return dev;
>  }
>  
> -static Aml *build_lpt_device_aml(void)
> +static void build_lpt_device_aml(Aml *scope)
>  {
>      Aml *dev;
>      Aml *crs;
> -    Aml *method;
> -    Aml *if_ctx;
> -    Aml *else_ctx;
> -    Aml *zero = aml_int(0);
> -    Aml *is_present = aml_local(0);
> +
> +    if (!memory_region_present(get_system_io(), 0x0378)) {
> +        return;
> +    }
>  
>      dev = aml_device("LPT");
>      aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
>  
> -    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> -    aml_append(method, aml_store(aml_name("LPEN"), is_present));
> -    if_ctx = aml_if(aml_equal(is_present, zero));
> -    {
> -        aml_append(if_ctx, aml_return(aml_int(0x00)));
> -    }
> -    aml_append(method, if_ctx);
> -    else_ctx = aml_else();
> -    {
> -        aml_append(else_ctx, aml_return(aml_int(0x0f)));
> -    }
> -    aml_append(method, else_ctx);
> -    aml_append(dev, method);
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
>  
>      crs = aml_resource_template();
>      aml_append(crs, aml_io(AML_DECODE16, 0x0378, 0x0378, 0x08, 0x08));
>      aml_append(crs, aml_irq_no_flags(7));
>      aml_append(dev, aml_name_decl("_CRS", crs));
>  
> -    return dev;
> +    aml_append(scope, dev);
>  }
>  
>  static void build_isa_devices_aml(Aml *table)
> @@ -1215,7 +1202,7 @@ static void build_isa_devices_aml(Aml *table)
>      if (fdc) {
>          aml_append(scope, build_fdc_device_aml(fdc));
>      }
> -    aml_append(scope, build_lpt_device_aml());
> +    build_lpt_device_aml(scope);
>  
>      if (ambiguous) {
>          error_report("Multiple ISA busses, unable to define IPMI ACPI data");

Re: [PATCH v3 13/15] acpi: move aml builder code for i8042 (kbd+mouse) device

[Igor Mammedov]

On Wed, 29 Apr 2020 16:00:01 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/i386/acpi-build.c | 39 ---------------------------------------
>  hw/input/pckbd.c     | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+), 39 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 7415bd5fdce0..643e875c05a5 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1052,42 +1052,6 @@ static void build_hpet_aml(Aml *table)
>      aml_append(table, scope);
>  }
>  
> -static Aml *build_kbd_device_aml(void)
> -{
> -    Aml *dev;
> -    Aml *crs;
> -
> -    dev = aml_device("KBD");
> -    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0303")));
> -
> -    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
> -
> -    crs = aml_resource_template();
> -    aml_append(crs, aml_io(AML_DECODE16, 0x0060, 0x0060, 0x01, 0x01));
> -    aml_append(crs, aml_io(AML_DECODE16, 0x0064, 0x0064, 0x01, 0x01));
> -    aml_append(crs, aml_irq_no_flags(1));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -
> -    return dev;
> -}
> -
> -static Aml *build_mouse_device_aml(void)
> -{
> -    Aml *dev;
> -    Aml *crs;
> -
> -    dev = aml_device("MOU");
> -    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0F13")));
> -
> -    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
> -
> -    crs = aml_resource_template();
> -    aml_append(crs, aml_irq_no_flags(12));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -
> -    return dev;
> -}
> -
>  static void build_isa_devices_aml(Aml *table)
>  {
>      bool ambiguous;
> @@ -1095,9 +1059,6 @@ static void build_isa_devices_aml(Aml *table)
>      Aml *scope = aml_scope("_SB.PCI0.ISA");
>      Object *obj = object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous);
>  
> -    aml_append(scope, build_kbd_device_aml());
> -    aml_append(scope, build_mouse_device_aml());
> -
>      if (ambiguous) {
>          error_report("Multiple ISA busses, unable to define IPMI ACPI data");
>      } else if (!obj) {
> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
> index 60a41303203a..29d633ca9478 100644
> --- a/hw/input/pckbd.c
> +++ b/hw/input/pckbd.c
> @@ -26,6 +26,7 @@
>  #include "qemu/log.h"
>  #include "hw/isa/isa.h"
>  #include "migration/vmstate.h"
> +#include "hw/acpi/aml-build.h"
>  #include "hw/input/ps2.h"
>  #include "hw/irq.h"
>  #include "hw/input/i8042.h"
> @@ -561,12 +562,42 @@ static void i8042_realizefn(DeviceState *dev, Error **errp)
>      qemu_register_reset(kbd_reset, s);
>  }
>  
> +static void i8042_build_aml(ISADevice *isadev, Aml *scope)
> +{
> +    Aml *kbd;
> +    Aml *mou;
> +    Aml *crs;
> +
> +    crs = aml_resource_template();
> +    aml_append(crs, aml_io(AML_DECODE16, 0x0060, 0x0060, 0x01, 0x01));
> +    aml_append(crs, aml_io(AML_DECODE16, 0x0064, 0x0064, 0x01, 0x01));
> +    aml_append(crs, aml_irq_no_flags(1));
> +
> +    kbd = aml_device("KBD");
> +    aml_append(kbd, aml_name_decl("_HID", aml_eisaid("PNP0303")));
> +    aml_append(kbd, aml_name_decl("_STA", aml_int(0xf)));
> +    aml_append(kbd, aml_name_decl("_CRS", crs));
> +
> +    crs = aml_resource_template();
> +    aml_append(crs, aml_irq_no_flags(12));
> +
> +    mou = aml_device("MOU");
> +    aml_append(mou, aml_name_decl("_HID", aml_eisaid("PNP0F13")));
> +    aml_append(mou, aml_name_decl("_STA", aml_int(0xf)));
> +    aml_append(mou, aml_name_decl("_CRS", crs));
> +
> +    aml_append(scope, kbd);
> +    aml_append(scope, mou);
> +}
> +
>  static void i8042_class_initfn(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    ISADeviceClass *isa = ISA_DEVICE_CLASS(klass);
>  
>      dc->realize = i8042_realizefn;
>      dc->vmsd = &vmstate_kbd_isa;
> +    isa->build_aml = i8042_build_aml;
>      set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
>  }
>  

Re: [PATCH v3 14/15] acpi: factor out fw_cfg_add_acpi_dsdt()

[Igor Mammedov]

On Wed, 29 Apr 2020 16:00:02 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Add helper function to add fw_cfg device,
> also move code to hw/i386/fw_cfg.c.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/i386/fw_cfg.h     |  1 +
>  hw/i386/acpi-build.c | 24 +-----------------------
>  hw/i386/fw_cfg.c     | 28 ++++++++++++++++++++++++++++
>  3 files changed, 30 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/i386/fw_cfg.h b/hw/i386/fw_cfg.h
> index 9e742787792b..275f15c1c5e8 100644
> --- a/hw/i386/fw_cfg.h
> +++ b/hw/i386/fw_cfg.h
> @@ -25,5 +25,6 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms,
>                                 uint16_t apic_id_limit);
>  void fw_cfg_build_smbios(MachineState *ms, FWCfgState *fw_cfg);
>  void fw_cfg_build_feature_control(MachineState *ms, FWCfgState *fw_cfg);
> +void fw_cfg_add_acpi_dsdt(Aml *scope, FWCfgState *fw_cfg);
>  
>  #endif
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 643e875c05a5..a8b790021974 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1874,30 +1874,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>  
>      /* create fw_cfg node, unconditionally */
>      {
> -        /* when using port i/o, the 8-bit data register *always* overlaps
> -         * with half of the 16-bit control register. Hence, the total size
> -         * of the i/o region used is FW_CFG_CTL_SIZE; when using DMA, the
> -         * DMA control register is located at FW_CFG_DMA_IO_BASE + 4 */
> -        uint8_t io_size = object_property_get_bool(OBJECT(x86ms->fw_cfg),
> -                                                   "dma_enabled", NULL) ?
> -                          ROUND_UP(FW_CFG_CTL_SIZE, 4) + sizeof(dma_addr_t) :
> -                          FW_CFG_CTL_SIZE;
> -
>          scope = aml_scope("\\_SB.PCI0");
> -        dev = aml_device("FWCF");
> -
> -        aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> -
> -        /* 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, FW_CFG_IO_BASE, FW_CFG_IO_BASE, 0x01, io_size)
> -        );
> -        aml_append(dev, aml_name_decl("_CRS", crs));
> -
> -        aml_append(scope, dev);
> +        fw_cfg_add_acpi_dsdt(scope, x86ms->fw_cfg);
>          aml_append(dsdt, scope);
>      }
>  
> diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
> index da60ada59462..c55abfb01abb 100644
> --- a/hw/i386/fw_cfg.c
> +++ b/hw/i386/fw_cfg.c
> @@ -15,6 +15,7 @@
>  #include "qemu/osdep.h"
>  #include "sysemu/numa.h"
>  #include "hw/acpi/acpi.h"
> +#include "hw/acpi/aml-build.h"
>  #include "hw/firmware/smbios.h"
>  #include "hw/i386/fw_cfg.h"
>  #include "hw/timer/hpet.h"
> @@ -179,3 +180,30 @@ void fw_cfg_build_feature_control(MachineState *ms, FWCfgState *fw_cfg)
>      *val = cpu_to_le64(feature_control_bits | FEATURE_CONTROL_LOCKED);
>      fw_cfg_add_file(fw_cfg, "etc/msr_feature_control", val, sizeof(*val));
>  }
> +
> +void fw_cfg_add_acpi_dsdt(Aml *scope, FWCfgState *fw_cfg)
> +{
> +    /*
> +     * when using port i/o, the 8-bit data register *always* overlaps
> +     * with half of the 16-bit control register. Hence, the total size
> +     * of the i/o region used is FW_CFG_CTL_SIZE; when using DMA, the
> +     * DMA control register is located at FW_CFG_DMA_IO_BASE + 4
> +     */
> +    Object *obj = OBJECT(fw_cfg);
> +    uint8_t io_size = object_property_get_bool(obj, "dma_enabled", NULL) ?
> +        ROUND_UP(FW_CFG_CTL_SIZE, 4) + sizeof(dma_addr_t) :
> +        FW_CFG_CTL_SIZE;
> +    Aml *dev = aml_device("FWCF");
> +    Aml *crs = aml_resource_template();
> +
> +    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> +
> +    /* device present, functioning, decoding, not shown in UI */
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> +
> +    aml_append(crs,
> +        aml_io(AML_DECODE16, FW_CFG_IO_BASE, FW_CFG_IO_BASE, 0x01, io_size));
> +
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +    aml_append(scope, dev);
> +}

Re: [PATCH v3 00/15] acpi: i386 tweaks

[Michael S. Tsirkin]

On Wed, Apr 29, 2020 at 03:59:48PM +0200, Gerd Hoffmann wrote:
> First batch of microvm patches, some generic acpi stuff.
> Split the acpi-build.c monster, specifically split the
> pc and q35 and pci bits into a separate file which we
> can skip building at some point in the future.

OK I applied 1st part of this that got acks.

> v2 changes: leave acpi-build.c largely as-is, move useful
> bits to other places to allow them being reused, specifically:
> 
>  * move isa device generator functions to individual isa devices.
>  * move fw_cfg generator function to fw_cfg.c
> 
> v3 changes: fix rtc, support multiple lpt devices.
> 
> take care,
>   Gerd
> 
> Gerd Hoffmann (15):
>   move 'typedef Aml' to qemu/types.h
>   acpi: add aml builder stubs
>   qtest: allow DSDT acpi table changes
>   acpi: drop pointless _STA method
>   acpi: add ISADeviceClass->build_aml()
>   rtc: add RTC_ISA_BASE
>   acpi: move aml builder code for rtc device
>   acpi: serial: don't use _STA method
>   acpi: move aml builder code for serial device
>   acpi: parallel: don't use _STA method
>   acpi: move aml builder code for parallel device
>   acpi: move aml builder code for floppy device
>   acpi: move aml builder code for i8042 (kbd+mouse) device
>   acpi: factor out fw_cfg_add_acpi_dsdt()
>   acpi: simplify build_isa_devices_aml()
> 
>  hw/i386/fw_cfg.h                            |   1 +
>  include/hw/acpi/aml-build.h                 |   1 -
>  include/hw/isa/isa.h                        |   2 +
>  include/hw/rtc/mc146818rtc.h                |   1 +
>  include/qemu/typedefs.h                     |   1 +
>  tests/qtest/bios-tables-test-allowed-diff.h |  17 ++
>  hw/acpi/aml-build-stub.c                    |  79 ++++++
>  hw/block/fdc.c                              |  83 ++++++
>  hw/char/parallel.c                          |  32 +++
>  hw/char/serial-isa.c                        |  32 +++
>  hw/i386/acpi-build.c                        | 271 +-------------------
>  hw/i386/fw_cfg.c                            |  28 ++
>  hw/input/pckbd.c                            |  31 +++
>  hw/isa/isa-bus.c                            |  15 ++
>  hw/rtc/mc146818rtc.c                        |  25 +-
>  stubs/cmos.c                                |   7 +
>  hw/acpi/Makefile.objs                       |   4 +-
>  stubs/Makefile.objs                         |   1 +
>  18 files changed, 361 insertions(+), 270 deletions(-)
>  create mode 100644 hw/acpi/aml-build-stub.c
>  create mode 100644 stubs/cmos.c
> 
> -- 
> 2.18.2

Re: [PATCH v3 08/15] acpi: serial: don't use _STA method

[Gerd Hoffmann]

> > -static Aml *build_com_device_aml(uint8_t uid)
> > +static void build_com_device_aml(Aml *scope, uint8_t uid)
> >   {
> >       Aml *dev;
> >       Aml *crs;
> > -    Aml *method;
> > -    Aml *if_ctx;
> > -    Aml *else_ctx;
> > -    Aml *zero = aml_int(0);
> > -    Aml *is_present = aml_local(0);
> > -    const char *enabled_field = "CAEN";
> >       uint8_t irq = 4;
> >       uint16_t io_port = 0x03F8;
> >       assert(uid == 1 || uid == 2);
> >       if (uid == 2) {
> > -        enabled_field = "CBEN";
> >           irq = 3;
> >           io_port = 0x02F8;
> >       }
> > +    if (!memory_region_present(get_system_io(), io_port)) {
> > +        return;
> > +    }
> 
> The patch looks OK, but an you split this check into a separate (previous?)
> patch please?

I don't think this belongs to a separate patch.  It is basically the
same check the lpc bridge is doing when filling the pci config space
(see hw/isa/lpc_ich9.c).  So this effectively maintains the existing
logic, only that we now check directly instead of letting the guest
check the pci config space bit via _STA method.

Also note this is only temporary for bisecting, the next patch in the
series moves the code to a device callback so this kind of "device
exists" check is not needed any more.

take care,
  Gerd

Re: [PATCH v3 10/15] acpi: parallel: don't use _STA method

[Gerd Hoffmann]

  Hi,

> Also, it may be worth a ISA_PARALLEL_IOBASE 0x378 definition cleanup like
> you did with the RTC.

Next patch in series deals with that.

take care,
  Gerd

Re: [PATCH v3 10/15] acpi: parallel: don't use _STA method

[Gerd Hoffmann]

On Thu, Apr 30, 2020 at 06:25:24PM +0200, Igor Mammedov wrote:
> On Wed, 29 Apr 2020 15:59:58 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> > The _STA method dates back to the days where we had a static DSDT.  The
> > device is listed in the DSDT table unconditionally and the _STA method
> > checks a bit in the isa bridge pci config space to figure whenever a
> > given is isa device is present or not, then evaluates to 0x0f (present)
> > or 0x00 (absent).
> > 
> > These days the DSDT is generated by qemu anyway, so if a device is not
> > present we can simply drop it from the DSDT instead.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> 
> looking more at it, we should also cleanup no longer used LPEN field
> the same applies to similar fields for serial and ... 

IIRC these bits are in the chipset specs so we should not remove them
from pci config space.

Removing from DSDT should be possible indeed (I guess this is what you
mean?)

take care,
  Gerd

Re: [PATCH v3 15/15] acpi: simplify build_isa_devices_aml()

[Gerd Hoffmann]

On Thu, Apr 30, 2020 at 08:48:31AM +0200, Philippe Mathieu-Daudé wrote:
> Cc'ing IPMI maintainer.
> 
> On 4/29/20 4:00 PM, Gerd Hoffmann wrote:
> > x86 machines can have a single ISA bus only.
> 
> I disagree with the comment.
> Machines can have multiple ISA bus.

Note *x86* machines.  Given x86 has a global io address space I still
think this is true.

On some !x86 archs where a mmio window on the bridge is used for the io
address space multiple isa busses are possible.  I'm not sure whenever
this is purely theoretical or whenever such machines exist in practice
and in case of the latter whenever qemu can emulate one.

> >   hw/i386/acpi-build.c | 15 +++++----------
         ^^^^
But in any case !x86 doesn't matter here ;)

take care,
  Gerd

Re: [PATCH v3 15/15] acpi: simplify build_isa_devices_aml()

[Philippe Mathieu-Daudé]

On 5/4/20 3:46 PM, Gerd Hoffmann wrote:
> On Thu, Apr 30, 2020 at 08:48:31AM +0200, Philippe Mathieu-Daudé wrote:
>> Cc'ing IPMI maintainer.
>>
>> On 4/29/20 4:00 PM, Gerd Hoffmann wrote:
>>> x86 machines can have a single ISA bus only.
>>
>> I disagree with the comment.
>> Machines can have multiple ISA bus.
> 
> Note *x86* machines.  Given x86 has a global io address space I still
> think this is true.
> 
> On some !x86 archs where a mmio window on the bridge is used for the io
> address space multiple isa busses are possible.  I'm not sure whenever
> this is purely theoretical or whenever such machines exist in practice
> and in case of the latter whenever qemu can emulate one.

It currently can't.

> 
>>>    hw/i386/acpi-build.c | 15 +++++----------
>           ^^^^
> But in any case !x86 doesn't matter here ;)

Oops I didn't notice...

> 
> take care,
>    Gerd
> 

Re: [PATCH v3 08/15] acpi: serial: don't use _STA method

[Philippe Mathieu-Daudé]

On 5/4/20 3:19 PM, Gerd Hoffmann wrote:
>>> -static Aml *build_com_device_aml(uint8_t uid)
>>> +static void build_com_device_aml(Aml *scope, uint8_t uid)
>>>    {
>>>        Aml *dev;
>>>        Aml *crs;
>>> -    Aml *method;
>>> -    Aml *if_ctx;
>>> -    Aml *else_ctx;
>>> -    Aml *zero = aml_int(0);
>>> -    Aml *is_present = aml_local(0);
>>> -    const char *enabled_field = "CAEN";
>>>        uint8_t irq = 4;
>>>        uint16_t io_port = 0x03F8;
>>>        assert(uid == 1 || uid == 2);
>>>        if (uid == 2) {
>>> -        enabled_field = "CBEN";
>>>            irq = 3;
>>>            io_port = 0x02F8;
>>>        }
>>> +    if (!memory_region_present(get_system_io(), io_port)) {
>>> +        return;
>>> +    }
>>
>> The patch looks OK, but an you split this check into a separate (previous?)
>> patch please?
> 
> I don't think this belongs to a separate patch.  It is basically the
> same check the lpc bridge is doing when filling the pci config space
> (see hw/isa/lpc_ich9.c).  So this effectively maintains the existing
> logic, only that we now check directly instead of letting the guest
> check the pci config space bit via _STA method.
> 
> Also note this is only temporary for bisecting, the next patch in the
> series moves the code to a device callback so this kind of "device
> exists" check is not needed any more.

Oh correct.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
> take care,
>    Gerd
> 

Re: [PATCH v3 10/15] acpi: parallel: don't use _STA method

[Igor Mammedov]

On Mon, 4 May 2020 15:25:16 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> On Thu, Apr 30, 2020 at 06:25:24PM +0200, Igor Mammedov wrote:
> > On Wed, 29 Apr 2020 15:59:58 +0200
> > Gerd Hoffmann <kraxel@redhat.com> wrote:
> >   
> > > The _STA method dates back to the days where we had a static DSDT.  The
> > > device is listed in the DSDT table unconditionally and the _STA method
> > > checks a bit in the isa bridge pci config space to figure whenever a
> > > given is isa device is present or not, then evaluates to 0x0f (present)
> > > or 0x00 (absent).
> > > 
> > > These days the DSDT is generated by qemu anyway, so if a device is not
> > > present we can simply drop it from the DSDT instead.
> > > 
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > Reviewed-by: Igor Mammedov <imammedo@redhat.com>  
> > 
> > looking more at it, we should also cleanup no longer used LPEN field
> > the same applies to similar fields for serial and ...   
> 
> IIRC these bits are in the chipset specs so we should not remove them
> from pci config space.
we also can't touch device model due to migration resons
(ACPI code that checks STA bits is in the wild, and us removing ACPI part
of it won't change that)

> 
> Removing from DSDT should be possible indeed (I guess this is what you
> mean?)
yep

> 
> take care,
>   Gerd
>