ACK: [PATCH] lib: fwts_acpi_table: multi-FADT table support fimware make sure the table from XSDT be checked first. (LP: #1258378)

[Colin Ian King]

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 at 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 at canonical.com>