This patch series adds in specific ACPI compliance testing for the MADT and all of its various subtables (16, currently).
The first three patches add in the idea of host and target architectures -- host being the arch that FWTS is running on, and target the arch whose firmware is being tested. This is needed later in the MADT tests since what is proper changes based on the architecture the firmware supports.
The fourth patch adds the detailed tests for the MADT and all but one of the subtables currently defined in ACPI 6.0. The last two patches add in the relatively new GIC ITS subtable and compliance tests for it.
There are still multiple TODOs in the compliance checks; these will be added as clarification of the spec becomes available.
Changes for v3: -- Add in support for the --arch=<name> parameter to specify the arch for the target firmware (default is that host == target). -- Add in the fwts_architecture typedef plus some helper functions so that tests in the future can adapt their behavior as needed, and so that the MADT tests can set themselves up properly. -- Instead of creating a new sourc file src/acpi/compliance/madt.c, replace the existing src/acpi/madt/madt.c tests since we're providing a superset. -- Various minor style and syntax corrections (from Ian Colin King)
Changes for v2: -- Clean up the white space problems -- Fix errors found by checkpatch (minor syntax things) -- Fix one logic error: while MADT and FADT table revisions *should* be in sync, they seldom are, so report this as a test failure and continue to test as much as possible instead of aborting completely, in some of those cases.
Al Stone (6): Start defining FWTS architectures as variables Define some utility functions for using the fwts_architecture enum Add mechanism to tell FWTS what architecture is being tested ACPI: MADT: add in compliance tests for the MADT and subtables ACPI: Add in MADT subtable description for GIC ITS subtable ACPI: MADT: add in compliance checks for the GIC ITS subtable
src/acpi/madt/madt.c | 1551 +++++++++++++++++++++++++++++++------- src/lib/include/fwts.h | 1 + src/lib/include/fwts_acpi.h | 10 + src/lib/include/fwts_arch.h | 41 + src/lib/include/fwts_framework.h | 3 + src/lib/src/Makefile.am | 1 + src/lib/src/fwts_arch.c | 88 +++ src/lib/src/fwts_framework.c | 25 + 8 files changed, 1460 insertions(+), 260 deletions(-) create mode 100644 src/lib/include/fwts_arch.h create mode 100644 src/lib/src/fwts_arch.c
Add in fwts_arch.h and a typedef for fwts_architecture so that we can start referring to the architectures used as variables. Later on we will define host and target architectures to make it easier for tests to adapt their behavior. What we want the tests to be able to do is change a test -- if necessary -- based on the architecture supported by the firmware being tested. For example, if hardware reduced mode is not being used on arm64, it is an error; on ia64 or x86, it may not matter.
Signed-off-by: Al Stone al.stone@linaro.org --- src/lib/include/fwts.h | 1 + src/lib/include/fwts_arch.h | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 src/lib/include/fwts_arch.h
diff --git a/src/lib/include/fwts.h b/src/lib/include/fwts.h index 483f4ad..711a379 100644 --- a/src/lib/include/fwts.h +++ b/src/lib/include/fwts.h @@ -47,6 +47,7 @@ #include "fwts_acpi.h" #include "fwts_acpi_tables.h" #include "fwts_acpid.h" +#include "fwts_arch.h" #include "fwts_checkeuid.h" #include "fwts_cpu.h" #include "fwts_dump.h" diff --git a/src/lib/include/fwts_arch.h b/src/lib/include/fwts_arch.h new file mode 100644 index 0000000..3fc03fc --- /dev/null +++ b/src/lib/include/fwts_arch.h @@ -0,0 +1,33 @@ +/* + * Copyright (C) 2016, Al Stone ahs3@redhat.com + * + * 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. + * + */ + +#ifndef __FWTS_ARCH_H__ +#define __FWTS_ARCH_H__ + +/* + * Possible architectures for either the host (where FWTS is being run) + * or the target (what is being tested -- e.g., running FWTS on x86 but + * examining arm64 ACPI tables). + * + * NB: kernel conventions are used for the arch names. + */ +typedef enum { + FWTS_ARCH_X86, + FWTS_ARCH_IA64, + FWTS_ARCH_ARM64, + FWTS_ARCH_OTHER +} fwts_architecture; + +#endif
Add in some helper functions to make it easier to use the enum for fwts_architecture.
Signed-off-by: Al Stone al.stone@linaro.org --- src/lib/include/fwts_arch.h | 4 +++ src/lib/src/Makefile.am | 1 + src/lib/src/fwts_arch.c | 88 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+) create mode 100644 src/lib/src/fwts_arch.c
diff --git a/src/lib/include/fwts_arch.h b/src/lib/include/fwts_arch.h index 3fc03fc..a950ea7 100644 --- a/src/lib/include/fwts_arch.h +++ b/src/lib/include/fwts_arch.h @@ -34,4 +34,8 @@ typedef enum { FWTS_ARCH_OTHER } fwts_architecture;
+extern fwts_architecture fwts_arch_get_host(void); +extern fwts_architecture fwts_arch_get_arch(const char *name); +extern const char *fwts_arch_names(void); + #endif diff --git a/src/lib/src/Makefile.am b/src/lib/src/Makefile.am index c16f129..5d63804 100644 --- a/src/lib/src/Makefile.am +++ b/src/lib/src/Makefile.am @@ -28,6 +28,7 @@ libfwts_la_SOURCES = \ fwts_acpi.c \ fwts_acpid.c \ fwts_alloc.c \ + fwts_arch.c \ fwts_args.c \ fwts_backtrace.c \ fwts_battery.c \ diff --git a/src/lib/src/fwts_arch.c b/src/lib/src/fwts_arch.c new file mode 100644 index 0000000..558458c --- /dev/null +++ b/src/lib/src/fwts_arch.c @@ -0,0 +1,84 @@ +/* + * Copyright (C) 2016, Al Stone ahs3@redhat.com + * + * 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 <stdlib.h> +#include <sys/utsname.h> + +#include "fwts.h" + +struct fwts_arch_info { + fwts_architecture arch; + char *name; +}; + +static const struct fwts_arch_info arch_info[] = { + { FWTS_ARCH_X86, "x86" }, + { FWTS_ARCH_X86, "x86_32" }, + { FWTS_ARCH_X86, "x86_64" }, + { FWTS_ARCH_IA64, "ia64" }, + { FWTS_ARCH_ARM64, "arm64" }, + { FWTS_ARCH_ARM64, "aarch64" }, + { FWTS_ARCH_OTHER, "other" } +}; + +static char *arch_names; + +static fwts_architecture __fwts_arch_get_arch(const char *name) +{ + const struct fwts_arch_info *ptr; + + for (ptr = arch_info; ptr->arch != FWTS_ARCH_OTHER; ptr++) + if (!strcmp(ptr->name, name)) + return ptr->arch; + + return FWTS_ARCH_OTHER; +} + +fwts_architecture fwts_arch_get_host(void) +{ + struct utsname buf; + + if (uname(&buf)) + return FWTS_ARCH_OTHER; + + return __fwts_arch_get_arch(buf.machine); +} + +fwts_architecture fwts_arch_get_arch(const char *name) +{ + return __fwts_arch_get_arch(name); +} + +const char *fwts_arch_names(void) +{ + const struct fwts_arch_info *ptr; + size_t len; + + if (arch_names) + return arch_names; + + for (ptr = arch_info, len = 0; ptr->arch != FWTS_ARCH_OTHER; ptr++) + len += strlen(ptr->name) + 1; + + arch_names = calloc(len, 1); + if (arch_names) { + for (ptr = arch_info; ptr->arch != FWTS_ARCH_OTHER; ptr++) { + strcat(arch_names, ptr->name); + strcat(arch_names, " "); + } + } + + return arch_names; +}
In some cases, we will want to run FWTS on one architecture, but use it test firmware tables that may be for another architecture. For example, we may want to test ACPI tables for arm64 machines on an x86 laptop.
Previous patches provided the fwts_architecture as a typedef, along with some helper functions. Here, we add in framework variables for the host (the machine running FWTS) and the target (the arch of the tables being tested) and then initialize them properly.
Further, we add the --arch=<name> parameter so that the user can specify the target architecture. By default, the target will be assumed to be the same as the host.
Signed-off-by: Al Stone al.stone@linaro.org --- src/lib/include/fwts_framework.h | 3 +++ src/lib/src/fwts_framework.c | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+)
diff --git a/src/lib/include/fwts_framework.h b/src/lib/include/fwts_framework.h index a35ed19..c9ea4bb 100644 --- a/src/lib/include/fwts_framework.h +++ b/src/lib/include/fwts_framework.h @@ -26,6 +26,7 @@
typedef struct fwts_framework fwts_framework;
+#include "fwts_arch.h" #include "fwts_log.h" #include "fwts_list.h" #include "fwts_acpica_mode.h" @@ -148,6 +149,8 @@ typedef struct fwts_framework { fwts_acpica_mode acpica_mode; /* ACPICA mode flags */ void *rsdp; /* ACPI RSDP address */ fwts_pm_method pm_method; + fwts_architecture host_arch; /* arch FWTS was built for */ + fwts_architecture target_arch; /* arch being tested */ } fwts_framework;
typedef struct { diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c index fec444f..e3f03f6 100644 --- a/src/lib/src/fwts_framework.c +++ b/src/lib/src/fwts_framework.c @@ -127,6 +127,7 @@ static fwts_option fwts_framework_options[] = { { "acpitests", "", 0, "Run general ACPI tests." }, { "acpicompliance", "", 0, "Run ACPI tests for spec compliance." }, { "log-level", "", 1, "Specify error level to report failed test messages," }, + { "arch", "", 1, "Specify arch of the tables being tested (defaults to current host)." }, { NULL, NULL, 0, NULL } };
@@ -1127,6 +1128,22 @@ static int fwts_framework_ll_parse(fwts_framework *fw, const char *arg) return FWTS_ERROR; }
+/* + * fwts_framework_an_parse() + * parse arch (architecture) name option + */ +static int fwts_framework_an_parse(fwts_framework *fw, const char *arg) +{ + fw->target_arch = fwts_arch_get_arch(arg); + if (fw->target_arch == FWTS_ARCH_OTHER) { + fprintf(stderr, "--arch can be one of: %s\n", + fwts_arch_names()); + return FWTS_ERROR; + } + + return FWTS_OK; +} + int fwts_framework_options_handler(fwts_framework *fw, int argc, char * const argv[], int option_char, int long_index) { FWTS_UNUSED(argc); @@ -1280,6 +1297,10 @@ int fwts_framework_options_handler(fwts_framework *fw, int argc, char * const ar if (fwts_framework_ll_parse(fw, optarg) != FWTS_OK) return FWTS_ERROR; break; + case 43: /* --arch */ + if (fwts_framework_an_parse(fw, optarg) != FWTS_OK) + return FWTS_ERROR; + break; } break; case 'a': /* --all */ @@ -1381,6 +1402,10 @@ int fwts_framework_args(const int argc, char **argv) /* Set the power method to FWTS_PM_UNDEFINED before we parse arguments */ fw->pm_method = FWTS_PM_UNDEFINED;
+ /* Set host/target test architecture defaults */ + fw->host_arch = fwts_arch_get_host(); + fw->target_arch = fw->host_arch; + ret = fwts_args_add_options(fwts_framework_options, fwts_framework_options_handler, NULL); if (ret == FWTS_ERROR)
This patch adds in a series of detailed ACPI specification compliance tests for the MADT and most of the possible subtables (tests for the GIC ITS subtable will be added in a separate patch due to changes needed in FWTS header files).
The added tests for the MADT include verifying the versions of the table being used and basic content checks. The revision of the MADT is also checked against the version of the FADT to make sure they correspond to each other. We also verify that each subtable used is actually defined for this revision of the MADT and that for each of the possible subtable types, their contents are reasonably correct.
While this patch is based on the original src/acpi/madt.c file, it has been reorganized so that each subtable has a separate function in order to keep things clearer.
NB: this version replaces src/acpi/madt/madt.c. Earlier versions of this patch created a new file src/acpi/compliance/madt.c.
Signed-off-by: Al Stone al.stone@linaro.org --- src/acpi/madt/madt.c | 1472 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 1212 insertions(+), 260 deletions(-)
diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c index b237d23..d66e0c2 100644 --- a/src/acpi/madt/madt.c +++ b/src/acpi/madt/madt.c @@ -1,7 +1,6 @@ /* - * Copyright (C) 2015-2016 Canonical - * - * Portions of this code original from the Linux-ready Firmware Developer Kit + * Copyright (C) 2015 Canonical + * Portions added (c) 2015, Al Stone ahs3@redhat.com * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -13,10 +12,6 @@ * 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"
@@ -25,327 +20,1284 @@ #include <unistd.h> #include <inttypes.h> #include <string.h> +#include <ctype.h> + +/* + * The Long, Sad, True Story of the MADT + * + * Once upon a time in ACPI 1.0, there was the MADT. It was a nice table, + * and it had two subtables all of its own. But, it was also a pretty + * busy table, too, so over time the MADT gathered up other nice little + * subtables. By the time ACPI 6.0 came around, the MADT had 16 of the + * little guys. + * + * Now, the MADT kept a little counter around for the subtables. In fact, + * it kept two counters: one was the revision level, which was supposed to + * change when new subtables came to be, or as the ones already around grew + * up. The second counter was a type number, because the MADT needed a unique + * type for each subtable so he could tell them apart. But, sometimes the + * MADT got so busy, he forgot to increment the revision level when he needed + * to. Fortunately, the type counter kept increasing since that's the only + * way the MADT could find each little subtable. It just wouldn't do to have + * every subtable called Number 6. + * + * In the next valley over, a castle full of wizards was watching the MADT + * and made a pact to keep their own counter. Every time the MADT found a + * new subtable, or a subtable grew up, the wizards promised they would + * increment their counter. Well, wizards being the forgetful sort, they + * didn't alway do that. And, since there quite a lot of them, they + * couldn't always remember who was supposed to keep track of the MADT, + * especially if dinner was coming up soon. Their counter was called the + * spec version. + * + * Every now and then, the MADT would gather up all its little subtables + * and take them in to the cobbler to get new boots. This was a very, very + * meticulous cobbler, so every time they came, he wrote down all the boot + * sizes for all of the little subtables. The cobbler would ask each subtable + * for its length, check that against his careful notes, and then go get the + * right boots. Sometimes, a little subtable would change a bit, and their + * length did not match what the cobbler had written down. If the wizards + * or the MADT had incremented their counters, the cobbler would breath a + * sigh of relief and write down the new length as the right one. But, if + * none of the counters had changed, this would make the cobbler very, very + * mad. He couldn't tell if he had the right size boots or not for the + * little subtable. He would have to *guess* and this really bugged him. + * + * Well, when the cobbler got mad like this, he would go into hiding. He + * would not make or sell any boots. He would not go out at all. Pretty + * soon, the coffee shop would have to close because the cobbler wasn't + * coming by twice a day any more. Then the grocery store would have to + * close because he wouldn't eat much. After a while, everyone would panic + * and have to move from the village and go live with all their relatives + * (usually the ones they didn't like very much). + * + * Eventually, the cobbler would work his way out of his bad mood, and + * open up his boot business again. Then, everyone else could move back + * to the village and restart their lives, too. + * + * Fortunately, we have been able to collect up all the cobbler's careful + * notes (and we wrote them down below). We'll have to keep checking these + * notes over time, too, just as the cobbler does. But, in the meantime, + * we can avoid the panic and the reboot since we can make sure that each + * subtable is doing okay. And that's what our tests below check. + * + * + * FADT Major Version -> 1 3 4 4 5 5 6 + * FADT Minor Version -> x x x x x 1 0 + * MADT revision -> 1 1 2 3 3 3 3 + * Spec Version -> 1.0 2.0 3.0b 4.0a 5.0b 5.1a 6.0 + * Subtable Name Type Expected Length -> + * Processor Local APIC 0x0 8 8 8 8 8 8 8 + * IO APIC 0x1 12 12 12 12 12 12 12 + * Int Src Override 0x2 10 10 10 10 10 10 10 + * NMI Src 0x3 8 8 8 8 8 8 8 + * Local APIC NMI Struct 0x4 6 6 6 6 6 6 6 + * Local APIC Addr Ovrrd 0x5 16 12 12 12 12 12 + * IO SAPIC 0x6 20 16 16 16 16 16 + * Local SAPIC 0x7 8 >16 >16 >16 >16 >16 + * Platform Int Src 0x8 16 16 16 16 16 16 + * Proc Local x2APIC 0x9 16 16 16 16 + * Local x2APIC NMI 0xa 12 12 12 12 + * GICC CPU I/F 0xb 40 76 80 + * GICD 0xc 24 24 24 + * GICv2m MSI 0xd 24 24 + * GICR 0xe 16 16 + * GIC ITS 0xf 20 + * + * In the table, each length entry is what should be in the length + * field of the subtable, and -- in general -- it should match the + * size of the struct for the subtable. Any value that is not set + * (i.e., is zero) indicates that the subtable is not defined for + * that version of the ACPI spec. + * + */ + +#define FADT_MAX_MAJOR_REVISION ((uint8_t)6) +#define FADT_MAX_MINOR_REVISION ((uint8_t)0) +#define MADT_MAX_REVISION ((uint8_t)4) + +#define SUBTABLE_UNDEFINED 0x00 +#define SUBTABLE_VARIABLE 0xff +#define NUM_SUBTABLE_TYPES 16 + +struct acpi_madt_subtable_lengths { + unsigned short major_version; /* from revision in FADT header */ + unsigned short minor_version; /* FADT field starting with 5.1 */ + unsigned short madt_version; /* MADT revision */ + unsigned short num_types; /* types possible for this version */ + unsigned short lengths[NUM_SUBTABLE_TYPES]; + /* subtable lengths, indexed by type */ +}; + +static struct acpi_madt_subtable_lengths spec_info[] = { + { /* for ACPI 1.0b */ + .major_version = 1, + .minor_version = 0, + .madt_version = 1, + .num_types = 5, + .lengths = { 8, 12, 10, 8, 6 } + }, + { /* for ACPI 2.0 */ + .major_version = 3, + .minor_version = 0, + .madt_version = 1, + .num_types = 9, + .lengths = { 8, 12, 10, 8, 6, 16, 20, 8, 16 } + }, + { /* for ACPI 3.0b */ + .major_version = 4, + .minor_version = 0, + .madt_version = 2, + .num_types = 9, + .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, 16 } + }, + { /* for ACPI 4.0a */ + .major_version = 4, + .minor_version = 0, + .madt_version = 3, + .num_types = 11, + .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, + 16, 16, 12 } + }, + { /* for ACPI 5.0b */ + .major_version = 5, + .minor_version = 0, + .madt_version = 3, + .num_types = 13, + .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, + 16, 16, 12, 40, 24 } + }, + { /* for ACPI 5.1a */ + .major_version = 5, + .minor_version = 1, + .madt_version = 3, + .num_types = 15, + .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, + 16, 16, 12, 76, 24, 24, 16 } + }, + { /* for ACPI 6.0 */ + .major_version = 6, + .minor_version = 0, + .madt_version = 3, + .num_types = 16, + .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, + 16, 16, 12, 80, 24, 24, 16, 16 } + }, + { /* terminator */ + .major_version = 0, + .minor_version = 0, + .madt_version = 0, + .num_types = 0, + .lengths = { 0 } + } +}; + +static struct acpi_madt_subtable_lengths *spec_data; +static uint8_t fadt_major; +static uint8_t fadt_minor;
-static fwts_acpi_table_info *table; +static fwts_acpi_table_info *mtable; +static fwts_acpi_table_info *ftable; + +static fwts_list msi_frame_ids;
static int madt_init(fwts_framework *fw) { - if (fwts_acpi_find_table(fw, "APIC", 0, &table) != FWTS_OK) { - fwts_log_error(fw, "Cannot read ACPI tables."); + fwts_acpi_table_madt *madt; + fwts_acpi_table_fadt *fadt; + struct acpi_madt_subtable_lengths *ms = spec_info; + + /* find the ACPI tables needed */ + if (fwts_acpi_find_table(fw, "APIC", 0, &mtable) != FWTS_OK) { + fwts_log_error(fw, "Cannot find ACPI MADT tables."); return FWTS_ERROR; } - if (table == NULL || (table && table->length == 0)) { - fwts_log_error(fw, "ACPI MADT (APIC) table does not exist, skipping test"); - return FWTS_SKIP; + if (!mtable) { + fwts_log_error(fw, "Cannot read ACPI MADT tables."); + return FWTS_ERROR; } + + if (fwts_acpi_find_table(fw, "FACP", 0, &ftable) != FWTS_OK) { + fwts_log_error(fw, "Cannot find ACPI FADT tables."); + return FWTS_ERROR; + } + if (!ftable) { + fwts_log_error(fw, "Cannot read ACPI FADT tables."); + return FWTS_ERROR; + } + + if (!mtable || mtable->length == 0) { + fwts_log_error(fw, "Required ACPI MADT (APIC) table not found"); + return FWTS_ERROR; + } + if (!ftable || ftable->length == 0) { + fwts_log_error(fw, "Required ACPI FADT (FACP) table not found"); + return FWTS_ERROR; + } + + /* determine the reference data needed */ + madt = (fwts_acpi_table_madt *)mtable->data; + fadt = (fwts_acpi_table_fadt *)ftable->data; + + fadt_major = fadt->header.revision; + fadt_minor = 0; + if (fadt_major >= 5 && fadt->header.length >= 268) + fadt_minor = fadt->minor_version; /* field added ACPI 5.1 */ + + /* find the first occurrence for this version of MADT */ + while (ms->num_types != 0) { + if (ms->madt_version == madt->header.revision) + break; + ms++; + } + + /* now, find the largest FADT version supported */ + spec_data = NULL; + while (ms->num_types && ms->madt_version == madt->header.revision) { + if (ms->major_version <= fadt_major && + ms->minor_version <= fadt_minor) { + spec_data = ms; + } + ms++; + } + + /* initialize the MSI frame ID list should we need it later */ + fwts_list_init(&msi_frame_ids); + + return (spec_data) ? FWTS_OK : FWTS_ERROR; +} + +static int madt_checksum(fwts_framework *fw) +{ + const uint8_t *data = mtable->data; + ssize_t length = mtable->length; + uint8_t checksum = 0; + + /* verify the table checksum */ + checksum = fwts_checksum(data, length); + if (checksum == 0) + fwts_passed(fw, "MADT checksum is correct"); + else + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTChecksum", + "MADT checksum is incorrect: 0x%x", checksum); + return FWTS_OK; }
-/* - * MADT Multiple APIC Description Table - */ -static int madt_test1(fwts_framework *fw) +static int madt_revision(fwts_framework *fw) { - bool passed = true; + fwts_acpi_table_madt *madt = (fwts_acpi_table_madt *)mtable->data; + struct acpi_madt_subtable_lengths *ms = spec_data;
- fwts_acpi_table_madt *madt = (fwts_acpi_table_madt*)table->data; - fwts_list msi_frame_ids; - const uint8_t *data = table->data; - ssize_t length = table->length; - int i = 0; + /* check the table revision */ + fwts_log_advice(fw, "Most recent FADT revision is %d.%d.", + FADT_MAX_MAJOR_REVISION, FADT_MAX_MINOR_REVISION); + fwts_log_advice(fw, "Most recent MADT revision is %d.", + MADT_MAX_REVISION);
- fwts_list_init(&msi_frame_ids); + /* is the madt revision defined at all? */ + if (!ms->num_types) { + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTRevision", + "Undefined MADT revision being used: %d", + madt->header.revision); + } else { + fwts_passed(fw, "MADT revision %d is defined.", + madt->header.revision); + } + + /* is the madt revision in sync with the fadt revision? */ + if (ms->major_version != fadt_major || + ms->minor_version != fadt_minor) { + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTFADTRevisions", + "MADT revision is not in sync with " + "the FADT revision;\n" + "MADT %d expects FADT %d.%d " + "but found %d.%d instead.", + madt->header.revision, + ms->major_version, ms->minor_version, + fadt_major, fadt_minor); + } else { + fwts_passed(fw, "MADT revision %d is in sync " + "with FADT revision %d.%d.", + madt->header.revision, fadt_major, fadt_minor); + } + + return FWTS_OK; +}
- if (madt->flags & 0xfffffffe) { - passed = false; +static int madt_arch_revision(fwts_framework *fw) +{ + fwts_acpi_table_madt *madt = (fwts_acpi_table_madt *)mtable->data; + uint8_t minrev; + const char *arch; + + /* initialize starting assumptions */ + if (fw->target_arch == FWTS_ARCH_ARM64) { + minrev = 3; + arch = "aarch64"; + } else { + minrev = 1; + arch = "intel"; + } + + /* check the supported revision for this architecture */ + if (madt->header.revision >= minrev) + fwts_passed(fw, "MADT revision %d meets the minimum needed " + "(%d) for the %s architecture.", + madt->header.revision, minrev, arch); + else + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTArchRevision", + "MADT revision is %d, must be >= %d " + "when running on %s", + madt->header.revision, minrev, arch); + + return FWTS_OK; +} + +static int madt_flags(fwts_framework *fw) +{ + fwts_acpi_table_madt *madt = (fwts_acpi_table_madt *)mtable->data; + + /* make sure the reserved bits in the flag field are zero */ + if (madt->flags & 0xfffffffe) fwts_failed(fw, LOG_LEVEL_MEDIUM, "MADTFlagsNonZero", "MADT flags field, bits 1..31 are reserved and " "should be zero, but are set as: %" PRIx32 ".\n", madt->flags); + else + fwts_passed(fw, "MADT flags reserved bits are not set."); + + return FWTS_OK; +} + +static const char *madt_sub_names[] = { + /* 0x00 */ "Processor Local APIC", + /* 0x01 */ "I/O APIC", + /* 0x02 */ "Interrupt Source Override", + /* 0x03 */ "Non-maskable Interrupt Source (NMI)", + /* 0x04 */ "Local APIC NMI", + /* 0x05 */ "Local APIC Address Override", + /* 0x06 */ "I/O SAPIC", + /* 0x07 */ "Local SAPIC", + /* 0x08 */ "Platform Interrupt Sources", + /* 0x09 */ "Processor Local x2APIC", + /* 0x0a */ "Local x2APIC NMI", + /* 0x0b */ "GICC CPU Interface", + /* 0x0c */ "GICD GIC Distributor", + /* 0x0d */ "GICv2m MSI Frame", + /* 0x0e */ "GICR Redistributor", + /* 0x0f */ "GIC Interrupt Translation Service (ITS)", + NULL +}; + +static int madt_local_apic(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 0: Processor Local APIC */ + fwts_acpi_madt_processor_local_apic *lapic = + (fwts_acpi_madt_processor_local_apic *)data; + + /* + * TODO: verify UID field has a corresponding + * Processor device object with a matching + * _UID result + */ + + if (lapic->flags & 0xfffffffe) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTAPICFlagsNonZero", + "MADT %s flags field, bits 1..31 are " + "reserved and should be zero, but are set " + "to: %" PRIx32 ".", + madt_sub_names[hdr->type], lapic->flags); + else + fwts_passed(fw, + "MADT %s flags field, bits 1..31 are " + "reserved and properly set to zero.", + madt_sub_names[hdr->type]); + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_io_apic(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 1: I/O APIC */ + fwts_acpi_madt_io_apic *ioapic = (fwts_acpi_madt_io_apic *)data; + + if (ioapic->reserved != 0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTIOAPICFlags", + "MADT %s flags are reserved, should be zero " + "but are set to: %" PRIx32 ".", + madt_sub_names[hdr->type], ioapic->reserved); + else + fwts_passed(fw, + "MADT %s flags are reserved, and set to zero.", + madt_sub_names[hdr->type]); + + if (ioapic->io_apic_phys_address == 0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTIOAPICAddrZero", + "MADT %s address is zero, needs to be defined.", + madt_sub_names[hdr->type]); + else + fwts_passed(fw, "MADT %s address in non-zero.", + madt_sub_names[hdr->type]); + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_interrupt_override(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 2: Interrupt Source Override */ + fwts_acpi_madt_interrupt_override *int_override = + (fwts_acpi_madt_interrupt_override *)data; + + if (int_override->bus != 0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTIRQSrcISA", + "MADT %s bus should be 0 for ISA bus.", + madt_sub_names[hdr->type]); + else + fwts_passed(fw, "MADT %s Bus is 0 for ISA bus.", + madt_sub_names[hdr->type]); + + if (int_override->flags & 0xfffffff0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTIRQSrcFlags", + "MADT %s flags, bits 4..31 are reserved " + "and should be zero, but are set as: %" PRIx32 ".", + madt_sub_names[hdr->type], + int_override->flags); + else + fwts_passed(fw, + "MADT %s flags, bits 4..31 are reserved " + "and set to zero.", + madt_sub_names[hdr->type]); + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_nmi_source(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 3: NMI Source */ + fwts_acpi_madt_nmi *nmi = (fwts_acpi_madt_nmi *)data; + + if (nmi->flags & 0xfffffff0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTNMISrcFlags", + "MADT %s flags, bits 4..31 are reserved " + "and should be zero, but are set as: %" PRIx32 ".", + madt_sub_names[hdr->type], nmi->flags); + else + fwts_passed(fw, + "MADT %s flags, bits 4..31 are reserved " + "and set to zero.", + madt_sub_names[hdr->type]); + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_local_apic_nmi(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 4: Local APIC NMI */ + fwts_acpi_madt_local_apic_nmi *nmi = + (fwts_acpi_madt_local_apic_nmi *)data; + + /* + * TODO: verify UID field has a corresponding + * Processor device object with a matching + * _UID result + */ + + if (nmi->flags & 0xfffffff0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTLAPICNMIFlags", + "MADT %s flags, bits 4..31 are reserved " + "and should be zero, but are set as: %" PRIx32 ".", + madt_sub_names[hdr->type], nmi->flags); + else + fwts_passed(fw, + "MADT %s flags, bits 4..31 are reserved " + "and set to zero.", + madt_sub_names[hdr->type]); + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_lapic_addr_override(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 5: Local APIC Address Override */ + static int count; + fwts_acpi_madt_local_apic_addr_override *laao = + (fwts_acpi_madt_local_apic_addr_override *)data; + + count++; + if (laao->reserved != 0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTLAPICOVRReserved", + "MADT %s bytes 2..4 are reserved " + "and should be zero, but are set as: %" PRIx32 ".", + madt_sub_names[hdr->type], laao->reserved); + else + fwts_passed(fw, + "MADT %s bytes 2..4 are reserved and set to zero.", + madt_sub_names[hdr->type]); + + if (laao->address == 0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTLAPICOVRAddress", + "MADT %s address should not be set to zero.", + madt_sub_names[hdr->type]); + else + fwts_passed(fw, + "MADT %s address set to non-zero value.", + madt_sub_names[hdr->type]); + + if (count > 1) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTLAPICOVRCount", + "Only one MADT %s allowed, %d found.", + madt_sub_names[hdr->type], count); + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_io_sapic(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 6: I/O SAPIC */ + fwts_acpi_madt_io_sapic *sapic = (fwts_acpi_madt_io_sapic *)data; + + if (sapic->reserved != 0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTIOSAPICReserved", + "MADT %s byte offset 3 is reserved " + "and should be zero, but is set to: %" PRIx32 ".", + madt_sub_names[hdr->type], sapic->reserved); + else + fwts_passed(fw, + "MADT %s bytes 2..4 are reserved and set to zero.", + madt_sub_names[hdr->type]); + + if (sapic->address == 0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADIOSPAICAddrZero", + "MADT %s address is zero, needs to be defined.", + madt_sub_names[hdr->type]); + else + fwts_passed(fw, + "MADT %s address set to non-zero value.", + madt_sub_names[hdr->type]); + + /* + * TODO: if both I/O APIC and I/O SAPIC subtables exist, there + * must be at least as many I/O SAPIC subtables as I/O APIC subtables, + * and that every I/O APIC has a corresponding I/O SAPIC with the + * same APIC ID. + */ + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_local_sapic(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 7: Processor Local SAPIC */ + fwts_acpi_madt_local_sapic *lsapic = (fwts_acpi_madt_local_sapic *)data; + uint8_t tmp; + int ii; + + /* + * TODO: if using the SAPIC model, check that each processor in + * the system has a SAPIC record as required. The table is not + * required to match hotplug additions, but should at least match + * initial boot state. + */ + + if (hdr->length != (strlen(lsapic->uid_string) + 16)) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTLSAPICLength", + "MADT %s length of %d bytes does not match " + "actual length of %ld bytes.", + madt_sub_names[hdr->type], hdr->length, + strlen(lsapic->uid_string) + 16); + else + fwts_passed(fw, + "MADT %s table length (%d) is correct.", + madt_sub_names[hdr->type], hdr->length); + + /* + * TODO: should the processor ID field be zero? The use of + * the Processor object has been deprecated in 6.0, and this + * field is used to match this local SAPIC to the processor + * via the ID. This has been replaced, I believe, with the + * processor device object _UID result. On the other hand, + * perhaps older tables are still using Processor objects.... + */ + + for (tmp = 0, ii = 0; ii < 3; tmp |= lsapic->reserved[ii], ii++) + continue; + if (tmp) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTLSAPICReservedNonZero", + "MADT %s reserved field, byte offsets 5..8 are " + "reserved and should be zero, but are set " + "to: 0x%2x %2x %2x.", + madt_sub_names[hdr->type], + lsapic->reserved[0], lsapic->reserved[1], + lsapic->reserved[2]); + else + fwts_passed(fw, + "MADT %s reserved field, byte offsets 5..8 are " + "reserved and properly set to zero.", + madt_sub_names[hdr->type]); + + if (lsapic->flags & 0xfffffffe) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTLSAPICFlagsNonZero", + "MADT %s flags field, bits 1..31 are " + "reserved and should be zero, but are set " + "to: %" PRIx32 ".", + madt_sub_names[hdr->type], lsapic->flags); + else + fwts_passed(fw, + "MADT %s flags field, bits 1..31 are " + "reserved and properly set to zero.", + madt_sub_names[hdr->type]); + + /* + * TODO: verify UID field has a corresponding + * Processor device object with a matching + * _UID result (or Processor object ID?) + */ + + for (tmp = 0, ii = 0; ii < (int)strlen(lsapic->uid_string); ii++) + if (isascii(lsapic->uid_string[ii])) + tmp++; + else + break; + if (tmp < strlen(lsapic->uid_string)) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTLSAPICUIDNonAscii", + "MADT %s UID string has non-ASCII character " + "in position %d.", + madt_sub_names[hdr->type], tmp); + else + fwts_passed(fw, + "MADT %s UID string is an ASCII string.", + madt_sub_names[hdr->type]); + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_platform_int_source(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 8: Platform Interrupt Sources */ + fwts_acpi_madt_platform_int_source *src = + (fwts_acpi_madt_platform_int_source *)data; + + if (src->flags & 0xfffffff0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTPlatIRQSrcFlags", + "MADT %s flags, bits 4..31 are reserved and " + "should be zero, but are set as: %" PRIx32 ".", + madt_sub_names[hdr->type], src->flags); + else + fwts_passed(fw, + "MADT %s flags, bits 4..31 are reserved and " + "set to zero.", + madt_sub_names[hdr->type]); + + if (src->type < 1 || src->type > 3) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTPlatIRQType", + "MADT %s type field is %" PRIu8 ", must be 1..3.", + madt_sub_names[hdr->type], src->type); + else + fwts_passed(fw, + "MADT %s type field is %" PRIu8 "and in 1..3.", + madt_sub_names[hdr->type], src->type); + + /* TODO: verify Processor ID actually exists. */ + + /* TODO: verify Processor EID actually exists. */ + + if (src->io_sapic_vector == 0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTPlatIRQIOSAPICVector", + "MADT %s IO SAPIC Vector is " + "zero, appears not to be defined.", + madt_sub_names[hdr->type]); + else + fwts_passed(fw, + "MADT %s IO SAPIC Vector is non-zero.", + madt_sub_names[hdr->type]); + + /* TODO: verify the GSI is properly set? */ + + if (src->pis_flags & 0xfffffffe) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTPlatIRQSrcFlagsNonZero", + "MADT %s, Platform Interrupt Source flag " + "bits 1..31 are reserved and should be zero, " + "but are set as: %" PRIx32 ".", + madt_sub_names[hdr->type], src->pis_flags); + else + fwts_passed(fw, + "MADT %s, Platform Interrupt Source flag " + "bits 1..31 are reserved and set to zero.", + madt_sub_names[hdr->type]); + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_local_x2apic(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 9: Processor Local x2APIC */ + fwts_acpi_madt_local_x2apic *lx2apic = + (fwts_acpi_madt_local_x2apic *)data; + + if (lx2apic->reserved) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTLX2APICReservedNonZero", + "MADT %s reserved field, byte offsets 2..3 are " + "reserved and should be zero, but are set " + "to: 0x%8x.", + madt_sub_names[hdr->type], lx2apic->reserved); + else + fwts_passed(fw, + "MADT %s reserved field, byte offsets 2..3 are " + "reserved and properly set to zero.", + madt_sub_names[hdr->type]); + + /* TODO: do we need to verify that the x2APIC ID is > 255? */ + + if (lx2apic->flags & 0xfffffffe) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTX2APICFlagsNonZero", + "MADT %s flags field, bits 1..31 are " + "reserved and should be zero, but are set " + "to: %" PRIx32 ".", + madt_sub_names[hdr->type], lx2apic->flags); + else + fwts_passed(fw, + "MADT %s flags field, bits 1..31 are " + "reserved and properly set to zero.", + madt_sub_names[hdr->type]); + + /* + * TODO: verify UID field has a corresponding + * Processor device object with a matching + * _UID result + */ + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_local_x2apic_nmi(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 0xa: Local x2APIC NMI */ + fwts_acpi_madt_local_x2apic_nmi *nmi = + (fwts_acpi_madt_local_x2apic_nmi *)data; + + if (nmi->flags & 0xfffffff0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTLAPICX2APICNMIFlags", + "MADT %s, flags, bits 4..31 are reserved and " + "should be zero, but are set as: %" PRIx32 ".", + madt_sub_names[hdr->type], nmi->flags); + else + fwts_passed(fw, + "MADT %s flags field, bits 4..31 are " + "reserved and properly set to zero.", + madt_sub_names[hdr->type]); + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_gicc(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 0xb: GICC */ + fwts_acpi_madt_gic *gic = (fwts_acpi_madt_gic *)data; + uint32_t mask; + int start; + + if (gic->reserved) + fwts_failed(fw, LOG_LEVEL_LOW, + "MADTGICCReservedNonZero", + "MADT %s reserved field should be zero, but is " + "instead 0x%" PRIx32 ".", + madt_sub_names[hdr->type], gic->reserved); + else + fwts_passed(fw, + "MADT %s reserved field properly set to zero.", + madt_sub_names[hdr->type]); + + /* + * TODO: verify UID field has a corresponding + * Processor device object with a matching + * _UID result + */ + + mask = 0xfffffffc; + start = 2; + if (hdr->length == 80) { /* ACPI 6.0 */ + mask = 0xfffffff8; + start = 3; + } + if (gic->flags & mask) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTGICFLags", + "MADT %s, flags, bits %d..31 are reserved " + "and should be zero, but are set as: %" PRIx32 ".", + madt_sub_names[hdr->type], start, gic->flags); + else + fwts_passed(fw, + "MADT %s, flags, bits %d..31 are reserved and " + "properly set to zero.", + madt_sub_names[hdr->type], start); + + if (gic->parking_protocol_version != 0 && + gic->parking_protocol_version != 1) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTGICCParkingVersion", + "MADT %s, protocol versions defined are 0..1 but " + "%d is being used.", + madt_sub_names[hdr->type], + gic->parking_protocol_version); + else + fwts_passed(fw, + "MADT %s, is using a defined parking protocol " + "version.", + madt_sub_names[hdr->type]); + + /* + * TODO: is it even possible to verify the MPIDR is valid? Or, + * is there sufficient variation that it is not predictable? + */ + + if (hdr->length == 80) { /* added in ACPI 6.0 */ + uint8_t tmp = gic->reserved2[0] | gic->reserved2[1] | + gic->reserved2[2]; + + if (tmp) + fwts_failed(fw, LOG_LEVEL_LOW, + "MADTGICCReserved2NonZero", + "MADT %s second reserved field must " + "be zero.", madt_sub_names[hdr->type]); + else + fwts_passed(fw, + "MADT %s second reserved field properly " + "set to zero.", + madt_sub_names[hdr->type]); }
+ /* + * TODO: the local GICC corresponding to the boot processor must + * be the the first entry in the interrupt controller structure + * list. + */ + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_gicd(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 0xc: GIC Distributor */ + fwts_acpi_madt_gicd *gicd = (fwts_acpi_madt_gicd *)data; + + uint32_t gicd_reserve2 = gicd->reserved2[0] + + (gicd->reserved2[1] << 4) + + (gicd->reserved2[2] << 8); + + if (gicd->reserved) + fwts_failed(fw, LOG_LEVEL_LOW, + "MADTGICDReservedNonZero", + "MADT %s reserved field should be zero, " + "instead got 0x%" PRIx32 ".", + madt_sub_names[hdr->type], gicd->reserved); + else + fwts_passed(fw, + "MADT %s reserved field properly set to zero.", + madt_sub_names[hdr->type]); + + /* TODO: is the physical base address required to be non-zero? */ + + if (gicd->gic_version != 0 && gicd->gic_version > 4) + fwts_failed(fw, LOG_LEVEL_LOW, + "SPECMADTGICDVersion", + "MADT %s GIC version field should be in 0..4, " + "but instead have 0x%" PRIx32 ".", + madt_sub_names[hdr->type], gicd->gic_version); + else + fwts_passed(fw, + "MADT %s GIC version field is in 0..4.", + madt_sub_names[hdr->type]); + + if (gicd_reserve2) + fwts_failed(fw, LOG_LEVEL_LOW, + "MADTGICDReserved2NonZero", + "MADT %s second reserved field should be zero, " + "instead got 0x%" PRIx32 ".", + madt_sub_names[hdr->type], gicd_reserve2); + else + fwts_passed(fw, + "MADT %s second reserved field is properly set " + "to zero.", madt_sub_names[hdr->type]); + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_gic_msi_frame(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 0xd: GIC MSI Frame */ + fwts_acpi_madt_gic_msi *gic_msi = (fwts_acpi_madt_gic_msi *)data; + fwts_list_link *item; + bool found; + + /* + * TODO: is there some way to test that the entries found are + * for only non-secure MSI frames? + */ + + if (gic_msi->reserved) + fwts_failed(fw, LOG_LEVEL_LOW, + "MADTGICMSIReservedNonZero", + "MADT %s reserved field should be zero, " + "instead got 0x%" PRIx32 ".", + madt_sub_names[hdr->type], gic_msi->reserved); + else + fwts_passed(fw, + "MADT %s reserved field properly set to zero.", + madt_sub_names[hdr->type]); + + /* + * Check MSI Frame ID against previously found IDs to see if it + * is unique. According to the spec, they must be. + */ + found = false; + fwts_list_foreach(item, &msi_frame_ids) { + uint32_t *frame_id = fwts_list_data(uint32_t *, item); + + if (*frame_id == gic_msi->frame_id) + found = true; + } + if (found) { + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTGICMSINonUniqueFrameId", + "MADT %s Frame ID 0x%" PRIx32 " is not unique " + "and has already be defined in a previous %s.", + madt_sub_names[hdr->type], + gic_msi->frame_id, + madt_sub_names[hdr->type]); + } else { + fwts_list_append(&msi_frame_ids, &(gic_msi->frame_id)); + fwts_passed(fw, + "MADT %s Frame ID 0x%" PRIx32 " is unique " + "as is required.", + madt_sub_names[hdr->type], + gic_msi->frame_id); + } + + /* + * TODO: can the physical base address be tested, or is zero + * allowed? + */ + + if (gic_msi->flags & 0xfffffffe) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "MADTGICMSIFLags", + "MADT %s, flags, bits 1..31 are reserved " + "and should be zero, but are set as: %" PRIx32 ".", + madt_sub_names[hdr->type], + gic_msi->flags); + else + fwts_passed(fw, + "MADT %s, flags, bits 1..31 are reserved " + "and properly set to zero.", + madt_sub_names[hdr->type]); + + /* + * TODO: can we check the SPI Count and SPI Base against the MSI_TYPER + * register in the frame at this point? Or is this something that + * can only been done when running on the arch we're testing for? + */ + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_gicr(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 0xe: GICR */ + fwts_acpi_madt_gicr *gicr = (fwts_acpi_madt_gicr *)data; + + /* + * TODO: GICR structures should only be used when GICs implement + * version 3 or higher. + */ + + if (gicr->reserved) + fwts_failed(fw, LOG_LEVEL_LOW, + "MADTGICRReservedNonZero", + "MADT %s reserved field should be zero, " + "instead got 0x%" PRIx32 ".", + madt_sub_names[hdr->type], + gicr->reserved); + else + fwts_passed(fw, + "MADT %s reserved field properly set to zero.", + madt_sub_names[hdr->type]); + + /* + * TODO: can Discovery Range Base Address ever be zero? + * Or, can we assume it must be non-zero? + */ + + if (gicr->discovery_range_length == 0) + fwts_failed(fw, LOG_LEVEL_LOW, + "SPECMADTGICRZeroLength", + "MADT %s discovery range length should be > 0.", + madt_sub_names[hdr->type]); + else + fwts_passed(fw, + "MADT %s discovery range length of %d > 0.", + madt_sub_names[hdr->type], + gicr->discovery_range_length); + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + +static int madt_subtables(fwts_framework *fw) +{ + fwts_acpi_table_madt *madt = (fwts_acpi_table_madt *)mtable->data; + fwts_acpi_madt_sub_table_header *hdr; + fwts_acpi_madt_local_sapic *lsapic; + struct acpi_madt_subtable_lengths *ms = spec_data; + const uint8_t *data = mtable->data; + ssize_t length = mtable->length; + ssize_t skip; + int ii = 0; + int len, proper_len; + bool passed = true; + + /* + * check the correctness of each subtable type, and whether or + * not the subtable is allowed for this revision of the MADT + */ + data += sizeof(fwts_acpi_table_madt); length -= sizeof(fwts_acpi_table_madt);
+ if (!ms->num_types) { + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTRevision", + "Undefined MADT revision being used: %d", + madt->header.revision); + } else { + fwts_passed(fw, "MADT revision %d is defined.", + madt->header.revision); + } + while (length > (ssize_t)sizeof(fwts_acpi_madt_sub_table_header)) { - fwts_acpi_madt_sub_table_header *hdr = (fwts_acpi_madt_sub_table_header*)data; - ssize_t skip = 0; - i++; + hdr = (fwts_acpi_madt_sub_table_header *)data; + skip = 0; + ii++;
data += sizeof(fwts_acpi_madt_sub_table_header); length -= sizeof(fwts_acpi_madt_sub_table_header);
- switch (hdr->type) { - case FWTS_ACPI_MADT_LOCAL_APIC: { - fwts_acpi_madt_processor_local_apic *lapic = (fwts_acpi_madt_processor_local_apic *)data; - if (lapic->flags & 0xfffffffe) { + /* is this subtable type defined? */ + len = ms->lengths[hdr->type]; + if (!len) { + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTSubType", + "Undefined MADT subtable type for this " + "version of the MADT: %d (%s)", + hdr->type, madt_sub_names[hdr->type]); + } else { + fwts_passed(fw, + "MADT subtable type %d (%s) is defined.", + hdr->type, madt_sub_names[hdr->type]); + } + + /* verify that the length is what we expect */ + passed = true; + if (len == SUBTABLE_VARIABLE) { + if (hdr->type == FWTS_ACPI_MADT_LOCAL_SAPIC) { + lsapic = (fwts_acpi_madt_local_sapic *)hdr; + proper_len = + sizeof(fwts_acpi_madt_local_sapic) + + strlen(lsapic->uid_string) + 1; + + if (proper_len != hdr->length) passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTAPICFlagsNonZero", - "MADT Local APIC flags field, bits 1..31 are reserved and " - "should be zero, but are set as: %" PRIx32 ".", - lapic->flags); - } - skip = sizeof(fwts_acpi_madt_processor_local_apic); } + } else { + if (hdr->length != len) + passed = false; + } + if (passed) { + fwts_passed(fw, + "Subtable %d of type %d (%s) is the " + " correct length: %d", + ii, hdr->type, + madt_sub_names[hdr->type], + hdr->length); + } else { + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTSubLen", + "Subtable %d of type %d (%s) is %d bytes " + " long but should be %d bytes", + ii, hdr->type, + madt_sub_names[hdr->type], + hdr->length, len); + } + + /* perform checks specific to subtable types */ + switch (hdr->type) { + case FWTS_ACPI_MADT_LOCAL_APIC: + skip = madt_local_apic(fw, hdr, data); break; - case FWTS_ACPI_MADT_IO_APIC: { - fwts_acpi_madt_io_apic *ioapic = (fwts_acpi_madt_io_apic*)data; - if (ioapic->io_apic_phys_address == 0) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTIOAPICAddrZero", - "MADT IO APIC address is zero, appears not to be defined."); - } - skip = sizeof(fwts_acpi_madt_io_apic); - } + + case FWTS_ACPI_MADT_IO_APIC: + skip = madt_io_apic(fw, hdr, data); break; - case FWTS_ACPI_MADT_INTERRUPT_OVERRIDE: { - fwts_acpi_madt_interrupt_override *int_override = (fwts_acpi_madt_interrupt_override*)data; - if (int_override->bus != 0) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTIRQSrcISA", - "MADT Interrupt Source Override Bus should be 0 for ISA bus."); - } - if (int_override->flags & 0xfffffff0) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTIRQSrcFlags", - "MADT Interrupt Source Override flags, bits 4..31 are reserved " - "and should be zero, but are set as: %" PRIx32 ".", - int_override->flags); - } - skip = sizeof(fwts_acpi_madt_interrupt_override); - } + + case FWTS_ACPI_MADT_INTERRUPT_OVERRIDE: + skip = madt_interrupt_override(fw, hdr, data); break; - case FWTS_ACPI_MADT_NMI_SOURCE: { - fwts_acpi_madt_nmi *nmi = (fwts_acpi_madt_nmi*)data; - if (nmi->flags & 0xfffffff0) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTNMISrcFlags", - "MADT Non-Maskable Interrupt Source, flags, bits 4..31 are reserved " - "and should be zero, but are set as: %" PRIx32 ".", - nmi->flags); - } - skip = sizeof(fwts_acpi_madt_nmi); - } + + case FWTS_ACPI_MADT_NMI_SOURCE: + skip = madt_nmi_source(fw, hdr, data); break; - case FWTS_ACPI_MADT_LOCAL_APIC_NMI: { - fwts_acpi_madt_local_apic_nmi *nmi = (fwts_acpi_madt_local_apic_nmi*)data; - if (nmi->flags & 0xfffffff0) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTLAPICNMIFlags", - "MADT Local APIC NMI flags, bits 4..31 are reserved " - "and should be zero, but are set as: %" PRIx32 ".", - nmi->flags); - } - skip = sizeof(fwts_acpi_madt_local_apic_nmi); - } + + case FWTS_ACPI_MADT_LOCAL_APIC_NMI: + skip = madt_local_apic_nmi(fw, hdr, data); break; + case FWTS_ACPI_MADT_LOCAL_APIC_OVERRIDE: - skip = sizeof(fwts_acpi_madt_local_apic_addr_override); + skip = madt_lapic_addr_override(fw, hdr, data); break; - case FWTS_ACPI_MADT_IO_SAPIC: { - fwts_acpi_madt_io_sapic *sapic = (fwts_acpi_madt_io_sapic*)data; - if (sapic->address == 0) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADIOSPAICAddrZero", - "MADT I/O SAPIC address is zero, appears not to be defined."); - } - skip = sizeof(fwts_acpi_madt_io_sapic); - } + + case FWTS_ACPI_MADT_IO_SAPIC: + skip = madt_io_sapic(fw, hdr, data); break; - case FWTS_ACPI_MADT_LOCAL_SAPIC: { - fwts_acpi_madt_local_sapic *local_sapic = (fwts_acpi_madt_local_sapic*)data; - skip = sizeof(fwts_acpi_madt_local_sapic) + strlen(local_sapic->uid_string) + 1; - } + + case FWTS_ACPI_MADT_LOCAL_SAPIC: + skip = madt_local_sapic(fw, hdr, data); break; - case FWTS_ACPI_MADT_INTERRUPT_SOURCE: { - fwts_acpi_madt_platform_int_source *src = (fwts_acpi_madt_platform_int_source*)data; - if (src->flags & 0xfffffff0) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTPlatIRQSrcFlags", - "MADT Platform Interrupt Source, flags, bits 4..31 are " - "reserved and should be zero, but are set as: %" PRIx32 ".", - src->flags); - } - if (src->type > 3) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTPlatIRQType", - "MADT Platform Interrupt Source, type field is %" PRIu8 - ", should be 1..3.", src->type); - } - if (src->io_sapic_vector == 0) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTPlatIRQIOSAPICVector", - "MADT Platform Interrupt Source, IO SAPIC Vector is " - "zero, appears not to be defined."); - } - if (src->pis_flags & 0xfffffffe) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTPlatIRQSrcFlagsNonZero", - "MADT Platform Interrupt Source, Platform Interrupt Source flag " - "bits 1..31 are reserved and should be zero, but are " - "set as: %" PRIx32 ".", src->pis_flags); - } - skip = (sizeof(fwts_acpi_madt_platform_int_source)); - } + + case FWTS_ACPI_MADT_INTERRUPT_SOURCE: + skip = madt_platform_int_source(fw, hdr, data); break; + case FWTS_ACPI_MADT_LOCAL_X2APIC: - skip = (sizeof(fwts_acpi_madt_local_x2apic)); + skip = madt_local_x2apic(fw, hdr, data); break; - case FWTS_ACPI_MADT_LOCAL_X2APIC_NMI: { - fwts_acpi_madt_local_x2apic_nmi *nmi = (fwts_acpi_madt_local_x2apic_nmi*)data; - if (nmi->flags & 0xfffffff0) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTLAPICX2APICNMIFlags", - "MADT Local x2APIC NMI, flags, bits 4..31 are reserved and " - "should be zero, but are set as: %" PRIx32 ".", - nmi->flags); - } - skip = (sizeof(fwts_acpi_madt_local_x2apic_nmi)); - } + + case FWTS_ACPI_MADT_LOCAL_X2APIC_NMI: + skip = madt_local_x2apic_nmi(fw, hdr, data); break; - case FWTS_ACPI_MADT_GIC_C_CPU_INTERFACE: { - fwts_acpi_madt_gic *gic = (fwts_acpi_madt_gic*)data;
- if (gic->reserved) { - passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "MADTGICCReservedNonZero", - "MADT GIC C Structure reserved field should be zero, " - "instead got 0x%" PRIx32 ".", gic->reserved); - } - if (gic->flags & 0xfffffff8) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTGICFLags", - "MADT GIC, flags, bits 2..31 are reserved " - "and should be zero, but are set as: %" PRIx32 ".", - gic->flags & 0xfffffff8); - } - skip = sizeof(fwts_acpi_madt_gic); - - // new in ACPI 6.0 - if (table->length == 80) { - uint32_t gic_reserve2 = gic->reserved2[0] + (gic->reserved2[1] << 4) + (gic->reserved2[2] << 8); - if (gic_reserve2) { - passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "MADTGICCReserved2NonZero", - "MADT GICC Structure second reserved field should be zero, " - "instead got 0x%" PRIx32 ".", gic_reserve2); - } - } - } + case FWTS_ACPI_MADT_GIC_C_CPU_INTERFACE: + skip = madt_gicc(fw, hdr, data); break; - case FWTS_ACPI_MADT_GIC_D_GOC_DISTRIBUTOR: { - fwts_acpi_madt_gicd *gicd = (fwts_acpi_madt_gicd*)data; - uint32_t gicd_reserve2 = gicd->reserved2[0] + (gicd->reserved2[1] << 4) + (gicd->reserved2[2] << 8);
- if (gicd->reserved) { - passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "MADTGICDReservedNonZero", - "MADT GIC Distributor Structure reserved field should be zero, " - "instead got 0x%" PRIx32 ".", gicd->reserved); - } - if (gicd_reserve2) { - passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "MADTGICDReserved2NonZero", - "MADT GIC Distributor Structure second reserved field should be zero, " - "instead got 0x%" PRIx32 ".", gicd_reserve2); - } - } - skip = sizeof(fwts_acpi_madt_gicd); + case FWTS_ACPI_MADT_GIC_D_GOC_DISTRIBUTOR: + skip = madt_gicd(fw, hdr, data); break; - case FWTS_ACPI_MADT_GIC_V2M_MSI_FRAME: { - fwts_acpi_madt_gic_msi *gic_msi = (fwts_acpi_madt_gic_msi*)data; - fwts_list_link *item;
- if (gic_msi->reserved) { - passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "MADTGICMSIReservedNonZero", - "MADT GIC MSI Structure reserved field should be zero, " - "instead got 0x%" PRIx32 ".", gic_msi->reserved); - } - if (gic_msi->flags & 0xfffffffe) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTGICMSIFLags", - "MADT GIC MSI Frame, flags, bits 1..31 are reserved " - "and should be zero, but are set as: %" PRIx32 ".", - gic_msi->flags); - } - - /* Check against previously IDs to see if unique */ - fwts_list_foreach(item, &msi_frame_ids) { - uint32_t *frame_id = fwts_list_data(uint32_t *, item); - - /* Frame ID must be unique according to the spec */ - if (*frame_id == gic_msi->frame_id) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "MADTGICMSINonUniqueFrameId", - "MADT GIC MSI Frame ID 0x%" PRIx32 " is not unique " - "and has already be defined in a previous GIC MSI " - "Frame.", gic_msi->frame_id); - } else { - fwts_list_append(&msi_frame_ids, &(gic_msi->frame_id)); - } - } - } - skip = sizeof(fwts_acpi_madt_gic_msi); + case FWTS_ACPI_MADT_GIC_V2M_MSI_FRAME: + skip = madt_gic_msi_frame(fw, hdr, data); break; - case FWTS_ACPI_MADT_GIC_R_REDISTRIBUTOR: { - fwts_acpi_madt_gicr *gicr = (fwts_acpi_madt_gicr*)data;
- if (gicr->reserved) { - passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "MADTGICRReservedNonZero", - "MADT GIC Redistributor Structure reserved field should be zero, " - "instead got 0x%" PRIx32 ".", gicr->reserved); - } - } - skip = sizeof(fwts_acpi_madt_gicr); + case FWTS_ACPI_MADT_GIC_R_REDISTRIBUTOR: + skip = madt_gicr(fw, hdr, data); break; + default: - skip = 0; + if (hdr->type >= 0x10 && hdr->type <= 0x7f) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTSubReservedID", + "MADT subtable %d is using the " + "reserved value 0x%x for a type. " + "Subtable type values 0x10..0x7f " + "are reserved; 0x80..0xff can be " + "used by OEMs.", + ii, hdr->type); + skip = hdr->length; + break; + } + + if (hdr->length == 0) { + fwts_log_error(fw, "INTERNAL ERROR: " + "zero length subtable means something " + "is seriously broken. Subtable %d has " + "the problem.", ii); break; } data += skip; length -= skip; } - fwts_list_free_items(&msi_frame_ids, NULL); - - if (passed) - fwts_passed(fw, "No issues found in MADT table.");
return FWTS_OK; }
+static int madt_deinit(fwts_framework *fw) +{ + /* only one minor clean up needed */ + fwts_list_free_items(&msi_frame_ids, NULL); + + return (fw) ? FWTS_ERROR : FWTS_OK; +} + static fwts_framework_minor_test madt_tests[] = { - { madt_test1, "MADT Multiple APIC Description Table test." }, + { madt_checksum, "MADT checksum test." }, + { madt_revision, "MADT revision test." }, + { madt_arch_revision, "MADT architecture minimum revision test." }, + { madt_flags, "MADT flags field reserved bits test." }, + { madt_subtables, "MADT subtable tests." }, { NULL, NULL } };
static fwts_framework_ops madt_ops = { - .description = "MADT Multiple APIC Description Table test.", + .description = "MADT Multiple APIC Description Table (spec compliant).", .init = madt_init, + .deinit = madt_deinit, .minor_tests = madt_tests };
-FWTS_REGISTER("madt", &madt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI) +FWTS_REGISTER("madt", &madt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI | FWTS_FLAG_TEST_COMPLIANCE_ACPI)
The GIC ITS subtable was added to the spec in ACPI 6.0. However, there was no struct definition for the subtable so that it could be parsed if it existed. This patch just adds in the struct for later use.
Signed-off-by: Al Stone al.stone@linaro.org --- src/lib/include/fwts_acpi.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h index 0b4bad9..a8a8276 100644 --- a/src/lib/include/fwts_acpi.h +++ b/src/lib/include/fwts_acpi.h @@ -415,6 +415,7 @@ typedef enum { FWTS_ACPI_MADT_GIC_D_GOC_DISTRIBUTOR, FWTS_ACPI_MADT_GIC_V2M_MSI_FRAME, FWTS_ACPI_MADT_GIC_R_REDISTRIBUTOR, + FWTS_ACPI_MADT_GIC_ITS, FWTS_ACPI_MADT_RESERVED } fwts_acpi_madt_type;
@@ -557,6 +558,15 @@ typedef struct { uint32_t discovery_range_length; } __attribute__ ((packed)) fwts_acpi_madt_gicr;
+/* New in ACPI 6.0, GIC ITS structure, 5.2.12.18 */ +/* Type 15, FWTS_ACPI_MADT_GIC_ITS */ +typedef struct { + uint16_t reserved; + uint32_t its_id; + uint64_t physical_base_address; + uint32_t reserved2; +} __attribute__ ((packed)) fwts_acpi_madt_gic_its; + /* * ACPI TCPA (Trusted Computing Platform Alliance Capabilities Table) * http://www.kuro5hin.org/story/2002/10/27/16622/530,
Having previously added the proper structs for the GIC ITS subtable of the MADT, add in test to make sure the content is reasonably correct if one is being used.
Signed-off-by: Al Stone al.stone@linaro.org --- src/acpi/madt/madt.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 81 insertions(+), 2 deletions(-)
diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c index d66e0c2..2c06d2c 100644 --- a/src/acpi/madt/madt.c +++ b/src/acpi/madt/madt.c @@ -200,6 +200,7 @@ static fwts_acpi_table_info *mtable; static fwts_acpi_table_info *ftable;
static fwts_list msi_frame_ids; +static fwts_list its_ids;
static int madt_init(fwts_framework *fw) { @@ -261,8 +262,12 @@ static int madt_init(fwts_framework *fw) ms++; }
- /* initialize the MSI frame ID list should we need it later */ + /* + * Initialize the MSI frame ID and ITS ID lists should we need + * them later + */ fwts_list_init(&msi_frame_ids); + fwts_list_init(&its_ids);
return (spec_data) ? FWTS_OK : FWTS_ERROR; } @@ -1100,6 +1105,75 @@ static int madt_gicr(fwts_framework *fw, return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); }
+static int madt_gic_its(fwts_framework *fw, + fwts_acpi_madt_sub_table_header *hdr, + const uint8_t *data) +{ + /* specific checks for subtable type 0xf: GIC ITS */ + fwts_acpi_madt_gic_its *gic_its = (fwts_acpi_madt_gic_its *)data; + fwts_list_link *item; + bool found; + + if (gic_its->reserved) + fwts_failed(fw, LOG_LEVEL_LOW, + "SPECMADTGICITSReservedNonZero", + "MADT %s first reserved field should be zero, " + "instead got 0x%" PRIx32 ".", + madt_sub_names[hdr->type], gic_its->reserved); + else + fwts_passed(fw, + "MADT %s first reserved field is properly set " + "to zero.", + madt_sub_names[hdr->type]); + + /* + * Check ITS ID against previously found IDs to see if it + * is unique. According to the spec, they must be. + */ + found = false; + fwts_list_foreach(item, &its_ids) { + uint32_t *its_id = fwts_list_data(uint32_t *, item); + + if (*its_id == gic_its->its_id) + found = true; + } + if (found) { + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "SPECMADTGICITSNonUniqueId", + "MADT %s ITS ID 0x%" PRIx32 " is not unique " + "and has already be defined in a previous %s.", + madt_sub_names[hdr->type], + gic_its->its_id, + madt_sub_names[hdr->type]); + } else { + fwts_list_append(&its_ids, &(gic_its->its_id)); + fwts_passed(fw, + "MADT %s ITS ID 0x%" PRIx32 " is unique " + "as is required.", + madt_sub_names[hdr->type], + gic_its->its_id); + } + + /* + * TODO: can the physical base address be tested, or is zero + * allowed? + */ + + if (gic_its->reserved2) + fwts_failed(fw, LOG_LEVEL_LOW, + "SPECMADTGICITSReserved2NonZero", + "MADT %s second reserved field should be zero, " + "instead got 0x%" PRIx32 ".", + madt_sub_names[hdr->type], gic_its->reserved2); + else + fwts_passed(fw, + "MADT %s second reserved field is properly set " + "to zero.", + madt_sub_names[hdr->type]); + + return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); +} + static int madt_subtables(fwts_framework *fw) { fwts_acpi_table_madt *madt = (fwts_acpi_table_madt *)mtable->data; @@ -1248,6 +1322,10 @@ static int madt_subtables(fwts_framework *fw) skip = madt_gicr(fw, hdr, data); break;
+ case FWTS_ACPI_MADT_GIC_ITS: + skip = madt_gic_its(fw, hdr, data); + break; + default: if (hdr->type >= 0x10 && hdr->type <= 0x7f) fwts_failed(fw, LOG_LEVEL_MEDIUM, @@ -1278,8 +1356,9 @@ static int madt_subtables(fwts_framework *fw)
static int madt_deinit(fwts_framework *fw) { - /* only one minor clean up needed */ + /* only minor clean up needed */ fwts_list_free_items(&msi_frame_ids, NULL); + fwts_list_free_items(&its_ids, NULL);
return (fw) ? FWTS_ERROR : FWTS_OK; }
On 19/01/16 00:26, Al Stone wrote:
This patch series adds in specific ACPI compliance testing for the MADT and all of its various subtables (16, currently).
The first three patches add in the idea of host and target architectures -- host being the arch that FWTS is running on, and target the arch whose firmware is being tested. This is needed later in the MADT tests since what is proper changes based on the architecture the firmware supports.
The fourth patch adds the detailed tests for the MADT and all but one of the subtables currently defined in ACPI 6.0. The last two patches add in the relatively new GIC ITS subtable and compliance tests for it.
There are still multiple TODOs in the compliance checks; these will be added as clarification of the spec becomes available.
Changes for v3: -- Add in support for the --arch=<name> parameter to specify the arch for the target firmware (default is that host == target). -- Add in the fwts_architecture typedef plus some helper functions so that tests in the future can adapt their behavior as needed, and so that the MADT tests can set themselves up properly. -- Instead of creating a new sourc file src/acpi/compliance/madt.c, replace the existing src/acpi/madt/madt.c tests since we're providing a superset. -- Various minor style and syntax corrections (from Ian Colin King)
Changes for v2: -- Clean up the white space problems -- Fix errors found by checkpatch (minor syntax things) -- Fix one logic error: while MADT and FADT table revisions *should* be in sync, they seldom are, so report this as a test failure and continue to test as much as possible instead of aborting completely, in some of those cases.
Al Stone (6): Start defining FWTS architectures as variables Define some utility functions for using the fwts_architecture enum Add mechanism to tell FWTS what architecture is being tested ACPI: MADT: add in compliance tests for the MADT and subtables ACPI: Add in MADT subtable description for GIC ITS subtable ACPI: MADT: add in compliance checks for the GIC ITS subtable
src/acpi/madt/madt.c | 1551 +++++++++++++++++++++++++++++++------- src/lib/include/fwts.h | 1 + src/lib/include/fwts_acpi.h | 10 + src/lib/include/fwts_arch.h | 41 + src/lib/include/fwts_framework.h | 3 + src/lib/src/Makefile.am | 1 + src/lib/src/fwts_arch.c | 88 +++ src/lib/src/fwts_framework.c | 25 + 8 files changed, 1460 insertions(+), 260 deletions(-) create mode 100644 src/lib/include/fwts_arch.h create mode 100644 src/lib/src/fwts_arch.c
I'm going to bulk-ACK these 6 patches as they do improve the ACPI MADT checking considerably. The MADT is such a mess, so this set of tests do seem to handle all the current combos of specification changes. Just a few comments:
1. Can you send a follow-up patch to update the man page for the new --arch option. I'll fix up the fwts wiki accordingly.
2. The fwts regression tests need updating. If this patchset gets ACK'd by the other team members then I'll fix these up for you as it is a little arcane to do this.
I've tested this on x86 and arm64 with ACPI tables from x86 and the --arch x86 option and it looks sane to me. Passes CoverityScan builds so, +1 ACK'd from me.
Thanks Al,
Acked-by: Colin Ian King colin.king@canonical.com
On 01/19/2016 05:57 AM, Colin Ian King wrote:
On 19/01/16 00:26, Al Stone wrote:
This patch series adds in specific ACPI compliance testing for the MADT and all of its various subtables (16, currently).
The first three patches add in the idea of host and target architectures -- host being the arch that FWTS is running on, and target the arch whose firmware is being tested. This is needed later in the MADT tests since what is proper changes based on the architecture the firmware supports.
The fourth patch adds the detailed tests for the MADT and all but one of the subtables currently defined in ACPI 6.0. The last two patches add in the relatively new GIC ITS subtable and compliance tests for it.
There are still multiple TODOs in the compliance checks; these will be added as clarification of the spec becomes available.
Changes for v3: -- Add in support for the --arch=<name> parameter to specify the arch for the target firmware (default is that host == target). -- Add in the fwts_architecture typedef plus some helper functions so that tests in the future can adapt their behavior as needed, and so that the MADT tests can set themselves up properly. -- Instead of creating a new sourc file src/acpi/compliance/madt.c, replace the existing src/acpi/madt/madt.c tests since we're providing a superset. -- Various minor style and syntax corrections (from Ian Colin King)
Changes for v2: -- Clean up the white space problems -- Fix errors found by checkpatch (minor syntax things) -- Fix one logic error: while MADT and FADT table revisions *should* be in sync, they seldom are, so report this as a test failure and continue to test as much as possible instead of aborting completely, in some of those cases.
Al Stone (6): Start defining FWTS architectures as variables Define some utility functions for using the fwts_architecture enum Add mechanism to tell FWTS what architecture is being tested ACPI: MADT: add in compliance tests for the MADT and subtables ACPI: Add in MADT subtable description for GIC ITS subtable ACPI: MADT: add in compliance checks for the GIC ITS subtable
src/acpi/madt/madt.c | 1551 +++++++++++++++++++++++++++++++------- src/lib/include/fwts.h | 1 + src/lib/include/fwts_acpi.h | 10 + src/lib/include/fwts_arch.h | 41 + src/lib/include/fwts_framework.h | 3 + src/lib/src/Makefile.am | 1 + src/lib/src/fwts_arch.c | 88 +++ src/lib/src/fwts_framework.c | 25 + 8 files changed, 1460 insertions(+), 260 deletions(-) create mode 100644 src/lib/include/fwts_arch.h create mode 100644 src/lib/src/fwts_arch.c
I'm going to bulk-ACK these 6 patches as they do improve the ACPI MADT checking considerably. The MADT is such a mess, so this set of tests do seem to handle all the current combos of specification changes. Just a few comments:
- Can you send a follow-up patch to update the man page for the new
--arch option. I'll fix up the fwts wiki accordingly.
D'oh. Of course. I should have thought of that :(.
- The fwts regression tests need updating. If this patchset gets ACK'd
by the other team members then I'll fix these up for you as it is a little arcane to do this.
Ah, thanks. I'd be glad to follow along and learn, if I can be of any help. Is there a pointer to a place to start?
I've tested this on x86 and arm64 with ACPI tables from x86 and the --arch x86 option and it looks sane to me. Passes CoverityScan builds so, +1 ACK'd from me.
Right, and running on x86 with --arch=arm64 works well, conversely.
Thanks Al,
Acked-by: Colin Ian King colin.king@canonical.com
Thanks, Colin!
On 19/01/16 15:24, Al Stone wrote:
On 01/19/2016 05:57 AM, Colin Ian King wrote:
On 19/01/16 00:26, Al Stone wrote:
This patch series adds in specific ACPI compliance testing for the MADT and all of its various subtables (16, currently).
The first three patches add in the idea of host and target architectures -- host being the arch that FWTS is running on, and target the arch whose firmware is being tested. This is needed later in the MADT tests since what is proper changes based on the architecture the firmware supports.
The fourth patch adds the detailed tests for the MADT and all but one of the subtables currently defined in ACPI 6.0. The last two patches add in the relatively new GIC ITS subtable and compliance tests for it.
There are still multiple TODOs in the compliance checks; these will be added as clarification of the spec becomes available.
Changes for v3: -- Add in support for the --arch=<name> parameter to specify the arch for the target firmware (default is that host == target). -- Add in the fwts_architecture typedef plus some helper functions so that tests in the future can adapt their behavior as needed, and so that the MADT tests can set themselves up properly. -- Instead of creating a new sourc file src/acpi/compliance/madt.c, replace the existing src/acpi/madt/madt.c tests since we're providing a superset. -- Various minor style and syntax corrections (from Ian Colin King)
Changes for v2: -- Clean up the white space problems -- Fix errors found by checkpatch (minor syntax things) -- Fix one logic error: while MADT and FADT table revisions *should* be in sync, they seldom are, so report this as a test failure and continue to test as much as possible instead of aborting completely, in some of those cases.
Al Stone (6): Start defining FWTS architectures as variables Define some utility functions for using the fwts_architecture enum Add mechanism to tell FWTS what architecture is being tested ACPI: MADT: add in compliance tests for the MADT and subtables ACPI: Add in MADT subtable description for GIC ITS subtable ACPI: MADT: add in compliance checks for the GIC ITS subtable
src/acpi/madt/madt.c | 1551 +++++++++++++++++++++++++++++++------- src/lib/include/fwts.h | 1 + src/lib/include/fwts_acpi.h | 10 + src/lib/include/fwts_arch.h | 41 + src/lib/include/fwts_framework.h | 3 + src/lib/src/Makefile.am | 1 + src/lib/src/fwts_arch.c | 88 +++ src/lib/src/fwts_framework.c | 25 + 8 files changed, 1460 insertions(+), 260 deletions(-) create mode 100644 src/lib/include/fwts_arch.h create mode 100644 src/lib/src/fwts_arch.c
I'm going to bulk-ACK these 6 patches as they do improve the ACPI MADT checking considerably. The MADT is such a mess, so this set of tests do seem to handle all the current combos of specification changes. Just a few comments:
- Can you send a follow-up patch to update the man page for the new
--arch option. I'll fix up the fwts wiki accordingly.
D'oh. Of course. I should have thought of that :(.
- The fwts regression tests need updating. If this patchset gets ACK'd
by the other team members then I'll fix these up for you as it is a little arcane to do this.
Ah, thanks. I'd be glad to follow along and learn, if I can be of any help. Is there a pointer to a place to start?
So you can see the failures by just running:
make check
and you see FAILS, e.g:
FAIL: fwts-test/arg-help-0001/test-0001.sh FAIL: fwts-test/arg-help-0001/test-0002.sh FAIL: fwts-test/arg-show-tests-0001/test-0001.sh FAIL: fwts-test/arg-show-tests-0001/test-0002.sh FAIL: fwts-test/arg-show-tests-full-0001/test-0001.sh FAIL: fwts-test/madt-0001/test-0001.sh FAIL: fwts-test/madt-0001/test-0002.sh
I use the following recipe: (let us suppose that fwts is in /home/king/repos/fwts)
cd /home/king/repos/fwts mkdir /tmp/fwts export export FWTS=/home/king/repos/fwts/src/fwts export TMP=/tmp/fwts export FAILURE_LOG=/tmp/fwts/failure.log export FWTSTESTDIR=/home/king/repos/fwts/fwts-test
And now to fix up the arg-help-0001 test
cd fwts-test/arg-help-0001
..and edit test-0001.sh and comment out the line near the end, so change:
rm $TMPLOG ${TMPLOG}.orig
to:
#rm $TMPLOG ${TMPLOG}.orig
..so that we don't remove these temp output files at the end of the test
Now run the test:
./test-0001.sh FAILED: Test -h option, test-0001.sh
the /tmp/fwts/failure.log will show you what is missing/different between the expected output from the test and what we get with your changes. If you look at the test you will see that the "new" output from fwts is dumped into /tmp/fwts/help.log.$$, so I just copy that over the original, e.g.:
cp /tmp/fwts/help.log.32394 arg-help-0001.log
and running the test again, it should pass:
./test-0001.sh PASSED: Test -h option, test-0001.sh
Finally, modify the test to uncomment the rm $TMPLOG ${TMPLOG}.orig line and git diff should show the change required to make test-0001.sh run OK with your MADT patches:
git diff diff --git a/fwts-test/arg-help-0001/arg-help-0001.log b/fwts-test/arg-help-0001/arg-help-0001.log index 88eee36..b6ad5e2 100644 --- a/fwts-test/arg-help-0001/arg-help-0001.log +++ b/fwts-test/arg-help-0001/arg-help-0001.log @@ -7,6 +7,10 @@ --acpitests Run general ACPI tests. -a, --all Run all tests. +--arch Specify arch of the + tables being tested + (defaults to current + host). -b, --batch Run non-Interactive tests. --batch-experimental Run Batch
Finally, remove the temp files in /tmp/fwts and then git add that arg-help-0001.log and then move on to the next test that failed the regression test.
This is painfully tedious. Hope that's enough info to work with.
Colin
I've tested this on x86 and arm64 with ACPI tables from x86 and the --arch x86 option and it looks sane to me. Passes CoverityScan builds so, +1 ACK'd from me.
Right, and running on x86 with --arch=arm64 works well, conversely.
Thanks Al,
Acked-by: Colin Ian King colin.king@canonical.com
Thanks, Colin!
On 01/19/2016 08:44 AM, Colin Ian King wrote:
On 19/01/16 15:24, Al Stone wrote:
On 01/19/2016 05:57 AM, Colin Ian King wrote:
On 19/01/16 00:26, Al Stone wrote:
This patch series adds in specific ACPI compliance testing for the MADT and all of its various subtables (16, currently).
The first three patches add in the idea of host and target architectures -- host being the arch that FWTS is running on, and target the arch whose firmware is being tested. This is needed later in the MADT tests since what is proper changes based on the architecture the firmware supports.
The fourth patch adds the detailed tests for the MADT and all but one of the subtables currently defined in ACPI 6.0. The last two patches add in the relatively new GIC ITS subtable and compliance tests for it.
There are still multiple TODOs in the compliance checks; these will be added as clarification of the spec becomes available.
Changes for v3: -- Add in support for the --arch=<name> parameter to specify the arch for the target firmware (default is that host == target). -- Add in the fwts_architecture typedef plus some helper functions so that tests in the future can adapt their behavior as needed, and so that the MADT tests can set themselves up properly. -- Instead of creating a new sourc file src/acpi/compliance/madt.c, replace the existing src/acpi/madt/madt.c tests since we're providing a superset. -- Various minor style and syntax corrections (from Ian Colin King)
Changes for v2: -- Clean up the white space problems -- Fix errors found by checkpatch (minor syntax things) -- Fix one logic error: while MADT and FADT table revisions *should* be in sync, they seldom are, so report this as a test failure and continue to test as much as possible instead of aborting completely, in some of those cases.
Al Stone (6): Start defining FWTS architectures as variables Define some utility functions for using the fwts_architecture enum Add mechanism to tell FWTS what architecture is being tested ACPI: MADT: add in compliance tests for the MADT and subtables ACPI: Add in MADT subtable description for GIC ITS subtable ACPI: MADT: add in compliance checks for the GIC ITS subtable
src/acpi/madt/madt.c | 1551 +++++++++++++++++++++++++++++++------- src/lib/include/fwts.h | 1 + src/lib/include/fwts_acpi.h | 10 + src/lib/include/fwts_arch.h | 41 + src/lib/include/fwts_framework.h | 3 + src/lib/src/Makefile.am | 1 + src/lib/src/fwts_arch.c | 88 +++ src/lib/src/fwts_framework.c | 25 + 8 files changed, 1460 insertions(+), 260 deletions(-) create mode 100644 src/lib/include/fwts_arch.h create mode 100644 src/lib/src/fwts_arch.c
I'm going to bulk-ACK these 6 patches as they do improve the ACPI MADT checking considerably. The MADT is such a mess, so this set of tests do seem to handle all the current combos of specification changes. Just a few comments:
- Can you send a follow-up patch to update the man page for the new
--arch option. I'll fix up the fwts wiki accordingly.
D'oh. Of course. I should have thought of that :(.
- The fwts regression tests need updating. If this patchset gets ACK'd
by the other team members then I'll fix these up for you as it is a little arcane to do this.
Ah, thanks. I'd be glad to follow along and learn, if I can be of any help. Is there a pointer to a place to start?
So you can see the failures by just running:
make check
and you see FAILS, e.g:
FAIL: fwts-test/arg-help-0001/test-0001.sh FAIL: fwts-test/arg-help-0001/test-0002.sh FAIL: fwts-test/arg-show-tests-0001/test-0001.sh FAIL: fwts-test/arg-show-tests-0001/test-0002.sh FAIL: fwts-test/arg-show-tests-full-0001/test-0001.sh FAIL: fwts-test/madt-0001/test-0001.sh FAIL: fwts-test/madt-0001/test-0002.sh
I use the following recipe: (let us suppose that fwts is in /home/king/repos/fwts)
cd /home/king/repos/fwts mkdir /tmp/fwts export export FWTS=/home/king/repos/fwts/src/fwts export TMP=/tmp/fwts export FAILURE_LOG=/tmp/fwts/failure.log export FWTSTESTDIR=/home/king/repos/fwts/fwts-test
And now to fix up the arg-help-0001 test
cd fwts-test/arg-help-0001
..and edit test-0001.sh and comment out the line near the end, so change:
rm $TMPLOG ${TMPLOG}.orig
to:
#rm $TMPLOG ${TMPLOG}.orig
..so that we don't remove these temp output files at the end of the test
Now run the test:
./test-0001.sh FAILED: Test -h option, test-0001.sh
the /tmp/fwts/failure.log will show you what is missing/different between the expected output from the test and what we get with your changes. If you look at the test you will see that the "new" output from fwts is dumped into /tmp/fwts/help.log.$$, so I just copy that over the original, e.g.:
cp /tmp/fwts/help.log.32394 arg-help-0001.log
and running the test again, it should pass:
./test-0001.sh PASSED: Test -h option, test-0001.sh
Finally, modify the test to uncomment the rm $TMPLOG ${TMPLOG}.orig line and git diff should show the change required to make test-0001.sh run OK with your MADT patches:
git diff diff --git a/fwts-test/arg-help-0001/arg-help-0001.log b/fwts-test/arg-help-0001/arg-help-0001.log index 88eee36..b6ad5e2 100644 --- a/fwts-test/arg-help-0001/arg-help-0001.log +++ b/fwts-test/arg-help-0001/arg-help-0001.log @@ -7,6 +7,10 @@ --acpitests Run general ACPI tests. -a, --all Run all tests. +--arch Specify arch of the
tables being tested
(defaults to current
host).
-b, --batch Run non-Interactive tests. --batch-experimental Run Batch
Finally, remove the temp files in /tmp/fwts and then git add that arg-help-0001.log and then move on to the next test that failed the regression test.
This is painfully tedious. Hope that's enough info to work with.
Colin
The recipe worked perfectly -- not too bad to do, but there weren't too many of them either. I'll send the patchset to the list in a few minutes.
Thanks!
I've tested this on x86 and arm64 with ACPI tables from x86 and the --arch x86 option and it looks sane to me. Passes CoverityScan builds so, +1 ACK'd from me.
Right, and running on x86 with --arch=arm64 works well, conversely.
Thanks Al,
Acked-by: Colin Ian King colin.king@canonical.com
Thanks, Colin!