[PATCH 1/2] acpi: add support for FPDT table (LP: #1476954)
Colin Ian King
colin.king at canonical.com
Thu Jul 23 10:17:52 UTC 2015
On 23/07/15 08:16, Ivan Hu wrote:
> Firmware Performance Data Table (FPDT) table defined after ACPI spec v5.0, 5.2.23
>
> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
> ---
> src/Makefile.am | 1 +
> src/acpi/fpdt/fpdt.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 214 insertions(+)
> create mode 100644 src/acpi/fpdt/fpdt.c
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 3e7fc3c..5d5a5ca 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -50,6 +50,7 @@ fwts_SOURCES = main.c \
> acpi/facs/facs.c \
> acpi/fadt/fadt.c \
> acpi/fan/fan.c \
> + acpi/fpdt/fpdt.c \
> acpi/gpedump/gpedump.c \
> acpi/gtdt/gtdt.c \
> acpi/hest/hest.c \
> diff --git a/src/acpi/fpdt/fpdt.c b/src/acpi/fpdt/fpdt.c
> new file mode 100644
> index 0000000..f5a0118
> --- /dev/null
> +++ b/src/acpi/fpdt/fpdt.c
> @@ -0,0 +1,213 @@
> +/*
> + * Copyright (C) 2015 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
> + *
> + */
> +#include "fwts.h"
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <inttypes.h>
> +#include <string.h>
> +#include <ctype.h>
> +
> +fwts_acpi_table_info *table;
> +
> +static int fpdt_init(fwts_framework *fw)
> +{
> +
> + if (fwts_acpi_find_table(fw, "FPDT", 0, &table) != FWTS_OK) {
> + fwts_log_error(fw, "Cannot read ACPI tables.");
> + return FWTS_ERROR;
> + }
> + if (table == NULL || (table && table->length == 0)) {
> + fwts_log_error(fw, "ACPI FPDT table does not exist, skipping test");
> + return FWTS_SKIP;
> + }
> +
> + return FWTS_OK;
> +}
> +
> +static void fpdt_rec_header_dump(
> + fwts_framework *fw,
> + const char *type_name,
> + fwts_acpi_table_fpdt_header *header)
> +{
> + fwts_log_info_verbatum(fw, " %s:", type_name);
> + fwts_log_info_verbatum(fw, " Perf Rec Type: 0x%4.4" PRIx16, header->type);
> + fwts_log_info_verbatum(fw, " Rec Length: 0x%2.2" PRIx8, header->length);
> + fwts_log_info_verbatum(fw, " Revision: 0x%2.2" PRIx8, header->revision);
> +}
> +
> +static void fpdt_dump_raw_data(
> + fwts_framework *fw,
> + const uint8_t *data,
> + const size_t length,
> + const size_t offset)
> +{
> + size_t n;
> +
> + fwts_log_info_verbatum(fw, " Data:");
> + for (n = 0; n < length; n+=16) {
> + int left = length - n;
> + char buffer[128];
> + fwts_dump_raw_data(buffer, sizeof(buffer), data + n, n + offset, left > 16 ? 16 : left);
> + fwts_log_info_verbatum(fw, "%s", buffer);
> + }
> +}
> +
> +static int fpdt_test1(fwts_framework *fw)
> +{
> + uint8_t *data = (uint8_t *)table->data, *ptr = data;
> + fwts_acpi_table_header *header= (fwts_acpi_table_header *)table->data;
> + bool passed = true;
> + const size_t fpdt_hdr_len = sizeof(fwts_acpi_table_fpdt_header);
> +
> + /* Size sanity, got enough table to get initial header */
> + if (table->length < (sizeof(fwts_acpi_table_header) + sizeof(fwts_acpi_table_fpdt_header))) {
> + passed = false;
> + fwts_failed(fw, LOG_LEVEL_HIGH,
> + "FPDTTooShort",
> + "FPDT table too short, must be at least %zu bytes, "
> + "instead got %zu bytes",
> + sizeof(fwts_acpi_table_header) + sizeof(fwts_acpi_table_fpdt_header),
> + table->length);
> + goto done;
> + }
> +
> + if (header->revision != 1) {
> + passed = false;
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FPDTBabRevision",
FPDTBabRevision should be FPDTBadRevsion
> + "FPDT revision is incorrect, expecting 0x01,"
> + " instead got 0x%2.2" PRIx8,
> + header->revision);
> + }
> +
> + ptr += sizeof(fwts_acpi_table_header);
> + while (ptr < data + table->length) {
what happens if table->length is sizeof(fwts_acpi_table_header) + 1? I
believe we fall off the end of the table when we access fpdt->type. So....
> + fwts_acpi_table_fpdt_header *fpdt = (fwts_acpi_table_fpdt_header *)ptr;
> + fwts_acpi_table_fpdt_basic_boot_perf_ptr *fbbpr = (fwts_acpi_table_fpdt_basic_boot_perf_ptr *)ptr;
> + fwts_acpi_table_fpdt_s3_perf_ptr *s3ptpr = (fwts_acpi_table_fpdt_s3_perf_ptr *)ptr;
...here we need to see if (ptr + sizeof(fwts_acpi_table_fpdt_header) <=
data + table->length) to see if the fpdt header is fully available
before we access fpdt->type.
> + switch (fpdt->type) {
> + case 0x0000: /* Firmware Basic Boot Performance Pointer Record */
> + if (fbbpr->fpdt.length != sizeof(fwts_acpi_table_fpdt_basic_boot_perf_ptr)) {
> + passed = false;
> + fwts_failed(fw, LOG_LEVEL_HIGH,
> + "FPDTFWBootPerfPrtRecBadLength",
> + "FPDT Firmware Basic Boot Performance Pointer Record is %" PRIu32
> + " bytes and should be %zu bytes in size",
> + fbbpr->fpdt.length, sizeof(fwts_acpi_table_fpdt_basic_boot_perf_ptr));
> + } else {
> + fpdt_rec_header_dump(fw, " Firmware Basic Boot Performance Pointer Record", fpdt);
> + fwts_log_info_verbatum(fw, " Reserved: 0x%8.8" PRIx32, fbbpr->reserved);
> + fwts_log_info_verbatum(fw, " FBPT Pointer: 0x%16.16" PRIx64, fbbpr->fbpt_addr);
> +
> + /*
> + * For the moment, only dump the 64-bit processor-relative physical address
> + * of the Firmware Basic Boot Performance Table, should also get and check
> + * the table from the address
> + */
It may be worth printing an info message saying we're not checking FBPT
for now.
> + }
> +
> + if (fbbpr->fpdt.revision != 1) {
> + passed = false;
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FPDTInvalidReversion",
> + "FPDT: Reversion field is 0x%" PRIx8
"Reversion" should be "Revision"
> + " and not the expected value of 0x01",
> + fbbpr->fpdt.revision);
> + }
> + /* Spec doesn't mention Reserved field should be 0 or not, skip checking the reserved field */
> + break;
> + case 0x0001: /* S3 Performance Table Pointer Record */
> + if (s3ptpr->fpdt.length != sizeof(fwts_acpi_table_fpdt_s3_perf_ptr)) {
> + passed = false;
> + fwts_failed(fw, LOG_LEVEL_HIGH,
> + "FPDTS3PerfPrtRecBadLength",
> + "FPDT S3 Performance Table Pointer Record is %" PRIu32
> + " bytes and should be %zu bytes in size",
> + s3ptpr->fpdt.length, sizeof(fwts_acpi_table_fpdt_s3_perf_ptr));
> + } else {
> + fpdt_rec_header_dump(fw, "S3 Performance Table Pointer Record", fpdt);
> + fwts_log_info_verbatum(fw, " Reserved: 0x%8.8" PRIx32, s3ptpr->reserved);
> + fwts_log_info_verbatum(fw, " S3PT Pointer: 0x%16.16" PRIx64, s3ptpr->s3pt_addr);
> +
> + /*
> + * For the moment, only dump 64-bit processor-relative physical
> + * address of the S3 Performance Table, should also get and check
> + * the table from the address
> + */
It may be worth printing an info message saying we're not checking S3PT
for now. e.g.
fwts_log_info(fw, "Note: currently fwts does not check S3PT validity
and the associated data");
we can come back and mmap and check this region on another
iteration/release of fwts.
> + }
> +
> + if (s3ptpr->fpdt.revision != 1) {
> + passed = false;
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FPDTInvalidReversion",
FPDTInvalidReversion should be FPDTInvalidRevision
> + "FPDT: Reversion field is 0x%" PRIx8
> + " and not the expected value of 0x01",
> + s3ptpr->fpdt.revision);
> + }
> + /* Spec doesn't mention Reserved field should be 0 or not, skip checking the reserved field */
> + break;
> + case 0x0002 ... 0x0fff:
> + fpdt_rec_header_dump(fw, "Reserved for ACPI specification usage", fpdt);
I'd recommend replacing "usage" with "use", same for occurrences below too
> + fpdt_dump_raw_data(fw, ptr + fpdt_hdr_len, fpdt->length - fpdt_hdr_len, ptr + fpdt_hdr_len - data);
> + break;
> + case 0x1000 ... 0x1fff:
> + fpdt_rec_header_dump(fw, "Reserved for Platform Vendor usage", fpdt);
> + fpdt_dump_raw_data(fw, ptr + fpdt_hdr_len, fpdt->length - fpdt_hdr_len, ptr + fpdt_hdr_len - data);
> + break;
> + case 0x2000 ... 0x2fff:
> + fpdt_rec_header_dump(fw, "Reserved for Hardware Vendor usage", fpdt);
> + fpdt_dump_raw_data(fw, ptr + fpdt_hdr_len, fpdt->length - fpdt_hdr_len, ptr + fpdt_hdr_len - data);
> + break;
> + case 0x3000 ... 0x3fff:
> + fpdt_rec_header_dump(fw, "Reserved for BIOS Vendor usage", fpdt);
> + fpdt_dump_raw_data(fw, ptr + fpdt_hdr_len, fpdt->length - fpdt_hdr_len, ptr + fpdt_hdr_len - data);
> + break;
> + default: /* For future use */
> + passed = false;
> + fwts_failed(fw, LOG_LEVEL_HIGH,
> + "FPDTPerformanceRecordTypeInvalid",
> + "FPDT Performance Record Type is 0x%4.4" PRIx16
> + " which is a reserved type for future use only",
> + fpdt->type);
> + break;
> + }
> + ptr += fpdt->length;
> + }
> +
> +done:
> + if (passed)
> + fwts_passed(fw, "No issues found in FPDT table.");
> +
> + return FWTS_OK;
> +}
> +
> +static fwts_framework_minor_test fpdt_tests[] = {
> + { fpdt_test1, "FPDT Firmware Performance Data Table test." },
> + { NULL, NULL }
> +};
> +
> +static fwts_framework_ops fpdt_ops = {
> + .description = "FPDT Firmware Performance Data Table test.",
> + .init = fpdt_init,
> + .minor_tests = fpdt_tests
> +};
> +
> +FWTS_REGISTER("fpdt", &fpdt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI)
>
More information about the fwts-devel
mailing list