[PATCH v5 00/15] acpi: i386 tweaks

[PATCH v5 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.

v4 changes:
 * drop merged patches.
 * split rtc crs change to separata patch.
 * added two cleanup patches.
 * picked up ack & review tags.

v5 changes:
 * add comment for rtc crs update.
 * add even more cleanup patches.
 * picked up ack & review tags.

take care,
  Gerd

Gerd Hoffmann (15):
  qtest: allow DSDT acpi table changes
  acpi: move aml builder code for rtc device
  acpi: rtc: use a single crs range
  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()
  acpi: drop serial/parallel enable bits from dsdt
  acpi: drop build_piix4_pm()
  acpi: q35: drop _SB.PCI0.ISA.LPCD opregion.
  floppy: make isa_fdc_get_drive_max_chs static

 hw/i386/fw_cfg.h                            |   1 +
 include/hw/block/fdc.h                      |   2 -
 tests/qtest/bios-tables-test-allowed-diff.h |  17 ++
 hw/block/fdc.c                              |  87 +++++-
 hw/char/parallel.c                          |  22 ++
 hw/char/serial-isa.c                        |  22 ++
 hw/i386/acpi-build.c                        | 312 +-------------------
 hw/i386/fw_cfg.c                            |  28 ++
 hw/input/pckbd.c                            |  31 ++
 hw/rtc/mc146818rtc.c                        |  24 ++
 stubs/cmos.c                                |   7 +
 stubs/Makefile.objs                         |   1 +
 12 files changed, 244 insertions(+), 310 deletions(-)
 create mode 100644 stubs/cmos.c

-- 
2.18.4

[PATCH v5 01/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.4

[PATCH v5 02/15] acpi: move aml builder code for rtc device

[Gerd Hoffmann]

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 | 17 -----------------
 hw/rtc/mc146818rtc.c | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 2e15f6848e7e..0bfa2dd23fcc 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1137,22 +1137,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;
@@ -1278,7 +1262,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..2104e0aa3b14 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,34 @@ 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, 0x02));
+    aml_append(crs, aml_irq_no_flags(RTC_ISA_IRQ));
+    aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE + 2, RTC_ISA_BASE + 2,
+                           0x02, 0x06));
+
+    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.4

[PATCH v5 03/15] acpi: rtc: use a single crs range

[Gerd Hoffmann]

Use a single io range for _CRS instead of two,
following what real hardware does.

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

diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index 2104e0aa3b14..ab0cc59973b3 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -1013,12 +1013,14 @@ static void rtc_build_aml(ISADevice *isadev, Aml *scope)
     Aml *dev;
     Aml *crs;
 
+    /*
+     * Reserving 8 io ports here, following what physical hardware
+     * does, even though qemu only responds to the first two ports.
+     */
     crs = aml_resource_template();
     aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE, RTC_ISA_BASE,
-                           0x10, 0x02));
+                           0x01, 0x08));
     aml_append(crs, aml_irq_no_flags(RTC_ISA_IRQ));
-    aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE + 2, RTC_ISA_BASE + 2,
-                           0x02, 0x06));
 
     dev = aml_device("RTC");
     aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0B00")));
-- 
2.18.4

Re: [PATCH v5 03/15] acpi: rtc: use a single crs range

[Igor Mammedov]

On Thu,  7 May 2020 15:16:28 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Use a single io range for _CRS instead of two,
> following what real hardware does.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

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

> ---
>  hw/rtc/mc146818rtc.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
> index 2104e0aa3b14..ab0cc59973b3 100644
> --- a/hw/rtc/mc146818rtc.c
> +++ b/hw/rtc/mc146818rtc.c
> @@ -1013,12 +1013,14 @@ static void rtc_build_aml(ISADevice *isadev, Aml *scope)
>      Aml *dev;
>      Aml *crs;
>  
> +    /*
> +     * Reserving 8 io ports here, following what physical hardware
> +     * does, even though qemu only responds to the first two ports.
> +     */
>      crs = aml_resource_template();
>      aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE, RTC_ISA_BASE,
> -                           0x10, 0x02));
> +                           0x01, 0x08));
>      aml_append(crs, aml_irq_no_flags(RTC_ISA_IRQ));
> -    aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE + 2, RTC_ISA_BASE + 2,
> -                           0x02, 0x06));
>  
>      dev = aml_device("RTC");
>      aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0B00")));

Re: [PATCH v5 03/15] acpi: rtc: use a single crs range

[Cameron Esfahani]

Reviewed-by: Cameron Esfahani <dirty@apple.com>

Cameron Esfahani
dirty@apple.com

"Americans are very skilled at creating a custom meaning from something that's mass-produced."

Ann Powers


> On May 7, 2020, at 6:16 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> Use a single io range for _CRS instead of two,
> following what real hardware does.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/rtc/mc146818rtc.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
> index 2104e0aa3b14..ab0cc59973b3 100644
> --- a/hw/rtc/mc146818rtc.c
> +++ b/hw/rtc/mc146818rtc.c
> @@ -1013,12 +1013,14 @@ static void rtc_build_aml(ISADevice *isadev, Aml *scope)
>     Aml *dev;
>     Aml *crs;
> 
> +    /*
> +     * Reserving 8 io ports here, following what physical hardware
> +     * does, even though qemu only responds to the first two ports.
> +     */
>     crs = aml_resource_template();
>     aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE, RTC_ISA_BASE,
> -                           0x10, 0x02));
> +                           0x01, 0x08));
>     aml_append(crs, aml_irq_no_flags(RTC_ISA_IRQ));
> -    aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE + 2, RTC_ISA_BASE + 2,
> -                           0x02, 0x06));
> 
>     dev = aml_device("RTC");
>     aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0B00")));
> -- 
> 2.18.4
> 
> 

[PATCH v5 04/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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@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 0bfa2dd23fcc..3a82730a0d19 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1208,50 +1208,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)
@@ -1268,8 +1252,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.4

[PATCH v5 05/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 | 22 ++++++++++++++++++++++
 hw/i386/acpi-build.c | 32 --------------------------------
 2 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
index f9b6eed7833d..165e320e65f0 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,25 @@ 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);
+    Aml *dev;
+    Aml *crs;
+
+    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", isa->index + 1);
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0501")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(isa->index + 1)));
+    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 +123,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 3a82730a0d19..0e6a5151f4c3 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1208,36 +1208,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();
@@ -1252,8 +1222,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.4

Re: [PATCH v5 05/15] acpi: move aml builder code for serial device

[Philippe Mathieu-Daudé]

On 5/7/20 3:16 PM, Gerd Hoffmann wrote:
> 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>

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

> ---
>   hw/char/serial-isa.c | 22 ++++++++++++++++++++++
>   hw/i386/acpi-build.c | 32 --------------------------------
>   2 files changed, 22 insertions(+), 32 deletions(-)
> 
> diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
> index f9b6eed7833d..165e320e65f0 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,25 @@ 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);
> +    Aml *dev;
> +    Aml *crs;
> +
> +    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", isa->index + 1);
> +    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0501")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(isa->index + 1)));
> +    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 +123,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 3a82730a0d19..0e6a5151f4c3 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1208,36 +1208,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();
> @@ -1252,8 +1222,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");
> 

[PATCH v5 06/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 0e6a5151f4c3..2188a2b99d18 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1173,39 +1173,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)
@@ -1221,7 +1208,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.4

[PATCH v5 07/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>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/char/parallel.c   | 22 ++++++++++++++++++++++
 hw/i386/acpi-build.c | 23 -----------------------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index 8dd67d13759b..c0f34bf924ec 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,25 @@ 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);
+    Aml *dev;
+    Aml *crs;
+
+    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", isa->index + 1);
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(isa->index + 1)));
+    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 +644,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 2188a2b99d18..443db94deb5b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1173,28 +1173,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();
@@ -1208,7 +1186,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.4

Re: [PATCH v5 07/15] acpi: move aml builder code for parallel device

[Philippe Mathieu-Daudé]

On 5/7/20 3:16 PM, Gerd Hoffmann wrote:
> Also adds support for multiple LPT devices.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>

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

> ---
>   hw/char/parallel.c   | 22 ++++++++++++++++++++++
>   hw/i386/acpi-build.c | 23 -----------------------
>   2 files changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/char/parallel.c b/hw/char/parallel.c
> index 8dd67d13759b..c0f34bf924ec 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,25 @@ 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);
> +    Aml *dev;
> +    Aml *crs;
> +
> +    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", isa->index + 1);
> +    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(isa->index + 1)));
> +    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 +644,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 2188a2b99d18..443db94deb5b 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1173,28 +1173,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();
> @@ -1208,7 +1186,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");
> 

[PATCH v5 08/15] acpi: move aml builder code for floppy device

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@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 9628cc171ef8..40faa088b5f7 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"
@@ -2765,6 +2767,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,
@@ -2798,11 +2879,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 443db94deb5b..775936e28b9a 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1058,85 +1058,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;
@@ -1175,7 +1096,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");
@@ -1183,9 +1103,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.4

Re: [PATCH v5 08/15] acpi: move aml builder code for floppy device

[Philippe Mathieu-Daudé]

+Hervé

On 5/7/20 3:16 PM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@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 9628cc171ef8..40faa088b5f7 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"

I'd rather not see this target-specific header used in the generic 
device model... The culprit seems cmos_get_fd_drive_type(). Is the value 
really PC specific?

Hervé, do you know if such info & RTC nvram layout is used in other arch?

> +#include "hw/acpi/aml-build.h"
>   #include "hw/irq.h"
>   #include "hw/isa/isa.h"
>   #include "hw/qdev-properties.h"
> @@ -2765,6 +2767,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,
> @@ -2798,11 +2879,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 443db94deb5b..775936e28b9a 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1058,85 +1058,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;
> @@ -1175,7 +1096,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");
> @@ -1183,9 +1103,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 v5 08/15] acpi: move aml builder code for floppy device

[John Snow]

On 5/7/20 10:05 AM, Philippe Mathieu-Daudé wrote:
> +Hervé
> 
> On 5/7/20 3:16 PM, Gerd Hoffmann wrote:
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>>   hw/block/fdc.c       | 83 ++++++++++++++++++++++++++++++++++++++++++++

oh no...

>>   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 9628cc171ef8..40faa088b5f7 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"
> 
> I'd rather not see this target-specific header used in the generic
> device model... The culprit seems cmos_get_fd_drive_type(). Is the value
> really PC specific?
> 
> Hervé, do you know if such info & RTC nvram layout is used in other arch?
> 

do we need pc-only floppy code somewhere to keep the makefile pure?
that's fine too.

>> +#include "hw/acpi/aml-build.h"
>>   #include "hw/irq.h"
>>   #include "hw/isa/isa.h"
>>   #include "hw/qdev-properties.h"
>> @@ -2765,6 +2767,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,
>> @@ -2798,11 +2879,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 443db94deb5b..775936e28b9a 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -1058,85 +1058,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;
>> @@ -1175,7 +1096,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");
>> @@ -1183,9 +1103,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
>>
> 

-- 
—js

Re: [PATCH v5 08/15] acpi: move aml builder code for floppy device

[Thomas Huth]

On 13/05/2020 22.43, John Snow wrote:
> 
> 
> On 5/7/20 10:05 AM, Philippe Mathieu-Daudé wrote:
>> +Hervé
>>
>> On 5/7/20 3:16 PM, Gerd Hoffmann wrote:
>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>>> ---
>>>   hw/block/fdc.c       | 83 ++++++++++++++++++++++++++++++++++++++++++++
> 
> oh no...
> 
>>>   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 9628cc171ef8..40faa088b5f7 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"
>>
>> I'd rather not see this target-specific header used in the generic
>> device model... The culprit seems cmos_get_fd_drive_type(). Is the value
>> really PC specific?
>>
>> Hervé, do you know if such info & RTC nvram layout is used in other arch?
>>
> 
> do we need pc-only floppy code somewhere to keep the makefile pure?

Yes, please put this into a separate file that only gets build if the
corresponding CONFIG switches are set. That way you should also get
along without that new stubs/cmos.c file.

 Thomas

Re: [PATCH v5 08/15] acpi: move aml builder code for floppy device

[Gerd Hoffmann]

> > +#include "hw/i386/pc.h"
> 
> I'd rather not see this target-specific header used in the generic device
> model... The culprit seems cmos_get_fd_drive_type(). Is the value really PC
> specific?

Given that the same value is used in acpi tables probably not really.
cmos_get_fd_drive_type() hasn't any dependency on pc or cmos btw, we
can simply move over the function to floppy.

> 
> Hervé, do you know if such info & RTC nvram layout is used in other arch?
> 
> > +#include "hw/acpi/aml-build.h"
> >   #include "hw/irq.h"
> >   #include "hw/isa/isa.h"
> >   #include "hw/qdev-properties.h"
> > @@ -2765,6 +2767,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,
> > @@ -2798,11 +2879,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 443db94deb5b..775936e28b9a 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1058,85 +1058,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;
> > @@ -1175,7 +1096,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");
> > @@ -1183,9 +1103,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
> > 
> 

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

[Gerd Hoffmann]

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/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 775936e28b9a..829e20664fff 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1058,42 +1058,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;
@@ -1101,9 +1065,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.4

[PATCH v5 10/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>
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 829e20664fff..cb3913d2ee76 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1880,30 +1880,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.4

[PATCH v5 11/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>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@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 cb3913d2ee76..1922868f3401 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1061,19 +1061,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.4

[PATCH v5 12/15] acpi: drop serial/parallel enable bits from dsdt

[Gerd Hoffmann]

The _STA methods for COM+LPT used to reference them,
but that isn't the case any more.

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

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1922868f3401..765409a90eb6 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1437,15 +1437,6 @@ static void build_q35_isa_bridge(Aml *table)
     aml_append(field, aml_named_field("LPTD", 2));
     aml_append(dev, field);
 
-    aml_append(dev, aml_operation_region("LPCE", AML_PCI_CONFIG,
-                                         aml_int(0x82), 0x02));
-    /* enable bits */
-    field = aml_field("LPCE", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
-    aml_append(field, aml_named_field("CAEN", 1));
-    aml_append(field, aml_named_field("CBEN", 1));
-    aml_append(field, aml_named_field("LPEN", 1));
-    aml_append(dev, field);
-
     aml_append(scope, dev);
     aml_append(table, scope);
 }
@@ -1469,7 +1460,6 @@ static void build_piix4_isa_bridge(Aml *table)
 {
     Aml *dev;
     Aml *scope;
-    Aml *field;
 
     scope =  aml_scope("_SB.PCI0");
     dev = aml_device("ISA");
@@ -1478,19 +1468,6 @@ static void build_piix4_isa_bridge(Aml *table)
     /* PIIX PCI to ISA irq remapping */
     aml_append(dev, aml_operation_region("P40C", AML_PCI_CONFIG,
                                          aml_int(0x60), 0x04));
-    /* enable bits */
-    field = aml_field("^PX13.P13C", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
-    /* Offset(0x5f),, 7, */
-    aml_append(field, aml_reserved_field(0x2f8));
-    aml_append(field, aml_reserved_field(7));
-    aml_append(field, aml_named_field("LPEN", 1));
-    /* Offset(0x67),, 3, */
-    aml_append(field, aml_reserved_field(0x38));
-    aml_append(field, aml_reserved_field(3));
-    aml_append(field, aml_named_field("CAEN", 1));
-    aml_append(field, aml_reserved_field(3));
-    aml_append(field, aml_named_field("CBEN", 1));
-    aml_append(dev, field);
 
     aml_append(scope, dev);
     aml_append(table, scope);
-- 
2.18.4

Re: [PATCH v5 12/15] acpi: drop serial/parallel enable bits from dsdt

[Igor Mammedov]

On Thu,  7 May 2020 15:16:37 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> The _STA methods for COM+LPT used to reference them,
> but that isn't the case any more.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

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


> ---
>  hw/i386/acpi-build.c | 23 -----------------------
>  1 file changed, 23 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 1922868f3401..765409a90eb6 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1437,15 +1437,6 @@ static void build_q35_isa_bridge(Aml *table)
>      aml_append(field, aml_named_field("LPTD", 2));
>      aml_append(dev, field);
>  
> -    aml_append(dev, aml_operation_region("LPCE", AML_PCI_CONFIG,
> -                                         aml_int(0x82), 0x02));
> -    /* enable bits */
> -    field = aml_field("LPCE", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> -    aml_append(field, aml_named_field("CAEN", 1));
> -    aml_append(field, aml_named_field("CBEN", 1));
> -    aml_append(field, aml_named_field("LPEN", 1));
> -    aml_append(dev, field);
> -
>      aml_append(scope, dev);
>      aml_append(table, scope);
>  }
> @@ -1469,7 +1460,6 @@ static void build_piix4_isa_bridge(Aml *table)
>  {
>      Aml *dev;
>      Aml *scope;
> -    Aml *field;
>  
>      scope =  aml_scope("_SB.PCI0");
>      dev = aml_device("ISA");
> @@ -1478,19 +1468,6 @@ static void build_piix4_isa_bridge(Aml *table)
>      /* PIIX PCI to ISA irq remapping */
>      aml_append(dev, aml_operation_region("P40C", AML_PCI_CONFIG,
>                                           aml_int(0x60), 0x04));
> -    /* enable bits */
> -    field = aml_field("^PX13.P13C", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> -    /* Offset(0x5f),, 7, */
> -    aml_append(field, aml_reserved_field(0x2f8));
> -    aml_append(field, aml_reserved_field(7));
> -    aml_append(field, aml_named_field("LPEN", 1));
> -    /* Offset(0x67),, 3, */
> -    aml_append(field, aml_reserved_field(0x38));
> -    aml_append(field, aml_reserved_field(3));
> -    aml_append(field, aml_named_field("CAEN", 1));
> -    aml_append(field, aml_reserved_field(3));
> -    aml_append(field, aml_named_field("CBEN", 1));
> -    aml_append(dev, field);
>  
>      aml_append(scope, dev);
>      aml_append(table, scope);

[PATCH v5 13/15] acpi: drop build_piix4_pm()

[Gerd Hoffmann]

The _SB.PCI0.PX13.P13C opregion (holds isa device enable bits)
is not used any more, remove it from DSDT.

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

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 765409a90eb6..c1e63cce5e8e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1441,21 +1441,6 @@ static void build_q35_isa_bridge(Aml *table)
     aml_append(table, scope);
 }
 
-static void build_piix4_pm(Aml *table)
-{
-    Aml *dev;
-    Aml *scope;
-
-    scope =  aml_scope("_SB.PCI0");
-    dev = aml_device("PX13");
-    aml_append(dev, aml_name_decl("_ADR", aml_int(0x00010003)));
-
-    aml_append(dev, aml_operation_region("P13C", AML_PCI_CONFIG,
-                                         aml_int(0x00), 0xff));
-    aml_append(scope, dev);
-    aml_append(table, scope);
-}
-
 static void build_piix4_isa_bridge(Aml *table)
 {
     Aml *dev;
@@ -1607,7 +1592,6 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         aml_append(dsdt, sb_scope);
 
         build_hpet_aml(dsdt);
-        build_piix4_pm(dsdt);
         build_piix4_isa_bridge(dsdt);
         build_isa_devices_aml(dsdt);
         build_piix4_pci_hotplug(dsdt);
-- 
2.18.4

Re: [PATCH v5 13/15] acpi: drop build_piix4_pm()

[Igor Mammedov]

On Thu,  7 May 2020 15:16:38 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> The _SB.PCI0.PX13.P13C opregion (holds isa device enable bits)
> is not used any more, remove it from DSDT.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/i386/acpi-build.c | 16 ----------------
>  1 file changed, 16 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 765409a90eb6..c1e63cce5e8e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1441,21 +1441,6 @@ static void build_q35_isa_bridge(Aml *table)
>      aml_append(table, scope);
>  }
>  
> -static void build_piix4_pm(Aml *table)
> -{
> -    Aml *dev;
> -    Aml *scope;
> -
> -    scope =  aml_scope("_SB.PCI0");
> -    dev = aml_device("PX13");
> -    aml_append(dev, aml_name_decl("_ADR", aml_int(0x00010003)));
I agree about removing P13C but I'm not sure if it's safe to remove
whole isa bridge

> -
> -    aml_append(dev, aml_operation_region("P13C", AML_PCI_CONFIG,
> -                                         aml_int(0x00), 0xff));
> -    aml_append(scope, dev);
> -    aml_append(table, scope);
> -}
> -
>  static void build_piix4_isa_bridge(Aml *table)
>  {
>      Aml *dev;
> @@ -1607,7 +1592,6 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          aml_append(dsdt, sb_scope);
>  
>          build_hpet_aml(dsdt);
> -        build_piix4_pm(dsdt);
>          build_piix4_isa_bridge(dsdt);
>          build_isa_devices_aml(dsdt);
>          build_piix4_pci_hotplug(dsdt);

Re: [PATCH v5 13/15] acpi: drop build_piix4_pm()

[Gerd Hoffmann]

On Mon, May 11, 2020 at 09:37:32PM +0200, Igor Mammedov wrote:
> On Thu,  7 May 2020 15:16:38 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> > The _SB.PCI0.PX13.P13C opregion (holds isa device enable bits)
> > is not used any more, remove it from DSDT.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  hw/i386/acpi-build.c | 16 ----------------
> >  1 file changed, 16 deletions(-)
> > 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 765409a90eb6..c1e63cce5e8e 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1441,21 +1441,6 @@ static void build_q35_isa_bridge(Aml *table)
> >      aml_append(table, scope);
> >  }
> >  
> > -static void build_piix4_pm(Aml *table)
> > -{
> > -    Aml *dev;
> > -    Aml *scope;
> > -
> > -    scope =  aml_scope("_SB.PCI0");
> > -    dev = aml_device("PX13");
> > -    aml_append(dev, aml_name_decl("_ADR", aml_int(0x00010003)));
> I agree about removing P13C but I'm not sure if it's safe to remove
> whole isa bridge

The isa bridge is _SB0.PCI0.ISA (piix4 function 0).
_SB.PCI0.PX13 is the acpi pm device (piix4 function 3).

So this does _not_ remove the isa bridge.  And given that PX13 has
nothing declared beside the opregion we are removing and I have not
found any other references to the PX13 device I assumed it is ok to drop
that one too.  Didn't notice any odd side effects in testing.

take care,
  Gerd

Re: [PATCH v5 13/15] acpi: drop build_piix4_pm()

[Igor Mammedov]

On Tue, 12 May 2020 13:16:05 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> On Mon, May 11, 2020 at 09:37:32PM +0200, Igor Mammedov wrote:
> > On Thu,  7 May 2020 15:16:38 +0200
> > Gerd Hoffmann <kraxel@redhat.com> wrote:
> >   
> > > The _SB.PCI0.PX13.P13C opregion (holds isa device enable bits)
> > > is not used any more, remove it from DSDT.
> > > 
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > ---
> > >  hw/i386/acpi-build.c | 16 ----------------
> > >  1 file changed, 16 deletions(-)
> > > 
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index 765409a90eb6..c1e63cce5e8e 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -1441,21 +1441,6 @@ static void build_q35_isa_bridge(Aml *table)
> > >      aml_append(table, scope);
> > >  }
> > >  
> > > -static void build_piix4_pm(Aml *table)
> > > -{
> > > -    Aml *dev;
> > > -    Aml *scope;
> > > -
> > > -    scope =  aml_scope("_SB.PCI0");
> > > -    dev = aml_device("PX13");
> > > -    aml_append(dev, aml_name_decl("_ADR", aml_int(0x00010003)));  
> > I agree about removing P13C but I'm not sure if it's safe to remove
> > whole isa bridge  
> 
> The isa bridge is _SB0.PCI0.ISA (piix4 function 0).
> _SB.PCI0.PX13 is the acpi pm device (piix4 function 3).
> 
> So this does _not_ remove the isa bridge.  And given that PX13 has
> nothing declared beside the opregion we are removing and I have not
> found any other references to the PX13 device I assumed it is ok to drop
> that one too.  Didn't notice any odd side effects in testing.
I don't see HID or some other way to tell guest that it should load a driver
for it so probably we can remove it.
If it breaks some legacy OS we can always add it back.


> 
> take care,
>   Gerd
> 

[PATCH v5 14/15] acpi: q35: drop _SB.PCI0.ISA.LPCD opregion.

[Gerd Hoffmann]

Seems to be unused.

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

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index c1e63cce5e8e..1afb47b09ee9 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1417,7 +1417,6 @@ static void build_q35_isa_bridge(Aml *table)
 {
     Aml *dev;
     Aml *scope;
-    Aml *field;
 
     scope =  aml_scope("_SB.PCI0");
     dev = aml_device("ISA");
@@ -1427,16 +1426,6 @@ static void build_q35_isa_bridge(Aml *table)
     aml_append(dev, aml_operation_region("PIRQ", AML_PCI_CONFIG,
                                          aml_int(0x60), 0x0C));
 
-    aml_append(dev, aml_operation_region("LPCD", AML_PCI_CONFIG,
-                                         aml_int(0x80), 0x02));
-    field = aml_field("LPCD", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
-    aml_append(field, aml_named_field("COMA", 3));
-    aml_append(field, aml_reserved_field(1));
-    aml_append(field, aml_named_field("COMB", 3));
-    aml_append(field, aml_reserved_field(1));
-    aml_append(field, aml_named_field("LPTD", 2));
-    aml_append(dev, field);
-
     aml_append(scope, dev);
     aml_append(table, scope);
 }
-- 
2.18.4

Re: [PATCH v5 14/15] acpi: q35: drop _SB.PCI0.ISA.LPCD opregion.

[Igor Mammedov]

On Thu,  7 May 2020 15:16:39 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Seems to be unused.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

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

> ---
>  hw/i386/acpi-build.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index c1e63cce5e8e..1afb47b09ee9 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1417,7 +1417,6 @@ static void build_q35_isa_bridge(Aml *table)
>  {
>      Aml *dev;
>      Aml *scope;
> -    Aml *field;
>  
>      scope =  aml_scope("_SB.PCI0");
>      dev = aml_device("ISA");
> @@ -1427,16 +1426,6 @@ static void build_q35_isa_bridge(Aml *table)
>      aml_append(dev, aml_operation_region("PIRQ", AML_PCI_CONFIG,
>                                           aml_int(0x60), 0x0C));
>  
> -    aml_append(dev, aml_operation_region("LPCD", AML_PCI_CONFIG,
> -                                         aml_int(0x80), 0x02));
> -    field = aml_field("LPCD", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> -    aml_append(field, aml_named_field("COMA", 3));
> -    aml_append(field, aml_reserved_field(1));
> -    aml_append(field, aml_named_field("COMB", 3));
> -    aml_append(field, aml_reserved_field(1));
> -    aml_append(field, aml_named_field("LPTD", 2));
> -    aml_append(dev, field);
> -
>      aml_append(scope, dev);
>      aml_append(table, scope);
>  }

[PATCH v5 15/15] floppy: make isa_fdc_get_drive_max_chs static

[Gerd Hoffmann]

acpi aml generator needs this, but it is in floppy code now
so we can make the function static.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/block/fdc.h | 2 --
 hw/block/fdc.c         | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
index c15ff4c62315..5d71cf972268 100644
--- a/include/hw/block/fdc.h
+++ b/include/hw/block/fdc.h
@@ -16,7 +16,5 @@ void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
                        DriveInfo **fds, qemu_irq *fdc_tc);
 
 FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
-void isa_fdc_get_drive_max_chs(FloppyDriveType type,
-                               uint8_t *maxc, uint8_t *maxh, uint8_t *maxs);
 
 #endif
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 40faa088b5f7..499a580b993c 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2744,8 +2744,8 @@ FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
     return isa->state.drives[i].drive;
 }
 
-void isa_fdc_get_drive_max_chs(FloppyDriveType type,
-                               uint8_t *maxc, uint8_t *maxh, uint8_t *maxs)
+static void isa_fdc_get_drive_max_chs(FloppyDriveType type, uint8_t *maxc,
+                                      uint8_t *maxh, uint8_t *maxs)
 {
     const FDFormat *fdf;
 
-- 
2.18.4

Re: [PATCH v5 15/15] floppy: make isa_fdc_get_drive_max_chs static

[Philippe Mathieu-Daudé]

On 5/7/20 3:16 PM, Gerd Hoffmann wrote:
> acpi aml generator needs this, but it is in floppy code now
> so we can make the function static.
> 
> 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/block/fdc.h | 2 --
>   hw/block/fdc.c         | 4 ++--
>   2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
> index c15ff4c62315..5d71cf972268 100644
> --- a/include/hw/block/fdc.h
> +++ b/include/hw/block/fdc.h
> @@ -16,7 +16,5 @@ void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
>                          DriveInfo **fds, qemu_irq *fdc_tc);
>   
>   FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
> -void isa_fdc_get_drive_max_chs(FloppyDriveType type,
> -                               uint8_t *maxc, uint8_t *maxh, uint8_t *maxs);
>   
>   #endif
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 40faa088b5f7..499a580b993c 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -2744,8 +2744,8 @@ FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
>       return isa->state.drives[i].drive;
>   }
>   
> -void isa_fdc_get_drive_max_chs(FloppyDriveType type,
> -                               uint8_t *maxc, uint8_t *maxh, uint8_t *maxs)
> +static void isa_fdc_get_drive_max_chs(FloppyDriveType type, uint8_t *maxc,
> +                                      uint8_t *maxh, uint8_t *maxs)
>   {
>       const FDFormat *fdf;
>   
> 

Re: [PATCH v5 15/15] floppy: make isa_fdc_get_drive_max_chs static

[John Snow]

On 5/7/20 10:06 AM, Philippe Mathieu-Daudé wrote:
> On 5/7/20 3:16 PM, Gerd Hoffmann wrote:
>> acpi aml generator needs this, but it is in floppy code now
>> so we can make the function static.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Acked-by: John Snow <jsnow@redhat.com>

I trust Philippe :)