Skip to contentSkip to conversation

PV tests part 1

PATCH Superseded Janosch Frank 4 participants 23 messages Jul 17, 2020 – Jul 23, 2020

Subsystems (2)
KVM s390

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Janosch Frank

author
#0 lore raw

cover PV tests part 1

Let's start bringing in some more PV related code.

Somehow I missed that we can also have a key in a exception new PSW. The interesting bit when skrf is active is that if such a PSW is loaded on an exception it will result in a specification exception and not a special operation exception like on all other key related actions.

The third patch adds a basic guest UV call API test. It has mostly been used for firmware testing but I also think it's good to have a building block like this for more PV tests.

GIT: https://github.com/frankjaa/kvm-unit-tests/tree/queue

Janosch Frank (3):
s390x: Add custom pgm cleanup function
s390x: skrf: Add exception new skey test and add test to unittests.cfg
s390x: Ultavisor guest API test

 lib/s390x/asm/interrupt.h |   1 +
 lib/s390x/asm/uv.h        |  68 +++++++++++++++++
 lib/s390x/interrupt.c     |   9 +++
 s390x/Makefile            |   1 +
 s390x/skrf.c              |  81 ++++++++++++++++++++
 s390x/unittests.cfg       |   7 ++
 s390x/uv-guest.c          | 156 ++++++++++++++++++++++++++++++++++++++
 7 files changed, 323 insertions(+)
 create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c
Signature · 1 line
2.25.1
author
#1 lore raw

PATCH 1/3 s390x: Add custom pgm cleanup function

Sometimes we need to do cleanup which we don't necessarily want to add to interrupt.c, so lets add a way to register a cleanup function.

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm/interrupt.h | 1 +
 lib/s390x/interrupt.c     | 9 +++++++++
 2 files changed, 10 insertions(+)
lib/s390x/asm/interrupt.h +1 -0
index 4cfade9..b2a7c83 100644
@@ -15,6 +15,7 @@
 #define EXT_IRQ_EXTERNAL_CALL	0x1202
 #define EXT_IRQ_SERVICE_SIG	0x2401
 
+void register_pgm_int_func(void (*f)(void));
 void handle_pgm_int(void);
 void handle_ext_int(void);
 void handle_mcck_int(void);
lib/s390x/interrupt.c +9 -0
index 243b9c2..36ba720 100644
@@ -16,6 +16,7 @@
 
 static bool pgm_int_expected;
 static bool ext_int_expected;
+static void (*pgm_int_func)(void);
 static struct lowcore *lc;
 
 void expect_pgm_int(void)

@@ -51,8 +52,16 @@ void check_pgm_int_code(uint16_t code)
 	       lc->pgm_int_code);
 }
 
+void register_pgm_int_func(void (*f)(void))
+{
+	pgm_int_func = f;
+}
+
 static void fixup_pgm_int(void)
 {
+	if (pgm_int_func)
+		return (*pgm_int_func)();
+
 	switch (lc->pgm_int_code) {
 	case PGM_INT_CODE_PRIVILEGED_OPERATION:
 		/* Normal operation is in supervisor state, so this exception
Signature · 1 line
2.25.1
Reviewed-by on v1 1/3
#2 lore raw

Re: [kvm-unit-tests PATCH 1/3] s390x: Add custom pgm cleanup function

Quoted 50 lines · On Fri, 17 Jul 2020 10:58:11 -0400 Janosch Frank <[email protected]> wrote:

Sometimes we need to do cleanup which we don't necessarily want to add to interrupt.c, so lets add a way to register a cleanup function.

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm/interrupt.h | 1 +
 lib/s390x/interrupt.c     | 9 +++++++++
 2 files changed, 10 insertions(+)
lib/s390x/asm/interrupt.h +1 -0
index 4cfade9..b2a7c83 100644
@@ -15,6 +15,7 @@
 #define EXT_IRQ_EXTERNAL_CALL	0x1202
 #define EXT_IRQ_SERVICE_SIG	0x2401
 
+void register_pgm_int_func(void (*f)(void));
 void handle_pgm_int(void);
 void handle_ext_int(void);
 void handle_mcck_int(void);
lib/s390x/interrupt.c +9 -0
index 243b9c2..36ba720 100644
@@ -16,6 +16,7 @@
 
 static bool pgm_int_expected;
 static bool ext_int_expected;
+static void (*pgm_int_func)(void);
 static struct lowcore *lc;
 
 void expect_pgm_int(void)

@@ -51,8 +52,16 @@ void check_pgm_int_code(uint16_t code)
 	       lc->pgm_int_code);
 }
 
+void register_pgm_int_func(void (*f)(void))
+{
+	pgm_int_func = f;
+}
+
 static void fixup_pgm_int(void)
 {
+	if (pgm_int_func)
+		return (*pgm_int_func)();
+
 	switch (lc->pgm_int_code) {
 	case PGM_INT_CODE_PRIVILEGED_OPERATION:
 		/* Normal operation is in supervisor state, so this
Quoted 1 line

exception

Reviewed-by Claudio Imbrenda <[email protected]>
Reviewed-by on v1 1/3
#3 lore raw

Re: [kvm-unit-tests PATCH 1/3] s390x: Add custom pgm cleanup function

In reply to Janosch Frank
Quoted 41 lines · Janosch Frank

Sometimes we need to do cleanup which we don't necessarily want to add to interrupt.c, so lets add a way to register a cleanup function.

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm/interrupt.h | 1 +
 lib/s390x/interrupt.c     | 9 +++++++++
 2 files changed, 10 insertions(+)
lib/s390x/asm/interrupt.h +1 -0
index 4cfade9..b2a7c83 100644
@@ -15,6 +15,7 @@
 #define EXT_IRQ_EXTERNAL_CALL	0x1202
 #define EXT_IRQ_SERVICE_SIG	0x2401
 
+void register_pgm_int_func(void (*f)(void));
 void handle_pgm_int(void);
 void handle_ext_int(void);
 void handle_mcck_int(void);
lib/s390x/interrupt.c +4 -0
index 243b9c2..36ba720 100644
@@ -16,6 +16,7 @@
 
 static bool pgm_int_expected;
 static bool ext_int_expected;
+static void (*pgm_int_func)(void);
 static struct lowcore *lc;
 
 void expect_pgm_int(void)

@@ -51,8 +52,16 @@ void check_pgm_int_code(uint16_t code)
 	       lc->pgm_int_code);
 }
 
+void register_pgm_int_func(void (*f)(void))
+{
+	pgm_int_func = f;

lib/s390x/interrupt.c void check_pgm_int_code(uint16_t code) · line 67

Quoted 10 lines
lib/s390x/interrupt.c +5 -0
+}
+
 static void fixup_pgm_int(void)
 {
+	if (pgm_int_func)
+		return (*pgm_int_func)();
+
 	switch (lc->pgm_int_code) {
 	case PGM_INT_CODE_PRIVILEGED_OPERATION:
 		/* Normal operation is in supervisor state, so this exception
Reviewed-by Thomas Huth <[email protected]>
#4 lore raw

Re: [kvm-unit-tests PATCH 1/3] s390x: Add custom pgm cleanup function

In reply to Janosch Frank
Quoted 2 lines · On Fri, 17 Jul 2020 10:58:11 -0400 Janosch Frank <[email protected]> wrote:

Sometimes we need to do cleanup which we don't necessarily want to add to interrupt.c, so lets add a way to register a cleanup function.

s/lets/let's/ :)

Quoted 35 lines
Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm/interrupt.h | 1 +
 lib/s390x/interrupt.c     | 9 +++++++++
 2 files changed, 10 insertions(+)
lib/s390x/asm/interrupt.h +1 -0
index 4cfade9..b2a7c83 100644
@@ -15,6 +15,7 @@
 #define EXT_IRQ_EXTERNAL_CALL	0x1202
 #define EXT_IRQ_SERVICE_SIG	0x2401
 
+void register_pgm_int_func(void (*f)(void));
 void handle_pgm_int(void);
 void handle_ext_int(void);
 void handle_mcck_int(void);
lib/s390x/interrupt.c +1 -0
index 243b9c2..36ba720 100644
@@ -16,6 +16,7 @@
 
 static bool pgm_int_expected;
 static bool ext_int_expected;
+static void (*pgm_int_func)(void);
 static struct lowcore *lc;
 
 void expect_pgm_int(void)

@@ -51,8 +52,16 @@ void check_pgm_int_code(uint16_t code)
 	       lc->pgm_int_code);
 }

lib/s390x/interrupt.c void check_pgm_int_code(uint16_t code) · line 64

Quoted 10 lines
lib/s390x/interrupt.c +8 -0
+void register_pgm_int_func(void (*f)(void))
+{
+	pgm_int_func = f;
+}
+
 static void fixup_pgm_int(void)
 {
+	if (pgm_int_func)
+		return (*pgm_int_func)();
+

Maybe rather call this function, if set, instead of fixup_pgm_int() in handle_pgm_int()? Feels a bit cleaner to me.

Quoted 3 lines
lib/s390x/interrupt.c +0 -0
 	switch (lc->pgm_int_code) {
 	case PGM_INT_CODE_PRIVILEGED_OPERATION:
 		/* Normal operation is in supervisor state, so this exception
author
#5 lore raw

Re: [kvm-unit-tests PATCH 1/3] s390x: Add custom pgm cleanup function

Quoted 43 lines · Cornelia Huck
Quoted 2 lines · On Fri, 17 Jul 2020 10:58:11 -0400 Janosch Frank <[email protected]> wrote:

Sometimes we need to do cleanup which we don't necessarily want to add to interrupt.c, so lets add a way to register a cleanup function.

s/lets/let's/ :)

Quoted 35 lines
Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm/interrupt.h | 1 +
 lib/s390x/interrupt.c     | 9 +++++++++
 2 files changed, 10 insertions(+)
lib/s390x/asm/interrupt.h +1 -0
index 4cfade9..b2a7c83 100644
@@ -15,6 +15,7 @@
 #define EXT_IRQ_EXTERNAL_CALL	0x1202
 #define EXT_IRQ_SERVICE_SIG	0x2401
 
+void register_pgm_int_func(void (*f)(void));
 void handle_pgm_int(void);
 void handle_ext_int(void);
 void handle_mcck_int(void);
lib/s390x/interrupt.c +1 -0
index 243b9c2..36ba720 100644
@@ -16,6 +16,7 @@
 
 static bool pgm_int_expected;
 static bool ext_int_expected;
+static void (*pgm_int_func)(void);
 static struct lowcore *lc;
 
 void expect_pgm_int(void)

@@ -51,8 +52,16 @@ void check_pgm_int_code(uint16_t code)
 	       lc->pgm_int_code);
 }
Quoted 13 lines

lib/s390x/interrupt.c void check_pgm_int_code(uint16_t code) · line 64

Quoted 10 lines
lib/s390x/interrupt.c +8 -0
+void register_pgm_int_func(void (*f)(void))
+{
+	pgm_int_func = f;
+}
+
 static void fixup_pgm_int(void)
 {
+	if (pgm_int_func)
+		return (*pgm_int_func)();
+

Maybe rather call this function, if set, instead of fixup_pgm_int() in handle_pgm_int()? Feels a bit cleaner to me.

Well it's currently a cleanup function so it should be in fixup_pgm_int() because it fixes up.

I don't need a handler here like Pierre with his IO changes.

So it might more sense to change the name of the function ptr and registration function:

register_pgm_cleanup_func()
static void (*pgm_cleanup_func)(void);

Quoted 5 lines
Quoted 4 lines
lib/s390x/interrupt.c +0 -0
 	switch (lc->pgm_int_code) {
 	case PGM_INT_CODE_PRIVILEGED_OPERATION:
 		/* Normal operation is in supervisor state, so this exception
#6 lore raw

Re: [kvm-unit-tests PATCH 1/3] s390x: Add custom pgm cleanup function

Quoted 61 lines · On Thu, 23 Jul 2020 14:23:48 +0200 Janosch Frank <[email protected]> wrote:
Quoted 43 lines · Cornelia Huck
Quoted 2 lines · On Fri, 17 Jul 2020 10:58:11 -0400 Janosch Frank <[email protected]> wrote:

Sometimes we need to do cleanup which we don't necessarily want to add to interrupt.c, so lets add a way to register a cleanup function.

s/lets/let's/ :)

Quoted 35 lines
Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm/interrupt.h | 1 +
 lib/s390x/interrupt.c     | 9 +++++++++
 2 files changed, 10 insertions(+)
lib/s390x/asm/interrupt.h +1 -0
index 4cfade9..b2a7c83 100644
@@ -15,6 +15,7 @@
 #define EXT_IRQ_EXTERNAL_CALL	0x1202
 #define EXT_IRQ_SERVICE_SIG	0x2401
 
+void register_pgm_int_func(void (*f)(void));
 void handle_pgm_int(void);
 void handle_ext_int(void);
 void handle_mcck_int(void);
lib/s390x/interrupt.c +1 -0
index 243b9c2..36ba720 100644
@@ -16,6 +16,7 @@
 
 static bool pgm_int_expected;
 static bool ext_int_expected;
+static void (*pgm_int_func)(void);
 static struct lowcore *lc;
 
 void expect_pgm_int(void)

@@ -51,8 +52,16 @@ void check_pgm_int_code(uint16_t code)
 	       lc->pgm_int_code);
 }
Quoted 13 lines

lib/s390x/interrupt.c void check_pgm_int_code(uint16_t code) · line 64

Quoted 10 lines
lib/s390x/interrupt.c +8 -0
+void register_pgm_int_func(void (*f)(void))
+{
+	pgm_int_func = f;
+}
+
 static void fixup_pgm_int(void)
 {
+	if (pgm_int_func)
+		return (*pgm_int_func)();
+

Maybe rather call this function, if set, instead of fixup_pgm_int() in handle_pgm_int()? Feels a bit cleaner to me.

Well it's currently a cleanup function so it should be in fixup_pgm_int() because it fixes up.

Quoted 7 lines

I don't need a handler here like Pierre with his IO changes.

So it might more sense to change the name of the function ptr and registration function:

register_pgm_cleanup_func()
static void (*pgm_cleanup_func)(void);

Sounds good.

But doesn't that cleanup func run instead of the 'normal' cleanup func? I think making that distinction in handle_pgm_int() is clearer.

Quoted 8 lines
Quoted 6 lines
Quoted 5 lines
lib/s390x/interrupt.c +0 -0
 	switch (lc->pgm_int_code) {
 	case PGM_INT_CODE_PRIVILEGED_OPERATION:
 		/* Normal operation is in supervisor state, so this exception
author
#7 lore raw

PATCH 2/3 s390x: skrf: Add exception new skey test and add test to unittests.cfg

In reply to Janosch Frank

If a exception new psw mask contains a key a specification exception instead of a special operation exception is presented. Let's test that.

Also let's add the test to unittests.cfg so it is run more often.

Signed-off-by Janosch Frank <[email protected]>
---
 s390x/skrf.c        | 81 +++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |  4 +++
 2 files changed, 85 insertions(+)
s390x/skrf.c +81 -0
index 9cae589..9733412 100644
@@ -15,6 +15,8 @@
 #include <asm/page.h>
 #include <asm/facility.h>
 #include <asm/mem.h>
+#include <asm/sigp.h>
+#include <smp.h>
 
 static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
 

@@ -106,6 +108,84 @@ static void test_tprot(void)
 	report_prefix_pop();
 }
 
+#include <asm-generic/barrier.h>
+static int testflag = 0;
+
+static void wait_for_flag(void)
+{
+	while (!testflag)
+		mb();
+}
+
+static void set_flag(int val)
+{
+	mb();
+	testflag = val;
+	mb();
+}
+
+static void ecall_cleanup(void)
+{
+	struct lowcore *lc = (void *)0x0;
+
+	lc->ext_new_psw.mask = 0x0000000180000000UL;
+	lc->sw_int_crs[0] = 0x0000000000040000;
+
+	/*
+	 * PGM old contains the ext new PSW, we need to clean it up,
+	 * so we don't get a special oepration exception on the lpswe
+	 * of pgm old.
+	 */
+	lc->pgm_old_psw.mask = 0x0000000180000000UL;
+	lc->pgm_old_psw.addr = (unsigned long)wait_for_flag;
+
+	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
+	set_flag(1);
+}
+
+/* Set a key into the external new psw mask and open external call masks */
+static void ecall_setup(void)
+{
+	struct lowcore *lc = (void *)0x0;
+	uint64_t mask;
+
+	register_pgm_int_func(ecall_cleanup);
+	expect_pgm_int();
+	/* Put a skey into the ext new psw */
+	lc->ext_new_psw.mask = 0x00F0000180000000UL;
+	/* Open up ext masks */
+	ctl_set_bit(0, 13);
+	mask = extract_psw_mask();
+	mask |= PSW_MASK_EXT;
+	load_psw_mask(mask);
+	/* Tell cpu 0 that we're ready */
+	set_flag(1);
+}
+
+static void test_exception_ext_new(void)
+{
+	struct psw psw = {
+		.mask = extract_psw_mask(),
+		.addr = (unsigned long)ecall_setup
+	};
+
+	report_prefix_push("exception external new");
+	if (smp_query_num_cpus() < 2) {
+		report_skip("Need second cpu for exception external new test.");
+		report_prefix_pop();
+		return;
+	}
+
+	smp_cpu_setup(1, psw);
+	wait_for_flag();
+	set_flag(0);
+
+	sigp(1, SIGP_EXTERNAL_CALL, 0, NULL);
+	wait_for_flag();
+	smp_cpu_stop(1);
+	report_prefix_pop();
+}
+
 int main(void)
 {
 	report_prefix_push("skrf");

@@ -121,6 +201,7 @@ int main(void)
 	test_mvcos();
 	test_spka();
 	test_tprot();
+	test_exception_ext_new();
 
 done:
 	report_prefix_pop();
s390x/unittests.cfg +4 -0
index 0f156af..b35269b 100644
@@ -88,3 +88,7 @@ extra_params = -m 3G
 [css]
 file = css.elf
 extra_params = -device virtio-net-ccw
+
+[skrf]
+file = skrf.elf
+smp = 2
Signature · 1 line
2.25.1
Reviewed-by on v1 2/3
#8 lore raw

Re: [kvm-unit-tests PATCH 2/3] s390x: skrf: Add exception new skey test and add test to unittests.cfg

Quoted 124 lines · On Fri, 17 Jul 2020 10:58:12 -0400 Janosch Frank <[email protected]> wrote:

If a exception new psw mask contains a key a specification exception instead of a special operation exception is presented. Let's test that.

Also let's add the test to unittests.cfg so it is run more often.

Signed-off-by Janosch Frank <[email protected]>
---
 s390x/skrf.c        | 81

+++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg | 4 +++ 2 files changed, 85 insertions(+)

s390x/skrf.c +80 -0
index 9cae589..9733412 100644
@@ -15,6 +15,8 @@
 #include <asm/page.h>
 #include <asm/facility.h>
 #include <asm/mem.h>
+#include <asm/sigp.h>
+#include <smp.h>
 
 static uint8_t pagebuf[PAGE_SIZE * 2]
 __attribute__((aligned(PAGE_SIZE * 2))); 

@@ -106,6 +108,84 @@ static void test_tprot(void)
 	report_prefix_pop();
 }
 
+#include <asm-generic/barrier.h>
+static int testflag = 0;
+
+static void wait_for_flag(void)
+{
+	while (!testflag)
+		mb();
+}
+
+static void set_flag(int val)
+{
+	mb();
+	testflag = val;
+	mb();
+}
+
+static void ecall_cleanup(void)
+{
+	struct lowcore *lc = (void *)0x0;
+
+	lc->ext_new_psw.mask = 0x0000000180000000UL;
+	lc->sw_int_crs[0] = 0x0000000000040000;
+
+	/*
+	 * PGM old contains the ext new PSW, we need to clean it up,
+	 * so we don't get a special oepration exception on the lpswe
+	 * of pgm old.
+	 */
+	lc->pgm_old_psw.mask = 0x0000000180000000UL;
+	lc->pgm_old_psw.addr = (unsigned long)wait_for_flag;
+
+	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
+	set_flag(1);
+}
+
+/* Set a key into the external new psw mask and open external call
 masks */ +static void ecall_setup(void)
+{
+	struct lowcore *lc = (void *)0x0;
+	uint64_t mask;
+
+	register_pgm_int_func(ecall_cleanup);
+	expect_pgm_int();
+	/* Put a skey into the ext new psw */
+	lc->ext_new_psw.mask = 0x00F0000180000000UL;
+	/* Open up ext masks */
+	ctl_set_bit(0, 13);
+	mask = extract_psw_mask();
+	mask |= PSW_MASK_EXT;
+	load_psw_mask(mask);
+	/* Tell cpu 0 that we're ready */
+	set_flag(1);
+}
+
+static void test_exception_ext_new(void)
+{
+	struct psw psw = {
+		.mask = extract_psw_mask(),
+		.addr = (unsigned long)ecall_setup
+	};
+
+	report_prefix_push("exception external new");
+	if (smp_query_num_cpus() < 2) {
+		report_skip("Need second cpu for exception external
 new test.");
+		report_prefix_pop();
+		return;
+	}
+
+	smp_cpu_setup(1, psw);
+	wait_for_flag();
+	set_flag(0);
+
+	sigp(1, SIGP_EXTERNAL_CALL, 0, NULL);
+	wait_for_flag();
+	smp_cpu_stop(1);
+	report_prefix_pop();
+}
+
 int main(void)
 {
 	report_prefix_push("skrf");

@@ -121,6 +201,7 @@ int main(void)
 	test_mvcos();
 	test_spka();
 	test_tprot();
+	test_exception_ext_new();
 
 done:
 	report_prefix_pop();
s390x/unittests.cfg +0 -0
index 0f156af..b35269b 100644

s390x/unittests.cfg extra_params = -m 3G · line 94

Quoted 7 lines
s390x/unittests.cfg +4 -0
@@ -88,3 +88,7 @@ extra_params = -m 3G
 [css]
 file = css.elf
 extra_params = -device virtio-net-ccw
+
+[skrf]
+file = skrf.elf
+smp = 2
Reviewed-by Claudio Imbrenda <[email protected]>
#9 lore raw

Re: [kvm-unit-tests PATCH 2/3] s390x: skrf: Add exception new skey test and add test to unittests.cfg

In reply to Janosch Frank
Quoted 2 lines · Janosch Frank

If a exception new psw mask contains a key a specification exception instead of a special operation exception is presented.

I have troubles parsing that sentence... could you write that differently? (and: "s/a exception/an exception/")

Quoted 25 lines

Let's test
that.

Also let's add the test to unittests.cfg so it is run more often.

Signed-off-by Janosch Frank <[email protected]>
---
 s390x/skrf.c        | 81 +++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |  4 +++
 2 files changed, 85 insertions(+)
s390x/skrf.c +2 -0
index 9cae589..9733412 100644
@@ -15,6 +15,8 @@
 #include <asm/page.h>
 #include <asm/facility.h>
 #include <asm/mem.h>
+#include <asm/sigp.h>
+#include <smp.h>
 
 static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));

s390x/skrf.c static void test_tprot(void) · line 111

Quoted 4 lines
s390x/skrf.c +1 -0
@@ -106,6 +108,84 @@ static void test_tprot(void)
 	report_prefix_pop();
 }
 
+#include <asm-generic/barrier.h>

Can we keep the #includes at the top of the file, please?

Quoted 10 lines
s390x/skrf.c +10 -0
+static int testflag = 0;
+
+static void wait_for_flag(void)
+{
+	while (!testflag)
+		mb();
+}
+
+static void set_flag(int val)
+{

s390x/skrf.c static void test_tprot(void) · line 131

Quoted 10 lines
s390x/skrf.c +10 -0
+	mb();
+	testflag = val;
+	mb();
+}
+
+static void ecall_cleanup(void)
+{
+	struct lowcore *lc = (void *)0x0;
+
+	lc->ext_new_psw.mask = 0x0000000180000000UL;

Don't we have defines for the PSW values yet?

s390x/skrf.c static void test_tprot(void) · line 136

Quoted 5 lines
s390x/skrf.c +5 -0
+	lc->sw_int_crs[0] = 0x0000000000040000;
+
+	/*
+	 * PGM old contains the ext new PSW, we need to clean it up,
+	 * so we don't get a special oepration exception on the lpswe

operation

s390x/skrf.c static void test_tprot(void) · line 144

Quoted 8 lines
s390x/skrf.c +8 -0
+	 * of pgm old.
+	 */
+	lc->pgm_old_psw.mask = 0x0000000180000000UL;
+	lc->pgm_old_psw.addr = (unsigned long)wait_for_flag;
+
+	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
+	set_flag(1);
+}

Thomas

author
#10 lore raw

Re: [kvm-unit-tests PATCH 2/3] s390x: skrf: Add exception new skey test and add test to unittests.cfg

Quoted 6 lines · Thomas Huth
Quoted 2 lines · Janosch Frank

If a exception new psw mask contains a key a specification exception instead of a special operation exception is presented.

I have troubles parsing that sentence... could you write that differently? (and: "s/a exception/an exception/")

How about:

When an exception psw new with a storage key in its mask is loaded from lowcore a specification exception is raised instead of the special operation exception that is normally presented when skrf is active.

Quoted 25 lines
Quoted 25 lines

Let's test
that.

Also let's add the test to unittests.cfg so it is run more often.

Signed-off-by Janosch Frank <[email protected]>
---
 s390x/skrf.c        | 81 +++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |  4 +++
 2 files changed, 85 insertions(+)
s390x/skrf.c +2 -0
index 9cae589..9733412 100644
@@ -15,6 +15,8 @@
 #include <asm/page.h>
 #include <asm/facility.h>
 #include <asm/mem.h>
+#include <asm/sigp.h>
+#include <smp.h>
 
 static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
Quoted 7 lines

s390x/skrf.c static void test_tprot(void) · line 111

Quoted 5 lines
s390x/skrf.c +1 -0
@@ -106,6 +108,84 @@ static void test_tprot(void)
 	report_prefix_pop();
 }
 
+#include <asm-generic/barrier.h>

Can we keep the #includes at the top of the file, please?

Yes

Quoted 15 lines
Quoted 15 lines
s390x/skrf.c +14 -0
+static int testflag = 0;
+
+static void wait_for_flag(void)
+{
+	while (!testflag)
+		mb();
+}
+
+static void set_flag(int val)
+{
+	mb();
+	testflag = val;
+	mb();
+}
Quoted 8 lines

s390x/skrf.c static void test_tprot(void) · line 131

Quoted 6 lines
s390x/skrf.c +6 -0
+
+static void ecall_cleanup(void)
+{
+	struct lowcore *lc = (void *)0x0;
+
+	lc->ext_new_psw.mask = 0x0000000180000000UL;

Don't we have defines for the PSW values yet?

Pierre dropped that patch because of your old binutils version...

Quoted 8 lines

s390x/skrf.c static void test_tprot(void) · line 136

Quoted 6 lines
s390x/skrf.c +5 -0
+	lc->sw_int_crs[0] = 0x0000000000040000;
+
+	/*
+	 * PGM old contains the ext new PSW, we need to clean it up,
+	 * so we don't get a special oepration exception on the lpswe

operation

fixed

Quoted 12 lines

s390x/skrf.c static void test_tprot(void) · line 144

Quoted 9 lines
s390x/skrf.c +8 -0
+	 * of pgm old.
+	 */
+	lc->pgm_old_psw.mask = 0x0000000180000000UL;
+	lc->pgm_old_psw.addr = (unsigned long)wait_for_flag;
+
+	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
+	set_flag(1);
+}

Thomas

#11 lore raw

Re: [kvm-unit-tests PATCH 2/3] s390x: skrf: Add exception new skey test and add test to unittests.cfg

Quoted 5 lines · Janosch Frank
Quoted 4 lines · Thomas Huth
Quoted 2 lines · Janosch Frank

If a exception new psw mask contains a key a specification exception instead of a special operation exception is presented.

Quoted 8 lines
Quoted 2 lines

I have troubles parsing that sentence... could you write that differently? (and: "s/a exception/an exception/")

How about:

When an exception psw new with a storage key in its mask is loaded from lowcore a specification exception is raised instead of the special operation exception that is normally presented when skrf is active.

Still a huge beast of a sentence. Could you maybe make two sentences out of it? For example:

" ... is raised. This differs from the normal case where ..."

?

Thomas

author
#12 lore raw

Re: [kvm-unit-tests PATCH 2/3] s390x: skrf: Add exception new skey test and add test to unittests.cfg

Quoted 11 lines · Thomas Huth
Quoted 10 lines · Janosch Frank
Quoted 6 lines · Thomas Huth
Quoted 2 lines · Janosch Frank

If a exception new psw mask contains a key a specification exception instead of a special operation exception is presented.

I have troubles parsing that sentence... could you write that differently? (and: "s/a exception/an exception/")

How about:

Quoted 8 lines
Quoted 3 lines

When an exception psw new with a storage key in its mask is loaded from lowcore a specification exception is raised instead of the special operation exception that is normally presented when skrf is active.

Still a huge beast of a sentence. Could you maybe make two sentences out of it? For example:

" ... is raised. This differs from the normal case where ..."

When an exception psw new with a storage key in its mask is loaded from lowcore a specification exception is raised. This behavior differs from the one that is presented when trying to execute skey related instructions which will raise special operation exceptions.

Quoted 5 lines

?

Thomas

#13 lore raw

Re: [kvm-unit-tests PATCH 2/3] s390x: skrf: Add exception new skey test and add test to unittests.cfg

Quoted 19 lines · On Tue, 21 Jul 2020 17:03:10 +0200 Janosch Frank <[email protected]> wrote:
Quoted 18 lines · Thomas Huth
Quoted 5 lines · Janosch Frank
Quoted 4 lines · Thomas Huth
Quoted 2 lines · Janosch Frank

If a exception new psw mask contains a key a specification exception instead of a special operation exception is presented.

Quoted 8 lines
Quoted 2 lines

I have troubles parsing that sentence... could you write that differently? (and: "s/a exception/an exception/")

How about:

When an exception psw new with a storage key in its mask is loaded from lowcore a specification exception is raised instead of the special operation exception that is normally presented when skrf is active.

Still a huge beast of a sentence. Could you maybe make two sentences out of it? For example:

Quoted 6 lines
Quoted 1 line

" ... is raised. This differs from the normal case where ..."

When an exception psw new with a storage key in its mask is loaded from lowcore a specification exception is raised. This behavior differs from the one that is presented when trying to execute skey related instructions which will raise special operation exceptions.

s/psw new/new psw/ ?

(And probably a comma after 'lowcore'.)

"This differs from the behaviour when trying to execute skey related instructions, which will result in special operation exceptions."

?

author
#14 lore raw

Re: [kvm-unit-tests PATCH 2/3] s390x: skrf: Add exception new skey test and add test to unittests.cfg

Quoted 22 lines · Cornelia Huck
Quoted 19 lines · On Tue, 21 Jul 2020 17:03:10 +0200 Janosch Frank <[email protected]> wrote:
Quoted 18 lines · Thomas Huth
Quoted 5 lines · Janosch Frank
Quoted 4 lines · Thomas Huth
Quoted 2 lines · Janosch Frank

If a exception new psw mask contains a key a specification exception instead of a special operation exception is presented.

Quoted 8 lines
Quoted 2 lines

I have troubles parsing that sentence... could you write that differently? (and: "s/a exception/an exception/")

How about:

When an exception psw new with a storage key in its mask is loaded from lowcore a specification exception is raised instead of the special operation exception that is normally presented when skrf is active.

Still a huge beast of a sentence. Could you maybe make two sentences out of it? For example:

Quoted 8 lines
Quoted 6 lines
Quoted 1 line

" ... is raised. This differs from the normal case where ..."

When an exception psw new with a storage key in its mask is loaded from lowcore a specification exception is raised. This behavior differs from the one that is presented when trying to execute skey related instructions which will raise special operation exceptions.

s/psw new/new psw/ ?

Yeah that would align the naming with the pop one.

Quoted 7 lines

(And probably a comma after 'lowcore'.)

"This differs from the behaviour when trying to execute skey related instructions, which will result in special operation exceptions."

?

Sure

author
#15 lore raw

PATCH 3/3 s390x: Ultavisor guest API test

In reply to Janosch Frank

Test the error conditions of guest 2 Ultravisor calls, namely:
* Query Ultravisor information
* Set shared access
* Remove shared access

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm/uv.h  |  68 +++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 156 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 228 insertions(+)
 create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c
lib/s390x/asm/uv.h new file +68 -0
new file mode 100644
index 0000000..14ab5cc
@@ -0,0 +1,68 @@
+/*
+ * s390x Ultravisor related definitions
+ *
+ * Copyright (c) 2020 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <[email protected]>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2.
+ */
+#ifndef UV_H
+#define UV_H
+
+#define UVC_RC_EXECUTED		0x0001
+#define UVC_RC_INV_CMD		0x0002
+#define UVC_RC_INV_STATE	0x0003
+#define UVC_RC_INV_LEN		0x0005
+#define UVC_RC_NO_RESUME	0x0007
+
+#define UVC_CMD_QUI			0x0001
+#define UVC_CMD_SET_SHARED_ACCESS	0x1000
+#define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
+
+/* Bits in installed uv calls */
+enum uv_cmds_inst {
+	BIT_UVC_CMD_QUI = 0,
+	BIT_UVC_CMD_SET_SHARED_ACCESS = 8,
+	BIT_UVC_CMD_REMOVE_SHARED_ACCESS = 9,
+};
+
+struct uv_cb_header {
+	u16 len;
+	u16 cmd;	/* Command Code */
+	u16 rc;		/* Response Code */
+	u16 rrc;	/* Return Reason Code */
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+struct uv_cb_qui {
+	struct uv_cb_header header;
+	u64 reserved08;
+	u64 inst_calls_list[4];
+	u64 reserved30[15];
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+struct uv_cb_share {
+	struct uv_cb_header header;
+	u64 reserved08[3];
+	u64 paddr;
+	u64 reserved28;
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+static inline int uv_call(unsigned long r1, unsigned long r2)
+{
+	int cc;
+
+	asm volatile(
+		"0:	.insn rrf,0xB9A40000,%[r1],%[r2],0,0\n"
+		"		brc	3,0b\n"
+		"		ipm	%[cc]\n"
+		"		srl	%[cc],28\n"
+		: [cc] "=d" (cc)
+		: [r1] "a" (r1), [r2] "a" (r2)
+		: "memory", "cc");
+	return cc;
+}
+
+#endif
s390x/Makefile +1 -0
index 0f54bf4..c2213ad 100644
@@ -18,6 +18,7 @@ tests += $(TEST_DIR)/skrf.elf
 tests += $(TEST_DIR)/smp.elf
 tests += $(TEST_DIR)/sclp.elf
 tests += $(TEST_DIR)/css.elf
+tests += $(TEST_DIR)/uv-guest.elf
 tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
 all: directories test_cases test_cases_binary
s390x/unittests.cfg +3 -0
index b35269b..38c3257 100644
@@ -92,3 +92,6 @@ extra_params = -device virtio-net-ccw
 [skrf]
 file = skrf.elf
 smp = 2
+
+[uv-guest]
+file = uv-guest.elf
\ No newline at end of file
s390x/uv-guest.c new file +156 -0

156 lines not on this page · raw message

Signature · 1 line
2.25.1
#16 lore raw

Re: [kvm-unit-tests PATCH 3/3] s390x: Ultavisor guest API test

Quoted 179 lines · On Fri, 17 Jul 2020 10:58:13 -0400 Janosch Frank <[email protected]> wrote:

Test the error conditions of guest 2 Ultravisor calls, namely:
* Query Ultravisor information
* Set shared access
* Remove shared access

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm/uv.h  |  68 +++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 156
++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 228
insertions(+) create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c
lib/s390x/asm/uv.h new file +9 -0
new file mode 100644
index 0000000..14ab5cc
@@ -0,0 +1,68 @@
+/*
+ * s390x Ultravisor related definitions
+ *
+ * Copyright (c) 2020 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <[email protected]>
+ *
+ * This code is free software; you can redistribute it and/or modify

it
+ * under the terms of the GNU General Public License version 2.
+ */

lib/s390x/asm/uv.h new file +57 -0
+#ifndef UV_H
+#define UV_H
+
+#define UVC_RC_EXECUTED		0x0001
+#define UVC_RC_INV_CMD		0x0002
+#define UVC_RC_INV_STATE	0x0003
+#define UVC_RC_INV_LEN		0x0005
+#define UVC_RC_NO_RESUME	0x0007
+
+#define UVC_CMD_QUI			0x0001
+#define UVC_CMD_SET_SHARED_ACCESS	0x1000
+#define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
+
+/* Bits in installed uv calls */
+enum uv_cmds_inst {
+	BIT_UVC_CMD_QUI = 0,
+	BIT_UVC_CMD_SET_SHARED_ACCESS = 8,
+	BIT_UVC_CMD_REMOVE_SHARED_ACCESS = 9,
+};
+
+struct uv_cb_header {
+	u16 len;
+	u16 cmd;	/* Command Code */
+	u16 rc;		/* Response Code */
+	u16 rrc;	/* Return Reason Code */
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+struct uv_cb_qui {
+	struct uv_cb_header header;
+	u64 reserved08;
+	u64 inst_calls_list[4];
+	u64 reserved30[15];
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+struct uv_cb_share {
+	struct uv_cb_header header;
+	u64 reserved08[3];
+	u64 paddr;
+	u64 reserved28;
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+static inline int uv_call(unsigned long r1, unsigned long r2)
+{
+	int cc;
+
+	asm volatile(
+		"0:	.insn rrf,0xB9A40000,%[r1],%[r2],0,0\n"
+		"		brc	3,0b\n"
+		"		ipm	%[cc]\n"
+		"		srl	%[cc],28\n"
+		: [cc] "=d" (cc)
+		: [r1] "a" (r1), [r2] "a" (r2)
+		: "memory", "cc");
+	return cc;
+}
+
+#endif
s390x/Makefile +1 -0
index 0f54bf4..c2213ad 100644
@@ -18,6 +18,7 @@ tests += $(TEST_DIR)/skrf.elf
 tests += $(TEST_DIR)/smp.elf
 tests += $(TEST_DIR)/sclp.elf
 tests += $(TEST_DIR)/css.elf
+tests += $(TEST_DIR)/uv-guest.elf
 tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
 all: directories test_cases test_cases_binary
s390x/unittests.cfg +3 -0
index b35269b..38c3257 100644
@@ -92,3 +92,6 @@ extra_params = -device virtio-net-ccw
 [skrf]
 file = skrf.elf
 smp = 2
+
+[uv-guest]
+file = uv-guest.elf
\ No newline at end of file
s390x/uv-guest.c new file +9 -0
new file mode 100644
index 0000000..0cb5fae
@@ -0,0 +1,156 @@
+/*
+ * Guest Ultravisor Call tests
+ *
+ * Copyright (c) 2020 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <[email protected]>
+ *
+ * This code is free software; you can redistribute it and/or modify

it
+ * under the terms of the GNU General Public License version 2.
+ */

s390x/uv-guest.c new file +47 -0
+
+#include <libcflat.h>
+#include <alloc_page.h>
+#include <asm/page.h>
+#include <asm/asm-offsets.h>
+#include <asm/interrupt.h>
+#include <asm/facility.h>
+#include <asm/uv.h>
+
+static inline int share(unsigned long addr, u16 cmd)
+{
+	struct uv_cb_share uvcb = {
+		.header.cmd = cmd,
+		.header.len = sizeof(uvcb),
+		.paddr = addr
+	};
+
+	uv_call(0, (u64)&uvcb);
+	return uvcb.header.rc;
+}
+
+static inline int uv_set_shared(unsigned long addr)
+{
+	return share(addr, UVC_CMD_SET_SHARED_ACCESS);
+}
+
+static inline int uv_remove_shared(unsigned long addr)
+{
+	return share(addr, UVC_CMD_REMOVE_SHARED_ACCESS);
+}
+
+static void test_priv(void)
+{
+	struct uv_cb_header uvcb = {};
+
+	report_prefix_push("privileged");
+
+	report_prefix_push("query");
+	expect_pgm_int();
+	uvcb.cmd = UVC_CMD_QUI;
+	uvcb.len = sizeof(struct uv_cb_qui);
+	enter_pstate();
+	uv_call(0, (u64)&uvcb);
+	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+	report_prefix_pop();
+
+	report_prefix_push("share");

s390x/uv-guest.c line 68

Quoted 10 lines
s390x/uv-guest.c new file +10 -0
+	expect_pgm_int();
+	enter_pstate();
+	uv_set_shared(0x42000);
+	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+	report_prefix_pop();
+
+	report_prefix_push("unshare");
+	expect_pgm_int();
+	enter_pstate();
+	uv_remove_shared(0x42000);

I don't like using a hardwired address here (and above). Things can get messy if the address ends up outside memory or overlapping code.

I think the best approach would be to declare a page aligned buffer and use that (or even allocate a page)

Quoted 82 lines
s390x/uv-guest.c new file +20 -0
+	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+	report_prefix_pop();
+
+	report_prefix_pop();
+}
+
+static void test_query(void)
+{
+	struct uv_cb_qui uvcb = {
+		.header.cmd = UVC_CMD_QUI,
+		.header.len = sizeof(uvcb) - 8,
+	};
+
+	report_prefix_push("query");
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_INV_LEN, "length");
+
+	uvcb.header.len = sizeof(uvcb);
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_EXECUTED, "successful

query"); +

s390x/uv-guest.c new file +6 -0
+	/*
+	 * These bits have been introduced with the very first
+	 * Ultravisor version and are expected to always be available
+	 * because they are basic building blocks.
+	 */
+	report(uvcb.inst_calls_list[0] & (1UL << (63 -

BIT_UVC_CMD_QUI)),

s390x/uv-guest.c new file +2 -0
+	       "query indicated");
+	report(uvcb.inst_calls_list[0] & (1UL << (63 -

BIT_UVC_CMD_SET_SHARED_ACCESS)),

s390x/uv-guest.c new file +2 -0
+	       "share indicated");
+	report(uvcb.inst_calls_list[0] & (1UL << (63 -

BIT_UVC_CMD_REMOVE_SHARED_ACCESS)),

s390x/uv-guest.c new file +47 -0
+	       "unshare indicated");
+	report_prefix_pop();
+}
+
+static void test_sharing(void)
+{
+	unsigned long mem = (unsigned long)alloc_page();
+	struct uv_cb_share uvcb = {
+		.header.cmd = UVC_CMD_SET_SHARED_ACCESS,
+		.header.len = sizeof(uvcb) - 8,
+	};
+
+	report_prefix_push("share");
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_INV_LEN, "length");
+	report(uv_set_shared(mem) == UVC_RC_EXECUTED, "share");
+	report_prefix_pop();
+
+	report_prefix_push("unshare");
+	uvcb.header.cmd = UVC_CMD_REMOVE_SHARED_ACCESS;
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_INV_LEN, "length");
+	report(uv_remove_shared(mem) == UVC_RC_EXECUTED, "unshare");
+	free_page((void *)mem);
+	report_prefix_pop();
+
+	report_prefix_pop();
+}
+
+static void test_invalid(void)
+{
+	struct uv_cb_header uvcb = {
+		.len = 16,
+		.cmd = 0x4242,
+	};
+
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.rc == UVC_RC_INV_CMD, "invalid command");
+}
+
+int main(void)
+{
+	bool has_uvc = test_facility(158);
+
+	report_prefix_push("uvc");
+	if (!has_uvc) {
+		report_skip("Ultravisor call facility is not

available");

s390x/uv-guest.c line 156

Quoted 10 lines
s390x/uv-guest.c new file +10 -0
+		goto done;
+	}
+
+	test_priv();
+	test_invalid();
+	test_query();
+	test_sharing();
+done:
+	return report_summary();
+}

once the above comment is addressed:
Claudio Imbrenda <[email protected]>

author
#17 lore raw

Re: [kvm-unit-tests PATCH 3/3] s390x: Ultavisor guest API test

Quoted 196 lines · Claudio Imbrenda
Quoted 179 lines · On Fri, 17 Jul 2020 10:58:13 -0400 Janosch Frank <[email protected]> wrote:

Test the error conditions of guest 2 Ultravisor calls, namely:
* Query Ultravisor information
* Set shared access
* Remove shared access

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm/uv.h  |  68 +++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 156
++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 228
insertions(+) create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c
lib/s390x/asm/uv.h new file +9 -0
new file mode 100644
index 0000000..14ab5cc
@@ -0,0 +1,68 @@
+/*
+ * s390x Ultravisor related definitions
+ *
+ * Copyright (c) 2020 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <[email protected]>
+ *
+ * This code is free software; you can redistribute it and/or modify

it
+ * under the terms of the GNU General Public License version 2.
+ */

lib/s390x/asm/uv.h new file +57 -0
+#ifndef UV_H
+#define UV_H
+
+#define UVC_RC_EXECUTED		0x0001
+#define UVC_RC_INV_CMD		0x0002
+#define UVC_RC_INV_STATE	0x0003
+#define UVC_RC_INV_LEN		0x0005
+#define UVC_RC_NO_RESUME	0x0007
+
+#define UVC_CMD_QUI			0x0001
+#define UVC_CMD_SET_SHARED_ACCESS	0x1000
+#define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
+
+/* Bits in installed uv calls */
+enum uv_cmds_inst {
+	BIT_UVC_CMD_QUI = 0,
+	BIT_UVC_CMD_SET_SHARED_ACCESS = 8,
+	BIT_UVC_CMD_REMOVE_SHARED_ACCESS = 9,
+};
+
+struct uv_cb_header {
+	u16 len;
+	u16 cmd;	/* Command Code */
+	u16 rc;		/* Response Code */
+	u16 rrc;	/* Return Reason Code */
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+struct uv_cb_qui {
+	struct uv_cb_header header;
+	u64 reserved08;
+	u64 inst_calls_list[4];
+	u64 reserved30[15];
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+struct uv_cb_share {
+	struct uv_cb_header header;
+	u64 reserved08[3];
+	u64 paddr;
+	u64 reserved28;
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+static inline int uv_call(unsigned long r1, unsigned long r2)
+{
+	int cc;
+
+	asm volatile(
+		"0:	.insn rrf,0xB9A40000,%[r1],%[r2],0,0\n"
+		"		brc	3,0b\n"
+		"		ipm	%[cc]\n"
+		"		srl	%[cc],28\n"
+		: [cc] "=d" (cc)
+		: [r1] "a" (r1), [r2] "a" (r2)
+		: "memory", "cc");
+	return cc;
+}
+
+#endif
s390x/Makefile +1 -0
index 0f54bf4..c2213ad 100644
@@ -18,6 +18,7 @@ tests += $(TEST_DIR)/skrf.elf
 tests += $(TEST_DIR)/smp.elf
 tests += $(TEST_DIR)/sclp.elf
 tests += $(TEST_DIR)/css.elf
+tests += $(TEST_DIR)/uv-guest.elf
 tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
 all: directories test_cases test_cases_binary
s390x/unittests.cfg +3 -0
index b35269b..38c3257 100644
@@ -92,3 +92,6 @@ extra_params = -device virtio-net-ccw
 [skrf]
 file = skrf.elf
 smp = 2
+
+[uv-guest]
+file = uv-guest.elf
\ No newline at end of file
s390x/uv-guest.c new file +9 -0
new file mode 100644
index 0000000..0cb5fae
@@ -0,0 +1,156 @@
+/*
+ * Guest Ultravisor Call tests
+ *
+ * Copyright (c) 2020 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <[email protected]>
+ *
+ * This code is free software; you can redistribute it and/or modify

it
+ * under the terms of the GNU General Public License version 2.
+ */

s390x/uv-guest.c new file +47 -0
+
+#include <libcflat.h>
+#include <alloc_page.h>
+#include <asm/page.h>
+#include <asm/asm-offsets.h>
+#include <asm/interrupt.h>
+#include <asm/facility.h>
+#include <asm/uv.h>
+
+static inline int share(unsigned long addr, u16 cmd)
+{
+	struct uv_cb_share uvcb = {
+		.header.cmd = cmd,
+		.header.len = sizeof(uvcb),
+		.paddr = addr
+	};
+
+	uv_call(0, (u64)&uvcb);
+	return uvcb.header.rc;
+}
+
+static inline int uv_set_shared(unsigned long addr)
+{
+	return share(addr, UVC_CMD_SET_SHARED_ACCESS);
+}
+
+static inline int uv_remove_shared(unsigned long addr)
+{
+	return share(addr, UVC_CMD_REMOVE_SHARED_ACCESS);
+}
+
+static void test_priv(void)
+{
+	struct uv_cb_header uvcb = {};
+
+	report_prefix_push("privileged");
+
+	report_prefix_push("query");
+	expect_pgm_int();
+	uvcb.cmd = UVC_CMD_QUI;
+	uvcb.len = sizeof(struct uv_cb_qui);
+	enter_pstate();
+	uv_call(0, (u64)&uvcb);
+	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+	report_prefix_pop();
+
+	report_prefix_push("share");

s390x/uv-guest.c line 68

Quoted 10 lines
s390x/uv-guest.c new file +10 -0
+	expect_pgm_int();
+	enter_pstate();
+	uv_set_shared(0x42000);
+	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+	report_prefix_pop();
+
+	report_prefix_push("unshare");
+	expect_pgm_int();
+	enter_pstate();
+	uv_remove_shared(0x42000);

I don't like using a hardwired address here (and above). Things can get messy if the address ends up outside memory or overlapping code.

Quoted 2 lines

I think the best approach would be to declare a page aligned buffer and use that (or even allocate a page)

Sure, will do

Quoted 88 lines
Quoted 88 lines
s390x/uv-guest.c new file +20 -0
+	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+	report_prefix_pop();
+
+	report_prefix_pop();
+}
+
+static void test_query(void)
+{
+	struct uv_cb_qui uvcb = {
+		.header.cmd = UVC_CMD_QUI,
+		.header.len = sizeof(uvcb) - 8,
+	};
+
+	report_prefix_push("query");
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_INV_LEN, "length");
+
+	uvcb.header.len = sizeof(uvcb);
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_EXECUTED, "successful

query"); +

s390x/uv-guest.c new file +6 -0
+	/*
+	 * These bits have been introduced with the very first
+	 * Ultravisor version and are expected to always be available
+	 * because they are basic building blocks.
+	 */
+	report(uvcb.inst_calls_list[0] & (1UL << (63 -

BIT_UVC_CMD_QUI)),

s390x/uv-guest.c new file +2 -0
+	       "query indicated");
+	report(uvcb.inst_calls_list[0] & (1UL << (63 -

BIT_UVC_CMD_SET_SHARED_ACCESS)),

s390x/uv-guest.c new file +2 -0
+	       "share indicated");
+	report(uvcb.inst_calls_list[0] & (1UL << (63 -

BIT_UVC_CMD_REMOVE_SHARED_ACCESS)),

s390x/uv-guest.c new file +47 -0
+	       "unshare indicated");
+	report_prefix_pop();
+}
+
+static void test_sharing(void)
+{
+	unsigned long mem = (unsigned long)alloc_page();
+	struct uv_cb_share uvcb = {
+		.header.cmd = UVC_CMD_SET_SHARED_ACCESS,
+		.header.len = sizeof(uvcb) - 8,
+	};
+
+	report_prefix_push("share");
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_INV_LEN, "length");
+	report(uv_set_shared(mem) == UVC_RC_EXECUTED, "share");
+	report_prefix_pop();
+
+	report_prefix_push("unshare");
+	uvcb.header.cmd = UVC_CMD_REMOVE_SHARED_ACCESS;
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_INV_LEN, "length");
+	report(uv_remove_shared(mem) == UVC_RC_EXECUTED, "unshare");
+	free_page((void *)mem);
+	report_prefix_pop();
+
+	report_prefix_pop();
+}
+
+static void test_invalid(void)
+{
+	struct uv_cb_header uvcb = {
+		.len = 16,
+		.cmd = 0x4242,
+	};
+
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.rc == UVC_RC_INV_CMD, "invalid command");
+}
+
+int main(void)
+{
+	bool has_uvc = test_facility(158);
+
+	report_prefix_push("uvc");
+	if (!has_uvc) {
+		report_skip("Ultravisor call facility is not

available");

s390x/uv-guest.c new file +5 -0
+		goto done;
+	}
+
+	test_priv();
+	test_invalid();
Quoted 9 lines

s390x/uv-guest.c line 156

Quoted 5 lines
s390x/uv-guest.c new file +5 -0
+	test_query();
+	test_sharing();
+done:
+	return report_summary();
+}

once the above comment is addressed:
Claudio Imbrenda <[email protected]>

?

Reviewed-by
#18 lore raw

Re: [kvm-unit-tests PATCH 3/3] s390x: Ultavisor guest API test

Quoted 301 lines · On Mon, 20 Jul 2020 13:42:35 +0200 Janosch Frank <[email protected]> wrote:
Quoted 196 lines · Claudio Imbrenda
Quoted 179 lines · On Fri, 17 Jul 2020 10:58:13 -0400 Janosch Frank <[email protected]> wrote:

Test the error conditions of guest 2 Ultravisor calls, namely:
* Query Ultravisor information
* Set shared access
* Remove shared access

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm/uv.h  |  68 +++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 156
++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 228
insertions(+) create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c
lib/s390x/asm/uv.h new file +9 -0
new file mode 100644
index 0000000..14ab5cc
@@ -0,0 +1,68 @@
+/*
+ * s390x Ultravisor related definitions
+ *
+ * Copyright (c) 2020 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <[email protected]>
+ *
+ * This code is free software; you can redistribute it and/or

modify it
+ * under the terms of the GNU General Public License version 2.
+ */

lib/s390x/asm/uv.h new file +57 -0
+#ifndef UV_H
+#define UV_H
+
+#define UVC_RC_EXECUTED		0x0001
+#define UVC_RC_INV_CMD		0x0002
+#define UVC_RC_INV_STATE	0x0003
+#define UVC_RC_INV_LEN		0x0005
+#define UVC_RC_NO_RESUME	0x0007
+
+#define UVC_CMD_QUI			0x0001
+#define UVC_CMD_SET_SHARED_ACCESS	0x1000
+#define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
+
+/* Bits in installed uv calls */
+enum uv_cmds_inst {
+	BIT_UVC_CMD_QUI = 0,
+	BIT_UVC_CMD_SET_SHARED_ACCESS = 8,
+	BIT_UVC_CMD_REMOVE_SHARED_ACCESS = 9,
+};
+
+struct uv_cb_header {
+	u16 len;
+	u16 cmd;	/* Command Code */
+	u16 rc;		/* Response Code */
+	u16 rrc;	/* Return Reason Code */
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+struct uv_cb_qui {
+	struct uv_cb_header header;
+	u64 reserved08;
+	u64 inst_calls_list[4];
+	u64 reserved30[15];
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+struct uv_cb_share {
+	struct uv_cb_header header;
+	u64 reserved08[3];
+	u64 paddr;
+	u64 reserved28;
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+static inline int uv_call(unsigned long r1, unsigned long r2)
+{
+	int cc;
+
+	asm volatile(
+		"0:	.insn rrf,0xB9A40000,%[r1],%[r2],0,0\n"
+		"		brc	3,0b\n"
+		"		ipm	%[cc]\n"
+		"		srl	%[cc],28\n"
+		: [cc] "=d" (cc)
+		: [r1] "a" (r1), [r2] "a" (r2)
+		: "memory", "cc");
+	return cc;
+}
+
+#endif
s390x/Makefile +1 -0
index 0f54bf4..c2213ad 100644
@@ -18,6 +18,7 @@ tests += $(TEST_DIR)/skrf.elf
 tests += $(TEST_DIR)/smp.elf
 tests += $(TEST_DIR)/sclp.elf
 tests += $(TEST_DIR)/css.elf
+tests += $(TEST_DIR)/uv-guest.elf
 tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
 all: directories test_cases test_cases_binary
s390x/unittests.cfg +3 -0
index b35269b..38c3257 100644
@@ -92,3 +92,6 @@ extra_params = -device virtio-net-ccw
 [skrf]
 file = skrf.elf
 smp = 2
+
+[uv-guest]
+file = uv-guest.elf
\ No newline at end of file
s390x/uv-guest.c new file +9 -0
new file mode 100644
index 0000000..0cb5fae
@@ -0,0 +1,156 @@
+/*
+ * Guest Ultravisor Call tests
+ *
+ * Copyright (c) 2020 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <[email protected]>
+ *
+ * This code is free software; you can redistribute it and/or

modify it
+ * under the terms of the GNU General Public License version 2.
+ */

s390x/uv-guest.c new file +47 -0
+
+#include <libcflat.h>
+#include <alloc_page.h>
+#include <asm/page.h>
+#include <asm/asm-offsets.h>
+#include <asm/interrupt.h>
+#include <asm/facility.h>
+#include <asm/uv.h>
+
+static inline int share(unsigned long addr, u16 cmd)
+{
+	struct uv_cb_share uvcb = {
+		.header.cmd = cmd,
+		.header.len = sizeof(uvcb),
+		.paddr = addr
+	};
+
+	uv_call(0, (u64)&uvcb);
+	return uvcb.header.rc;
+}
+
+static inline int uv_set_shared(unsigned long addr)
+{
+	return share(addr, UVC_CMD_SET_SHARED_ACCESS);
+}
+
+static inline int uv_remove_shared(unsigned long addr)
+{
+	return share(addr, UVC_CMD_REMOVE_SHARED_ACCESS);
+}
+
+static void test_priv(void)
+{
+	struct uv_cb_header uvcb = {};
+
+	report_prefix_push("privileged");
+
+	report_prefix_push("query");
+	expect_pgm_int();
+	uvcb.cmd = UVC_CMD_QUI;
+	uvcb.len = sizeof(struct uv_cb_qui);
+	enter_pstate();
+	uv_call(0, (u64)&uvcb);
+	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+	report_prefix_pop();
+
+	report_prefix_push("share");

s390x/uv-guest.c line 68

Quoted 10 lines
s390x/uv-guest.c new file +10 -0
+	expect_pgm_int();
+	enter_pstate();
+	uv_set_shared(0x42000);
+	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+	report_prefix_pop();
+
+	report_prefix_push("unshare");
+	expect_pgm_int();
+	enter_pstate();
+	uv_remove_shared(0x42000);

I don't like using a hardwired address here (and above). Things can get messy if the address ends up outside memory or overlapping code.

Quoted 2 lines

I think the best approach would be to declare a page aligned buffer and use that (or even allocate a page)

Sure, will do

Quoted 90 lines
Quoted 90 lines
s390x/uv-guest.c new file +20 -0
+	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+	report_prefix_pop();
+
+	report_prefix_pop();
+}
+
+static void test_query(void)
+{
+	struct uv_cb_qui uvcb = {
+		.header.cmd = UVC_CMD_QUI,
+		.header.len = sizeof(uvcb) - 8,
+	};
+
+	report_prefix_push("query");
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_INV_LEN, "length");
+
+	uvcb.header.len = sizeof(uvcb);
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_EXECUTED, "successful

query"); +

s390x/uv-guest.c new file +3 -0
+	/*
+	 * These bits have been introduced with the very first
+	 * Ultravisor version and are expected to always be

available

s390x/uv-guest.c new file +3 -0
+	 * because they are basic building blocks.
+	 */
+	report(uvcb.inst_calls_list[0] & (1UL << (63 -

BIT_UVC_CMD_QUI)),

s390x/uv-guest.c new file +2 -0
+	       "query indicated");
+	report(uvcb.inst_calls_list[0] & (1UL << (63 -

BIT_UVC_CMD_SET_SHARED_ACCESS)),

s390x/uv-guest.c new file +2 -0
+	       "share indicated");
+	report(uvcb.inst_calls_list[0] & (1UL << (63 -

BIT_UVC_CMD_REMOVE_SHARED_ACCESS)),

s390x/uv-guest.c new file +23 -0
+	       "unshare indicated");
+	report_prefix_pop();
+}
+
+static void test_sharing(void)
+{
+	unsigned long mem = (unsigned long)alloc_page();
+	struct uv_cb_share uvcb = {
+		.header.cmd = UVC_CMD_SET_SHARED_ACCESS,
+		.header.len = sizeof(uvcb) - 8,
+	};
+
+	report_prefix_push("share");
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_INV_LEN, "length");
+	report(uv_set_shared(mem) == UVC_RC_EXECUTED, "share");
+	report_prefix_pop();
+
+	report_prefix_push("unshare");
+	uvcb.header.cmd = UVC_CMD_REMOVE_SHARED_ACCESS;
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_INV_LEN, "length");
+	report(uv_remove_shared(mem) == UVC_RC_EXECUTED,

"unshare");

s390x/uv-guest.c new file +24 -0
+	free_page((void *)mem);
+	report_prefix_pop();
+
+	report_prefix_pop();
+}
+
+static void test_invalid(void)
+{
+	struct uv_cb_header uvcb = {
+		.len = 16,
+		.cmd = 0x4242,
+	};
+
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.rc == UVC_RC_INV_CMD, "invalid command");
+}
+
+int main(void)
+{
+	bool has_uvc = test_facility(158);
+
+	report_prefix_push("uvc");
+	if (!has_uvc) {
+		report_skip("Ultravisor call facility is not

available");

s390x/uv-guest.c new file +5 -0
+		goto done;
+	}
+
+	test_priv();
+	test_invalid();
Quoted 9 lines

s390x/uv-guest.c line 156

Quoted 5 lines
s390x/uv-guest.c new file +5 -0
+	test_query();
+	test_sharing();
+done:
+	return report_summary();
+}

once the above comment is addressed:
Claudio Imbrenda <[email protected]>

Quoted 2 lines

?

copy-paste error.... I meant:

Reviewed-by Claudio Imbrenda <[email protected]>
author
#19 lore raw

PATCH v2 1/1 s390x: Ultavisor guest API test

In reply to Janosch Frank

Test the error conditions of guest 2 Ultravisor calls, namely:
* Query Ultravisor information
* Set shared access
* Remove shared access

Signed-off-by Janosch Frank <[email protected]>
Reviewed-by Claudio Imbrenda <[email protected]>
---
 lib/s390x/asm/uv.h  |  68 +++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 158 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 230 insertions(+)
 create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c
lib/s390x/asm/uv.h new file +68 -0
new file mode 100644
index 0000000..14ab5cc
@@ -0,0 +1,68 @@
+/*
+ * s390x Ultravisor related definitions
+ *
+ * Copyright (c) 2020 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <[email protected]>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2.
+ */
+#ifndef UV_H
+#define UV_H
+
+#define UVC_RC_EXECUTED		0x0001
+#define UVC_RC_INV_CMD		0x0002
+#define UVC_RC_INV_STATE	0x0003
+#define UVC_RC_INV_LEN		0x0005
+#define UVC_RC_NO_RESUME	0x0007
+
+#define UVC_CMD_QUI			0x0001
+#define UVC_CMD_SET_SHARED_ACCESS	0x1000
+#define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
+
+/* Bits in installed uv calls */
+enum uv_cmds_inst {
+	BIT_UVC_CMD_QUI = 0,
+	BIT_UVC_CMD_SET_SHARED_ACCESS = 8,
+	BIT_UVC_CMD_REMOVE_SHARED_ACCESS = 9,
+};
+
+struct uv_cb_header {
+	u16 len;
+	u16 cmd;	/* Command Code */
+	u16 rc;		/* Response Code */
+	u16 rrc;	/* Return Reason Code */
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+struct uv_cb_qui {
+	struct uv_cb_header header;
+	u64 reserved08;
+	u64 inst_calls_list[4];
+	u64 reserved30[15];
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+struct uv_cb_share {
+	struct uv_cb_header header;
+	u64 reserved08[3];
+	u64 paddr;
+	u64 reserved28;
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+static inline int uv_call(unsigned long r1, unsigned long r2)
+{
+	int cc;
+
+	asm volatile(
+		"0:	.insn rrf,0xB9A40000,%[r1],%[r2],0,0\n"
+		"		brc	3,0b\n"
+		"		ipm	%[cc]\n"
+		"		srl	%[cc],28\n"
+		: [cc] "=d" (cc)
+		: [r1] "a" (r1), [r2] "a" (r2)
+		: "memory", "cc");
+	return cc;
+}
+
+#endif
s390x/Makefile +1 -0
index 0f54bf4..c2213ad 100644
@@ -18,6 +18,7 @@ tests += $(TEST_DIR)/skrf.elf
 tests += $(TEST_DIR)/smp.elf
 tests += $(TEST_DIR)/sclp.elf
 tests += $(TEST_DIR)/css.elf
+tests += $(TEST_DIR)/uv-guest.elf
 tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
 all: directories test_cases test_cases_binary
s390x/unittests.cfg +3 -0
index b35269b..38c3257 100644
@@ -92,3 +92,6 @@ extra_params = -device virtio-net-ccw
 [skrf]
 file = skrf.elf
 smp = 2
+
+[uv-guest]
+file = uv-guest.elf
\ No newline at end of file
s390x/uv-guest.c new file +158 -0

158 lines not on this page · raw message

Signature · 1 line
2.25.1
#20 lore raw

Re: [kvm-unit-tests PATCH v2] s390x: Ultavisor guest API test

Quoted 15 lines · Janosch Frank

Test the error conditions of guest 2 Ultravisor calls, namely:
* Query Ultravisor information
* Set shared access
* Remove shared access

Signed-off-by Janosch Frank <[email protected]>
Reviewed-by Claudio Imbrenda <[email protected]>
---
 lib/s390x/asm/uv.h  |  68 +++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 158 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 230 insertions(+)
 create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c

[...]

Quoted 7 lines
s390x/uv-guest.c new file +7 -0
+int main(void)
+{
+	bool has_uvc = test_facility(158);
+
+	report_prefix_push("uvc");
+	if (!has_uvc) {
+		report_skip("Ultravisor call facility is not available");

s390x/uv-guest.c line 156

Quoted 10 lines
s390x/uv-guest.c new file +10 -0
+		goto done;
+	}
+
+	page = (unsigned long)alloc_page();
+	test_priv();
+	test_invalid();
+	test_query();
+	test_sharing();
+	free_page((void *)page);
+done:

It likely does not matter much, but for cleanliness, please add a report_prefix_pop() here.

s390x/uv-guest.c line 158

Quoted 3 lines
s390x/uv-guest.c new file +2 -0
+	return report_summary();
+}

Thomas

#21 lore raw

Re: [kvm-unit-tests PATCH 3/3] s390x: Ultavisor guest API test

In reply to Janosch Frank
Quoted 14 lines · Janosch Frank

Test the error conditions of guest 2 Ultravisor calls, namely:
* Query Ultravisor information
* Set shared access
* Remove shared access

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm/uv.h  |  68 +++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 156 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 228 insertions(+)
 create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c

[...]

s390x/unittests.cfg extra_params = -device virtio-net-ccw · line 97

Quoted 12 lines
s390x/unittests.cfg +3 -0
index b35269b..38c3257 100644
@@ -92,3 +92,6 @@ extra_params = -device virtio-net-ccw
 [skrf]
 file = skrf.elf
 smp = 2
+
+[uv-guest]
+file = uv-guest.elf
\ No newline at end of file

Add the newline, please.

Thomas

#22 lore raw

Re: [kvm-unit-tests PATCH 3/3] s390x: Ultavisor guest API test

In reply to Janosch Frank

btw, s/Ultavisor/Ultravisor/ in the patch title (the R is missing!)

Quoted 281 lines · On Fri, 17 Jul 2020 10:58:13 -0400 Janosch Frank <[email protected]> wrote:

Test the error conditions of guest 2 Ultravisor calls, namely:
* Query Ultravisor information
* Set shared access
* Remove shared access

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm/uv.h  |  68 +++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 156
++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 228
insertions(+) create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c
lib/s390x/asm/uv.h new file +9 -0
new file mode 100644
index 0000000..14ab5cc
@@ -0,0 +1,68 @@
+/*
+ * s390x Ultravisor related definitions
+ *
+ * Copyright (c) 2020 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <[email protected]>
+ *
+ * This code is free software; you can redistribute it and/or modify

it
+ * under the terms of the GNU General Public License version 2.
+ */

lib/s390x/asm/uv.h new file +57 -0
+#ifndef UV_H
+#define UV_H
+
+#define UVC_RC_EXECUTED		0x0001
+#define UVC_RC_INV_CMD		0x0002
+#define UVC_RC_INV_STATE	0x0003
+#define UVC_RC_INV_LEN		0x0005
+#define UVC_RC_NO_RESUME	0x0007
+
+#define UVC_CMD_QUI			0x0001
+#define UVC_CMD_SET_SHARED_ACCESS	0x1000
+#define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
+
+/* Bits in installed uv calls */
+enum uv_cmds_inst {
+	BIT_UVC_CMD_QUI = 0,
+	BIT_UVC_CMD_SET_SHARED_ACCESS = 8,
+	BIT_UVC_CMD_REMOVE_SHARED_ACCESS = 9,
+};
+
+struct uv_cb_header {
+	u16 len;
+	u16 cmd;	/* Command Code */
+	u16 rc;		/* Response Code */
+	u16 rrc;	/* Return Reason Code */
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+struct uv_cb_qui {
+	struct uv_cb_header header;
+	u64 reserved08;
+	u64 inst_calls_list[4];
+	u64 reserved30[15];
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+struct uv_cb_share {
+	struct uv_cb_header header;
+	u64 reserved08[3];
+	u64 paddr;
+	u64 reserved28;
+} __attribute__((packed))  __attribute__((aligned(8)));
+
+static inline int uv_call(unsigned long r1, unsigned long r2)
+{
+	int cc;
+
+	asm volatile(
+		"0:	.insn rrf,0xB9A40000,%[r1],%[r2],0,0\n"
+		"		brc	3,0b\n"
+		"		ipm	%[cc]\n"
+		"		srl	%[cc],28\n"
+		: [cc] "=d" (cc)
+		: [r1] "a" (r1), [r2] "a" (r2)
+		: "memory", "cc");
+	return cc;
+}
+
+#endif
s390x/Makefile +1 -0
index 0f54bf4..c2213ad 100644
@@ -18,6 +18,7 @@ tests += $(TEST_DIR)/skrf.elf
 tests += $(TEST_DIR)/smp.elf
 tests += $(TEST_DIR)/sclp.elf
 tests += $(TEST_DIR)/css.elf
+tests += $(TEST_DIR)/uv-guest.elf
 tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
 all: directories test_cases test_cases_binary
s390x/unittests.cfg +3 -0
index b35269b..38c3257 100644
@@ -92,3 +92,6 @@ extra_params = -device virtio-net-ccw
 [skrf]
 file = skrf.elf
 smp = 2
+
+[uv-guest]
+file = uv-guest.elf
\ No newline at end of file
s390x/uv-guest.c new file +9 -0
new file mode 100644
index 0000000..0cb5fae
@@ -0,0 +1,156 @@
+/*
+ * Guest Ultravisor Call tests
+ *
+ * Copyright (c) 2020 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <[email protected]>
+ *
+ * This code is free software; you can redistribute it and/or modify

it
+ * under the terms of the GNU General Public License version 2.
+ */

s390x/uv-guest.c new file +77 -0
+
+#include <libcflat.h>
+#include <alloc_page.h>
+#include <asm/page.h>
+#include <asm/asm-offsets.h>
+#include <asm/interrupt.h>
+#include <asm/facility.h>
+#include <asm/uv.h>
+
+static inline int share(unsigned long addr, u16 cmd)
+{
+	struct uv_cb_share uvcb = {
+		.header.cmd = cmd,
+		.header.len = sizeof(uvcb),
+		.paddr = addr
+	};
+
+	uv_call(0, (u64)&uvcb);
+	return uvcb.header.rc;
+}
+
+static inline int uv_set_shared(unsigned long addr)
+{
+	return share(addr, UVC_CMD_SET_SHARED_ACCESS);
+}
+
+static inline int uv_remove_shared(unsigned long addr)
+{
+	return share(addr, UVC_CMD_REMOVE_SHARED_ACCESS);
+}
+
+static void test_priv(void)
+{
+	struct uv_cb_header uvcb = {};
+
+	report_prefix_push("privileged");
+
+	report_prefix_push("query");
+	expect_pgm_int();
+	uvcb.cmd = UVC_CMD_QUI;
+	uvcb.len = sizeof(struct uv_cb_qui);
+	enter_pstate();
+	uv_call(0, (u64)&uvcb);
+	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+	report_prefix_pop();
+
+	report_prefix_push("share");
+	expect_pgm_int();
+	enter_pstate();
+	uv_set_shared(0x42000);
+	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+	report_prefix_pop();
+
+	report_prefix_push("unshare");
+	expect_pgm_int();
+	enter_pstate();
+	uv_remove_shared(0x42000);
+	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+	report_prefix_pop();
+
+	report_prefix_pop();
+}
+
+static void test_query(void)
+{
+	struct uv_cb_qui uvcb = {
+		.header.cmd = UVC_CMD_QUI,
+		.header.len = sizeof(uvcb) - 8,
+	};
+
+	report_prefix_push("query");
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_INV_LEN, "length");
+
+	uvcb.header.len = sizeof(uvcb);
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_EXECUTED, "successful

query"); +

s390x/uv-guest.c new file +6 -0
+	/*
+	 * These bits have been introduced with the very first
+	 * Ultravisor version and are expected to always be available
+	 * because they are basic building blocks.
+	 */
+	report(uvcb.inst_calls_list[0] & (1UL << (63 -

BIT_UVC_CMD_QUI)),

s390x/uv-guest.c new file +2 -0
+	       "query indicated");
+	report(uvcb.inst_calls_list[0] & (1UL << (63 -

BIT_UVC_CMD_SET_SHARED_ACCESS)),

s390x/uv-guest.c new file +2 -0
+	       "share indicated");
+	report(uvcb.inst_calls_list[0] & (1UL << (63 -

BIT_UVC_CMD_REMOVE_SHARED_ACCESS)),

s390x/uv-guest.c new file +47 -0
+	       "unshare indicated");
+	report_prefix_pop();
+}
+
+static void test_sharing(void)
+{
+	unsigned long mem = (unsigned long)alloc_page();
+	struct uv_cb_share uvcb = {
+		.header.cmd = UVC_CMD_SET_SHARED_ACCESS,
+		.header.len = sizeof(uvcb) - 8,
+	};
+
+	report_prefix_push("share");
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_INV_LEN, "length");
+	report(uv_set_shared(mem) == UVC_RC_EXECUTED, "share");
+	report_prefix_pop();
+
+	report_prefix_push("unshare");
+	uvcb.header.cmd = UVC_CMD_REMOVE_SHARED_ACCESS;
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.header.rc == UVC_RC_INV_LEN, "length");
+	report(uv_remove_shared(mem) == UVC_RC_EXECUTED, "unshare");
+	free_page((void *)mem);
+	report_prefix_pop();
+
+	report_prefix_pop();
+}
+
+static void test_invalid(void)
+{
+	struct uv_cb_header uvcb = {
+		.len = 16,
+		.cmd = 0x4242,
+	};
+
+	uv_call(0, (u64)&uvcb);
+	report(uvcb.rc == UVC_RC_INV_CMD, "invalid command");
+}
+
+int main(void)
+{
+	bool has_uvc = test_facility(158);
+
+	report_prefix_push("uvc");
+	if (!has_uvc) {
+		report_skip("Ultravisor call facility is not

available");

s390x/uv-guest.c new file +10 -0
+		goto done;
+	}
+
+	test_priv();
+	test_invalid();
+	test_query();
+	test_sharing();
+done:
+	return report_summary();
+}