From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40670) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBPKu-0003Rf-Rb for qemu-devel@nongnu.org; Tue, 22 Dec 2015 10:58:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aBPKt-0000oM-BH for qemu-devel@nongnu.org; Tue, 22 Dec 2015 10:58:52 -0500 Date: Tue, 22 Dec 2015 17:58:41 +0200 From: "Michael S. Tsirkin" Message-ID: <20151222174855-mutt-send-email-mst@redhat.com> References: <1449704528-289297-1-git-send-email-imammedo@redhat.com> <1449704528-289297-25-git-send-email-imammedo@redhat.com> <20151222171313-mutt-send-email-mst@redhat.com> <20151222163711.313eeca6@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151222163711.313eeca6@nial.brq.redhat.com> Subject: Re: [Qemu-devel] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-arm@nongnu.org, qemu-devel@nongnu.org, zhaoshenglong@huawei.com 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" 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 > > > --- > > > 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 > > >