All of lore.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH 0/6] s390x: uv-host: Access check extensions and improvements
@ 2022-05-13  9:50 Janosch Frank
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 1/6] s390x: uv-host: Add access checks for donated memory Janosch Frank
                   ` (5 more replies)
  0 siblings, 6 replies; 21+ messages in thread
From: Janosch Frank @ 2022-05-13  9:50 UTC (permalink / raw)
  To: kvm390 mailing list; +Cc: kvm, linux-s390, imbrenda, thuth, seiden, nrb, scgl

Over the last few weeks I had a few ideas on how to extend the uv-host
test to get more coverage. Most checks are access checks for secure
pages or the UVCB and it's satellites.

I've had limited time to cleanup this series, so consider having a
closer look.

Janosch Frank (6):
  s390x: uv-host: Add access checks for donated memory
  s390x: uv-host: Add uninitialized UV tests
  s390x: uv-host: Test uv immediate parameter
  s390x: uv-host: Add access exception test
  s390x: uv-host: Add a set secure config parameters test function
  s390x: uv-host: Remove duplicated +

 s390x/uv-host.c | 244 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 239 insertions(+), 5 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [kvm-unit-tests PATCH 1/6] s390x: uv-host: Add access checks for donated memory
  2022-05-13  9:50 [kvm-unit-tests PATCH 0/6] s390x: uv-host: Access check extensions and improvements Janosch Frank
@ 2022-05-13  9:50 ` Janosch Frank
  2022-05-16  8:21   ` Nico Boehr
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 2/6] s390x: uv-host: Add uninitialized UV tests Janosch Frank
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Janosch Frank @ 2022-05-13  9:50 UTC (permalink / raw)
  To: kvm390 mailing list; +Cc: kvm, linux-s390, imbrenda, thuth, seiden, nrb, scgl

Let's check if the UV really protected all the memory we donated.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/uv-host.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/s390x/uv-host.c b/s390x/uv-host.c
index a1a6d120..0f0b18a1 100644
--- a/s390x/uv-host.c
+++ b/s390x/uv-host.c
@@ -142,7 +142,8 @@ static void test_cpu_destroy(void)
 static void test_cpu_create(void)
 {
 	int rc;
-	unsigned long tmp;
+	unsigned long tmp, i;
+	uint8_t *access_ptr;
 
 	report_prefix_push("csc");
 	uvcb_csc.header.len = sizeof(uvcb_csc);
@@ -194,6 +195,18 @@ static void test_cpu_create(void)
 	report(rc == 0 && uvcb_csc.header.rc == UVC_RC_EXECUTED &&
 	       uvcb_csc.cpu_handle, "success");
 
+	rc = 1;
+	for (i = 0; i < uvcb_qui.cpu_stor_len / PAGE_SIZE; i++) {
+		expect_pgm_int();
+		access_ptr = (void *)uvcb_csc.stor_origin + PAGE_SIZE * i;
+		*access_ptr = 42;
+		if (clear_pgm_int() != PGM_INT_CODE_SECURE_STOR_ACCESS) {
+			rc = 0;
+			break;
+		}
+	}
+	report(rc, "Storage protection");
+
 	tmp = uvcb_csc.stor_origin;
 	uvcb_csc.stor_origin = (unsigned long)memalign(PAGE_SIZE, uvcb_qui.cpu_stor_len);
 	rc = uv_call(0, (uint64_t)&uvcb_csc);
@@ -205,8 +218,9 @@ static void test_cpu_create(void)
 static void test_config_create(void)
 {
 	int rc;
-	unsigned long vsize, tmp;
+	unsigned long vsize, tmp, i;
 	static struct uv_cb_cgc uvcb;
+	uint8_t *access_ptr;
 
 	uvcb_cgc.header.cmd = UVC_CMD_CREATE_SEC_CONF;
 	uvcb_cgc.header.len = sizeof(uvcb_cgc);
@@ -292,6 +306,30 @@ static void test_config_create(void)
 	rc = uv_call(0, (uint64_t)&uvcb_cgc);
 	report(rc == 0 && uvcb_cgc.header.rc == UVC_RC_EXECUTED, "successful");
 
+	rc = 1;
+	for (i = 0; i < vsize / PAGE_SIZE; i++) {
+		expect_pgm_int();
+		access_ptr = (void *)uvcb_cgc.conf_var_stor_origin + PAGE_SIZE * i;
+		*access_ptr = 42;
+		if (clear_pgm_int() != PGM_INT_CODE_SECURE_STOR_ACCESS) {
+			rc = 0;
+			break;
+		}
+	}
+	report(rc, "Base storage protection");
+
+	rc = 1;
+	for (i = 0; i < uvcb_qui.conf_base_phys_stor_len / PAGE_SIZE; i++) {
+		expect_pgm_int();
+		access_ptr = (void *)uvcb_cgc.conf_base_stor_origin + PAGE_SIZE * i;
+		*access_ptr = 42;
+		if (clear_pgm_int() != PGM_INT_CODE_SECURE_STOR_ACCESS) {
+			rc = 0;
+			break;
+		}
+	}
+	report(rc, "Variable storage protection");
+
 	uvcb_cgc.header.rc = 0;
 	uvcb_cgc.header.rrc = 0;
 	tmp = uvcb_cgc.guest_handle;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [kvm-unit-tests PATCH 2/6] s390x: uv-host: Add uninitialized UV tests
  2022-05-13  9:50 [kvm-unit-tests PATCH 0/6] s390x: uv-host: Access check extensions and improvements Janosch Frank
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 1/6] s390x: uv-host: Add access checks for donated memory Janosch Frank
@ 2022-05-13  9:50 ` Janosch Frank
  2022-05-16  8:37   ` Nico Boehr
  2022-05-16 15:02   ` Steffen Eiden
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 3/6] s390x: uv-host: Test uv immediate parameter Janosch Frank
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 21+ messages in thread
From: Janosch Frank @ 2022-05-13  9:50 UTC (permalink / raw)
  To: kvm390 mailing list; +Cc: kvm, linux-s390, imbrenda, thuth, seiden, nrb, scgl

Let's also test for rc 0x3

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/uv-host.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 2 deletions(-)

diff --git a/s390x/uv-host.c b/s390x/uv-host.c
index 0f0b18a1..f846fc42 100644
--- a/s390x/uv-host.c
+++ b/s390x/uv-host.c
@@ -83,6 +83,24 @@ static void test_priv(void)
 	report_prefix_pop();
 }
 
+static void test_uv_uninitialized(void)
+{
+	struct uv_cb_header uvcb = {};
+	int i;
+
+	report_prefix_push("uninitialized");
+
+	/* i = 1 to skip over initialize */
+	for (i = 1; cmds[i].name; i++) {
+		expect_pgm_int();
+		uvcb.cmd = cmds[i].cmd;
+		uvcb.len = cmds[i].len;
+		uv_call_once(0, (uint64_t)&uvcb);
+		report(uvcb.rc == UVC_RC_INV_STATE, "%s", cmds[i].name);
+	}
+	report_prefix_pop();
+}
+
 static void test_config_destroy(void)
 {
 	int rc;
@@ -477,13 +495,68 @@ static void test_invalid(void)
 	report_prefix_pop();
 }
 
+static void test_clear_setup(void)
+{
+	unsigned long vsize;
+	int rc;
+
+	uvcb_cgc.header.cmd = UVC_CMD_CREATE_SEC_CONF;
+	uvcb_cgc.header.len = sizeof(uvcb_cgc);
+
+	uvcb_cgc.guest_stor_origin = 0;
+	uvcb_cgc.guest_stor_len = 42 * (1UL << 20);
+	vsize = uvcb_qui.conf_base_virt_stor_len +
+		((uvcb_cgc.guest_stor_len / (1UL << 20)) * uvcb_qui.conf_virt_var_stor_len);
+
+	uvcb_cgc.conf_base_stor_origin = (uint64_t)memalign(PAGE_SIZE * 4, uvcb_qui.conf_base_phys_stor_len);
+	uvcb_cgc.conf_var_stor_origin = (uint64_t)memalign(PAGE_SIZE, vsize);
+	uvcb_cgc.guest_asce = (uint64_t)memalign(PAGE_SIZE, 4 * PAGE_SIZE) | ASCE_DT_SEGMENT | REGION_TABLE_LENGTH | ASCE_P;
+	uvcb_cgc.guest_sca = (uint64_t)memalign(PAGE_SIZE * 4, PAGE_SIZE * 4);
+
+	rc = uv_call(0, (uint64_t)&uvcb_cgc);
+	assert(rc == 0);
+
+	uvcb_csc.header.len = sizeof(uvcb_csc);
+	uvcb_csc.header.cmd = UVC_CMD_CREATE_SEC_CPU;
+	uvcb_csc.guest_handle = uvcb_cgc.guest_handle;
+	uvcb_csc.stor_origin = (unsigned long)memalign(PAGE_SIZE, uvcb_qui.cpu_stor_len);
+	uvcb_csc.state_origin = (unsigned long)memalign(PAGE_SIZE, PAGE_SIZE);
+
+	rc = uv_call(0, (uint64_t)&uvcb_csc);
+	assert(rc == 0);
+}
+
 static void test_clear(void)
 {
-	uint64_t *tmp = (void *)uvcb_init.stor_origin;
+	uint64_t *tmp;
+
+	report_prefix_push("load normal reset");
+
+	/*
+	 * Setup a config and a cpu so we can check if a diag308 reset
+	 * clears the donated memory and makes the pages unsecure.
+	 */
+	test_clear_setup();
 
 	diag308_load_reset(1);
 	sclp_console_setup();
-	report(!*tmp, "memory cleared after reset 1");
+
+	tmp = (void *)uvcb_init.stor_origin;
+	report(!*tmp, "uv init donated memory cleared");
+
+	tmp = (void *)uvcb_cgc.conf_base_stor_origin;
+	report(!*tmp, "config base donated memory cleared");
+
+	tmp = (void *)uvcb_cgc.conf_base_stor_origin;
+	report(!*tmp, "config variable donated memory cleared");
+
+	tmp = (void *)uvcb_csc.stor_origin;
+	report(!*tmp, "cpu donated memory cleared after reset 1");
+
+	/* Check if uninitialized after reset */
+	test_uv_uninitialized();
+
+	report_prefix_pop();
 }
 
 static void setup_vmem(void)
@@ -514,6 +587,7 @@ int main(void)
 
 	test_priv();
 	test_invalid();
+	test_uv_uninitialized();
 	test_query();
 	test_init();
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [kvm-unit-tests PATCH 3/6] s390x: uv-host: Test uv immediate parameter
  2022-05-13  9:50 [kvm-unit-tests PATCH 0/6] s390x: uv-host: Access check extensions and improvements Janosch Frank
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 1/6] s390x: uv-host: Add access checks for donated memory Janosch Frank
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 2/6] s390x: uv-host: Add uninitialized UV tests Janosch Frank
@ 2022-05-13  9:50 ` Janosch Frank
  2022-05-17  8:29   ` Steffen Eiden
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 4/6] s390x: uv-host: Add access exception test Janosch Frank
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Janosch Frank @ 2022-05-13  9:50 UTC (permalink / raw)
  To: kvm390 mailing list; +Cc: kvm, linux-s390, imbrenda, thuth, seiden, nrb, scgl

Let's check if we get a specification PGM exception if we set a
non-zero i3 when doing a UV call.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/uv-host.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/s390x/uv-host.c b/s390x/uv-host.c
index f846fc42..fcb82d24 100644
--- a/s390x/uv-host.c
+++ b/s390x/uv-host.c
@@ -64,6 +64,28 @@ static struct cmd_list cmds[] = {
 	{ NULL, 0, 0 },
 };
 
+static void test_i3(void)
+{
+	struct uv_cb_header uvcb = {
+		.cmd = UVC_CMD_INIT_UV,
+		.len = sizeof(struct uv_cb_init),
+	};
+	unsigned long r1 = 0;
+	int cc;
+
+	report_prefix_push("i3");
+	expect_pgm_int();
+	asm volatile(
+		"0:	.insn rrf,0xB9A40000,%[r1],%[r2],4,2\n"
+		"		ipm	%[cc]\n"
+		"		srl	%[cc],28\n"
+		: [cc] "=d" (cc)
+		: [r1] "a" (r1), [r2] "a" (&uvcb)
+		: "memory", "cc");
+	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
+	report_prefix_pop();
+}
+
 static void test_priv(void)
 {
 	struct uv_cb_header uvcb = {};
@@ -585,6 +607,7 @@ int main(void)
 		goto done;
 	}
 
+	test_i3();
 	test_priv();
 	test_invalid();
 	test_uv_uninitialized();
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [kvm-unit-tests PATCH 4/6] s390x: uv-host: Add access exception test
  2022-05-13  9:50 [kvm-unit-tests PATCH 0/6] s390x: uv-host: Access check extensions and improvements Janosch Frank
                   ` (2 preceding siblings ...)
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 3/6] s390x: uv-host: Test uv immediate parameter Janosch Frank
@ 2022-05-13  9:50 ` Janosch Frank
  2022-05-16  8:42   ` Nico Boehr
  2022-05-17  8:53   ` Steffen Eiden
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 5/6] s390x: uv-host: Add a set secure config parameters test function Janosch Frank
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 6/6] s390x: uv-host: Remove duplicated + Janosch Frank
  5 siblings, 2 replies; 21+ messages in thread
From: Janosch Frank @ 2022-05-13  9:50 UTC (permalink / raw)
  To: kvm390 mailing list; +Cc: kvm, linux-s390, imbrenda, thuth, seiden, nrb, scgl

Let's check that we get access exceptions if the UVCB is on an invalid
page or starts at a valid page and crosses into an invalid one.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/uv-host.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/s390x/uv-host.c b/s390x/uv-host.c
index fcb82d24..153a94e1 100644
--- a/s390x/uv-host.c
+++ b/s390x/uv-host.c
@@ -15,12 +15,14 @@
 #include <sclp.h>
 #include <smp.h>
 #include <uv.h>
+#include <mmu.h>
 #include <asm/page.h>
 #include <asm/sigp.h>
 #include <asm/pgtable.h>
 #include <asm/asm-offsets.h>
 #include <asm/interrupt.h>
 #include <asm/facility.h>
+#include <asm/pgtable.h>
 #include <asm/uv.h>
 #include <asm-generic/barrier.h>
 
@@ -123,6 +125,54 @@ static void test_uv_uninitialized(void)
 	report_prefix_pop();
 }
 
+static void test_access(void)
+{
+	struct uv_cb_header *uvcb;
+	void *pages =  alloc_pages(1);
+	uint16_t pgm;
+	int i;
+
+	/* Put UVCB on second page which we will protect later */
+	uvcb = pages + PAGE_SIZE;
+
+	report_prefix_push("access");
+
+	report_prefix_push("non-crossing");
+	protect_page(uvcb, PAGE_ENTRY_I);
+	for (i = 0; cmds[i].name; i++) {
+		expect_pgm_int();
+		mb();
+		uv_call_once(0, (uint64_t)uvcb);
+		pgm = clear_pgm_int();
+		report(pgm == PGM_INT_CODE_PAGE_TRANSLATION, "%s", cmds[i].name);
+	}
+	report_prefix_pop();
+
+	report_prefix_push("crossing");
+	/*
+	 * Put the header into the readable page 1, everything after
+	 * the header will be on the second, invalid page.
+	 */
+	uvcb -= 1;
+	for (i = 0; cmds[i].name; i++) {
+		uvcb->cmd = cmds[i].cmd;
+		uvcb->len = cmds[i].len;
+
+		expect_pgm_int();
+		mb();
+		uv_call_once(0, (uint64_t)uvcb);
+		pgm = clear_pgm_int();
+		report(pgm == PGM_INT_CODE_PAGE_TRANSLATION, "%s", cmds[i].name);
+	}
+	report_prefix_pop();
+
+	uvcb += 1;
+	unprotect_page(uvcb, PAGE_ENTRY_I);
+
+	free_pages(pages);
+	report_prefix_pop();
+}
+
 static void test_config_destroy(void)
 {
 	int rc;
@@ -615,6 +665,8 @@ int main(void)
 	test_init();
 
 	setup_vmem();
+	test_access();
+
 	test_config_create();
 	test_cpu_create();
 	test_cpu_destroy();
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [kvm-unit-tests PATCH 5/6] s390x: uv-host: Add a set secure config parameters test function
  2022-05-13  9:50 [kvm-unit-tests PATCH 0/6] s390x: uv-host: Access check extensions and improvements Janosch Frank
                   ` (3 preceding siblings ...)
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 4/6] s390x: uv-host: Add access exception test Janosch Frank
@ 2022-05-13  9:50 ` Janosch Frank
  2022-05-17 11:32   ` Steffen Eiden
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 6/6] s390x: uv-host: Remove duplicated + Janosch Frank
  5 siblings, 1 reply; 21+ messages in thread
From: Janosch Frank @ 2022-05-13  9:50 UTC (permalink / raw)
  To: kvm390 mailing list; +Cc: kvm, linux-s390, imbrenda, thuth, seiden, nrb, scgl

Time for more tests.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/uv-host.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/s390x/uv-host.c b/s390x/uv-host.c
index 153a94e1..20d805b8 100644
--- a/s390x/uv-host.c
+++ b/s390x/uv-host.c
@@ -229,6 +229,52 @@ static void test_cpu_destroy(void)
 	report_prefix_pop();
 }
 
+static void test_set_se_header(void)
+{
+	struct uv_cb_ssc uvcb = {
+		.header.cmd = UVC_CMD_SET_SEC_CONF_PARAMS,
+		.header.len = sizeof(uvcb),
+		.guest_handle = uvcb_cgc.guest_handle,
+		.sec_header_origin = 0,
+		.sec_header_len = 0x1000,
+	};
+	void *pages =  alloc_pages(1);
+	void *inv;
+	int rc;
+
+	report_prefix_push("sscp");
+
+	uvcb.header.len -= 8;
+	rc = uv_call(0, (uint64_t)&uvcb);
+	report(rc == 1 && uvcb.header.rc == UVC_RC_INV_LEN,
+	       "hdr invalid length");
+	uvcb.header.len += 8;
+
+	uvcb.guest_handle += 1;
+	rc = uv_call(0, (uint64_t)&uvcb);
+	report(rc == 1 && uvcb.header.rc == UVC_RC_INV_GHANDLE, "invalid handle");
+	uvcb.guest_handle -= 1;
+
+	inv = pages + PAGE_SIZE;
+	uvcb.sec_header_origin = (uint64_t)inv;
+	protect_page(inv, PAGE_ENTRY_I);
+	rc = uv_call(0, (uint64_t)&uvcb);
+	report(rc == 1 && uvcb.header.rc == 0x103,
+	       "se hdr access exception");
+
+	/*
+	 * Shift the ptr so the first few DWORDs are accessible but
+	 * the following are on an invalid page.
+	 */
+	uvcb.sec_header_origin -= 0x20;
+	rc = uv_call(0, (uint64_t)&uvcb);
+	report(rc == 1 && uvcb.header.rc == 0x103,
+	       "se hdr access exception crossing");
+	unprotect_page(inv, PAGE_ENTRY_I);
+
+	report_prefix_pop();
+}
+
 static void test_cpu_create(void)
 {
 	int rc;
@@ -669,6 +715,7 @@ int main(void)
 
 	test_config_create();
 	test_cpu_create();
+	test_set_se_header();
 	test_cpu_destroy();
 	test_config_destroy();
 	test_clear();
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [kvm-unit-tests PATCH 6/6] s390x: uv-host: Remove duplicated +
  2022-05-13  9:50 [kvm-unit-tests PATCH 0/6] s390x: uv-host: Access check extensions and improvements Janosch Frank
                   ` (4 preceding siblings ...)
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 5/6] s390x: uv-host: Add a set secure config parameters test function Janosch Frank
@ 2022-05-13  9:50 ` Janosch Frank
  2022-05-13 15:30   ` Claudio Imbrenda
  2022-05-17 11:33   ` Steffen Eiden
  5 siblings, 2 replies; 21+ messages in thread
From: Janosch Frank @ 2022-05-13  9:50 UTC (permalink / raw)
  To: kvm390 mailing list; +Cc: kvm, linux-s390, imbrenda, thuth, seiden, nrb, scgl

One + is definitely enough here.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/uv-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/s390x/uv-host.c b/s390x/uv-host.c
index 20d805b8..ed16f850 100644
--- a/s390x/uv-host.c
+++ b/s390x/uv-host.c
@@ -433,7 +433,7 @@ static void test_config_create(void)
 	uvcb_cgc.guest_sca = tmp;
 
 	tmp = uvcb_cgc.guest_sca;
-	uvcb_cgc.guest_sca = get_max_ram_size() + + PAGE_SIZE * 4;
+	uvcb_cgc.guest_sca = get_max_ram_size() + PAGE_SIZE * 4;
 	rc = uv_call(0, (uint64_t)&uvcb_cgc);
 	report(uvcb_cgc.header.rc == 0x10d && rc == 1,
 	       "sca inaccessible");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 6/6] s390x: uv-host: Remove duplicated +
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 6/6] s390x: uv-host: Remove duplicated + Janosch Frank
@ 2022-05-13 15:30   ` Claudio Imbrenda
  2022-05-17 11:33   ` Steffen Eiden
  1 sibling, 0 replies; 21+ messages in thread
From: Claudio Imbrenda @ 2022-05-13 15:30 UTC (permalink / raw)
  To: Janosch Frank
  Cc: kvm390 mailing list, kvm, linux-s390, thuth, seiden, nrb, scgl

On Fri, 13 May 2022 09:50:17 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> One + is definitely enough here.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  s390x/uv-host.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
> index 20d805b8..ed16f850 100644
> --- a/s390x/uv-host.c
> +++ b/s390x/uv-host.c
> @@ -433,7 +433,7 @@ static void test_config_create(void)
>  	uvcb_cgc.guest_sca = tmp;
>  
>  	tmp = uvcb_cgc.guest_sca;
> -	uvcb_cgc.guest_sca = get_max_ram_size() + + PAGE_SIZE * 4;
> +	uvcb_cgc.guest_sca = get_max_ram_size() + PAGE_SIZE * 4;
>  	rc = uv_call(0, (uint64_t)&uvcb_cgc);
>  	report(uvcb_cgc.header.rc == 0x10d && rc == 1,
>  	       "sca inaccessible");


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 1/6] s390x: uv-host: Add access checks for donated memory
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 1/6] s390x: uv-host: Add access checks for donated memory Janosch Frank
@ 2022-05-16  8:21   ` Nico Boehr
  2022-05-16  9:22     ` Janosch Frank
  0 siblings, 1 reply; 21+ messages in thread
From: Nico Boehr @ 2022-05-16  8:21 UTC (permalink / raw)
  To: Janosch Frank, kvm390 mailing list
  Cc: kvm, linux-s390, imbrenda, thuth, seiden, scgl

On Fri, 2022-05-13 at 09:50 +0000, Janosch Frank wrote:
> Let's check if the UV really protected all the memory we donated.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Nico Boehr <nrb@linux.ibm.com>

One suggestion below for you to consider.

> ---
>  s390x/uv-host.c | 42 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
> index a1a6d120..0f0b18a1 100644
> --- a/s390x/uv-host.c
> +++ b/s390x/uv-host.c
> @@ -142,7 +142,8 @@ static void test_cpu_destroy(void)
>  static void test_cpu_create(void)
>  {
>         int rc;
> -       unsigned long tmp;
> +       unsigned long tmp, i;
> +       uint8_t *access_ptr;
>  
>         report_prefix_push("csc");
>         uvcb_csc.header.len = sizeof(uvcb_csc);
> @@ -194,6 +195,18 @@ static void test_cpu_create(void)
>         report(rc == 0 && uvcb_csc.header.rc == UVC_RC_EXECUTED &&
>                uvcb_csc.cpu_handle, "success");
>  
> +       rc = 1;
> +       for (i = 0; i < uvcb_qui.cpu_stor_len / PAGE_SIZE; i++) {
> +               expect_pgm_int();
> +               access_ptr = (void *)uvcb_csc.stor_origin + PAGE_SIZE
> * i;
> +               *access_ptr = 42;
> +               if (clear_pgm_int() !=
> PGM_INT_CODE_SECURE_STOR_ACCESS) {
> +                       rc = 0;
> +                       break;
> +               }
> +       }
> +       report(rc, "Storage protection");

All of these for loops look pretty similar, would it make sense to move
them to their own function like:

assert_range_write_protected(void *start, size_t len, int
pgm_int_code)?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 2/6] s390x: uv-host: Add uninitialized UV tests
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 2/6] s390x: uv-host: Add uninitialized UV tests Janosch Frank
@ 2022-05-16  8:37   ` Nico Boehr
  2022-05-16 15:02   ` Steffen Eiden
  1 sibling, 0 replies; 21+ messages in thread
From: Nico Boehr @ 2022-05-16  8:37 UTC (permalink / raw)
  To: Janosch Frank; +Cc: kvm, linux-s390, imbrenda, thuth, seiden, scgl

On Fri, 2022-05-13 at 09:50 +0000, Janosch Frank wrote:
> Let's also test for rc 0x3
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
if you fix the nit below.

> ---
>  s390x/uv-host.c | 78
> +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
> index 0f0b18a1..f846fc42 100644
> --- a/s390x/uv-host.c
> +++ b/s390x/uv-host.c
> @@ -83,6 +83,24 @@ static void test_priv(void)
>         report_prefix_pop();
>  }
>  
> +static void test_uv_uninitialized(void)
> +{
> +       struct uv_cb_header uvcb = {};
> +       int i;
> +
> +       report_prefix_push("uninitialized");
> +
> +       /* i = 1 to skip over initialize */

Just say

if (cmds[i].cmd == UVC_CMD_INIT_UV)
  continue;

inside the loop.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 4/6] s390x: uv-host: Add access exception test
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 4/6] s390x: uv-host: Add access exception test Janosch Frank
@ 2022-05-16  8:42   ` Nico Boehr
  2022-05-17  8:53   ` Steffen Eiden
  1 sibling, 0 replies; 21+ messages in thread
From: Nico Boehr @ 2022-05-16  8:42 UTC (permalink / raw)
  To: Janosch Frank; +Cc: kvm, linux-s390, imbrenda, thuth, seiden, scgl

On Fri, 2022-05-13 at 09:50 +0000, Janosch Frank wrote:
> Let's check that we get access exceptions if the UVCB is on an
> invalid
> page or starts at a valid page and crosses into an invalid one.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Nico Boehr <nrb@linux.ibm.com>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 1/6] s390x: uv-host: Add access checks for donated memory
  2022-05-16  8:21   ` Nico Boehr
@ 2022-05-16  9:22     ` Janosch Frank
  0 siblings, 0 replies; 21+ messages in thread
From: Janosch Frank @ 2022-05-16  9:22 UTC (permalink / raw)
  To: Nico Boehr, kvm390 mailing list
  Cc: kvm, linux-s390, imbrenda, thuth, seiden, scgl

On 5/16/22 10:21, Nico Boehr wrote:
> On Fri, 2022-05-13 at 09:50 +0000, Janosch Frank wrote:
>> Let's check if the UV really protected all the memory we donated.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> 
> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>

Thanks

> 
> One suggestion below for you to consider.

Sure, I'll rework the loops

> 
>> ---
>>   s390x/uv-host.c | 42 ++++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 40 insertions(+), 2 deletions(-)
>>
>> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
>> index a1a6d120..0f0b18a1 100644
>> --- a/s390x/uv-host.c
>> +++ b/s390x/uv-host.c
>> @@ -142,7 +142,8 @@ static void test_cpu_destroy(void)
>>   static void test_cpu_create(void)
>>   {
>>          int rc;
>> -       unsigned long tmp;
>> +       unsigned long tmp, i;
>> +       uint8_t *access_ptr;
>>   
>>          report_prefix_push("csc");
>>          uvcb_csc.header.len = sizeof(uvcb_csc);
>> @@ -194,6 +195,18 @@ static void test_cpu_create(void)
>>          report(rc == 0 && uvcb_csc.header.rc == UVC_RC_EXECUTED &&
>>                 uvcb_csc.cpu_handle, "success");
>>   
>> +       rc = 1;
>> +       for (i = 0; i < uvcb_qui.cpu_stor_len / PAGE_SIZE; i++) {
>> +               expect_pgm_int();
>> +               access_ptr = (void *)uvcb_csc.stor_origin + PAGE_SIZE
>> * i;
>> +               *access_ptr = 42;
>> +               if (clear_pgm_int() !=
>> PGM_INT_CODE_SECURE_STOR_ACCESS) {
>> +                       rc = 0;
>> +                       break;
>> +               }
>> +       }
>> +       report(rc, "Storage protection");
> 
> All of these for loops look pretty similar, would it make sense to move
> them to their own function like:
> 
> assert_range_write_protected(void *start, size_t len, int
> pgm_int_code)?


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 2/6] s390x: uv-host: Add uninitialized UV tests
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 2/6] s390x: uv-host: Add uninitialized UV tests Janosch Frank
  2022-05-16  8:37   ` Nico Boehr
@ 2022-05-16 15:02   ` Steffen Eiden
  2022-05-16 15:15     ` Janosch Frank
  1 sibling, 1 reply; 21+ messages in thread
From: Steffen Eiden @ 2022-05-16 15:02 UTC (permalink / raw)
  To: Janosch Frank, kvm390 mailing list
  Cc: kvm, linux-s390, imbrenda, thuth, nrb, scgl



On 5/13/22 11:50, Janosch Frank wrote:
> Let's also test for rc 0x3
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
I, however, have some nits below.

> ---
>   s390x/uv-host.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
> index 0f0b18a1..f846fc42 100644
> --- a/s390x/uv-host.c
> +++ b/s390x/uv-host.c
> @@ -83,6 +83,24 @@ static void test_priv(void)
>   	report_prefix_pop();
>   }
>   
> +static void test_uv_uninitialized(void)
> +{
> +	struct uv_cb_header uvcb = {};
> +	int i;
> +
> +	report_prefix_push("uninitialized");
> +
> +	/* i = 1 to skip over initialize */
> +	for (i = 1; cmds[i].name; i++) {
> +		expect_pgm_int();
> +		uvcb.cmd = cmds[i].cmd;
> +		uvcb.len = cmds[i].len;
> +		uv_call_once(0, (uint64_t)&uvcb);
> +		report(uvcb.rc == UVC_RC_INV_STATE, "%s", cmds[i].name);
> +	}
> +	report_prefix_pop();
> +}
> +
>   static void test_config_destroy(void)
>   {
>   	int rc;
> @@ -477,13 +495,68 @@ static void test_invalid(void)
>   	report_prefix_pop();
>   }
>   
> +static void test_clear_setup(void)
maybe rename this to setup_test_clear(void)
I initially mistook this function as a test and not a setup function for
a test

> +{
> +	unsigned long vsize;
> +	int rc;
> +
> +	uvcb_cgc.header.cmd = UVC_CMD_CREATE_SEC_CONF;
> +	uvcb_cgc.header.len = sizeof(uvcb_cgc);
> +
> +	uvcb_cgc.guest_stor_origin = 0;
> +	uvcb_cgc.guest_stor_len = 42 * (1UL << 20);
> +	vsize = uvcb_qui.conf_base_virt_stor_len +
> +		((uvcb_cgc.guest_stor_len / (1UL << 20)) * uvcb_qui.conf_virt_var_stor_len);
> +
> +	uvcb_cgc.conf_base_stor_origin = (uint64_t)memalign(PAGE_SIZE * 4, uvcb_qui.conf_base_phys_stor_len);
> +	uvcb_cgc.conf_var_stor_origin = (uint64_t)memalign(PAGE_SIZE, vsize);
> +	uvcb_cgc.guest_asce = (uint64_t)memalign(PAGE_SIZE, 4 * PAGE_SIZE) | ASCE_DT_SEGMENT | REGION_TABLE_LENGTH | ASCE_P;
> +	uvcb_cgc.guest_sca = (uint64_t)memalign(PAGE_SIZE * 4, PAGE_SIZE * 4);
> +
> +	rc = uv_call(0, (uint64_t)&uvcb_cgc);
> +	assert(rc == 0);
> +
> +	uvcb_csc.header.len = sizeof(uvcb_csc);
> +	uvcb_csc.header.cmd = UVC_CMD_CREATE_SEC_CPU;
> +	uvcb_csc.guest_handle = uvcb_cgc.guest_handle;
> +	uvcb_csc.stor_origin = (unsigned long)memalign(PAGE_SIZE, uvcb_qui.cpu_stor_len);
> +	uvcb_csc.state_origin = (unsigned long)memalign(PAGE_SIZE, PAGE_SIZE);
> +
> +	rc = uv_call(0, (uint64_t)&uvcb_csc);
> +	assert(rc == 0);
> +}
> +
>   static void test_clear(void)
>   {
> -	uint64_t *tmp = (void *)uvcb_init.stor_origin;
> +	uint64_t *tmp;
> +
> +	report_prefix_push("load normal reset");
> +
> +	/*
> +	 * Setup a config and a cpu so we can check if a diag308 reset
> +	 * clears the donated memory and makes the pages unsecure.
> +	 */
> +	test_clear_setup();
>   
>   	diag308_load_reset(1);
>   	sclp_console_setup();
> -	report(!*tmp, "memory cleared after reset 1");
> +
> +	tmp = (void *)uvcb_init.stor_origin;
> +	report(!*tmp, "uv init donated memory cleared");
> +
> +	tmp = (void *)uvcb_cgc.conf_base_stor_origin;
> +	report(!*tmp, "config base donated memory cleared");
> +
> +	tmp = (void *)uvcb_cgc.conf_base_stor_origin;
> +	report(!*tmp, "config variable donated memory cleared");
> +
> +	tmp = (void *)uvcb_csc.stor_origin;
> +	report(!*tmp, "cpu donated memory cleared after reset 1");
> +
> +	/* Check if uninitialized after reset */
> +	test_uv_uninitialized();
> +
> +	report_prefix_pop();
>   }
>   
>   static void setup_vmem(void)
> @@ -514,6 +587,7 @@ int main(void)
>   
>   	test_priv();
>   	test_invalid();
> +	test_uv_uninitialized();
>   	test_query();
>   	test_init();
IIRC this test must be done last, as a following test has an 
uninitialized UV. Maybe add a short comment for that here.
>   


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 2/6] s390x: uv-host: Add uninitialized UV tests
  2022-05-16 15:02   ` Steffen Eiden
@ 2022-05-16 15:15     ` Janosch Frank
  2022-05-16 15:34       ` Steffen Eiden
  0 siblings, 1 reply; 21+ messages in thread
From: Janosch Frank @ 2022-05-16 15:15 UTC (permalink / raw)
  To: Steffen Eiden, kvm390 mailing list
  Cc: kvm, linux-s390, imbrenda, thuth, nrb, scgl

On 5/16/22 17:02, Steffen Eiden wrote:
> 
> 
> On 5/13/22 11:50, Janosch Frank wrote:
>> Let's also test for rc 0x3
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
> I, however, have some nits below.
> 
>> ---
>>    s390x/uv-host.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++--
>>    1 file changed, 76 insertions(+), 2 deletions(-)
>>
>> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
>> index 0f0b18a1..f846fc42 100644
>> --- a/s390x/uv-host.c
>> +++ b/s390x/uv-host.c
>> @@ -83,6 +83,24 @@ static void test_priv(void)
>>    	report_prefix_pop();
>>    }
>>    
>> +static void test_uv_uninitialized(void)
>> +{
>> +	struct uv_cb_header uvcb = {};
>> +	int i;
>> +
>> +	report_prefix_push("uninitialized");
>> +
>> +	/* i = 1 to skip over initialize */
>> +	for (i = 1; cmds[i].name; i++) {
>> +		expect_pgm_int();
>> +		uvcb.cmd = cmds[i].cmd;
>> +		uvcb.len = cmds[i].len;
>> +		uv_call_once(0, (uint64_t)&uvcb);
>> +		report(uvcb.rc == UVC_RC_INV_STATE, "%s", cmds[i].name);
>> +	}
>> +	report_prefix_pop();
>> +}
>> +
>>    static void test_config_destroy(void)
>>    {
>>    	int rc;
>> @@ -477,13 +495,68 @@ static void test_invalid(void)
>>    	report_prefix_pop();
>>    }
>>    
>> +static void test_clear_setup(void)
> maybe rename this to setup_test_clear(void)
> I initially mistook this function as a test and not a setup function for
> a test

Sure

> 
>> +{
>> +	unsigned long vsize;
>> +	int rc;
>> +
[...]
>>    static void setup_vmem(void)
>> @@ -514,6 +587,7 @@ int main(void)
>>    
>>    	test_priv();
>>    	test_invalid();
>> +	test_uv_uninitialized();
>>    	test_query();
>>    	test_init();
> IIRC this test must be done last, as a following test has an
> uninitialized UV. Maybe add a short comment for that here.

You're referring to the test_init()?

The test_clear() function must be done last but you're commenting under 
the test_init() call. So I'm a bit confused about what you want me to do 
here.

>>    
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 2/6] s390x: uv-host: Add uninitialized UV tests
  2022-05-16 15:15     ` Janosch Frank
@ 2022-05-16 15:34       ` Steffen Eiden
  0 siblings, 0 replies; 21+ messages in thread
From: Steffen Eiden @ 2022-05-16 15:34 UTC (permalink / raw)
  To: Janosch Frank, kvm390 mailing list
  Cc: kvm, linux-s390, imbrenda, thuth, nrb, scgl

Hi Janosch,

On 5/16/22 17:15, Janosch Frank wrote:
> On 5/16/22 17:02, Steffen Eiden wrote:
>>
[snip]
> [...]
>>>    static void setup_vmem(void)
>>> @@ -514,6 +587,7 @@ int main(void)
>>>        test_priv();
>>>        test_invalid();
>>> +    test_uv_uninitialized();
>>>        test_query();
>>>        test_init();
>> IIRC this test must be done last, as a following test has an
>> uninitialized UV. Maybe add a short comment for that here.
> 
> You're referring to the test_init()?
> 
> The test_clear() function must be done last but you're commenting under 
> the test_init() call. So I'm a bit confused about what you want me to do 
> here.

Sorry, my fault. Yes I wanted to suggest a comment for the 'test_clear' 
function.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 3/6] s390x: uv-host: Test uv immediate parameter
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 3/6] s390x: uv-host: Test uv immediate parameter Janosch Frank
@ 2022-05-17  8:29   ` Steffen Eiden
  2022-05-17  9:03     ` Janosch Frank
  0 siblings, 1 reply; 21+ messages in thread
From: Steffen Eiden @ 2022-05-17  8:29 UTC (permalink / raw)
  To: Janosch Frank, kvm390 mailing list
  Cc: kvm, linux-s390, imbrenda, thuth, nrb, scgl

Hey Janosch,

On 5/13/22 11:50, Janosch Frank wrote:
> Let's check if we get a specification PGM exception if we set a
> non-zero i3 when doing a UV call.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   s390x/uv-host.c | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
> 
> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
> index f846fc42..fcb82d24 100644
> --- a/s390x/uv-host.c
> +++ b/s390x/uv-host.c
> @@ -64,6 +64,28 @@ static struct cmd_list cmds[] = {
>   	{ NULL, 0, 0 },
>   };
>   
> +static void test_i3(void)
> +{
> +	struct uv_cb_header uvcb = {
> +		.cmd = UVC_CMD_INIT_UV,
> +		.len = sizeof(struct uv_cb_init),
> +	};
> +	unsigned long r1 = 0;
Did you forgot 'r2' or is it missing for a reason?

> +	int cc;
> +
> +	report_prefix_push("i3");
> +	expect_pgm_int();
> +	asm volatile(
> +		"0:	.insn rrf,0xB9A40000,%[r1],%[r2],4,2\n"
> +		"		ipm	%[cc]\n"
> +		"		srl	%[cc],28\n"
> +		: [cc] "=d" (cc)
> +		: [r1] "a" (r1), [r2] "a" (&uvcb)
> +		: "memory", "cc");
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +}
> +
>   static void test_priv(void)
>   {
>   	struct uv_cb_header uvcb = {};
> @@ -585,6 +607,7 @@ int main(void)
>   		goto done;
>   	}
>   
> +	test_i3();
>   	test_priv();
>   	test_invalid();
>   	test_uv_uninitialized();

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 4/6] s390x: uv-host: Add access exception test
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 4/6] s390x: uv-host: Add access exception test Janosch Frank
  2022-05-16  8:42   ` Nico Boehr
@ 2022-05-17  8:53   ` Steffen Eiden
  1 sibling, 0 replies; 21+ messages in thread
From: Steffen Eiden @ 2022-05-17  8:53 UTC (permalink / raw)
  To: Janosch Frank, kvm390 mailing list
  Cc: kvm, linux-s390, imbrenda, thuth, nrb, scgl



On 5/13/22 11:50, Janosch Frank wrote:
> Let's check that we get access exceptions if the UVCB is on an invalid
> page or starts at a valid page and crosses into an invalid one.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 3/6] s390x: uv-host: Test uv immediate parameter
  2022-05-17  8:29   ` Steffen Eiden
@ 2022-05-17  9:03     ` Janosch Frank
  2022-05-17  9:04       ` Steffen Eiden
  0 siblings, 1 reply; 21+ messages in thread
From: Janosch Frank @ 2022-05-17  9:03 UTC (permalink / raw)
  To: Steffen Eiden, kvm390 mailing list
  Cc: kvm, linux-s390, imbrenda, thuth, nrb, scgl

On 5/17/22 10:29, Steffen Eiden wrote:
> Hey Janosch,
> 
> On 5/13/22 11:50, Janosch Frank wrote:
>> Let's check if we get a specification PGM exception if we set a
>> non-zero i3 when doing a UV call.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>    s390x/uv-host.c | 23 +++++++++++++++++++++++
>>    1 file changed, 23 insertions(+)
>>
>> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
>> index f846fc42..fcb82d24 100644
>> --- a/s390x/uv-host.c
>> +++ b/s390x/uv-host.c
>> @@ -64,6 +64,28 @@ static struct cmd_list cmds[] = {
>>    	{ NULL, 0, 0 },
>>    };
>>    
>> +static void test_i3(void)
>> +{
>> +	struct uv_cb_header uvcb = {
>> +		.cmd = UVC_CMD_INIT_UV,
>> +		.len = sizeof(struct uv_cb_init),
>> +	};
>> +	unsigned long r1 = 0;
> Did you forgot 'r2' or is it missing for a reason?

The uvcb is the r2, have a look at the clobbers below

> 
>> +	int cc;
>> +
>> +	report_prefix_push("i3");
>> +	expect_pgm_int();
>> +	asm volatile(
>> +		"0:	.insn rrf,0xB9A40000,%[r1],%[r2],4,2\n"
>> +		"		ipm	%[cc]\n"
>> +		"		srl	%[cc],28\n"
>> +		: [cc] "=d" (cc)
>> +		: [r1] "a" (r1), [r2] "a" (&uvcb)
>> +		: "memory", "cc");
>> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
>> +	report_prefix_pop();
>> +}
>> +
>>    static void test_priv(void)
>>    {
>>    	struct uv_cb_header uvcb = {};
>> @@ -585,6 +607,7 @@ int main(void)
>>    		goto done;
>>    	}
>>    
>> +	test_i3();
>>    	test_priv();
>>    	test_invalid();
>>    	test_uv_uninitialized();


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 3/6] s390x: uv-host: Test uv immediate parameter
  2022-05-17  9:03     ` Janosch Frank
@ 2022-05-17  9:04       ` Steffen Eiden
  0 siblings, 0 replies; 21+ messages in thread
From: Steffen Eiden @ 2022-05-17  9:04 UTC (permalink / raw)
  To: Janosch Frank, kvm390 mailing list
  Cc: kvm, linux-s390, imbrenda, thuth, nrb, scgl



On 5/17/22 11:03, Janosch Frank wrote:
> On 5/17/22 10:29, Steffen Eiden wrote:
>> Hey Janosch,
>>
>> On 5/13/22 11:50, Janosch Frank wrote:
>>> Let's check if we get a specification PGM exception if we set a
>>> non-zero i3 when doing a UV call.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>>    s390x/uv-host.c | 23 +++++++++++++++++++++++
>>>    1 file changed, 23 insertions(+)
>>>
>>> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
>>> index f846fc42..fcb82d24 100644
>>> --- a/s390x/uv-host.c
>>> +++ b/s390x/uv-host.c
>>> @@ -64,6 +64,28 @@ static struct cmd_list cmds[] = {
>>>        { NULL, 0, 0 },
>>>    };
>>> +static void test_i3(void)
>>> +{
>>> +    struct uv_cb_header uvcb = {
>>> +        .cmd = UVC_CMD_INIT_UV,
>>> +        .len = sizeof(struct uv_cb_init),
>>> +    };
>>> +    unsigned long r1 = 0;
>> Did you forgot 'r2' or is it missing for a reason?
> 
> The uvcb is the r2, have a look at the clobbers below

Oh right, my bad; Sorry.
> 
>>
>>> +    int cc;
>>> +
>>> +    report_prefix_push("i3");
>>> +    expect_pgm_int();
>>> +    asm volatile(
>>> +        "0:    .insn rrf,0xB9A40000,%[r1],%[r2],4,2\n"
>>> +        "        ipm    %[cc]\n"
>>> +        "        srl    %[cc],28\n"
>>> +        : [cc] "=d" (cc)
>>> +        : [r1] "a" (r1), [r2] "a" (&uvcb)
>>> +        : "memory", "cc");
>>> +    check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
>>> +    report_prefix_pop();
>>> +}
>>> +
>>>    static void test_priv(void)
>>>    {
>>>        struct uv_cb_header uvcb = {};
>>> @@ -585,6 +607,7 @@ int main(void)
>>>            goto done;
>>>        }
>>> +    test_i3();
>>>        test_priv();
>>>        test_invalid();
>>>        test_uv_uninitialized();
> 

-- 
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 5/6] s390x: uv-host: Add a set secure config parameters test function
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 5/6] s390x: uv-host: Add a set secure config parameters test function Janosch Frank
@ 2022-05-17 11:32   ` Steffen Eiden
  0 siblings, 0 replies; 21+ messages in thread
From: Steffen Eiden @ 2022-05-17 11:32 UTC (permalink / raw)
  To: Janosch Frank, kvm390 mailing list
  Cc: kvm, linux-s390, imbrenda, thuth, nrb, scgl

Hey Janosch,

On 5/13/22 11:50, Janosch Frank wrote:
> Time for more tests.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
If you free 'pages'

> ---
>   s390x/uv-host.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 47 insertions(+)
> 
> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
> index 153a94e1..20d805b8 100644
> --- a/s390x/uv-host.c
> +++ b/s390x/uv-host.c
> @@ -229,6 +229,52 @@ static void test_cpu_destroy(void)
>   	report_prefix_pop();
>   }
>   
> +static void test_set_se_header(void)
> +{
> +	struct uv_cb_ssc uvcb = {
> +		.header.cmd = UVC_CMD_SET_SEC_CONF_PARAMS,
> +		.header.len = sizeof(uvcb),
> +		.guest_handle = uvcb_cgc.guest_handle,
> +		.sec_header_origin = 0,
> +		.sec_header_len = 0x1000,
> +	};
> +	void *pages =  alloc_pages(1);
> +	void *inv;
> +	int rc;
> +
> +	report_prefix_push("sscp");
> +
> +	uvcb.header.len -= 8;
> +	rc = uv_call(0, (uint64_t)&uvcb);
> +	report(rc == 1 && uvcb.header.rc == UVC_RC_INV_LEN,
> +	       "hdr invalid length");
> +	uvcb.header.len += 8;
> +
> +	uvcb.guest_handle += 1;
> +	rc = uv_call(0, (uint64_t)&uvcb);
> +	report(rc == 1 && uvcb.header.rc == UVC_RC_INV_GHANDLE, "invalid handle");
> +	uvcb.guest_handle -= 1;
> +
> +	inv = pages + PAGE_SIZE;
> +	uvcb.sec_header_origin = (uint64_t)inv;
> +	protect_page(inv, PAGE_ENTRY_I);
> +	rc = uv_call(0, (uint64_t)&uvcb);
> +	report(rc == 1 && uvcb.header.rc == 0x103,
> +	       "se hdr access exception");
> +
> +	/*
> +	 * Shift the ptr so the first few DWORDs are accessible but
> +	 * the following are on an invalid page.
> +	 */
> +	uvcb.sec_header_origin -= 0x20;
> +	rc = uv_call(0, (uint64_t)&uvcb);
> +	report(rc == 1 && uvcb.header.rc == 0x103,
> +	       "se hdr access exception crossing");
> +	unprotect_page(inv, PAGE_ENTRY_I);

please free 'pages'.

> +
> +	report_prefix_pop();
> +}
> +
>   static void test_cpu_create(void)
>   {
>   	int rc;
> @@ -669,6 +715,7 @@ int main(void)
>   
>   	test_config_create();
>   	test_cpu_create();
> +	test_set_se_header();
>   	test_cpu_destroy();
>   	test_config_destroy();
>   	test_clear();

Steffen

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [kvm-unit-tests PATCH 6/6] s390x: uv-host: Remove duplicated +
  2022-05-13  9:50 ` [kvm-unit-tests PATCH 6/6] s390x: uv-host: Remove duplicated + Janosch Frank
  2022-05-13 15:30   ` Claudio Imbrenda
@ 2022-05-17 11:33   ` Steffen Eiden
  1 sibling, 0 replies; 21+ messages in thread
From: Steffen Eiden @ 2022-05-17 11:33 UTC (permalink / raw)
  To: Janosch Frank, kvm390 mailing list
  Cc: kvm, linux-s390, imbrenda, thuth, nrb, scgl

Hey Janosch,

On 5/13/22 11:50, Janosch Frank wrote:
> One + is definitely enough here.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
>   s390x/uv-host.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
> index 20d805b8..ed16f850 100644
> --- a/s390x/uv-host.c
> +++ b/s390x/uv-host.c
> @@ -433,7 +433,7 @@ static void test_config_create(void)
>   	uvcb_cgc.guest_sca = tmp;
>   
>   	tmp = uvcb_cgc.guest_sca;
> -	uvcb_cgc.guest_sca = get_max_ram_size() + + PAGE_SIZE * 4;
> +	uvcb_cgc.guest_sca = get_max_ram_size() + PAGE_SIZE * 4;
>   	rc = uv_call(0, (uint64_t)&uvcb_cgc);
>   	report(uvcb_cgc.header.rc == 0x10d && rc == 1,
>   	       "sca inaccessible");

Steffen

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-05-17 11:34 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-13  9:50 [kvm-unit-tests PATCH 0/6] s390x: uv-host: Access check extensions and improvements Janosch Frank
2022-05-13  9:50 ` [kvm-unit-tests PATCH 1/6] s390x: uv-host: Add access checks for donated memory Janosch Frank
2022-05-16  8:21   ` Nico Boehr
2022-05-16  9:22     ` Janosch Frank
2022-05-13  9:50 ` [kvm-unit-tests PATCH 2/6] s390x: uv-host: Add uninitialized UV tests Janosch Frank
2022-05-16  8:37   ` Nico Boehr
2022-05-16 15:02   ` Steffen Eiden
2022-05-16 15:15     ` Janosch Frank
2022-05-16 15:34       ` Steffen Eiden
2022-05-13  9:50 ` [kvm-unit-tests PATCH 3/6] s390x: uv-host: Test uv immediate parameter Janosch Frank
2022-05-17  8:29   ` Steffen Eiden
2022-05-17  9:03     ` Janosch Frank
2022-05-17  9:04       ` Steffen Eiden
2022-05-13  9:50 ` [kvm-unit-tests PATCH 4/6] s390x: uv-host: Add access exception test Janosch Frank
2022-05-16  8:42   ` Nico Boehr
2022-05-17  8:53   ` Steffen Eiden
2022-05-13  9:50 ` [kvm-unit-tests PATCH 5/6] s390x: uv-host: Add a set secure config parameters test function Janosch Frank
2022-05-17 11:32   ` Steffen Eiden
2022-05-13  9:50 ` [kvm-unit-tests PATCH 6/6] s390x: uv-host: Remove duplicated + Janosch Frank
2022-05-13 15:30   ` Claudio Imbrenda
2022-05-17 11:33   ` Steffen Eiden

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.