diff mbox series

[v2,1/1] kvm-unit-tests: s390: add cpu model checks

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

Commit Message

Christian Borntraeger July 25, 2019, 3:11 p.m. UTC 
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

Comments

David Hildenbrand July 25, 2019, 3:36 p.m. UTC | #1 
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.
Christian Borntraeger July 25, 2019, 3:48 p.m. UTC | #2 
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.
>
Thomas Huth July 25, 2019, 3:52 p.m. UTC | #3 
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
David Hildenbrand July 25, 2019, 6:40 p.m. UTC | #4 
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 mbox series

Patch

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