| Message ID | 20240913115248.211071-2-schlameuss@linux.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | selftests: kvm: s390: Add ucontrol memory selftests | expand |
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>
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 */ [...]
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 */ > > [...]
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.
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.
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;
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(-)