Skip to contentSkip to conversation

PV tests part 1

PATCH In review Janosch Frank 3 participants 15 messages Aug 7, 2020 – Aug 11, 2020

Subsystems (2)
KVM s390
lore
Share & tools
lore

Janosch Frank

author
#0 lore raw

v2 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 is that if such a PSW is loaded on an exception it will result in a specification exception and not a special operation exception.

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://gitlab.com/frankja/kvm-unit-tests/-/tree/queue

v2:
* Renamed pgm_int_func to pgm_cleanup_func() and moved the call to handle_pgm_int()
* Added page allocation to UV test
* Cleanups

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

 lib/s390x/asm/interrupt.h |   1 +
 lib/s390x/asm/uv.h        |  74 +++++++++++++++++
 lib/s390x/interrupt.c     |  12 ++-
 s390x/Makefile            |   1 +
 s390x/skrf.c              |  79 +++++++++++++++++++
 s390x/unittests.cfg       |   7 ++
 s390x/uv-guest.c          | 162 ++++++++++++++++++++++++++++++++++++++
 7 files changed, 335 insertions(+), 1 deletion(-)
 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 v2 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 let's add a way to register a cleanup function.

Signed-off-by Janosch Frank <[email protected]>
Reviewed-by Claudio Imbrenda <[email protected]>
Reviewed-by Thomas Huth <[email protected]>
---
 lib/s390x/asm/interrupt.h |  1 +
 lib/s390x/interrupt.c     | 12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)
lib/s390x/asm/interrupt.h +1 -0
index 4cfade9..2772e6b 100644
@@ -15,6 +15,7 @@
 #define EXT_IRQ_EXTERNAL_CALL	0x1202
 #define EXT_IRQ_SERVICE_SIG	0x2401
 
+void register_pgm_cleanup_func(void (*f)(void));
 void handle_pgm_int(void);
 void handle_ext_int(void);
 void handle_mcck_int(void);
lib/s390x/interrupt.c +11 -1
index 243b9c2..a074505 100644
@@ -16,6 +16,7 @@
 
 static bool pgm_int_expected;
 static bool ext_int_expected;
+static void (*pgm_cleanup_func)(void);
 static struct lowcore *lc;
 
 void expect_pgm_int(void)

@@ -51,6 +52,11 @@ void check_pgm_int_code(uint16_t code)
 	       lc->pgm_int_code);
 }
 
+void register_pgm_cleanup_func(void (*f)(void))
+{
+	pgm_cleanup_func = f;
+}
+
 static void fixup_pgm_int(void)
 {
 	switch (lc->pgm_int_code) {

@@ -115,7 +121,11 @@ void handle_pgm_int(void)
 	}
 
 	pgm_int_expected = false;
-	fixup_pgm_int();
+
+	if (pgm_cleanup_func)
+		(*pgm_cleanup_func)();
+	else
+		fixup_pgm_int();
 }
 
 void handle_ext_int(void)
Signature · 1 line
2.25.1
Reviewed-by on v2 1/3
#2 lore raw

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

Quoted 10 lines · On Fri, 7 Aug 2020 07:15:53 -0400 Janosch Frank <[email protected]> wrote:

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

Signed-off-by Janosch Frank <[email protected]>
Reviewed-by Claudio Imbrenda <[email protected]>
Reviewed-by Thomas Huth <[email protected]>
---
 lib/s390x/asm/interrupt.h |  1 +
 lib/s390x/interrupt.c     | 12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)
Reviewed-by Cornelia Huck <[email protected]>
author
#3 lore raw

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

In reply to Janosch Frank

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

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

Signed-off-by Janosch Frank <[email protected]>
Reviewed-by Claudio Imbrenda <[email protected]>
---
 s390x/skrf.c        | 79 +++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |  4 +++
 2 files changed, 83 insertions(+)
s390x/skrf.c +79 -0
index 9cae589..b19d0f4 100644
@@ -11,12 +11,16 @@
  */
 #include <libcflat.h>
 #include <asm/asm-offsets.h>
+#include <asm-generic/barrier.h>
 #include <asm/interrupt.h>
 #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)));
+static int testflag = 0;
 
 static void test_facilities(void)
 {

@@ -106,6 +110,80 @@ static void test_tprot(void)
 	report_prefix_pop();
 }
 
+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 operation exception on the lpswe
+	 * of pgm old.
+	 */
+	lc->pgm_old_psw.mask = 0x0000000180000000UL;
+
+	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_cleanup_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 +199,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 v2 2/3
#4 lore raw

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

Quoted 5 lines · Janosch Frank

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

Quoted 8 lines

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

Signed-off-by Janosch Frank <[email protected]>
Reviewed-by Claudio Imbrenda <[email protected]>
---
 s390x/skrf.c        | 79 +++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |  4 +++
 2 files changed, 83 insertions(+)

It's hot here today, so my review skills might have melted a little bit, but as far as I can see, it looks fine now:

Reviewed-by Thomas Huth <[email protected]>
Acked-by on v2 2/3
#5 lore raw

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

In reply to Janosch Frank
Quoted 5 lines · On Fri, 7 Aug 2020 07:15:54 -0400 Janosch Frank <[email protected]> wrote:

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

Quoted 8 lines

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

Signed-off-by Janosch Frank <[email protected]>
Reviewed-by Claudio Imbrenda <[email protected]>
---
 s390x/skrf.c        | 79 +++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |  4 +++
 2 files changed, 83 insertions(+)

Didn't review deeply.

Acked-by Cornelia Huck <[email protected]>
author
#6 lore raw

PATCH v2 3/3 s390x: Ultravisor 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  |  74 ++++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 162 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 240 insertions(+)
 create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c
lib/s390x/asm/uv.h new file +74 -0
new file mode 100644
index 0000000..4c2fc48
@@ -0,0 +1,74 @@
+/*
+ * 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;
+
+	/*
+	 * The brc instruction will take care of the cc 2/3 case where
+	 * we need to continue the execution because we were
+	 * interrupted. The inline assembly will only return on
+	 * success/error i.e. cc 0/1.
+	*/
+	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..6d50c63 100644
@@ -92,3 +92,6 @@ extra_params = -device virtio-net-ccw
 [skrf]
 file = skrf.elf
 smp = 2
+
+[uv-guest]
+file = uv-guest.elf
s390x/uv-guest.c new file +162 -0

162 lines not on this page · raw message

Signature · 1 line
2.25.1
Acked-by on v2 3/3
#7 lore raw

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

Quoted 7 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]>
Quoted 8 lines
---
 lib/s390x/asm/uv.h  |  74 ++++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 162 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 240 insertions(+)
 create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.
Acked-by Thomas Huth <[email protected]>
#8 lore raw

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

In reply to Janosch Frank
Quoted 15 lines · On Fri, 7 Aug 2020 07:15:55 -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]>
Reviewed-by Claudio Imbrenda <[email protected]>
---
 lib/s390x/asm/uv.h  |  74 ++++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 162 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 240 insertions(+)
 create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c

(...)

lib/s390x/asm/uv.h line 62

Quoted 10 lines
lib/s390x/asm/uv.h new file +10 -0
+static inline int uv_call(unsigned long r1, unsigned long r2)
+{
+	int cc;
+
+	/*
+	 * The brc instruction will take care of the cc 2/3 case where
+	 * we need to continue the execution because we were
+	 * interrupted. The inline assembly will only return on
+	 * success/error i.e. cc 0/1.
+	*/

Thanks, that is helpful.

Quoted 12 lines
lib/s390x/asm/uv.h new file +12 -0
+	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

(...)

Quoted 28 lines
s390x/uv-guest.c new file +22 -0
new file mode 100644
index 0000000..1aaf7ca
@@ -0,0 +1,162 @@
+/*
+ * 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.
+ */
+
+#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 unsigned long page;
+

s390x/uv-guest.c line 32

Quoted 10 lines
s390x/uv-guest.c new file +10 -0
+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;

Any reason why you're not checking rc and cc here...

Quoted 11 lines
s390x/uv-guest.c new file +11 -0
+}
+
+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);
+}

(...)

s390x/uv-guest.c line 117

Quoted 11 lines
s390x/uv-guest.c new file +11 -0
+static void test_sharing(void)
+{
+	struct uv_cb_share uvcb = {
+		.header.cmd = UVC_CMD_SET_SHARED_ACCESS,
+		.header.len = sizeof(uvcb) - 8,
+	};
+	int cc;
+
+	report_prefix_push("share");
+	cc = uv_call(0, (u64)&uvcb);
+	report(cc == 1 && uvcb.header.rc == UVC_RC_INV_LEN, "length");

...while you do it for this command (as for all the others)?

s390x/uv-guest.c line 118

Quoted 1 line
s390x/uv-guest.c new file +1 -0
+	report(uv_set_shared(page) == UVC_RC_EXECUTED, "share");

So, is that one of the cases where something is actually indicated in rc on success? Or does cc=0/1 have a different meaning for these calls?

Quoted 11 lines
s390x/uv-guest.c new file +11 -0
+	report_prefix_pop();
+
+	report_prefix_push("unshare");
+	uvcb.header.cmd = UVC_CMD_REMOVE_SHARED_ACCESS;
+	cc = uv_call(0, (u64)&uvcb);
+	report(cc == 1 && uvcb.header.rc == UVC_RC_INV_LEN, "length");
+	report(uv_remove_shared(page) == UVC_RC_EXECUTED, "unshare");
+	report_prefix_pop();
+
+	report_prefix_pop();
+}

(...)

author
#9 lore raw

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

Quoted 81 lines · Cornelia Huck
Quoted 15 lines · On Fri, 7 Aug 2020 07:15:55 -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]>
Reviewed-by Claudio Imbrenda <[email protected]>
---
 lib/s390x/asm/uv.h  |  74 ++++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 162 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 240 insertions(+)
 create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c

(...)

lib/s390x/asm/uv.h line 62

Quoted 10 lines
lib/s390x/asm/uv.h new file +10 -0
+static inline int uv_call(unsigned long r1, unsigned long r2)
+{
+	int cc;
+
+	/*
+	 * The brc instruction will take care of the cc 2/3 case where
+	 * we need to continue the execution because we were
+	 * interrupted. The inline assembly will only return on
+	 * success/error i.e. cc 0/1.
+	*/

Thanks, that is helpful.

Quoted 12 lines
lib/s390x/asm/uv.h new file +12 -0
+	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

(...)

Quoted 32 lines
s390x/uv-guest.c new file +26 -0
new file mode 100644
index 0000000..1aaf7ca
@@ -0,0 +1,162 @@
+/*
+ * 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.
+ */
+
+#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 unsigned long page;
+
+static inline int share(unsigned long addr, u16 cmd)
+{
+	struct uv_cb_share uvcb = {
+		.header.cmd = cmd,
Quoted 8 lines

s390x/uv-guest.c line 32

Quoted 6 lines
s390x/uv-guest.c new file +6 -0
+		.header.len = sizeof(uvcb),
+		.paddr = addr
+	};
+
+	uv_call(0, (u64)&uvcb);
+	return uvcb.header.rc;

Any reason why you're not checking rc and cc here...

Well, this is a helper function not a test function.
Since I can only return one value and since I'm lazy, I chose to ignore the CC and went for the uvcb rc. That's basically also the answer for your following questions.

Alright, I'll remove the helpers and execute those tests the hard way.

Quoted 48 lines
Quoted 12 lines
s390x/uv-guest.c new file +11 -0
+}
+
+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);
+}

(...)

s390x/uv-guest.c line 117

Quoted 11 lines
s390x/uv-guest.c new file +11 -0
+static void test_sharing(void)
+{
+	struct uv_cb_share uvcb = {
+		.header.cmd = UVC_CMD_SET_SHARED_ACCESS,
+		.header.len = sizeof(uvcb) - 8,
+	};
+	int cc;
+
+	report_prefix_push("share");
+	cc = uv_call(0, (u64)&uvcb);
+	report(cc == 1 && uvcb.header.rc == UVC_RC_INV_LEN, "length");

...while you do it for this command (as for all the others)?

s390x/uv-guest.c line 118

Quoted 1 line
s390x/uv-guest.c new file +1 -0
+	report(uv_set_shared(page) == UVC_RC_EXECUTED, "share");

So, is that one of the cases where something is actually indicated in rc on success? Or does cc=0/1 have a different meaning for these calls?

Quoted 11 lines
s390x/uv-guest.c new file +11 -0
+	report_prefix_pop();
+
+	report_prefix_push("unshare");
+	uvcb.header.cmd = UVC_CMD_REMOVE_SHARED_ACCESS;
+	cc = uv_call(0, (u64)&uvcb);
+	report(cc == 1 && uvcb.header.rc == UVC_RC_INV_LEN, "length");
+	report(uv_remove_shared(page) == UVC_RC_EXECUTED, "unshare");
+	report_prefix_pop();
+
+	report_prefix_pop();
+}

(...)

#10 lore raw

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

Quoted 17 lines · On Mon, 10 Aug 2020 17:27:36 +0200 Janosch Frank <[email protected]> wrote:
Quoted 15 lines · Cornelia Huck

s390x/uv-guest.c line 32

Quoted 10 lines · On Fri, 7 Aug 2020 07:15:55 -0400 Janosch Frank <[email protected]> wrote:
s390x/uv-guest.c new file +10 -0
+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;

Any reason why you're not checking rc and cc here...

Quoted 4 lines

Well, this is a helper function not a test function.
Since I can only return one value and since I'm lazy, I chose to ignore the CC and went for the uvcb rc. That's basically also the answer for your following questions.

Maybe I'm just confused regarding the command execution here.

Quoted 3 lines

Alright, I'll remove the helpers and execute those tests the hard way.

As a plus point, you see exactly what is being done.

author
#11 lore raw

PATCH v3 1/1 s390x: Ultravisor guest API test

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]>
Acked-by Thomas Huth <[email protected]>
Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm/uv.h  |  74 ++++++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 150 ++++++++++++++++++++++++++++++++++++++++++++
 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 +74 -0
new file mode 100644
index 0000000..4c2fc48
@@ -0,0 +1,74 @@
+/*
+ * 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;
+
+	/*
+	 * The brc instruction will take care of the cc 2/3 case where
+	 * we need to continue the execution because we were
+	 * interrupted. The inline assembly will only return on
+	 * success/error i.e. cc 0/1.
+	*/
+	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..6d50c63 100644
@@ -92,3 +92,6 @@ extra_params = -device virtio-net-ccw
 [skrf]
 file = skrf.elf
 smp = 2
+
+[uv-guest]
+file = uv-guest.elf
s390x/uv-guest.c new file +150 -0

150 lines not on this page · raw message

Signature · 1 line
2.25.1
Acked-by on v3 1/1
#12 lore raw

Re: [kvm-unit-tests PATCH v3] s390x: Ultravisor guest API test

Quoted 9 lines · On Mon, 10 Aug 2020 11:45:41 -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]>
Reviewed-by Claudio Imbrenda <[email protected]>
Acked-by Thomas Huth <[email protected]>
Signed-off-by Janosch Frank <[email protected]>
Quoted 8 lines
---
 lib/s390x/asm/uv.h  |  74 ++++++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 150 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 228 insertions(+)
 create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c
Acked-by Cornelia Huck <[email protected]>
author
#13 lore raw

Re: [kvm-unit-tests PATCH v3] s390x: Ultravisor guest API test

Quoted 21 lines · Cornelia Huck
Quoted 9 lines · On Mon, 10 Aug 2020 11:45:41 -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]>
Reviewed-by Claudio Imbrenda <[email protected]>
Acked-by Thomas Huth <[email protected]>
Signed-off-by Janosch Frank <[email protected]>
Quoted 8 lines
---
 lib/s390x/asm/uv.h  |  74 ++++++++++++++++++++++
 s390x/Makefile      |   1 +
 s390x/unittests.cfg |   3 +
 s390x/uv-guest.c    | 150 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 228 insertions(+)
 create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c
Quoted 2 lines
Acked-by Cornelia Huck <[email protected]>

Thanks!

author
#14 lore raw
In reply to Janosch Frank
Quoted 7 lines · Janosch Frank

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 is that if such a PSW is loaded on an exception it will result in a specification exception and not a special operation exception.

Quoted 6 lines

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://gitlab.com/frankja/kvm-unit-tests/-/tree/queue

Picked the series onto:
https://gitlab.com/frankja/kvm-unit-tests/-/commits/next

Quoted 23 lines

v2:
* Renamed pgm_int_func to pgm_cleanup_func() and moved the call to handle_pgm_int()
* Added page allocation to UV test
* Cleanups

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

 lib/s390x/asm/interrupt.h |   1 +
 lib/s390x/asm/uv.h        |  74 +++++++++++++++++
 lib/s390x/interrupt.c     |  12 ++-
 s390x/Makefile            |   1 +
 s390x/skrf.c              |  79 +++++++++++++++++++
 s390x/unittests.cfg       |   7 ++
 s390x/uv-guest.c          | 162 ++++++++++++++++++++++++++++++++++++++
 7 files changed, 335 insertions(+), 1 deletion(-)
 create mode 100644 lib/s390x/asm/uv.h
 create mode 100644 s390x/uv-guest.c