[PATCH v3 0/9] KVM: s390: Extend MEM_OP ioctl by storage key checked cmpxchg

Janis Schoetterl-Glausch
2022-11-17T22:19:38Z

User space can use the MEM_OP ioctl to make storage key checked reads
and writes to the guest, however, it has no way of performing atomic,
key checked, accesses to the guest.
Extend the MEM_OP ioctl in order to allow for this, by adding a cmpxchg
mode. For now, support this mode for absolute accesses only.

This mode can be use, for example, to set the device-state-change
indicator and the adapter-local-summary indicator atomically.

Also contains some fixes/changes for the memop selftest independent of
the cmpxchg changes.

v2 -> v3
* rebase onto the wip/cmpxchg_user_key branch in the s390 kernel repo
* use __uint128_t instead of unsigned __int128
* put moving of testlist into main into separate patch
* pick up R-b's (thanks Nico)

v1 -> v2
* get rid of xrk instruction for cmpxchg byte and short implementation
* pass old parameter via pointer instead of in mem_op struct
* indicate failure of cmpxchg due to wrong old value by special return
code
* picked up R-b's (thanks Thomas)

Janis Schoetterl-Glausch (9):
KVM: s390: Extend MEM_OP ioctl by storage key checked cmpxchg
Documentation: KVM: s390: Describe KVM_S390_MEMOP_F_CMPXCHG
KVM: s390: selftest: memop: Pass mop_desc via pointer
KVM: s390: selftest: memop: Replace macros by functions
KVM: s390: selftest: memop: Move testlist into main
KVM: s390: selftest: memop: Add cmpxchg tests
KVM: s390: selftest: memop: Add bad address test
KVM: s390: selftest: memop: Fix typo
KVM: s390: selftest: memop: Fix wrong address being used in test

Documentation/virt/kvm/api.rst | 21 +-
include/uapi/linux/kvm.h | 5 +
arch/s390/kvm/gaccess.h | 3 +
arch/s390/kvm/gaccess.c | 101 ++++
arch/s390/kvm/kvm-s390.c | 35 +-
tools/testing/selftests/kvm/s390x/memop.c | 670 +++++++++++++++++-----
6 files changed, 683 insertions(+), 152 deletions(-)

Range-diff against v2:
1: c6731b0063ab ! 1: 94820a73e9b0 KVM: s390: Extend MEM_OP ioctl by storage key checked cmpxchg
@@ arch/s390/kvm/gaccess.h: int access_guest_with_key(struct kvm_vcpu *vcpu, unsign
void *data, unsigned long len, enum gacc_mode mode);

+int cmpxchg_guest_abs_with_key(struct kvm *kvm, gpa_t gpa, int len,
-+ unsigned __int128 *old,
-+ unsigned __int128 new, u8 access_key);
++ __uint128_t *old, __uint128_t new, u8 access_key);
+
/**
* write_guest_with_key - copy data from kernel space to guest space
@@ arch/s390/kvm/gaccess.c: int access_guest_real(struct kvm_vcpu *vcpu, unsigned l
+ * * a program interruption code indicating the reason cmpxchg could
+ * not be attempted
+ * * -EINVAL: address misaligned or len not power of two
++ * * -EAGAIN: transient failure (len 1 or 2)
+ */
+int cmpxchg_guest_abs_with_key(struct kvm *kvm, gpa_t gpa, int len,
-+ unsigned __int128 *old_p, unsigned __int128 new,
++ __uint128_t *old_p, __uint128_t new,
+ u8 access_key)
+{
+ gfn_t gfn = gpa >> PAGE_SHIFT;
@@ arch/s390/kvm/gaccess.c: int access_guest_real(struct kvm_vcpu *vcpu, unsigned l
+ return -EOPNOTSUPP;
+
+ hva += offset_in_page(gpa);
-+ ret = cmpxchg_user_key_size(len, (void __user *)hva, old_p, new, access_key);
++ switch (len) {
++ case 1: {
++ u8 old;
++
++ ret = cmpxchg_user_key((u8 *)hva, &old, *old_p, new, access_key);
++ ret = ret < 0 ? ret : old != *old_p;
++ *old_p = old;
++ break;
++ }
++ case 2: {
++ u16 old;
++
++ ret = cmpxchg_user_key((u16 *)hva, &old, *old_p, new, access_key);
++ ret = ret < 0 ? ret : old != *old_p;
++ *old_p = old;
++ break;
++ }
++ case 4: {
++ u32 old;
++
++ ret = cmpxchg_user_key((u32 *)hva, &old, *old_p, new, access_key);
++ ret = ret < 0 ? ret : old != *old_p;
++ *old_p = old;
++ break;
++ }
++ case 8: {
++ u64 old;
++
++ ret = cmpxchg_user_key((u64 *)hva, &old, *old_p, new, access_key);
++ ret = ret < 0 ? ret : old != *old_p;
++ *old_p = old;
++ break;
++ }
++ case 16: {
++ __uint128_t old;
++
++ ret = cmpxchg_user_key((__uint128_t *)hva, &old, *old_p, new, access_key);
++ ret = ret < 0 ? ret : old != *old_p;
++ *old_p = old;
++ break;
++ }
++ default:
++ return -EINVAL;
++ }
+ mark_page_dirty_in_slot(kvm, slot, gfn);
+ /*
+ * Assume that the fault is caused by protection, either key protection
@@ arch/s390/kvm/kvm-s390.c: static bool access_key_invalid(u8 access_key)
void __user *uaddr = (void __user *)mop->buf;
+ void __user *old_p = (void __user *)mop->old_p;
+ union {
-+ unsigned __int128 quad;
-+ char raw[sizeof(unsigned __int128)];
++ __uint128_t quad;
++ char raw[sizeof(__uint128_t)];
+ } old = { .quad = 0}, new = { .quad = 0 };
-+ unsigned int off_in_quad = sizeof(unsigned __int128) - mop->size;
++ unsigned int off_in_quad = sizeof(__uint128_t) - mop->size;
u64 supported_flags;
void *tmpbuf = NULL;
int r, srcu_idx;
2: 6cb32b244899 = 2: 28637a03f63e Documentation: KVM: s390: Describe KVM_S390_MEMOP_F_CMPXCHG
3: 5f1217ad9d31 = 3: 31f45e4377c5 KVM: s390: selftest: memop: Pass mop_desc via pointer
4: 86a15b53846a = 4: dba0a8ba3ba2 KVM: s390: selftest: memop: Replace macros by functions
-: ------------ > 5: ba8cdd064c02 KVM: s390: selftest: memop: Move testlist into main
5: 49e67d7559de ! 6: 3cf7f710e5db KVM: s390: selftest: memop: Add cmpxchg tests
@@ tools/testing/selftests/kvm/s390x/memop.c: struct mop_desc {
uint8_t ar;
uint8_t key;
};
-
-+typedef unsigned __int128 uint128;
-+
- const uint8_t NO_KEY = 0xff;
-
- static struct kvm_s390_mem_op ksmo_from_desc(const struct mop_desc *desc)
@@ tools/testing/selftests/kvm/s390x/memop.c: static struct kvm_s390_mem_op ksmo_from_desc(const struct mop_desc *desc)
ksmo.flags |= KVM_S390_MEMOP_F_SKEY_PROTECTION;
ksmo.key = desc->key;
@@ tools/testing/selftests/kvm/s390x/memop.c: enum stage {
vcpu_run(__vcpu); \
get_ucall(__vcpu, &uc); \
+ if (uc.cmd == UCALL_ABORT) { \
-+ TEST_FAIL("line %lu: %s, hints: %lu, %lu", \
-+ uc.args[1], (const char *)uc.args[0], \
-+ uc.args[2], uc.args[3]); \
++ REPORT_GUEST_ASSERT_2(uc, "hints: %lu, %lu"); \
+ } \
ASSERT_EQ(uc.cmd, UCALL_SYNC); \
ASSERT_EQ(uc.args[1], __stage); \
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_copy_key(void)
+ kvm_vm_free(t.kvm_vm);
+}
+
-+static uint128 cut_to_size(int size, uint128 val)
++static __uint128_t cut_to_size(int size, __uint128_t val)
+{
+ switch (size) {
+ case 1:
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_copy_key(void)
+ return 0;
+}
+
-+static bool popcount_eq(uint128 a, uint128 b)
++static bool popcount_eq(__uint128_t a, __uint128_t b)
+{
+ unsigned int count_a, count_b;
+
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_copy_key(void)
+ return count_a == count_b;
+}
+
-+static uint128 rotate(int size, uint128 val, int amount)
++static __uint128_t rotate(int size, __uint128_t val, int amount)
+{
+ unsigned int bits = size * 8;
+
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_copy_key(void)
+ }
+}
+
-+static uint128 permutate_bits(bool guest, int i, int size, uint128 old)
++static __uint128_t permutate_bits(bool guest, int i, int size, __uint128_t old)
+{
+ unsigned int rand;
+ bool swap;
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_copy_key(void)
+ swap = rand % 2 == 0;
+ if (swap) {
+ int i, j;
-+ uint128 new;
++ __uint128_t new;
+ uint8_t byte0, byte1;
+
+ rand = rand * 3 + 1;
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_copy_key(void)
+ }
+}
+
-+static bool _cmpxchg(int size, void *target, uint128 *old_p, uint128 new)
++static bool _cmpxchg(int size, void *target, __uint128_t *old_p, __uint128_t new)
+{
+ bool ret;
+
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_copy_key(void)
+ return ret;
+ }
+ case 16: {
-+ uint128 old = *old_p;
++ __uint128_t old = *old_p;
+
+ asm volatile ("cdsg %[old],%[new],%[address]"
+ : [old] "+d" (old),
-+ [address] "+Q" (*(uint128 *)(target))
++ [address] "+Q" (*(__uint128_t *)(target))
+ : [new] "d" (new)
+ : "cc"
+ );
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_copy_key(void)
+static void guest_cmpxchg_key(void)
+{
+ int size, offset;
-+ uint128 old, new;
++ __uint128_t old, new;
+
+ set_storage_key_range(mem1, max_block, 0x10);
+ set_storage_key_range(mem2, max_block, 0x10);
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_copy_key(void)
+ return NULL;
+}
+
-+static char *quad_to_char(uint128 *quad, int size)
++static char *quad_to_char(__uint128_t *quad, int size)
+{
+ return ((char *)quad) + (sizeof(*quad) - size);
+}
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_copy_key(void)
+{
+ struct test_default t = test_default_init(guest_cmpxchg_key);
+ int size, offset;
-+ uint128 old, new;
++ __uint128_t old, new;
+ bool success;
+ pthread_t thread;
+
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_copy_key(void)
+ pthread_join(thread, NULL);
+
+ MOP(t.vcpu, LOGICAL, READ, mem2, max_block, GADDR_V(mem2));
-+ TEST_ASSERT(popcount_eq(*(uint128 *)mem1, *(uint128 *)mem2),
++ TEST_ASSERT(popcount_eq(*(__uint128_t *)mem1, *(__uint128_t *)mem2),
+ "Must retain number of set bits");
+
+ kvm_vm_free(t.kvm_vm);
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_errors_key(void)
+ HOST_SYNC(t.vcpu, STAGE_SKEYS_SET);
+
+ for (i = 1; i <= 16; i *= 2) {
-+ uint128 old = 0;
++ __uint128_t old = 0;
+
+ CHECK_N_DO(ERR_PROT_MOP, t.vm, ABSOLUTE, WRITE, mem2, i, GADDR_V(mem2),
+ CMPXCHG_OLD(&old), KEY(2));
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_errors(void)
kvm_vm_free(t.kvm_vm);
}

--struct testdef {
-- const char *name;
-- void (*test)(void);
-- int extension;
--} testlist[] = {
-- {
-- .name = "simple copy",
-- .test = test_copy,
-- },
-- {
-- .name = "generic error checks",
-- .test = test_errors,
-- },
-- {
-- .name = "copy with storage keys",
-- .test = test_copy_key,
-- .extension = 1,
-- },
-- {
-- .name = "copy with key storage protection override",
-- .test = test_copy_key_storage_prot_override,
-- .extension = 1,
-- },
-- {
-- .name = "copy with key fetch protection",
-- .test = test_copy_key_fetch_prot,
-- .extension = 1,
-- },
-- {
-- .name = "copy with key fetch protection override",
-- .test = test_copy_key_fetch_prot_override,
-- .extension = 1,
-- },
-- {
-- .name = "error checks with key",
-- .test = test_errors_key,
-- .extension = 1,
-- },
-- {
-- .name = "termination",
-- .test = test_termination,
-- .extension = 1,
-- },
-- {
-- .name = "error checks with key storage protection override",
-- .test = test_errors_key_storage_prot_override,
-- .extension = 1,
-- },
-- {
-- .name = "error checks without key fetch prot override",
-- .test = test_errors_key_fetch_prot_override_not_enabled,
-- .extension = 1,
-- },
-- {
-- .name = "error checks with key fetch prot override",
-- .test = test_errors_key_fetch_prot_override_enabled,
-- .extension = 1,
-- },
--};
+static void test_errors_cmpxchg(void)
+{
+ struct test_default t = test_default_init(guest_idle);
-+ uint128 old;
++ __uint128_t old;
+ int rv, i, power = 1;
+
+ HOST_SYNC(t.vcpu, STAGE_INITED);
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_errors(void)

int main(int argc, char *argv[])
{
- int extension_cap, idx;
-
-+ setbuf(stdout, NULL); /* Tell stdout not to buffer its content */
- TEST_REQUIRE(kvm_has_cap(KVM_CAP_S390_MEM_OP));
-+ extension_cap = kvm_check_cap(KVM_CAP_S390_MEM_OP_EXTENSION);
-
-- setbuf(stdout, NULL); /* Tell stdout not to buffer its content */
-+ struct testdef {
-+ const char *name;
-+ void (*test)(void);
-+ bool requirements_met;
-+ } testlist[] = {
-+ {
-+ .name = "simple copy",
-+ .test = test_copy,
-+ .requirements_met = true,
-+ },
-+ {
-+ .name = "generic error checks",
-+ .test = test_errors,
-+ .requirements_met = true,
-+ },
-+ {
-+ .name = "copy with storage keys",
-+ .test = test_copy_key,
-+ .requirements_met = extension_cap > 0,
-+ },
+@@ tools/testing/selftests/kvm/s390x/memop.c: int main(int argc, char *argv[])
+ .test = test_copy_key,
+ .requirements_met = extension_cap > 0,
+ },
+ {
+ .name = "cmpxchg with storage keys",
+ .test = test_cmpxchg_key,
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_errors(void)
+ .test = test_cmpxchg_key_concurrent,
+ .requirements_met = extension_cap & 0x2,
+ },
-+ {
-+ .name = "copy with key storage protection override",
-+ .test = test_copy_key_storage_prot_override,
-+ .requirements_met = extension_cap > 0,
-+ },
-+ {
-+ .name = "copy with key fetch protection",
-+ .test = test_copy_key_fetch_prot,
-+ .requirements_met = extension_cap > 0,
-+ },
-+ {
-+ .name = "copy with key fetch protection override",
-+ .test = test_copy_key_fetch_prot_override,
-+ .requirements_met = extension_cap > 0,
-+ },
-+ {
-+ .name = "error checks with key",
-+ .test = test_errors_key,
-+ .requirements_met = extension_cap > 0,
-+ },
+ {
+ .name = "copy with key storage protection override",
+ .test = test_copy_key_storage_prot_override,
+@@ tools/testing/selftests/kvm/s390x/memop.c: int main(int argc, char *argv[])
+ .test = test_errors_key,
+ .requirements_met = extension_cap > 0,
+ },
+ {
+ .name = "error checks for cmpxchg with key",
+ .test = test_errors_cmpxchg_key,
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_errors(void)
+ .test = test_errors_cmpxchg,
+ .requirements_met = extension_cap & 0x2,
+ },
-+ {
-+ .name = "termination",
-+ .test = test_termination,
-+ .requirements_met = extension_cap > 0,
-+ },
-+ {
-+ .name = "error checks with key storage protection override",
-+ .test = test_errors_key_storage_prot_override,
-+ .requirements_met = extension_cap > 0,
-+ },
-+ {
-+ .name = "error checks without key fetch prot override",
-+ .test = test_errors_key_fetch_prot_override_not_enabled,
-+ .requirements_met = extension_cap > 0,
-+ },
-+ {
-+ .name = "error checks with key fetch prot override",
-+ .test = test_errors_key_fetch_prot_override_enabled,
-+ .requirements_met = extension_cap > 0,
-+ },
-+ };
-
- ksft_print_header();
--
- ksft_set_plan(ARRAY_SIZE(testlist));
-
-- extension_cap = kvm_check_cap(KVM_CAP_S390_MEM_OP_EXTENSION);
- for (idx = 0; idx < ARRAY_SIZE(testlist); idx++) {
-- if (extension_cap >= testlist[idx].extension) {
-+ if (testlist[idx].requirements_met) {
- testlist[idx].test();
- ksft_test_result_pass("%s\n", testlist[idx].name);
- } else {
-- ksft_test_result_skip("%s - extension level %d not supported\n",
-- testlist[idx].name,
-- testlist[idx].extension);
-+ ksft_test_result_skip("%s - requirements not met (kernel has extension cap %#x\n)",
-+ testlist[idx].name, extension_cap);
- }
- }
-
+ {
+ .name = "termination",
+ .test = test_termination,
6: faad9cf03ea6 ! 7: d81d5697ba4b KVM: s390: selftest: memop: Add bad address test
@@ Commit message
Add test that tries to access, instead of CHECK_ONLY.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
+ Reviewed-by: Nico Boehr <nrb@linux.ibm.com>

## tools/testing/selftests/kvm/s390x/memop.c ##
@@ tools/testing/selftests/kvm/s390x/memop.c: static void _test_errors_common(struct test_info info, enum mop_target target, i
7: 8070036aa89a ! 8: 22c9cd9589ba KVM: s390: selftest: memop: Fix typo
@@ Commit message

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
+ Reviewed-by: Nico Boehr <nrb@linux.ibm.com>

## tools/testing/selftests/kvm/s390x/memop.c ##
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_errors_key_fetch_prot_override_enabled(void)
8: 18c423e4e3ad ! 9: 4647be0790c8 KVM: s390: selftest: memop: Fix wrong address being used in test
@@ Commit message
protection exception the test codes needs to address mem1.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
+ Reviewed-by: Nico Boehr <nrb@linux.ibm.com>

## tools/testing/selftests/kvm/s390x/memop.c ##
@@ tools/testing/selftests/kvm/s390x/memop.c: static void test_errors_key(void)

base-commit: b5b2a05e6de7b88bcfca9d4bbc8ac74e7db80c52
--
2.34.1