From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56796) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z1X6T-00056n-WA for qemu-devel@nongnu.org; Sun, 07 Jun 2015 05:42:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z1X6Q-0006S7-Pf for qemu-devel@nongnu.org; Sun, 07 Jun 2015 05:42:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49043) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z1X6Q-0006RL-I2 for qemu-devel@nongnu.org; Sun, 07 Jun 2015 05:42:50 -0400 Date: Sun, 7 Jun 2015 11:42:44 +0200 From: "Michael S. Tsirkin" Message-ID: <20150607114023-mutt-send-email-mst@redhat.com> References: <1433434873-25200-1-git-send-email-mst@redhat.com> <1433434873-25200-4-git-send-email-mst@redhat.com> <55710BA0.8060104@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55710BA0.8060104@huawei.com> Subject: Re: [Qemu-devel] [PATCH 3/4] i386/acpi: add XSDT List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Shannon Zhao Cc: Eduardo Habkost , ghammer@redhat.com, qemu-devel@nongnu.org, shannon.zhao@linaro.org, pbonzini@redhat.com, imammedo@redhat.com, lersek@redhat.com, Richard Henderson On Fri, Jun 05, 2015 at 10:38:24AM +0800, Shannon Zhao wrote: > > > On 2015/6/5 0:21, Michael S. Tsirkin wrote: > > At the moment it mirrors RSDT exactly. > > > > Signed-off-by: Michael S. Tsirkin > > --- > > include/hw/acpi/acpi-defs.h | 15 ++++++++++++--- > > include/hw/acpi/aml-build.h | 2 ++ > > hw/acpi/aml-build.c | 41 ++++++++++++++++++++++++++++------------- > > hw/i386/acpi-build.c | 29 ++++++++++++++++++++++++----- > > 4 files changed, 66 insertions(+), 21 deletions(-) > > > > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h > > index 59cf277..fe1dc7d 100644 > > --- a/include/hw/acpi/acpi-defs.h > > +++ b/include/hw/acpi/acpi-defs.h > > @@ -201,13 +201,22 @@ enum { > > */ > > struct AcpiRsdtDescriptorRev1 > > { > > - ACPI_TABLE_HEADER_DEF /* ACPI common table header */ > > - uint32_t table_offset_entry[0]; /* Array of pointers to other */ > > - /* ACPI tables */ > > + ACPI_TABLE_HEADER_DEF /* ACPI common table header */ > > + uint32_t table_offset_entry[0]; /* Array of pointers to other ACPI tables */ > > } QEMU_PACKED; > > typedef struct AcpiRsdtDescriptorRev1 AcpiRsdtDescriptorRev1; > > > > /* > > + * ACPI 2.0 Extended System Description Table (XSDT) > > + */ > > +struct AcpiXsdtDescriptor > > +{ > > + ACPI_TABLE_HEADER_DEF /* ACPI common table header */ > > + uint64_t table_offset_entry[0]; /* Array of pointers to other ACPI tables */ > > +} QEMU_PACKED; > > +typedef struct AcpiXsdtDescriptor AcpiXsdtDescriptor; > > + > > +/* > > * ACPI 1.0 Firmware ACPI Control Structure (FACS) > > */ > > struct AcpiFacsDescriptorRev1 > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > > index edc1dfa..0a00402 100644 > > --- a/include/hw/acpi/aml-build.h > > +++ b/include/hw/acpi/aml-build.h > > @@ -287,5 +287,7 @@ void acpi_build_tables_init(AcpiBuildTables *tables); > > void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre); > > void > > build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets); > > +void > > +build_xsdt(GArray *table_data, GArray *linker, GArray *table_offsets); > > > > #endif > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > index 4c930b6..d0e52c4 100644 > > --- a/hw/acpi/aml-build.c > > +++ b/hw/acpi/aml-build.c > > @@ -1200,25 +1200,40 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre) > > } > > > > /* Build rsdt table */ > > -void > > -build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets) > > +static void > > +build_sdt(GArray *table_data, GArray *linker, GArray *table_offsets, > > + const char *sig, int entry_size) > > { > > - AcpiRsdtDescriptorRev1 *rsdt; > > - size_t rsdt_len; > > + AcpiTableHeader *sdt; > > + size_t sdt_len; > > int i; > > - const int table_data_len = (sizeof(uint32_t) * table_offsets->len); > > + const int table_data_len = (entry_size * table_offsets->len); > > + char *entries; > > > > - rsdt_len = sizeof(*rsdt) + table_data_len; > > - rsdt = acpi_data_push(table_data, rsdt_len); > > - memcpy(rsdt->table_offset_entry, table_offsets->data, table_data_len); > > + sdt_len = sizeof(*sdt) + table_data_len; > > + sdt = acpi_data_push(table_data, sdt_len); > > + entries = ((char *)sdt) + sizeof(*sdt); > > + memcpy(entries, table_offsets->data, table_data_len); > > for (i = 0; i < table_offsets->len; ++i) { > > - /* rsdt->table_offset_entry to be filled by Guest linker */ > > + /* entry to be filled by Guest linker */ > > bios_linker_loader_add_pointer(linker, > > ACPI_BUILD_TABLE_FILE, > > ACPI_BUILD_TABLE_FILE, > > - table_data, &rsdt->table_offset_entry[i], > > - sizeof(uint32_t)); > > + table_data, entries + i * entry_size, > > + entry_size); > > } > > - build_header(linker, table_data, > > - (void *)rsdt, "RSDT", rsdt_len, 1); > > + build_header(linker, table_data, sdt, sig, sdt_len, 1); > > The table revision should update as well. Make it as a parameter of > build_sdt. How do you figure this? The spec I have lists XSDT revision as 1. > > +} > > + > > +void > > +build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets) > > +{ > > + build_sdt(table_data, linker, table_offsets, "RSDT", sizeof(uint32_t)); > > +} > > + > > +/* Build xsdt table */ > > +void > > +build_xsdt(GArray *table_data, GArray *linker, GArray *table_offsets) > > +{ > > + build_sdt(table_data, linker, table_offsets, "XSDT", sizeof(uint64_t)); > > } > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index aaa149b..3551a08 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -1593,7 +1593,7 @@ build_dsdt(GArray *table_data, GArray *linker, AcpiMiscInfo *misc) > > } > > > > static GArray * > > -build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt) > > +build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt, unsigned xsdt) > > uint64_t xsdt? No, because it's an offset within the blob, not the physical address. We can't really ask bios to allocate > 4G blob for us. > > { > > AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp); > > > > @@ -1602,16 +1602,33 @@ build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt) > > > > memcpy(&rsdp->signature, "RSD PTR ", 8); > > memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6); > > + > > + rsdp->revision = 0x02; > > + rsdp->length = cpu_to_le32(sizeof *rsdp); > > + > > rsdp->rsdt_physical_address = cpu_to_le32(rsdt); > > /* Address to be filled by Guest linker */ > > bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE, > > ACPI_BUILD_TABLE_FILE, > > rsdp_table, &rsdp->rsdt_physical_address, > > sizeof rsdp->rsdt_physical_address); > > + > > + rsdp->xsdt_physical_address = cpu_to_le32(xsdt); > > cpu_to_le64(xsdt) ? Good catch. Will fix, thanks! > > + bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE, > > + ACPI_BUILD_TABLE_FILE, > > + rsdp_table, &rsdp->xsdt_physical_address, > > + sizeof rsdp->xsdt_physical_address); > > rsdp->checksum = 0; > > /* Checksum to be filled by Guest linker */ > > bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > > - rsdp, rsdp, sizeof *rsdp, &rsdp->checksum); > > + rsdp, rsdp, offsetof(AcpiRsdpDescriptor, length), > > + &rsdp->checksum); > > + > > + rsdp->extended_checksum = 0; > > + /* Checksum to be filled by Guest linker */ > > + bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > > + rsdp, rsdp, sizeof *rsdp, > > + &rsdp->extended_checksum); > > > > return rsdp_table; > > } > > @@ -1665,7 +1682,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables) > > { > > GArray *rsdt_table_offsets; > > GArray *xsdt_table_offsets; > > - unsigned facs, ssdt, dsdt, rsdt; > > + unsigned facs, ssdt, dsdt, rsdt, xsdt; > > AcpiCpuInfo cpu; > > AcpiPmInfo pm; > > AcpiMiscInfo misc; > > @@ -1762,12 +1779,14 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables) > > g_array_append_vals(tables_blob, u, len); > > } > > > > - /* RSDT is pointed to by RSDP */ > > + /* RSDT and XSDT pointed to by RSDP */ > > rsdt = tables_blob->len; > > build_rsdt(tables_blob, tables->linker, rsdt_table_offsets); > > + xsdt = tables_blob->len; > > + build_xsdt(tables_blob, tables->linker, xsdt_table_offsets); > > > > /* RSDP is in FSEG memory, so allocate it separately */ > > - build_rsdp(tables->rsdp, tables->linker, rsdt); > > + build_rsdp(tables->rsdp, tables->linker, rsdt, xsdt); > > > > /* We'll expose it all to Guest so we want to reduce > > * chance of size changes. > > > > -- > Shannon