[prev in list] [next in list] [prev in thread] [next in thread] 

List:       fwts-devel
Subject:    ACK: [PATCH] acpi: gtdt: add checking the valid base physical address
From:       Alex Hung <alex.hung () canonical ! com>
Date:       2018-08-20 5:16:55
Message-ID: 92b5c107-01bd-bc8a-6c6e-4fb6a3a92a86 () canonical ! com
[Download RAW message or body]

On 2018-08-15 12: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

A typo "ail" here. It should be "failing" but I will fix when I apply 
this patch.

> base address. Add checking for the valid base addresses on GTDT table.
> 
> Signed-off-by: Ivan Hu <ivan.hu@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, 

Acked-by: Alex Hung <alex.hung@canonical.com>

-- 
fwts-devel mailing list
fwts-devel@lists.ubuntu.com
Modify settings or unsubscribe at: \
https://lists.ubuntu.com/mailman/listinfo/fwts-devel


[prev in list] [next in list] [prev in thread] [next in thread]