Message ID | 20190725151125.145362-2-borntraeger@de.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | kvm-unit-tests: s390x: add check for mandatory | expand |
On 25.07.19 17:11, Christian Borntraeger wrote: > This adds a check for documented stfle dependencies. > Expected error under TCG: FAIL: cpumodel: dependency: 37 implies 42 DFP not implemented (yet). We also don't warn about this in check_consistency(), which is nice for TCG ;) > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > s390x/Makefile | 1 + > s390x/cpumodel.c | 58 +++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 3 +++ > 3 files changed, 62 insertions(+) > create mode 100644 s390x/cpumodel.c > > diff --git a/s390x/Makefile b/s390x/Makefile > index 1f21ddb9c943..574a9a20824d 100644 > --- a/s390x/Makefile > +++ b/s390x/Makefile > @@ -11,6 +11,7 @@ tests += $(TEST_DIR)/cmm.elf > tests += $(TEST_DIR)/vector.elf > tests += $(TEST_DIR)/gs.elf > tests += $(TEST_DIR)/iep.elf > +tests += $(TEST_DIR)/cpumodel.elf > tests_binary = $(patsubst %.elf,%.bin,$(tests)) > > all: directories test_cases test_cases_binary > diff --git a/s390x/cpumodel.c b/s390x/cpumodel.c > new file mode 100644 > index 000000000000..8ff61f7f6ec9 > --- /dev/null > +++ b/s390x/cpumodel.c > @@ -0,0 +1,58 @@ > +/* > + * Test the known dependencies for facilities > + * > + * Copyright 2019 IBM Corp. > + * > + * Authors: > + * Christian Borntraeger <borntraeger@de.ibm.com> > + * > + * This code is free software; you can redistribute it and/or modify it > + * under the terms of the GNU Library General Public License version 2. > + */ > + > +#include <asm/facility.h> > + > +static int dep[][2] = { > + /* from SA22-7832-11 4-98 facility indications */ > + { 4, 3}, > + { 5, 3}, > + { 5, 4}, > + { 19, 18}, > + { 37, 42}, > + { 43, 42}, > + { 73, 49}, > + {134, 129}, > + {139, 25}, > + {139, 28}, > + {146, 76}, > + /* indirectly documented in description */ > + { 78, 8}, /* EDAT */ > + /* new dependencies from gen15 */ > + { 61, 45}, > + {148, 129}, > + {148, 135}, > + {152, 129}, > + {152, 134}, > + {155, 76}, > + {155, 77}, > +}; > + > +int main(void) > +{ > + int i; > + > + report_prefix_push("cpumodel"); > + > + report_prefix_push("dependency"); > + for (i = 0; i < ARRAY_SIZE(dep); i++) { > + if (test_facility(dep[i][0])) { > + report("%d implies %d", > + !(test_facility(dep[i][0]) && !test_facility(dep[i][1])), > + dep[i][0], dep[i][1]); > + } else { > + report_skip("facility %d not present", dep[i][0]); > + } > + } > + report_prefix_pop(); Are you missing another pop here? > + return report_summary(); > +} > diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg > index 546b1f281f8f..db58bad5a038 100644 > --- a/s390x/unittests.cfg > +++ b/s390x/unittests.cfg > @@ -61,3 +61,6 @@ file = gs.elf > > [iep] > file = iep.elf > + > +[cpumodel] > +file = cpumodel.elf > Didn't verify the facilities. In general, looks good to me.
On 25.07.19 17:36, David Hildenbrand wrote: > On 25.07.19 17:11, Christian Borntraeger wrote: >> This adds a check for documented stfle dependencies. >> > > Expected error under TCG: > > FAIL: cpumodel: dependency: 37 implies 42 > > DFP not implemented (yet). > > We also don't warn about this in check_consistency(), which is nice for > TCG ;) So should I force this to KVM? Or should I try to detect TCG and make this xfail? > >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> --- >> s390x/Makefile | 1 + >> s390x/cpumodel.c | 58 +++++++++++++++++++++++++++++++++++++++++++++ >> s390x/unittests.cfg | 3 +++ >> 3 files changed, 62 insertions(+) >> create mode 100644 s390x/cpumodel.c >> >> diff --git a/s390x/Makefile b/s390x/Makefile >> index 1f21ddb9c943..574a9a20824d 100644 >> --- a/s390x/Makefile >> +++ b/s390x/Makefile >> @@ -11,6 +11,7 @@ tests += $(TEST_DIR)/cmm.elf >> tests += $(TEST_DIR)/vector.elf >> tests += $(TEST_DIR)/gs.elf >> tests += $(TEST_DIR)/iep.elf >> +tests += $(TEST_DIR)/cpumodel.elf >> tests_binary = $(patsubst %.elf,%.bin,$(tests)) >> >> all: directories test_cases test_cases_binary >> diff --git a/s390x/cpumodel.c b/s390x/cpumodel.c >> new file mode 100644 >> index 000000000000..8ff61f7f6ec9 >> --- /dev/null >> +++ b/s390x/cpumodel.c >> @@ -0,0 +1,58 @@ >> +/* >> + * Test the known dependencies for facilities >> + * >> + * Copyright 2019 IBM Corp. >> + * >> + * Authors: >> + * Christian Borntraeger <borntraeger@de.ibm.com> >> + * >> + * This code is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU Library General Public License version 2. >> + */ >> + >> +#include <asm/facility.h> >> + >> +static int dep[][2] = { >> + /* from SA22-7832-11 4-98 facility indications */ >> + { 4, 3}, >> + { 5, 3}, >> + { 5, 4}, >> + { 19, 18}, >> + { 37, 42}, >> + { 43, 42}, >> + { 73, 49}, >> + {134, 129}, >> + {139, 25}, >> + {139, 28}, >> + {146, 76}, >> + /* indirectly documented in description */ >> + { 78, 8}, /* EDAT */ >> + /* new dependencies from gen15 */ >> + { 61, 45}, >> + {148, 129}, >> + {148, 135}, >> + {152, 129}, >> + {152, 134}, >> + {155, 76}, >> + {155, 77}, >> +}; >> + >> +int main(void) >> +{ >> + int i; >> + >> + report_prefix_push("cpumodel"); >> + >> + report_prefix_push("dependency"); >> + for (i = 0; i < ARRAY_SIZE(dep); i++) { >> + if (test_facility(dep[i][0])) { >> + report("%d implies %d", >> + !(test_facility(dep[i][0]) && !test_facility(dep[i][1])), >> + dep[i][0], dep[i][1]); >> + } else { >> + report_skip("facility %d not present", dep[i][0]); >> + } >> + } >> + report_prefix_pop(); > > Are you missing another pop here? Yes it seems. > >> + return report_summary(); >> +} >> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg >> index 546b1f281f8f..db58bad5a038 100644 >> --- a/s390x/unittests.cfg >> +++ b/s390x/unittests.cfg >> @@ -61,3 +61,6 @@ file = gs.elf >> >> [iep] >> file = iep.elf >> + >> +[cpumodel] >> +file = cpumodel.elf >> > > Didn't verify the facilities. In general, looks good to me. >
On 25/07/2019 17.11, Christian Borntraeger wrote: > This adds a check for documented stfle dependencies. > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > s390x/Makefile | 1 + > s390x/cpumodel.c | 58 +++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 3 +++ > 3 files changed, 62 insertions(+) > create mode 100644 s390x/cpumodel.c > > diff --git a/s390x/Makefile b/s390x/Makefile > index 1f21ddb9c943..574a9a20824d 100644 > --- a/s390x/Makefile > +++ b/s390x/Makefile > @@ -11,6 +11,7 @@ tests += $(TEST_DIR)/cmm.elf > tests += $(TEST_DIR)/vector.elf > tests += $(TEST_DIR)/gs.elf > tests += $(TEST_DIR)/iep.elf > +tests += $(TEST_DIR)/cpumodel.elf > tests_binary = $(patsubst %.elf,%.bin,$(tests)) > > all: directories test_cases test_cases_binary > diff --git a/s390x/cpumodel.c b/s390x/cpumodel.c > new file mode 100644 > index 000000000000..8ff61f7f6ec9 > --- /dev/null > +++ b/s390x/cpumodel.c > @@ -0,0 +1,58 @@ > +/* > + * Test the known dependencies for facilities > + * > + * Copyright 2019 IBM Corp. > + * > + * Authors: > + * Christian Borntraeger <borntraeger@de.ibm.com> > + * > + * This code is free software; you can redistribute it and/or modify it > + * under the terms of the GNU Library General Public License version 2. > + */ > + > +#include <asm/facility.h> > + > +static int dep[][2] = { > + /* from SA22-7832-11 4-98 facility indications */ > + { 4, 3}, > + { 5, 3}, > + { 5, 4}, > + { 19, 18}, > + { 37, 42}, > + { 43, 42}, > + { 73, 49}, > + {134, 129}, You've missed {135, 129} here. Also maybe add a space between the number and the curly braces? > + {139, 25}, > + {139, 28}, > + {146, 76}, > + /* indirectly documented in description */ > + { 78, 8}, /* EDAT */ > + /* new dependencies from gen15 */ > + { 61, 45}, > + {148, 129}, > + {148, 135}, > + {152, 129}, > + {152, 134}, > + {155, 76}, > + {155, 77}, > +}; > + > +int main(void) > +{ > + int i; > + > + report_prefix_push("cpumodel"); > + > + report_prefix_push("dependency"); > + for (i = 0; i < ARRAY_SIZE(dep); i++) { > + if (test_facility(dep[i][0])) { > + report("%d implies %d", > + !(test_facility(dep[i][0]) && !test_facility(dep[i][1])), I think you can simplify that line to: test_facility(dep[i][1]) since you've checked dep[i][0] already in the if-statement. > + dep[i][0], dep[i][1]); > + } else { > + report_skip("facility %d not present", dep[i][0]); > + } > + } > + report_prefix_pop(); > + return report_summary(); > +} > diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg > index 546b1f281f8f..db58bad5a038 100644 > --- a/s390x/unittests.cfg > +++ b/s390x/unittests.cfg > @@ -61,3 +61,6 @@ file = gs.elf > > [iep] > file = iep.elf > + > +[cpumodel] > +file = cpumodel.elf > Apart from the minor issues, this looks good to me. Thomas
On 25.07.19 17:48, Christian Borntraeger wrote: > > > On 25.07.19 17:36, David Hildenbrand wrote: >> On 25.07.19 17:11, Christian Borntraeger wrote: >>> This adds a check for documented stfle dependencies. >>> >> >> Expected error under TCG: >> >> FAIL: cpumodel: dependency: 37 implies 42 >> >> DFP not implemented (yet). >> >> We also don't warn about this in check_consistency(), which is nice for >> TCG ;) > > So should I force this to KVM? Or should I try to detect TCG and make this > xfail? Hmm, good question. If it's broken, it's broken - maybe it's just the right thing to do here - let the test fail for TCG. Might take quite some time to fix, though :)
diff --git a/s390x/Makefile b/s390x/Makefile index 1f21ddb9c943..574a9a20824d 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -11,6 +11,7 @@ tests += $(TEST_DIR)/cmm.elf tests += $(TEST_DIR)/vector.elf tests += $(TEST_DIR)/gs.elf tests += $(TEST_DIR)/iep.elf +tests += $(TEST_DIR)/cpumodel.elf tests_binary = $(patsubst %.elf,%.bin,$(tests)) all: directories test_cases test_cases_binary diff --git a/s390x/cpumodel.c b/s390x/cpumodel.c new file mode 100644 index 000000000000..8ff61f7f6ec9 --- /dev/null +++ b/s390x/cpumodel.c @@ -0,0 +1,58 @@ +/* + * Test the known dependencies for facilities + * + * Copyright 2019 IBM Corp. + * + * Authors: + * Christian Borntraeger <borntraeger@de.ibm.com> + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU Library General Public License version 2. + */ + +#include <asm/facility.h> + +static int dep[][2] = { + /* from SA22-7832-11 4-98 facility indications */ + { 4, 3}, + { 5, 3}, + { 5, 4}, + { 19, 18}, + { 37, 42}, + { 43, 42}, + { 73, 49}, + {134, 129}, + {139, 25}, + {139, 28}, + {146, 76}, + /* indirectly documented in description */ + { 78, 8}, /* EDAT */ + /* new dependencies from gen15 */ + { 61, 45}, + {148, 129}, + {148, 135}, + {152, 129}, + {152, 134}, + {155, 76}, + {155, 77}, +}; + +int main(void) +{ + int i; + + report_prefix_push("cpumodel"); + + report_prefix_push("dependency"); + for (i = 0; i < ARRAY_SIZE(dep); i++) { + if (test_facility(dep[i][0])) { + report("%d implies %d", + !(test_facility(dep[i][0]) && !test_facility(dep[i][1])), + dep[i][0], dep[i][1]); + } else { + report_skip("facility %d not present", dep[i][0]); + } + } + report_prefix_pop(); + return report_summary(); +} diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg index 546b1f281f8f..db58bad5a038 100644 --- a/s390x/unittests.cfg +++ b/s390x/unittests.cfg @@ -61,3 +61,6 @@ file = gs.elf [iep] file = iep.elf + +[cpumodel] +file = cpumodel.elf
This adds a check for documented stfle dependencies. Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- s390x/Makefile | 1 + s390x/cpumodel.c | 58 +++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg | 3 +++ 3 files changed, 62 insertions(+) create mode 100644 s390x/cpumodel.c