[PATCH] acpi: gtdt: add checking the valid base physical address

[ivanhu]

Hi Hugo,

The dmesg what I got,

[    6.832162] ACPI GTDT: [Firmware Bug]: failed to get the Watchdog
base address.


Cheers,
Ivan

On 08/15/2018 11:16 PM, Jeffrey Hugo wrote:
> On 8/15/2018 1:38 AM, Ivan Hu wrote:
>> Some buggy firmwares on Arm server implement GTDT table without
>> impementing
>> reasonable base physical address and causes kernel complains about
>> ail to get
>> base address. Add checking for the valid base addresses on GTDT table.
>
> I'm interested to know, what is the exact complaint the kernel gives?
> Can you point me to a log or something?
>
>> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
>> ---
>>   src/acpi/gtdt/gtdt.c | 54
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 54 insertions(+)
>>
>> diff --git a/src/acpi/gtdt/gtdt.c b/src/acpi/gtdt/gtdt.c
>> index ded1d5d..dc00785 100644
>> --- a/src/acpi/gtdt/gtdt.c
>> +++ b/src/acpi/gtdt/gtdt.c
>> @@ -56,6 +56,24 @@ static int gtdt_test1(fwts_framework *fw)
>>       uint32_t i = 0, n;
>>       const fwts_acpi_table_gtdt *gtdt = (const fwts_acpi_table_gtdt
>> *)table->data;
>>   +    if (gtdt->cnt_control_base_phys_addr == 0) {
>> +        passed = false;
>> +        fwts_failed(fw, LOG_LEVEL_MEDIUM,
>> +            "GTDTInvalidBaseAddr",
>> +            "The 64-bit physical address at which the "
>> +            "Counter Control block should not be Zero. "
>> +            "If not provided, this field must be 0xFFFFFFFFFFFFFFFF");
>> +    }
>> +
>> +    if (gtdt->cnt_read_base_phys_addr == 0) {
>> +        passed = false;
>> +        fwts_failed(fw, LOG_LEVEL_MEDIUM,
>> +            "GTDTInvalidBaseAddr",
>> +            "The 64-bit physical address at which the "
>> +            "Counter Read block Read block should not be Zero. "
>> +            "If not provided, this field must be 0xFFFFFFFFFFFFFFFF");
>> +    }
>> +
>>       fwts_acpi_reserved_bits_check(fw, "GTDT", "Flags",
>> gtdt->virtual_timer_flags, sizeof(gtdt->virtual_timer_flags), 3, 31,
>> &passed);
>>         ptr = (uint8_t *)table->data + gtdt->platform_timer_offset;
>> @@ -89,6 +107,13 @@ static int gtdt_test1(fwts_framework *fw)
>>                       i, block->length);
>>                   goto done;
>>               }
>> +            if (block->physical_address == 0) {
>> +                passed = false;
>> +                fwts_failed(fw, LOG_LEVEL_HIGH,
>> +                    "GTDTBlockInvalidBaseAddr",
>> +                    "The 64-bit physical address at which the "
>> +                    "GT CntCTLBase Block shouldn't be zero.");
>> +            }
>>               if (block->reserved) {
>>                   passed = false;
>>                   fwts_failed(fw, LOG_LEVEL_HIGH,
>> @@ -146,6 +171,21 @@ static int gtdt_test1(fwts_framework *fw)
>>                           block_timer->reserved[1],
>>                           block_timer->reserved[2]);
>>                   }
>> +                if (block_timer->cntbase == 0) {
>> +                    passed = false;
>> +                    fwts_failed(fw, LOG_LEVEL_HIGH,
>> +                        "GTDTGTxInvalidBaseAddr",
>> +                        "Physical Address at which the "
>> +                        "CntBase block for GTx shouldn't be zero.");
>> +                }
>> +                if (block_timer->cntel0base == 0) {
>> +                    passed = false;
>> +                    fwts_failed(fw, LOG_LEVEL_MEDIUM,
>> +                        "GTDTGTxInvalidBaseAddr",
>> +                        "Physical Address at which the "
>> +                        "CntEL0Base block for GTx should not be Zero. "
>> +                        "If not provided, this field must be
>> 0xFFFFFFFFFFFFFFFF");
>> +                }
>>                     snprintf(field, sizeof(field), "block %" PRIu32 "
>> physical timer flags", i);
>>                   fwts_acpi_reserved_bits_check(fw, "GTDT", field,
>> block_timer->phys_timer_flags,
>> @@ -189,6 +229,20 @@ static int gtdt_test1(fwts_framework *fw)
>>                       " reserved is non-zero, got 0x%" PRIx8,
>>                       i, watchdog->reserved);
>>               }
>> +            if (watchdog->refresh_frame_addr == 0) {
>> +                passed = false;
>> +                fwts_failed(fw, LOG_LEVEL_HIGH,
>> +                    "GTDTInvalidWatchDogBaseAddr",
>> +                    "Physical Address at which the "
>> +                    "RefreshFrame block shouldn't be zero.");
>> +            }
>> +            if (watchdog->watchdog_control_frame_addr == 0) {
>> +                passed = false;
>> +                fwts_failed(fw, LOG_LEVEL_HIGH,
>> +                    "GTDTInvalidWatchDogBaseAddr",
>> +                    "Physical Address at which the "
>> +                    "Watchdog Control Frame block shouldn't be zero.");
>> +            }
>>                 snprintf(field, sizeof(field), "SBSA generic watchdog
>> timer %" PRIu32 " flags", i);
>>               fwts_acpi_reserved_bits_check(fw, "GTDT", field,
>> watchdog->watchdog_timer_flags,
>>
>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/fwts-devel/attachments/20180820/0c08470e/attachment.sig>