[PATCH v3 0/3] selftests: kvm: s390: Add ucontrol memory selftests

[PATCH v3 0/3] selftests: kvm: s390: Add ucontrol memory selftests

[Christoph Schlameuss]

This patch series adds a some not yet picked selftests to the kvm s390x
selftest suite.

The additional test cases are covering:
* Assert KVM_EXIT_S390_UCONTROL exit on not mapped memory access
* Assert functionality of storage keys in ucontrol VM
* Assert that memory region operations are rejected for ucontrol VMs

Running the test cases requires sys_admin capabilities to start the
ucontrol VM.
This can be achieved by running as root or with a command like:

    sudo setpriv --reuid nobody --inh-caps -all,+sys_admin \
      --ambient-caps -all,+sys_admin --bounding-set -all,+sys_admin \
      ./ucontrol_test

---

The patches in this series have been part of the previous patch series.
The test cases added here do depend on the fixture added in the earlier
patches.
From v5 PATCH 7-9 the segment and page table generation has been removed
and DAT
has been disabled. Since DAT is not necessary to validate the KVM code.

https://lore.kernel.org/kvm/20240807154512.316936-1-schlameuss@linux.ibm.com/

v3:
- fix skey assertion (thanks Claudio)
- introduce a wrapper around UCAS map and unmap ioctls to improve
  readability (Claudio)
- add an displacement to accessed memory to assert translation
  intercepts actually point to segments to the uc_map_unmap test
- add an misaligned failing mapping try to the uc_map_unmap test

v2:
- Reenable KSS intercept and handle it within skey test.
- Modify the checked register between storing (sske) and reading (iske)
  it within the test program to make sure the.
- Add an additional state assertion in the end of uc_skey
- Fix some typos and white spaces.

v1:
- Remove segment and page table generation and disable DAT. This is not
  necessary to validate the KVM code.

Christoph Schlameuss (3):
  selftests: kvm: s390: Add uc_map_unmap VM test case
  selftests: kvm: s390: Add uc_skey VM test case
  selftests: kvm: s390: Verify reject memory region operations for
    ucontrol VMs

 .../selftests/kvm/s390x/ucontrol_test.c       | 256 +++++++++++++++++-
 1 file changed, 254 insertions(+), 2 deletions(-)

-- 
2.46.0

[PATCH v3 1/3] selftests: kvm: s390: Add uc_map_unmap VM test case

[Christoph Schlameuss]

Add a test case verifying basic running and interaction of ucontrol VMs.
Fill the segment and page tables for allocated memory and map memory on
first access.

* uc_map_unmap
  Store and load data to mapped and unmapped memory and use pic segment
  translation handling to map memory on access.

Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
 .../selftests/kvm/s390x/ucontrol_test.c       | 145 +++++++++++++++++-
 1 file changed, 144 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
index 030c59010fe1..084cea02c2fa 100644
--- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c
+++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
@@ -16,7 +16,11 @@
 #include <linux/capability.h>
 #include <linux/sizes.h>
 
+#define PGM_SEGMENT_TRANSLATION 0x10
+
 #define VM_MEM_SIZE (4 * SZ_1M)
+#define VM_MEM_EXT_SIZE (2 * SZ_1M)
+#define VM_MEM_MAX_M ((VM_MEM_SIZE + VM_MEM_EXT_SIZE) / SZ_1M)
 
 /* so directly declare capget to check caps without libcap */
 int capget(cap_user_header_t header, cap_user_data_t data);
@@ -58,6 +62,23 @@ asm("test_gprs_asm:\n"
 	"	j	0b\n"
 );
 
+/* Test program manipulating memory */
+extern char test_mem_asm[];
+asm("test_mem_asm:\n"
+	"xgr	%r0, %r0\n"
+
+	"0:\n"
+	"	ahi	%r0,1\n"
+	"	st	%r1,0(%r5,%r6)\n"
+
+	"	xgr	%r1,%r1\n"
+	"	l	%r1,0(%r5,%r6)\n"
+	"	ahi	%r0,1\n"
+	"	diag	0,0,0x44\n"
+
+	"	j	0b\n"
+);
+
 FIXTURE(uc_kvm)
 {
 	struct kvm_s390_sie_block *sie_block;
@@ -67,6 +88,7 @@ FIXTURE(uc_kvm)
 	uintptr_t base_hva;
 	uintptr_t code_hva;
 	int kvm_run_size;
+	vm_paddr_t pgd;
 	void *vm_mem;
 	int vcpu_fd;
 	int kvm_fd;
@@ -116,7 +138,7 @@ FIXTURE_SETUP(uc_kvm)
 	self->base_gpa = 0;
 	self->code_gpa = self->base_gpa + (3 * SZ_1M);
 
-	self->vm_mem = aligned_alloc(SZ_1M, VM_MEM_SIZE);
+	self->vm_mem = aligned_alloc(SZ_1M, VM_MEM_MAX_M * SZ_1M);
 	ASSERT_NE(NULL, self->vm_mem) TH_LOG("malloc failed %u", errno);
 	self->base_hva = (uintptr_t)self->vm_mem;
 	self->code_hva = self->base_hva - self->base_gpa + self->code_gpa;
@@ -222,6 +244,60 @@ TEST(uc_cap_hpage)
 	close(kvm_fd);
 }
 
+/* calculate host virtual addr from guest physical addr */
+static void *gpa2hva(FIXTURE_DATA(uc_kvm) * self, u64 gpa)
+{
+	return (void *)(self->base_hva - self->base_gpa + gpa);
+}
+
+/* map / make additional memory available */
+static int uc_map_ext(FIXTURE_DATA(uc_kvm) * self, u64 vcpu_addr, u64 length)
+{
+	struct kvm_s390_ucas_mapping map = {
+		.user_addr = (u64)gpa2hva(self, vcpu_addr),
+		.vcpu_addr = vcpu_addr,
+		.length = length,
+	};
+	pr_info("ucas map %p %p 0x%llx",
+		(void *)map.user_addr, (void *)map.vcpu_addr, map.length);
+	return ioctl(self->vcpu_fd, KVM_S390_UCAS_MAP, &map);
+}
+
+/* unmap previously mapped memory */
+static int uc_unmap_ext(FIXTURE_DATA(uc_kvm) * self, u64 vcpu_addr, u64 length)
+{
+	struct kvm_s390_ucas_mapping map = {
+		.user_addr = (u64)gpa2hva(self, vcpu_addr),
+		.vcpu_addr = vcpu_addr,
+		.length = length,
+	};
+	pr_info("ucas unmap %p %p 0x%llx",
+		(void *)map.user_addr, (void *)map.vcpu_addr, map.length);
+	return ioctl(self->vcpu_fd, KVM_S390_UCAS_UNMAP, &map);
+}
+
+/* handle ucontrol exit by mapping the accessed segment */
+static void uc_handle_exit_ucontrol(FIXTURE_DATA(uc_kvm) * self)
+{
+	struct kvm_run *run = self->run;
+	u64 seg_addr;
+	int rc;
+
+	TEST_ASSERT_EQ(KVM_EXIT_S390_UCONTROL, run->exit_reason);
+	switch (run->s390_ucontrol.pgm_code) {
+	case PGM_SEGMENT_TRANSLATION:
+		seg_addr = run->s390_ucontrol.trans_exc_code & ~(SZ_1M - 1);
+		pr_info("ucontrol pic segment translation 0x%llx, mapping segment 0x%lx\n",
+			run->s390_ucontrol.trans_exc_code, seg_addr);
+		/* map / make additional memory available */
+		rc = uc_map_ext(self, seg_addr, SZ_1M);
+		TEST_ASSERT_EQ(0, rc);
+		break;
+	default:
+		TEST_FAIL("UNEXPECTED PGM CODE %d", run->s390_ucontrol.pgm_code);
+	}
+}
+
 /* verify SIEIC exit
  * * reset stop requests
  * * fail on codes not expected in the test cases
@@ -256,6 +332,12 @@ static bool uc_handle_exit(FIXTURE_DATA(uc_kvm) * self)
 	struct kvm_run *run = self->run;
 
 	switch (run->exit_reason) {
+	case KVM_EXIT_S390_UCONTROL:
+		/** check program interruption code
+		 * handle page fault --> ucas map
+		 */
+		uc_handle_exit_ucontrol(self);
+		break;
 	case KVM_EXIT_S390_SIEIC:
 		return uc_handle_sieic(self);
 	default:
@@ -287,6 +369,67 @@ static void uc_assert_diag44(FIXTURE_DATA(uc_kvm) * self)
 	TEST_ASSERT_EQ(0x440000, sie_block->ipb);
 }
 
+TEST_F(uc_kvm, uc_map_unmap)
+{
+	struct kvm_sync_regs *sync_regs = &self->run->s.regs;
+	struct kvm_run *run = self->run;
+	const u64 disp = 1;
+	int rc;
+
+	/* copy test_mem_asm to code_hva / code_gpa */
+	TH_LOG("copy code %p to vm mapped memory %p / %p",
+	       &test_mem_asm, (void *)self->code_hva, (void *)self->code_gpa);
+	memcpy((void *)self->code_hva, &test_mem_asm, PAGE_SIZE);
+
+	/* DAT disabled + 64 bit mode */
+	run->psw_mask = 0x0000000180000000ULL;
+	run->psw_addr = self->code_gpa;
+
+	/* set register content for test_mem_asm to access not mapped memory*/
+	sync_regs->gprs[1] = 0x55;
+	sync_regs->gprs[5] = self->base_gpa;
+	sync_regs->gprs[6] = VM_MEM_SIZE + disp;
+	run->kvm_dirty_regs |= KVM_SYNC_GPRS;
+
+	/* run and expect to fail with ucontrol pic segment translation */
+	ASSERT_EQ(0, uc_run_once(self));
+	ASSERT_EQ(1, sync_regs->gprs[0]);
+	ASSERT_EQ(KVM_EXIT_S390_UCONTROL, run->exit_reason);
+
+	ASSERT_EQ(PGM_SEGMENT_TRANSLATION, run->s390_ucontrol.pgm_code);
+	ASSERT_EQ(self->base_gpa + VM_MEM_SIZE, run->s390_ucontrol.trans_exc_code);
+
+	/* fail to map memory with not segment aligned address */
+	rc = uc_map_ext(self, self->base_gpa + VM_MEM_SIZE + disp, VM_MEM_EXT_SIZE);
+	ASSERT_GT(0, rc)
+		TH_LOG("ucas map for non segment address should fail but didn't; "
+		       "result %d not expected, %s", rc, strerror(errno));
+
+	/* map / make additional memory available */
+	rc = uc_map_ext(self, self->base_gpa + VM_MEM_SIZE, VM_MEM_EXT_SIZE);
+	ASSERT_EQ(0, rc)
+		TH_LOG("ucas map result %d not expected, %s", rc, strerror(errno));
+	ASSERT_EQ(0, uc_run_once(self));
+	ASSERT_EQ(false, uc_handle_exit(self));
+	uc_assert_diag44(self);
+
+	/* assert registers and memory are in expected state */
+	ASSERT_EQ(2, sync_regs->gprs[0]);
+	ASSERT_EQ(0x55, sync_regs->gprs[1]);
+	ASSERT_EQ(0x55, *(u32 *)gpa2hva(self, self->base_gpa + VM_MEM_SIZE + disp));
+
+	/* unmap and run loop again */
+	rc = uc_unmap_ext(self, self->base_gpa + VM_MEM_SIZE, VM_MEM_EXT_SIZE);
+	ASSERT_EQ(0, rc)
+		TH_LOG("ucas unmap result %d not expected, %s", rc, strerror(errno));
+	ASSERT_EQ(0, uc_run_once(self));
+	ASSERT_EQ(3, sync_regs->gprs[0]);
+	ASSERT_EQ(KVM_EXIT_S390_UCONTROL, run->exit_reason);
+	ASSERT_EQ(PGM_SEGMENT_TRANSLATION, run->s390_ucontrol.pgm_code);
+	/* handle ucontrol exit and remap memory after previous map and unmap */
+	ASSERT_EQ(true, uc_handle_exit(self));
+}
+
 TEST_F(uc_kvm, uc_gprs)
 {
 	struct kvm_sync_regs *sync_regs = &self->run->s.regs;
-- 
2.46.0

[PATCH v3 2/3] selftests: kvm: s390: Add uc_skey VM test case

[Christoph Schlameuss]

Add a test case manipulating s390 storage keys from within the ucontrol
VM.

Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
 .../selftests/kvm/s390x/ucontrol_test.c       | 89 ++++++++++++++++++-
 1 file changed, 88 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
index 084cea02c2fa..f6e3a68f89a9 100644
--- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c
+++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
@@ -79,6 +79,33 @@ asm("test_mem_asm:\n"
 	"	j	0b\n"
 );
 
+/* Test program manipulating storage keys */
+extern char test_skey_asm[];
+asm("test_skey_asm:\n"
+	"xgr	%r0, %r0\n"
+
+	"0:\n"
+	"	ahi	%r0,1\n"
+	"	st	%r1,0(%r5,%r6)\n"
+
+	"	iske	%r1,%r6\n"
+	"	ahi	%r0,1\n"
+	"	diag	0,0,0x44\n"
+
+	"	sske	%r1,%r6\n"
+	"	xgr	%r1,%r1\n"
+	"	iske	%r1,%r6\n"
+	"	ahi	%r0,1\n"
+	"	diag	0,0,0x44\n"
+
+	"	rrbe	%r1,%r6\n"
+	"	iske	%r1,%r6\n"
+	"	ahi	%r0,1\n"
+	"	diag	0,0,0x44\n"
+
+	"	j	0b\n"
+);
+
 FIXTURE(uc_kvm)
 {
 	struct kvm_s390_sie_block *sie_block;
@@ -299,8 +326,9 @@ static void uc_handle_exit_ucontrol(FIXTURE_DATA(uc_kvm) * self)
 }
 
 /* verify SIEIC exit
- * * reset stop requests
+ * * handle expected interception codes
  * * fail on codes not expected in the test cases
+ * Returns if interception is handled / execution can be continued
  */
 static bool uc_handle_sieic(FIXTURE_DATA(uc_kvm) * self)
 {
@@ -317,6 +345,10 @@ static bool uc_handle_sieic(FIXTURE_DATA(uc_kvm) * self)
 		/* end execution in caller on intercepted instruction */
 		pr_info("sie instruction interception\n");
 		return false;
+	case ICPT_KSS:
+		/* disable KSS and continue; KVM would init the skeys here */
+		sie_block->cpuflags &= ~CPUSTAT_KSS;
+		return true;
 	case ICPT_OPEREXC:
 		/* operation exception */
 		TEST_FAIL("sie exception on %.4x%.8x", sie_block->ipa, sie_block->ipb);
@@ -473,4 +505,59 @@ TEST_F(uc_kvm, uc_gprs)
 	ASSERT_EQ(1, sync_regs->gprs[0]);
 }
 
+TEST_F(uc_kvm, uc_skey)
+{
+	u64 test_vaddr = VM_MEM_SIZE - (SZ_1M / 2);
+	struct kvm_sync_regs *sync_regs = &self->run->s.regs;
+	struct kvm_run *run = self->run;
+	u8 skeyvalue = 0x34;
+
+	/* copy test_skey_asm to code_hva / code_gpa */
+	TH_LOG("copy code %p to vm mapped memory %p / %p",
+	       &test_skey_asm, (void *)self->code_hva, (void *)self->code_gpa);
+	memcpy((void *)self->code_hva, &test_skey_asm, PAGE_SIZE);
+
+	/* set register content for test_skey_asm to access not mapped memory */
+	sync_regs->gprs[1] = skeyvalue;
+	sync_regs->gprs[5] = self->base_gpa;
+	sync_regs->gprs[6] = test_vaddr;
+	run->kvm_dirty_regs |= KVM_SYNC_GPRS;
+
+	/* DAT disabled + 64 bit mode */
+	run->psw_mask = 0x0000000180000000ULL;
+	run->psw_addr = self->code_gpa;
+
+	ASSERT_EQ(0, uc_run_once(self));
+	ASSERT_EQ(true, uc_handle_exit(self));
+	ASSERT_EQ(1, sync_regs->gprs[0]);
+
+	/* ISKE */
+	ASSERT_EQ(0, uc_run_once(self));
+	ASSERT_EQ(false, uc_handle_exit(self));
+	ASSERT_EQ(2, sync_regs->gprs[0]);
+	/* assert initial skey (ACC = 0, R & C = 1) */
+	ASSERT_EQ(0x06, sync_regs->gprs[1]);
+	uc_assert_diag44(self);
+
+	/* SSKE */
+	sync_regs->gprs[1] = skeyvalue;
+	run->kvm_dirty_regs |= KVM_SYNC_GPRS;
+	ASSERT_EQ(0, uc_run_once(self));
+	ASSERT_EQ(false, uc_handle_exit(self));
+	ASSERT_EQ(3, sync_regs->gprs[0]);
+	ASSERT_EQ(skeyvalue, sync_regs->gprs[1]);
+	uc_assert_diag44(self);
+
+	/* RRBE */
+	sync_regs->gprs[1] = skeyvalue;
+	run->kvm_dirty_regs |= KVM_SYNC_GPRS;
+	ASSERT_EQ(0, uc_run_once(self));
+	ASSERT_EQ(false, uc_handle_exit(self));
+	ASSERT_EQ(4, sync_regs->gprs[0]);
+	/* assert R reset but rest of skey unchanged*/
+	ASSERT_EQ(skeyvalue & 0xfa, sync_regs->gprs[1]);
+	ASSERT_EQ(0x00, sync_regs->gprs[1] & 0x04);
+	uc_assert_diag44(self);
+}
+
 TEST_HARNESS_MAIN
-- 
2.46.0

[PATCH v3 3/3] selftests: kvm: s390: Verify reject memory region operations for ucontrol VMs

[Christoph Schlameuss]

Add a test case verifying KVM_SET_USER_MEMORY_REGION and
KVM_SET_USER_MEMORY_REGION2 cannot be executed on ucontrol VMs.

Executing this test case on not patched kernels will cause a null
pointer dereference in the host kernel.
This is fixed with commit:
commit 7816e58967d0 ("kvm: s390: Reject memory region operations for ucontrol VMs")

Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
 .../selftests/kvm/s390x/ucontrol_test.c       | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
index f6e3a68f89a9..11fbb09eff66 100644
--- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c
+++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
@@ -401,6 +401,28 @@ static void uc_assert_diag44(FIXTURE_DATA(uc_kvm) * self)
 	TEST_ASSERT_EQ(0x440000, sie_block->ipb);
 }
 
+TEST_F(uc_kvm, uc_no_user_region)
+{
+	struct kvm_userspace_memory_region region = {
+		.slot = 1,
+		.guest_phys_addr = self->code_gpa,
+		.memory_size = VM_MEM_EXT_SIZE,
+		.userspace_addr = (uintptr_t)self->code_hva,
+	};
+	struct kvm_userspace_memory_region2 region2 = {
+		.slot = 1,
+		.guest_phys_addr = self->code_gpa,
+		.memory_size = VM_MEM_EXT_SIZE,
+		.userspace_addr = (uintptr_t)self->code_hva,
+	};
+
+	ASSERT_EQ(-1, ioctl(self->vm_fd, KVM_SET_USER_MEMORY_REGION, &region));
+	ASSERT_EQ(EINVAL, errno);
+
+	ASSERT_EQ(-1, ioctl(self->vm_fd, KVM_SET_USER_MEMORY_REGION2, &region2));
+	ASSERT_EQ(EINVAL, errno);
+}
+
 TEST_F(uc_kvm, uc_map_unmap)
 {
 	struct kvm_sync_regs *sync_regs = &self->run->s.regs;
-- 
2.46.0

Re: [PATCH v3 1/3] selftests: kvm: s390: Add uc_map_unmap VM test case

[Janosch Frank]

On 9/13/24 1:52 PM, Christoph Schlameuss wrote:
> Add a test case verifying basic running and interaction of ucontrol VMs.
> Fill the segment and page tables for allocated memory and map memory on
> first access.
> 
> * uc_map_unmap
>    Store and load data to mapped and unmapped memory and use pic segment
>    translation handling to map memory on access.
> 
> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

Re: [PATCH v3 2/3] selftests: kvm: s390: Add uc_skey VM test case

[Janosch Frank]

On 9/13/24 1:52 PM, Christoph Schlameuss wrote:
> Add a test case manipulating s390 storage keys from within the ucontrol
> VM.
> 
> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>

Except for the two nits:
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>


I'll think about what to do with the nits next week.

> ---
>   .../selftests/kvm/s390x/ucontrol_test.c       | 89 ++++++++++++++++++-
>   1 file changed, 88 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> index 084cea02c2fa..f6e3a68f89a9 100644
> --- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> +++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> @@ -79,6 +79,33 @@ asm("test_mem_asm:\n"
>   	"	j	0b\n"
>   );
>   
> +/* Test program manipulating storage keys */
> +extern char test_skey_asm[];
> +asm("test_skey_asm:\n"
> +	"xgr	%r0, %r0\n"
> +
> +	"0:\n"
> +	"	ahi	%r0,1\n"
> +	"	st	%r1,0(%r5,%r6)\n"
> +
> +	"	iske	%r1,%r6\n"
> +	"	ahi	%r0,1\n"
> +	"	diag	0,0,0x44\n"
> +
> +	"	sske	%r1,%r6\n"
> +	"	xgr	%r1,%r1\n"
> +	"	iske	%r1,%r6\n"
> +	"	ahi	%r0,1\n"
> +	"	diag	0,0,0x44\n"
> +
> +	"	rrbe	%r1,%r6\n"
> +	"	iske	%r1,%r6\n"
> +	"	ahi	%r0,1\n"
> +	"	diag	0,0,0x44\n"
> +
> +	"	j	0b\n"
> +);
> +
>   FIXTURE(uc_kvm)
>   {
>   	struct kvm_s390_sie_block *sie_block;
> @@ -299,8 +326,9 @@ static void uc_handle_exit_ucontrol(FIXTURE_DATA(uc_kvm) * self)
>   }
>   
>   /* verify SIEIC exit
> - * * reset stop requests

Argh, that made it in the patches that I've picked up.

> + * * handle expected interception codes
>    * * fail on codes not expected in the test cases
> + * Returns if interception is handled / execution can be continued
>    */
>   static bool uc_handle_sieic(FIXTURE_DATA(uc_kvm) * self)
>   {
> @@ -317,6 +345,10 @@ static bool uc_handle_sieic(FIXTURE_DATA(uc_kvm) * self)
>   		/* end execution in caller on intercepted instruction */
>   		pr_info("sie instruction interception\n");
>   		return false;
> +	case ICPT_KSS:
> +		/* disable KSS and continue; KVM would init the skeys here */
> +		sie_block->cpuflags &= ~CPUSTAT_KSS;
> +		return true;
>   	case ICPT_OPEREXC:
>   		/* operation exception */
>   		TEST_FAIL("sie exception on %.4x%.8x", sie_block->ipa, sie_block->ipb);
> @@ -473,4 +505,59 @@ TEST_F(uc_kvm, uc_gprs)
>   	ASSERT_EQ(1, sync_regs->gprs[0]);
>   }
>   
> +TEST_F(uc_kvm, uc_skey)
> +{
> +	u64 test_vaddr = VM_MEM_SIZE - (SZ_1M / 2);
> +	struct kvm_sync_regs *sync_regs = &self->run->s.regs;
> +	struct kvm_run *run = self->run;
> +	u8 skeyvalue = 0x34;
> +
> +	/* copy test_skey_asm to code_hva / code_gpa */
> +	TH_LOG("copy code %p to vm mapped memory %p / %p",
> +	       &test_skey_asm, (void *)self->code_hva, (void *)self->code_gpa);
> +	memcpy((void *)self->code_hva, &test_skey_asm, PAGE_SIZE);
> +
> +	/* set register content for test_skey_asm to access not mapped memory */
> +	sync_regs->gprs[1] = skeyvalue;
> +	sync_regs->gprs[5] = self->base_gpa;
> +	sync_regs->gprs[6] = test_vaddr;
> +	run->kvm_dirty_regs |= KVM_SYNC_GPRS;
> +
> +	/* DAT disabled + 64 bit mode */
> +	run->psw_mask = 0x0000000180000000ULL;
> +	run->psw_addr = self->code_gpa;
> +
> +	ASSERT_EQ(0, uc_run_once(self));
> +	ASSERT_EQ(true, uc_handle_exit(self));
> +	ASSERT_EQ(1, sync_regs->gprs[0]);
> +
> +	/* ISKE */
> +	ASSERT_EQ(0, uc_run_once(self));
> +	ASSERT_EQ(false, uc_handle_exit(self));
> +	ASSERT_EQ(2, sync_regs->gprs[0]);
> +	/* assert initial skey (ACC = 0, R & C = 1) */
> +	ASSERT_EQ(0x06, sync_regs->gprs[1]);
> +	uc_assert_diag44(self);
> +
> +	/* SSKE */
> +	sync_regs->gprs[1] = skeyvalue;
> +	run->kvm_dirty_regs |= KVM_SYNC_GPRS;
> +	ASSERT_EQ(0, uc_run_once(self));
> +	ASSERT_EQ(false, uc_handle_exit(self));
> +	ASSERT_EQ(3, sync_regs->gprs[0]);
> +	ASSERT_EQ(skeyvalue, sync_regs->gprs[1]);
> +	uc_assert_diag44(self);
> +
> +	/* RRBE */
> +	sync_regs->gprs[1] = skeyvalue;
> +	run->kvm_dirty_regs |= KVM_SYNC_GPRS;
> +	ASSERT_EQ(0, uc_run_once(self));
> +	ASSERT_EQ(false, uc_handle_exit(self));
> +	ASSERT_EQ(4, sync_regs->gprs[0]);
> +	/* assert R reset but rest of skey unchanged*/

Missing space before "*"

> +	ASSERT_EQ(skeyvalue & 0xfa, sync_regs->gprs[1]);
> +	ASSERT_EQ(0x00, sync_regs->gprs[1] & 0x04);
> +	uc_assert_diag44(self);
> +}
> +
>   TEST_HARNESS_MAIN

Re: [PATCH v3 1/3] selftests: kvm: s390: Add uc_map_unmap VM test case

[Claudio Imbrenda]

On Fri, 13 Sep 2024 13:52:46 +0200
Christoph Schlameuss <schlameuss@linux.ibm.com> wrote:

> Add a test case verifying basic running and interaction of ucontrol VMs.
> Fill the segment and page tables for allocated memory and map memory on
> first access.
> 
> * uc_map_unmap
>   Store and load data to mapped and unmapped memory and use pic segment
>   translation handling to map memory on access.
> 
> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
> ---
>  .../selftests/kvm/s390x/ucontrol_test.c       | 145 +++++++++++++++++-
>  1 file changed, 144 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> index 030c59010fe1..084cea02c2fa 100644
> --- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> +++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c

[...]

>base_gpa + self->code_gpa;
> @@ -222,6 +244,60 @@ TEST(uc_cap_hpage)
>  	close(kvm_fd);
>  }
>  
> +/* calculate host virtual addr from guest physical addr */
> +static void *gpa2hva(FIXTURE_DATA(uc_kvm) * self, u64 gpa)

why the space? I would have expected *self

> +{
> +	return (void *)(self->base_hva - self->base_gpa + gpa);
> +}
> +
> +/* map / make additional memory available */

[...]

Re: [PATCH v3 1/3] selftests: kvm: s390: Add uc_map_unmap VM test case

[Christoph Schlameuss]

On Fri Sep 13, 2024 at 6:48 PM CEST, Claudio Imbrenda wrote:
> On Fri, 13 Sep 2024 13:52:46 +0200
> Christoph Schlameuss <schlameuss@linux.ibm.com> wrote:
>
> > Add a test case verifying basic running and interaction of ucontrol VMs.
> > Fill the segment and page tables for allocated memory and map memory on
> > first access.
> > 
> > * uc_map_unmap
> >   Store and load data to mapped and unmapped memory and use pic segment
> >   translation handling to map memory on access.
> > 
> > Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
> > ---
> >  .../selftests/kvm/s390x/ucontrol_test.c       | 145 +++++++++++++++++-
> >  1 file changed, 144 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> > index 030c59010fe1..084cea02c2fa 100644
> > --- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> > +++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
>
> [...]
>
> >base_gpa + self->code_gpa;
> > @@ -222,6 +244,60 @@ TEST(uc_cap_hpage)
> >  	close(kvm_fd);
> >  }
> >  
> > +/* calculate host virtual addr from guest physical addr */
> > +static void *gpa2hva(FIXTURE_DATA(uc_kvm) * self, u64 gpa)
>
> why the space? I would have expected *self
>

That is how checkpatch.pl --strict prefers it.

Output from checkpatch without the space:

CHECK: spaces preferred around that '*' (ctx:WxV)
#19: FILE: tools/testing/selftests/kvm/s390x/ucontrol_test.c:278:
+static void *gpa2hva(FIXTURE_DATA(uc_kvm) *self, u64 gpa)
                                           ^

> > +{
> > +	return (void *)(self->base_hva - self->base_gpa + gpa);
> > +}
> > +
> > +/* map / make additional memory available */
>
> [...]

Re: [PATCH v3 1/3] selftests: kvm: s390: Add uc_map_unmap VM test case

[Janosch Frank]

On 9/16/24 8:25 AM, Christoph Schlameuss wrote:
> On Fri Sep 13, 2024 at 6:48 PM CEST, Claudio Imbrenda wrote:
>> On Fri, 13 Sep 2024 13:52:46 +0200
>> Christoph Schlameuss <schlameuss@linux.ibm.com> wrote:
>>
>>> Add a test case verifying basic running and interaction of ucontrol VMs.
>>> Fill the segment and page tables for allocated memory and map memory on
>>> first access.
>>>
>>> * uc_map_unmap
>>>    Store and load data to mapped and unmapped memory and use pic segment
>>>    translation handling to map memory on access.
>>>
>>> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
>>> ---
>>>   .../selftests/kvm/s390x/ucontrol_test.c       | 145 +++++++++++++++++-
>>>   1 file changed, 144 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
>>> index 030c59010fe1..084cea02c2fa 100644
>>> --- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c
>>> +++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
>>
>> [...]
>>
>>> base_gpa + self->code_gpa;
>>> @@ -222,6 +244,60 @@ TEST(uc_cap_hpage)
>>>   	close(kvm_fd);
>>>   }
>>>   
>>> +/* calculate host virtual addr from guest physical addr */
>>> +static void *gpa2hva(FIXTURE_DATA(uc_kvm) * self, u64 gpa)
>>
>> why the space? I would have expected *self
>>
> 
> That is how checkpatch.pl --strict prefers it.
> 
> Output from checkpatch without the space:
> 
> CHECK: spaces preferred around that '*' (ctx:WxV)
> #19: FILE: tools/testing/selftests/kvm/s390x/ucontrol_test.c:278:
> +static void *gpa2hva(FIXTURE_DATA(uc_kvm) *self, u64 gpa)

I'd guess checkpatch thinks this is a multiplication and that's why it 
complains here. It's checking against the wrong rule.

Re: [PATCH v3 1/3] selftests: kvm: s390: Add uc_map_unmap VM test case

[Christoph Schlameuss]

On Mon Sep 16, 2024 at 8:33 AM CEST, Janosch Frank wrote:
> On 9/16/24 8:25 AM, Christoph Schlameuss wrote:
> > On Fri Sep 13, 2024 at 6:48 PM CEST, Claudio Imbrenda wrote:
> >> On Fri, 13 Sep 2024 13:52:46 +0200
> >> Christoph Schlameuss <schlameuss@linux.ibm.com> wrote:
> >>
> >>> Add a test case verifying basic running and interaction of ucontrol VMs.
> >>> Fill the segment and page tables for allocated memory and map memory on
> >>> first access.
> >>>
> >>> * uc_map_unmap
> >>>    Store and load data to mapped and unmapped memory and use pic segment
> >>>    translation handling to map memory on access.
> >>>
> >>> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
> >>> ---
> >>>   .../selftests/kvm/s390x/ucontrol_test.c       | 145 +++++++++++++++++-
> >>>   1 file changed, 144 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> >>> index 030c59010fe1..084cea02c2fa 100644
> >>> --- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> >>> +++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> >>
> >> [...]
> >>
> >>> base_gpa + self->code_gpa;
> >>> @@ -222,6 +244,60 @@ TEST(uc_cap_hpage)
> >>>   	close(kvm_fd);
> >>>   }
> >>>   
> >>> +/* calculate host virtual addr from guest physical addr */
> >>> +static void *gpa2hva(FIXTURE_DATA(uc_kvm) * self, u64 gpa)
> >>
> >> why the space? I would have expected *self
> >>
> > 
> > That is how checkpatch.pl --strict prefers it.
> > 
> > Output from checkpatch without the space:
> > 
> > CHECK: spaces preferred around that '*' (ctx:WxV)
> > #19: FILE: tools/testing/selftests/kvm/s390x/ucontrol_test.c:278:
> > +static void *gpa2hva(FIXTURE_DATA(uc_kvm) *self, u64 gpa)
>
> I'd guess checkpatch thinks this is a multiplication and that's why it 
> complains here. It's checking against the wrong rule.

I see. I did experiment a bit. There is no obvious way to get rid of the CHECK
notices from checkpatch.pl. But I will correct the whitespaces for the function
calls.