[PATCH 3/5] ACPI tables: when bodging up an RSDP, do the right thing for the arch

[Al Stone]

On 01/26/2016 08:01 PM, Alex Hung wrote:
> On 2016-01-22 09:14 AM, Al Stone wrote:
>> If it is necessary to create an RSDP table because there is none that
>> can be read, add in only the RSDT or XSDT pointers as needed.  For x86,
>> it can be either, but for arm64 it should only be the XSDT address that
>> is used in the RSDP.
>>
>> Signed-off-by: Al Stone <al.stone at linaro.org>
>> ---
>>   src/lib/src/fwts_acpi_tables.c | 19 +++++++++++--------
>>   1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c
>> index 0191b6b..83bd1dd 100644
>> --- a/src/lib/src/fwts_acpi_tables.c
>> +++ b/src/lib/src/fwts_acpi_tables.c
>> @@ -1101,14 +1101,17 @@ static int fwts_acpi_load_tables_fixup(fwts_framework
>> *fw)
>>       }
>>
>>       /* Now we have all the tables, final fix up is required */
>> -    if (rsdp->rsdt_address != rsdt_fake_addr) {
>> -        rsdp->rsdt_address = rsdt_fake_addr;
>> -        redo_rsdp_checksum = true;
>> -    }
>> -    if ((rsdp->revision > 0) && (rsdp->length >= 36) &&
>> -        (rsdp->xsdt_address != xsdt_fake_addr)) {
>> -        rsdp->xsdt_address = xsdt_fake_addr;
>> -        redo_rsdp_checksum = true;
>> +    if (fw->target_arch == FWTS_ARCH_ARM64) {
>> +        if ((rsdp->revision > 0) && (rsdp->length >= 36) &&
>> +            (rsdp->xsdt_address != xsdt_fake_addr)) {
>> +            rsdp->xsdt_address = xsdt_fake_addr;
>> +            redo_rsdp_checksum = true;
>> +        }
>> +    } else {
>> +        if (rsdp->rsdt_address != rsdt_fake_addr) {
>> +            rsdp->rsdt_address = rsdt_fake_addr;
>> +            redo_rsdp_checksum = true;
>> +        }
> 
> Since non-ARM (ex. x86) can have either RSDP or XSDP, should the below

I assume that's a typo and RSDT or XSDT was meant...

> statements be included here as well?
> 
>         if ((rsdp->revision > 0) && (rsdp->length >= 36) &&
>             (rsdp->xsdt_address != xsdt_fake_addr)) {
>             rsdp->xsdt_address = xsdt_fake_addr;
>             redo_rsdp_checksum = true;
>         }

That's a good question.  For 6.0 and later, either RSDT or XSDT can
be used but only one is supposed to be used at a time.  Unfortunately,
the spec has been really bad about aligning table revisions with spec
revisions.  And as a practical matter, many vendors fill in both of
the fields on x86, and a lot of them tend to use whatever table version
they like, even if it's inconsistent with the length.

Should the check be on the FADT major/minor numbers instead?  I think
that might be a more reliable check.

>>       }
>>       /* And update checksum if we've updated the rsdp */
>>       if (redo_rsdp_checksum) {
>>
> 
> 


-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone at linaro.org
-----------------------------------