From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51884) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yg97S-0005rm-MA for qemu-devel@nongnu.org; Thu, 09 Apr 2015 05:51:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yg97O-000451-6t for qemu-devel@nongnu.org; Thu, 09 Apr 2015 05:51:30 -0400 Received: from static.88-198-71-155.clients.your-server.de ([88.198.71.155]:36099 helo=socrates.bennee.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yg97N-00044X-O2 for qemu-devel@nongnu.org; Thu, 09 Apr 2015 05:51:26 -0400 References: <1428055432-12120-1-git-send-email-zhaoshenglong@huawei.com> <1428055432-12120-7-git-send-email-zhaoshenglong@huawei.com> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <1428055432-12120-7-git-send-email-zhaoshenglong@huawei.com> Date: Thu, 09 Apr 2015 10:51:33 +0100 Message-ID: <87d23d1vfe.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v4 06/20] hw/arm/virt-acpi-build: Generation of DSDT table for virt devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Shannon Zhao Cc: peter.maydell@linaro.org, hangaohuai@huawei.com, mst@redhat.com, a.spyridakis@virtualopensystems.com, msalter@redhat.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, peter.huangpeng@huawei.com, hanjun.guo@linaro.org, imammedo@redhat.com, pbonzini@redhat.com, lersek@redhat.com, christoffer.dall@linaro.org, shannon.zhao@linaro.org Shannon Zhao writes: > From: Shannon Zhao > > DSDT consists of the usual common table header plus a definition > block in AML encoding which describes all devices in the platform. > > After initializing DSDT with header information the namespace is > created which is followed by the device encodings. The devices are > described using the Resource Template for the 32-Bit Fixed Memory > Range and the Extended Interrupt Descriptors. > > Signed-off-by: Shannon Zhao > Signed-off-by: Shannon Zhao > --- > hw/arm/virt-acpi-build.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 137 insertions(+) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 388838a..516c1d0 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -57,6 +57,139 @@ > #define ACPI_BUILD_DPRINTF(fmt, ...) > #endif > > +static void acpi_dsdt_add_cpus(Aml *scope, int max_cpus) > +{ > + Aml *dev, *crs; > + int i; > + char name[5]; name, dev and crs could be declared inside the for() loop. > + for (i = 0; i < max_cpus; i++) { > + snprintf(name, 5, "CPU%u", i); What happens here if you have 10 or 100 CPUs? > + dev = aml_device("%s", name); Also aml_device seems to take a format string so why not simply: dev = aml_device("CPU%u", i) > + aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007"))); > + aml_append(dev, aml_name_decl("_UID", aml_int(i))); > + crs = aml_resource_template(); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > + } > +} > + > +static void acpi_dsdt_add_uart(Aml *scope, const hwaddr *uart_addr, > + const int *uart_irq) > +{ > + Aml *dev, *crs; > + > + dev = aml_device("COM0"); > + aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011"))); > + aml_append(dev, aml_name_decl("_UID", aml_int(0))); > + > + crs = aml_resource_template(); > + aml_append(crs, > + aml_memory32_fixed(uart_addr[0], uart_addr[1], 0x01)); > + aml_append(crs, > + aml_interrupt(0x01, *uart_irq + 32)); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > +} > + > +static void acpi_dsdt_add_rtc(Aml *scope, const hwaddr *rtc_addr, > + const int *rtc_irq) > +{ > + Aml *dev, *crs; > + > + dev = aml_device("RTC0"); > + aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013"))); > + aml_append(dev, aml_name_decl("_UID", aml_int(0))); > + > + crs = aml_resource_template(); > + aml_append(crs, > + aml_memory32_fixed(rtc_addr[0], rtc_addr[1], 0x01)); > + aml_append(crs, > + aml_interrupt(0x01, *rtc_irq + 32)); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > +} > + > +static void acpi_dsdt_add_flash(Aml *scope, const hwaddr *flash_addr) > +{ > + Aml *dev, *crs; > + hwaddr base = flash_addr[0]; > + hwaddr size = flash_addr[1]; > + > + dev = aml_device("FLS0"); > + aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015"))); > + aml_append(dev, aml_name_decl("_UID", aml_int(0))); > + > + crs = aml_resource_template(); > + aml_append(crs, > + aml_memory32_fixed(base, size, 0x01)); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > + > + dev = aml_device("FLS1"); > + aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015"))); > + aml_append(dev, aml_name_decl("_UID", aml_int(1))); > + crs = aml_resource_template(); > + aml_append(crs, > + aml_memory32_fixed(base + size, size, 0x01)); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > +} > + > +static void acpi_dsdt_add_virtio(Aml *scope, const hwaddr *mmio_addrs, > + const int *mmio_irq, int num) > +{ > + Aml *dev, *crs; > + hwaddr base = mmio_addrs[0]; > + hwaddr size = mmio_addrs[1]; What ensures all these hw addresses are in 32 bit space on 64 bit platforms? > + int irq = *mmio_irq + 32; > + int i; > + char name[5]; > + > + for (i = 0; i < num; i++) { > + snprintf(name, 5, "VR%02u", i); > + dev = aml_device("%s", name); Again why not call aml_device directly? > + aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005"))); > + aml_append(dev, aml_name_decl("_UID", aml_int(i))); > + > + crs = aml_resource_template(); > + aml_append(crs, > + aml_memory32_fixed(base, size, 0x01)); > + aml_append(crs, > + aml_interrupt(0x01, irq + i)); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > + base += size; > + } > +} > + > +/* DSDT */ > +static void > +build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info) > +{ > + Aml *scope, *dsdt; > + acpi_dsdt_info *info = guest_info->dsdt_info; > + > + dsdt = init_aml_allocator(); > + /* Reserve space for header */ > + acpi_data_push(dsdt->buf, sizeof(AcpiTableHeader)); > + > + scope = aml_scope("\\_SB"); > + acpi_dsdt_add_cpus(scope, guest_info->max_cpus); > + acpi_dsdt_add_uart(scope, info->uart_addr, info->uart_irq); > + acpi_dsdt_add_rtc(scope, info->rtc_addr, info->rtc_irq); > + acpi_dsdt_add_flash(scope, info->flash_addr); > + acpi_dsdt_add_virtio(scope, info->virtio_mmio_addr, > + info->virtio_mmio_irq, info->virtio_mmio_num); > + > + aml_append(dsdt, scope); > + /* copy AML table into ACPI tables blob and patch header there */ > + g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len); > + build_header(linker, table_data, > + (void *)(table_data->data + table_data->len - dsdt->buf->len), > + "DSDT", dsdt->buf->len, 1); > + free_aml_allocator(); > +} > + > typedef > struct AcpiBuildState { > /* Copy of table in RAM (for patching). */ > @@ -72,6 +205,7 @@ static > void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables) > { > GArray *table_offsets; > + GArray *tables_blob = tables->table_data; > > table_offsets = g_array_new(false, true /* clear */, > sizeof(uint32_t)); > @@ -89,6 +223,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables) > * DSDT > */ > > + /* DSDT is pointed to by FADT */ > + build_dsdt(tables_blob, tables->linker, guest_info); > + > /* Cleanup memory that's no longer used. */ > g_array_free(table_offsets, true); > } -- Alex Bennée