diff mbox series

[kvm-unit-tests,v3,4/5] s390x: STSI tests

Message ID 20190826163502.1298-5-frankja@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: More emulation tests | expand

Commit Message

Janosch Frank Aug. 26, 2019, 4:35 p.m. UTC 
For now let's concentrate on the error conditions.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/Makefile      |  1 +
 s390x/stsi.c        | 84 +++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |  3 ++
 3 files changed, 88 insertions(+)
 create mode 100644 s390x/stsi.c

Comments

David Hildenbrand Aug. 30, 2019, 12:07 p.m. UTC | #1 
On 26.08.19 18:35, Janosch Frank wrote:
> For now let's concentrate on the error conditions.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  s390x/Makefile      |  1 +
>  s390x/stsi.c        | 84 +++++++++++++++++++++++++++++++++++++++++++++
>  s390x/unittests.cfg |  3 ++
>  3 files changed, 88 insertions(+)
>  create mode 100644 s390x/stsi.c
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 3453373..76db0bb 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -13,6 +13,7 @@ tests += $(TEST_DIR)/gs.elf
>  tests += $(TEST_DIR)/iep.elf
>  tests += $(TEST_DIR)/cpumodel.elf
>  tests += $(TEST_DIR)/diag288.elf
> +tests += $(TEST_DIR)/stsi.elf
>  tests_binary = $(patsubst %.elf,%.bin,$(tests))
>  
>  all: directories test_cases test_cases_binary
> diff --git a/s390x/stsi.c b/s390x/stsi.c
> new file mode 100644
> index 0000000..b8195b2
> --- /dev/null
> +++ b/s390x/stsi.c
> @@ -0,0 +1,84 @@
> +/*
> + * Store System Information tests
> + *
> + * Copyright (c) 2019 IBM Corp
> + *
> + * Authors:
> + *  Janosch Frank <frankja@linux.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 <libcflat.h>
> +#include <asm/page.h>
> +#include <asm/asm-offsets.h>
> +#include <asm/interrupt.h>
> +
> +static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
> +
> +static void test_specs(void)
> +{
> +	report_prefix_push("specification");
> +
> +	report_prefix_push("inv r0");
> +	expect_pgm_int();
> +	stsi(pagebuf, 0, 1 << 8, 0);
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +
> +	report_prefix_push("inv r1");
> +	expect_pgm_int();
> +	stsi(pagebuf, 1, 0, 1 << 16);
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +
> +	report_prefix_push("unaligned");
> +	expect_pgm_int();
> +	stsi(pagebuf + 42, 1, 0, 0);
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +
> +	report_prefix_pop();
> +}
> +
> +static void test_priv(void)
> +{
> +	report_prefix_push("privileged");
> +	expect_pgm_int();
> +	enter_pstate();
> +	stsi(pagebuf, 0, 0, 0);
> +	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
> +	report_prefix_pop();
> +}
> +
> +static inline unsigned long stsi_get_fc(void *addr)
> +{
> +	register unsigned long r0 asm("0") = 0;
> +	register unsigned long r1 asm("1") = 0;
> +	int cc;
> +
> +	asm volatile("stsi	0(%3)\n"
> +		     "ipm	%[cc]\n"
> +		     "srl	%[cc],28\n"
> +		     : "+d" (r0), [cc] "=d" (cc)
> +		     : "d" (r1), "a" (addr)

maybe [addr], so you can avoid the %3 above

> +		     : "cc", "memory");
> +	assert(!cc);
> +	return r0 >> 28;

I think I'd prefer "get_configuration_level()" and move it to an header
- because the fc actually allows more values (0, 15 ...) - however the
level can be used as an fc.


> +}
> +
> +static void test_fc(void)
> +{
> +	report("invalid fc",  stsi(pagebuf, 7, 0, 0) == 3);
> +	report("query fc >= 2",  stsi_get_fc(pagebuf) >= 2);
> +}
> +
> +int main(void)
> +{
> +	report_prefix_push("stsi");
> +	test_priv();
> +	test_specs();
> +	test_fc();
> +	return report_summary();
> +}
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index 9dd288a..cc79a4e 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -68,3 +68,6 @@ file = cpumodel.elf
>  [diag288]
>  file = diag288.elf
>  extra_params=-device diag288,id=watchdog0 --watchdog-action inject-nmi
> +
> +[stsi]
> +file = stsi.elf
> 

Apart from that

Reviewed-by: David Hildenbrand <david@redhat.com>
Janosch Frank Sept. 3, 2019, 10:53 a.m. UTC | #2 
On 8/30/19 2:07 PM, David Hildenbrand wrote:
> On 26.08.19 18:35, Janosch Frank wrote:
>> For now let's concentrate on the error conditions.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
[...]
>> +static inline unsigned long stsi_get_fc(void *addr)
>> +{
>> +	register unsigned long r0 asm("0") = 0;
>> +	register unsigned long r1 asm("1") = 0;
>> +	int cc;
>> +
>> +	asm volatile("stsi	0(%3)\n"
>> +		     "ipm	%[cc]\n"
>> +		     "srl	%[cc],28\n"
>> +		     : "+d" (r0), [cc] "=d" (cc)
>> +		     : "d" (r1), "a" (addr)
> 
> maybe [addr], so you can avoid the %3 above

Sure, maybe Thomas can also fix that on picking for the previous patch?

> 
>> +		     : "cc", "memory");
>> +	assert(!cc);
>> +	return r0 >> 28;
> 
> I think I'd prefer "get_configuration_level()" and move it to an header
> - because the fc actually allows more values (0, 15 ...) - however the
> level can be used as an fc.

The rename works for me, but that's currently used only once, so why
should it go to a header file?

I though about starting lib/s390x/asm/misc-instr.h if we have enough (>=
2) instruction definitions which are shared.

> 
> 
>> +}
>> +
>> +static void test_fc(void)
>> +{
>> +	report("invalid fc",  stsi(pagebuf, 7, 0, 0) == 3);
>> +	report("query fc >= 2",  stsi_get_fc(pagebuf) >= 2);
>> +}
>> +
>> +int main(void)
>> +{
>> +	report_prefix_push("stsi");
>> +	test_priv();
>> +	test_specs();
>> +	test_fc();
>> +	return report_summary();
>> +}
>> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
>> index 9dd288a..cc79a4e 100644
>> --- a/s390x/unittests.cfg
>> +++ b/s390x/unittests.cfg
>> @@ -68,3 +68,6 @@ file = cpumodel.elf
>>  [diag288]
>>  file = diag288.elf
>>  extra_params=-device diag288,id=watchdog0 --watchdog-action inject-nmi
>> +
>> +[stsi]
>> +file = stsi.elf
>>
> 
> Apart from that
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
>
David Hildenbrand Sept. 3, 2019, 10:57 a.m. UTC | #3 
On 03.09.19 12:53, Janosch Frank wrote:
> On 8/30/19 2:07 PM, David Hildenbrand wrote:
>> On 26.08.19 18:35, Janosch Frank wrote:
>>> For now let's concentrate on the error conditions.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
> [...]
>>> +static inline unsigned long stsi_get_fc(void *addr)
>>> +{
>>> +	register unsigned long r0 asm("0") = 0;
>>> +	register unsigned long r1 asm("1") = 0;
>>> +	int cc;
>>> +
>>> +	asm volatile("stsi	0(%3)\n"
>>> +		     "ipm	%[cc]\n"
>>> +		     "srl	%[cc],28\n"
>>> +		     : "+d" (r0), [cc] "=d" (cc)
>>> +		     : "d" (r1), "a" (addr)
>>
>> maybe [addr], so you can avoid the %3 above
> 
> Sure, maybe Thomas can also fix that on picking for the previous patch?
> 
>>
>>> +		     : "cc", "memory");
>>> +	assert(!cc);
>>> +	return r0 >> 28;
>>
>> I think I'd prefer "get_configuration_level()" and move it to an header
>> - because the fc actually allows more values (0, 15 ...) - however the
>> level can be used as an fc.
> 
> The rename works for me, but that's currently used only once, so why
> should it go to a header file?

No strong opinion about that, I would have moved it out of the test to
make the test itself more compact .

> 
> I though about starting lib/s390x/asm/misc-instr.h if we have enough (>=
> 2) instruction definitions which are shared.
Thomas Huth Sept. 3, 2019, 10:58 a.m. UTC | #4 
On 03/09/2019 12.53, Janosch Frank wrote:
> On 8/30/19 2:07 PM, David Hildenbrand wrote:
>> On 26.08.19 18:35, Janosch Frank wrote:
>>> For now let's concentrate on the error conditions.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
> [...]
>>> +static inline unsigned long stsi_get_fc(void *addr)
>>> +{
>>> +	register unsigned long r0 asm("0") = 0;
>>> +	register unsigned long r1 asm("1") = 0;
>>> +	int cc;
>>> +
>>> +	asm volatile("stsi	0(%3)\n"
>>> +		     "ipm	%[cc]\n"
>>> +		     "srl	%[cc],28\n"
>>> +		     : "+d" (r0), [cc] "=d" (cc)
>>> +		     : "d" (r1), "a" (addr)
>>
>> maybe [addr], so you can avoid the %3 above
> 
> Sure, maybe Thomas can also fix that on picking for the previous patch?

Yes, I can do that.

>>
>>> +		     : "cc", "memory");
>>> +	assert(!cc);
>>> +	return r0 >> 28;
>>
>> I think I'd prefer "get_configuration_level()" and move it to an header
>> - because the fc actually allows more values (0, 15 ...) - however the
>> level can be used as an fc.
> 
> The rename works for me, but that's currently used only once, so why
> should it go to a header file?
> 
> I though about starting lib/s390x/asm/misc-instr.h if we have enough (>=
> 2) instruction definitions which are shared.

Let's keep it here until we need it in another file, too - then we can
still move it to a header instead.

 Thomas
diff mbox series

Patch

diff --git a/s390x/Makefile b/s390x/Makefile
index 3453373..76db0bb 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -13,6 +13,7 @@  tests += $(TEST_DIR)/gs.elf
 tests += $(TEST_DIR)/iep.elf
 tests += $(TEST_DIR)/cpumodel.elf
 tests += $(TEST_DIR)/diag288.elf
+tests += $(TEST_DIR)/stsi.elf
 tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
 all: directories test_cases test_cases_binary
diff --git a/s390x/stsi.c b/s390x/stsi.c
new file mode 100644
index 0000000..b8195b2
--- /dev/null
+++ b/s390x/stsi.c
@@ -0,0 +1,84 @@ 
+/*
+ * Store System Information tests
+ *
+ * Copyright (c) 2019 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <frankja@linux.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 <libcflat.h>
+#include <asm/page.h>
+#include <asm/asm-offsets.h>
+#include <asm/interrupt.h>
+
+static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
+
+static void test_specs(void)
+{
+	report_prefix_push("specification");
+
+	report_prefix_push("inv r0");
+	expect_pgm_int();
+	stsi(pagebuf, 0, 1 << 8, 0);
+	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
+	report_prefix_pop();
+
+	report_prefix_push("inv r1");
+	expect_pgm_int();
+	stsi(pagebuf, 1, 0, 1 << 16);
+	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
+	report_prefix_pop();
+
+	report_prefix_push("unaligned");
+	expect_pgm_int();
+	stsi(pagebuf + 42, 1, 0, 0);
+	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
+	report_prefix_pop();
+
+	report_prefix_pop();
+}
+
+static void test_priv(void)
+{
+	report_prefix_push("privileged");
+	expect_pgm_int();
+	enter_pstate();
+	stsi(pagebuf, 0, 0, 0);
+	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+	report_prefix_pop();
+}
+
+static inline unsigned long stsi_get_fc(void *addr)
+{
+	register unsigned long r0 asm("0") = 0;
+	register unsigned long r1 asm("1") = 0;
+	int cc;
+
+	asm volatile("stsi	0(%3)\n"
+		     "ipm	%[cc]\n"
+		     "srl	%[cc],28\n"
+		     : "+d" (r0), [cc] "=d" (cc)
+		     : "d" (r1), "a" (addr)
+		     : "cc", "memory");
+	assert(!cc);
+	return r0 >> 28;
+}
+
+static void test_fc(void)
+{
+	report("invalid fc",  stsi(pagebuf, 7, 0, 0) == 3);
+	report("query fc >= 2",  stsi_get_fc(pagebuf) >= 2);
+}
+
+int main(void)
+{
+	report_prefix_push("stsi");
+	test_priv();
+	test_specs();
+	test_fc();
+	return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 9dd288a..cc79a4e 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -68,3 +68,6 @@  file = cpumodel.elf
 [diag288]
 file = diag288.elf
 extra_params=-device diag288,id=watchdog0 --watchdog-action inject-nmi
+
+[stsi]
+file = stsi.elf