qemu-arm.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-arm] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support
@ 2015-12-09 23:40 Igor Mammedov
  2015-12-09 23:41 ` [Qemu-arm] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs Igor Mammedov
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Igor Mammedov @ 2015-12-09 23:40 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
	open list:ARM ACPI Subsystem, Shannon Zhao, Paolo Bonzini,
	Richard Henderson

Due to huge size, CCing only cover letter instead of individual patches.

Series consist of 2 parts the 1st part prefixed 'acpi:' adds necessary
AML API functions and the second part converts DSDT using existing and
new AML API.

Series does exact byte by byte conversion and passes ACPI tables
'make check' tests.
The conversion first moves common for PIIX4/Q35 parts, getting rid of *.dsl
includes and then converts PIIX4 and Q35 parts of DSDT.

Diff-stat looks nice but actual code base is reduced by ~2000LOC
while the rest of 10000 removals is dropping precompiled AML
templates from tree.

There are some AML parts that could be optimized/simplified and shared
between PIIX4/Q35/ARM but doing it will break exact match with original
tests, hence it's left out of the scope of this series.

CC: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
CC: Shannon Zhao <zhaoshenglong@huawei.com> (maintainer:ARM ACPI Subsystem)
CC: Peter Maydell <peter.maydell@linaro.org> (maintainer:ARM)
CC: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86)
CC: Richard Henderson <rth@twiddle.net> (maintainer:X86)
CC: Eduardo Habkost <ehabkost@redhat.com> (maintainer:X86)
CC: qemu-arm@nongnu.org (open list:ARM ACPI Subsystem)

Igor Mammedov (70):
  tests: acpi: print ASL diff in verbose mode
  acpi: add aml_lgreater_equal()
  acpi: add aml_create_qword_field()
  acpi: aml: add helper for Opcode Arg2 Arg2 [Dst] AML pattern
  acpi: extend aml_add() to accept target argument
  acpi: add aml_decrement() and aml_subtract()
  acpi: add aml_call0() helper
  acpi: add aml_to_integer()
  acpi: extend aml_shiftright() to accept target argument
  acpi: add aml_alias()
  acpi: add aml_sleep()
  acpi: add aml_lor()
  acpi: add aml_lgreater()
  acpi: extend aml_field() to support LockRule
  acpi: add aml_to_hexstring()
  acpi: add aml_to_buffer()
  acpi add aml_dma()
  acpi: extend aml_or() to accept target argument
  acpi: extend aml_and() to accept target argument
  acpi: extend aml_interrupt() to support multiple irqs
  pc: acpi: memhp: prepare context in SSDT for moving memhp DSDT code
  pc: acpi: memhp: move MHPD._STA method into SSDT
  pc: acpi: memhp: move MHPD.MLCK mutex into SSDT
  pc: acpi: memhp: move MHPD.MSCN method into SSDT
  pc: acpi: memhp: move MHPD.MRST method into SSDT
  pc: acpi: memhp: move MHPD.MPXM method into SSDT
  pc: acpi: memhp: move MHPD.MOST method into SSDT
  pc: acpi: memhp: move MHPD.MEJ0 method into SSDT
  pc: acpi: memhp: move MHPD.MCRS method into SSDT
  pc: acpi: memhp: move MHPD Device into SSDT
  pc: acpi: factor out memhp code from build_ssdt() into separate
    function
  pc: acpi: memhp: move \_GPE._E03 into SSDT
  pc: acpi: memhp: drop not needed stringify(MEMORY_foo) usage
  pc: acpi: drop unused CPU_STATUS_LEN from DSDT
  pc: acpi: cpuhp: move CPEJ() method to SSDT
  pc: acpi: cpuhp: move CPMA() method into SSDT
  pc: acpi: cpuhp: move CPST() method into SSDT
  pc: acpi: cpuhp: move PRSC() method into SSDT
  pc: acpi: cpuhp: move \_GPE._E02() into SSDT
  pc: acpi: factor out cpu hotplug code from build_ssdt() into separate
    function
  pc: acpi: move HPET from DSDT to SSDT
  pc: acpi: move DBUG() from DSDT to SSDT
  pc: acpi: move RTC device from DSDT to SSDT
  pc: acpi: move KBD device from DSDT to SSDT
  pc: acpi: move MOU device from DSDT to SSDT
  pc: acpi: move FDC0 device from DSDT to SSDT
  pc: acpi: move LPT device from DSDT to SSDT
  pc: acpi: move COM devices from DSDT to SSDT
  pc: acpi: move PIIX4 isa-bridge and pm devices into SSDT
  pc: acpi: move remaining GPE handlers into SSDT
  pc: acpi: pci: move link devices into SSDT
  pc: acpi: piix4: move IQCR() into SSDT
  pc: acpi: piix4: move IQST() into SSDT
  pc: acpi: piix4: move PCI0._PRT() into SSDT
  pc: acpi: piix4: move remaining PCI hotplug bits into SSDT
  pc: acpi: piix4: acpi move PCI0 device to SSDT
  pc: acpi: q35: move GSI links to SSDT
  pc: acpi: q35: move link devices to SSDT
  pc: acpi: q35: move IQCR() into SSDT
  pc: acpi: q35: move IQST() into SSDT
  pc: acpi: q35: move ISA bridge into SSDT
  pc: acpi: q35: move _PRT() into SSDT
  pc: acpi: q35: move PRTA routing table into SSDT
  pc: acpi: q35: move PRTP routing table into SSDT
  pc: acpi: q35: move _PIC() method into SSDT
  pc: acpi: q35: move PCI0._OSC() method into SSDT
  pc: acpi: q35: move PCI0 device definition into SSDT
  pc: acpi: q35: PCST, PCSB opregions and PCIB field into SSDT
  pc: acpi: switch to AML API composed DSDT
  pc: acpi: remove unused ASL templates and related blobs/utils

Xiao Guangrong (4):
  acpi: add aml_derefof
  acpi: add aml_sizeof
  acpi: add aml_mutex(), aml_acquire(), aml_release()
  acpi: support serialized method

 hw/acpi/Makefile.objs               |    4 +-
 hw/acpi/aml-build.c                 |  294 +-
 hw/acpi/cpu_hotplug_acpi_table.c    |  124 +
 hw/acpi/memory_hotplug_acpi_table.c |  249 ++
 hw/arm/virt-acpi-build.c            |   41 +-
 hw/i386/Makefile.objs               |   31 +-
 hw/i386/acpi-build.c                | 1329 ++++--
 hw/i386/acpi-dsdt-cpu-hotplug.dsl   |   90 -
 hw/i386/acpi-dsdt-dbug.dsl          |   41 -
 hw/i386/acpi-dsdt-hpet.dsl          |   48 -
 hw/i386/acpi-dsdt-isa.dsl           |  117 -
 hw/i386/acpi-dsdt-mem-hotplug.dsl   |  171 -
 hw/i386/acpi-dsdt.dsl               |  303 --
 hw/i386/acpi-dsdt.hex.generated     | 2972 --------------
 hw/i386/q35-acpi-dsdt.dsl           |  436 --
 hw/i386/q35-acpi-dsdt.hex.generated | 7610 -----------------------------------
 hw/timer/hpet.c                     |    2 +-
 include/hw/acpi/aml-build.h         |   66 +-
 include/hw/acpi/cpu_hotplug.h       |   10 +
 include/hw/acpi/memory_hotplug.h    |    9 +
 include/hw/acpi/pc-hotplug.h        |   44 +-
 include/hw/timer/hpet.h             |    1 +
 scripts/acpi_extract.py             |  367 --
 scripts/acpi_extract_preprocess.py  |   51 -
 scripts/update-acpi.sh              |    4 -
 tests/bios-tables-test.c            |    7 +
 26 files changed, 1848 insertions(+), 12573 deletions(-)
 create mode 100644 hw/acpi/cpu_hotplug_acpi_table.c
 create mode 100644 hw/acpi/memory_hotplug_acpi_table.c
 delete mode 100644 hw/i386/acpi-dsdt-cpu-hotplug.dsl
 delete mode 100644 hw/i386/acpi-dsdt-dbug.dsl
 delete mode 100644 hw/i386/acpi-dsdt-hpet.dsl
 delete mode 100644 hw/i386/acpi-dsdt-isa.dsl
 delete mode 100644 hw/i386/acpi-dsdt-mem-hotplug.dsl
 delete mode 100644 hw/i386/acpi-dsdt.dsl
 delete mode 100644 hw/i386/acpi-dsdt.hex.generated
 delete mode 100644 hw/i386/q35-acpi-dsdt.dsl
 delete mode 100644 hw/i386/q35-acpi-dsdt.hex.generated
 delete mode 100755 scripts/acpi_extract.py
 delete mode 100755 scripts/acpi_extract_preprocess.py
 delete mode 100644 scripts/update-acpi.sh

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-arm] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs
  2015-12-09 23:40 [Qemu-arm] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support Igor Mammedov
@ 2015-12-09 23:41 ` Igor Mammedov
  2015-12-10  1:50   ` Shannon Zhao
  2015-12-22 15:17   ` Michael S. Tsirkin
  2015-12-10 15:53 ` [Qemu-devel] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support Marcel Apfelbaum
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 15+ messages in thread
From: Igor Mammedov @ 2015-12-09 23:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-arm, zhaoshenglong

ASL Interrupt() macro translates to Extended Interrupt Descriptor
which supports variable number of IRQs. It will be used for
conversion of ASL code for pc/q35 machines that use it for
returning several IRQs in _PSR object.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
CC: zhaoshenglong@huawei.com
CC: qemu-arm@nongnu.org
---
 hw/acpi/aml-build.c         | 22 +++++++++++++---------
 hw/arm/virt-acpi-build.c    | 23 ++++++++++++-----------
 include/hw/acpi/aml-build.h |  2 +-
 3 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 2ca9207..ee64d12 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -667,23 +667,27 @@ Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
 Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
                    AmlLevelAndEdge level_and_edge,
                    AmlActiveHighAndLow high_and_low, AmlShared shared,
-                   uint32_t irq)
+                   uint32_t *irq_list, uint8_t irq_count)
 {
+    int i;
     Aml *var = aml_alloc();
     uint8_t irq_flags = con_and_pro | (level_and_edge << 1)
                         | (high_and_low << 2) | (shared << 3);
+    const int header_bytes_in_len = 2;
+    uint16_t len = header_bytes_in_len + irq_count * sizeof(uint32_t);
+
+    assert(irq_count > 0);
 
     build_append_byte(var->buf, 0x89); /* Extended irq descriptor */
-    build_append_byte(var->buf, 6); /* Length, bits[7:0] minimum value = 6 */
-    build_append_byte(var->buf, 0); /* Length, bits[15:8] minimum value = 0 */
+    build_append_byte(var->buf, len & 0xFF); /* Length, bits[7:0] */
+    build_append_byte(var->buf, len >> 8); /* Length, bits[15:8] */
     build_append_byte(var->buf, irq_flags); /* Interrupt Vector Information. */
-    build_append_byte(var->buf, 0x01);      /* Interrupt table length = 1 */
+    build_append_byte(var->buf, irq_count);   /* Interrupt table length */
 
-    /* Interrupt Number */
-    build_append_byte(var->buf, extract32(irq, 0, 8));  /* bits[7:0] */
-    build_append_byte(var->buf, extract32(irq, 8, 8));  /* bits[15:8] */
-    build_append_byte(var->buf, extract32(irq, 16, 8)); /* bits[23:16] */
-    build_append_byte(var->buf, extract32(irq, 24, 8)); /* bits[31:24] */
+    /* Interrupt Number List */
+    for (i = 0; i < irq_count; i++) {
+        build_append_int_noprefix(var->buf, irq_list[i], 4);
+    }
     return var;
 }
 
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 698b5f2..02ba822 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -71,7 +71,7 @@ static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
 }
 
 static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
-                                           int uart_irq)
+                                           uint32_t uart_irq)
 {
     Aml *dev = aml_device("COM0");
     aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011")));
@@ -82,7 +82,7 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
                                        uart_memmap->size, AML_READ_WRITE));
     aml_append(crs,
                aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
-                             AML_EXCLUSIVE, uart_irq));
+                             AML_EXCLUSIVE, &uart_irq, 1));
     aml_append(dev, aml_name_decl("_CRS", crs));
 
     /* The _ADR entry is used to link this device to the UART described
@@ -94,7 +94,7 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
 }
 
 static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
-                                          int rtc_irq)
+                                          uint32_t rtc_irq)
 {
     Aml *dev = aml_device("RTC0");
     aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
@@ -105,7 +105,7 @@ static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
                                        rtc_memmap->size, AML_READ_WRITE));
     aml_append(crs,
                aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
-                             AML_EXCLUSIVE, rtc_irq));
+                             AML_EXCLUSIVE, &rtc_irq, 1));
     aml_append(dev, aml_name_decl("_CRS", crs));
     aml_append(scope, dev);
 }
@@ -136,11 +136,11 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
 
 static void acpi_dsdt_add_virtio(Aml *scope,
                                  const MemMapEntry *virtio_mmio_memmap,
-                                 int mmio_irq, int num)
+                                 uint32_t mmio_irq, int num)
 {
     hwaddr base = virtio_mmio_memmap->base;
     hwaddr size = virtio_mmio_memmap->size;
-    int irq = mmio_irq;
+    uint32_t irq = mmio_irq;
     int i;
 
     for (i = 0; i < num; i++) {
@@ -152,15 +152,15 @@ static void acpi_dsdt_add_virtio(Aml *scope,
         aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
         aml_append(crs,
                    aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
-                                 AML_EXCLUSIVE, irq + i));
+                                 AML_EXCLUSIVE, &irq, 1));
         aml_append(dev, aml_name_decl("_CRS", crs));
         aml_append(scope, dev);
         base += size;
     }
 }
 
-static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
-                              bool use_highmem)
+static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
+                              uint32_t irq, bool use_highmem)
 {
     Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
     int i, bus_no;
@@ -199,18 +199,19 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
 
     /* Create GSI link device */
     for (i = 0; i < PCI_NUM_PINS; i++) {
+        uint32_t irqs =  irq + i;
         Aml *dev_gsi = aml_device("GSI%d", i);
         aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F")));
         aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
         crs = aml_resource_template();
         aml_append(crs,
                    aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
-                                 AML_EXCLUSIVE, irq + i));
+                                 AML_EXCLUSIVE, &irqs, 1));
         aml_append(dev_gsi, aml_name_decl("_PRS", crs));
         crs = aml_resource_template();
         aml_append(crs,
                    aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
-                                 AML_EXCLUSIVE, irq + i));
+                                 AML_EXCLUSIVE, &irqs, 1));
         aml_append(dev_gsi, aml_name_decl("_CRS", crs));
         method = aml_method("_SRS", 1, AML_NOTSERIALIZED);
         aml_append(dev_gsi, method);
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index ca365c9..a3a058f 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -253,7 +253,7 @@ Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
 Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
                    AmlLevelAndEdge level_and_edge,
                    AmlActiveHighAndLow high_and_low, AmlShared shared,
-                   uint32_t irq);
+                   uint32_t *irq_list, uint8_t irq_count);
 Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
             uint8_t aln, uint8_t len);
 Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-arm] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs
  2015-12-09 23:41 ` [Qemu-arm] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs Igor Mammedov
@ 2015-12-10  1:50   ` Shannon Zhao
  2015-12-22 15:17   ` Michael S. Tsirkin
  1 sibling, 0 replies; 15+ messages in thread
From: Shannon Zhao @ 2015-12-10  1:50 UTC (permalink / raw)
  To: Igor Mammedov, qemu-devel; +Cc: qemu-arm



On 2015/12/10 7:41, Igor Mammedov wrote:
>  static void acpi_dsdt_add_virtio(Aml *scope,
>                                   const MemMapEntry *virtio_mmio_memmap,
> -                                 int mmio_irq, int num)
> +                                 uint32_t mmio_irq, int num)
>  {
>      hwaddr base = virtio_mmio_memmap->base;
>      hwaddr size = virtio_mmio_memmap->size;
> -    int irq = mmio_irq;
> +    uint32_t irq = mmio_irq;
>      int i;
>  
>      for (i = 0; i < num; i++) {
> @@ -152,15 +152,15 @@ static void acpi_dsdt_add_virtio(Aml *scope,
>          aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
>          aml_append(crs,
>                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> -                                 AML_EXCLUSIVE, irq + i));
> +                                 AML_EXCLUSIVE, &irq, 1));

This looks not right. You could move uint32_t irq = mmio_irq; into the
for loop and make it as uint32_t irq = mmio_irq + i;

Thanks,
-- 
Shannon


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support
  2015-12-09 23:40 [Qemu-arm] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support Igor Mammedov
  2015-12-09 23:41 ` [Qemu-arm] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs Igor Mammedov
@ 2015-12-10 15:53 ` Marcel Apfelbaum
  2015-12-10 16:31   ` Igor Mammedov
  2015-12-10 16:44 ` Igor Mammedov
  2015-12-19 20:38 ` [Qemu-arm] " Michael S. Tsirkin
  3 siblings, 1 reply; 15+ messages in thread
From: Marcel Apfelbaum @ 2015-12-10 15:53 UTC (permalink / raw)
  To: Igor Mammedov, qemu-devel
  Cc: Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
	open list:ARM ACPI Subsystem, Shannon Zhao, Paolo Bonzini,
	Richard Henderson

On 12/10/2015 01:40 AM, Igor Mammedov wrote:
> Due to huge size, CCing only cover letter instead of individual patches.
>
> Series consist of 2 parts the 1st part prefixed 'acpi:' adds necessary
> AML API functions and the second part converts DSDT using existing and
> new AML API.
>
> Series does exact byte by byte conversion and passes ACPI tables
> 'make check' tests.
> The conversion first moves common for PIIX4/Q35 parts, getting rid of *.dsl
> includes and then converts PIIX4 and Q35 parts of DSDT.
>
> Diff-stat looks nice but actual code base is reduced by ~2000LOC
> while the rest of 10000 removals is dropping precompiled AML
> templates from tree.
>
> There are some AML parts that could be optimized/simplified and shared
> between PIIX4/Q35/ARM but doing it will break exact match with original
> tests, hence it's left out of the scope of this series.


Hi Igor,
Please consider splinting this series...

Maybe ACPI new constructs first, then memhp/cpu, piix/q35. (only a suggestion)
In this you will have some "sane" 20 something patches series that
can be more easily "swallowed".

Thanks,
Marcel

>
> CC: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
> CC: Shannon Zhao <zhaoshenglong@huawei.com> (maintainer:ARM ACPI Subsystem)
> CC: Peter Maydell <peter.maydell@linaro.org> (maintainer:ARM)
> CC: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86)
> CC: Richard Henderson <rth@twiddle.net> (maintainer:X86)
> CC: Eduardo Habkost <ehabkost@redhat.com> (maintainer:X86)
> CC: qemu-arm@nongnu.org (open list:ARM ACPI Subsystem)
>
> Igor Mammedov (70):
>    tests: acpi: print ASL diff in verbose mode
>    acpi: add aml_lgreater_equal()
>    acpi: add aml_create_qword_field()
>    acpi: aml: add helper for Opcode Arg2 Arg2 [Dst] AML pattern
>    acpi: extend aml_add() to accept target argument
>    acpi: add aml_decrement() and aml_subtract()
>    acpi: add aml_call0() helper
>    acpi: add aml_to_integer()
>    acpi: extend aml_shiftright() to accept target argument
>    acpi: add aml_alias()
>    acpi: add aml_sleep()
>    acpi: add aml_lor()
>    acpi: add aml_lgreater()
>    acpi: extend aml_field() to support LockRule
>    acpi: add aml_to_hexstring()
>    acpi: add aml_to_buffer()
>    acpi add aml_dma()
>    acpi: extend aml_or() to accept target argument
>    acpi: extend aml_and() to accept target argument
>    acpi: extend aml_interrupt() to support multiple irqs
>    pc: acpi: memhp: prepare context in SSDT for moving memhp DSDT code
>    pc: acpi: memhp: move MHPD._STA method into SSDT
>    pc: acpi: memhp: move MHPD.MLCK mutex into SSDT
>    pc: acpi: memhp: move MHPD.MSCN method into SSDT
>    pc: acpi: memhp: move MHPD.MRST method into SSDT
>    pc: acpi: memhp: move MHPD.MPXM method into SSDT
>    pc: acpi: memhp: move MHPD.MOST method into SSDT
>    pc: acpi: memhp: move MHPD.MEJ0 method into SSDT
>    pc: acpi: memhp: move MHPD.MCRS method into SSDT
>    pc: acpi: memhp: move MHPD Device into SSDT
>    pc: acpi: factor out memhp code from build_ssdt() into separate
>      function
>    pc: acpi: memhp: move \_GPE._E03 into SSDT
>    pc: acpi: memhp: drop not needed stringify(MEMORY_foo) usage
>    pc: acpi: drop unused CPU_STATUS_LEN from DSDT
>    pc: acpi: cpuhp: move CPEJ() method to SSDT
>    pc: acpi: cpuhp: move CPMA() method into SSDT
>    pc: acpi: cpuhp: move CPST() method into SSDT
>    pc: acpi: cpuhp: move PRSC() method into SSDT
>    pc: acpi: cpuhp: move \_GPE._E02() into SSDT
>    pc: acpi: factor out cpu hotplug code from build_ssdt() into separate
>      function
>    pc: acpi: move HPET from DSDT to SSDT
>    pc: acpi: move DBUG() from DSDT to SSDT
>    pc: acpi: move RTC device from DSDT to SSDT
>    pc: acpi: move KBD device from DSDT to SSDT
>    pc: acpi: move MOU device from DSDT to SSDT
>    pc: acpi: move FDC0 device from DSDT to SSDT
>    pc: acpi: move LPT device from DSDT to SSDT
>    pc: acpi: move COM devices from DSDT to SSDT
>    pc: acpi: move PIIX4 isa-bridge and pm devices into SSDT
>    pc: acpi: move remaining GPE handlers into SSDT
>    pc: acpi: pci: move link devices into SSDT
>    pc: acpi: piix4: move IQCR() into SSDT
>    pc: acpi: piix4: move IQST() into SSDT
>    pc: acpi: piix4: move PCI0._PRT() into SSDT
>    pc: acpi: piix4: move remaining PCI hotplug bits into SSDT
>    pc: acpi: piix4: acpi move PCI0 device to SSDT
>    pc: acpi: q35: move GSI links to SSDT
>    pc: acpi: q35: move link devices to SSDT
>    pc: acpi: q35: move IQCR() into SSDT
>    pc: acpi: q35: move IQST() into SSDT
>    pc: acpi: q35: move ISA bridge into SSDT
>    pc: acpi: q35: move _PRT() into SSDT
>    pc: acpi: q35: move PRTA routing table into SSDT
>    pc: acpi: q35: move PRTP routing table into SSDT
>    pc: acpi: q35: move _PIC() method into SSDT
>    pc: acpi: q35: move PCI0._OSC() method into SSDT
>    pc: acpi: q35: move PCI0 device definition into SSDT
>    pc: acpi: q35: PCST, PCSB opregions and PCIB field into SSDT
>    pc: acpi: switch to AML API composed DSDT
>    pc: acpi: remove unused ASL templates and related blobs/utils
>
> Xiao Guangrong (4):
>    acpi: add aml_derefof
>    acpi: add aml_sizeof
>    acpi: add aml_mutex(), aml_acquire(), aml_release()
>    acpi: support serialized method
>
>   hw/acpi/Makefile.objs               |    4 +-
>   hw/acpi/aml-build.c                 |  294 +-
>   hw/acpi/cpu_hotplug_acpi_table.c    |  124 +
>   hw/acpi/memory_hotplug_acpi_table.c |  249 ++
>   hw/arm/virt-acpi-build.c            |   41 +-
>   hw/i386/Makefile.objs               |   31 +-
>   hw/i386/acpi-build.c                | 1329 ++++--
>   hw/i386/acpi-dsdt-cpu-hotplug.dsl   |   90 -
>   hw/i386/acpi-dsdt-dbug.dsl          |   41 -
>   hw/i386/acpi-dsdt-hpet.dsl          |   48 -
>   hw/i386/acpi-dsdt-isa.dsl           |  117 -
>   hw/i386/acpi-dsdt-mem-hotplug.dsl   |  171 -
>   hw/i386/acpi-dsdt.dsl               |  303 --
>   hw/i386/acpi-dsdt.hex.generated     | 2972 --------------
>   hw/i386/q35-acpi-dsdt.dsl           |  436 --
>   hw/i386/q35-acpi-dsdt.hex.generated | 7610 -----------------------------------
>   hw/timer/hpet.c                     |    2 +-
>   include/hw/acpi/aml-build.h         |   66 +-
>   include/hw/acpi/cpu_hotplug.h       |   10 +
>   include/hw/acpi/memory_hotplug.h    |    9 +
>   include/hw/acpi/pc-hotplug.h        |   44 +-
>   include/hw/timer/hpet.h             |    1 +
>   scripts/acpi_extract.py             |  367 --
>   scripts/acpi_extract_preprocess.py  |   51 -
>   scripts/update-acpi.sh              |    4 -
>   tests/bios-tables-test.c            |    7 +
>   26 files changed, 1848 insertions(+), 12573 deletions(-)
>   create mode 100644 hw/acpi/cpu_hotplug_acpi_table.c
>   create mode 100644 hw/acpi/memory_hotplug_acpi_table.c
>   delete mode 100644 hw/i386/acpi-dsdt-cpu-hotplug.dsl
>   delete mode 100644 hw/i386/acpi-dsdt-dbug.dsl
>   delete mode 100644 hw/i386/acpi-dsdt-hpet.dsl
>   delete mode 100644 hw/i386/acpi-dsdt-isa.dsl
>   delete mode 100644 hw/i386/acpi-dsdt-mem-hotplug.dsl
>   delete mode 100644 hw/i386/acpi-dsdt.dsl
>   delete mode 100644 hw/i386/acpi-dsdt.hex.generated
>   delete mode 100644 hw/i386/q35-acpi-dsdt.dsl
>   delete mode 100644 hw/i386/q35-acpi-dsdt.hex.generated
>   delete mode 100755 scripts/acpi_extract.py
>   delete mode 100755 scripts/acpi_extract_preprocess.py
>   delete mode 100644 scripts/update-acpi.sh
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support
  2015-12-10 15:53 ` [Qemu-devel] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support Marcel Apfelbaum
@ 2015-12-10 16:31   ` Igor Mammedov
  2015-12-13 15:24     ` [Qemu-arm] " Marcel Apfelbaum
  0 siblings, 1 reply; 15+ messages in thread
From: Igor Mammedov @ 2015-12-10 16:31 UTC (permalink / raw)
  To: Marcel Apfelbaum
  Cc: Peter Maydell, Eduardo Habkost, Michael S. Tsirkin, qemu-devel,
	open list:ARM ACPI Subsystem, Shannon Zhao, Paolo Bonzini,
	Richard Henderson

On Thu, 10 Dec 2015 17:53:31 +0200
Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:

[...]
> Hi Igor,
> Please consider splinting this series...
> 
> Maybe ACPI new constructs first, then memhp/cpu, piix/q35. (only a
> suggestion) In this you will have some "sane" 20 something patches
> series that can be more easily "swallowed".
I'd also prefer to do it incrementally but
I've tried something like this before, but Michael suggested to
avoid optimizations/refactoring and make it so that conversion would
pass 'make check' so it would be easy to prove that it hasn't regressed
anything and to do it the only way is to convert whole DSDT at once.

Nevertheless, the series more or less organized with split in mind,
so it's possible first commit AML API patches ('acpi:' prefixed)
and then just go in order the blocks as they are in series:

memhp/cpuhp/misc devices/piix4/q35/final cleanup


> 
> Thanks,
> Marcel
> 
> >
> > CC: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
> > CC: Shannon Zhao <zhaoshenglong@huawei.com> (maintainer:ARM ACPI
> > Subsystem) CC: Peter Maydell <peter.maydell@linaro.org>
> > (maintainer:ARM) CC: Paolo Bonzini <pbonzini@redhat.com>
> > (maintainer:X86) CC: Richard Henderson <rth@twiddle.net>
> > (maintainer:X86) CC: Eduardo Habkost <ehabkost@redhat.com>
> > (maintainer:X86) CC: qemu-arm@nongnu.org (open list:ARM ACPI
> > Subsystem)
> >
> > Igor Mammedov (70):
> >    tests: acpi: print ASL diff in verbose mode
> >    acpi: add aml_lgreater_equal()
> >    acpi: add aml_create_qword_field()
> >    acpi: aml: add helper for Opcode Arg2 Arg2 [Dst] AML pattern
> >    acpi: extend aml_add() to accept target argument
> >    acpi: add aml_decrement() and aml_subtract()
> >    acpi: add aml_call0() helper
> >    acpi: add aml_to_integer()
> >    acpi: extend aml_shiftright() to accept target argument
> >    acpi: add aml_alias()
> >    acpi: add aml_sleep()
> >    acpi: add aml_lor()
> >    acpi: add aml_lgreater()
> >    acpi: extend aml_field() to support LockRule
> >    acpi: add aml_to_hexstring()
> >    acpi: add aml_to_buffer()
> >    acpi add aml_dma()
> >    acpi: extend aml_or() to accept target argument
> >    acpi: extend aml_and() to accept target argument
> >    acpi: extend aml_interrupt() to support multiple irqs
> >    pc: acpi: memhp: prepare context in SSDT for moving memhp DSDT
> > code pc: acpi: memhp: move MHPD._STA method into SSDT
> >    pc: acpi: memhp: move MHPD.MLCK mutex into SSDT
> >    pc: acpi: memhp: move MHPD.MSCN method into SSDT
> >    pc: acpi: memhp: move MHPD.MRST method into SSDT
> >    pc: acpi: memhp: move MHPD.MPXM method into SSDT
> >    pc: acpi: memhp: move MHPD.MOST method into SSDT
> >    pc: acpi: memhp: move MHPD.MEJ0 method into SSDT
> >    pc: acpi: memhp: move MHPD.MCRS method into SSDT
> >    pc: acpi: memhp: move MHPD Device into SSDT
> >    pc: acpi: factor out memhp code from build_ssdt() into separate
> >      function
> >    pc: acpi: memhp: move \_GPE._E03 into SSDT
> >    pc: acpi: memhp: drop not needed stringify(MEMORY_foo) usage
> >    pc: acpi: drop unused CPU_STATUS_LEN from DSDT
> >    pc: acpi: cpuhp: move CPEJ() method to SSDT
> >    pc: acpi: cpuhp: move CPMA() method into SSDT
> >    pc: acpi: cpuhp: move CPST() method into SSDT
> >    pc: acpi: cpuhp: move PRSC() method into SSDT
> >    pc: acpi: cpuhp: move \_GPE._E02() into SSDT
> >    pc: acpi: factor out cpu hotplug code from build_ssdt() into
> > separate function
> >    pc: acpi: move HPET from DSDT to SSDT
> >    pc: acpi: move DBUG() from DSDT to SSDT
> >    pc: acpi: move RTC device from DSDT to SSDT
> >    pc: acpi: move KBD device from DSDT to SSDT
> >    pc: acpi: move MOU device from DSDT to SSDT
> >    pc: acpi: move FDC0 device from DSDT to SSDT
> >    pc: acpi: move LPT device from DSDT to SSDT
> >    pc: acpi: move COM devices from DSDT to SSDT
> >    pc: acpi: move PIIX4 isa-bridge and pm devices into SSDT
> >    pc: acpi: move remaining GPE handlers into SSDT
> >    pc: acpi: pci: move link devices into SSDT
> >    pc: acpi: piix4: move IQCR() into SSDT
> >    pc: acpi: piix4: move IQST() into SSDT
> >    pc: acpi: piix4: move PCI0._PRT() into SSDT
> >    pc: acpi: piix4: move remaining PCI hotplug bits into SSDT
> >    pc: acpi: piix4: acpi move PCI0 device to SSDT
> >    pc: acpi: q35: move GSI links to SSDT
> >    pc: acpi: q35: move link devices to SSDT
> >    pc: acpi: q35: move IQCR() into SSDT
> >    pc: acpi: q35: move IQST() into SSDT
> >    pc: acpi: q35: move ISA bridge into SSDT
> >    pc: acpi: q35: move _PRT() into SSDT
> >    pc: acpi: q35: move PRTA routing table into SSDT
> >    pc: acpi: q35: move PRTP routing table into SSDT
> >    pc: acpi: q35: move _PIC() method into SSDT
> >    pc: acpi: q35: move PCI0._OSC() method into SSDT
> >    pc: acpi: q35: move PCI0 device definition into SSDT
> >    pc: acpi: q35: PCST, PCSB opregions and PCIB field into SSDT
> >    pc: acpi: switch to AML API composed DSDT
> >    pc: acpi: remove unused ASL templates and related blobs/utils
> >
> > Xiao Guangrong (4):
> >    acpi: add aml_derefof
> >    acpi: add aml_sizeof
> >    acpi: add aml_mutex(), aml_acquire(), aml_release()
> >    acpi: support serialized method
> >
> >   hw/acpi/Makefile.objs               |    4 +-
> >   hw/acpi/aml-build.c                 |  294 +-
> >   hw/acpi/cpu_hotplug_acpi_table.c    |  124 +
> >   hw/acpi/memory_hotplug_acpi_table.c |  249 ++
> >   hw/arm/virt-acpi-build.c            |   41 +-
> >   hw/i386/Makefile.objs               |   31 +-
> >   hw/i386/acpi-build.c                | 1329 ++++--
> >   hw/i386/acpi-dsdt-cpu-hotplug.dsl   |   90 -
> >   hw/i386/acpi-dsdt-dbug.dsl          |   41 -
> >   hw/i386/acpi-dsdt-hpet.dsl          |   48 -
> >   hw/i386/acpi-dsdt-isa.dsl           |  117 -
> >   hw/i386/acpi-dsdt-mem-hotplug.dsl   |  171 -
> >   hw/i386/acpi-dsdt.dsl               |  303 --
> >   hw/i386/acpi-dsdt.hex.generated     | 2972 --------------
> >   hw/i386/q35-acpi-dsdt.dsl           |  436 --
> >   hw/i386/q35-acpi-dsdt.hex.generated | 7610
> > -----------------------------------
> > hw/timer/hpet.c                     |    2 +-
> > include/hw/acpi/aml-build.h         |   66 +-
> > include/hw/acpi/cpu_hotplug.h       |   10 +
> > include/hw/acpi/memory_hotplug.h    |    9 +
> > include/hw/acpi/pc-hotplug.h        |   44 +-
> > include/hw/timer/hpet.h             |    1 +
> > scripts/acpi_extract.py             |  367 --
> > scripts/acpi_extract_preprocess.py  |   51 -
> > scripts/update-acpi.sh              |    4 -
> > tests/bios-tables-test.c            |    7 + 26 files changed, 1848
> > insertions(+), 12573 deletions(-) create mode 100644
> > hw/acpi/cpu_hotplug_acpi_table.c create mode 100644
> > hw/acpi/memory_hotplug_acpi_table.c delete mode 100644
> > hw/i386/acpi-dsdt-cpu-hotplug.dsl delete mode 100644
> > hw/i386/acpi-dsdt-dbug.dsl delete mode 100644
> > hw/i386/acpi-dsdt-hpet.dsl delete mode 100644
> > hw/i386/acpi-dsdt-isa.dsl delete mode 100644
> > hw/i386/acpi-dsdt-mem-hotplug.dsl delete mode 100644
> > hw/i386/acpi-dsdt.dsl delete mode 100644
> > hw/i386/acpi-dsdt.hex.generated delete mode 100644
> > hw/i386/q35-acpi-dsdt.dsl delete mode 100644
> > hw/i386/q35-acpi-dsdt.hex.generated delete mode 100755
> > scripts/acpi_extract.py delete mode 100755
> > scripts/acpi_extract_preprocess.py delete mode 100644
> > scripts/update-acpi.sh
> >
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-arm] [Qemu-devel] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support
  2015-12-09 23:40 [Qemu-arm] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support Igor Mammedov
  2015-12-09 23:41 ` [Qemu-arm] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs Igor Mammedov
  2015-12-10 15:53 ` [Qemu-devel] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support Marcel Apfelbaum
@ 2015-12-10 16:44 ` Igor Mammedov
  2015-12-19 20:38 ` [Qemu-arm] " Michael S. Tsirkin
  3 siblings, 0 replies; 15+ messages in thread
From: Igor Mammedov @ 2015-12-10 16:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
	open list:ARM ACPI Subsystem, Shannon Zhao, Paolo Bonzini,
	Richard Henderson

forgot to mention
git tree for testing is available at:

git@github.com:imammedo/qemu.git drop_ASL_support_v1
or
https://github.com/imammedo/qemu/commits/drop_ASL_support_v1

and fixed-up series with fixed comments is at:

git@github.com:imammedo/qemu.git drop_ASL_support_wip
or
https://github.com/imammedo/qemu/commits/drop_ASL_support_wip

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-arm] [Qemu-devel] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support
  2015-12-10 16:31   ` Igor Mammedov
@ 2015-12-13 15:24     ` Marcel Apfelbaum
  0 siblings, 0 replies; 15+ messages in thread
From: Marcel Apfelbaum @ 2015-12-13 15:24 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Peter Maydell, Eduardo Habkost, Michael S. Tsirkin, qemu-devel,
	open list:ARM ACPI Subsystem, Shannon Zhao, Paolo Bonzini,
	Richard Henderson

On 12/10/2015 06:31 PM, Igor Mammedov wrote:
> On Thu, 10 Dec 2015 17:53:31 +0200
> Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:
>
> [...]
>> Hi Igor,
>> Please consider splinting this series...
>>
>> Maybe ACPI new constructs first, then memhp/cpu, piix/q35. (only a
>> suggestion) In this you will have some "sane" 20 something patches
>> series that can be more easily "swallowed".
> I'd also prefer to do it incrementally but
> I've tried something like this before, but Michael suggested to
> avoid optimizations/refactoring and make it so that conversion would
> pass 'make check' so it would be easy to prove that it hasn't regressed
> anything and to do it the only way is to convert whole DSDT at once.
>
> Nevertheless, the series more or less organized with split in mind,
> so it's possible first commit AML API patches ('acpi:' prefixed)
> and then just go in order the blocks as they are in series:
>
> memhp/cpuhp/misc devices/piix4/q35/final cleanup

Indeed, the way the series is organized, it can be split-ted/committed
into the mentioned blocks without breaking make check.

However, if Michael is willing to take it "as is" ... :)
I'll do my best to go over it this week.
Marcel


>
>
>>
>> Thanks,
>> Marcel
>>
>>>
>>> CC: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
>>> CC: Shannon Zhao <zhaoshenglong@huawei.com> (maintainer:ARM ACPI
>>> Subsystem) CC: Peter Maydell <peter.maydell@linaro.org>
>>> (maintainer:ARM) CC: Paolo Bonzini <pbonzini@redhat.com>
>>> (maintainer:X86) CC: Richard Henderson <rth@twiddle.net>
>>> (maintainer:X86) CC: Eduardo Habkost <ehabkost@redhat.com>
>>> (maintainer:X86) CC: qemu-arm@nongnu.org (open list:ARM ACPI
>>> Subsystem)
>>>
>>> Igor Mammedov (70):
>>>     tests: acpi: print ASL diff in verbose mode
>>>     acpi: add aml_lgreater_equal()
>>>     acpi: add aml_create_qword_field()
>>>     acpi: aml: add helper for Opcode Arg2 Arg2 [Dst] AML pattern
>>>     acpi: extend aml_add() to accept target argument
>>>     acpi: add aml_decrement() and aml_subtract()
>>>     acpi: add aml_call0() helper
>>>     acpi: add aml_to_integer()
>>>     acpi: extend aml_shiftright() to accept target argument
>>>     acpi: add aml_alias()
>>>     acpi: add aml_sleep()
>>>     acpi: add aml_lor()
>>>     acpi: add aml_lgreater()
>>>     acpi: extend aml_field() to support LockRule
>>>     acpi: add aml_to_hexstring()
>>>     acpi: add aml_to_buffer()
>>>     acpi add aml_dma()
>>>     acpi: extend aml_or() to accept target argument
>>>     acpi: extend aml_and() to accept target argument
>>>     acpi: extend aml_interrupt() to support multiple irqs
>>>     pc: acpi: memhp: prepare context in SSDT for moving memhp DSDT
>>> code pc: acpi: memhp: move MHPD._STA method into SSDT
>>>     pc: acpi: memhp: move MHPD.MLCK mutex into SSDT
>>>     pc: acpi: memhp: move MHPD.MSCN method into SSDT
>>>     pc: acpi: memhp: move MHPD.MRST method into SSDT
>>>     pc: acpi: memhp: move MHPD.MPXM method into SSDT
>>>     pc: acpi: memhp: move MHPD.MOST method into SSDT
>>>     pc: acpi: memhp: move MHPD.MEJ0 method into SSDT
>>>     pc: acpi: memhp: move MHPD.MCRS method into SSDT
>>>     pc: acpi: memhp: move MHPD Device into SSDT
>>>     pc: acpi: factor out memhp code from build_ssdt() into separate
>>>       function
>>>     pc: acpi: memhp: move \_GPE._E03 into SSDT
>>>     pc: acpi: memhp: drop not needed stringify(MEMORY_foo) usage
>>>     pc: acpi: drop unused CPU_STATUS_LEN from DSDT
>>>     pc: acpi: cpuhp: move CPEJ() method to SSDT
>>>     pc: acpi: cpuhp: move CPMA() method into SSDT
>>>     pc: acpi: cpuhp: move CPST() method into SSDT
>>>     pc: acpi: cpuhp: move PRSC() method into SSDT
>>>     pc: acpi: cpuhp: move \_GPE._E02() into SSDT
>>>     pc: acpi: factor out cpu hotplug code from build_ssdt() into
>>> separate function
>>>     pc: acpi: move HPET from DSDT to SSDT
>>>     pc: acpi: move DBUG() from DSDT to SSDT
>>>     pc: acpi: move RTC device from DSDT to SSDT
>>>     pc: acpi: move KBD device from DSDT to SSDT
>>>     pc: acpi: move MOU device from DSDT to SSDT
>>>     pc: acpi: move FDC0 device from DSDT to SSDT
>>>     pc: acpi: move LPT device from DSDT to SSDT
>>>     pc: acpi: move COM devices from DSDT to SSDT
>>>     pc: acpi: move PIIX4 isa-bridge and pm devices into SSDT
>>>     pc: acpi: move remaining GPE handlers into SSDT
>>>     pc: acpi: pci: move link devices into SSDT
>>>     pc: acpi: piix4: move IQCR() into SSDT
>>>     pc: acpi: piix4: move IQST() into SSDT
>>>     pc: acpi: piix4: move PCI0._PRT() into SSDT
>>>     pc: acpi: piix4: move remaining PCI hotplug bits into SSDT
>>>     pc: acpi: piix4: acpi move PCI0 device to SSDT
>>>     pc: acpi: q35: move GSI links to SSDT
>>>     pc: acpi: q35: move link devices to SSDT
>>>     pc: acpi: q35: move IQCR() into SSDT
>>>     pc: acpi: q35: move IQST() into SSDT
>>>     pc: acpi: q35: move ISA bridge into SSDT
>>>     pc: acpi: q35: move _PRT() into SSDT
>>>     pc: acpi: q35: move PRTA routing table into SSDT
>>>     pc: acpi: q35: move PRTP routing table into SSDT
>>>     pc: acpi: q35: move _PIC() method into SSDT
>>>     pc: acpi: q35: move PCI0._OSC() method into SSDT
>>>     pc: acpi: q35: move PCI0 device definition into SSDT
>>>     pc: acpi: q35: PCST, PCSB opregions and PCIB field into SSDT
>>>     pc: acpi: switch to AML API composed DSDT
>>>     pc: acpi: remove unused ASL templates and related blobs/utils
>>>
>>> Xiao Guangrong (4):
>>>     acpi: add aml_derefof
>>>     acpi: add aml_sizeof
>>>     acpi: add aml_mutex(), aml_acquire(), aml_release()
>>>     acpi: support serialized method
>>>
>>>    hw/acpi/Makefile.objs               |    4 +-
>>>    hw/acpi/aml-build.c                 |  294 +-
>>>    hw/acpi/cpu_hotplug_acpi_table.c    |  124 +
>>>    hw/acpi/memory_hotplug_acpi_table.c |  249 ++
>>>    hw/arm/virt-acpi-build.c            |   41 +-
>>>    hw/i386/Makefile.objs               |   31 +-
>>>    hw/i386/acpi-build.c                | 1329 ++++--
>>>    hw/i386/acpi-dsdt-cpu-hotplug.dsl   |   90 -
>>>    hw/i386/acpi-dsdt-dbug.dsl          |   41 -
>>>    hw/i386/acpi-dsdt-hpet.dsl          |   48 -
>>>    hw/i386/acpi-dsdt-isa.dsl           |  117 -
>>>    hw/i386/acpi-dsdt-mem-hotplug.dsl   |  171 -
>>>    hw/i386/acpi-dsdt.dsl               |  303 --
>>>    hw/i386/acpi-dsdt.hex.generated     | 2972 --------------
>>>    hw/i386/q35-acpi-dsdt.dsl           |  436 --
>>>    hw/i386/q35-acpi-dsdt.hex.generated | 7610
>>> -----------------------------------
>>> hw/timer/hpet.c                     |    2 +-
>>> include/hw/acpi/aml-build.h         |   66 +-
>>> include/hw/acpi/cpu_hotplug.h       |   10 +
>>> include/hw/acpi/memory_hotplug.h    |    9 +
>>> include/hw/acpi/pc-hotplug.h        |   44 +-
>>> include/hw/timer/hpet.h             |    1 +
>>> scripts/acpi_extract.py             |  367 --
>>> scripts/acpi_extract_preprocess.py  |   51 -
>>> scripts/update-acpi.sh              |    4 -
>>> tests/bios-tables-test.c            |    7 + 26 files changed, 1848
>>> insertions(+), 12573 deletions(-) create mode 100644
>>> hw/acpi/cpu_hotplug_acpi_table.c create mode 100644
>>> hw/acpi/memory_hotplug_acpi_table.c delete mode 100644
>>> hw/i386/acpi-dsdt-cpu-hotplug.dsl delete mode 100644
>>> hw/i386/acpi-dsdt-dbug.dsl delete mode 100644
>>> hw/i386/acpi-dsdt-hpet.dsl delete mode 100644
>>> hw/i386/acpi-dsdt-isa.dsl delete mode 100644
>>> hw/i386/acpi-dsdt-mem-hotplug.dsl delete mode 100644
>>> hw/i386/acpi-dsdt.dsl delete mode 100644
>>> hw/i386/acpi-dsdt.hex.generated delete mode 100644
>>> hw/i386/q35-acpi-dsdt.dsl delete mode 100644
>>> hw/i386/q35-acpi-dsdt.hex.generated delete mode 100755
>>> scripts/acpi_extract.py delete mode 100755
>>> scripts/acpi_extract_preprocess.py delete mode 100644
>>> scripts/update-acpi.sh
>>>
>>
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-arm] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support
  2015-12-09 23:40 [Qemu-arm] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support Igor Mammedov
                   ` (2 preceding siblings ...)
  2015-12-10 16:44 ` Igor Mammedov
@ 2015-12-19 20:38 ` Michael S. Tsirkin
  2015-12-21 13:00   ` [Qemu-arm] [Qemu-devel] " Igor Mammedov
  3 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2015-12-19 20:38 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Peter Maydell, Eduardo Habkost, qemu-devel,
	open list:ARM ACPI Subsystem, Shannon Zhao, Paolo Bonzini,
	Richard Henderson

On Thu, Dec 10, 2015 at 12:40:54AM +0100, Igor Mammedov wrote:
> Due to huge size, CCing only cover letter instead of individual patches.
> 
> Series consist of 2 parts the 1st part prefixed 'acpi:' adds necessary
> AML API functions and the second part converts DSDT using existing and
> new AML API.
> 
> Series does exact byte by byte conversion and passes ACPI tables
> 'make check' tests.
> The conversion first moves common for PIIX4/Q35 parts, getting rid of *.dsl
> includes and then converts PIIX4 and Q35 parts of DSDT.
> 
> Diff-stat looks nice but actual code base is reduced by ~2000LOC
> while the rest of 10000 removals is dropping precompiled AML
> templates from tree.
> 
> There are some AML parts that could be optimized/simplified and shared
> between PIIX4/Q35/ARM but doing it will break exact match with original
> tests, hence it's left out of the scope of this series.
> 
> CC: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
> CC: Shannon Zhao <zhaoshenglong@huawei.com> (maintainer:ARM ACPI Subsystem)
> CC: Peter Maydell <peter.maydell@linaro.org> (maintainer:ARM)
> CC: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86)
> CC: Richard Henderson <rth@twiddle.net> (maintainer:X86)
> CC: Eduardo Habkost <ehabkost@redhat.com> (maintainer:X86)
> CC: qemu-arm@nongnu.org (open list:ARM ACPI Subsystem)

I think this is a nice series, and I think we can live with the fact
make check warns in the middle.

I sent some comments I'd like to see addressed.

Main points
	- one v2 patch seems garbled
	- please don't create functions with tons of parameters
	  which just do if (a) on each of these internally.
	  instead, move common code into a small function that
	  can be reused without so many conditionals.
	- I really dislike local variables starting with a_ for
	  no good reason.

Thanks!

> Igor Mammedov (70):
>   tests: acpi: print ASL diff in verbose mode
>   acpi: add aml_lgreater_equal()
>   acpi: add aml_create_qword_field()
>   acpi: aml: add helper for Opcode Arg2 Arg2 [Dst] AML pattern
>   acpi: extend aml_add() to accept target argument
>   acpi: add aml_decrement() and aml_subtract()
>   acpi: add aml_call0() helper
>   acpi: add aml_to_integer()
>   acpi: extend aml_shiftright() to accept target argument
>   acpi: add aml_alias()
>   acpi: add aml_sleep()
>   acpi: add aml_lor()
>   acpi: add aml_lgreater()
>   acpi: extend aml_field() to support LockRule
>   acpi: add aml_to_hexstring()
>   acpi: add aml_to_buffer()
>   acpi add aml_dma()
>   acpi: extend aml_or() to accept target argument
>   acpi: extend aml_and() to accept target argument
>   acpi: extend aml_interrupt() to support multiple irqs
>   pc: acpi: memhp: prepare context in SSDT for moving memhp DSDT code
>   pc: acpi: memhp: move MHPD._STA method into SSDT
>   pc: acpi: memhp: move MHPD.MLCK mutex into SSDT
>   pc: acpi: memhp: move MHPD.MSCN method into SSDT
>   pc: acpi: memhp: move MHPD.MRST method into SSDT
>   pc: acpi: memhp: move MHPD.MPXM method into SSDT
>   pc: acpi: memhp: move MHPD.MOST method into SSDT
>   pc: acpi: memhp: move MHPD.MEJ0 method into SSDT
>   pc: acpi: memhp: move MHPD.MCRS method into SSDT
>   pc: acpi: memhp: move MHPD Device into SSDT
>   pc: acpi: factor out memhp code from build_ssdt() into separate
>     function
>   pc: acpi: memhp: move \_GPE._E03 into SSDT
>   pc: acpi: memhp: drop not needed stringify(MEMORY_foo) usage
>   pc: acpi: drop unused CPU_STATUS_LEN from DSDT
>   pc: acpi: cpuhp: move CPEJ() method to SSDT
>   pc: acpi: cpuhp: move CPMA() method into SSDT
>   pc: acpi: cpuhp: move CPST() method into SSDT
>   pc: acpi: cpuhp: move PRSC() method into SSDT
>   pc: acpi: cpuhp: move \_GPE._E02() into SSDT
>   pc: acpi: factor out cpu hotplug code from build_ssdt() into separate
>     function
>   pc: acpi: move HPET from DSDT to SSDT
>   pc: acpi: move DBUG() from DSDT to SSDT
>   pc: acpi: move RTC device from DSDT to SSDT
>   pc: acpi: move KBD device from DSDT to SSDT
>   pc: acpi: move MOU device from DSDT to SSDT
>   pc: acpi: move FDC0 device from DSDT to SSDT
>   pc: acpi: move LPT device from DSDT to SSDT
>   pc: acpi: move COM devices from DSDT to SSDT
>   pc: acpi: move PIIX4 isa-bridge and pm devices into SSDT
>   pc: acpi: move remaining GPE handlers into SSDT
>   pc: acpi: pci: move link devices into SSDT
>   pc: acpi: piix4: move IQCR() into SSDT
>   pc: acpi: piix4: move IQST() into SSDT
>   pc: acpi: piix4: move PCI0._PRT() into SSDT
>   pc: acpi: piix4: move remaining PCI hotplug bits into SSDT
>   pc: acpi: piix4: acpi move PCI0 device to SSDT
>   pc: acpi: q35: move GSI links to SSDT
>   pc: acpi: q35: move link devices to SSDT
>   pc: acpi: q35: move IQCR() into SSDT
>   pc: acpi: q35: move IQST() into SSDT
>   pc: acpi: q35: move ISA bridge into SSDT
>   pc: acpi: q35: move _PRT() into SSDT
>   pc: acpi: q35: move PRTA routing table into SSDT
>   pc: acpi: q35: move PRTP routing table into SSDT
>   pc: acpi: q35: move _PIC() method into SSDT
>   pc: acpi: q35: move PCI0._OSC() method into SSDT
>   pc: acpi: q35: move PCI0 device definition into SSDT
>   pc: acpi: q35: PCST, PCSB opregions and PCIB field into SSDT
>   pc: acpi: switch to AML API composed DSDT
>   pc: acpi: remove unused ASL templates and related blobs/utils
> 
> Xiao Guangrong (4):
>   acpi: add aml_derefof
>   acpi: add aml_sizeof
>   acpi: add aml_mutex(), aml_acquire(), aml_release()
>   acpi: support serialized method
> 
>  hw/acpi/Makefile.objs               |    4 +-
>  hw/acpi/aml-build.c                 |  294 +-
>  hw/acpi/cpu_hotplug_acpi_table.c    |  124 +
>  hw/acpi/memory_hotplug_acpi_table.c |  249 ++
>  hw/arm/virt-acpi-build.c            |   41 +-
>  hw/i386/Makefile.objs               |   31 +-
>  hw/i386/acpi-build.c                | 1329 ++++--
>  hw/i386/acpi-dsdt-cpu-hotplug.dsl   |   90 -
>  hw/i386/acpi-dsdt-dbug.dsl          |   41 -
>  hw/i386/acpi-dsdt-hpet.dsl          |   48 -
>  hw/i386/acpi-dsdt-isa.dsl           |  117 -
>  hw/i386/acpi-dsdt-mem-hotplug.dsl   |  171 -
>  hw/i386/acpi-dsdt.dsl               |  303 --
>  hw/i386/acpi-dsdt.hex.generated     | 2972 --------------
>  hw/i386/q35-acpi-dsdt.dsl           |  436 --
>  hw/i386/q35-acpi-dsdt.hex.generated | 7610 -----------------------------------
>  hw/timer/hpet.c                     |    2 +-
>  include/hw/acpi/aml-build.h         |   66 +-
>  include/hw/acpi/cpu_hotplug.h       |   10 +
>  include/hw/acpi/memory_hotplug.h    |    9 +
>  include/hw/acpi/pc-hotplug.h        |   44 +-
>  include/hw/timer/hpet.h             |    1 +
>  scripts/acpi_extract.py             |  367 --
>  scripts/acpi_extract_preprocess.py  |   51 -
>  scripts/update-acpi.sh              |    4 -
>  tests/bios-tables-test.c            |    7 +
>  26 files changed, 1848 insertions(+), 12573 deletions(-)
>  create mode 100644 hw/acpi/cpu_hotplug_acpi_table.c
>  create mode 100644 hw/acpi/memory_hotplug_acpi_table.c
>  delete mode 100644 hw/i386/acpi-dsdt-cpu-hotplug.dsl
>  delete mode 100644 hw/i386/acpi-dsdt-dbug.dsl
>  delete mode 100644 hw/i386/acpi-dsdt-hpet.dsl
>  delete mode 100644 hw/i386/acpi-dsdt-isa.dsl
>  delete mode 100644 hw/i386/acpi-dsdt-mem-hotplug.dsl
>  delete mode 100644 hw/i386/acpi-dsdt.dsl
>  delete mode 100644 hw/i386/acpi-dsdt.hex.generated
>  delete mode 100644 hw/i386/q35-acpi-dsdt.dsl
>  delete mode 100644 hw/i386/q35-acpi-dsdt.hex.generated
>  delete mode 100755 scripts/acpi_extract.py
>  delete mode 100755 scripts/acpi_extract_preprocess.py
>  delete mode 100644 scripts/update-acpi.sh
> 
> -- 
> 1.8.3.1
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-arm] [Qemu-devel] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support
  2015-12-19 20:38 ` [Qemu-arm] " Michael S. Tsirkin
@ 2015-12-21 13:00   ` Igor Mammedov
  2015-12-21 13:12     ` Shannon Zhao
  0 siblings, 1 reply; 15+ messages in thread
From: Igor Mammedov @ 2015-12-21 13:00 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Peter Maydell, Eduardo Habkost, qemu-devel,
	open list:ARM ACPI Subsystem, Shannon Zhao, Paolo Bonzini,
	Richard Henderson

On Sat, 19 Dec 2015 22:38:41 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Dec 10, 2015 at 12:40:54AM +0100, Igor Mammedov wrote:
> > Due to huge size, CCing only cover letter instead of individual patches.
> > 
> > Series consist of 2 parts the 1st part prefixed 'acpi:' adds necessary
> > AML API functions and the second part converts DSDT using existing and
> > new AML API.
> > 
> > Series does exact byte by byte conversion and passes ACPI tables
> > 'make check' tests.
> > The conversion first moves common for PIIX4/Q35 parts, getting rid of *.dsl
> > includes and then converts PIIX4 and Q35 parts of DSDT.
> > 
> > Diff-stat looks nice but actual code base is reduced by ~2000LOC
> > while the rest of 10000 removals is dropping precompiled AML
> > templates from tree.
> > 
> > There are some AML parts that could be optimized/simplified and shared
> > between PIIX4/Q35/ARM but doing it will break exact match with original
> > tests, hence it's left out of the scope of this series.
> > 
> > CC: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
> > CC: Shannon Zhao <zhaoshenglong@huawei.com> (maintainer:ARM ACPI Subsystem)
> > CC: Peter Maydell <peter.maydell@linaro.org> (maintainer:ARM)
> > CC: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86)
> > CC: Richard Henderson <rth@twiddle.net> (maintainer:X86)
> > CC: Eduardo Habkost <ehabkost@redhat.com> (maintainer:X86)
> > CC: qemu-arm@nongnu.org (open list:ARM ACPI Subsystem)
> 
> I think this is a nice series, and I think we can live with the fact
> make check warns in the middle.
> 
> I sent some comments I'd like to see addressed.
> 
> Main points
> 	- one v2 patch seems garbled
> 	- please don't create functions with tons of parameters
> 	  which just do if (a) on each of these internally.
> 	  instead, move common code into a small function that
> 	  can be reused without so many conditionals.
> 	- I really dislike local variables starting with a_ for
> 	  no good reason.
Michael,

Could you apply already reviewed AML API patches 1-24
and push it to master early, pls?

That will help to reduce collisions with other AML
patches on list and provide more complete API for others to use.

> 
> Thanks!
> 
> > Igor Mammedov (70):
> >   tests: acpi: print ASL diff in verbose mode
> >   acpi: add aml_lgreater_equal()
> >   acpi: add aml_create_qword_field()
> >   acpi: aml: add helper for Opcode Arg2 Arg2 [Dst] AML pattern
> >   acpi: extend aml_add() to accept target argument
> >   acpi: add aml_decrement() and aml_subtract()
> >   acpi: add aml_call0() helper
> >   acpi: add aml_to_integer()
> >   acpi: extend aml_shiftright() to accept target argument
> >   acpi: add aml_alias()
> >   acpi: add aml_sleep()
> >   acpi: add aml_lor()
> >   acpi: add aml_lgreater()
> >   acpi: extend aml_field() to support LockRule
> >   acpi: add aml_to_hexstring()
> >   acpi: add aml_to_buffer()
> >   acpi add aml_dma()
> >   acpi: extend aml_or() to accept target argument
> >   acpi: extend aml_and() to accept target argument
> >   acpi: extend aml_interrupt() to support multiple irqs
> >   pc: acpi: memhp: prepare context in SSDT for moving memhp DSDT code
> >   pc: acpi: memhp: move MHPD._STA method into SSDT
> >   pc: acpi: memhp: move MHPD.MLCK mutex into SSDT
> >   pc: acpi: memhp: move MHPD.MSCN method into SSDT
> >   pc: acpi: memhp: move MHPD.MRST method into SSDT
> >   pc: acpi: memhp: move MHPD.MPXM method into SSDT
> >   pc: acpi: memhp: move MHPD.MOST method into SSDT
> >   pc: acpi: memhp: move MHPD.MEJ0 method into SSDT
> >   pc: acpi: memhp: move MHPD.MCRS method into SSDT
> >   pc: acpi: memhp: move MHPD Device into SSDT
> >   pc: acpi: factor out memhp code from build_ssdt() into separate
> >     function
> >   pc: acpi: memhp: move \_GPE._E03 into SSDT
> >   pc: acpi: memhp: drop not needed stringify(MEMORY_foo) usage
> >   pc: acpi: drop unused CPU_STATUS_LEN from DSDT
> >   pc: acpi: cpuhp: move CPEJ() method to SSDT
> >   pc: acpi: cpuhp: move CPMA() method into SSDT
> >   pc: acpi: cpuhp: move CPST() method into SSDT
> >   pc: acpi: cpuhp: move PRSC() method into SSDT
> >   pc: acpi: cpuhp: move \_GPE._E02() into SSDT
> >   pc: acpi: factor out cpu hotplug code from build_ssdt() into separate
> >     function
> >   pc: acpi: move HPET from DSDT to SSDT
> >   pc: acpi: move DBUG() from DSDT to SSDT
> >   pc: acpi: move RTC device from DSDT to SSDT
> >   pc: acpi: move KBD device from DSDT to SSDT
> >   pc: acpi: move MOU device from DSDT to SSDT
> >   pc: acpi: move FDC0 device from DSDT to SSDT
> >   pc: acpi: move LPT device from DSDT to SSDT
> >   pc: acpi: move COM devices from DSDT to SSDT
> >   pc: acpi: move PIIX4 isa-bridge and pm devices into SSDT
> >   pc: acpi: move remaining GPE handlers into SSDT
> >   pc: acpi: pci: move link devices into SSDT
> >   pc: acpi: piix4: move IQCR() into SSDT
> >   pc: acpi: piix4: move IQST() into SSDT
> >   pc: acpi: piix4: move PCI0._PRT() into SSDT
> >   pc: acpi: piix4: move remaining PCI hotplug bits into SSDT
> >   pc: acpi: piix4: acpi move PCI0 device to SSDT
> >   pc: acpi: q35: move GSI links to SSDT
> >   pc: acpi: q35: move link devices to SSDT
> >   pc: acpi: q35: move IQCR() into SSDT
> >   pc: acpi: q35: move IQST() into SSDT
> >   pc: acpi: q35: move ISA bridge into SSDT
> >   pc: acpi: q35: move _PRT() into SSDT
> >   pc: acpi: q35: move PRTA routing table into SSDT
> >   pc: acpi: q35: move PRTP routing table into SSDT
> >   pc: acpi: q35: move _PIC() method into SSDT
> >   pc: acpi: q35: move PCI0._OSC() method into SSDT
> >   pc: acpi: q35: move PCI0 device definition into SSDT
> >   pc: acpi: q35: PCST, PCSB opregions and PCIB field into SSDT
> >   pc: acpi: switch to AML API composed DSDT
> >   pc: acpi: remove unused ASL templates and related blobs/utils
> > 
> > Xiao Guangrong (4):
> >   acpi: add aml_derefof
> >   acpi: add aml_sizeof
> >   acpi: add aml_mutex(), aml_acquire(), aml_release()
> >   acpi: support serialized method
> > 
> >  hw/acpi/Makefile.objs               |    4 +-
> >  hw/acpi/aml-build.c                 |  294 +-
> >  hw/acpi/cpu_hotplug_acpi_table.c    |  124 +
> >  hw/acpi/memory_hotplug_acpi_table.c |  249 ++
> >  hw/arm/virt-acpi-build.c            |   41 +-
> >  hw/i386/Makefile.objs               |   31 +-
> >  hw/i386/acpi-build.c                | 1329 ++++--
> >  hw/i386/acpi-dsdt-cpu-hotplug.dsl   |   90 -
> >  hw/i386/acpi-dsdt-dbug.dsl          |   41 -
> >  hw/i386/acpi-dsdt-hpet.dsl          |   48 -
> >  hw/i386/acpi-dsdt-isa.dsl           |  117 -
> >  hw/i386/acpi-dsdt-mem-hotplug.dsl   |  171 -
> >  hw/i386/acpi-dsdt.dsl               |  303 --
> >  hw/i386/acpi-dsdt.hex.generated     | 2972 --------------
> >  hw/i386/q35-acpi-dsdt.dsl           |  436 --
> >  hw/i386/q35-acpi-dsdt.hex.generated | 7610 -----------------------------------
> >  hw/timer/hpet.c                     |    2 +-
> >  include/hw/acpi/aml-build.h         |   66 +-
> >  include/hw/acpi/cpu_hotplug.h       |   10 +
> >  include/hw/acpi/memory_hotplug.h    |    9 +
> >  include/hw/acpi/pc-hotplug.h        |   44 +-
> >  include/hw/timer/hpet.h             |    1 +
> >  scripts/acpi_extract.py             |  367 --
> >  scripts/acpi_extract_preprocess.py  |   51 -
> >  scripts/update-acpi.sh              |    4 -
> >  tests/bios-tables-test.c            |    7 +
> >  26 files changed, 1848 insertions(+), 12573 deletions(-)
> >  create mode 100644 hw/acpi/cpu_hotplug_acpi_table.c
> >  create mode 100644 hw/acpi/memory_hotplug_acpi_table.c
> >  delete mode 100644 hw/i386/acpi-dsdt-cpu-hotplug.dsl
> >  delete mode 100644 hw/i386/acpi-dsdt-dbug.dsl
> >  delete mode 100644 hw/i386/acpi-dsdt-hpet.dsl
> >  delete mode 100644 hw/i386/acpi-dsdt-isa.dsl
> >  delete mode 100644 hw/i386/acpi-dsdt-mem-hotplug.dsl
> >  delete mode 100644 hw/i386/acpi-dsdt.dsl
> >  delete mode 100644 hw/i386/acpi-dsdt.hex.generated
> >  delete mode 100644 hw/i386/q35-acpi-dsdt.dsl
> >  delete mode 100644 hw/i386/q35-acpi-dsdt.hex.generated
> >  delete mode 100755 scripts/acpi_extract.py
> >  delete mode 100755 scripts/acpi_extract_preprocess.py
> >  delete mode 100644 scripts/update-acpi.sh
> > 
> > -- 
> > 1.8.3.1
> > 
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-arm] [Qemu-devel] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support
  2015-12-21 13:00   ` [Qemu-arm] [Qemu-devel] " Igor Mammedov
@ 2015-12-21 13:12     ` Shannon Zhao
  0 siblings, 0 replies; 15+ messages in thread
From: Shannon Zhao @ 2015-12-21 13:12 UTC (permalink / raw)
  To: Igor Mammedov, Michael S. Tsirkin
  Cc: Peter Maydell, Eduardo Habkost, qemu-devel, ARM ACPI Subsystem,
	Paolo Bonzini, Richard Henderson



On 2015/12/21 21:00, Igor Mammedov wrote:
> On Sat, 19 Dec 2015 22:38:41 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
>> > On Thu, Dec 10, 2015 at 12:40:54AM +0100, Igor Mammedov wrote:
>>> > > Due to huge size, CCing only cover letter instead of individual patches.
>>> > > 
>>> > > Series consist of 2 parts the 1st part prefixed 'acpi:' adds necessary
>>> > > AML API functions and the second part converts DSDT using existing and
>>> > > new AML API.
>>> > > 
>>> > > Series does exact byte by byte conversion and passes ACPI tables
>>> > > 'make check' tests.
>>> > > The conversion first moves common for PIIX4/Q35 parts, getting rid of *.dsl
>>> > > includes and then converts PIIX4 and Q35 parts of DSDT.
>>> > > 
>>> > > Diff-stat looks nice but actual code base is reduced by ~2000LOC
>>> > > while the rest of 10000 removals is dropping precompiled AML
>>> > > templates from tree.
>>> > > 
>>> > > There are some AML parts that could be optimized/simplified and shared
>>> > > between PIIX4/Q35/ARM but doing it will break exact match with original
>>> > > tests, hence it's left out of the scope of this series.
>>> > > 
>>> > > CC: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
>>> > > CC: Shannon Zhao <zhaoshenglong@huawei.com> (maintainer:ARM ACPI Subsystem)
>>> > > CC: Peter Maydell <peter.maydell@linaro.org> (maintainer:ARM)
>>> > > CC: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86)
>>> > > CC: Richard Henderson <rth@twiddle.net> (maintainer:X86)
>>> > > CC: Eduardo Habkost <ehabkost@redhat.com> (maintainer:X86)
>>> > > CC: qemu-arm@nongnu.org (open list:ARM ACPI Subsystem)
>> > 
>> > I think this is a nice series, and I think we can live with the fact
>> > make check warns in the middle.
>> > 
>> > I sent some comments I'd like to see addressed.
>> > 
>> > Main points
>> > 	- one v2 patch seems garbled
>> > 	- please don't create functions with tons of parameters
>> > 	  which just do if (a) on each of these internally.
>> > 	  instead, move common code into a small function that
>> > 	  can be reused without so many conditionals.
>> > 	- I really dislike local variables starting with a_ for
>> > 	  no good reason.
> Michael,
> 
> Could you apply already reviewed AML API patches 1-24
> and push it to master early, pls?
> 
+1

BTW, Below patches are already in master.

[PATCH 15/74] acpi: support serialized method
[PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs

> That will help to reduce collisions with other AML
> patches on list and provide more complete API for others to use.
> 

-- 
Shannon


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-arm] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs
  2015-12-09 23:41 ` [Qemu-arm] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs Igor Mammedov
  2015-12-10  1:50   ` Shannon Zhao
@ 2015-12-22 15:17   ` Michael S. Tsirkin
  2015-12-22 15:37     ` Igor Mammedov
  1 sibling, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2015-12-22 15:17 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: qemu-arm, qemu-devel, zhaoshenglong

On Thu, Dec 10, 2015 at 12:41:18AM +0100, Igor Mammedov wrote:
> ASL Interrupt() macro translates to Extended Interrupt Descriptor
> which supports variable number of IRQs. It will be used for
> conversion of ASL code for pc/q35 machines that use it for
> returning several IRQs in _PSR object.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> CC: zhaoshenglong@huawei.com
> CC: qemu-arm@nongnu.org
> ---
>  hw/acpi/aml-build.c         | 22 +++++++++++++---------
>  hw/arm/virt-acpi-build.c    | 23 ++++++++++++-----------
>  include/hw/acpi/aml-build.h |  2 +-
>  3 files changed, 26 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 2ca9207..ee64d12 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -667,23 +667,27 @@ Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
>  Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
>                     AmlLevelAndEdge level_and_edge,
>                     AmlActiveHighAndLow high_and_low, AmlShared shared,
> -                   uint32_t irq)
> +                   uint32_t *irq_list, uint8_t irq_count)

const uint32_t *irq_list?

>  {
> +    int i;
>      Aml *var = aml_alloc();
>      uint8_t irq_flags = con_and_pro | (level_and_edge << 1)
>                          | (high_and_low << 2) | (shared << 3);
> +    const int header_bytes_in_len = 2;
> +    uint16_t len = header_bytes_in_len + irq_count * sizeof(uint32_t);
> +
> +    assert(irq_count > 0);
>  
>      build_append_byte(var->buf, 0x89); /* Extended irq descriptor */
> -    build_append_byte(var->buf, 6); /* Length, bits[7:0] minimum value = 6 */
> -    build_append_byte(var->buf, 0); /* Length, bits[15:8] minimum value = 0 */
> +    build_append_byte(var->buf, len & 0xFF); /* Length, bits[7:0] */
> +    build_append_byte(var->buf, len >> 8); /* Length, bits[15:8] */

build_append_int_noprefix ?

Which really needs a better name btw.



>      build_append_byte(var->buf, irq_flags); /* Interrupt Vector Information. */
> -    build_append_byte(var->buf, 0x01);      /* Interrupt table length = 1 */
> +    build_append_byte(var->buf, irq_count);   /* Interrupt table length */
>  
> -    /* Interrupt Number */
> -    build_append_byte(var->buf, extract32(irq, 0, 8));  /* bits[7:0] */
> -    build_append_byte(var->buf, extract32(irq, 8, 8));  /* bits[15:8] */
> -    build_append_byte(var->buf, extract32(irq, 16, 8)); /* bits[23:16] */
> -    build_append_byte(var->buf, extract32(irq, 24, 8)); /* bits[31:24] */
> +    /* Interrupt Number List */
> +    for (i = 0; i < irq_count; i++) {
> +        build_append_int_noprefix(var->buf, irq_list[i], 4);
> +    }
>      return var;
>  }
>  
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 698b5f2..02ba822 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -71,7 +71,7 @@ static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
>  }
>  
>  static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> -                                           int uart_irq)
> +                                           uint32_t uart_irq)
>  {
>      Aml *dev = aml_device("COM0");
>      aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011")));
> @@ -82,7 +82,7 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>                                         uart_memmap->size, AML_READ_WRITE));
>      aml_append(crs,
>                 aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> -                             AML_EXCLUSIVE, uart_irq));
> +                             AML_EXCLUSIVE, &uart_irq, 1));
>      aml_append(dev, aml_name_decl("_CRS", crs));
>  
>      /* The _ADR entry is used to link this device to the UART described
> @@ -94,7 +94,7 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>  }
>  
>  static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
> -                                          int rtc_irq)
> +                                          uint32_t rtc_irq)
>  {
>      Aml *dev = aml_device("RTC0");
>      aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
> @@ -105,7 +105,7 @@ static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
>                                         rtc_memmap->size, AML_READ_WRITE));
>      aml_append(crs,
>                 aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> -                             AML_EXCLUSIVE, rtc_irq));
> +                             AML_EXCLUSIVE, &rtc_irq, 1));
>      aml_append(dev, aml_name_decl("_CRS", crs));
>      aml_append(scope, dev);
>  }
> @@ -136,11 +136,11 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
>  
>  static void acpi_dsdt_add_virtio(Aml *scope,
>                                   const MemMapEntry *virtio_mmio_memmap,
> -                                 int mmio_irq, int num)
> +                                 uint32_t mmio_irq, int num)
>  {
>      hwaddr base = virtio_mmio_memmap->base;
>      hwaddr size = virtio_mmio_memmap->size;
> -    int irq = mmio_irq;
> +    uint32_t irq = mmio_irq;
>      int i;
>  
>      for (i = 0; i < num; i++) {
> @@ -152,15 +152,15 @@ static void acpi_dsdt_add_virtio(Aml *scope,
>          aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
>          aml_append(crs,
>                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> -                                 AML_EXCLUSIVE, irq + i));
> +                                 AML_EXCLUSIVE, &irq, 1));
>          aml_append(dev, aml_name_decl("_CRS", crs));
>          aml_append(scope, dev);
>          base += size;
>      }
>  }
>  
> -static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
> -                              bool use_highmem)
> +static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> +                              uint32_t irq, bool use_highmem)
>  {
>      Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
>      int i, bus_no;
> @@ -199,18 +199,19 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
>  
>      /* Create GSI link device */
>      for (i = 0; i < PCI_NUM_PINS; i++) {
> +        uint32_t irqs =  irq + i;
>          Aml *dev_gsi = aml_device("GSI%d", i);
>          aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F")));
>          aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
>          crs = aml_resource_template();
>          aml_append(crs,
>                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> -                                 AML_EXCLUSIVE, irq + i));
> +                                 AML_EXCLUSIVE, &irqs, 1));
>          aml_append(dev_gsi, aml_name_decl("_PRS", crs));
>          crs = aml_resource_template();
>          aml_append(crs,
>                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> -                                 AML_EXCLUSIVE, irq + i));
> +                                 AML_EXCLUSIVE, &irqs, 1));
>          aml_append(dev_gsi, aml_name_decl("_CRS", crs));
>          method = aml_method("_SRS", 1, AML_NOTSERIALIZED);
>          aml_append(dev_gsi, method);
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index ca365c9..a3a058f 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -253,7 +253,7 @@ Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
>  Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
>                     AmlLevelAndEdge level_and_edge,
>                     AmlActiveHighAndLow high_and_low, AmlShared shared,
> -                   uint32_t irq);
> +                   uint32_t *irq_list, uint8_t irq_count);
>  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
>              uint8_t aln, uint8_t len);
>  Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
> -- 
> 1.8.3.1
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-arm] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs
  2015-12-22 15:17   ` Michael S. Tsirkin
@ 2015-12-22 15:37     ` Igor Mammedov
  2015-12-22 15:58       ` Michael S. Tsirkin
  0 siblings, 1 reply; 15+ messages in thread
From: Igor Mammedov @ 2015-12-22 15:37 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-arm, qemu-devel, zhaoshenglong

On Tue, 22 Dec 2015 17:17:33 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Dec 10, 2015 at 12:41:18AM +0100, Igor Mammedov wrote:
> > ASL Interrupt() macro translates to Extended Interrupt Descriptor
> > which supports variable number of IRQs. It will be used for
> > conversion of ASL code for pc/q35 machines that use it for
> > returning several IRQs in _PSR object.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > CC: zhaoshenglong@huawei.com
> > CC: qemu-arm@nongnu.org
> > ---
> >  hw/acpi/aml-build.c         | 22 +++++++++++++---------
> >  hw/arm/virt-acpi-build.c    | 23 ++++++++++++-----------
> >  include/hw/acpi/aml-build.h |  2 +-
> >  3 files changed, 26 insertions(+), 21 deletions(-)
> > 
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index 2ca9207..ee64d12 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
> > @@ -667,23 +667,27 @@ Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
> >  Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
> >                     AmlLevelAndEdge level_and_edge,
> >                     AmlActiveHighAndLow high_and_low, AmlShared shared,
> > -                   uint32_t irq)
> > +                   uint32_t *irq_list, uint8_t irq_count)
> 
> const uint32_t *irq_list?
will do it on top as it's in master by now.

> 
> >  {
> > +    int i;
> >      Aml *var = aml_alloc();
> >      uint8_t irq_flags = con_and_pro | (level_and_edge << 1)
> >                          | (high_and_low << 2) | (shared << 3);
> > +    const int header_bytes_in_len = 2;
> > +    uint16_t len = header_bytes_in_len + irq_count * sizeof(uint32_t);
> > +
> > +    assert(irq_count > 0);
> >  
> >      build_append_byte(var->buf, 0x89); /* Extended irq descriptor */
> > -    build_append_byte(var->buf, 6); /* Length, bits[7:0] minimum value = 6 */
> > -    build_append_byte(var->buf, 0); /* Length, bits[15:8] minimum value = 0 */
> > +    build_append_byte(var->buf, len & 0xFF); /* Length, bits[7:0] */
> > +    build_append_byte(var->buf, len >> 8); /* Length, bits[15:8] */
> 
> build_append_int_noprefix ?
it will do job to, though I'd prefer it by byte as it exactly matches
table description in spec.

> 
> Which really needs a better name btw.
I'd like to get rid of long build_append_ prefix but would like to
keep aml_ one only for ASL constructs.
How about 'acpi_int(Aml*, val, sz)' replacing both 'build_append_int[_noprefix]()'
where if sz == 0 do what build_append_int() does.

same name suggestion goes for byte:
  s/build_append_byte/acpi_byte/

> 
> 
> 
> >      build_append_byte(var->buf, irq_flags); /* Interrupt Vector Information. */
> > -    build_append_byte(var->buf, 0x01);      /* Interrupt table length = 1 */
> > +    build_append_byte(var->buf, irq_count);   /* Interrupt table length */
> >  
> > -    /* Interrupt Number */
> > -    build_append_byte(var->buf, extract32(irq, 0, 8));  /* bits[7:0] */
> > -    build_append_byte(var->buf, extract32(irq, 8, 8));  /* bits[15:8] */
> > -    build_append_byte(var->buf, extract32(irq, 16, 8)); /* bits[23:16] */
> > -    build_append_byte(var->buf, extract32(irq, 24, 8)); /* bits[31:24] */
> > +    /* Interrupt Number List */
> > +    for (i = 0; i < irq_count; i++) {
> > +        build_append_int_noprefix(var->buf, irq_list[i], 4);
> > +    }
> >      return var;
> >  }
> >  
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 698b5f2..02ba822 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -71,7 +71,7 @@ static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
> >  }
> >  
> >  static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> > -                                           int uart_irq)
> > +                                           uint32_t uart_irq)
> >  {
> >      Aml *dev = aml_device("COM0");
> >      aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011")));
> > @@ -82,7 +82,7 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> >                                         uart_memmap->size, AML_READ_WRITE));
> >      aml_append(crs,
> >                 aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > -                             AML_EXCLUSIVE, uart_irq));
> > +                             AML_EXCLUSIVE, &uart_irq, 1));
> >      aml_append(dev, aml_name_decl("_CRS", crs));
> >  
> >      /* The _ADR entry is used to link this device to the UART described
> > @@ -94,7 +94,7 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> >  }
> >  
> >  static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
> > -                                          int rtc_irq)
> > +                                          uint32_t rtc_irq)
> >  {
> >      Aml *dev = aml_device("RTC0");
> >      aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
> > @@ -105,7 +105,7 @@ static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
> >                                         rtc_memmap->size, AML_READ_WRITE));
> >      aml_append(crs,
> >                 aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > -                             AML_EXCLUSIVE, rtc_irq));
> > +                             AML_EXCLUSIVE, &rtc_irq, 1));
> >      aml_append(dev, aml_name_decl("_CRS", crs));
> >      aml_append(scope, dev);
> >  }
> > @@ -136,11 +136,11 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
> >  
> >  static void acpi_dsdt_add_virtio(Aml *scope,
> >                                   const MemMapEntry *virtio_mmio_memmap,
> > -                                 int mmio_irq, int num)
> > +                                 uint32_t mmio_irq, int num)
> >  {
> >      hwaddr base = virtio_mmio_memmap->base;
> >      hwaddr size = virtio_mmio_memmap->size;
> > -    int irq = mmio_irq;
> > +    uint32_t irq = mmio_irq;
> >      int i;
> >  
> >      for (i = 0; i < num; i++) {
> > @@ -152,15 +152,15 @@ static void acpi_dsdt_add_virtio(Aml *scope,
> >          aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
> >          aml_append(crs,
> >                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > -                                 AML_EXCLUSIVE, irq + i));
> > +                                 AML_EXCLUSIVE, &irq, 1));
> >          aml_append(dev, aml_name_decl("_CRS", crs));
> >          aml_append(scope, dev);
> >          base += size;
> >      }
> >  }
> >  
> > -static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
> > -                              bool use_highmem)
> > +static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> > +                              uint32_t irq, bool use_highmem)
> >  {
> >      Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
> >      int i, bus_no;
> > @@ -199,18 +199,19 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
> >  
> >      /* Create GSI link device */
> >      for (i = 0; i < PCI_NUM_PINS; i++) {
> > +        uint32_t irqs =  irq + i;
> >          Aml *dev_gsi = aml_device("GSI%d", i);
> >          aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F")));
> >          aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
> >          crs = aml_resource_template();
> >          aml_append(crs,
> >                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > -                                 AML_EXCLUSIVE, irq + i));
> > +                                 AML_EXCLUSIVE, &irqs, 1));
> >          aml_append(dev_gsi, aml_name_decl("_PRS", crs));
> >          crs = aml_resource_template();
> >          aml_append(crs,
> >                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > -                                 AML_EXCLUSIVE, irq + i));
> > +                                 AML_EXCLUSIVE, &irqs, 1));
> >          aml_append(dev_gsi, aml_name_decl("_CRS", crs));
> >          method = aml_method("_SRS", 1, AML_NOTSERIALIZED);
> >          aml_append(dev_gsi, method);
> > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> > index ca365c9..a3a058f 100644
> > --- a/include/hw/acpi/aml-build.h
> > +++ b/include/hw/acpi/aml-build.h
> > @@ -253,7 +253,7 @@ Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
> >  Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
> >                     AmlLevelAndEdge level_and_edge,
> >                     AmlActiveHighAndLow high_and_low, AmlShared shared,
> > -                   uint32_t irq);
> > +                   uint32_t *irq_list, uint8_t irq_count);
> >  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
> >              uint8_t aln, uint8_t len);
> >  Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
> > -- 
> > 1.8.3.1
> > 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-arm] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs
  2015-12-22 15:37     ` Igor Mammedov
@ 2015-12-22 15:58       ` Michael S. Tsirkin
  2015-12-22 16:19         ` Igor Mammedov
  0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2015-12-22 15:58 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: qemu-arm, qemu-devel, zhaoshenglong

On Tue, Dec 22, 2015 at 04:37:11PM +0100, Igor Mammedov wrote:
> On Tue, 22 Dec 2015 17:17:33 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Thu, Dec 10, 2015 at 12:41:18AM +0100, Igor Mammedov wrote:
> > > ASL Interrupt() macro translates to Extended Interrupt Descriptor
> > > which supports variable number of IRQs. It will be used for
> > > conversion of ASL code for pc/q35 machines that use it for
> > > returning several IRQs in _PSR object.
> > > 
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > > CC: zhaoshenglong@huawei.com
> > > CC: qemu-arm@nongnu.org
> > > ---
> > >  hw/acpi/aml-build.c         | 22 +++++++++++++---------
> > >  hw/arm/virt-acpi-build.c    | 23 ++++++++++++-----------
> > >  include/hw/acpi/aml-build.h |  2 +-
> > >  3 files changed, 26 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > > index 2ca9207..ee64d12 100644
> > > --- a/hw/acpi/aml-build.c
> > > +++ b/hw/acpi/aml-build.c
> > > @@ -667,23 +667,27 @@ Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
> > >  Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
> > >                     AmlLevelAndEdge level_and_edge,
> > >                     AmlActiveHighAndLow high_and_low, AmlShared shared,
> > > -                   uint32_t irq)
> > > +                   uint32_t *irq_list, uint8_t irq_count)
> > 
> > const uint32_t *irq_list?
> will do it on top as it's in master by now.
> 
> > 
> > >  {
> > > +    int i;
> > >      Aml *var = aml_alloc();
> > >      uint8_t irq_flags = con_and_pro | (level_and_edge << 1)
> > >                          | (high_and_low << 2) | (shared << 3);
> > > +    const int header_bytes_in_len = 2;
> > > +    uint16_t len = header_bytes_in_len + irq_count * sizeof(uint32_t);
> > > +
> > > +    assert(irq_count > 0);
> > >  
> > >      build_append_byte(var->buf, 0x89); /* Extended irq descriptor */
> > > -    build_append_byte(var->buf, 6); /* Length, bits[7:0] minimum value = 6 */
> > > -    build_append_byte(var->buf, 0); /* Length, bits[15:8] minimum value = 0 */
> > > +    build_append_byte(var->buf, len & 0xFF); /* Length, bits[7:0] */
> > > +    build_append_byte(var->buf, len >> 8); /* Length, bits[15:8] */
> > 
> > build_append_int_noprefix ?
> it will do job to, though I'd prefer it by byte as it exactly matches
> table description in spec.

ok then, it's not a big deal.

> > 
> > Which really needs a better name btw.
> I'd like to get rid of long build_append_ prefix but would like to
> keep aml_ one only for ASL constructs.
> How about 'acpi_int(Aml*, val, sz)' replacing both 'build_append_int[_noprefix]()'
> where if sz == 0 do what build_append_int() does.

We can't drop GArray things yet - I want acpi tables to be
built like this and get rid of structs.

> same name suggestion goes for byte:
>   s/build_append_byte/acpi_byte/

I prefer append in there. byte implies it returns byte.

build_append_bytes?

> > 
> > 
> > 
> > >      build_append_byte(var->buf, irq_flags); /* Interrupt Vector Information. */
> > > -    build_append_byte(var->buf, 0x01);      /* Interrupt table length = 1 */
> > > +    build_append_byte(var->buf, irq_count);   /* Interrupt table length */
> > >  
> > > -    /* Interrupt Number */
> > > -    build_append_byte(var->buf, extract32(irq, 0, 8));  /* bits[7:0] */
> > > -    build_append_byte(var->buf, extract32(irq, 8, 8));  /* bits[15:8] */
> > > -    build_append_byte(var->buf, extract32(irq, 16, 8)); /* bits[23:16] */
> > > -    build_append_byte(var->buf, extract32(irq, 24, 8)); /* bits[31:24] */
> > > +    /* Interrupt Number List */
> > > +    for (i = 0; i < irq_count; i++) {
> > > +        build_append_int_noprefix(var->buf, irq_list[i], 4);
> > > +    }
> > >      return var;
> > >  }
> > >  
> > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > > index 698b5f2..02ba822 100644
> > > --- a/hw/arm/virt-acpi-build.c
> > > +++ b/hw/arm/virt-acpi-build.c
> > > @@ -71,7 +71,7 @@ static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
> > >  }
> > >  
> > >  static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> > > -                                           int uart_irq)
> > > +                                           uint32_t uart_irq)
> > >  {
> > >      Aml *dev = aml_device("COM0");
> > >      aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011")));
> > > @@ -82,7 +82,7 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> > >                                         uart_memmap->size, AML_READ_WRITE));
> > >      aml_append(crs,
> > >                 aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > -                             AML_EXCLUSIVE, uart_irq));
> > > +                             AML_EXCLUSIVE, &uart_irq, 1));
> > >      aml_append(dev, aml_name_decl("_CRS", crs));
> > >  
> > >      /* The _ADR entry is used to link this device to the UART described
> > > @@ -94,7 +94,7 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> > >  }
> > >  
> > >  static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
> > > -                                          int rtc_irq)
> > > +                                          uint32_t rtc_irq)
> > >  {
> > >      Aml *dev = aml_device("RTC0");
> > >      aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
> > > @@ -105,7 +105,7 @@ static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
> > >                                         rtc_memmap->size, AML_READ_WRITE));
> > >      aml_append(crs,
> > >                 aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > -                             AML_EXCLUSIVE, rtc_irq));
> > > +                             AML_EXCLUSIVE, &rtc_irq, 1));
> > >      aml_append(dev, aml_name_decl("_CRS", crs));
> > >      aml_append(scope, dev);
> > >  }
> > > @@ -136,11 +136,11 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
> > >  
> > >  static void acpi_dsdt_add_virtio(Aml *scope,
> > >                                   const MemMapEntry *virtio_mmio_memmap,
> > > -                                 int mmio_irq, int num)
> > > +                                 uint32_t mmio_irq, int num)
> > >  {
> > >      hwaddr base = virtio_mmio_memmap->base;
> > >      hwaddr size = virtio_mmio_memmap->size;
> > > -    int irq = mmio_irq;
> > > +    uint32_t irq = mmio_irq;
> > >      int i;
> > >  
> > >      for (i = 0; i < num; i++) {
> > > @@ -152,15 +152,15 @@ static void acpi_dsdt_add_virtio(Aml *scope,
> > >          aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
> > >          aml_append(crs,
> > >                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > -                                 AML_EXCLUSIVE, irq + i));
> > > +                                 AML_EXCLUSIVE, &irq, 1));
> > >          aml_append(dev, aml_name_decl("_CRS", crs));
> > >          aml_append(scope, dev);
> > >          base += size;
> > >      }
> > >  }
> > >  
> > > -static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
> > > -                              bool use_highmem)
> > > +static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> > > +                              uint32_t irq, bool use_highmem)
> > >  {
> > >      Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
> > >      int i, bus_no;
> > > @@ -199,18 +199,19 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
> > >  
> > >      /* Create GSI link device */
> > >      for (i = 0; i < PCI_NUM_PINS; i++) {
> > > +        uint32_t irqs =  irq + i;
> > >          Aml *dev_gsi = aml_device("GSI%d", i);
> > >          aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F")));
> > >          aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
> > >          crs = aml_resource_template();
> > >          aml_append(crs,
> > >                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > -                                 AML_EXCLUSIVE, irq + i));
> > > +                                 AML_EXCLUSIVE, &irqs, 1));
> > >          aml_append(dev_gsi, aml_name_decl("_PRS", crs));
> > >          crs = aml_resource_template();
> > >          aml_append(crs,
> > >                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > -                                 AML_EXCLUSIVE, irq + i));
> > > +                                 AML_EXCLUSIVE, &irqs, 1));
> > >          aml_append(dev_gsi, aml_name_decl("_CRS", crs));
> > >          method = aml_method("_SRS", 1, AML_NOTSERIALIZED);
> > >          aml_append(dev_gsi, method);
> > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> > > index ca365c9..a3a058f 100644
> > > --- a/include/hw/acpi/aml-build.h
> > > +++ b/include/hw/acpi/aml-build.h
> > > @@ -253,7 +253,7 @@ Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
> > >  Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
> > >                     AmlLevelAndEdge level_and_edge,
> > >                     AmlActiveHighAndLow high_and_low, AmlShared shared,
> > > -                   uint32_t irq);
> > > +                   uint32_t *irq_list, uint8_t irq_count);
> > >  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
> > >              uint8_t aln, uint8_t len);
> > >  Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
> > > -- 
> > > 1.8.3.1
> > > 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-arm] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs
  2015-12-22 15:58       ` Michael S. Tsirkin
@ 2015-12-22 16:19         ` Igor Mammedov
  2015-12-22 16:43           ` Michael S. Tsirkin
  0 siblings, 1 reply; 15+ messages in thread
From: Igor Mammedov @ 2015-12-22 16:19 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-arm, qemu-devel, zhaoshenglong

On Tue, 22 Dec 2015 17:58:41 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Dec 22, 2015 at 04:37:11PM +0100, Igor Mammedov wrote:
> > On Tue, 22 Dec 2015 17:17:33 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > 
> > > On Thu, Dec 10, 2015 at 12:41:18AM +0100, Igor Mammedov wrote:
> > > > ASL Interrupt() macro translates to Extended Interrupt Descriptor
> > > > which supports variable number of IRQs. It will be used for
> > > > conversion of ASL code for pc/q35 machines that use it for
> > > > returning several IRQs in _PSR object.
> > > > 
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > ---
> > > > CC: zhaoshenglong@huawei.com
> > > > CC: qemu-arm@nongnu.org
> > > > ---
> > > >  hw/acpi/aml-build.c         | 22 +++++++++++++---------
> > > >  hw/arm/virt-acpi-build.c    | 23 ++++++++++++-----------
> > > >  include/hw/acpi/aml-build.h |  2 +-
> > > >  3 files changed, 26 insertions(+), 21 deletions(-)
> > > > 
> > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > > > index 2ca9207..ee64d12 100644
> > > > --- a/hw/acpi/aml-build.c
> > > > +++ b/hw/acpi/aml-build.c
> > > > @@ -667,23 +667,27 @@ Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
> > > >  Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
> > > >                     AmlLevelAndEdge level_and_edge,
> > > >                     AmlActiveHighAndLow high_and_low, AmlShared shared,
> > > > -                   uint32_t irq)
> > > > +                   uint32_t *irq_list, uint8_t irq_count)
> > > 
> > > const uint32_t *irq_list?
> > will do it on top as it's in master by now.
> > 
> > > 
> > > >  {
> > > > +    int i;
> > > >      Aml *var = aml_alloc();
> > > >      uint8_t irq_flags = con_and_pro | (level_and_edge << 1)
> > > >                          | (high_and_low << 2) | (shared << 3);
> > > > +    const int header_bytes_in_len = 2;
> > > > +    uint16_t len = header_bytes_in_len + irq_count * sizeof(uint32_t);
> > > > +
> > > > +    assert(irq_count > 0);
> > > >  
> > > >      build_append_byte(var->buf, 0x89); /* Extended irq descriptor */
> > > > -    build_append_byte(var->buf, 6); /* Length, bits[7:0] minimum value = 6 */
> > > > -    build_append_byte(var->buf, 0); /* Length, bits[15:8] minimum value = 0 */
> > > > +    build_append_byte(var->buf, len & 0xFF); /* Length, bits[7:0] */
> > > > +    build_append_byte(var->buf, len >> 8); /* Length, bits[15:8] */
> > > 
> > > build_append_int_noprefix ?
> > it will do job to, though I'd prefer it by byte as it exactly matches
> > table description in spec.
> 
> ok then, it's not a big deal.
> 
> > > 
> > > Which really needs a better name btw.
> > I'd like to get rid of long build_append_ prefix but would like to
> > keep aml_ one only for ASL constructs.
> > How about 'acpi_int(Aml*, val, sz)' replacing both 'build_append_int[_noprefix]()'
> > where if sz == 0 do what build_append_int() does.
> 
> We can't drop GArray things yet - I want acpi tables to be
> built like this and get rid of structs.
it doesn't have to be Aml* it could be Garray*
the point were to have single build_append_int() which does prefix/noprefix job
but that might be confusing and easy to misuse, so scratch that off.

> 
> > same name suggestion goes for byte:
> >   s/build_append_byte/acpi_byte/
> 
> I prefer append in there. byte implies it returns byte.
> 
> build_append_bytes?
'bytes' doesn't imply any encoding rules, while build_append_int_noprefix
implies ACPI integer encoding and that was whole point of adding it
so user whon't have to care about endianness.

> 
> > > 
> > > 
> > > 
> > > >      build_append_byte(var->buf, irq_flags); /* Interrupt Vector Information. */
> > > > -    build_append_byte(var->buf, 0x01);      /* Interrupt table length = 1 */
> > > > +    build_append_byte(var->buf, irq_count);   /* Interrupt table length */
> > > >  
> > > > -    /* Interrupt Number */
> > > > -    build_append_byte(var->buf, extract32(irq, 0, 8));  /* bits[7:0] */
> > > > -    build_append_byte(var->buf, extract32(irq, 8, 8));  /* bits[15:8] */
> > > > -    build_append_byte(var->buf, extract32(irq, 16, 8)); /* bits[23:16] */
> > > > -    build_append_byte(var->buf, extract32(irq, 24, 8)); /* bits[31:24] */
> > > > +    /* Interrupt Number List */
> > > > +    for (i = 0; i < irq_count; i++) {
> > > > +        build_append_int_noprefix(var->buf, irq_list[i], 4);
> > > > +    }
> > > >      return var;
> > > >  }
> > > >  
> > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > > > index 698b5f2..02ba822 100644
> > > > --- a/hw/arm/virt-acpi-build.c
> > > > +++ b/hw/arm/virt-acpi-build.c
> > > > @@ -71,7 +71,7 @@ static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
> > > >  }
> > > >  
> > > >  static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> > > > -                                           int uart_irq)
> > > > +                                           uint32_t uart_irq)
> > > >  {
> > > >      Aml *dev = aml_device("COM0");
> > > >      aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011")));
> > > > @@ -82,7 +82,7 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> > > >                                         uart_memmap->size, AML_READ_WRITE));
> > > >      aml_append(crs,
> > > >                 aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > > -                             AML_EXCLUSIVE, uart_irq));
> > > > +                             AML_EXCLUSIVE, &uart_irq, 1));
> > > >      aml_append(dev, aml_name_decl("_CRS", crs));
> > > >  
> > > >      /* The _ADR entry is used to link this device to the UART described
> > > > @@ -94,7 +94,7 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> > > >  }
> > > >  
> > > >  static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
> > > > -                                          int rtc_irq)
> > > > +                                          uint32_t rtc_irq)
> > > >  {
> > > >      Aml *dev = aml_device("RTC0");
> > > >      aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
> > > > @@ -105,7 +105,7 @@ static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
> > > >                                         rtc_memmap->size, AML_READ_WRITE));
> > > >      aml_append(crs,
> > > >                 aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > > -                             AML_EXCLUSIVE, rtc_irq));
> > > > +                             AML_EXCLUSIVE, &rtc_irq, 1));
> > > >      aml_append(dev, aml_name_decl("_CRS", crs));
> > > >      aml_append(scope, dev);
> > > >  }
> > > > @@ -136,11 +136,11 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
> > > >  
> > > >  static void acpi_dsdt_add_virtio(Aml *scope,
> > > >                                   const MemMapEntry *virtio_mmio_memmap,
> > > > -                                 int mmio_irq, int num)
> > > > +                                 uint32_t mmio_irq, int num)
> > > >  {
> > > >      hwaddr base = virtio_mmio_memmap->base;
> > > >      hwaddr size = virtio_mmio_memmap->size;
> > > > -    int irq = mmio_irq;
> > > > +    uint32_t irq = mmio_irq;
> > > >      int i;
> > > >  
> > > >      for (i = 0; i < num; i++) {
> > > > @@ -152,15 +152,15 @@ static void acpi_dsdt_add_virtio(Aml *scope,
> > > >          aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
> > > >          aml_append(crs,
> > > >                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > > -                                 AML_EXCLUSIVE, irq + i));
> > > > +                                 AML_EXCLUSIVE, &irq, 1));
> > > >          aml_append(dev, aml_name_decl("_CRS", crs));
> > > >          aml_append(scope, dev);
> > > >          base += size;
> > > >      }
> > > >  }
> > > >  
> > > > -static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
> > > > -                              bool use_highmem)
> > > > +static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> > > > +                              uint32_t irq, bool use_highmem)
> > > >  {
> > > >      Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
> > > >      int i, bus_no;
> > > > @@ -199,18 +199,19 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
> > > >  
> > > >      /* Create GSI link device */
> > > >      for (i = 0; i < PCI_NUM_PINS; i++) {
> > > > +        uint32_t irqs =  irq + i;
> > > >          Aml *dev_gsi = aml_device("GSI%d", i);
> > > >          aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F")));
> > > >          aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
> > > >          crs = aml_resource_template();
> > > >          aml_append(crs,
> > > >                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > > -                                 AML_EXCLUSIVE, irq + i));
> > > > +                                 AML_EXCLUSIVE, &irqs, 1));
> > > >          aml_append(dev_gsi, aml_name_decl("_PRS", crs));
> > > >          crs = aml_resource_template();
> > > >          aml_append(crs,
> > > >                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > > -                                 AML_EXCLUSIVE, irq + i));
> > > > +                                 AML_EXCLUSIVE, &irqs, 1));
> > > >          aml_append(dev_gsi, aml_name_decl("_CRS", crs));
> > > >          method = aml_method("_SRS", 1, AML_NOTSERIALIZED);
> > > >          aml_append(dev_gsi, method);
> > > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> > > > index ca365c9..a3a058f 100644
> > > > --- a/include/hw/acpi/aml-build.h
> > > > +++ b/include/hw/acpi/aml-build.h
> > > > @@ -253,7 +253,7 @@ Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
> > > >  Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
> > > >                     AmlLevelAndEdge level_and_edge,
> > > >                     AmlActiveHighAndLow high_and_low, AmlShared shared,
> > > > -                   uint32_t irq);
> > > > +                   uint32_t *irq_list, uint8_t irq_count);
> > > >  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
> > > >              uint8_t aln, uint8_t len);
> > > >  Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
> > > > -- 
> > > > 1.8.3.1
> > > > 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-arm] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs
  2015-12-22 16:19         ` Igor Mammedov
@ 2015-12-22 16:43           ` Michael S. Tsirkin
  0 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2015-12-22 16:43 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: qemu-arm, qemu-devel, zhaoshenglong

On Tue, Dec 22, 2015 at 05:19:02PM +0100, Igor Mammedov wrote:
> On Tue, 22 Dec 2015 17:58:41 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Tue, Dec 22, 2015 at 04:37:11PM +0100, Igor Mammedov wrote:
> > > On Tue, 22 Dec 2015 17:17:33 +0200
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > 
> > > > On Thu, Dec 10, 2015 at 12:41:18AM +0100, Igor Mammedov wrote:
> > > > > ASL Interrupt() macro translates to Extended Interrupt Descriptor
> > > > > which supports variable number of IRQs. It will be used for
> > > > > conversion of ASL code for pc/q35 machines that use it for
> > > > > returning several IRQs in _PSR object.
> > > > > 
> > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > > ---
> > > > > CC: zhaoshenglong@huawei.com
> > > > > CC: qemu-arm@nongnu.org
> > > > > ---
> > > > >  hw/acpi/aml-build.c         | 22 +++++++++++++---------
> > > > >  hw/arm/virt-acpi-build.c    | 23 ++++++++++++-----------
> > > > >  include/hw/acpi/aml-build.h |  2 +-
> > > > >  3 files changed, 26 insertions(+), 21 deletions(-)
> > > > > 
> > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > > > > index 2ca9207..ee64d12 100644
> > > > > --- a/hw/acpi/aml-build.c
> > > > > +++ b/hw/acpi/aml-build.c
> > > > > @@ -667,23 +667,27 @@ Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
> > > > >  Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
> > > > >                     AmlLevelAndEdge level_and_edge,
> > > > >                     AmlActiveHighAndLow high_and_low, AmlShared shared,
> > > > > -                   uint32_t irq)
> > > > > +                   uint32_t *irq_list, uint8_t irq_count)
> > > > 
> > > > const uint32_t *irq_list?
> > > will do it on top as it's in master by now.
> > > 
> > > > 
> > > > >  {
> > > > > +    int i;
> > > > >      Aml *var = aml_alloc();
> > > > >      uint8_t irq_flags = con_and_pro | (level_and_edge << 1)
> > > > >                          | (high_and_low << 2) | (shared << 3);
> > > > > +    const int header_bytes_in_len = 2;
> > > > > +    uint16_t len = header_bytes_in_len + irq_count * sizeof(uint32_t);
> > > > > +
> > > > > +    assert(irq_count > 0);
> > > > >  
> > > > >      build_append_byte(var->buf, 0x89); /* Extended irq descriptor */
> > > > > -    build_append_byte(var->buf, 6); /* Length, bits[7:0] minimum value = 6 */
> > > > > -    build_append_byte(var->buf, 0); /* Length, bits[15:8] minimum value = 0 */
> > > > > +    build_append_byte(var->buf, len & 0xFF); /* Length, bits[7:0] */
> > > > > +    build_append_byte(var->buf, len >> 8); /* Length, bits[15:8] */
> > > > 
> > > > build_append_int_noprefix ?
> > > it will do job to, though I'd prefer it by byte as it exactly matches
> > > table description in spec.
> > 
> > ok then, it's not a big deal.
> > 
> > > > 
> > > > Which really needs a better name btw.
> > > I'd like to get rid of long build_append_ prefix but would like to
> > > keep aml_ one only for ASL constructs.
> > > How about 'acpi_int(Aml*, val, sz)' replacing both 'build_append_int[_noprefix]()'
> > > where if sz == 0 do what build_append_int() does.
> > 
> > We can't drop GArray things yet - I want acpi tables to be
> > built like this and get rid of structs.
> it doesn't have to be Aml* it could be Garray*
> the point were to have single build_append_int() which does prefix/noprefix job
> but that might be confusing and easy to misuse, so scratch that off.
> 
> > 
> > > same name suggestion goes for byte:
> > >   s/build_append_byte/acpi_byte/
> > 
> > I prefer append in there. byte implies it returns byte.
> > 
> > build_append_bytes?
> 'bytes' doesn't imply any encoding rules, while build_append_int_noprefix
> implies ACPI integer encoding and that was whole point of adding it
> so user whon't have to care about endianness.

No - it's not integer encoding. It's merely LE (as most of ACPI).
We are adding integer byte by byte, not encoding it.

> > 
> > > > 
> > > > 
> > > > 
> > > > >      build_append_byte(var->buf, irq_flags); /* Interrupt Vector Information. */
> > > > > -    build_append_byte(var->buf, 0x01);      /* Interrupt table length = 1 */
> > > > > +    build_append_byte(var->buf, irq_count);   /* Interrupt table length */
> > > > >  
> > > > > -    /* Interrupt Number */
> > > > > -    build_append_byte(var->buf, extract32(irq, 0, 8));  /* bits[7:0] */
> > > > > -    build_append_byte(var->buf, extract32(irq, 8, 8));  /* bits[15:8] */
> > > > > -    build_append_byte(var->buf, extract32(irq, 16, 8)); /* bits[23:16] */
> > > > > -    build_append_byte(var->buf, extract32(irq, 24, 8)); /* bits[31:24] */
> > > > > +    /* Interrupt Number List */
> > > > > +    for (i = 0; i < irq_count; i++) {
> > > > > +        build_append_int_noprefix(var->buf, irq_list[i], 4);
> > > > > +    }
> > > > >      return var;
> > > > >  }
> > > > >  
> > > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > > > > index 698b5f2..02ba822 100644
> > > > > --- a/hw/arm/virt-acpi-build.c
> > > > > +++ b/hw/arm/virt-acpi-build.c
> > > > > @@ -71,7 +71,7 @@ static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
> > > > >  }
> > > > >  
> > > > >  static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> > > > > -                                           int uart_irq)
> > > > > +                                           uint32_t uart_irq)
> > > > >  {
> > > > >      Aml *dev = aml_device("COM0");
> > > > >      aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011")));
> > > > > @@ -82,7 +82,7 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> > > > >                                         uart_memmap->size, AML_READ_WRITE));
> > > > >      aml_append(crs,
> > > > >                 aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > > > -                             AML_EXCLUSIVE, uart_irq));
> > > > > +                             AML_EXCLUSIVE, &uart_irq, 1));
> > > > >      aml_append(dev, aml_name_decl("_CRS", crs));
> > > > >  
> > > > >      /* The _ADR entry is used to link this device to the UART described
> > > > > @@ -94,7 +94,7 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> > > > >  }
> > > > >  
> > > > >  static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
> > > > > -                                          int rtc_irq)
> > > > > +                                          uint32_t rtc_irq)
> > > > >  {
> > > > >      Aml *dev = aml_device("RTC0");
> > > > >      aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
> > > > > @@ -105,7 +105,7 @@ static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
> > > > >                                         rtc_memmap->size, AML_READ_WRITE));
> > > > >      aml_append(crs,
> > > > >                 aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > > > -                             AML_EXCLUSIVE, rtc_irq));
> > > > > +                             AML_EXCLUSIVE, &rtc_irq, 1));
> > > > >      aml_append(dev, aml_name_decl("_CRS", crs));
> > > > >      aml_append(scope, dev);
> > > > >  }
> > > > > @@ -136,11 +136,11 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
> > > > >  
> > > > >  static void acpi_dsdt_add_virtio(Aml *scope,
> > > > >                                   const MemMapEntry *virtio_mmio_memmap,
> > > > > -                                 int mmio_irq, int num)
> > > > > +                                 uint32_t mmio_irq, int num)
> > > > >  {
> > > > >      hwaddr base = virtio_mmio_memmap->base;
> > > > >      hwaddr size = virtio_mmio_memmap->size;
> > > > > -    int irq = mmio_irq;
> > > > > +    uint32_t irq = mmio_irq;
> > > > >      int i;
> > > > >  
> > > > >      for (i = 0; i < num; i++) {
> > > > > @@ -152,15 +152,15 @@ static void acpi_dsdt_add_virtio(Aml *scope,
> > > > >          aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
> > > > >          aml_append(crs,
> > > > >                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > > > -                                 AML_EXCLUSIVE, irq + i));
> > > > > +                                 AML_EXCLUSIVE, &irq, 1));
> > > > >          aml_append(dev, aml_name_decl("_CRS", crs));
> > > > >          aml_append(scope, dev);
> > > > >          base += size;
> > > > >      }
> > > > >  }
> > > > >  
> > > > > -static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
> > > > > -                              bool use_highmem)
> > > > > +static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> > > > > +                              uint32_t irq, bool use_highmem)
> > > > >  {
> > > > >      Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
> > > > >      int i, bus_no;
> > > > > @@ -199,18 +199,19 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
> > > > >  
> > > > >      /* Create GSI link device */
> > > > >      for (i = 0; i < PCI_NUM_PINS; i++) {
> > > > > +        uint32_t irqs =  irq + i;
> > > > >          Aml *dev_gsi = aml_device("GSI%d", i);
> > > > >          aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F")));
> > > > >          aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
> > > > >          crs = aml_resource_template();
> > > > >          aml_append(crs,
> > > > >                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > > > -                                 AML_EXCLUSIVE, irq + i));
> > > > > +                                 AML_EXCLUSIVE, &irqs, 1));
> > > > >          aml_append(dev_gsi, aml_name_decl("_PRS", crs));
> > > > >          crs = aml_resource_template();
> > > > >          aml_append(crs,
> > > > >                     aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> > > > > -                                 AML_EXCLUSIVE, irq + i));
> > > > > +                                 AML_EXCLUSIVE, &irqs, 1));
> > > > >          aml_append(dev_gsi, aml_name_decl("_CRS", crs));
> > > > >          method = aml_method("_SRS", 1, AML_NOTSERIALIZED);
> > > > >          aml_append(dev_gsi, method);
> > > > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> > > > > index ca365c9..a3a058f 100644
> > > > > --- a/include/hw/acpi/aml-build.h
> > > > > +++ b/include/hw/acpi/aml-build.h
> > > > > @@ -253,7 +253,7 @@ Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
> > > > >  Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
> > > > >                     AmlLevelAndEdge level_and_edge,
> > > > >                     AmlActiveHighAndLow high_and_low, AmlShared shared,
> > > > > -                   uint32_t irq);
> > > > > +                   uint32_t *irq_list, uint8_t irq_count);
> > > > >  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
> > > > >              uint8_t aln, uint8_t len);
> > > > >  Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
> > > > > -- 
> > > > > 1.8.3.1
> > > > > 

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-12-22 16:44 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-09 23:40 [Qemu-arm] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support Igor Mammedov
2015-12-09 23:41 ` [Qemu-arm] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs Igor Mammedov
2015-12-10  1:50   ` Shannon Zhao
2015-12-22 15:17   ` Michael S. Tsirkin
2015-12-22 15:37     ` Igor Mammedov
2015-12-22 15:58       ` Michael S. Tsirkin
2015-12-22 16:19         ` Igor Mammedov
2015-12-22 16:43           ` Michael S. Tsirkin
2015-12-10 15:53 ` [Qemu-devel] [PATCH 00/74] pc: acpi: convert DSDT to AML API and drop ASL templates support Marcel Apfelbaum
2015-12-10 16:31   ` Igor Mammedov
2015-12-13 15:24     ` [Qemu-arm] " Marcel Apfelbaum
2015-12-10 16:44 ` Igor Mammedov
2015-12-19 20:38 ` [Qemu-arm] " Michael S. Tsirkin
2015-12-21 13:00   ` [Qemu-arm] [Qemu-devel] " Igor Mammedov
2015-12-21 13:12     ` Shannon Zhao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).