Message ID | 569E59DB.9080902@canonical.com |
---|---|
State | Rejected |
Headers | show |
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: >>> >>> 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? > > 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! >> >
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