ACK: [PATCH v3 0/6] ACPI compliance testing for MADT and its subtables

[Al Stone]

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:
> 
> 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.

D'oh.  Of course.  I should have thought of that :(.

> 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.

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 at canonical.com>
> 

Thanks, Colin!

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone at linaro.org
-----------------------------------