| Message ID | 1386304013-25029-1-git-send-email-ivan.hu@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
On 06/12/13 04:26, Ivan Hu wrote: > Some fimwares that contain two FADT tables, one comes from the RSDT and > the other comes from the XSDT. The FWTS will add the FADT tables followed > the order RSDT and XSDT. And the fadt test will load the fadt table first > added to the table what was from RSDT. > > Unfortunately, some firmware provide the multi-fadt tables, one(from > XSDT) is correct and the other is uncompleted/wrong (from RSDT). See the > bug(LP: #1253871) > > From the ACPI spec: it said "An ACPI-compatible OS must use the XSDT if > present.". So change table adding order to XSDT, RSDT to make sure fwts > check the fadt from XSDT first. > > Signed-off-by: Ivan Hu <ivan.hu@canonical.com> > --- > src/lib/src/fwts_acpi_tables.c | 36 ++++++++++++++++++------------------ > 1 file changed, 18 insertions(+), 18 deletions(-) > > diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c > index b23efa7..6f1e964 100644 > --- a/src/lib/src/fwts_acpi_tables.c > +++ b/src/lib/src/fwts_acpi_tables.c > @@ -260,19 +260,19 @@ static int fwts_acpi_load_tables_from_firmware(void) > return FWTS_ERROR; > fwts_acpi_add_table("RSDP", rsdp, (uint64_t)(off_t)rsdp_addr, rsdp_len, FWTS_ACPI_TABLE_FROM_FIRMWARE); > > - /* Load any tables from RSDT if it's valid */ > - if (rsdp->rsdt_address) { > - if ((rsdt = fwts_acpi_load_table((off_t)rsdp->rsdt_address)) != NULL) { > - fwts_acpi_add_table("RSDT", rsdt, (uint64_t)rsdp->rsdt_address, > - rsdt->header.length, FWTS_ACPI_TABLE_FROM_FIRMWARE); > - num_entries = (rsdt->header.length - sizeof(fwts_acpi_table_header)) / 4; > + /* Load any tables from XSDT if it's valid */ > + if (rsdp->xsdt_address) { > + if ((xsdt = fwts_acpi_load_table((off_t)rsdp->xsdt_address)) != NULL) { > + fwts_acpi_add_table("XSDT", xsdt, (uint64_t)rsdp->xsdt_address, > + xsdt->header.length, FWTS_ACPI_TABLE_FROM_FIRMWARE); > + num_entries = (xsdt->header.length - sizeof(fwts_acpi_table_header)) / 8; > for (i=0; i<num_entries; i++) { > - if (rsdt->entries[i]) { > - if ((header = fwts_acpi_load_table((off_t)rsdt->entries[i])) != NULL) { > + if (xsdt->entries[i]) { > + if ((header = fwts_acpi_load_table((off_t)xsdt->entries[i])) != NULL) { > if (strncmp("FACP", header->signature, 4) == 0) > fwts_acpi_handle_fadt((fwts_acpi_table_fadt*)header, > FWTS_ACPI_TABLE_FROM_FIRMWARE); > - fwts_acpi_add_table(header->signature, header, (uint64_t)rsdt->entries[i], > + fwts_acpi_add_table(header->signature, header, xsdt->entries[i], > header->length, FWTS_ACPI_TABLE_FROM_FIRMWARE); > } > } > @@ -280,19 +280,19 @@ static int fwts_acpi_load_tables_from_firmware(void) > } > } > > - /* Load any tables from XSDT if it's valid */ > - if (rsdp->xsdt_address) { > - if ((xsdt = fwts_acpi_load_table((off_t)rsdp->xsdt_address)) != NULL) { > - fwts_acpi_add_table("XSDT", xsdt, (uint64_t)rsdp->xsdt_address, > - xsdt->header.length, FWTS_ACPI_TABLE_FROM_FIRMWARE); > - num_entries = (xsdt->header.length - sizeof(fwts_acpi_table_header)) / 8; > + /* Load any tables from RSDT if it's valid */ > + if (rsdp->rsdt_address) { > + if ((rsdt = fwts_acpi_load_table((off_t)rsdp->rsdt_address)) != NULL) { > + fwts_acpi_add_table("RSDT", rsdt, (uint64_t)rsdp->rsdt_address, > + rsdt->header.length, FWTS_ACPI_TABLE_FROM_FIRMWARE); > + num_entries = (rsdt->header.length - sizeof(fwts_acpi_table_header)) / 4; > for (i=0; i<num_entries; i++) { > - if (xsdt->entries[i]) { > - if ((header = fwts_acpi_load_table((off_t)xsdt->entries[i])) != NULL) { > + if (rsdt->entries[i]) { > + if ((header = fwts_acpi_load_table((off_t)rsdt->entries[i])) != NULL) { > if (strncmp("FACP", header->signature, 4) == 0) > fwts_acpi_handle_fadt((fwts_acpi_table_fadt*)header, > FWTS_ACPI_TABLE_FROM_FIRMWARE); > - fwts_acpi_add_table(header->signature, header, xsdt->entries[i], > + fwts_acpi_add_table(header->signature, header, (uint64_t)rsdt->entries[i], > header->length, FWTS_ACPI_TABLE_FROM_FIRMWARE); > } > } > Thanks for spotting this, good catch. Acked-by: Colin Ian King <colin.king@canonical.com>
On 12/06/2013 12:26 PM, Ivan Hu wrote: > Some fimwares that contain two FADT tables, one comes from the RSDT and > the other comes from the XSDT. The FWTS will add the FADT tables followed > the order RSDT and XSDT. And the fadt test will load the fadt table first > added to the table what was from RSDT. > > Unfortunately, some firmware provide the multi-fadt tables, one(from > XSDT) is correct and the other is uncompleted/wrong (from RSDT). See the > bug(LP: #1253871) > > From the ACPI spec: it said "An ACPI-compatible OS must use the XSDT if > present.". So change table adding order to XSDT, RSDT to make sure fwts > check the fadt from XSDT first. > > Signed-off-by: Ivan Hu <ivan.hu@canonical.com> > --- > src/lib/src/fwts_acpi_tables.c | 36 ++++++++++++++++++------------------ > 1 file changed, 18 insertions(+), 18 deletions(-) > > diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c > index b23efa7..6f1e964 100644 > --- a/src/lib/src/fwts_acpi_tables.c > +++ b/src/lib/src/fwts_acpi_tables.c > @@ -260,19 +260,19 @@ static int fwts_acpi_load_tables_from_firmware(void) > return FWTS_ERROR; > fwts_acpi_add_table("RSDP", rsdp, (uint64_t)(off_t)rsdp_addr, rsdp_len, FWTS_ACPI_TABLE_FROM_FIRMWARE); > > - /* Load any tables from RSDT if it's valid */ > - if (rsdp->rsdt_address) { > - if ((rsdt = fwts_acpi_load_table((off_t)rsdp->rsdt_address)) != NULL) { > - fwts_acpi_add_table("RSDT", rsdt, (uint64_t)rsdp->rsdt_address, > - rsdt->header.length, FWTS_ACPI_TABLE_FROM_FIRMWARE); > - num_entries = (rsdt->header.length - sizeof(fwts_acpi_table_header)) / 4; > + /* Load any tables from XSDT if it's valid */ > + if (rsdp->xsdt_address) { > + if ((xsdt = fwts_acpi_load_table((off_t)rsdp->xsdt_address)) != NULL) { > + fwts_acpi_add_table("XSDT", xsdt, (uint64_t)rsdp->xsdt_address, > + xsdt->header.length, FWTS_ACPI_TABLE_FROM_FIRMWARE); > + num_entries = (xsdt->header.length - sizeof(fwts_acpi_table_header)) / 8; > for (i=0; i<num_entries; i++) { > - if (rsdt->entries[i]) { > - if ((header = fwts_acpi_load_table((off_t)rsdt->entries[i])) != NULL) { > + if (xsdt->entries[i]) { > + if ((header = fwts_acpi_load_table((off_t)xsdt->entries[i])) != NULL) { > if (strncmp("FACP", header->signature, 4) == 0) > fwts_acpi_handle_fadt((fwts_acpi_table_fadt*)header, > FWTS_ACPI_TABLE_FROM_FIRMWARE); > - fwts_acpi_add_table(header->signature, header, (uint64_t)rsdt->entries[i], > + fwts_acpi_add_table(header->signature, header, xsdt->entries[i], > header->length, FWTS_ACPI_TABLE_FROM_FIRMWARE); > } > } > @@ -280,19 +280,19 @@ static int fwts_acpi_load_tables_from_firmware(void) > } > } > > - /* Load any tables from XSDT if it's valid */ > - if (rsdp->xsdt_address) { > - if ((xsdt = fwts_acpi_load_table((off_t)rsdp->xsdt_address)) != NULL) { > - fwts_acpi_add_table("XSDT", xsdt, (uint64_t)rsdp->xsdt_address, > - xsdt->header.length, FWTS_ACPI_TABLE_FROM_FIRMWARE); > - num_entries = (xsdt->header.length - sizeof(fwts_acpi_table_header)) / 8; > + /* Load any tables from RSDT if it's valid */ > + if (rsdp->rsdt_address) { > + if ((rsdt = fwts_acpi_load_table((off_t)rsdp->rsdt_address)) != NULL) { > + fwts_acpi_add_table("RSDT", rsdt, (uint64_t)rsdp->rsdt_address, > + rsdt->header.length, FWTS_ACPI_TABLE_FROM_FIRMWARE); > + num_entries = (rsdt->header.length - sizeof(fwts_acpi_table_header)) / 4; > for (i=0; i<num_entries; i++) { > - if (xsdt->entries[i]) { > - if ((header = fwts_acpi_load_table((off_t)xsdt->entries[i])) != NULL) { > + if (rsdt->entries[i]) { > + if ((header = fwts_acpi_load_table((off_t)rsdt->entries[i])) != NULL) { > if (strncmp("FACP", header->signature, 4) == 0) > fwts_acpi_handle_fadt((fwts_acpi_table_fadt*)header, > FWTS_ACPI_TABLE_FROM_FIRMWARE); > - fwts_acpi_add_table(header->signature, header, xsdt->entries[i], > + fwts_acpi_add_table(header->signature, header, (uint64_t)rsdt->entries[i], > header->length, FWTS_ACPI_TABLE_FROM_FIRMWARE); > } > } > Acked-by: Alex Hung <alex.hung@canonical.com>
diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c index b23efa7..6f1e964 100644 --- a/src/lib/src/fwts_acpi_tables.c +++ b/src/lib/src/fwts_acpi_tables.c @@ -260,19 +260,19 @@ static int fwts_acpi_load_tables_from_firmware(void) return FWTS_ERROR; fwts_acpi_add_table("RSDP", rsdp, (uint64_t)(off_t)rsdp_addr, rsdp_len, FWTS_ACPI_TABLE_FROM_FIRMWARE); - /* Load any tables from RSDT if it's valid */ - if (rsdp->rsdt_address) { - if ((rsdt = fwts_acpi_load_table((off_t)rsdp->rsdt_address)) != NULL) { - fwts_acpi_add_table("RSDT", rsdt, (uint64_t)rsdp->rsdt_address, - rsdt->header.length, FWTS_ACPI_TABLE_FROM_FIRMWARE); - num_entries = (rsdt->header.length - sizeof(fwts_acpi_table_header)) / 4; + /* Load any tables from XSDT if it's valid */ + if (rsdp->xsdt_address) { + if ((xsdt = fwts_acpi_load_table((off_t)rsdp->xsdt_address)) != NULL) { + fwts_acpi_add_table("XSDT", xsdt, (uint64_t)rsdp->xsdt_address, + xsdt->header.length, FWTS_ACPI_TABLE_FROM_FIRMWARE); + num_entries = (xsdt->header.length - sizeof(fwts_acpi_table_header)) / 8; for (i=0; i<num_entries; i++) { - if (rsdt->entries[i]) { - if ((header = fwts_acpi_load_table((off_t)rsdt->entries[i])) != NULL) { + if (xsdt->entries[i]) { + if ((header = fwts_acpi_load_table((off_t)xsdt->entries[i])) != NULL) { if (strncmp("FACP", header->signature, 4) == 0) fwts_acpi_handle_fadt((fwts_acpi_table_fadt*)header, FWTS_ACPI_TABLE_FROM_FIRMWARE); - fwts_acpi_add_table(header->signature, header, (uint64_t)rsdt->entries[i], + fwts_acpi_add_table(header->signature, header, xsdt->entries[i], header->length, FWTS_ACPI_TABLE_FROM_FIRMWARE); } } @@ -280,19 +280,19 @@ static int fwts_acpi_load_tables_from_firmware(void) } } - /* Load any tables from XSDT if it's valid */ - if (rsdp->xsdt_address) { - if ((xsdt = fwts_acpi_load_table((off_t)rsdp->xsdt_address)) != NULL) { - fwts_acpi_add_table("XSDT", xsdt, (uint64_t)rsdp->xsdt_address, - xsdt->header.length, FWTS_ACPI_TABLE_FROM_FIRMWARE); - num_entries = (xsdt->header.length - sizeof(fwts_acpi_table_header)) / 8; + /* Load any tables from RSDT if it's valid */ + if (rsdp->rsdt_address) { + if ((rsdt = fwts_acpi_load_table((off_t)rsdp->rsdt_address)) != NULL) { + fwts_acpi_add_table("RSDT", rsdt, (uint64_t)rsdp->rsdt_address, + rsdt->header.length, FWTS_ACPI_TABLE_FROM_FIRMWARE); + num_entries = (rsdt->header.length - sizeof(fwts_acpi_table_header)) / 4; for (i=0; i<num_entries; i++) { - if (xsdt->entries[i]) { - if ((header = fwts_acpi_load_table((off_t)xsdt->entries[i])) != NULL) { + if (rsdt->entries[i]) { + if ((header = fwts_acpi_load_table((off_t)rsdt->entries[i])) != NULL) { if (strncmp("FACP", header->signature, 4) == 0) fwts_acpi_handle_fadt((fwts_acpi_table_fadt*)header, FWTS_ACPI_TABLE_FROM_FIRMWARE); - fwts_acpi_add_table(header->signature, header, xsdt->entries[i], + fwts_acpi_add_table(header->signature, header, (uint64_t)rsdt->entries[i], header->length, FWTS_ACPI_TABLE_FROM_FIRMWARE); } }
Some fimwares that contain two FADT tables, one comes from the RSDT and the other comes from the XSDT. The FWTS will add the FADT tables followed the order RSDT and XSDT. And the fadt test will load the fadt table first added to the table what was from RSDT. Unfortunately, some firmware provide the multi-fadt tables, one(from XSDT) is correct and the other is uncompleted/wrong (from RSDT). See the bug(LP: #1253871) From the ACPI spec: it said "An ACPI-compatible OS must use the XSDT if present.". So change table adding order to XSDT, RSDT to make sure fwts check the fadt from XSDT first. Signed-off-by: Ivan Hu <ivan.hu@canonical.com> --- src/lib/src/fwts_acpi_tables.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-)