[PATCH 1/2] acpi: Add CSRT ACPI Table test (LP: #1470116)

Colin Ian King colin.king at canonical.com
Wed Jul 1 08:48:37 UTC 2015


On 01/07/15 09:41, Colin Ian King wrote:
> On 01/07/15 09:35, ivanhu wrote:
>>
>>
>> On 2015年06月30日 22:17, Colin King wrote:
>>> From: Colin Ian King <colin.king at canonical.com>
>>>
>>> Implement CSRT table parsing + checking. This checks lengths and
>>> specific field.  Does not check the shared info region as this is
>>> not described in the specification.
>>>
>>> Signed-off-by: Colin Ian King <colin.king at canonical.com>
>>> ---
>>>   src/Makefile.am             |   1 +
>>>   src/acpi/csrt/csrt.c        | 246
>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>   src/lib/include/fwts_acpi.h |  31 ++++++
>>>   3 files changed, 278 insertions(+)
>>>   create mode 100644 src/acpi/csrt/csrt.c
>>>
>>> diff --git a/src/Makefile.am b/src/Makefile.am
>>> index 4a1794a..5542574 100644
>>> --- a/src/Makefile.am
>>> +++ b/src/Makefile.am
>>> @@ -40,6 +40,7 @@ fwts_SOURCES = main.c                 \
>>>       acpi/cpep/cpep.c            \
>>>       acpi/crsdump/crsdump.c            \
>>>       acpi/crsdump/prsdump.c            \
>>> +    acpi/csrt/csrt.c            \
>>>       acpi/cstates/cstates.c             \
>>>       acpi/dbgp/dbgp.c             \
>>>       acpi/dbg2/dbg2.c             \
>>> diff --git a/src/acpi/csrt/csrt.c b/src/acpi/csrt/csrt.c
>>> new file mode 100644
>>> index 0000000..5f680db
>>> --- /dev/null
>>> +++ b/src/acpi/csrt/csrt.c
>>> @@ -0,0 +1,246 @@
>>> +/*
>>> + * Copyright (C) 2015 Canonical
>>> + *
>>> + * Portions of this code original from the Linux-ready Firmware
>>> Developer Kit
>>> + *
>>> + * 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>
>>> +
>>> +static fwts_acpi_table_info *table;
>>> +
>>> +static int csrt_init(fwts_framework *fw)
>>> +{
>>> +    if (fwts_acpi_find_table(fw, "CSRT", 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 CSRT table does not exist, skipping
>>> test");
>>> +        return FWTS_SKIP;
>>> +    }
>>> +    return FWTS_OK;
>>> +}
>>> +
>>> +/*
>>> + *  CSRT Core System Resource Table
>>> + *    https://acpica.org/sites/acpica/files/CSRT.doc
>>> + */
>>> +static int csrt_test1(fwts_framework *fw)
>>> +{
>>> +    size_t rg_offset;
>>> +    fwts_acpi_table_csrt_resource_group *rg;
>>> +    bool passed = true;
>>> +    int rg_count = 0;
>>> +
>>> +    /* Note: CSRT header is actually just and ACPI header */
>>> +    if (table->length < sizeof(fwts_acpi_table_csrt)) {
>>> +        passed = false;
>>> +        fwts_failed(fw, LOG_LEVEL_HIGH,
>>> +            "CSRTToosShort",
>>> +            "CSRT is only %zu bytes long, expecting at least "
>>> +            "%zu bytes", table->length,
>>> +            sizeof(fwts_acpi_table_csrt));
>>> +        goto done;
>>> +    }
>>> +
>>> +    rg_offset = sizeof(fwts_acpi_table_csrt);
>>> +    rg = (fwts_acpi_table_csrt_resource_group *)
>>> +        (table->data + sizeof(fwts_acpi_table_csrt));
>>> +
>>> +    while (rg_offset < table->length) {
>>> +        size_t rg_length = table->length - rg_offset;
>>> +        /*
>>> +        fwts_acpi_table_csrt_resource_group *rg =
>>> +            (fwts_acpi_table_csrt_resource_group *)rd;
>>> +        */
>> forget to remove?
> 
> Argh, yep, I'll remove that cruft
> 
>>> +        /* Sanity check size of what we've got left */
>>> +        if (rg_length < sizeof(fwts_acpi_table_csrt_resource_group)) {
>>> +            passed = false;
>>> +            fwts_failed(fw, LOG_LEVEL_HIGH,
>>> +                "CSRTResourceGroupTooShort",
>>> +                "CSRT Resource Group %d is only %zu bytes long, "
>>> +                "expecting at least %zu bytes",
>>> +                rg_count, rg_length,
>>> +                sizeof(fwts_acpi_table_csrt_resource_group));
>>> +            goto done; /* Bad size, bail out */
>>> +        }
>>> +        /* Sanity check resource group header size */
>>> +        if (rg->length < sizeof(fwts_acpi_table_csrt_resource_group)) {
>>> +            passed = false;
>>> +            fwts_failed(fw, LOG_LEVEL_HIGH,
>>> +                "CSRTResourceGroupLengthFieldTooShort",
>>> +                "CSRT Resource Group %d Length Field is %"
>>> +                PRIu32 " and was expecting at least %zu ",
>>> +                rg_count, rg->length,
>>> +                sizeof(fwts_acpi_table_csrt_resource_group));
>>> +            goto done; /* Bad size, bail out */
>>> +        }
>>> +
>>> +        /*
>>> +         *  Enough space for a header, so we can now check the
>>> +         *  if we have a shared info following the header and before the
>>> +         *  resource descriptor
>>> +         */
>>> +        if (rg->length < (sizeof(fwts_acpi_table_csrt_resource_group) +
>>> +                  rg->shared_info_length)) {
>>> +            passed = false;
>>> +            fwts_failed(fw, LOG_LEVEL_HIGH,
>>> +                "CSRTResourceGroupLengthFieldTooShort",
>>> +                "CSRT Resource Group %d Length Field is %"
>>> +                PRIu32 " and was expecting at least %zu ",
>>> +                rg_count, rg->length,
>>> +                sizeof(fwts_acpi_table_csrt_resource_group) +
>>> +                rg->shared_info_length);
>>> +            goto done; /* Bad size, bail out */
>>> +        }
>>> +        /* Reserved must be zero */
>>> +        if (rg->reserved != 0) {
>>> +            passed = false;
>>> +            fwts_failed(fw, LOG_LEVEL_HIGH,
>>> +                "CSRTResourceGroupReservedFieldNonZero",
>>> +                "CSRT Resource Group %d Reserved Field is 0x%4.4" PRIx16
>>> +                " and was expecting a zero value",
>>> +                rg_count, rg->reserved);
>>> +        }
>>> +
>>> +        /* Do we have enough space for a resource descriptor? */
>>> +        if (rg_length >
>>> sizeof(fwts_acpi_table_csrt_resource_descriptor)) {
>>> +            size_t rd_offset =
>>> sizeof(fwts_acpi_table_csrt_resource_group);
>>> +            int rd_count = 0;
>>> +            fwts_acpi_table_csrt_resource_descriptor *rd =
>>> +                (fwts_acpi_table_csrt_resource_descriptor *)
>>> +                ((uint8_t *)rg +
>>> sizeof(fwts_acpi_table_csrt_resource_group) +
>>> +                 rg->shared_info_length);
>>> +
>>> +            /* Handle 1 or more resource descriptors */
>>> +            while (rd_offset < rg->length) {
>>> +                /* Advertised length must be sane */
>>> +                if (rd->length <
>>> sizeof(fwts_acpi_table_csrt_resource_descriptor)) {
>>> +                    passed = false;
>>> +                    fwts_failed(fw, LOG_LEVEL_HIGH,
>>> +                        "CSRTResourceDescriptorLengthFieldTooShort",
>>> +                        "CSRT Resource Group %d Descriptor %d "
>>> +                        "Length Field is %" PRIu32 " and was "
>>> +                        "expecting at least %zu",
>>> +                        rg_count, rd_count, rd->length,
>>> +                       
>>> sizeof(fwts_acpi_table_csrt_resource_descriptor));
>>> +                    goto next; /* Bad size, skip to next */
>>> +                }
>>> +                /* Sanity check type and subtype */
>>> +                switch (rd->type) {
>>> +                case 0x0000:
>>> +                case 0x0800 ... 0xffff:
>>> +                    passed = false;
>>> +                    fwts_failed(fw, LOG_LEVEL_HIGH,
>>> +                        "CSRTResourceDescriptorResourceTypeInvalid",
>>> +                        "CSRT Resource Group %d Descriptor %d "
>>> +                        "Resource Type is 0x%4.4" PRIx16 " which is a "
>>> +                        "reserved type",
>>> +                        rg_count, rd_count, rd->type);
>>> +                    break;
>>> +                case 0x0001: /* Interrupt */
>>> +                    if (rd->subtype > 1) {
>>> +                        passed = false;
>>> +                        fwts_failed(fw, LOG_LEVEL_HIGH,
>>> +                           
>>> "CSRTResourceDescriptorResourceSubTypeInvalid",
>>> +                            "CSRT Resource Group %d Descriptor %d "
>>> +                            "Resource Subtype is 0x%4.4" PRIx16 "
>>> which is a "
>>> +                            "reserved type, expecting 0x0000
>>> (Interrupt "
>>> +                            "Line) or 0x0001 (Interrupt Controller)",
>>> +                            rg_count, rd_count, rd->subtype);
>>> +                    }
>>> +                    break;
>>> +                case 0x0002: /* Timer */
>>> +                    if (rd->subtype > 0) {
>>> +                        passed = false;
>>> +                        fwts_failed(fw, LOG_LEVEL_HIGH,
>>> +                           
>>> "CSRTResourceDescriptorResourceSubTypeInvalid",
>>> +                            "CSRT Resource Group %d Descriptor %d "
>>> +                            "Resource Subtype is 0x%4.4" PRIx16 "
>>> which is a "
>>> +                            "reserved type, expecting 0x0000 (Timer)",
>>> +                            rg_count, rd_count, rd->subtype);
>>> +                    }
>>> +                    break;
>>> +                case 0x0003: /* DMA */
>>> +                    if (rd->subtype > 0) {
>> should be (rd->subtype > 1) ?
> 
> allowed subtypes for DMA are just 0x0000, so anything more than that is
> an error. Or have I read the spec incorrectly?

Yep, its my mistake, I read the spec incorrectly, I didn't see the
0x0001 DMA controller subtype that was on the next page.  I'll fix that

> 
>>> +                        passed = false;
>>> +                        fwts_failed(fw, LOG_LEVEL_HIGH,
>>> +                           
>>> "CSRTResourceDescriptorResourceSubTypeInvalid",
>>> +                            "CSRT Resource Group %d Descriptor %d "
>>> +                            "Resource Subtype is 0x%4.4" PRIx16 "
>>> which is a "
>>> +                            "reserved type, expecting 0x0000 (DMA
>>> channel)",
>>> +                            rg_count, rd_count, rd->subtype);
>>> +                    }
>>> +                    break;
>>> +                default:     /* For future use */
>>> +                    passed = false;
>>> +                    fwts_failed(fw, LOG_LEVEL_HIGH,
>>> +                        "CSRTResourceDescriptorResourceTypeInvalid",
>>> +                        "CSRT Resource Group %d Descriptor %d "
>>> +                        "Resource Type is 0x%4.4" PRIx16 " which is a "
>>> +                        "reserved type for future use only",
>>> +                        rg_count, rd_count, rd->type);
>>> +                    break;
>>> +                }
>>> +                /* Check for reserved UID */
>>> +                if (rd->uid == 0xffffffff) {
>>> +                    passed = false;
>>> +                    fwts_failed(fw, LOG_LEVEL_HIGH,
>>> +                        "CSRTResourceDescriptorReservedUID",
>>> +                        "CSRT Resource Group %d Descriptor %d "
>>> +                        "UID Field is 0x%8.8" PRIx32 " and this is "
>>> +                        "a reserved UID value",
>>> +                        rg_count, rd_count, rd->uid);
>>> +                }
>>> +                /* the silicon vendor info is not specified, can't
>>> check */
>>> +
>>> +                rd_offset += rd->length;
>>> +                rd_count++;
>>> +                rd++;
>> Shouldn't consider the "silicon vendor defined information" offset for
>> next rd, if it exists?
> 
> Yes, thanks for spotting that. I'll send V2 later today
> 
>>> +            }
>>> +        }
>>> +next:
>>> +        rg_offset += rg->length;
>>> +        rg_count++;
>>> +        rg = (fwts_acpi_table_csrt_resource_group *)
>>> +            ((uint8_t*)rg + rg->length);
>>> +    }
>>> +done:
>>> +    if (passed)
>>> +        fwts_passed(fw, "No issues found in CSRT table.");
>>> +
>>> +    return FWTS_OK;
>>> +}
>>> +
>>> +static fwts_framework_minor_test csrt_tests[] = {
>>> +    { csrt_test1, "CSRT Core System Resource Table test." },
>>> +    { NULL, NULL }
>>> +};
>>> +
>>> +static fwts_framework_ops csrt_ops = {
>>> +    .description = "CSRT Core System Resource Table test.",
>>> +    .init        = csrt_init,
>>> +    .minor_tests = csrt_tests
>>> +};
>>> +
>>> +FWTS_REGISTER("csrt", &csrt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH |
>>> FWTS_FLAG_TEST_ACPI)
>>> diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h
>>> index e27c68c..a6b929c 100644
>>> --- a/src/lib/include/fwts_acpi.h
>>> +++ b/src/lib/include/fwts_acpi.h
>>> @@ -1068,4 +1068,35 @@ typedef struct {
>>>     void fwts_acpi_table_get_header(fwts_acpi_table_header *hdr,
>>> uint8_t *data);
>>>   +/*
>>> + * Core System Resources Table (CSRT)
>>> + *   https://acpica.org/sites/acpica/files/CSRT.doc
>>> + */
>>> +typedef struct {
>>> +    fwts_acpi_table_header  header;
>>> +} __attribute__ ((packed)) fwts_acpi_table_csrt;
>>> +
>>> +typedef struct {
>>> +    uint32_t    length;
>>> +    uint32_t    vendor_id;
>>> +    uint32_t    subvendor_id;
>>> +    uint16_t    device_id;
>>> +    uint16_t    subdevice_id;
>>> +    uint16_t    revision;
>>> +    uint16_t    reserved;
>>> +    uint32_t    shared_info_length;
>>> +} __attribute__ ((packed)) fwts_acpi_table_csrt_resource_group;
>>> +
>>> +typedef struct {
>>> +    uint32_t    length;
>>> +    uint16_t    type;
>>> +    uint16_t    subtype;
>>> +    uint32_t    uid;
>>> +    uint8_t        silicon_vendor_info[0];
>>> +} __attribute__ ((packed)) fwts_acpi_table_csrt_resource_descriptor;
>>> +
>>> +#define FWTS_ACPI_TABLE_CSRT_TYPE_INTERRUPT    (0x0001)
>>> +#define FWTS_ACPI_TABLE_CSRT_TYPE_TIMER        (0x0002)
>>> +#define FWTS_ACPI_TABLE_CSRT_TYPE_DMA        (0x0003)
>>> +
>>>   #endif
>>
> 
> 




More information about the fwts-devel mailing list