[PATCH 2/3] acpi: phat: add a test for PHAT table
Alex Hung
alex.hung at canonical.com
Sat Apr 10 01:37:43 UTC 2021
Thanks for the comments and I will fix them in V2 except one comment below.
On 2021-04-09 2:33 a.m., Colin Ian King wrote:
> On 09/04/2021 03:27, Alex Hung wrote:
>> Signed-off-by: Alex Hung <alex.hung at canonical.com>
>> ---
>> src/Makefile.am | 1 +
>> src/acpi/phat/phat.c | 153 ++++++++++++++++++++++++++++++++++++
>> src/lib/include/fwts_acpi.h | 40 ++++++++++
>> 3 files changed, 194 insertions(+)
>> create mode 100644 src/acpi/phat/phat.c
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index a26a197b..95293080 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -118,6 +118,7 @@ fwts_SOURCES = main.c \
>> acpi/pdtt/pdtt.c \
>> acpi/powerbutton/powerbutton.c \
>> acpi/plddump/plddump.c \
>> + acpi/phat/phat.c \
>> acpi/pmtt/pmtt.c \
>> acpi/pptt/pptt.c \
>> acpi/rasf/rasf.c \
>> diff --git a/src/acpi/phat/phat.c b/src/acpi/phat/phat.c
>> new file mode 100644
>> index 00000000..066d1313
>> --- /dev/null
>> +++ b/src/acpi/phat/phat.c
>> @@ -0,0 +1,153 @@
>> +/*
>> + * Copyright (C) 2021 Canonical
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version 2
>> + * of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +#include "fwts.h"
>> +
>> +#if defined(FWTS_HAS_ACPI)
>> +
>> +#include <stdlib.h>
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <unistd.h>
>> +#include <inttypes.h>
>> +#include <stdbool.h>
>> +
>> +static fwts_acpi_table_info *table;
>> +acpi_table_init(PHAT, &table)
>> +
>> +static void print_record_header(fwts_framework *fw, const fwts_acpi_table_phat_header *header)
>> +{
>> + fwts_log_info_simp_int(fw, " Type: ", header->type);
>> + fwts_log_info_simp_int(fw, " Record Length: ", header->length);
>> + fwts_log_info_simp_int(fw, " Revision: ", header->revision);
>> +}
>> +
>> +static void phat_version_test(fwts_framework *fw, fwts_acpi_table_phat_version *entry, bool *passed)
>> +{
>> + uint32_t reserved, i;
>> +
>> + reserved = entry->reserved[0] + (entry->reserved[1] << 8) + (entry->reserved[2] << 16);
>> +
>> + fwts_log_info_verbatim(fw, " Firmware Version Data Record (Type 0):");
>> + print_record_header(fw, &entry->header);
>> + fwts_log_info_simp_int(fw, " Reserved: ", reserved);
>> + fwts_log_info_simp_int(fw, " Record Count: ", entry->element_count);
>> +
>> + for (i = 0 ; i < entry->element_count; i++) {
>
> What happens if element->count is incorrect? I think some check that we
> don't fall off the end of the table with the element pointer below.
>
>> + char guid[37];
>
> blank line here plase
>
>> + fwts_acpi_table_phat_version_elem *element =
>> + (fwts_acpi_table_phat_version_elem *) ((char *)entry + sizeof(fwts_acpi_table_phat_version));
>> +
>> + fwts_guid_buf_to_str(element->component_id, guid, sizeof(guid));
>> +
>> + fwts_log_info_verbatim(fw, " Component ID: %s", guid);
>> + fwts_log_info_simp_int(fw, " Version: ", element->version);
>> + fwts_log_info_simp_int(fw, " Producer ID: ", element->producer_id);
>> + }
>> +
>> + fwts_acpi_reserved_zero_check("PHAT", "Reserved", reserved, passed);
>> +}
>> +
>> +static void phat_health_test(fwts_framework *fw, fwts_acpi_table_phat_health *entry, bool *passed)
>> +{
>> + char *device_path;
>> + char guid[37];
>> +
>> + fwts_guid_buf_to_str(entry->data_signature, guid, sizeof(guid));
>> + device_path = (char *)entry + sizeof(fwts_acpi_table_phat_health);
>> +
>> + fwts_log_info_verbatim(fw, " Firmware Health Data Record (Type 1):");
>> + print_record_header(fw, &entry->header);
>> + fwts_log_info_simp_int(fw, " Reserved: ", entry->reserved);
>> + fwts_log_info_simp_int(fw, " AmHealthy: ", entry->healthy);
>> + fwts_log_info_verbatim(fw, " Device Signature: %s", guid);
>> + fwts_log_info_simp_int(fw, " Device-specific Data Offset: ", entry->data_offset);
>> + fwts_log_info_verbatim(fw, " Device Path: %s", device_path);
>> +
>> + if (entry->data_offset != 0) {
>> + uint16_t i;
>> +
>
> Same here for entry->header.length - we must ensure vendor_data does not
> fall off the end of the table
>
>> + for (i = entry->data_offset; i < entry->header.length; i++) {
>> + uint8_t *vendor_data = (uint8_t *)entry + i;
>
> blank line here please
>
>> + fwts_log_info_simp_int(fw, " Vendor Data: ", *vendor_data);
>> + }
>> + }
>> +
>> + fwts_acpi_reserved_zero_check("PHAT", "Reserved", entry->reserved, passed);
>> +
>> + if (entry->data_offset > entry->header.length) {
>> + fwts_failed(fw, LOG_LEVEL_CRITICAL,
>> + "PHATOutOfRangeOffset",
>> + "PHAT Type 1's Data Offset is out of range");
>> + *passed = false;
>> + }
>> +}
>> +
>> +static int phat_test1(fwts_framework *fw)
>> +{
>> + fwts_acpi_table_phat_header *entry;
>> + bool passed = true;
>> + uint32_t offset;
>> +
>> + fwts_log_info_verbatim(fw, "PHAT Platform Health Assessment Table:");
>> +
>> + entry = (fwts_acpi_table_phat_header *) (table->data + sizeof(fwts_acpi_table_phat));
>> + offset = sizeof(fwts_acpi_table_phat);
>> + while (offset < table->length) {
>> +
>
> blank line can be removed
>
>> + if (fwts_acpi_structure_length_zero_check(fw, "PHAT", entry->length, offset)) {
>> + passed = false;
>> + break;
>> + }
>> +
>> + if (entry->type == FWTS_ACPI_PHAT_VERSION) {
>> + phat_version_test(fw, (fwts_acpi_table_phat_version *) entry, &passed);
>> + } else if (entry->type == FWTS_ACPI_PHAT_HEALTH) {
>> + phat_health_test(fw, (fwts_acpi_table_phat_health *) entry, &passed);
>> + } else {
>> + fwts_acpi_structure_type_error(fw, "PHAT", FWTS_ACPI_PHAT_END - 1, entry->type);
>> + passed = false;
>> + break;
>> + }
>> +
>> + offset += entry->length;
>
> probably worth checking if offset falls off the end of the table before
> calling this check
The fwts_acpi_structure_range_check checks whether offset out of table
length, but I made a mistake in its comments and I will fix it in v2.
>
>> + if (fwts_acpi_structure_range_check(fw, "PHAT", table->length, offset)) {
>> + passed = false;
>> + break;
>> + }
>> +
>> + entry = (fwts_acpi_table_phat_header *) (table->data + offset);
>> + fwts_log_nl(fw);
>> + }
>> +
>> + if (passed)
>> + fwts_passed(fw, "No issues found in PHAT table.");
>> +
>> + return FWTS_OK;
>> +}
>> +
>> +static fwts_framework_minor_test phat_tests[] = {
>> + { phat_test1, "Validate PHAT table." },
>> + { NULL, NULL }
>> +};
>> +
>> +static fwts_framework_ops phat_ops = {
>> + .description = "PHAT Platform Health Assessment Table test.",
>> + .init = PHAT_init,
>> + .minor_tests = phat_tests
>> +};
>> +
>> +FWTS_REGISTER("phat", &phat_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI)
>> +
>> +#endif
>> diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h
>> index d223d934..e9ccdb0c 100644
>> --- a/src/lib/include/fwts_acpi.h
>> +++ b/src/lib/include/fwts_acpi.h
>> @@ -1512,6 +1512,46 @@ typedef struct {
>> uint16_t spin_rev;
>> } __attribute__ ((packed)) fwts_acpi_table_pptt_id;
>>
>> +/*
>> + * ACPI PHAT (Platform Health Assessment Table), 5.2.30
>> + */
>> +typedef struct {
>> + fwts_acpi_table_header header;
>> +} __attribute__ ((packed)) fwts_acpi_table_phat;
>> +
>> +typedef enum {
>> + FWTS_ACPI_PHAT_VERSION = 0,
>> + FWTS_ACPI_PHAT_HEALTH = 1,
>> + FWTS_ACPI_PHAT_END
>> +} fwts_acpi_phat_type;
>> +
>> +typedef struct {
>> + uint16_t type;
>> + uint16_t length;
>> + uint8_t revision;
>> +} __attribute__ ((packed)) fwts_acpi_table_phat_header;
>> +
>> +/* 0: Firmware Version Data Record */
>> +typedef struct {
>> + fwts_acpi_table_phat_header header;
>> + uint8_t reserved[3];
>> + uint32_t element_count;
>> +} __attribute__ ((packed)) fwts_acpi_table_phat_version;
>> +
>> +typedef struct {
>> + uint8_t component_id[16];
>> + uint64_t version;
>> + uint32_t producer_id;
>> +} __attribute__ ((packed)) fwts_acpi_table_phat_version_elem;
>> +
>> +/* 1: Firmware Health Data Record */
>> +typedef struct {
>> + fwts_acpi_table_phat_header header;
>> + uint16_t reserved;
>> + uint8_t healthy;
>> + uint8_t data_signature[16];
>> + uint32_t data_offset;
>> +} __attribute__ ((packed)) fwts_acpi_table_phat_health;
>>
>> /*
>> * ACPI PCCT (Platform Communications Channel Table), 14.1
>>
>
>
--
Cheers,
Alex Hung
More information about the fwts-devel
mailing list