* [PATCH v3 00/10] KVM: s390: Do storage key checking
@ 2022-02-09 17:04 Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 01/10] s390/uaccess: Add copy_from/to_user_key functions Janis Schoetterl-Glausch
` (9 more replies)
0 siblings, 10 replies; 21+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-09 17:04 UTC (permalink / raw)
To: Christian Borntraeger, Heiko Carstens, Janosch Frank
Cc: Janis Schoetterl-Glausch, Alexander Gordeev, Claudio Imbrenda,
David Hildenbrand, Jonathan Corbet, kvm, linux-doc, linux-kernel,
linux-s390, Paolo Bonzini, Sven Schnelle, Vasily Gorbik
Check keys when emulating instructions and let user space do key checked
accesses.
User space can do so via an extension of the MEMOP IOCTL:
* allow optional key checking
* allow MEMOP on vm fd, so key checked accesses on absolute memory
become possible
v2: https://lore.kernel.org/kvm/20220207165930.1608621-1-scgl@linux.ibm.com/
v2 -> v3
* get rid of reserved bytes check in vm,vcpu memop
* minor documentation changes
* moved memop selftest patches to end of series and squashed them,
currently working on making the test pretty
v1 -> v2
* rebase
* storage key variants of _?copy_from/to_user instead of
__copy_from/to_user_key, with long key arg instead of char
* refactor protection override checks
* u8 instead of char for key argument in s390 KVM code
* add comments
* pass ar (access register) to trans_exec in access_guest_with_key
* check reserved/unused fields (backwards compatible)
* move key arg of MEMOP out of flags
* rename new MEMOP capability to KVM_CAP_S390_MEM_OP_EXTENSION
* minor changes
Janis Schoetterl-Glausch (10):
s390/uaccess: Add copy_from/to_user_key functions
KVM: s390: Honor storage keys when accessing guest memory
KVM: s390: handle_tprot: Honor storage keys
KVM: s390: selftests: Test TEST PROTECTION emulation
KVM: s390: Add optional storage key checking to MEMOP IOCTL
KVM: s390: Add vm IOCTL for key checked guest absolute memory access
KVM: s390: Rename existing vcpu memop functions
KVM: s390: Add capability for storage key extension of MEM_OP IOCTL
KVM: s390: Update api documentation for memop ioctl
KVM: s390: selftests: Test memops with storage keys
Documentation/virt/kvm/api.rst | 112 ++++-
arch/s390/include/asm/ctl_reg.h | 2 +
arch/s390/include/asm/page.h | 2 +
arch/s390/include/asm/uaccess.h | 22 +
arch/s390/kvm/gaccess.c | 250 +++++++++-
arch/s390/kvm/gaccess.h | 84 +++-
arch/s390/kvm/intercept.c | 12 +-
arch/s390/kvm/kvm-s390.c | 129 ++++-
arch/s390/kvm/priv.c | 66 +--
arch/s390/lib/uaccess.c | 81 +++-
include/uapi/linux/kvm.h | 11 +-
tools/testing/selftests/kvm/.gitignore | 1 +
tools/testing/selftests/kvm/Makefile | 1 +
tools/testing/selftests/kvm/s390x/memop.c | 558 +++++++++++++++++++---
tools/testing/selftests/kvm/s390x/tprot.c | 227 +++++++++
15 files changed, 1372 insertions(+), 186 deletions(-)
create mode 100644 tools/testing/selftests/kvm/s390x/tprot.c
diff against v2, a bit more readable than the range-diff below:
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 7b28657fe9de..2d131af44576 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -3683,7 +3683,7 @@ The fields in each entry are defined as follows:
4.89 KVM_S390_MEM_OP
--------------------
-:Capability: KVM_CAP_S390_MEM_OP, KVM_CAP_S390_MEM_OP_EXTENSION
+:Capability: KVM_CAP_S390_MEM_OP, KVM_CAP_S390_PROTECTED, KVM_CAP_S390_MEM_OP_EXTENSION
:Architectures: s390
:Type: vm ioctl, vcpu ioctl
:Parameters: struct kvm_s390_mem_op (in)
@@ -3706,10 +3706,10 @@ Parameters are specified via the following structure::
union {
struct {
__u8 ar; /* the access register number */
- __u8 key; /* access key to use for storage key protection */
+ __u8 key; /* access key, ignored if flag unset */
};
__u32 sida_offset; /* offset into the sida */
- __u8 reserved[32]; /* must be set to 0 */
+ __u8 reserved[32]; /* ignored */
};
};
@@ -3720,9 +3720,8 @@ KVM_CAP_S390_MEM_OP capability. "buf" is the buffer supplied by the
userspace application where the read data should be written to for
a read access, or where the data that should be written is stored for
a write access. The "reserved" field is meant for future extensions.
-Reserved and unused bytes must be set to 0. If any of the following are used,
-this is enforced and -EINVAL will be returned:
-``KVM_S390_MEMOP_ABSOLUTE_READ/WRITE``, ``KVM_S390_MEMOP_F_SKEY_PROTECTION``.
+Reserved and unused values are ignored. Future extension that add members must
+introduce new flags.
The type of operation is specified in the "op" field. Flags modifying
their behavior can be set in the "flags" field. Undefined flag bits must
@@ -3792,6 +3791,7 @@ SIDA read/write:
Access the secure instruction data area which contains memory operands necessary
for instruction emulation for secure guests.
+SIDA accesses are available if the KVM_CAP_S390_PROTECTED capability is available.
SIDA accesses are permitted for the VCPU ioctl only.
SIDA accesses are permitted for secure guests only.
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 4a502cac068c..5b387e75cb5b 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2371,7 +2371,6 @@ static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
void __user *uaddr = (void __user *)mop->buf;
u64 supported_flags;
void *tmpbuf = NULL;
- u8 access_key = 0;
int r, srcu_idx;
supported_flags = KVM_S390_MEMOP_F_SKEY_PROTECTION
@@ -2382,19 +2381,15 @@ static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
return -E2BIG;
if (kvm_s390_pv_is_protected(kvm))
return -EINVAL;
+ if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
+ if (access_key_invalid(mop->key))
+ return -EINVAL;
+ }
if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
tmpbuf = vmalloc(mop->size);
if (!tmpbuf)
return -ENOMEM;
}
- if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
- access_key = mop->key;
- mop->key = 0;
- if (access_key_invalid(access_key))
- return -EINVAL;
- }
- if (memchr_inv(&mop->reserved, 0, sizeof(mop->reserved)))
- return -EINVAL;
srcu_idx = srcu_read_lock(&kvm->srcu);
@@ -2406,10 +2401,10 @@ static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
switch (mop->op) {
case KVM_S390_MEMOP_ABSOLUTE_READ: {
if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
- r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_FETCH, access_key);
+ r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_FETCH, mop->key);
} else {
r = access_guest_abs_with_key(kvm, mop->gaddr, tmpbuf,
- mop->size, GACC_FETCH, access_key);
+ mop->size, GACC_FETCH, mop->key);
if (r == 0) {
if (copy_to_user(uaddr, tmpbuf, mop->size))
r = -EFAULT;
@@ -2419,14 +2414,14 @@ static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
}
case KVM_S390_MEMOP_ABSOLUTE_WRITE: {
if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
- r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_STORE, access_key);
+ r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_STORE, mop->key);
} else {
if (copy_from_user(tmpbuf, uaddr, mop->size)) {
r = -EFAULT;
break;
}
r = access_guest_abs_with_key(kvm, mop->gaddr, tmpbuf,
- mop->size, GACC_STORE, access_key);
+ mop->size, GACC_STORE, mop->key);
}
break;
}
@@ -4779,54 +4774,37 @@ static long kvm_s390_vcpu_mem_op(struct kvm_vcpu *vcpu,
struct kvm_s390_mem_op *mop)
{
void __user *uaddr = (void __user *)mop->buf;
- u8 access_key = 0, ar = 0;
void *tmpbuf = NULL;
- bool check_reserved;
int r = 0;
const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION
| KVM_S390_MEMOP_F_CHECK_ONLY
| KVM_S390_MEMOP_F_SKEY_PROTECTION;
- if (mop->flags & ~supported_flags || !mop->size)
+ if (mop->flags & ~supported_flags || mop->ar >= NUM_ACRS || !mop->size)
return -EINVAL;
if (mop->size > MEM_OP_MAX_SIZE)
return -E2BIG;
if (kvm_s390_pv_cpu_is_protected(vcpu))
return -EINVAL;
+ if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
+ if (access_key_invalid(mop->key))
+ return -EINVAL;
+ }
if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
tmpbuf = vmalloc(mop->size);
if (!tmpbuf)
return -ENOMEM;
}
- ar = mop->ar;
- mop->ar = 0;
- if (ar >= NUM_ACRS)
- return -EINVAL;
- if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
- access_key = mop->key;
- mop->key = 0;
- if (access_key_invalid(access_key))
- return -EINVAL;
- }
- /*
- * Check that reserved/unused == 0, but only for extensions,
- * so we stay backward compatible.
- * This gives us more design flexibility for future extensions, i.e.
- * we can add functionality without adding a flag.
- */
- check_reserved = mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION;
- if (check_reserved && memchr_inv(&mop->reserved, 0, sizeof(mop->reserved)))
- return -EINVAL;
switch (mop->op) {
case KVM_S390_MEMOP_LOGICAL_READ:
if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
- r = check_gva_range(vcpu, mop->gaddr, ar, mop->size,
- GACC_FETCH, access_key);
+ r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
+ GACC_FETCH, mop->key);
break;
}
- r = read_guest_with_key(vcpu, mop->gaddr, ar, tmpbuf,
- mop->size, access_key);
+ r = read_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
+ mop->size, mop->key);
if (r == 0) {
if (copy_to_user(uaddr, tmpbuf, mop->size))
r = -EFAULT;
@@ -4834,16 +4812,16 @@ static long kvm_s390_vcpu_mem_op(struct kvm_vcpu *vcpu,
break;
case KVM_S390_MEMOP_LOGICAL_WRITE:
if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
- r = check_gva_range(vcpu, mop->gaddr, ar, mop->size,
- GACC_STORE, access_key);
+ r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
+ GACC_STORE, mop->key);
break;
}
if (copy_from_user(tmpbuf, uaddr, mop->size)) {
r = -EFAULT;
break;
}
- r = write_guest_with_key(vcpu, mop->gaddr, ar, tmpbuf,
- mop->size, access_key);
+ r = write_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
+ mop->size, mop->key);
break;
}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 50ce814267b3..fd01fe04a183 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -564,10 +564,10 @@ struct kvm_s390_mem_op {
union {
struct {
__u8 ar; /* the access register number */
- __u8 key; /* access key to use for storage key protection */
+ __u8 key; /* access key, ignored if flag unset */
};
__u32 sida_offset; /* offset into the sida */
- __u8 reserved[32]; /* must be set to 0 */
+ __u8 reserved[32]; /* ignored */
};
};
/* types for kvm_s390_mem_op->op */
Range-diff against v2:
1: bcbcf21bdc2f ! 1: 0049c4412978 s390/uaccess: Add copy_from/to_user_key functions
@@ Commit message
existing implementation.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
+ Acked-by: Heiko Carstens <hca@linux.ibm.com>
+ Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
+ Acked-by: Janosch Frank <frankja@linux.ibm.com>
## arch/s390/include/asm/uaccess.h ##
@@ arch/s390/include/asm/uaccess.h: raw_copy_to_user(void __user *to, const void *from, unsigned long n);
2: d634b7e34245 ! 2: 296096b9a7b9 KVM: s390: Honor storage keys when accessing guest memory
@@ Commit message
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
+ Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
## arch/s390/include/asm/ctl_reg.h ##
@@
3: dc1f00356bf5 = 3: a5976cb3a147 KVM: s390: handle_tprot: Honor storage keys
4: 6eac8a0f969a ! 4: 5f5e056e66df KVM: s390: selftests: Test TEST PROTECTION emulation
@@ Commit message
Trigger this by protecting the test pages via mprotect.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
+ Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
## tools/testing/selftests/kvm/.gitignore ##
@@
5: ccd4ec096613 ! 5: 64fa17a83b26 KVM: s390: Add optional storage key checking to MEMOP IOCTL
@@ arch/s390/kvm/kvm-s390.c: static int kvm_s390_handle_pv(struct kvm *kvm, struct
unsigned int ioctl, unsigned long arg)
{
@@ arch/s390/kvm/kvm-s390.c: static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
- struct kvm_s390_mem_op *mop)
- {
- void __user *uaddr = (void __user *)mop->buf;
-+ u8 access_key = 0, ar = 0;
void *tmpbuf = NULL;
-+ bool check_reserved;
int r = 0;
const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION
- | KVM_S390_MEMOP_F_CHECK_ONLY;
+ | KVM_S390_MEMOP_F_CHECK_ONLY
+ | KVM_S390_MEMOP_F_SKEY_PROTECTION;
-- if (mop->flags & ~supported_flags || mop->ar >= NUM_ACRS || !mop->size)
-+ if (mop->flags & ~supported_flags || !mop->size)
+ if (mop->flags & ~supported_flags || mop->ar >= NUM_ACRS || !mop->size)
return -EINVAL;
-
if (mop->size > MEM_OP_MAX_SIZE)
@@ arch/s390/kvm/kvm-s390.c: static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcp
if (kvm_s390_pv_cpu_is_protected(vcpu))
return -EINVAL;
-
- if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
- tmpbuf = vmalloc(mop->size);
- if (!tmpbuf)
- return -ENOMEM;
- }
-+ ar = mop->ar;
-+ mop->ar = 0;
-+ if (ar >= NUM_ACRS)
-+ return -EINVAL;
+ if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
-+ access_key = mop->key;
-+ mop->key = 0;
-+ if (access_key_invalid(access_key))
++ if (access_key_invalid(mop->key))
+ return -EINVAL;
+ }
-+ /*
-+ * Check that reserved/unused == 0, but only for extensions,
-+ * so we stay backward compatible.
-+ * This gives us more design flexibility for future extensions, i.e.
-+ * we can add functionality without adding a flag.
-+ */
-+ check_reserved = mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION;
-+ if (check_reserved && memchr_inv(&mop->reserved, 0, sizeof(mop->reserved)))
-+ return -EINVAL;
-
+ if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
+ tmpbuf = vmalloc(mop->size);
+ if (!tmpbuf)
+@@ arch/s390/kvm/kvm-s390.c: static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
switch (mop->op) {
case KVM_S390_MEMOP_LOGICAL_READ:
if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
- r = check_gva_range(vcpu, mop->gaddr, mop->ar,
- mop->size, GACC_FETCH, 0);
-+ r = check_gva_range(vcpu, mop->gaddr, ar, mop->size,
-+ GACC_FETCH, access_key);
++ r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
++ GACC_FETCH, mop->key);
break;
}
- r = read_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
-+ r = read_guest_with_key(vcpu, mop->gaddr, ar, tmpbuf,
-+ mop->size, access_key);
++ r = read_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
++ mop->size, mop->key);
if (r == 0) {
if (copy_to_user(uaddr, tmpbuf, mop->size))
r = -EFAULT;
@@ arch/s390/kvm/kvm-s390.c: static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcp
if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
- r = check_gva_range(vcpu, mop->gaddr, mop->ar,
- mop->size, GACC_STORE, 0);
-+ r = check_gva_range(vcpu, mop->gaddr, ar, mop->size,
-+ GACC_STORE, access_key);
++ r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
++ GACC_STORE, mop->key);
break;
}
if (copy_from_user(tmpbuf, uaddr, mop->size)) {
@@ arch/s390/kvm/kvm-s390.c: static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcp
break;
}
- r = write_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
-+ r = write_guest_with_key(vcpu, mop->gaddr, ar, tmpbuf,
-+ mop->size, access_key);
++ r = write_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
++ mop->size, mop->key);
break;
}
@@ include/uapi/linux/kvm.h: struct kvm_s390_mem_op {
- __u8 ar; /* the access register number */
+ struct {
+ __u8 ar; /* the access register number */
-+ __u8 key; /* access key to use for storage key protection */
++ __u8 key; /* access key, ignored if flag unset */
+ };
__u32 sida_offset; /* offset into the sida */
-- __u8 reserved[32]; /* should be set to 0 */
-+ __u8 reserved[32]; /* must be set to 0 */
+ __u8 reserved[32]; /* should be set to 0 */
};
- };
- /* types for kvm_s390_mem_op->op */
@@ include/uapi/linux/kvm.h: struct kvm_s390_mem_op {
/* flags for kvm_s390_mem_op->flags */
#define KVM_S390_MEMOP_F_CHECK_ONLY (1ULL << 0)
6: 9b99399f7958 ! 6: 57e3ad332677 KVM: s390: Add vm IOCTL for key checked guest absolute memory access
@@ arch/s390/kvm/kvm-s390.c: static bool access_key_invalid(u8 access_key)
+ void __user *uaddr = (void __user *)mop->buf;
+ u64 supported_flags;
+ void *tmpbuf = NULL;
-+ u8 access_key = 0;
+ int r, srcu_idx;
+
+ supported_flags = KVM_S390_MEMOP_F_SKEY_PROTECTION
@@ arch/s390/kvm/kvm-s390.c: static bool access_key_invalid(u8 access_key)
+ return -E2BIG;
+ if (kvm_s390_pv_is_protected(kvm))
+ return -EINVAL;
++ if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
++ if (access_key_invalid(mop->key))
++ return -EINVAL;
++ }
+ if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
+ tmpbuf = vmalloc(mop->size);
+ if (!tmpbuf)
+ return -ENOMEM;
+ }
-+ if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
-+ access_key = mop->key;
-+ mop->key = 0;
-+ if (access_key_invalid(access_key))
-+ return -EINVAL;
-+ }
-+ if (memchr_inv(&mop->reserved, 0, sizeof(mop->reserved)))
-+ return -EINVAL;
+
+ srcu_idx = srcu_read_lock(&kvm->srcu);
+
@@ arch/s390/kvm/kvm-s390.c: static bool access_key_invalid(u8 access_key)
+ switch (mop->op) {
+ case KVM_S390_MEMOP_ABSOLUTE_READ: {
+ if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
-+ r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_FETCH, access_key);
++ r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_FETCH, mop->key);
+ } else {
+ r = access_guest_abs_with_key(kvm, mop->gaddr, tmpbuf,
-+ mop->size, GACC_FETCH, access_key);
++ mop->size, GACC_FETCH, mop->key);
+ if (r == 0) {
+ if (copy_to_user(uaddr, tmpbuf, mop->size))
+ r = -EFAULT;
@@ arch/s390/kvm/kvm-s390.c: static bool access_key_invalid(u8 access_key)
+ }
+ case KVM_S390_MEMOP_ABSOLUTE_WRITE: {
+ if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
-+ r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_STORE, access_key);
++ r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_STORE, mop->key);
+ } else {
+ if (copy_from_user(tmpbuf, uaddr, mop->size)) {
+ r = -EFAULT;
+ break;
+ }
+ r = access_guest_abs_with_key(kvm, mop->gaddr, tmpbuf,
-+ mop->size, GACC_STORE, access_key);
++ mop->size, GACC_STORE, mop->key);
+ }
+ break;
+ }
7: e3047ac90594 ! 7: 1615f5ab6e30 KVM: s390: Rename existing vcpu memop functions
@@ arch/s390/kvm/kvm-s390.c: static long kvm_s390_guest_sida_op(struct kvm_vcpu *vc
+ struct kvm_s390_mem_op *mop)
{
void __user *uaddr = (void __user *)mop->buf;
- u8 access_key = 0, ar = 0;
+ void *tmpbuf = NULL;
@@ arch/s390/kvm/kvm-s390.c: static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
return r;
}
9: f93003ab633d ! 8: a8420e0f1b7f KVM: s390: Add capability for storage key extension of MEM_OP IOCTL
@@ Commit message
* The vm MEM_OP IOCTL exists.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
+ Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
## arch/s390/kvm/kvm-s390.c ##
@@ arch/s390/kvm/kvm-s390.c: int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
11: 20476660a710 ! 9: c59952ee362b KVM: s390: Update api documentation for memop ioctl
@@ Documentation/virt/kvm/api.rst: The fields in each entry are defined as follows:
--------------------
-:Capability: KVM_CAP_S390_MEM_OP
-+:Capability: KVM_CAP_S390_MEM_OP, KVM_CAP_S390_MEM_OP_EXTENSION
++:Capability: KVM_CAP_S390_MEM_OP, KVM_CAP_S390_PROTECTED, KVM_CAP_S390_MEM_OP_EXTENSION
:Architectures: s390
-:Type: vcpu ioctl
+:Type: vm ioctl, vcpu ioctl
@@ Documentation/virt/kvm/api.rst: Parameters are specified via the following struc
+ union {
+ struct {
+ __u8 ar; /* the access register number */
-+ __u8 key; /* access key to use for storage key protection */
++ __u8 key; /* access key, ignored if flag unset */
+ };
+ __u32 sida_offset; /* offset into the sida */
-+ __u8 reserved[32]; /* must be set to 0 */
++ __u8 reserved[32]; /* ignored */
+ };
};
@@ Documentation/virt/kvm/api.rst: Parameters are specified via the following struc
-register number to be used; the valid range is 0..15.
+a read access, or where the data that should be written is stored for
+a write access. The "reserved" field is meant for future extensions.
-+Reserved and unused bytes must be set to 0. If any of the following are used,
-+this is enforced and -EINVAL will be returned:
-+``KVM_S390_MEMOP_ABSOLUTE_READ/WRITE``, ``KVM_S390_MEMOP_F_SKEY_PROTECTION``.
++Reserved and unused values are ignored. Future extension that add members must
++introduce new flags.
+
+The type of operation is specified in the "op" field. Flags modifying
+their behavior can be set in the "flags" field. Undefined flag bits must
@@ Documentation/virt/kvm/api.rst: Parameters are specified via the following struc
+
+Access the secure instruction data area which contains memory operands necessary
+for instruction emulation for secure guests.
++SIDA accesses are available if the KVM_CAP_S390_PROTECTED capability is available.
+SIDA accesses are permitted for the VCPU ioctl only.
+SIDA accesses are permitted for secure guests only.
@@ Documentation/virt/kvm/api.rst: Parameters are specified via the following struc
4.90 KVM_S390_GET_SKEYS
-----------------------
+
+ ## include/uapi/linux/kvm.h ##
+@@ include/uapi/linux/kvm.h: struct kvm_s390_mem_op {
+ __u8 key; /* access key, ignored if flag unset */
+ };
+ __u32 sida_offset; /* offset into the sida */
+- __u8 reserved[32]; /* should be set to 0 */
++ __u8 reserved[32]; /* ignored */
+ };
+ };
+ /* types for kvm_s390_mem_op->op */
8: 058a6fbaf7dc ! 10: 68752e1eca95 KVM: s390: selftests: Test memops with storage keys
@@ tools/testing/selftests/kvm/s390x/memop.c
+ };
+
+ return _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, &ksmo);
- }
-
++}
++
+static void vcpu_read_guest(struct kvm_vm *vm, void *host_addr,
+ uintptr_t guest_addr, size_t len)
+{
@@ tools/testing/selftests/kvm/s390x/memop.c
+
+ rv = _vm_write_guest_key(vm, guest_addr, host_addr, len, access_key);
+ TEST_ASSERT(rv == 0, "vm memop write failed: reason = %d\n", rv);
-+}
-+
+ }
+
+enum access_mode {
+ ACCESS_READ,
+ ACCESS_WRITE
@@ tools/testing/selftests/kvm/s390x/memop.c
struct kvm_run *run;
struct kvm_s390_mem_op ksmo;
- int rv, i, maxsize;
++ bool has_skey_ext;
+ vm_vaddr_t guest_mem1;
+ vm_vaddr_t guest_mem2;
+ vm_paddr_t guest_mem1_abs;
@@ tools/testing/selftests/kvm/s390x/memop.c
setbuf(stdout, NULL); /* Tell stdout not to buffer its content */
@@ tools/testing/selftests/kvm/s390x/memop.c: int main(int argc, char *argv[])
+ }
+ if (maxsize > sizeof(mem1))
+ maxsize = sizeof(mem1);
++ has_skey_ext = kvm_check_cap(KVM_CAP_S390_MEM_OP_EXTENSION);
++ if (!has_skey_ext)
++ print_skip("Storage key extension not supported");
+
/* Create VM */
vm = vm_create_default(VCPU_ID, 0, guest_code);
run = vcpu_state(vm, VCPU_ID);
@@ tools/testing/selftests/kvm/s390x/memop.c: int main(int argc, char *argv[])
- "Unexpected exit reason: %u (%s)\n",
- run->exit_reason,
- exit_reason_str(run->exit_reason));
-+ {
++ if (has_skey_ext) {
+ vm_vaddr_t guest_0_page = vm_vaddr_alloc(vm, PAGE_SIZE, 0);
+ vm_vaddr_t guest_last_page = vm_vaddr_alloc(vm, PAGE_SIZE, last_page_addr);
+ vm_paddr_t guest_mem2_abs = addr_gva2gpa(vm, guest_mem2);
@@ tools/testing/selftests/kvm/s390x/memop.c: int main(int argc, char *argv[])
+ TEST_ASSERT(rv != 0, "Fetch should result in exception");
+ rv = _vm_read_guest_key(vm, mem2, addr_gva2gpa(vm, 0), 2048, 2);
+ TEST_ASSERT(rv == 4, "Fetch should result in protection exception");
++ } else {
++ struct ucall uc;
++
++ do {
++ vcpu_run(vm, VCPU_ID);
++ get_ucall(vm, VCPU_ID, &uc);
++ ASSERT_EQ(uc.cmd, UCALL_SYNC);
++ } while (uc.args[1] < 100);
+ }
+
+ /* Check error conditions */
10: 2ff8d7f47ffd < -: ------------ KVM: s390: selftests: Make use of capability in MEM_OP test
base-commit: dcb85f85fa6f142aae1fe86f399d4503d49f2b60
--
2.32.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 01/10] s390/uaccess: Add copy_from/to_user_key functions
2022-02-09 17:04 [PATCH v3 00/10] KVM: s390: Do storage key checking Janis Schoetterl-Glausch
@ 2022-02-09 17:04 ` Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 02/10] KVM: s390: Honor storage keys when accessing guest memory Janis Schoetterl-Glausch
` (8 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-09 17:04 UTC (permalink / raw)
To: Christian Borntraeger, Heiko Carstens, Janosch Frank
Cc: Janis Schoetterl-Glausch, Alexander Gordeev, Claudio Imbrenda,
David Hildenbrand, Jonathan Corbet, kvm, linux-doc, linux-kernel,
linux-s390, Paolo Bonzini, Sven Schnelle, Vasily Gorbik
Add copy_from/to_user_key functions, which perform storage key checking.
These functions can be used by KVM for emulating instructions that need
to be key checked.
These functions differ from their non _key counterparts in
include/linux/uaccess.h only in the additional key argument and must be
kept in sync with those.
Since the existing uaccess implementation on s390 makes use of move
instructions that support having an additional access key supplied,
we can implement raw_copy_from/to_user_key by enhancing the
existing implementation.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Acked-by: Heiko Carstens <hca@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Acked-by: Janosch Frank <frankja@linux.ibm.com>
---
arch/s390/include/asm/uaccess.h | 22 +++++++++
arch/s390/lib/uaccess.c | 81 +++++++++++++++++++++++++--------
2 files changed, 85 insertions(+), 18 deletions(-)
diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h
index d74e26b48604..ba1bcb91af95 100644
--- a/arch/s390/include/asm/uaccess.h
+++ b/arch/s390/include/asm/uaccess.h
@@ -44,6 +44,28 @@ raw_copy_to_user(void __user *to, const void *from, unsigned long n);
#define INLINE_COPY_TO_USER
#endif
+unsigned long __must_check
+_copy_from_user_key(void *to, const void __user *from, unsigned long n, unsigned long key);
+
+static __always_inline unsigned long __must_check
+copy_from_user_key(void *to, const void __user *from, unsigned long n, unsigned long key)
+{
+ if (likely(check_copy_size(to, n, false)))
+ n = _copy_from_user_key(to, from, n, key);
+ return n;
+}
+
+unsigned long __must_check
+_copy_to_user_key(void __user *to, const void *from, unsigned long n, unsigned long key);
+
+static __always_inline unsigned long __must_check
+copy_to_user_key(void __user *to, const void *from, unsigned long n, unsigned long key)
+{
+ if (likely(check_copy_size(from, n, true)))
+ n = _copy_to_user_key(to, from, n, key);
+ return n;
+}
+
int __put_user_bad(void) __attribute__((noreturn));
int __get_user_bad(void) __attribute__((noreturn));
diff --git a/arch/s390/lib/uaccess.c b/arch/s390/lib/uaccess.c
index 8a5d21461889..b709239feb5d 100644
--- a/arch/s390/lib/uaccess.c
+++ b/arch/s390/lib/uaccess.c
@@ -59,11 +59,13 @@ static inline int copy_with_mvcos(void)
#endif
static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr,
- unsigned long size)
+ unsigned long size, unsigned long key)
{
unsigned long tmp1, tmp2;
union oac spec = {
+ .oac2.key = key,
.oac2.as = PSW_BITS_AS_SECONDARY,
+ .oac2.k = 1,
.oac2.a = 1,
};
@@ -94,19 +96,19 @@ static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr
}
static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
- unsigned long size)
+ unsigned long size, unsigned long key)
{
unsigned long tmp1, tmp2;
tmp1 = -256UL;
asm volatile(
" sacf 0\n"
- "0: mvcp 0(%0,%2),0(%1),%3\n"
+ "0: mvcp 0(%0,%2),0(%1),%[key]\n"
"7: jz 5f\n"
"1: algr %0,%3\n"
" la %1,256(%1)\n"
" la %2,256(%2)\n"
- "2: mvcp 0(%0,%2),0(%1),%3\n"
+ "2: mvcp 0(%0,%2),0(%1),%[key]\n"
"8: jnz 1b\n"
" j 5f\n"
"3: la %4,255(%1)\n" /* %4 = ptr + 255 */
@@ -115,7 +117,7 @@ static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
" slgr %4,%1\n"
" clgr %0,%4\n" /* copy crosses next page boundary? */
" jnh 6f\n"
- "4: mvcp 0(%4,%2),0(%1),%3\n"
+ "4: mvcp 0(%4,%2),0(%1),%[key]\n"
"9: slgr %0,%4\n"
" j 6f\n"
"5: slgr %0,%0\n"
@@ -123,24 +125,49 @@ static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
EX_TABLE(0b,3b) EX_TABLE(2b,3b) EX_TABLE(4b,6b)
EX_TABLE(7b,3b) EX_TABLE(8b,3b) EX_TABLE(9b,6b)
: "+a" (size), "+a" (ptr), "+a" (x), "+a" (tmp1), "=a" (tmp2)
- : : "cc", "memory");
+ : [key] "d" (key << 4)
+ : "cc", "memory");
return size;
}
-unsigned long raw_copy_from_user(void *to, const void __user *from, unsigned long n)
+static unsigned long raw_copy_from_user_key(void *to, const void __user *from,
+ unsigned long n, unsigned long key)
{
if (copy_with_mvcos())
- return copy_from_user_mvcos(to, from, n);
- return copy_from_user_mvcp(to, from, n);
+ return copy_from_user_mvcos(to, from, n, key);
+ return copy_from_user_mvcp(to, from, n, key);
+}
+
+unsigned long raw_copy_from_user(void *to, const void __user *from, unsigned long n)
+{
+ return raw_copy_from_user_key(to, from, n, 0);
}
EXPORT_SYMBOL(raw_copy_from_user);
+unsigned long _copy_from_user_key(void *to, const void __user *from,
+ unsigned long n, unsigned long key)
+{
+ unsigned long res = n;
+
+ might_fault();
+ if (!should_fail_usercopy()) {
+ instrument_copy_from_user(to, from, n);
+ res = raw_copy_from_user_key(to, from, n, key);
+ }
+ if (unlikely(res))
+ memset(to + (n - res), 0, res);
+ return res;
+}
+EXPORT_SYMBOL(_copy_from_user_key);
+
static inline unsigned long copy_to_user_mvcos(void __user *ptr, const void *x,
- unsigned long size)
+ unsigned long size, unsigned long key)
{
unsigned long tmp1, tmp2;
union oac spec = {
+ .oac1.key = key,
.oac1.as = PSW_BITS_AS_SECONDARY,
+ .oac1.k = 1,
.oac1.a = 1,
};
@@ -171,19 +198,19 @@ static inline unsigned long copy_to_user_mvcos(void __user *ptr, const void *x,
}
static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
- unsigned long size)
+ unsigned long size, unsigned long key)
{
unsigned long tmp1, tmp2;
tmp1 = -256UL;
asm volatile(
" sacf 0\n"
- "0: mvcs 0(%0,%1),0(%2),%3\n"
+ "0: mvcs 0(%0,%1),0(%2),%[key]\n"
"7: jz 5f\n"
"1: algr %0,%3\n"
" la %1,256(%1)\n"
" la %2,256(%2)\n"
- "2: mvcs 0(%0,%1),0(%2),%3\n"
+ "2: mvcs 0(%0,%1),0(%2),%[key]\n"
"8: jnz 1b\n"
" j 5f\n"
"3: la %4,255(%1)\n" /* %4 = ptr + 255 */
@@ -192,7 +219,7 @@ static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
" slgr %4,%1\n"
" clgr %0,%4\n" /* copy crosses next page boundary? */
" jnh 6f\n"
- "4: mvcs 0(%4,%1),0(%2),%3\n"
+ "4: mvcs 0(%4,%1),0(%2),%[key]\n"
"9: slgr %0,%4\n"
" j 6f\n"
"5: slgr %0,%0\n"
@@ -200,18 +227,36 @@ static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
EX_TABLE(0b,3b) EX_TABLE(2b,3b) EX_TABLE(4b,6b)
EX_TABLE(7b,3b) EX_TABLE(8b,3b) EX_TABLE(9b,6b)
: "+a" (size), "+a" (ptr), "+a" (x), "+a" (tmp1), "=a" (tmp2)
- : : "cc", "memory");
+ : [key] "d" (key << 4)
+ : "cc", "memory");
return size;
}
-unsigned long raw_copy_to_user(void __user *to, const void *from, unsigned long n)
+static unsigned long raw_copy_to_user_key(void __user *to, const void *from,
+ unsigned long n, unsigned long key)
{
if (copy_with_mvcos())
- return copy_to_user_mvcos(to, from, n);
- return copy_to_user_mvcs(to, from, n);
+ return copy_to_user_mvcos(to, from, n, key);
+ return copy_to_user_mvcs(to, from, n, key);
+}
+
+unsigned long raw_copy_to_user(void __user *to, const void *from, unsigned long n)
+{
+ return raw_copy_to_user_key(to, from, n, 0);
}
EXPORT_SYMBOL(raw_copy_to_user);
+unsigned long _copy_to_user_key(void __user *to, const void *from,
+ unsigned long n, unsigned long key)
+{
+ might_fault();
+ if (should_fail_usercopy())
+ return n;
+ instrument_copy_to_user(to, from, n);
+ return raw_copy_to_user_key(to, from, n, key);
+}
+EXPORT_SYMBOL(_copy_to_user_key);
+
static inline unsigned long clear_user_mvcos(void __user *to, unsigned long size)
{
unsigned long tmp1, tmp2;
--
2.32.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 02/10] KVM: s390: Honor storage keys when accessing guest memory
2022-02-09 17:04 [PATCH v3 00/10] KVM: s390: Do storage key checking Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 01/10] s390/uaccess: Add copy_from/to_user_key functions Janis Schoetterl-Glausch
@ 2022-02-09 17:04 ` Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 03/10] KVM: s390: handle_tprot: Honor storage keys Janis Schoetterl-Glausch
` (7 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-09 17:04 UTC (permalink / raw)
To: Christian Borntraeger, Heiko Carstens, Janosch Frank
Cc: Janis Schoetterl-Glausch, Alexander Gordeev, Claudio Imbrenda,
David Hildenbrand, Jonathan Corbet, kvm, linux-doc, linux-kernel,
linux-s390, Paolo Bonzini, Sven Schnelle, Vasily Gorbik
Storage key checking had not been implemented for instructions emulated
by KVM. Implement it by enhancing the functions used for guest access,
in particular those making use of access_guest which has been renamed
to access_guest_with_key.
Accesses via access_guest_real should not be key checked.
For actual accesses, key checking is done by
copy_from/to_user_key (which internally uses MVCOS/MVCP/MVCS).
In cases where accessibility is checked without an actual access,
this is performed by getting the storage key and checking if the access
key matches. In both cases, if applicable, storage and fetch protection
override are honored.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
arch/s390/include/asm/ctl_reg.h | 2 +
arch/s390/include/asm/page.h | 2 +
arch/s390/kvm/gaccess.c | 187 ++++++++++++++++++++++++++++++--
arch/s390/kvm/gaccess.h | 77 +++++++++++--
arch/s390/kvm/intercept.c | 12 +-
arch/s390/kvm/kvm-s390.c | 4 +-
6 files changed, 253 insertions(+), 31 deletions(-)
diff --git a/arch/s390/include/asm/ctl_reg.h b/arch/s390/include/asm/ctl_reg.h
index 04dc65f8901d..c800199a376b 100644
--- a/arch/s390/include/asm/ctl_reg.h
+++ b/arch/s390/include/asm/ctl_reg.h
@@ -12,6 +12,8 @@
#define CR0_CLOCK_COMPARATOR_SIGN BIT(63 - 10)
#define CR0_LOW_ADDRESS_PROTECTION BIT(63 - 35)
+#define CR0_FETCH_PROTECTION_OVERRIDE BIT(63 - 38)
+#define CR0_STORAGE_PROTECTION_OVERRIDE BIT(63 - 39)
#define CR0_EMERGENCY_SIGNAL_SUBMASK BIT(63 - 49)
#define CR0_EXTERNAL_CALL_SUBMASK BIT(63 - 50)
#define CR0_CLOCK_COMPARATOR_SUBMASK BIT(63 - 52)
diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h
index d98d17a36c7b..cfc4d6fb2385 100644
--- a/arch/s390/include/asm/page.h
+++ b/arch/s390/include/asm/page.h
@@ -20,6 +20,8 @@
#define PAGE_SIZE _PAGE_SIZE
#define PAGE_MASK _PAGE_MASK
#define PAGE_DEFAULT_ACC 0
+/* storage-protection override */
+#define PAGE_SPO_ACC 9
#define PAGE_DEFAULT_KEY (PAGE_DEFAULT_ACC << 4)
#define HPAGE_SHIFT 20
diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index 4460808c3b9a..7fca0cff4c12 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -10,6 +10,7 @@
#include <linux/mm_types.h>
#include <linux/err.h>
#include <linux/pgtable.h>
+#include <linux/bitfield.h>
#include <asm/gmap.h>
#include "kvm-s390.h"
@@ -794,6 +795,79 @@ static int low_address_protection_enabled(struct kvm_vcpu *vcpu,
return 1;
}
+static bool fetch_prot_override_applicable(struct kvm_vcpu *vcpu, enum gacc_mode mode,
+ union asce asce)
+{
+ psw_t *psw = &vcpu->arch.sie_block->gpsw;
+ unsigned long override;
+
+ if (mode == GACC_FETCH || mode == GACC_IFETCH) {
+ /* check if fetch protection override enabled */
+ override = vcpu->arch.sie_block->gcr[0];
+ override &= CR0_FETCH_PROTECTION_OVERRIDE;
+ /* not applicable if subject to DAT && private space */
+ override = override && !(psw_bits(*psw).dat && asce.p);
+ return override;
+ }
+ return false;
+}
+
+static bool fetch_prot_override_applies(unsigned long ga, unsigned int len)
+{
+ return ga < 2048 && ga + len <= 2048;
+}
+
+static bool storage_prot_override_applicable(struct kvm_vcpu *vcpu)
+{
+ /* check if storage protection override enabled */
+ return vcpu->arch.sie_block->gcr[0] & CR0_STORAGE_PROTECTION_OVERRIDE;
+}
+
+static bool storage_prot_override_applies(u8 access_control)
+{
+ /* matches special storage protection override key (9) -> allow */
+ return access_control == PAGE_SPO_ACC;
+}
+
+static int vcpu_check_access_key(struct kvm_vcpu *vcpu, u8 access_key,
+ enum gacc_mode mode, union asce asce, gpa_t gpa,
+ unsigned long ga, unsigned int len)
+{
+ u8 storage_key, access_control;
+ unsigned long hva;
+ int r;
+
+ /* access key 0 matches any storage key -> allow */
+ if (access_key == 0)
+ return 0;
+ /*
+ * caller needs to ensure that gfn is accessible, so we can
+ * assume that this cannot fail
+ */
+ hva = gfn_to_hva(vcpu->kvm, gpa_to_gfn(gpa));
+ mmap_read_lock(current->mm);
+ r = get_guest_storage_key(current->mm, hva, &storage_key);
+ mmap_read_unlock(current->mm);
+ if (r)
+ return r;
+ access_control = FIELD_GET(_PAGE_ACC_BITS, storage_key);
+ /* access key matches storage key -> allow */
+ if (access_control == access_key)
+ return 0;
+ if (mode == GACC_FETCH || mode == GACC_IFETCH) {
+ /* it is a fetch and fetch protection is off -> allow */
+ if (!(storage_key & _PAGE_FP_BIT))
+ return 0;
+ if (fetch_prot_override_applicable(vcpu, mode, asce) &&
+ fetch_prot_override_applies(ga, len))
+ return 0;
+ }
+ if (storage_prot_override_applicable(vcpu) &&
+ storage_prot_override_applies(access_control))
+ return 0;
+ return PGM_PROTECTION;
+}
+
/**
* guest_range_to_gpas() - Calculate guest physical addresses of page fragments
* covering a logical range
@@ -804,6 +878,7 @@ static int low_address_protection_enabled(struct kvm_vcpu *vcpu,
* @len: length of range in bytes
* @asce: address-space-control element to use for translation
* @mode: access mode
+ * @access_key: access key to mach the range's storage keys against
*
* Translate a logical range to a series of guest absolute addresses,
* such that the concatenation of page fragments starting at each gpa make up
@@ -830,7 +905,8 @@ static int low_address_protection_enabled(struct kvm_vcpu *vcpu,
*/
static int guest_range_to_gpas(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
unsigned long *gpas, unsigned long len,
- const union asce asce, enum gacc_mode mode)
+ const union asce asce, enum gacc_mode mode,
+ u8 access_key)
{
psw_t *psw = &vcpu->arch.sie_block->gpsw;
unsigned int offset = offset_in_page(ga);
@@ -857,6 +933,10 @@ static int guest_range_to_gpas(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
}
if (rc)
return trans_exc(vcpu, rc, ga, ar, mode, prot);
+ rc = vcpu_check_access_key(vcpu, access_key, mode, asce, gpa, ga,
+ fragment_len);
+ if (rc)
+ return trans_exc(vcpu, rc, ga, ar, mode, PROT_TYPE_KEYC);
if (gpas)
*gpas++ = gpa;
offset = 0;
@@ -880,16 +960,54 @@ static int access_guest_page(struct kvm *kvm, enum gacc_mode mode, gpa_t gpa,
return rc;
}
-int access_guest(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar, void *data,
- unsigned long len, enum gacc_mode mode)
+static int
+access_guest_page_with_key(struct kvm *kvm, enum gacc_mode mode, gpa_t gpa,
+ void *data, unsigned int len, u8 access_key)
+{
+ struct kvm_memory_slot *slot;
+ bool writable;
+ gfn_t gfn;
+ hva_t hva;
+ int rc;
+
+ gfn = gpa >> PAGE_SHIFT;
+ slot = gfn_to_memslot(kvm, gfn);
+ hva = gfn_to_hva_memslot_prot(slot, gfn, &writable);
+
+ if (kvm_is_error_hva(hva))
+ return PGM_ADDRESSING;
+ /*
+ * Check if it's a ro memslot, even tho that can't occur (they're unsupported).
+ * Don't try to actually handle that case.
+ */
+ if (!writable && mode == GACC_STORE)
+ return -EOPNOTSUPP;
+ hva += offset_in_page(gpa);
+ if (mode == GACC_STORE)
+ rc = copy_to_user_key((void __user *)hva, data, len, access_key);
+ else
+ rc = copy_from_user_key(data, (void __user *)hva, len, access_key);
+ if (rc)
+ return PGM_PROTECTION;
+ if (mode == GACC_STORE)
+ mark_page_dirty_in_slot(kvm, slot, gfn);
+ return 0;
+}
+
+int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
+ void *data, unsigned long len, enum gacc_mode mode,
+ u8 access_key)
{
psw_t *psw = &vcpu->arch.sie_block->gpsw;
unsigned long nr_pages, idx;
unsigned long gpa_array[2];
unsigned int fragment_len;
unsigned long *gpas;
+ enum prot_type prot;
int need_ipte_lock;
union asce asce;
+ bool try_storage_prot_override;
+ bool try_fetch_prot_override;
int rc;
if (!len)
@@ -904,16 +1022,47 @@ int access_guest(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar, void *data,
gpas = vmalloc(array_size(nr_pages, sizeof(unsigned long)));
if (!gpas)
return -ENOMEM;
+ try_fetch_prot_override = fetch_prot_override_applicable(vcpu, mode, asce);
+ try_storage_prot_override = storage_prot_override_applicable(vcpu);
need_ipte_lock = psw_bits(*psw).dat && !asce.r;
if (need_ipte_lock)
ipte_lock(vcpu);
- rc = guest_range_to_gpas(vcpu, ga, ar, gpas, len, asce, mode);
- for (idx = 0; idx < nr_pages && !rc; idx++) {
+ /*
+ * Since we do the access further down ultimately via a move instruction
+ * that does key checking and returns an error in case of a protection
+ * violation, we don't need to do the check during address translation.
+ * Skip it by passing access key 0, which matches any storage key,
+ * obviating the need for any further checks. As a result the check is
+ * handled entirely in hardware on access, we only need to take care to
+ * forego key protection checking if fetch protection override applies or
+ * retry with the special key 9 in case of storage protection override.
+ */
+ rc = guest_range_to_gpas(vcpu, ga, ar, gpas, len, asce, mode, 0);
+ if (rc)
+ goto out_unlock;
+ for (idx = 0; idx < nr_pages; idx++) {
fragment_len = min(PAGE_SIZE - offset_in_page(gpas[idx]), len);
- rc = access_guest_page(vcpu->kvm, mode, gpas[idx], data, fragment_len);
+ if (try_fetch_prot_override && fetch_prot_override_applies(ga, fragment_len)) {
+ rc = access_guest_page(vcpu->kvm, mode, gpas[idx],
+ data, fragment_len);
+ } else {
+ rc = access_guest_page_with_key(vcpu->kvm, mode, gpas[idx],
+ data, fragment_len, access_key);
+ }
+ if (rc == PGM_PROTECTION && try_storage_prot_override)
+ rc = access_guest_page_with_key(vcpu->kvm, mode, gpas[idx],
+ data, fragment_len, PAGE_SPO_ACC);
+ if (rc == PGM_PROTECTION)
+ prot = PROT_TYPE_KEYC;
+ if (rc)
+ break;
len -= fragment_len;
data += fragment_len;
+ ga = kvm_s390_logical_to_effective(vcpu, ga + fragment_len);
}
+ if (rc > 0)
+ rc = trans_exc(vcpu, rc, ga, ar, mode, prot);
+out_unlock:
if (need_ipte_lock)
ipte_unlock(vcpu);
if (nr_pages > ARRAY_SIZE(gpa_array))
@@ -940,12 +1089,13 @@ int access_guest_real(struct kvm_vcpu *vcpu, unsigned long gra,
}
/**
- * guest_translate_address - translate guest logical into guest absolute address
+ * guest_translate_address_with_key - translate guest logical into guest absolute address
* @vcpu: virtual cpu
* @gva: Guest virtual address
* @ar: Access register
* @gpa: Guest physical address
* @mode: Translation access mode
+ * @access_key: access key to mach the storage key with
*
* Parameter semantics are the same as the ones from guest_translate.
* The memory contents at the guest address are not changed.
@@ -953,8 +1103,9 @@ int access_guest_real(struct kvm_vcpu *vcpu, unsigned long gra,
* Note: The IPTE lock is not taken during this function, so the caller
* has to take care of this.
*/
-int guest_translate_address(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
- unsigned long *gpa, enum gacc_mode mode)
+int guest_translate_address_with_key(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
+ unsigned long *gpa, enum gacc_mode mode,
+ u8 access_key)
{
union asce asce;
int rc;
@@ -963,7 +1114,17 @@ int guest_translate_address(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
rc = get_vcpu_asce(vcpu, &asce, gva, ar, mode);
if (rc)
return rc;
- return guest_range_to_gpas(vcpu, gva, ar, gpa, 1, asce, mode);
+ return guest_range_to_gpas(vcpu, gva, ar, gpa, 1, asce, mode,
+ access_key);
+}
+
+int guest_translate_address(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
+ unsigned long *gpa, enum gacc_mode mode)
+{
+ u8 access_key = psw_bits(vcpu->arch.sie_block->gpsw).key;
+
+ return guest_translate_address_with_key(vcpu, gva, ar, gpa, mode,
+ access_key);
}
/**
@@ -973,9 +1134,10 @@ int guest_translate_address(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
* @ar: Access register
* @length: Length of test range
* @mode: Translation access mode
+ * @access_key: access key to mach the storage keys with
*/
int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
- unsigned long length, enum gacc_mode mode)
+ unsigned long length, enum gacc_mode mode, u8 access_key)
{
union asce asce;
int rc = 0;
@@ -984,7 +1146,8 @@ int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
if (rc)
return rc;
ipte_lock(vcpu);
- rc = guest_range_to_gpas(vcpu, gva, ar, NULL, length, asce, mode);
+ rc = guest_range_to_gpas(vcpu, gva, ar, NULL, length, asce, mode,
+ access_key);
ipte_unlock(vcpu);
return rc;
diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
index 7c72a5e3449f..e5b2f56e7962 100644
--- a/arch/s390/kvm/gaccess.h
+++ b/arch/s390/kvm/gaccess.h
@@ -186,24 +186,31 @@ enum gacc_mode {
GACC_IFETCH,
};
+int guest_translate_address_with_key(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
+ unsigned long *gpa, enum gacc_mode mode,
+ u8 access_key);
+
int guest_translate_address(struct kvm_vcpu *vcpu, unsigned long gva,
u8 ar, unsigned long *gpa, enum gacc_mode mode);
+
int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
- unsigned long length, enum gacc_mode mode);
+ unsigned long length, enum gacc_mode mode, u8 access_key);
-int access_guest(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar, void *data,
- unsigned long len, enum gacc_mode mode);
+int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
+ void *data, unsigned long len, enum gacc_mode mode,
+ u8 access_key);
int access_guest_real(struct kvm_vcpu *vcpu, unsigned long gra,
void *data, unsigned long len, enum gacc_mode mode);
/**
- * write_guest - copy data from kernel space to guest space
+ * write_guest_with_key - copy data from kernel space to guest space
* @vcpu: virtual cpu
* @ga: guest address
* @ar: access register
* @data: source address in kernel space
* @len: number of bytes to copy
+ * @access_key: access key the storage key needs to match
*
* Copy @len bytes from @data (kernel space) to @ga (guest address).
* In order to copy data to guest space the PSW of the vcpu is inspected:
@@ -214,8 +221,8 @@ int access_guest_real(struct kvm_vcpu *vcpu, unsigned long gra,
* The addressing mode of the PSW is also inspected, so that address wrap
* around is taken into account for 24-, 31- and 64-bit addressing mode,
* if the to be copied data crosses page boundaries in guest address space.
- * In addition also low address and DAT protection are inspected before
- * copying any data (key protection is currently not implemented).
+ * In addition low address, DAT and key protection checks are performed before
+ * copying any data.
*
* This function modifies the 'struct kvm_s390_pgm_info pgm' member of @vcpu.
* In case of an access exception (e.g. protection exception) pgm will contain
@@ -243,10 +250,53 @@ int access_guest_real(struct kvm_vcpu *vcpu, unsigned long gra,
* if data has been changed in guest space in case of an exception.
*/
static inline __must_check
+int write_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
+ void *data, unsigned long len, u8 access_key)
+{
+ return access_guest_with_key(vcpu, ga, ar, data, len, GACC_STORE,
+ access_key);
+}
+
+/**
+ * write_guest - copy data from kernel space to guest space
+ * @vcpu: virtual cpu
+ * @ga: guest address
+ * @ar: access register
+ * @data: source address in kernel space
+ * @len: number of bytes to copy
+ *
+ * The behaviour of write_guest is identical to write_guest_with_key, except
+ * that the PSW access key is used instead of an explicit argument.
+ */
+static inline __must_check
int write_guest(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar, void *data,
unsigned long len)
{
- return access_guest(vcpu, ga, ar, data, len, GACC_STORE);
+ u8 access_key = psw_bits(vcpu->arch.sie_block->gpsw).key;
+
+ return write_guest_with_key(vcpu, ga, ar, data, len, access_key);
+}
+
+/**
+ * read_guest_with_key - copy data from guest space to kernel space
+ * @vcpu: virtual cpu
+ * @ga: guest address
+ * @ar: access register
+ * @data: destination address in kernel space
+ * @len: number of bytes to copy
+ * @access_key: access key the storage key needs to match
+ *
+ * Copy @len bytes from @ga (guest address) to @data (kernel space).
+ *
+ * The behaviour of read_guest_with_key is identical to write_guest_with_key,
+ * except that data will be copied from guest space to kernel space.
+ */
+static inline __must_check
+int read_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
+ void *data, unsigned long len, u8 access_key)
+{
+ return access_guest_with_key(vcpu, ga, ar, data, len, GACC_FETCH,
+ access_key);
}
/**
@@ -259,14 +309,16 @@ int write_guest(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar, void *data,
*
* Copy @len bytes from @ga (guest address) to @data (kernel space).
*
- * The behaviour of read_guest is identical to write_guest, except that
- * data will be copied from guest space to kernel space.
+ * The behaviour of read_guest is identical to read_guest_with_key, except
+ * that the PSW access key is used instead of an explicit argument.
*/
static inline __must_check
int read_guest(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar, void *data,
unsigned long len)
{
- return access_guest(vcpu, ga, ar, data, len, GACC_FETCH);
+ u8 access_key = psw_bits(vcpu->arch.sie_block->gpsw).key;
+
+ return read_guest_with_key(vcpu, ga, ar, data, len, access_key);
}
/**
@@ -287,7 +339,10 @@ static inline __must_check
int read_guest_instr(struct kvm_vcpu *vcpu, unsigned long ga, void *data,
unsigned long len)
{
- return access_guest(vcpu, ga, 0, data, len, GACC_IFETCH);
+ u8 access_key = psw_bits(vcpu->arch.sie_block->gpsw).key;
+
+ return access_guest_with_key(vcpu, ga, 0, data, len, GACC_IFETCH,
+ access_key);
}
/**
diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
index d07ff646d844..8bd42a20d924 100644
--- a/arch/s390/kvm/intercept.c
+++ b/arch/s390/kvm/intercept.c
@@ -331,18 +331,18 @@ static int handle_mvpg_pei(struct kvm_vcpu *vcpu)
kvm_s390_get_regs_rre(vcpu, ®1, ®2);
- /* Make sure that the source is paged-in */
- rc = guest_translate_address(vcpu, vcpu->run->s.regs.gprs[reg2],
- reg2, &srcaddr, GACC_FETCH);
+ /* Ensure that the source is paged-in, no actual access -> no key checking */
+ rc = guest_translate_address_with_key(vcpu, vcpu->run->s.regs.gprs[reg2],
+ reg2, &srcaddr, GACC_FETCH, 0);
if (rc)
return kvm_s390_inject_prog_cond(vcpu, rc);
rc = kvm_arch_fault_in_page(vcpu, srcaddr, 0);
if (rc != 0)
return rc;
- /* Make sure that the destination is paged-in */
- rc = guest_translate_address(vcpu, vcpu->run->s.regs.gprs[reg1],
- reg1, &dstaddr, GACC_STORE);
+ /* Ensure that the source is paged-in, no actual access -> no key checking */
+ rc = guest_translate_address_with_key(vcpu, vcpu->run->s.regs.gprs[reg1],
+ reg1, &dstaddr, GACC_STORE, 0);
if (rc)
return kvm_s390_inject_prog_cond(vcpu, rc);
rc = kvm_arch_fault_in_page(vcpu, dstaddr, 1);
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 577f1ead6a51..cf347e1a4f17 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -4711,7 +4711,7 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
case KVM_S390_MEMOP_LOGICAL_READ:
if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
r = check_gva_range(vcpu, mop->gaddr, mop->ar,
- mop->size, GACC_FETCH);
+ mop->size, GACC_FETCH, 0);
break;
}
r = read_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
@@ -4723,7 +4723,7 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
case KVM_S390_MEMOP_LOGICAL_WRITE:
if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
r = check_gva_range(vcpu, mop->gaddr, mop->ar,
- mop->size, GACC_STORE);
+ mop->size, GACC_STORE, 0);
break;
}
if (copy_from_user(tmpbuf, uaddr, mop->size)) {
--
2.32.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 03/10] KVM: s390: handle_tprot: Honor storage keys
2022-02-09 17:04 [PATCH v3 00/10] KVM: s390: Do storage key checking Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 01/10] s390/uaccess: Add copy_from/to_user_key functions Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 02/10] KVM: s390: Honor storage keys when accessing guest memory Janis Schoetterl-Glausch
@ 2022-02-09 17:04 ` Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 04/10] KVM: s390: selftests: Test TEST PROTECTION emulation Janis Schoetterl-Glausch
` (6 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-09 17:04 UTC (permalink / raw)
To: Christian Borntraeger, Heiko Carstens, Janosch Frank
Cc: Janis Schoetterl-Glausch, Alexander Gordeev, Claudio Imbrenda,
David Hildenbrand, Jonathan Corbet, kvm, linux-doc, linux-kernel,
linux-s390, Paolo Bonzini, Sven Schnelle, Vasily Gorbik
Use the access key operand to check for key protection when
translating guest addresses.
Since the translation code checks for accessing exceptions/error hvas,
we can remove the check here and simplify the control flow.
Keep checking if the memory is read-only even if such memslots are
currently not supported.
handle_tprot was the last user of guest_translate_address,
so remove it.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
arch/s390/kvm/gaccess.c | 9 ------
arch/s390/kvm/gaccess.h | 3 --
arch/s390/kvm/priv.c | 66 ++++++++++++++++++++++-------------------
3 files changed, 35 insertions(+), 43 deletions(-)
diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index 7fca0cff4c12..37838f637707 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -1118,15 +1118,6 @@ int guest_translate_address_with_key(struct kvm_vcpu *vcpu, unsigned long gva, u
access_key);
}
-int guest_translate_address(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
- unsigned long *gpa, enum gacc_mode mode)
-{
- u8 access_key = psw_bits(vcpu->arch.sie_block->gpsw).key;
-
- return guest_translate_address_with_key(vcpu, gva, ar, gpa, mode,
- access_key);
-}
-
/**
* check_gva_range - test a range of guest virtual addresses for accessibility
* @vcpu: virtual cpu
diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
index e5b2f56e7962..c5f2e7311b17 100644
--- a/arch/s390/kvm/gaccess.h
+++ b/arch/s390/kvm/gaccess.h
@@ -190,9 +190,6 @@ int guest_translate_address_with_key(struct kvm_vcpu *vcpu, unsigned long gva, u
unsigned long *gpa, enum gacc_mode mode,
u8 access_key);
-int guest_translate_address(struct kvm_vcpu *vcpu, unsigned long gva,
- u8 ar, unsigned long *gpa, enum gacc_mode mode);
-
int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
unsigned long length, enum gacc_mode mode, u8 access_key);
diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index 417154b314a6..30b24c42ef99 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -1443,10 +1443,11 @@ int kvm_s390_handle_eb(struct kvm_vcpu *vcpu)
static int handle_tprot(struct kvm_vcpu *vcpu)
{
- u64 address1, address2;
- unsigned long hva, gpa;
- int ret = 0, cc = 0;
+ u64 address, operand2;
+ unsigned long gpa;
+ u8 access_key;
bool writable;
+ int ret, cc;
u8 ar;
vcpu->stat.instruction_tprot++;
@@ -1454,43 +1455,46 @@ static int handle_tprot(struct kvm_vcpu *vcpu)
if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_PSTATE)
return kvm_s390_inject_program_int(vcpu, PGM_PRIVILEGED_OP);
- kvm_s390_get_base_disp_sse(vcpu, &address1, &address2, &ar, NULL);
+ kvm_s390_get_base_disp_sse(vcpu, &address, &operand2, &ar, NULL);
+ access_key = (operand2 & 0xf0) >> 4;
- /* we only handle the Linux memory detection case:
- * access key == 0
- * everything else goes to userspace. */
- if (address2 & 0xf0)
- return -EOPNOTSUPP;
if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_DAT)
ipte_lock(vcpu);
- ret = guest_translate_address(vcpu, address1, ar, &gpa, GACC_STORE);
- if (ret == PGM_PROTECTION) {
+
+ ret = guest_translate_address_with_key(vcpu, address, ar, &gpa,
+ GACC_STORE, access_key);
+ if (ret == 0) {
+ gfn_to_hva_prot(vcpu->kvm, gpa_to_gfn(gpa), &writable);
+ } else if (ret == PGM_PROTECTION) {
+ writable = false;
/* Write protected? Try again with read-only... */
- cc = 1;
- ret = guest_translate_address(vcpu, address1, ar, &gpa,
- GACC_FETCH);
+ ret = guest_translate_address_with_key(vcpu, address, ar, &gpa,
+ GACC_FETCH, access_key);
}
- if (ret) {
- if (ret == PGM_ADDRESSING || ret == PGM_TRANSLATION_SPEC) {
- ret = kvm_s390_inject_program_int(vcpu, ret);
- } else if (ret > 0) {
- /* Translation not available */
- kvm_s390_set_psw_cc(vcpu, 3);
+ if (ret >= 0) {
+ cc = -1;
+
+ /* Fetching permitted; storing permitted */
+ if (ret == 0 && writable)
+ cc = 0;
+ /* Fetching permitted; storing not permitted */
+ else if (ret == 0 && !writable)
+ cc = 1;
+ /* Fetching not permitted; storing not permitted */
+ else if (ret == PGM_PROTECTION)
+ cc = 2;
+ /* Translation not available */
+ else if (ret != PGM_ADDRESSING && ret != PGM_TRANSLATION_SPEC)
+ cc = 3;
+
+ if (cc != -1) {
+ kvm_s390_set_psw_cc(vcpu, cc);
ret = 0;
+ } else {
+ ret = kvm_s390_inject_program_int(vcpu, ret);
}
- goto out_unlock;
}
- hva = gfn_to_hva_prot(vcpu->kvm, gpa_to_gfn(gpa), &writable);
- if (kvm_is_error_hva(hva)) {
- ret = kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
- } else {
- if (!writable)
- cc = 1; /* Write not permitted ==> read-only */
- kvm_s390_set_psw_cc(vcpu, cc);
- /* Note: CC2 only occurs for storage keys (not supported yet) */
- }
-out_unlock:
if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_DAT)
ipte_unlock(vcpu);
return ret;
--
2.32.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 04/10] KVM: s390: selftests: Test TEST PROTECTION emulation
2022-02-09 17:04 [PATCH v3 00/10] KVM: s390: Do storage key checking Janis Schoetterl-Glausch
` (2 preceding siblings ...)
2022-02-09 17:04 ` [PATCH v3 03/10] KVM: s390: handle_tprot: Honor storage keys Janis Schoetterl-Glausch
@ 2022-02-09 17:04 ` Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 05/10] KVM: s390: Add optional storage key checking to MEMOP IOCTL Janis Schoetterl-Glausch
` (5 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-09 17:04 UTC (permalink / raw)
To: Christian Borntraeger, Heiko Carstens, Janosch Frank
Cc: Janis Schoetterl-Glausch, Alexander Gordeev, Claudio Imbrenda,
David Hildenbrand, Jonathan Corbet, kvm, linux-doc, linux-kernel,
linux-s390, Paolo Bonzini, Sven Schnelle, Vasily Gorbik
Test the emulation of TEST PROTECTION in the presence of storage keys.
Emulation only occurs under certain conditions, one of which is the host
page being protected.
Trigger this by protecting the test pages via mprotect.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
tools/testing/selftests/kvm/.gitignore | 1 +
tools/testing/selftests/kvm/Makefile | 1 +
tools/testing/selftests/kvm/s390x/tprot.c | 227 ++++++++++++++++++++++
3 files changed, 229 insertions(+)
create mode 100644 tools/testing/selftests/kvm/s390x/tprot.c
diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index dce7de7755e6..7903580a48ac 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -8,6 +8,7 @@
/s390x/memop
/s390x/resets
/s390x/sync_regs_test
+/s390x/tprot
/x86_64/amx_test
/x86_64/cpuid_test
/x86_64/cr4_cpuid_sync_test
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 0e4926bc9a58..086f490e808d 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -121,6 +121,7 @@ TEST_GEN_PROGS_aarch64 += kvm_binary_stats_test
TEST_GEN_PROGS_s390x = s390x/memop
TEST_GEN_PROGS_s390x += s390x/resets
TEST_GEN_PROGS_s390x += s390x/sync_regs_test
+TEST_GEN_PROGS_s390x += s390x/tprot
TEST_GEN_PROGS_s390x += demand_paging_test
TEST_GEN_PROGS_s390x += dirty_log_test
TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
diff --git a/tools/testing/selftests/kvm/s390x/tprot.c b/tools/testing/selftests/kvm/s390x/tprot.c
new file mode 100644
index 000000000000..c097b9db495e
--- /dev/null
+++ b/tools/testing/selftests/kvm/s390x/tprot.c
@@ -0,0 +1,227 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Test TEST PROTECTION emulation.
+ *
+ * Copyright IBM Corp. 2021
+ */
+
+#include <sys/mman.h>
+#include "test_util.h"
+#include "kvm_util.h"
+
+#define PAGE_SHIFT 12
+#define PAGE_SIZE (1 << PAGE_SHIFT)
+#define CR0_FETCH_PROTECTION_OVERRIDE (1UL << (63 - 38))
+#define CR0_STORAGE_PROTECTION_OVERRIDE (1UL << (63 - 39))
+
+#define VCPU_ID 1
+
+static __aligned(PAGE_SIZE) uint8_t pages[2][PAGE_SIZE];
+static uint8_t *const page_store_prot = pages[0];
+static uint8_t *const page_fetch_prot = pages[1];
+
+/* Nonzero return value indicates that address not mapped */
+static int set_storage_key(void *addr, uint8_t key)
+{
+ int not_mapped = 0;
+
+ asm volatile (
+ "lra %[addr], 0(0,%[addr])\n"
+ " jz 0f\n"
+ " llill %[not_mapped],1\n"
+ " j 1f\n"
+ "0: sske %[key], %[addr]\n"
+ "1:"
+ : [addr] "+&a" (addr), [not_mapped] "+r" (not_mapped)
+ : [key] "r" (key)
+ : "cc"
+ );
+ return -not_mapped;
+}
+
+enum permission {
+ READ_WRITE = 0,
+ READ = 1,
+ RW_PROTECTED = 2,
+ TRANSL_UNAVAIL = 3,
+};
+
+static enum permission test_protection(void *addr, uint8_t key)
+{
+ uint64_t mask;
+
+ asm volatile (
+ "tprot %[addr], 0(%[key])\n"
+ " ipm %[mask]\n"
+ : [mask] "=r" (mask)
+ : [addr] "Q" (*(char *)addr),
+ [key] "a" (key)
+ : "cc"
+ );
+
+ return (enum permission)(mask >> 28);
+}
+
+enum stage {
+ STAGE_END,
+ STAGE_INIT_SIMPLE,
+ TEST_SIMPLE,
+ STAGE_INIT_FETCH_PROT_OVERRIDE,
+ TEST_FETCH_PROT_OVERRIDE,
+ TEST_STORAGE_PROT_OVERRIDE,
+};
+
+struct test {
+ enum stage stage;
+ void *addr;
+ uint8_t key;
+ enum permission expected;
+} tests[] = {
+ /*
+ * We perform each test in the array by executing TEST PROTECTION on
+ * the specified addr with the specified key and checking if the returned
+ * permissions match the expected value.
+ * Both guest and host cooperate to set up the required test conditions.
+ * A central condition is that the page targeted by addr has to be DAT
+ * protected in the host mappings, in order for KVM to emulate the
+ * TEST PROTECTION instruction.
+ * Since the page tables are shared, the host uses mprotect to achieve
+ * this.
+ *
+ * Test resulting in RW_PROTECTED/TRANSL_UNAVAIL will be interpreted
+ * by SIE, not KVM, but there is no harm in testing them also.
+ * See Enhanced Suppression-on-Protection Facilities in the
+ * Interpretive-Execution Mode
+ */
+ /*
+ * guest: set storage key of page_store_prot to 1
+ * storage key of page_fetch_prot to 9 and enable
+ * protection for it
+ * STAGE_INIT_SIMPLE
+ * host: write protect both via mprotect
+ */
+ /* access key 0 matches any storage key -> RW */
+ { TEST_SIMPLE, page_store_prot, 0x00, READ_WRITE },
+ /* access key matches storage key -> RW */
+ { TEST_SIMPLE, page_store_prot, 0x10, READ_WRITE },
+ /* mismatched keys, but no fetch protection -> RO */
+ { TEST_SIMPLE, page_store_prot, 0x20, READ },
+ /* access key 0 matches any storage key -> RW */
+ { TEST_SIMPLE, page_fetch_prot, 0x00, READ_WRITE },
+ /* access key matches storage key -> RW */
+ { TEST_SIMPLE, page_fetch_prot, 0x90, READ_WRITE },
+ /* mismatched keys, fetch protection -> inaccessible */
+ { TEST_SIMPLE, page_fetch_prot, 0x10, RW_PROTECTED },
+ /* page 0 not mapped yet -> translation not available */
+ { TEST_SIMPLE, (void *)0x00, 0x10, TRANSL_UNAVAIL },
+ /*
+ * host: try to map page 0
+ * guest: set storage key of page 0 to 9 and enable fetch protection
+ * STAGE_INIT_FETCH_PROT_OVERRIDE
+ * host: write protect page 0
+ * enable fetch protection override
+ */
+ /* mismatched keys, fetch protection, but override applies -> RO */
+ { TEST_FETCH_PROT_OVERRIDE, (void *)0x00, 0x10, READ },
+ /* mismatched keys, fetch protection, override applies to 0-2048 only -> inaccessible */
+ { TEST_FETCH_PROT_OVERRIDE, (void *)2049, 0x10, RW_PROTECTED },
+ /*
+ * host: enable storage protection override
+ */
+ /* mismatched keys, but override applies (storage key 9) -> RW */
+ { TEST_STORAGE_PROT_OVERRIDE, page_fetch_prot, 0x10, READ_WRITE },
+ /* mismatched keys, no fetch protection, override doesn't apply -> RO */
+ { TEST_STORAGE_PROT_OVERRIDE, page_store_prot, 0x20, READ },
+ /* mismatched keys, but override applies (storage key 9) -> RW */
+ { TEST_STORAGE_PROT_OVERRIDE, (void *)2049, 0x10, READ_WRITE },
+ /* end marker */
+ { STAGE_END, 0, 0, 0 },
+};
+
+static enum stage perform_next_stage(int *i, bool mapped_0)
+{
+ enum stage stage = tests[*i].stage;
+ enum permission result;
+ bool skip;
+
+ for (; tests[*i].stage == stage; (*i)++) {
+ /*
+ * Some fetch protection override tests require that page 0
+ * be mapped, however, when the hosts tries to map that page via
+ * vm_vaddr_alloc, it may happen that some other page gets mapped
+ * instead.
+ * In order to skip these tests we detect this inside the guest
+ */
+ skip = tests[*i].addr < (void *)4096 &&
+ tests[*i].expected != TRANSL_UNAVAIL &&
+ !mapped_0;
+ if (!skip) {
+ result = test_protection(tests[*i].addr, tests[*i].key);
+ GUEST_ASSERT_2(result == tests[*i].expected, *i, result);
+ }
+ }
+ return stage;
+}
+
+static void guest_code(void)
+{
+ bool mapped_0;
+ int i = 0;
+
+ GUEST_ASSERT_EQ(set_storage_key(page_store_prot, 0x10), 0);
+ GUEST_ASSERT_EQ(set_storage_key(page_fetch_prot, 0x98), 0);
+ GUEST_SYNC(STAGE_INIT_SIMPLE);
+ GUEST_SYNC(perform_next_stage(&i, false));
+
+ /* Fetch-protection override */
+ mapped_0 = !set_storage_key((void *)0, 0x98);
+ GUEST_SYNC(STAGE_INIT_FETCH_PROT_OVERRIDE);
+ GUEST_SYNC(perform_next_stage(&i, mapped_0));
+
+ /* Storage-protection override */
+ GUEST_SYNC(perform_next_stage(&i, mapped_0));
+}
+
+#define HOST_SYNC(vmp, stage) \
+({ \
+ struct kvm_vm *__vm = (vmp); \
+ struct ucall uc; \
+ int __stage = (stage); \
+ \
+ vcpu_run(__vm, VCPU_ID); \
+ get_ucall(__vm, VCPU_ID, &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]); \
+ } \
+ ASSERT_EQ(uc.cmd, UCALL_SYNC); \
+ ASSERT_EQ(uc.args[1], __stage); \
+})
+
+int main(int argc, char *argv[])
+{
+ struct kvm_vm *vm;
+ struct kvm_run *run;
+ vm_vaddr_t guest_0_page;
+
+ vm = vm_create_default(VCPU_ID, 0, guest_code);
+ run = vcpu_state(vm, VCPU_ID);
+
+ HOST_SYNC(vm, STAGE_INIT_SIMPLE);
+ mprotect(addr_gva2hva(vm, (vm_vaddr_t)pages), PAGE_SIZE * 2, PROT_READ);
+ HOST_SYNC(vm, TEST_SIMPLE);
+
+ guest_0_page = vm_vaddr_alloc(vm, PAGE_SIZE, 0);
+ if (guest_0_page != 0)
+ print_skip("Did not allocate page at 0 for fetch protection override tests");
+ HOST_SYNC(vm, STAGE_INIT_FETCH_PROT_OVERRIDE);
+ if (guest_0_page == 0)
+ mprotect(addr_gva2hva(vm, (vm_vaddr_t)0), PAGE_SIZE, PROT_READ);
+ run->s.regs.crs[0] |= CR0_FETCH_PROTECTION_OVERRIDE;
+ run->kvm_dirty_regs = KVM_SYNC_CRS;
+ HOST_SYNC(vm, TEST_FETCH_PROT_OVERRIDE);
+
+ run->s.regs.crs[0] |= CR0_STORAGE_PROTECTION_OVERRIDE;
+ run->kvm_dirty_regs = KVM_SYNC_CRS;
+ HOST_SYNC(vm, TEST_STORAGE_PROT_OVERRIDE);
+}
--
2.32.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 05/10] KVM: s390: Add optional storage key checking to MEMOP IOCTL
2022-02-09 17:04 [PATCH v3 00/10] KVM: s390: Do storage key checking Janis Schoetterl-Glausch
` (3 preceding siblings ...)
2022-02-09 17:04 ` [PATCH v3 04/10] KVM: s390: selftests: Test TEST PROTECTION emulation Janis Schoetterl-Glausch
@ 2022-02-09 17:04 ` Janis Schoetterl-Glausch
2022-02-10 9:29 ` Christian Borntraeger
2022-02-10 11:59 ` Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 06/10] KVM: s390: Add vm IOCTL for key checked guest absolute memory access Janis Schoetterl-Glausch
` (4 subsequent siblings)
9 siblings, 2 replies; 21+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-09 17:04 UTC (permalink / raw)
To: Christian Borntraeger, Heiko Carstens, Janosch Frank
Cc: Janis Schoetterl-Glausch, Alexander Gordeev, Claudio Imbrenda,
David Hildenbrand, Jonathan Corbet, kvm, linux-doc, linux-kernel,
linux-s390, Paolo Bonzini, Sven Schnelle, Vasily Gorbik
User space needs a mechanism to perform key checked accesses when
emulating instructions.
The key can be passed as an additional argument.
Having an additional argument is flexible, as user space can
pass the guest PSW's key, in order to make an access the same way the
CPU would, or pass another key if necessary.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Acked-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
arch/s390/kvm/kvm-s390.c | 30 ++++++++++++++++++++----------
include/uapi/linux/kvm.h | 6 +++++-
2 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index cf347e1a4f17..85763ec7bc60 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -32,6 +32,7 @@
#include <linux/sched/signal.h>
#include <linux/string.h>
#include <linux/pgtable.h>
+#include <linux/bitfield.h>
#include <asm/asm-offsets.h>
#include <asm/lowcore.h>
@@ -2359,6 +2360,11 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
return r;
}
+static bool access_key_invalid(u8 access_key)
+{
+ return access_key > 0xf;
+}
+
long kvm_arch_vm_ioctl(struct file *filp,
unsigned int ioctl, unsigned long arg)
{
@@ -4690,17 +4696,19 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
void *tmpbuf = NULL;
int r = 0;
const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION
- | KVM_S390_MEMOP_F_CHECK_ONLY;
+ | KVM_S390_MEMOP_F_CHECK_ONLY
+ | KVM_S390_MEMOP_F_SKEY_PROTECTION;
if (mop->flags & ~supported_flags || mop->ar >= NUM_ACRS || !mop->size)
return -EINVAL;
-
if (mop->size > MEM_OP_MAX_SIZE)
return -E2BIG;
-
if (kvm_s390_pv_cpu_is_protected(vcpu))
return -EINVAL;
-
+ if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
+ if (access_key_invalid(mop->key))
+ return -EINVAL;
+ }
if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
tmpbuf = vmalloc(mop->size);
if (!tmpbuf)
@@ -4710,11 +4718,12 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
switch (mop->op) {
case KVM_S390_MEMOP_LOGICAL_READ:
if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
- r = check_gva_range(vcpu, mop->gaddr, mop->ar,
- mop->size, GACC_FETCH, 0);
+ r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
+ GACC_FETCH, mop->key);
break;
}
- r = read_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
+ r = read_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
+ mop->size, mop->key);
if (r == 0) {
if (copy_to_user(uaddr, tmpbuf, mop->size))
r = -EFAULT;
@@ -4722,15 +4731,16 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
break;
case KVM_S390_MEMOP_LOGICAL_WRITE:
if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
- r = check_gva_range(vcpu, mop->gaddr, mop->ar,
- mop->size, GACC_STORE, 0);
+ r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
+ GACC_STORE, mop->key);
break;
}
if (copy_from_user(tmpbuf, uaddr, mop->size)) {
r = -EFAULT;
break;
}
- r = write_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
+ r = write_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
+ mop->size, mop->key);
break;
}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index b46bcdb0cab1..44558cf4c52e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -562,7 +562,10 @@ struct kvm_s390_mem_op {
__u32 op; /* type of operation */
__u64 buf; /* buffer in userspace */
union {
- __u8 ar; /* the access register number */
+ struct {
+ __u8 ar; /* the access register number */
+ __u8 key; /* access key, ignored if flag unset */
+ };
__u32 sida_offset; /* offset into the sida */
__u8 reserved[32]; /* should be set to 0 */
};
@@ -575,6 +578,7 @@ struct kvm_s390_mem_op {
/* flags for kvm_s390_mem_op->flags */
#define KVM_S390_MEMOP_F_CHECK_ONLY (1ULL << 0)
#define KVM_S390_MEMOP_F_INJECT_EXCEPTION (1ULL << 1)
+#define KVM_S390_MEMOP_F_SKEY_PROTECTION (1ULL << 2)
/* for KVM_INTERRUPT */
struct kvm_interrupt {
--
2.32.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 06/10] KVM: s390: Add vm IOCTL for key checked guest absolute memory access
2022-02-09 17:04 [PATCH v3 00/10] KVM: s390: Do storage key checking Janis Schoetterl-Glausch
` (4 preceding siblings ...)
2022-02-09 17:04 ` [PATCH v3 05/10] KVM: s390: Add optional storage key checking to MEMOP IOCTL Janis Schoetterl-Glausch
@ 2022-02-09 17:04 ` Janis Schoetterl-Glausch
2022-02-10 14:40 ` Christian Borntraeger
2022-02-09 17:04 ` [PATCH v3 07/10] KVM: s390: Rename existing vcpu memop functions Janis Schoetterl-Glausch
` (3 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-09 17:04 UTC (permalink / raw)
To: Christian Borntraeger, Heiko Carstens, Janosch Frank
Cc: Janis Schoetterl-Glausch, Alexander Gordeev, Claudio Imbrenda,
David Hildenbrand, Jonathan Corbet, kvm, linux-doc, linux-kernel,
linux-s390, Paolo Bonzini, Sven Schnelle, Vasily Gorbik
Channel I/O honors storage keys and is performed on absolute memory.
For I/O emulation user space therefore needs to be able to do key
checked accesses.
The vm IOCTL supports read/write accesses, as well as checking
if an access would succeed.
Unlike relying on KVM_S390_GET_SKEYS for key checking would,
the vm IOCTL performs the check in lockstep with the read or write,
by, ultimately, mapping the access to move instructions that
support key protection checking with a supplied key.
Fetch and storage protection override are not applicable to absolute
accesses and so are not applied as they are when using the vcpu memop.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Acked-by: Janosch Frank <frankja@linux.ibm.com>
---
arch/s390/kvm/gaccess.c | 72 ++++++++++++++++++++++++++++++++++++
arch/s390/kvm/gaccess.h | 6 +++
arch/s390/kvm/kvm-s390.c | 79 ++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/kvm.h | 2 +
4 files changed, 159 insertions(+)
diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index 37838f637707..d53a183c2005 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -795,6 +795,35 @@ static int low_address_protection_enabled(struct kvm_vcpu *vcpu,
return 1;
}
+static int vm_check_access_key(struct kvm *kvm, u8 access_key,
+ enum gacc_mode mode, gpa_t gpa)
+{
+ u8 storage_key, access_control;
+ bool fetch_protected;
+ unsigned long hva;
+ int r;
+
+ if (access_key == 0)
+ return 0;
+
+ hva = gfn_to_hva(kvm, gpa_to_gfn(gpa));
+ if (kvm_is_error_hva(hva))
+ return PGM_ADDRESSING;
+
+ mmap_read_lock(current->mm);
+ r = get_guest_storage_key(current->mm, hva, &storage_key);
+ mmap_read_unlock(current->mm);
+ if (r)
+ return r;
+ access_control = FIELD_GET(_PAGE_ACC_BITS, storage_key);
+ if (access_control == access_key)
+ return 0;
+ fetch_protected = storage_key & _PAGE_FP_BIT;
+ if ((mode == GACC_FETCH || mode == GACC_IFETCH) && !fetch_protected)
+ return 0;
+ return PGM_PROTECTION;
+}
+
static bool fetch_prot_override_applicable(struct kvm_vcpu *vcpu, enum gacc_mode mode,
union asce asce)
{
@@ -994,6 +1023,26 @@ access_guest_page_with_key(struct kvm *kvm, enum gacc_mode mode, gpa_t gpa,
return 0;
}
+int access_guest_abs_with_key(struct kvm *kvm, gpa_t gpa, void *data,
+ unsigned long len, enum gacc_mode mode, u8 access_key)
+{
+ int offset = offset_in_page(gpa);
+ int fragment_len;
+ int rc;
+
+ while (min(PAGE_SIZE - offset, len) > 0) {
+ fragment_len = min(PAGE_SIZE - offset, len);
+ rc = access_guest_page_with_key(kvm, mode, gpa, data, fragment_len, access_key);
+ if (rc)
+ return rc;
+ offset = 0;
+ len -= fragment_len;
+ data += fragment_len;
+ gpa += fragment_len;
+ }
+ return 0;
+}
+
int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
void *data, unsigned long len, enum gacc_mode mode,
u8 access_key)
@@ -1144,6 +1193,29 @@ int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
return rc;
}
+/**
+ * check_gpa_range - test a range of guest physical addresses for accessibility
+ * @kvm: virtual machine instance
+ * @gpa: guest physical address
+ * @length: length of test range
+ * @mode: access mode to test, relevant for storage keys
+ * @access_key: access key to mach the storage keys with
+ */
+int check_gpa_range(struct kvm *kvm, unsigned long gpa, unsigned long length,
+ enum gacc_mode mode, u8 access_key)
+{
+ unsigned int fragment_len;
+ int rc = 0;
+
+ while (length && !rc) {
+ fragment_len = min(PAGE_SIZE - offset_in_page(gpa), length);
+ rc = vm_check_access_key(kvm, access_key, mode, gpa);
+ length -= fragment_len;
+ gpa += fragment_len;
+ }
+ return rc;
+}
+
/**
* kvm_s390_check_low_addr_prot_real - check for low-address protection
* @vcpu: virtual cpu
diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
index c5f2e7311b17..1124ff282012 100644
--- a/arch/s390/kvm/gaccess.h
+++ b/arch/s390/kvm/gaccess.h
@@ -193,6 +193,12 @@ int guest_translate_address_with_key(struct kvm_vcpu *vcpu, unsigned long gva, u
int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
unsigned long length, enum gacc_mode mode, u8 access_key);
+int check_gpa_range(struct kvm *kvm, unsigned long gpa, unsigned long length,
+ enum gacc_mode mode, u8 access_key);
+
+int access_guest_abs_with_key(struct kvm *kvm, gpa_t gpa, void *data,
+ unsigned long len, enum gacc_mode mode, u8 access_key);
+
int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
void *data, unsigned long len, enum gacc_mode mode,
u8 access_key);
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 85763ec7bc60..7ee3d2e8ecf2 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2365,6 +2365,76 @@ static bool access_key_invalid(u8 access_key)
return access_key > 0xf;
}
+static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
+{
+ void __user *uaddr = (void __user *)mop->buf;
+ u64 supported_flags;
+ void *tmpbuf = NULL;
+ int r, srcu_idx;
+
+ supported_flags = KVM_S390_MEMOP_F_SKEY_PROTECTION
+ | KVM_S390_MEMOP_F_CHECK_ONLY;
+ if (mop->flags & ~supported_flags)
+ return -EINVAL;
+ if (mop->size > MEM_OP_MAX_SIZE)
+ return -E2BIG;
+ if (kvm_s390_pv_is_protected(kvm))
+ return -EINVAL;
+ if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
+ if (access_key_invalid(mop->key))
+ return -EINVAL;
+ }
+ if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
+ tmpbuf = vmalloc(mop->size);
+ if (!tmpbuf)
+ return -ENOMEM;
+ }
+
+ srcu_idx = srcu_read_lock(&kvm->srcu);
+
+ if (kvm_is_error_gpa(kvm, mop->gaddr)) {
+ r = PGM_ADDRESSING;
+ goto out_unlock;
+ }
+
+ switch (mop->op) {
+ case KVM_S390_MEMOP_ABSOLUTE_READ: {
+ if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
+ r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_FETCH, mop->key);
+ } else {
+ r = access_guest_abs_with_key(kvm, mop->gaddr, tmpbuf,
+ mop->size, GACC_FETCH, mop->key);
+ if (r == 0) {
+ if (copy_to_user(uaddr, tmpbuf, mop->size))
+ r = -EFAULT;
+ }
+ }
+ break;
+ }
+ case KVM_S390_MEMOP_ABSOLUTE_WRITE: {
+ if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
+ r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_STORE, mop->key);
+ } else {
+ if (copy_from_user(tmpbuf, uaddr, mop->size)) {
+ r = -EFAULT;
+ break;
+ }
+ r = access_guest_abs_with_key(kvm, mop->gaddr, tmpbuf,
+ mop->size, GACC_STORE, mop->key);
+ }
+ break;
+ }
+ default:
+ r = -EINVAL;
+ }
+
+out_unlock:
+ srcu_read_unlock(&kvm->srcu, srcu_idx);
+
+ vfree(tmpbuf);
+ return r;
+}
+
long kvm_arch_vm_ioctl(struct file *filp,
unsigned int ioctl, unsigned long arg)
{
@@ -2489,6 +2559,15 @@ long kvm_arch_vm_ioctl(struct file *filp,
}
break;
}
+ case KVM_S390_MEM_OP: {
+ struct kvm_s390_mem_op mem_op;
+
+ if (copy_from_user(&mem_op, argp, sizeof(mem_op)) == 0)
+ r = kvm_s390_vm_mem_op(kvm, &mem_op);
+ else
+ r = -EFAULT;
+ break;
+ }
default:
r = -ENOTTY;
}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 44558cf4c52e..b83c9286e017 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -575,6 +575,8 @@ struct kvm_s390_mem_op {
#define KVM_S390_MEMOP_LOGICAL_WRITE 1
#define KVM_S390_MEMOP_SIDA_READ 2
#define KVM_S390_MEMOP_SIDA_WRITE 3
+#define KVM_S390_MEMOP_ABSOLUTE_READ 4
+#define KVM_S390_MEMOP_ABSOLUTE_WRITE 5
/* flags for kvm_s390_mem_op->flags */
#define KVM_S390_MEMOP_F_CHECK_ONLY (1ULL << 0)
#define KVM_S390_MEMOP_F_INJECT_EXCEPTION (1ULL << 1)
--
2.32.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 07/10] KVM: s390: Rename existing vcpu memop functions
2022-02-09 17:04 [PATCH v3 00/10] KVM: s390: Do storage key checking Janis Schoetterl-Glausch
` (5 preceding siblings ...)
2022-02-09 17:04 ` [PATCH v3 06/10] KVM: s390: Add vm IOCTL for key checked guest absolute memory access Janis Schoetterl-Glausch
@ 2022-02-09 17:04 ` Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 08/10] KVM: s390: Add capability for storage key extension of MEM_OP IOCTL Janis Schoetterl-Glausch
` (2 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-09 17:04 UTC (permalink / raw)
To: Christian Borntraeger, Heiko Carstens, Janosch Frank
Cc: Janis Schoetterl-Glausch, Alexander Gordeev, Claudio Imbrenda,
David Hildenbrand, Jonathan Corbet, kvm, linux-doc, linux-kernel,
linux-s390, Paolo Bonzini, Sven Schnelle, Vasily Gorbik
Makes the naming consistent, now that we also have a vm ioctl.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
arch/s390/kvm/kvm-s390.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 7ee3d2e8ecf2..eb034f2398ef 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -4740,8 +4740,8 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
return r;
}
-static long kvm_s390_guest_sida_op(struct kvm_vcpu *vcpu,
- struct kvm_s390_mem_op *mop)
+static long kvm_s390_vcpu_sida_op(struct kvm_vcpu *vcpu,
+ struct kvm_s390_mem_op *mop)
{
void __user *uaddr = (void __user *)mop->buf;
int r = 0;
@@ -4768,8 +4768,9 @@ static long kvm_s390_guest_sida_op(struct kvm_vcpu *vcpu,
}
return r;
}
-static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
- struct kvm_s390_mem_op *mop)
+
+static long kvm_s390_vcpu_mem_op(struct kvm_vcpu *vcpu,
+ struct kvm_s390_mem_op *mop)
{
void __user *uaddr = (void __user *)mop->buf;
void *tmpbuf = NULL;
@@ -4830,8 +4831,8 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
return r;
}
-static long kvm_s390_guest_memsida_op(struct kvm_vcpu *vcpu,
- struct kvm_s390_mem_op *mop)
+static long kvm_s390_vcpu_memsida_op(struct kvm_vcpu *vcpu,
+ struct kvm_s390_mem_op *mop)
{
int r, srcu_idx;
@@ -4840,12 +4841,12 @@ static long kvm_s390_guest_memsida_op(struct kvm_vcpu *vcpu,
switch (mop->op) {
case KVM_S390_MEMOP_LOGICAL_READ:
case KVM_S390_MEMOP_LOGICAL_WRITE:
- r = kvm_s390_guest_mem_op(vcpu, mop);
+ r = kvm_s390_vcpu_mem_op(vcpu, mop);
break;
case KVM_S390_MEMOP_SIDA_READ:
case KVM_S390_MEMOP_SIDA_WRITE:
/* we are locked against sida going away by the vcpu->mutex */
- r = kvm_s390_guest_sida_op(vcpu, mop);
+ r = kvm_s390_vcpu_sida_op(vcpu, mop);
break;
default:
r = -EINVAL;
@@ -5008,7 +5009,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
struct kvm_s390_mem_op mem_op;
if (copy_from_user(&mem_op, argp, sizeof(mem_op)) == 0)
- r = kvm_s390_guest_memsida_op(vcpu, &mem_op);
+ r = kvm_s390_vcpu_memsida_op(vcpu, &mem_op);
else
r = -EFAULT;
break;
--
2.32.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 08/10] KVM: s390: Add capability for storage key extension of MEM_OP IOCTL
2022-02-09 17:04 [PATCH v3 00/10] KVM: s390: Do storage key checking Janis Schoetterl-Glausch
` (6 preceding siblings ...)
2022-02-09 17:04 ` [PATCH v3 07/10] KVM: s390: Rename existing vcpu memop functions Janis Schoetterl-Glausch
@ 2022-02-09 17:04 ` Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 09/10] KVM: s390: Update api documentation for memop ioctl Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 10/10] KVM: s390: selftests: Test memops with storage keys Janis Schoetterl-Glausch
9 siblings, 0 replies; 21+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-09 17:04 UTC (permalink / raw)
To: Christian Borntraeger, Heiko Carstens, Janosch Frank
Cc: Janis Schoetterl-Glausch, Alexander Gordeev, Claudio Imbrenda,
David Hildenbrand, Jonathan Corbet, kvm, linux-doc, linux-kernel,
linux-s390, Paolo Bonzini, Sven Schnelle, Vasily Gorbik
Availability of the KVM_CAP_S390_MEM_OP_EXTENSION capability signals that:
* The vcpu MEM_OP IOCTL supports storage key checking.
* The vm MEM_OP IOCTL exists.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
arch/s390/kvm/kvm-s390.c | 1 +
include/uapi/linux/kvm.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index eb034f2398ef..5b387e75cb5b 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -565,6 +565,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_S390_VCPU_RESETS:
case KVM_CAP_SET_GUEST_DEBUG:
case KVM_CAP_S390_DIAG318:
+ case KVM_CAP_S390_MEM_OP_EXTENSION:
r = 1;
break;
case KVM_CAP_SET_GUEST_DEBUG2:
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index b83c9286e017..26bff414f1a0 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1140,6 +1140,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_VM_GPA_BITS 207
#define KVM_CAP_XSAVE2 208
#define KVM_CAP_SYS_ATTRIBUTES 209
+#define KVM_CAP_S390_MEM_OP_EXTENSION 210
#ifdef KVM_CAP_IRQ_ROUTING
--
2.32.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 09/10] KVM: s390: Update api documentation for memop ioctl
2022-02-09 17:04 [PATCH v3 00/10] KVM: s390: Do storage key checking Janis Schoetterl-Glausch
` (7 preceding siblings ...)
2022-02-09 17:04 ` [PATCH v3 08/10] KVM: s390: Add capability for storage key extension of MEM_OP IOCTL Janis Schoetterl-Glausch
@ 2022-02-09 17:04 ` Janis Schoetterl-Glausch
2022-02-10 14:41 ` Christian Borntraeger
2022-02-11 9:17 ` Janosch Frank
2022-02-09 17:04 ` [PATCH v3 10/10] KVM: s390: selftests: Test memops with storage keys Janis Schoetterl-Glausch
9 siblings, 2 replies; 21+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-09 17:04 UTC (permalink / raw)
To: Christian Borntraeger, Heiko Carstens, Janosch Frank
Cc: Janis Schoetterl-Glausch, Alexander Gordeev, Claudio Imbrenda,
David Hildenbrand, Jonathan Corbet, kvm, linux-doc, linux-kernel,
linux-s390, Paolo Bonzini, Sven Schnelle, Vasily Gorbik
Document all currently existing operations, flags and explain under
which circumstances they are available. Document the recently
introduced absolute operations and the storage key protection flag,
as well as the existing SIDA operations.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
Documentation/virt/kvm/api.rst | 112 ++++++++++++++++++++++++++-------
include/uapi/linux/kvm.h | 2 +-
2 files changed, 91 insertions(+), 23 deletions(-)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index a4267104db50..2d131af44576 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -3683,15 +3683,17 @@ The fields in each entry are defined as follows:
4.89 KVM_S390_MEM_OP
--------------------
-:Capability: KVM_CAP_S390_MEM_OP
+:Capability: KVM_CAP_S390_MEM_OP, KVM_CAP_S390_PROTECTED, KVM_CAP_S390_MEM_OP_EXTENSION
:Architectures: s390
-:Type: vcpu ioctl
+:Type: vm ioctl, vcpu ioctl
:Parameters: struct kvm_s390_mem_op (in)
:Returns: = 0 on success,
< 0 on generic error (e.g. -EFAULT or -ENOMEM),
> 0 if an exception occurred while walking the page tables
-Read or write data from/to the logical (virtual) memory of a VCPU.
+Read or write data from/to the VM's memory.
+The KVM_CAP_S390_MEM_OP_EXTENSION capability specifies what functionality is
+supported.
Parameters are specified via the following structure::
@@ -3701,33 +3703,99 @@ Parameters are specified via the following structure::
__u32 size; /* amount of bytes */
__u32 op; /* type of operation */
__u64 buf; /* buffer in userspace */
- __u8 ar; /* the access register number */
- __u8 reserved[31]; /* should be set to 0 */
+ union {
+ struct {
+ __u8 ar; /* the access register number */
+ __u8 key; /* access key, ignored if flag unset */
+ };
+ __u32 sida_offset; /* offset into the sida */
+ __u8 reserved[32]; /* ignored */
+ };
};
-The type of operation is specified in the "op" field. It is either
-KVM_S390_MEMOP_LOGICAL_READ for reading from logical memory space or
-KVM_S390_MEMOP_LOGICAL_WRITE for writing to logical memory space. The
-KVM_S390_MEMOP_F_CHECK_ONLY flag can be set in the "flags" field to check
-whether the corresponding memory access would create an access exception
-(without touching the data in the memory at the destination). In case an
-access exception occurred while walking the MMU tables of the guest, the
-ioctl returns a positive error number to indicate the type of exception.
-This exception is also raised directly at the corresponding VCPU if the
-flag KVM_S390_MEMOP_F_INJECT_EXCEPTION is set in the "flags" field.
-
The start address of the memory region has to be specified in the "gaddr"
field, and the length of the region in the "size" field (which must not
be 0). The maximum value for "size" can be obtained by checking the
KVM_CAP_S390_MEM_OP capability. "buf" is the buffer supplied by the
userspace application where the read data should be written to for
-KVM_S390_MEMOP_LOGICAL_READ, or where the data that should be written is
-stored for a KVM_S390_MEMOP_LOGICAL_WRITE. When KVM_S390_MEMOP_F_CHECK_ONLY
-is specified, "buf" is unused and can be NULL. "ar" designates the access
-register number to be used; the valid range is 0..15.
+a read access, or where the data that should be written is stored for
+a write access. The "reserved" field is meant for future extensions.
+Reserved and unused values are ignored. Future extension that add members must
+introduce new flags.
+
+The type of operation is specified in the "op" field. Flags modifying
+their behavior can be set in the "flags" field. Undefined flag bits must
+be set to 0.
+
+Possible operations are:
+ * ``KVM_S390_MEMOP_LOGICAL_READ``
+ * ``KVM_S390_MEMOP_LOGICAL_WRITE``
+ * ``KVM_S390_MEMOP_ABSOLUTE_READ``
+ * ``KVM_S390_MEMOP_ABSOLUTE_WRITE``
+ * ``KVM_S390_MEMOP_SIDA_READ``
+ * ``KVM_S390_MEMOP_SIDA_WRITE``
+
+Logical read/write:
+^^^^^^^^^^^^^^^^^^^
+
+Access logical memory, i.e. translate the given guest address to an absolute
+address given the state of the VCPU and use the absolute address as target of
+the access. "ar" designates the access register number to be used; the valid
+range is 0..15.
+Logical accesses are permitted for the VCPU ioctl only.
+Logical accesses are permitted for non secure guests only.
+
+Supported flags:
+ * ``KVM_S390_MEMOP_F_CHECK_ONLY``
+ * ``KVM_S390_MEMOP_F_INJECT_EXCEPTION``
+ * ``KVM_S390_MEMOP_F_SKEY_PROTECTION``
+
+The KVM_S390_MEMOP_F_CHECK_ONLY flag can be set to check whether the
+corresponding memory access would cause an access exception, without touching
+the data in memory at the destination.
+In this case, "buf" is unused and can be NULL.
+
+In case an access exception occurred during the access (or would occur
+in case of KVM_S390_MEMOP_F_CHECK_ONLY), the ioctl returns a positive
+error number indicating the type of exception. This exception is also
+raised directly at the corresponding VCPU if the flag
+KVM_S390_MEMOP_F_INJECT_EXCEPTION is set.
+
+If the KVM_S390_MEMOP_F_SKEY_PROTECTION flag is set, storage key
+protection is also in effect and may cause exceptions if accesses are
+prohibited given the access key passed in "key".
+KVM_S390_MEMOP_F_SKEY_PROTECTION is available if KVM_CAP_S390_MEM_OP_EXTENSION
+is > 0.
+
+Absolute read/write:
+^^^^^^^^^^^^^^^^^^^^
+
+Access absolute memory. This operation is intended to be used with the
+KVM_S390_MEMOP_F_SKEY_PROTECTION flag, to allow accessing memory and performing
+the checks required for storage key protection as one operation (as opposed to
+user space getting the storage keys, performing the checks, and accessing
+memory thereafter, which could lead to a delay between check and access).
+Absolute accesses are permitted for the VM ioctl if KVM_CAP_S390_MEM_OP_EXTENSION
+is > 0.
+Currently absolute accesses are not permitted for VCPU ioctls.
+Absolute accesses are permitted for non secure guests only.
+
+Supported flags:
+ * ``KVM_S390_MEMOP_F_CHECK_ONLY``
+ * ``KVM_S390_MEMOP_F_SKEY_PROTECTION``
+
+The semantics of the flags are as for logical accesses.
+
+SIDA read/write:
+^^^^^^^^^^^^^^^^
+
+Access the secure instruction data area which contains memory operands necessary
+for instruction emulation for secure guests.
+SIDA accesses are available if the KVM_CAP_S390_PROTECTED capability is available.
+SIDA accesses are permitted for the VCPU ioctl only.
+SIDA accesses are permitted for secure guests only.
-The "reserved" field is meant for future extensions. It is not used by
-KVM with the currently defined set of flags.
+No flags are supported.
4.90 KVM_S390_GET_SKEYS
-----------------------
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 26bff414f1a0..fd01fe04a183 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -567,7 +567,7 @@ struct kvm_s390_mem_op {
__u8 key; /* access key, ignored if flag unset */
};
__u32 sida_offset; /* offset into the sida */
- __u8 reserved[32]; /* should be set to 0 */
+ __u8 reserved[32]; /* ignored */
};
};
/* types for kvm_s390_mem_op->op */
--
2.32.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 10/10] KVM: s390: selftests: Test memops with storage keys
2022-02-09 17:04 [PATCH v3 00/10] KVM: s390: Do storage key checking Janis Schoetterl-Glausch
` (8 preceding siblings ...)
2022-02-09 17:04 ` [PATCH v3 09/10] KVM: s390: Update api documentation for memop ioctl Janis Schoetterl-Glausch
@ 2022-02-09 17:04 ` Janis Schoetterl-Glausch
9 siblings, 0 replies; 21+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-09 17:04 UTC (permalink / raw)
To: Christian Borntraeger, Heiko Carstens, Janosch Frank
Cc: Janis Schoetterl-Glausch, Alexander Gordeev, Claudio Imbrenda,
David Hildenbrand, Jonathan Corbet, kvm, linux-doc, linux-kernel,
linux-s390, Paolo Bonzini, Sven Schnelle, Vasily Gorbik
Test vm and vcpu memops with storage keys, both successful accesses
as well as various exception conditions.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
tools/testing/selftests/kvm/s390x/memop.c | 558 +++++++++++++++++++---
1 file changed, 495 insertions(+), 63 deletions(-)
diff --git a/tools/testing/selftests/kvm/s390x/memop.c b/tools/testing/selftests/kvm/s390x/memop.c
index 9f49ead380ab..ac08fd5aa746 100644
--- a/tools/testing/selftests/kvm/s390x/memop.c
+++ b/tools/testing/selftests/kvm/s390x/memop.c
@@ -13,28 +13,304 @@
#include "test_util.h"
#include "kvm_util.h"
+#define PAGE_SHIFT 12
+#define PAGE_SIZE (1 << PAGE_SHIFT)
+#define PAGE_MASK (~(PAGE_SIZE - 1))
+#define CR0_FETCH_PROTECTION_OVERRIDE (1UL << (63 - 38))
+#define CR0_STORAGE_PROTECTION_OVERRIDE (1UL << (63 - 39))
+
#define VCPU_ID 1
+const uint64_t last_page_addr = UINT64_MAX - PAGE_SIZE + 1;
+
static uint8_t mem1[65536];
static uint8_t mem2[65536];
+static void set_storage_key_range(void *addr, size_t len, u8 key)
+{
+ uintptr_t _addr, abs, i;
+
+ _addr = (uintptr_t)addr;
+ for (i = _addr & PAGE_MASK; i < _addr + len; i += PAGE_SIZE) {
+ abs = i;
+ asm volatile (
+ "lra %[abs], 0(0,%[abs])\n"
+ " sske %[key], %[abs]\n"
+ : [abs] "+&a" (abs)
+ : [key] "r" (key)
+ : "cc"
+ );
+ }
+}
+
static void guest_code(void)
+{
+ /* Set storage key */
+ set_storage_key_range(mem1, sizeof(mem1), 0x90);
+ set_storage_key_range(mem2, sizeof(mem2), 0x90);
+ GUEST_SYNC(0);
+
+ /* Write, read back, without keys */
+ memcpy(mem2, mem1, sizeof(mem2));
+ GUEST_SYNC(10);
+
+ /* Write, read back, key 0 */
+ memcpy(mem2, mem1, sizeof(mem2));
+ GUEST_SYNC(20);
+
+ /* Write, read back, matching key, 1 page */
+ memcpy(mem2, mem1, sizeof(mem2));
+ GUEST_SYNC(30);
+
+ /* Write, read back, matching key, all pages */
+ memcpy(mem2, mem1, sizeof(mem2));
+ GUEST_SYNC(40);
+
+ /* Set fetch protection */
+ set_storage_key_range(0, 1, 0x18);
+ GUEST_SYNC(50);
+
+ /* Enable fetch protection override */
+ GUEST_SYNC(60);
+
+ /* Enable storage protection override, set fetch protection*/
+ set_storage_key_range(mem1, sizeof(mem1), 0x98);
+ set_storage_key_range(mem2, sizeof(mem2), 0x98);
+ GUEST_SYNC(70);
+
+ /* Write, read back, mismatching key,
+ * storage protection override, all pages
+ */
+ memcpy(mem2, mem1, sizeof(mem2));
+ GUEST_SYNC(80);
+
+ /* VM memop, write, read back, matching key */
+ memcpy(mem2, mem1, sizeof(mem2));
+ GUEST_SYNC(90);
+
+ /* VM memop, write, read back, key 0 */
+ memcpy(mem2, mem1, sizeof(mem2));
+ /* VM memop, fail to read from 0 absolute/virtual, mismatching key,
+ * fetch protection override does not apply to VM memops
+ */
+ asm volatile ("sske %1,%0\n"
+ : : "r"(0), "r"(0x18) : "cc"
+ );
+ GUEST_SYNC(100);
+
+ /* Enable AR mode */
+ GUEST_SYNC(110);
+
+ /* Disable AR mode */
+ GUEST_SYNC(120);
+}
+
+static void reroll_mem1(void)
{
int i;
- for (;;) {
- for (i = 0; i < sizeof(mem2); i++)
- mem2[i] = mem1[i];
- GUEST_SYNC(0);
- }
+ for (i = 0; i < sizeof(mem1); i++)
+ mem1[i] = rand();
+}
+
+static int _vcpu_read_guest(struct kvm_vm *vm, void *host_addr,
+ uintptr_t guest_addr, size_t len)
+{
+ struct kvm_s390_mem_op ksmo = {
+ .gaddr = guest_addr,
+ .flags = 0,
+ .size = len,
+ .op = KVM_S390_MEMOP_LOGICAL_READ,
+ .buf = (uintptr_t)host_addr,
+ .ar = 0,
+ };
+
+ return _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, &ksmo);
+}
+
+static void vcpu_read_guest(struct kvm_vm *vm, void *host_addr,
+ uintptr_t guest_addr, size_t len)
+{
+ int rv;
+
+ rv = _vcpu_read_guest(vm, host_addr, guest_addr, len);
+ TEST_ASSERT(rv == 0, "vcpu memop read failed: reason = %d\n", rv);
+}
+
+static int _vcpu_read_guest_key(struct kvm_vm *vm, void *host_addr,
+ uintptr_t guest_addr, size_t len, u8 access_key)
+{
+ struct kvm_s390_mem_op ksmo = {0};
+
+ ksmo.gaddr = guest_addr;
+ ksmo.flags = KVM_S390_MEMOP_F_SKEY_PROTECTION;
+ ksmo.size = len;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_READ;
+ ksmo.buf = (uintptr_t)host_addr;
+ ksmo.ar = 0;
+ ksmo.key = access_key;
+
+ return _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, &ksmo);
+}
+
+static void vcpu_read_guest_key(struct kvm_vm *vm, void *host_addr,
+ uintptr_t guest_addr, size_t len, u8 access_key)
+{
+ int rv;
+
+ rv = _vcpu_read_guest_key(vm, host_addr, guest_addr, len, access_key);
+ TEST_ASSERT(rv == 0, "vcpu memop read failed: reason = %d\n", rv);
+}
+
+static int _vcpu_write_guest(struct kvm_vm *vm, uintptr_t guest_addr,
+ void *host_addr, size_t len)
+{
+ struct kvm_s390_mem_op ksmo = {
+ .gaddr = guest_addr,
+ .flags = 0,
+ .size = len,
+ .op = KVM_S390_MEMOP_LOGICAL_WRITE,
+ .buf = (uintptr_t)host_addr,
+ .ar = 0,
+ };
+ return _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, &ksmo);
+}
+
+static void vcpu_write_guest(struct kvm_vm *vm, uintptr_t guest_addr,
+ void *host_addr, size_t len)
+{
+ int rv;
+
+ rv = _vcpu_write_guest(vm, guest_addr, host_addr, len);
+ TEST_ASSERT(rv == 0, "vcpu memop write failed: reason = %d\n", rv);
+}
+
+static int _vcpu_write_guest_key(struct kvm_vm *vm, uintptr_t guest_addr,
+ void *host_addr, size_t len, u8 access_key)
+{
+ struct kvm_s390_mem_op ksmo = {0};
+
+ ksmo.gaddr = guest_addr;
+ ksmo.flags = KVM_S390_MEMOP_F_SKEY_PROTECTION;
+ ksmo.size = len;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
+ ksmo.buf = (uintptr_t)host_addr;
+ ksmo.ar = 0;
+ ksmo.key = access_key;
+
+ return _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, &ksmo);
+}
+
+static void vcpu_write_guest_key(struct kvm_vm *vm, uintptr_t guest_addr,
+ void *host_addr, size_t len, u8 access_key)
+{
+ int rv;
+
+ rv = _vcpu_write_guest_key(vm, guest_addr, host_addr, len, access_key);
+ TEST_ASSERT(rv == 0, "vcpu memop write failed: reason = %d\n", rv);
+}
+
+static int _vm_read_guest_key(struct kvm_vm *vm, void *host_addr,
+ uintptr_t guest_addr, size_t len, u8 access_key)
+{
+ struct kvm_s390_mem_op ksmo = {0};
+
+ ksmo.gaddr = guest_addr;
+ ksmo.flags = KVM_S390_MEMOP_F_SKEY_PROTECTION;
+ ksmo.size = len;
+ ksmo.op = KVM_S390_MEMOP_ABSOLUTE_READ;
+ ksmo.buf = (uintptr_t)host_addr;
+ ksmo.key = access_key;
+
+ return _vm_ioctl(vm, KVM_S390_MEM_OP, &ksmo);
+}
+
+static void vm_read_guest_key(struct kvm_vm *vm, void *host_addr,
+ uintptr_t guest_addr, size_t len, u8 access_key)
+{
+ int rv;
+
+ rv = _vm_read_guest_key(vm, host_addr, guest_addr, len, access_key);
+ TEST_ASSERT(rv == 0, "vm memop read failed: reason = %d\n", rv);
+}
+
+static int _vm_write_guest_key(struct kvm_vm *vm, uintptr_t guest_addr,
+ void *host_addr, size_t len, u8 access_key)
+{
+ struct kvm_s390_mem_op ksmo = {0};
+
+ ksmo.gaddr = guest_addr;
+ ksmo.flags = KVM_S390_MEMOP_F_SKEY_PROTECTION;
+ ksmo.size = len;
+ ksmo.op = KVM_S390_MEMOP_ABSOLUTE_WRITE;
+ ksmo.buf = (uintptr_t)host_addr;
+ ksmo.key = access_key;
+
+ return _vm_ioctl(vm, KVM_S390_MEM_OP, &ksmo);
+}
+
+static void vm_write_guest_key(struct kvm_vm *vm, uintptr_t guest_addr,
+ void *host_addr, size_t len, u8 access_key)
+{
+ int rv;
+
+ rv = _vm_write_guest_key(vm, guest_addr, host_addr, len, access_key);
+ TEST_ASSERT(rv == 0, "vm memop write failed: reason = %d\n", rv);
}
+enum access_mode {
+ ACCESS_READ,
+ ACCESS_WRITE
+};
+
+static int _vm_check_guest_key(struct kvm_vm *vm, enum access_mode mode,
+ uintptr_t guest_addr, size_t len, u8 access_key)
+{
+ struct kvm_s390_mem_op ksmo = {0};
+
+ ksmo.gaddr = guest_addr;
+ ksmo.flags = KVM_S390_MEMOP_F_CHECK_ONLY | KVM_S390_MEMOP_F_SKEY_PROTECTION;
+ ksmo.size = len;
+ if (mode == ACCESS_READ)
+ ksmo.op = KVM_S390_MEMOP_ABSOLUTE_READ;
+ else
+ ksmo.op = KVM_S390_MEMOP_ABSOLUTE_WRITE;
+ ksmo.key = access_key;
+
+ return _vm_ioctl(vm, KVM_S390_MEM_OP, &ksmo);
+}
+
+static void vm_check_guest_key(struct kvm_vm *vm, enum access_mode mode,
+ uintptr_t guest_addr, size_t len, u8 access_key)
+{
+ int rv;
+
+ rv = _vm_check_guest_key(vm, mode, guest_addr, len, access_key);
+ TEST_ASSERT(rv == 0, "vm memop write failed: reason = %d\n", rv);
+}
+
+#define HOST_SYNC(vmp, stage) \
+({ \
+ struct kvm_vm *__vm = (vmp); \
+ struct ucall uc; \
+ int __stage = (stage); \
+ \
+ vcpu_run(__vm, VCPU_ID); \
+ get_ucall(__vm, VCPU_ID, &uc); \
+ ASSERT_EQ(uc.cmd, UCALL_SYNC); \
+ ASSERT_EQ(uc.args[1], __stage); \
+}) \
+
int main(int argc, char *argv[])
{
struct kvm_vm *vm;
struct kvm_run *run;
struct kvm_s390_mem_op ksmo;
- int rv, i, maxsize;
+ bool has_skey_ext;
+ vm_vaddr_t guest_mem1;
+ vm_vaddr_t guest_mem2;
+ vm_paddr_t guest_mem1_abs;
+ int rv, maxsize;
setbuf(stdout, NULL); /* Tell stdout not to buffer its content */
@@ -45,67 +321,225 @@ int main(int argc, char *argv[])
}
if (maxsize > sizeof(mem1))
maxsize = sizeof(mem1);
+ has_skey_ext = kvm_check_cap(KVM_CAP_S390_MEM_OP_EXTENSION);
+ if (!has_skey_ext)
+ print_skip("Storage key extension not supported");
/* Create VM */
vm = vm_create_default(VCPU_ID, 0, guest_code);
run = vcpu_state(vm, VCPU_ID);
+ guest_mem1 = (uintptr_t)mem1;
+ guest_mem2 = (uintptr_t)mem2;
+ guest_mem1_abs = addr_gva2gpa(vm, guest_mem1);
- for (i = 0; i < sizeof(mem1); i++)
- mem1[i] = i * i + i;
+ /* Set storage key */
+ HOST_SYNC(vm, 0);
- /* Set the first array */
- ksmo.gaddr = addr_gva2gpa(vm, (uintptr_t)mem1);
- ksmo.flags = 0;
- ksmo.size = maxsize;
- ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
- ksmo.buf = (uintptr_t)mem1;
- ksmo.ar = 0;
- vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, &ksmo);
+ /* Write, read back, without keys */
+ reroll_mem1();
+ vcpu_write_guest(vm, guest_mem1, mem1, maxsize);
+ HOST_SYNC(vm, 10); // Copy in vm
+ memset(mem2, 0xaa, sizeof(mem2));
+ vcpu_read_guest(vm, mem2, guest_mem2, maxsize);
+ TEST_ASSERT(!memcmp(mem1, mem2, maxsize),
+ "Memory contents do not match!");
- /* Let the guest code copy the first array to the second */
- vcpu_run(vm, VCPU_ID);
- TEST_ASSERT(run->exit_reason == KVM_EXIT_S390_SIEIC,
- "Unexpected exit reason: %u (%s)\n",
- run->exit_reason,
- exit_reason_str(run->exit_reason));
+ if (has_skey_ext) {
+ vm_vaddr_t guest_0_page = vm_vaddr_alloc(vm, PAGE_SIZE, 0);
+ vm_vaddr_t guest_last_page = vm_vaddr_alloc(vm, PAGE_SIZE, last_page_addr);
+ vm_paddr_t guest_mem2_abs = addr_gva2gpa(vm, guest_mem2);
- memset(mem2, 0xaa, sizeof(mem2));
+ /* Write, read back, key 0 */
+ reroll_mem1();
+ vcpu_write_guest_key(vm, guest_mem1, mem1, maxsize, 0);
+ HOST_SYNC(vm, 20); // Copy in vm
+ memset(mem2, 0xaa, sizeof(mem2));
+ vcpu_read_guest_key(vm, mem2, guest_mem2, maxsize, 0);
+ TEST_ASSERT(!memcmp(mem1, mem2, maxsize),
+ "Memory contents do not match!");
- /* Get the second array */
- ksmo.gaddr = (uintptr_t)mem2;
- ksmo.flags = 0;
- ksmo.size = maxsize;
- ksmo.op = KVM_S390_MEMOP_LOGICAL_READ;
- ksmo.buf = (uintptr_t)mem2;
- ksmo.ar = 0;
- vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, &ksmo);
+ /* Write, read back, matching key, 1 page */
+ reroll_mem1();
+ vcpu_write_guest_key(vm, guest_mem1, mem1, PAGE_SIZE, 9);
+ HOST_SYNC(vm, 30); // Copy in vm
+ memset(mem2, 0xaa, sizeof(mem2));
+ vcpu_read_guest_key(vm, mem2, guest_mem2, PAGE_SIZE, 9);
+ TEST_ASSERT(!memcmp(mem1, mem2, PAGE_SIZE),
+ "Memory contents do not match!");
- TEST_ASSERT(!memcmp(mem1, mem2, maxsize),
- "Memory contents do not match!");
+ /* Write, read back, matching key, all pages */
+ reroll_mem1();
+ vcpu_write_guest_key(vm, guest_mem1, mem1, maxsize, 9);
+ HOST_SYNC(vm, 40); // Copy in vm
+ memset(mem2, 0xaa, sizeof(mem2));
+ vcpu_read_guest_key(vm, mem2, guest_mem2, maxsize, 9);
+ TEST_ASSERT(!memcmp(mem1, mem2, maxsize),
+ "Memory contents do not match!");
- /* Check error conditions - first bad size: */
- ksmo.gaddr = (uintptr_t)mem1;
- ksmo.flags = 0;
- ksmo.size = -1;
- ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
- ksmo.buf = (uintptr_t)mem1;
- ksmo.ar = 0;
- rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, &ksmo);
+ /* Fail to write, read back old value, mismatching key */
+ rv = _vcpu_write_guest_key(vm, guest_mem1, mem1, maxsize, 2);
+ TEST_ASSERT(rv == 4, "Store should result in protection exception");
+ memset(mem2, 0xaa, sizeof(mem2));
+ vcpu_read_guest_key(vm, mem2, guest_mem2, maxsize, 2);
+ TEST_ASSERT(!memcmp(mem1, mem2, maxsize),
+ "Memory contents do not match!");
+
+ /* Set fetch protection */
+ HOST_SYNC(vm, 50);
+
+ /* Write without key, read back, matching key, fetch protection */
+ reroll_mem1();
+ vcpu_write_guest(vm, guest_0_page, mem1, PAGE_SIZE);
+ memset(mem2, 0xaa, sizeof(mem2));
+ /* Lets not copy in the guest, in case guest_0_page != 0 */
+ vcpu_read_guest_key(vm, mem2, guest_0_page, PAGE_SIZE, 1);
+ TEST_ASSERT(!memcmp(mem1, mem2, PAGE_SIZE),
+ "Memory contents do not match!");
+
+ /* Fail to read, mismatching key, fetch protection */
+ rv = _vcpu_read_guest_key(vm, mem2, guest_0_page, PAGE_SIZE, 2);
+ TEST_ASSERT(rv == 4, "Fetch should result in protection exception");
+
+ /* Enable fetch protection override */
+ run->s.regs.crs[0] |= CR0_FETCH_PROTECTION_OVERRIDE;
+ run->kvm_dirty_regs = KVM_SYNC_CRS;
+ HOST_SYNC(vm, 60);
+
+ if (guest_0_page != 0)
+ print_skip("Did not allocate page at 0 for fetch protection override test");
+
+ /* Write without key, read back, mismatching key,
+ * fetch protection override, 1 page
+ */
+ if (guest_0_page == 0) {
+ reroll_mem1();
+ vcpu_write_guest(vm, guest_0_page, mem1, PAGE_SIZE);
+ memset(mem2, 0xaa, sizeof(mem2));
+ /* Lets not copy in the guest, in case guest_0_page != 0 */
+ vcpu_read_guest_key(vm, mem2, guest_0_page, 2048, 2);
+ TEST_ASSERT(!memcmp(mem1, mem2, 2048),
+ "Memory contents do not match!");
+ }
+
+ /* Fail to read, mismatching key,
+ * fetch protection override address exceeded, 1 page
+ */
+ if (guest_0_page == 0) {
+ rv = _vcpu_read_guest_key(vm, mem2, 0, 2048 + 1, 2);
+ TEST_ASSERT(rv == 4,
+ "Fetch should result in protection exception");
+ }
+
+ if (guest_last_page != last_page_addr)
+ print_skip("Did not allocate last page for fetch protection override test");
+
+ /* Write without key, read back, mismatching key,
+ * fetch protection override, 2 pages, last page not fetch protected
+ */
+ reroll_mem1();
+ vcpu_write_guest(vm, guest_last_page, mem1, PAGE_SIZE);
+ vcpu_write_guest(vm, guest_0_page, mem1 + PAGE_SIZE, PAGE_SIZE);
+ if (guest_0_page == 0 && guest_last_page == last_page_addr) {
+ memset(mem2, 0xaa, sizeof(mem2));
+ /* Lets not copy in the guest, in case guest_0_page != 0 */
+ vcpu_read_guest_key(vm, mem2, last_page_addr,
+ PAGE_SIZE + 2048, 2);
+ TEST_ASSERT(!memcmp(mem1, mem2, PAGE_SIZE + 2048),
+ "Memory contents do not match!");
+ }
+
+ /* Fail to read, mismatching key, fetch protection override address
+ * exceeded, 2 pages, last page not fetch protected
+ */
+ if (guest_0_page == 0 && guest_last_page == last_page_addr) {
+ rv = _vcpu_read_guest_key(vm, mem2, last_page_addr,
+ PAGE_SIZE + 2048 + 1, 2);
+ TEST_ASSERT(rv == 4,
+ "Fetch should result in protection exception");
+ }
+
+ /* Enable storage protection override, set fetch protection*/
+ run->s.regs.crs[0] |= CR0_STORAGE_PROTECTION_OVERRIDE;
+ run->kvm_dirty_regs = KVM_SYNC_CRS;
+ HOST_SYNC(vm, 70);
+
+ /* Write, read back, mismatching key,
+ * storage protection override, all pages
+ */
+ reroll_mem1();
+ vcpu_write_guest_key(vm, guest_mem1, mem1, maxsize, 2);
+ HOST_SYNC(vm, 80); // Copy in vm
+ memset(mem2, 0xaa, sizeof(mem2));
+ vcpu_read_guest_key(vm, mem2, guest_mem2, maxsize, 2);
+ TEST_ASSERT(!memcmp(mem1, mem2, maxsize),
+ "Memory contents do not match!");
+
+ /* VM memop, write, read back, matching key */
+ reroll_mem1();
+ vm_write_guest_key(vm, guest_mem1_abs, mem1, maxsize, 9);
+ HOST_SYNC(vm, 90); // Copy in vm
+ memset(mem2, 0xaa, sizeof(mem2));
+ vm_read_guest_key(vm, mem2, guest_mem2_abs, maxsize, 9);
+ TEST_ASSERT(!memcmp(mem1, mem2, maxsize),
+ "Memory contents do not match!");
+ vm_check_guest_key(vm, ACCESS_WRITE, guest_mem1_abs, maxsize, 9);
+ vm_check_guest_key(vm, ACCESS_READ, guest_mem2_abs, maxsize, 9);
+
+ /* VM memop, write, read back, key 0 */
+ reroll_mem1();
+ vm_write_guest_key(vm, guest_mem1_abs, mem1, maxsize, 0);
+ HOST_SYNC(vm, 100); // Copy in vm
+ memset(mem2, 0xaa, sizeof(mem2));
+ vm_read_guest_key(vm, mem2, guest_mem2_abs, maxsize, 0);
+ TEST_ASSERT(!memcmp(mem1, mem2, maxsize),
+ "Memory contents do not match!");
+ rv = _vm_check_guest_key(vm, ACCESS_READ, guest_mem1_abs, maxsize, 9);
+ TEST_ASSERT(rv == 0, "Check should succeed");
+ vm_check_guest_key(vm, ACCESS_WRITE, guest_mem1_abs, maxsize, 0);
+ vm_check_guest_key(vm, ACCESS_READ, guest_mem2_abs, maxsize, 0);
+
+ /* VM memop, fail to write, fail to read, mismatching key,
+ * storage protection override does not apply to VM memops
+ */
+ rv = _vm_write_guest_key(vm, guest_mem1_abs, mem1, maxsize, 2);
+ TEST_ASSERT(rv == 4, "Store should result in protection exception");
+ rv = _vm_read_guest_key(vm, mem2, guest_mem2_abs, maxsize, 2);
+ TEST_ASSERT(rv == 4, "Fetch should result in protection exception");
+ rv = _vm_check_guest_key(vm, ACCESS_WRITE, guest_mem1_abs, maxsize, 2);
+ TEST_ASSERT(rv == 4, "Check should indicate protection exception");
+ rv = _vm_check_guest_key(vm, ACCESS_READ, guest_mem2_abs, maxsize, 2);
+ TEST_ASSERT(rv == 4, "Check should indicate protection exception");
+
+ /* VM memop, fail to read from 0 absolute/virtual, mismatching key,
+ * fetch protection override does not apply to VM memops
+ */
+ rv = _vm_read_guest_key(vm, mem2, 0, 2048, 2);
+ TEST_ASSERT(rv != 0, "Fetch should result in exception");
+ rv = _vm_read_guest_key(vm, mem2, addr_gva2gpa(vm, 0), 2048, 2);
+ TEST_ASSERT(rv == 4, "Fetch should result in protection exception");
+ } else {
+ struct ucall uc;
+
+ do {
+ vcpu_run(vm, VCPU_ID);
+ get_ucall(vm, VCPU_ID, &uc);
+ ASSERT_EQ(uc.cmd, UCALL_SYNC);
+ } while (uc.args[1] < 100);
+ }
+
+ /* Check error conditions */
+
+ /* Bad size: */
+ rv = _vcpu_write_guest(vm, (uintptr_t)mem1, mem1, -1);
TEST_ASSERT(rv == -1 && errno == E2BIG, "ioctl allows insane sizes");
/* Zero size: */
- ksmo.gaddr = (uintptr_t)mem1;
- ksmo.flags = 0;
- ksmo.size = 0;
- ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
- ksmo.buf = (uintptr_t)mem1;
- ksmo.ar = 0;
- rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, &ksmo);
+ rv = _vcpu_write_guest(vm, (uintptr_t)mem1, mem1, 0);
TEST_ASSERT(rv == -1 && (errno == EINVAL || errno == ENOMEM),
"ioctl allows 0 as size");
/* Bad flags: */
- ksmo.gaddr = (uintptr_t)mem1;
+ ksmo.gaddr = guest_mem1;
ksmo.flags = -1;
ksmo.size = maxsize;
ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
@@ -115,7 +549,7 @@ int main(int argc, char *argv[])
TEST_ASSERT(rv == -1 && errno == EINVAL, "ioctl allows all flags");
/* Bad operation: */
- ksmo.gaddr = (uintptr_t)mem1;
+ ksmo.gaddr = guest_mem1;
ksmo.flags = 0;
ksmo.size = maxsize;
ksmo.op = -1;
@@ -135,21 +569,17 @@ int main(int argc, char *argv[])
TEST_ASSERT(rv > 0, "ioctl does not report bad guest memory access");
/* Bad host address: */
- ksmo.gaddr = (uintptr_t)mem1;
- ksmo.flags = 0;
- ksmo.size = maxsize;
- ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
- ksmo.buf = 0;
- ksmo.ar = 0;
- rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, &ksmo);
+ rv = _vcpu_write_guest(vm, guest_mem1, 0, maxsize);
TEST_ASSERT(rv == -1 && errno == EFAULT,
"ioctl does not report bad host memory address");
- /* Bad access register: */
+ /* Enable AR mode */
run->psw_mask &= ~(3UL << (63 - 17));
- run->psw_mask |= 1UL << (63 - 17); /* Enable AR mode */
- vcpu_run(vm, VCPU_ID); /* To sync new state to SIE block */
- ksmo.gaddr = (uintptr_t)mem1;
+ run->psw_mask |= 1UL << (63 - 17);
+ HOST_SYNC(vm, 110);
+
+ /* Bad access register: */
+ ksmo.gaddr = guest_mem1;
ksmo.flags = 0;
ksmo.size = maxsize;
ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
@@ -157,8 +587,10 @@ int main(int argc, char *argv[])
ksmo.ar = 17;
rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, &ksmo);
TEST_ASSERT(rv == -1 && errno == EINVAL, "ioctl allows ARs > 15");
- run->psw_mask &= ~(3UL << (63 - 17)); /* Disable AR mode */
- vcpu_run(vm, VCPU_ID); /* Run to sync new state */
+
+ /* Disable AR mode */
+ run->psw_mask &= ~(3UL << (63 - 17));
+ HOST_SYNC(vm, 120);
kvm_vm_free(vm);
--
2.32.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3 05/10] KVM: s390: Add optional storage key checking to MEMOP IOCTL
2022-02-09 17:04 ` [PATCH v3 05/10] KVM: s390: Add optional storage key checking to MEMOP IOCTL Janis Schoetterl-Glausch
@ 2022-02-10 9:29 ` Christian Borntraeger
2022-02-10 9:40 ` Janis Schoetterl-Glausch
2022-02-11 8:53 ` Janosch Frank
2022-02-10 11:59 ` Janis Schoetterl-Glausch
1 sibling, 2 replies; 21+ messages in thread
From: Christian Borntraeger @ 2022-02-10 9:29 UTC (permalink / raw)
To: Janis Schoetterl-Glausch, Janosch Frank, Claudio Imbrenda
Cc: Alexander Gordeev, David Hildenbrand, Jonathan Corbet, kvm,
linux-doc, linux-kernel, linux-s390, Paolo Bonzini,
Sven Schnelle, Vasily Gorbik, Heiko Carstens
Am 09.02.22 um 18:04 schrieb Janis Schoetterl-Glausch:
> User space needs a mechanism to perform key checked accesses when
> emulating instructions.
>
> The key can be passed as an additional argument.
> Having an additional argument is flexible, as user space can
> pass the guest PSW's key, in order to make an access the same way the
> CPU would, or pass another key if necessary.
>
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> Acked-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Claudio, Janosch, can you confirm that this is still valid?
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
minor thing below
> ---
> arch/s390/kvm/kvm-s390.c | 30 ++++++++++++++++++++----------
> include/uapi/linux/kvm.h | 6 +++++-
> 2 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index cf347e1a4f17..85763ec7bc60 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -32,6 +32,7 @@
> #include <linux/sched/signal.h>
> #include <linux/string.h>
> #include <linux/pgtable.h>
> +#include <linux/bitfield.h>
do we still need that after the changes?
>
> #include <asm/asm-offsets.h>
> #include <asm/lowcore.h>
> @@ -2359,6 +2360,11 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
> return r;
> }
>
> +static bool access_key_invalid(u8 access_key)
> +{
> + return access_key > 0xf;
> +}
> +
> long kvm_arch_vm_ioctl(struct file *filp,
> unsigned int ioctl, unsigned long arg)
> {
> @@ -4690,17 +4696,19 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> void *tmpbuf = NULL;
> int r = 0;
> const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION
> - | KVM_S390_MEMOP_F_CHECK_ONLY;
> + | KVM_S390_MEMOP_F_CHECK_ONLY
> + | KVM_S390_MEMOP_F_SKEY_PROTECTION;
>
> if (mop->flags & ~supported_flags || mop->ar >= NUM_ACRS || !mop->size)
> return -EINVAL;
> -
> if (mop->size > MEM_OP_MAX_SIZE)
> return -E2BIG;
> -
> if (kvm_s390_pv_cpu_is_protected(vcpu))
> return -EINVAL;
> -
> + if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
> + if (access_key_invalid(mop->key))
> + return -EINVAL;
> + }
> if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
> tmpbuf = vmalloc(mop->size);
> if (!tmpbuf)
> @@ -4710,11 +4718,12 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> switch (mop->op) {
> case KVM_S390_MEMOP_LOGICAL_READ:
> if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
> - r = check_gva_range(vcpu, mop->gaddr, mop->ar,
> - mop->size, GACC_FETCH, 0);
> + r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
> + GACC_FETCH, mop->key);
> break;
> }
> - r = read_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
> + r = read_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
> + mop->size, mop->key);
> if (r == 0) {
> if (copy_to_user(uaddr, tmpbuf, mop->size))
> r = -EFAULT;
> @@ -4722,15 +4731,16 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> break;
> case KVM_S390_MEMOP_LOGICAL_WRITE:
> if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
> - r = check_gva_range(vcpu, mop->gaddr, mop->ar,
> - mop->size, GACC_STORE, 0);
> + r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
> + GACC_STORE, mop->key);
> break;
> }
> if (copy_from_user(tmpbuf, uaddr, mop->size)) {
> r = -EFAULT;
> break;
> }
> - r = write_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
> + r = write_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
> + mop->size, mop->key);
> break;
> }
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index b46bcdb0cab1..44558cf4c52e 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -562,7 +562,10 @@ struct kvm_s390_mem_op {
> __u32 op; /* type of operation */
> __u64 buf; /* buffer in userspace */
> union {
> - __u8 ar; /* the access register number */
> + struct {
> + __u8 ar; /* the access register number */
> + __u8 key; /* access key, ignored if flag unset */
> + };
> __u32 sida_offset; /* offset into the sida */
> __u8 reserved[32]; /* should be set to 0 */
> };
> @@ -575,6 +578,7 @@ struct kvm_s390_mem_op {
> /* flags for kvm_s390_mem_op->flags */
> #define KVM_S390_MEMOP_F_CHECK_ONLY (1ULL << 0)
> #define KVM_S390_MEMOP_F_INJECT_EXCEPTION (1ULL << 1)
> +#define KVM_S390_MEMOP_F_SKEY_PROTECTION (1ULL << 2)
>
> /* for KVM_INTERRUPT */
> struct kvm_interrupt {
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 05/10] KVM: s390: Add optional storage key checking to MEMOP IOCTL
2022-02-10 9:29 ` Christian Borntraeger
@ 2022-02-10 9:40 ` Janis Schoetterl-Glausch
2022-02-10 9:43 ` Christian Borntraeger
2022-02-11 8:53 ` Janosch Frank
1 sibling, 1 reply; 21+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-10 9:40 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank, Claudio Imbrenda
Cc: Alexander Gordeev, David Hildenbrand, Jonathan Corbet, kvm,
linux-doc, linux-kernel, linux-s390, Paolo Bonzini,
Sven Schnelle, Vasily Gorbik, Heiko Carstens
On 2/10/22 10:29, Christian Borntraeger wrote:
> Am 09.02.22 um 18:04 schrieb Janis Schoetterl-Glausch:
>> User space needs a mechanism to perform key checked accesses when
>> emulating instructions.
>>
>> The key can be passed as an additional argument.
>> Having an additional argument is flexible, as user space can
>> pass the guest PSW's key, in order to make an access the same way the
>> CPU would, or pass another key if necessary.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>> Acked-by: Janosch Frank <frankja@linux.ibm.com>
>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>
> Claudio, Janosch, can you confirm that this is still valid?
>
>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Not @linux.ibm.com?
>
> minor thing below
>> ---
>> arch/s390/kvm/kvm-s390.c | 30 ++++++++++++++++++++----------
>> include/uapi/linux/kvm.h | 6 +++++-
>> 2 files changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index cf347e1a4f17..85763ec7bc60 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -32,6 +32,7 @@
>> #include <linux/sched/signal.h>
>> #include <linux/string.h>
>> #include <linux/pgtable.h>
>> +#include <linux/bitfield.h>
>
> do we still need that after the changes?
No, not since we moved the key out of the flags.
>
>> #include <asm/asm-offsets.h>
>> #include <asm/lowcore.h>
>> @@ -2359,6 +2360,11 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>> return r;
>> }
>> +static bool access_key_invalid(u8 access_key)
>> +{
>> + return access_key > 0xf;
>> +}
>> +
>> long kvm_arch_vm_ioctl(struct file *filp,
>> unsigned int ioctl, unsigned long arg)
>> {
>> @@ -4690,17 +4696,19 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>> void *tmpbuf = NULL;
>> int r = 0;
>> const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION
>> - | KVM_S390_MEMOP_F_CHECK_ONLY;
>> + | KVM_S390_MEMOP_F_CHECK_ONLY
>> + | KVM_S390_MEMOP_F_SKEY_PROTECTION;
>> if (mop->flags & ~supported_flags || mop->ar >= NUM_ACRS || !mop->size)
>> return -EINVAL;
>> -
>> if (mop->size > MEM_OP_MAX_SIZE)
>> return -E2BIG;
>> -
>> if (kvm_s390_pv_cpu_is_protected(vcpu))
>> return -EINVAL;
>> -
>> + if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
>> + if (access_key_invalid(mop->key))
>> + return -EINVAL;
>> + }
>> if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
>> tmpbuf = vmalloc(mop->size);
>> if (!tmpbuf)
>> @@ -4710,11 +4718,12 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>> switch (mop->op) {
>> case KVM_S390_MEMOP_LOGICAL_READ:
>> if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
>> - r = check_gva_range(vcpu, mop->gaddr, mop->ar,
>> - mop->size, GACC_FETCH, 0);
>> + r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
>> + GACC_FETCH, mop->key);
>> break;
>> }
>> - r = read_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
>> + r = read_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
>> + mop->size, mop->key);
>> if (r == 0) {
>> if (copy_to_user(uaddr, tmpbuf, mop->size))
>> r = -EFAULT;
>> @@ -4722,15 +4731,16 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>> break;
>> case KVM_S390_MEMOP_LOGICAL_WRITE:
>> if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
>> - r = check_gva_range(vcpu, mop->gaddr, mop->ar,
>> - mop->size, GACC_STORE, 0);
>> + r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
>> + GACC_STORE, mop->key);
>> break;
>> }
>> if (copy_from_user(tmpbuf, uaddr, mop->size)) {
>> r = -EFAULT;
>> break;
>> }
>> - r = write_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
>> + r = write_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
>> + mop->size, mop->key);
>> break;
>> }
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index b46bcdb0cab1..44558cf4c52e 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -562,7 +562,10 @@ struct kvm_s390_mem_op {
>> __u32 op; /* type of operation */
>> __u64 buf; /* buffer in userspace */
>> union {
>> - __u8 ar; /* the access register number */
>> + struct {
>> + __u8 ar; /* the access register number */
>> + __u8 key; /* access key, ignored if flag unset */
>> + };
>> __u32 sida_offset; /* offset into the sida */
>> __u8 reserved[32]; /* should be set to 0 */
>> };
>> @@ -575,6 +578,7 @@ struct kvm_s390_mem_op {
>> /* flags for kvm_s390_mem_op->flags */
>> #define KVM_S390_MEMOP_F_CHECK_ONLY (1ULL << 0)
>> #define KVM_S390_MEMOP_F_INJECT_EXCEPTION (1ULL << 1)
>> +#define KVM_S390_MEMOP_F_SKEY_PROTECTION (1ULL << 2)
>> /* for KVM_INTERRUPT */
>> struct kvm_interrupt {
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 05/10] KVM: s390: Add optional storage key checking to MEMOP IOCTL
2022-02-10 9:40 ` Janis Schoetterl-Glausch
@ 2022-02-10 9:43 ` Christian Borntraeger
0 siblings, 0 replies; 21+ messages in thread
From: Christian Borntraeger @ 2022-02-10 9:43 UTC (permalink / raw)
To: Janis Schoetterl-Glausch, Janosch Frank, Claudio Imbrenda
Cc: Alexander Gordeev, David Hildenbrand, Jonathan Corbet, kvm,
linux-doc, linux-kernel, linux-s390, Paolo Bonzini,
Sven Schnelle, Vasily Gorbik, Heiko Carstens
Am 10.02.22 um 10:40 schrieb Janis Schoetterl-Glausch:
> On 2/10/22 10:29, Christian Borntraeger wrote:
>> Am 09.02.22 um 18:04 schrieb Janis Schoetterl-Glausch:
>>> User space needs a mechanism to perform key checked accesses when
>>> emulating instructions.
>>>
>>> The key can be passed as an additional argument.
>>> Having an additional argument is flexible, as user space can
>>> pass the guest PSW's key, in order to make an access the same way the
>>> CPU would, or pass another key if necessary.
>>>
>>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>>> Acked-by: Janosch Frank <frankja@linux.ibm.com>
>>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>>
>> Claudio, Janosch, can you confirm that this is still valid?
>>
>>
>> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
>
> Not @linux.ibm.com?
Yes, of course. Old habits...
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
>>
>> minor thing below
>>> ---
>>> arch/s390/kvm/kvm-s390.c | 30 ++++++++++++++++++++----------
>>> include/uapi/linux/kvm.h | 6 +++++-
>>> 2 files changed, 25 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index cf347e1a4f17..85763ec7bc60 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -32,6 +32,7 @@
>>> #include <linux/sched/signal.h>
>>> #include <linux/string.h>
>>> #include <linux/pgtable.h>
>>> +#include <linux/bitfield.h>
>>
>> do we still need that after the changes?
>
> No, not since we moved the key out of the flags.
>>
>>> #include <asm/asm-offsets.h>
>>> #include <asm/lowcore.h>
>>> @@ -2359,6 +2360,11 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>>> return r;
>>> }
>>> +static bool access_key_invalid(u8 access_key)
>>> +{
>>> + return access_key > 0xf;
>>> +}
>>> +
>>> long kvm_arch_vm_ioctl(struct file *filp,
>>> unsigned int ioctl, unsigned long arg)
>>> {
>>> @@ -4690,17 +4696,19 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>>> void *tmpbuf = NULL;
>>> int r = 0;
>>> const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION
>>> - | KVM_S390_MEMOP_F_CHECK_ONLY;
>>> + | KVM_S390_MEMOP_F_CHECK_ONLY
>>> + | KVM_S390_MEMOP_F_SKEY_PROTECTION;
>>> if (mop->flags & ~supported_flags || mop->ar >= NUM_ACRS || !mop->size)
>>> return -EINVAL;
>>> -
>>> if (mop->size > MEM_OP_MAX_SIZE)
>>> return -E2BIG;
>>> -
>>> if (kvm_s390_pv_cpu_is_protected(vcpu))
>>> return -EINVAL;
>>> -
>>> + if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
>>> + if (access_key_invalid(mop->key))
>>> + return -EINVAL;
>>> + }
>>> if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
>>> tmpbuf = vmalloc(mop->size);
>>> if (!tmpbuf)
>>> @@ -4710,11 +4718,12 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>>> switch (mop->op) {
>>> case KVM_S390_MEMOP_LOGICAL_READ:
>>> if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
>>> - r = check_gva_range(vcpu, mop->gaddr, mop->ar,
>>> - mop->size, GACC_FETCH, 0);
>>> + r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
>>> + GACC_FETCH, mop->key);
>>> break;
>>> }
>>> - r = read_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
>>> + r = read_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
>>> + mop->size, mop->key);
>>> if (r == 0) {
>>> if (copy_to_user(uaddr, tmpbuf, mop->size))
>>> r = -EFAULT;
>>> @@ -4722,15 +4731,16 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>>> break;
>>> case KVM_S390_MEMOP_LOGICAL_WRITE:
>>> if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
>>> - r = check_gva_range(vcpu, mop->gaddr, mop->ar,
>>> - mop->size, GACC_STORE, 0);
>>> + r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
>>> + GACC_STORE, mop->key);
>>> break;
>>> }
>>> if (copy_from_user(tmpbuf, uaddr, mop->size)) {
>>> r = -EFAULT;
>>> break;
>>> }
>>> - r = write_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
>>> + r = write_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
>>> + mop->size, mop->key);
>>> break;
>>> }
>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>>> index b46bcdb0cab1..44558cf4c52e 100644
>>> --- a/include/uapi/linux/kvm.h
>>> +++ b/include/uapi/linux/kvm.h
>>> @@ -562,7 +562,10 @@ struct kvm_s390_mem_op {
>>> __u32 op; /* type of operation */
>>> __u64 buf; /* buffer in userspace */
>>> union {
>>> - __u8 ar; /* the access register number */
>>> + struct {
>>> + __u8 ar; /* the access register number */
>>> + __u8 key; /* access key, ignored if flag unset */
>>> + };
>>> __u32 sida_offset; /* offset into the sida */
>>> __u8 reserved[32]; /* should be set to 0 */
>>> };
>>> @@ -575,6 +578,7 @@ struct kvm_s390_mem_op {
>>> /* flags for kvm_s390_mem_op->flags */
>>> #define KVM_S390_MEMOP_F_CHECK_ONLY (1ULL << 0)
>>> #define KVM_S390_MEMOP_F_INJECT_EXCEPTION (1ULL << 1)
>>> +#define KVM_S390_MEMOP_F_SKEY_PROTECTION (1ULL << 2)
>>> /* for KVM_INTERRUPT */
>>> struct kvm_interrupt {
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 05/10] KVM: s390: Add optional storage key checking to MEMOP IOCTL
2022-02-09 17:04 ` [PATCH v3 05/10] KVM: s390: Add optional storage key checking to MEMOP IOCTL Janis Schoetterl-Glausch
2022-02-10 9:29 ` Christian Borntraeger
@ 2022-02-10 11:59 ` Janis Schoetterl-Glausch
2022-02-10 16:33 ` Claudio Imbrenda
1 sibling, 1 reply; 21+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-10 11:59 UTC (permalink / raw)
To: Christian Borntraeger, Heiko Carstens, Janosch Frank
Cc: Alexander Gordeev, Claudio Imbrenda, David Hildenbrand,
Jonathan Corbet, kvm, linux-doc, linux-kernel, linux-s390,
Paolo Bonzini, Sven Schnelle, Vasily Gorbik
On 2/9/22 18:04, Janis Schoetterl-Glausch wrote:
> User space needs a mechanism to perform key checked accesses when
> emulating instructions.
>
> The key can be passed as an additional argument.
> Having an additional argument is flexible, as user space can
> pass the guest PSW's key, in order to make an access the same way the
> CPU would, or pass another key if necessary.
>
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> Acked-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> arch/s390/kvm/kvm-s390.c | 30 ++++++++++++++++++++----------
> include/uapi/linux/kvm.h | 6 +++++-
> 2 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index cf347e1a4f17..85763ec7bc60 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -32,6 +32,7 @@
> #include <linux/sched/signal.h>
> #include <linux/string.h>
> #include <linux/pgtable.h>
> +#include <linux/bitfield.h>
>
> #include <asm/asm-offsets.h>
> #include <asm/lowcore.h>
> @@ -2359,6 +2360,11 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
> return r;
> }
>
> +static bool access_key_invalid(u8 access_key)
> +{
> + return access_key > 0xf;
> +}
> +
> long kvm_arch_vm_ioctl(struct file *filp,
> unsigned int ioctl, unsigned long arg)
> {
> @@ -4690,17 +4696,19 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> void *tmpbuf = NULL;
> int r = 0;
> const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION
> - | KVM_S390_MEMOP_F_CHECK_ONLY;
> + | KVM_S390_MEMOP_F_CHECK_ONLY
> + | KVM_S390_MEMOP_F_SKEY_PROTECTION;
>
> if (mop->flags & ~supported_flags || mop->ar >= NUM_ACRS || !mop->size)
> return -EINVAL;
> -
> if (mop->size > MEM_OP_MAX_SIZE)
> return -E2BIG;
> -
> if (kvm_s390_pv_cpu_is_protected(vcpu))
> return -EINVAL;
> -
> + if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
> + if (access_key_invalid(mop->key))
> + return -EINVAL;
I got this wrong unfortunately, we need to explicitly default to key 0, i.e.
+ } else {
+ mop->key = 0;
Same for the vm memop.
Didn't have a test case for this, yet.
> + }> if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
> tmpbuf = vmalloc(mop->size);
> if (!tmpbuf)
> @@ -4710,11 +4718,12 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> switch (mop->op) {
> case KVM_S390_MEMOP_LOGICAL_READ:
> if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
> - r = check_gva_range(vcpu, mop->gaddr, mop->ar,
> - mop->size, GACC_FETCH, 0);
> + r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
> + GACC_FETCH, mop->key);
> break;
> }
> - r = read_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
> + r = read_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
> + mop->size, mop->key);
> if (r == 0) {
> if (copy_to_user(uaddr, tmpbuf, mop->size))
> r = -EFAULT;
> @@ -4722,15 +4731,16 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> break;
> case KVM_S390_MEMOP_LOGICAL_WRITE:
> if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
> - r = check_gva_range(vcpu, mop->gaddr, mop->ar,
> - mop->size, GACC_STORE, 0);
> + r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
> + GACC_STORE, mop->key);
> break;
> }
> if (copy_from_user(tmpbuf, uaddr, mop->size)) {
> r = -EFAULT;
> break;
> }
> - r = write_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
> + r = write_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
> + mop->size, mop->key);
> break;
> }
>
[...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 06/10] KVM: s390: Add vm IOCTL for key checked guest absolute memory access
2022-02-09 17:04 ` [PATCH v3 06/10] KVM: s390: Add vm IOCTL for key checked guest absolute memory access Janis Schoetterl-Glausch
@ 2022-02-10 14:40 ` Christian Borntraeger
2022-02-10 16:25 ` Claudio Imbrenda
0 siblings, 1 reply; 21+ messages in thread
From: Christian Borntraeger @ 2022-02-10 14:40 UTC (permalink / raw)
To: Janis Schoetterl-Glausch, Heiko Carstens, Janosch Frank
Cc: Alexander Gordeev, Claudio Imbrenda, David Hildenbrand,
Jonathan Corbet, kvm, linux-doc, linux-kernel, linux-s390,
Paolo Bonzini, Sven Schnelle, Vasily Gorbik
Am 09.02.22 um 18:04 schrieb Janis Schoetterl-Glausch:
> Channel I/O honors storage keys and is performed on absolute memory.
> For I/O emulation user space therefore needs to be able to do key
> checked accesses.
> The vm IOCTL supports read/write accesses, as well as checking
> if an access would succeed.
> Unlike relying on KVM_S390_GET_SKEYS for key checking would,
> the vm IOCTL performs the check in lockstep with the read or write,
> by, ultimately, mapping the access to move instructions that
> support key protection checking with a supplied key.
> Fetch and storage protection override are not applicable to absolute
> accesses and so are not applied as they are when using the vcpu memop.
>
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> Acked-by: Janosch Frank <frankja@linux.ibm.com>
same comment that you did for the vcpu part (default key)
with that.
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> arch/s390/kvm/gaccess.c | 72 ++++++++++++++++++++++++++++++++++++
> arch/s390/kvm/gaccess.h | 6 +++
> arch/s390/kvm/kvm-s390.c | 79 ++++++++++++++++++++++++++++++++++++++++
> include/uapi/linux/kvm.h | 2 +
> 4 files changed, 159 insertions(+)
>
> diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> index 37838f637707..d53a183c2005 100644
> --- a/arch/s390/kvm/gaccess.c
> +++ b/arch/s390/kvm/gaccess.c
> @@ -795,6 +795,35 @@ static int low_address_protection_enabled(struct kvm_vcpu *vcpu,
> return 1;
> }
>
> +static int vm_check_access_key(struct kvm *kvm, u8 access_key,
> + enum gacc_mode mode, gpa_t gpa)
> +{
> + u8 storage_key, access_control;
> + bool fetch_protected;
> + unsigned long hva;
> + int r;
> +
> + if (access_key == 0)
> + return 0;
> +
> + hva = gfn_to_hva(kvm, gpa_to_gfn(gpa));
> + if (kvm_is_error_hva(hva))
> + return PGM_ADDRESSING;
> +
> + mmap_read_lock(current->mm);
> + r = get_guest_storage_key(current->mm, hva, &storage_key);
> + mmap_read_unlock(current->mm);
> + if (r)
> + return r;
> + access_control = FIELD_GET(_PAGE_ACC_BITS, storage_key);
> + if (access_control == access_key)
> + return 0;
> + fetch_protected = storage_key & _PAGE_FP_BIT;
> + if ((mode == GACC_FETCH || mode == GACC_IFETCH) && !fetch_protected)
> + return 0;
> + return PGM_PROTECTION;
> +}
> +
> static bool fetch_prot_override_applicable(struct kvm_vcpu *vcpu, enum gacc_mode mode,
> union asce asce)
> {
> @@ -994,6 +1023,26 @@ access_guest_page_with_key(struct kvm *kvm, enum gacc_mode mode, gpa_t gpa,
> return 0;
> }
>
> +int access_guest_abs_with_key(struct kvm *kvm, gpa_t gpa, void *data,
> + unsigned long len, enum gacc_mode mode, u8 access_key)
> +{
> + int offset = offset_in_page(gpa);
> + int fragment_len;
> + int rc;
> +
> + while (min(PAGE_SIZE - offset, len) > 0) {
> + fragment_len = min(PAGE_SIZE - offset, len);
> + rc = access_guest_page_with_key(kvm, mode, gpa, data, fragment_len, access_key);
> + if (rc)
> + return rc;
> + offset = 0;
> + len -= fragment_len;
> + data += fragment_len;
> + gpa += fragment_len;
> + }
> + return 0;
> +}
> +
> int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
> void *data, unsigned long len, enum gacc_mode mode,
> u8 access_key)
> @@ -1144,6 +1193,29 @@ int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
> return rc;
> }
>
> +/**
> + * check_gpa_range - test a range of guest physical addresses for accessibility
> + * @kvm: virtual machine instance
> + * @gpa: guest physical address
> + * @length: length of test range
> + * @mode: access mode to test, relevant for storage keys
> + * @access_key: access key to mach the storage keys with
> + */
> +int check_gpa_range(struct kvm *kvm, unsigned long gpa, unsigned long length,
> + enum gacc_mode mode, u8 access_key)
> +{
> + unsigned int fragment_len;
> + int rc = 0;
> +
> + while (length && !rc) {
> + fragment_len = min(PAGE_SIZE - offset_in_page(gpa), length);
> + rc = vm_check_access_key(kvm, access_key, mode, gpa);
> + length -= fragment_len;
> + gpa += fragment_len;
> + }
> + return rc;
> +}
> +
> /**
> * kvm_s390_check_low_addr_prot_real - check for low-address protection
> * @vcpu: virtual cpu
> diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
> index c5f2e7311b17..1124ff282012 100644
> --- a/arch/s390/kvm/gaccess.h
> +++ b/arch/s390/kvm/gaccess.h
> @@ -193,6 +193,12 @@ int guest_translate_address_with_key(struct kvm_vcpu *vcpu, unsigned long gva, u
> int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
> unsigned long length, enum gacc_mode mode, u8 access_key);
>
> +int check_gpa_range(struct kvm *kvm, unsigned long gpa, unsigned long length,
> + enum gacc_mode mode, u8 access_key);
> +
> +int access_guest_abs_with_key(struct kvm *kvm, gpa_t gpa, void *data,
> + unsigned long len, enum gacc_mode mode, u8 access_key);
> +
> int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
> void *data, unsigned long len, enum gacc_mode mode,
> u8 access_key);
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 85763ec7bc60..7ee3d2e8ecf2 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2365,6 +2365,76 @@ static bool access_key_invalid(u8 access_key)
> return access_key > 0xf;
> }
>
> +static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
> +{
> + void __user *uaddr = (void __user *)mop->buf;
> + u64 supported_flags;
> + void *tmpbuf = NULL;
> + int r, srcu_idx;
> +
> + supported_flags = KVM_S390_MEMOP_F_SKEY_PROTECTION
> + | KVM_S390_MEMOP_F_CHECK_ONLY;
> + if (mop->flags & ~supported_flags)
> + return -EINVAL;
> + if (mop->size > MEM_OP_MAX_SIZE)
> + return -E2BIG;
> + if (kvm_s390_pv_is_protected(kvm))
> + return -EINVAL;
> + if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
> + if (access_key_invalid(mop->key))
> + return -EINVAL;
> + }
> + if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
> + tmpbuf = vmalloc(mop->size);
> + if (!tmpbuf)
> + return -ENOMEM;
> + }
> +
> + srcu_idx = srcu_read_lock(&kvm->srcu);
> +
> + if (kvm_is_error_gpa(kvm, mop->gaddr)) {
> + r = PGM_ADDRESSING;
> + goto out_unlock;
> + }
> +
> + switch (mop->op) {
> + case KVM_S390_MEMOP_ABSOLUTE_READ: {
> + if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
> + r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_FETCH, mop->key);
> + } else {
> + r = access_guest_abs_with_key(kvm, mop->gaddr, tmpbuf,
> + mop->size, GACC_FETCH, mop->key);
> + if (r == 0) {
> + if (copy_to_user(uaddr, tmpbuf, mop->size))
> + r = -EFAULT;
> + }
> + }
> + break;
> + }
> + case KVM_S390_MEMOP_ABSOLUTE_WRITE: {
> + if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
> + r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_STORE, mop->key);
> + } else {
> + if (copy_from_user(tmpbuf, uaddr, mop->size)) {
> + r = -EFAULT;
> + break;
> + }
> + r = access_guest_abs_with_key(kvm, mop->gaddr, tmpbuf,
> + mop->size, GACC_STORE, mop->key);
> + }
> + break;
> + }
> + default:
> + r = -EINVAL;
> + }
> +
> +out_unlock:
> + srcu_read_unlock(&kvm->srcu, srcu_idx);
> +
> + vfree(tmpbuf);
> + return r;
> +}
> +
> long kvm_arch_vm_ioctl(struct file *filp,
> unsigned int ioctl, unsigned long arg)
> {
> @@ -2489,6 +2559,15 @@ long kvm_arch_vm_ioctl(struct file *filp,
> }
> break;
> }
> + case KVM_S390_MEM_OP: {
> + struct kvm_s390_mem_op mem_op;
> +
> + if (copy_from_user(&mem_op, argp, sizeof(mem_op)) == 0)
> + r = kvm_s390_vm_mem_op(kvm, &mem_op);
> + else
> + r = -EFAULT;
> + break;
> + }
> default:
> r = -ENOTTY;
> }
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 44558cf4c52e..b83c9286e017 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -575,6 +575,8 @@ struct kvm_s390_mem_op {
> #define KVM_S390_MEMOP_LOGICAL_WRITE 1
> #define KVM_S390_MEMOP_SIDA_READ 2
> #define KVM_S390_MEMOP_SIDA_WRITE 3
> +#define KVM_S390_MEMOP_ABSOLUTE_READ 4
> +#define KVM_S390_MEMOP_ABSOLUTE_WRITE 5
> /* flags for kvm_s390_mem_op->flags */
> #define KVM_S390_MEMOP_F_CHECK_ONLY (1ULL << 0)
> #define KVM_S390_MEMOP_F_INJECT_EXCEPTION (1ULL << 1)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 09/10] KVM: s390: Update api documentation for memop ioctl
2022-02-09 17:04 ` [PATCH v3 09/10] KVM: s390: Update api documentation for memop ioctl Janis Schoetterl-Glausch
@ 2022-02-10 14:41 ` Christian Borntraeger
2022-02-11 9:17 ` Janosch Frank
1 sibling, 0 replies; 21+ messages in thread
From: Christian Borntraeger @ 2022-02-10 14:41 UTC (permalink / raw)
To: Janis Schoetterl-Glausch, Heiko Carstens, Janosch Frank, Jonathan Corbet
Cc: Alexander Gordeev, Claudio Imbrenda, David Hildenbrand, kvm,
linux-doc, linux-kernel, linux-s390, Paolo Bonzini,
Sven Schnelle, Vasily Gorbik
Am 09.02.22 um 18:04 schrieb Janis Schoetterl-Glausch:
> Document all currently existing operations, flags and explain under
> which circumstances they are available. Document the recently
> introduced absolute operations and the storage key protection flag,
> as well as the existing SIDA operations.
Jonathan, doc team, we will carry this patch via the KVM tree.
>
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
> Documentation/virt/kvm/api.rst | 112 ++++++++++++++++++++++++++-------
> include/uapi/linux/kvm.h | 2 +-
> 2 files changed, 91 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index a4267104db50..2d131af44576 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -3683,15 +3683,17 @@ The fields in each entry are defined as follows:
> 4.89 KVM_S390_MEM_OP
> --------------------
>
> -:Capability: KVM_CAP_S390_MEM_OP
> +:Capability: KVM_CAP_S390_MEM_OP, KVM_CAP_S390_PROTECTED, KVM_CAP_S390_MEM_OP_EXTENSION
> :Architectures: s390
> -:Type: vcpu ioctl
> +:Type: vm ioctl, vcpu ioctl
> :Parameters: struct kvm_s390_mem_op (in)
> :Returns: = 0 on success,
> < 0 on generic error (e.g. -EFAULT or -ENOMEM),
> > 0 if an exception occurred while walking the page tables
>
> -Read or write data from/to the logical (virtual) memory of a VCPU.
> +Read or write data from/to the VM's memory.
> +The KVM_CAP_S390_MEM_OP_EXTENSION capability specifies what functionality is
> +supported.
>
> Parameters are specified via the following structure::
>
> @@ -3701,33 +3703,99 @@ Parameters are specified via the following structure::
> __u32 size; /* amount of bytes */
> __u32 op; /* type of operation */
> __u64 buf; /* buffer in userspace */
> - __u8 ar; /* the access register number */
> - __u8 reserved[31]; /* should be set to 0 */
> + union {
> + struct {
> + __u8 ar; /* the access register number */
> + __u8 key; /* access key, ignored if flag unset */
> + };
> + __u32 sida_offset; /* offset into the sida */
> + __u8 reserved[32]; /* ignored */
> + };
> };
>
> -The type of operation is specified in the "op" field. It is either
> -KVM_S390_MEMOP_LOGICAL_READ for reading from logical memory space or
> -KVM_S390_MEMOP_LOGICAL_WRITE for writing to logical memory space. The
> -KVM_S390_MEMOP_F_CHECK_ONLY flag can be set in the "flags" field to check
> -whether the corresponding memory access would create an access exception
> -(without touching the data in the memory at the destination). In case an
> -access exception occurred while walking the MMU tables of the guest, the
> -ioctl returns a positive error number to indicate the type of exception.
> -This exception is also raised directly at the corresponding VCPU if the
> -flag KVM_S390_MEMOP_F_INJECT_EXCEPTION is set in the "flags" field.
> -
> The start address of the memory region has to be specified in the "gaddr"
> field, and the length of the region in the "size" field (which must not
> be 0). The maximum value for "size" can be obtained by checking the
> KVM_CAP_S390_MEM_OP capability. "buf" is the buffer supplied by the
> userspace application where the read data should be written to for
> -KVM_S390_MEMOP_LOGICAL_READ, or where the data that should be written is
> -stored for a KVM_S390_MEMOP_LOGICAL_WRITE. When KVM_S390_MEMOP_F_CHECK_ONLY
> -is specified, "buf" is unused and can be NULL. "ar" designates the access
> -register number to be used; the valid range is 0..15.
> +a read access, or where the data that should be written is stored for
> +a write access. The "reserved" field is meant for future extensions.
> +Reserved and unused values are ignored. Future extension that add members must
> +introduce new flags.
> +
> +The type of operation is specified in the "op" field. Flags modifying
> +their behavior can be set in the "flags" field. Undefined flag bits must
> +be set to 0.
> +
> +Possible operations are:
> + * ``KVM_S390_MEMOP_LOGICAL_READ``
> + * ``KVM_S390_MEMOP_LOGICAL_WRITE``
> + * ``KVM_S390_MEMOP_ABSOLUTE_READ``
> + * ``KVM_S390_MEMOP_ABSOLUTE_WRITE``
> + * ``KVM_S390_MEMOP_SIDA_READ``
> + * ``KVM_S390_MEMOP_SIDA_WRITE``
> +
> +Logical read/write:
> +^^^^^^^^^^^^^^^^^^^
> +
> +Access logical memory, i.e. translate the given guest address to an absolute
> +address given the state of the VCPU and use the absolute address as target of
> +the access. "ar" designates the access register number to be used; the valid
> +range is 0..15.
> +Logical accesses are permitted for the VCPU ioctl only.
> +Logical accesses are permitted for non secure guests only.
> +
> +Supported flags:
> + * ``KVM_S390_MEMOP_F_CHECK_ONLY``
> + * ``KVM_S390_MEMOP_F_INJECT_EXCEPTION``
> + * ``KVM_S390_MEMOP_F_SKEY_PROTECTION``
> +
> +The KVM_S390_MEMOP_F_CHECK_ONLY flag can be set to check whether the
> +corresponding memory access would cause an access exception, without touching
> +the data in memory at the destination.
> +In this case, "buf" is unused and can be NULL.
> +
> +In case an access exception occurred during the access (or would occur
> +in case of KVM_S390_MEMOP_F_CHECK_ONLY), the ioctl returns a positive
> +error number indicating the type of exception. This exception is also
> +raised directly at the corresponding VCPU if the flag
> +KVM_S390_MEMOP_F_INJECT_EXCEPTION is set.
> +
> +If the KVM_S390_MEMOP_F_SKEY_PROTECTION flag is set, storage key
> +protection is also in effect and may cause exceptions if accesses are
> +prohibited given the access key passed in "key".
> +KVM_S390_MEMOP_F_SKEY_PROTECTION is available if KVM_CAP_S390_MEM_OP_EXTENSION
> +is > 0.
> +
> +Absolute read/write:
> +^^^^^^^^^^^^^^^^^^^^
> +
> +Access absolute memory. This operation is intended to be used with the
> +KVM_S390_MEMOP_F_SKEY_PROTECTION flag, to allow accessing memory and performing
> +the checks required for storage key protection as one operation (as opposed to
> +user space getting the storage keys, performing the checks, and accessing
> +memory thereafter, which could lead to a delay between check and access).
> +Absolute accesses are permitted for the VM ioctl if KVM_CAP_S390_MEM_OP_EXTENSION
> +is > 0.
> +Currently absolute accesses are not permitted for VCPU ioctls.
> +Absolute accesses are permitted for non secure guests only.
> +
> +Supported flags:
> + * ``KVM_S390_MEMOP_F_CHECK_ONLY``
> + * ``KVM_S390_MEMOP_F_SKEY_PROTECTION``
> +
> +The semantics of the flags are as for logical accesses.
> +
> +SIDA read/write:
> +^^^^^^^^^^^^^^^^
> +
> +Access the secure instruction data area which contains memory operands necessary
> +for instruction emulation for secure guests.
> +SIDA accesses are available if the KVM_CAP_S390_PROTECTED capability is available.
> +SIDA accesses are permitted for the VCPU ioctl only.
> +SIDA accesses are permitted for secure guests only.
>
> -The "reserved" field is meant for future extensions. It is not used by
> -KVM with the currently defined set of flags.
> +No flags are supported.
>
> 4.90 KVM_S390_GET_SKEYS
> -----------------------
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 26bff414f1a0..fd01fe04a183 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -567,7 +567,7 @@ struct kvm_s390_mem_op {
> __u8 key; /* access key, ignored if flag unset */
> };
> __u32 sida_offset; /* offset into the sida */
> - __u8 reserved[32]; /* should be set to 0 */
> + __u8 reserved[32]; /* ignored */
> };
> };
> /* types for kvm_s390_mem_op->op */
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 06/10] KVM: s390: Add vm IOCTL for key checked guest absolute memory access
2022-02-10 14:40 ` Christian Borntraeger
@ 2022-02-10 16:25 ` Claudio Imbrenda
0 siblings, 0 replies; 21+ messages in thread
From: Claudio Imbrenda @ 2022-02-10 16:25 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Janis Schoetterl-Glausch, Heiko Carstens, Janosch Frank,
Alexander Gordeev, David Hildenbrand, Jonathan Corbet, kvm,
linux-doc, linux-kernel, linux-s390, Paolo Bonzini,
Sven Schnelle, Vasily Gorbik
On Thu, 10 Feb 2022 15:40:03 +0100
Christian Borntraeger <borntraeger@linux.ibm.com> wrote:
> Am 09.02.22 um 18:04 schrieb Janis Schoetterl-Glausch:
> > Channel I/O honors storage keys and is performed on absolute memory.
> > For I/O emulation user space therefore needs to be able to do key
> > checked accesses.
> > The vm IOCTL supports read/write accesses, as well as checking
> > if an access would succeed.
> > Unlike relying on KVM_S390_GET_SKEYS for key checking would,
> > the vm IOCTL performs the check in lockstep with the read or write,
> > by, ultimately, mapping the access to move instructions that
> > support key protection checking with a supplied key.
> > Fetch and storage protection override are not applicable to absolute
> > accesses and so are not applied as they are when using the vcpu memop.
> >
> > Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> > Acked-by: Janosch Frank <frankja@linux.ibm.com>
>
> same comment that you did for the vcpu part (default key)
> with that.
>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
I guess @linux.ibm.com ?
> > ---
> > arch/s390/kvm/gaccess.c | 72 ++++++++++++++++++++++++++++++++++++
> > arch/s390/kvm/gaccess.h | 6 +++
> > arch/s390/kvm/kvm-s390.c | 79 ++++++++++++++++++++++++++++++++++++++++
> > include/uapi/linux/kvm.h | 2 +
> > 4 files changed, 159 insertions(+)
> >
> > diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> > index 37838f637707..d53a183c2005 100644
> > --- a/arch/s390/kvm/gaccess.c
> > +++ b/arch/s390/kvm/gaccess.c
> > @@ -795,6 +795,35 @@ static int low_address_protection_enabled(struct kvm_vcpu *vcpu,
> > return 1;
> > }
> >
> > +static int vm_check_access_key(struct kvm *kvm, u8 access_key,
> > + enum gacc_mode mode, gpa_t gpa)
> > +{
> > + u8 storage_key, access_control;
> > + bool fetch_protected;
> > + unsigned long hva;
> > + int r;
> > +
> > + if (access_key == 0)
> > + return 0;
> > +
> > + hva = gfn_to_hva(kvm, gpa_to_gfn(gpa));
> > + if (kvm_is_error_hva(hva))
> > + return PGM_ADDRESSING;
> > +
> > + mmap_read_lock(current->mm);
> > + r = get_guest_storage_key(current->mm, hva, &storage_key);
> > + mmap_read_unlock(current->mm);
> > + if (r)
> > + return r;
> > + access_control = FIELD_GET(_PAGE_ACC_BITS, storage_key);
> > + if (access_control == access_key)
> > + return 0;
> > + fetch_protected = storage_key & _PAGE_FP_BIT;
> > + if ((mode == GACC_FETCH || mode == GACC_IFETCH) && !fetch_protected)
> > + return 0;
> > + return PGM_PROTECTION;
> > +}
> > +
> > static bool fetch_prot_override_applicable(struct kvm_vcpu *vcpu, enum gacc_mode mode,
> > union asce asce)
> > {
> > @@ -994,6 +1023,26 @@ access_guest_page_with_key(struct kvm *kvm, enum gacc_mode mode, gpa_t gpa,
> > return 0;
> > }
> >
> > +int access_guest_abs_with_key(struct kvm *kvm, gpa_t gpa, void *data,
> > + unsigned long len, enum gacc_mode mode, u8 access_key)
> > +{
> > + int offset = offset_in_page(gpa);
> > + int fragment_len;
> > + int rc;
> > +
> > + while (min(PAGE_SIZE - offset, len) > 0) {
> > + fragment_len = min(PAGE_SIZE - offset, len);
> > + rc = access_guest_page_with_key(kvm, mode, gpa, data, fragment_len, access_key);
> > + if (rc)
> > + return rc;
> > + offset = 0;
> > + len -= fragment_len;
> > + data += fragment_len;
> > + gpa += fragment_len;
> > + }
> > + return 0;
> > +}
> > +
> > int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
> > void *data, unsigned long len, enum gacc_mode mode,
> > u8 access_key)
> > @@ -1144,6 +1193,29 @@ int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
> > return rc;
> > }
> >
> > +/**
> > + * check_gpa_range - test a range of guest physical addresses for accessibility
> > + * @kvm: virtual machine instance
> > + * @gpa: guest physical address
> > + * @length: length of test range
> > + * @mode: access mode to test, relevant for storage keys
> > + * @access_key: access key to mach the storage keys with
> > + */
> > +int check_gpa_range(struct kvm *kvm, unsigned long gpa, unsigned long length,
> > + enum gacc_mode mode, u8 access_key)
> > +{
> > + unsigned int fragment_len;
> > + int rc = 0;
> > +
> > + while (length && !rc) {
> > + fragment_len = min(PAGE_SIZE - offset_in_page(gpa), length);
> > + rc = vm_check_access_key(kvm, access_key, mode, gpa);
> > + length -= fragment_len;
> > + gpa += fragment_len;
> > + }
> > + return rc;
> > +}
> > +
> > /**
> > * kvm_s390_check_low_addr_prot_real - check for low-address protection
> > * @vcpu: virtual cpu
> > diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
> > index c5f2e7311b17..1124ff282012 100644
> > --- a/arch/s390/kvm/gaccess.h
> > +++ b/arch/s390/kvm/gaccess.h
> > @@ -193,6 +193,12 @@ int guest_translate_address_with_key(struct kvm_vcpu *vcpu, unsigned long gva, u
> > int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
> > unsigned long length, enum gacc_mode mode, u8 access_key);
> >
> > +int check_gpa_range(struct kvm *kvm, unsigned long gpa, unsigned long length,
> > + enum gacc_mode mode, u8 access_key);
> > +
> > +int access_guest_abs_with_key(struct kvm *kvm, gpa_t gpa, void *data,
> > + unsigned long len, enum gacc_mode mode, u8 access_key);
> > +
> > int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
> > void *data, unsigned long len, enum gacc_mode mode,
> > u8 access_key);
> > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> > index 85763ec7bc60..7ee3d2e8ecf2 100644
> > --- a/arch/s390/kvm/kvm-s390.c
> > +++ b/arch/s390/kvm/kvm-s390.c
> > @@ -2365,6 +2365,76 @@ static bool access_key_invalid(u8 access_key)
> > return access_key > 0xf;
> > }
> >
> > +static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
> > +{
> > + void __user *uaddr = (void __user *)mop->buf;
> > + u64 supported_flags;
> > + void *tmpbuf = NULL;
> > + int r, srcu_idx;
> > +
> > + supported_flags = KVM_S390_MEMOP_F_SKEY_PROTECTION
> > + | KVM_S390_MEMOP_F_CHECK_ONLY;
> > + if (mop->flags & ~supported_flags)
> > + return -EINVAL;
> > + if (mop->size > MEM_OP_MAX_SIZE)
> > + return -E2BIG;
> > + if (kvm_s390_pv_is_protected(kvm))
> > + return -EINVAL;
> > + if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
> > + if (access_key_invalid(mop->key))
> > + return -EINVAL;
> > + }
> > + if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
> > + tmpbuf = vmalloc(mop->size);
> > + if (!tmpbuf)
> > + return -ENOMEM;
> > + }
> > +
> > + srcu_idx = srcu_read_lock(&kvm->srcu);
> > +
> > + if (kvm_is_error_gpa(kvm, mop->gaddr)) {
> > + r = PGM_ADDRESSING;
> > + goto out_unlock;
> > + }
> > +
> > + switch (mop->op) {
> > + case KVM_S390_MEMOP_ABSOLUTE_READ: {
> > + if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
> > + r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_FETCH, mop->key);
> > + } else {
> > + r = access_guest_abs_with_key(kvm, mop->gaddr, tmpbuf,
> > + mop->size, GACC_FETCH, mop->key);
> > + if (r == 0) {
> > + if (copy_to_user(uaddr, tmpbuf, mop->size))
> > + r = -EFAULT;
> > + }
> > + }
> > + break;
> > + }
> > + case KVM_S390_MEMOP_ABSOLUTE_WRITE: {
> > + if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
> > + r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_STORE, mop->key);
> > + } else {
> > + if (copy_from_user(tmpbuf, uaddr, mop->size)) {
> > + r = -EFAULT;
> > + break;
> > + }
> > + r = access_guest_abs_with_key(kvm, mop->gaddr, tmpbuf,
> > + mop->size, GACC_STORE, mop->key);
> > + }
> > + break;
> > + }
> > + default:
> > + r = -EINVAL;
> > + }
> > +
> > +out_unlock:
> > + srcu_read_unlock(&kvm->srcu, srcu_idx);
> > +
> > + vfree(tmpbuf);
> > + return r;
> > +}
> > +
> > long kvm_arch_vm_ioctl(struct file *filp,
> > unsigned int ioctl, unsigned long arg)
> > {
> > @@ -2489,6 +2559,15 @@ long kvm_arch_vm_ioctl(struct file *filp,
> > }
> > break;
> > }
> > + case KVM_S390_MEM_OP: {
> > + struct kvm_s390_mem_op mem_op;
> > +
> > + if (copy_from_user(&mem_op, argp, sizeof(mem_op)) == 0)
> > + r = kvm_s390_vm_mem_op(kvm, &mem_op);
> > + else
> > + r = -EFAULT;
> > + break;
> > + }
> > default:
> > r = -ENOTTY;
> > }
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 44558cf4c52e..b83c9286e017 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -575,6 +575,8 @@ struct kvm_s390_mem_op {
> > #define KVM_S390_MEMOP_LOGICAL_WRITE 1
> > #define KVM_S390_MEMOP_SIDA_READ 2
> > #define KVM_S390_MEMOP_SIDA_WRITE 3
> > +#define KVM_S390_MEMOP_ABSOLUTE_READ 4
> > +#define KVM_S390_MEMOP_ABSOLUTE_WRITE 5
> > /* flags for kvm_s390_mem_op->flags */
> > #define KVM_S390_MEMOP_F_CHECK_ONLY (1ULL << 0)
> > #define KVM_S390_MEMOP_F_INJECT_EXCEPTION (1ULL << 1)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 05/10] KVM: s390: Add optional storage key checking to MEMOP IOCTL
2022-02-10 11:59 ` Janis Schoetterl-Glausch
@ 2022-02-10 16:33 ` Claudio Imbrenda
0 siblings, 0 replies; 21+ messages in thread
From: Claudio Imbrenda @ 2022-02-10 16:33 UTC (permalink / raw)
To: Janis Schoetterl-Glausch
Cc: Christian Borntraeger, Heiko Carstens, Janosch Frank,
Alexander Gordeev, David Hildenbrand, Jonathan Corbet, kvm,
linux-doc, linux-kernel, linux-s390, Paolo Bonzini,
Sven Schnelle, Vasily Gorbik
On Thu, 10 Feb 2022 12:59:03 +0100
Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:
> On 2/9/22 18:04, Janis Schoetterl-Glausch wrote:
> > User space needs a mechanism to perform key checked accesses when
> > emulating instructions.
> >
> > The key can be passed as an additional argument.
> > Having an additional argument is flexible, as user space can
> > pass the guest PSW's key, in order to make an access the same way the
> > CPU would, or pass another key if necessary.
> >
> > Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> > Acked-by: Janosch Frank <frankja@linux.ibm.com>
> > Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > ---
> > arch/s390/kvm/kvm-s390.c | 30 ++++++++++++++++++++----------
> > include/uapi/linux/kvm.h | 6 +++++-
> > 2 files changed, 25 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> > index cf347e1a4f17..85763ec7bc60 100644
> > --- a/arch/s390/kvm/kvm-s390.c
> > +++ b/arch/s390/kvm/kvm-s390.c
> > @@ -32,6 +32,7 @@
> > #include <linux/sched/signal.h>
> > #include <linux/string.h>
> > #include <linux/pgtable.h>
> > +#include <linux/bitfield.h>
> >
> > #include <asm/asm-offsets.h>
> > #include <asm/lowcore.h>
> > @@ -2359,6 +2360,11 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
> > return r;
> > }
> >
> > +static bool access_key_invalid(u8 access_key)
> > +{
> > + return access_key > 0xf;
> > +}
> > +
> > long kvm_arch_vm_ioctl(struct file *filp,
> > unsigned int ioctl, unsigned long arg)
> > {
> > @@ -4690,17 +4696,19 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> > void *tmpbuf = NULL;
> > int r = 0;
> > const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION
> > - | KVM_S390_MEMOP_F_CHECK_ONLY;
> > + | KVM_S390_MEMOP_F_CHECK_ONLY
> > + | KVM_S390_MEMOP_F_SKEY_PROTECTION;
> >
> > if (mop->flags & ~supported_flags || mop->ar >= NUM_ACRS || !mop->size)
> > return -EINVAL;
> > -
> > if (mop->size > MEM_OP_MAX_SIZE)
> > return -E2BIG;
> > -
> > if (kvm_s390_pv_cpu_is_protected(vcpu))
> > return -EINVAL;
> > -
> > + if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
> > + if (access_key_invalid(mop->key))
> > + return -EINVAL;
>
> I got this wrong unfortunately, we need to explicitly default to key 0, i.e.
> + } else {
> + mop->key = 0;
> Same for the vm memop.
> Didn't have a test case for this, yet.
you can keep my r-b once you fix this (and the spurious include)
> > + }> if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
> > tmpbuf = vmalloc(mop->size);
> > if (!tmpbuf)
> > @@ -4710,11 +4718,12 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> > switch (mop->op) {
> > case KVM_S390_MEMOP_LOGICAL_READ:
> > if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
> > - r = check_gva_range(vcpu, mop->gaddr, mop->ar,
> > - mop->size, GACC_FETCH, 0);
> > + r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
> > + GACC_FETCH, mop->key);
> > break;
> > }
> > - r = read_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
> > + r = read_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
> > + mop->size, mop->key);
> > if (r == 0) {
> > if (copy_to_user(uaddr, tmpbuf, mop->size))
> > r = -EFAULT;
> > @@ -4722,15 +4731,16 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> > break;
> > case KVM_S390_MEMOP_LOGICAL_WRITE:
> > if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
> > - r = check_gva_range(vcpu, mop->gaddr, mop->ar,
> > - mop->size, GACC_STORE, 0);
> > + r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
> > + GACC_STORE, mop->key);
> > break;
> > }
> > if (copy_from_user(tmpbuf, uaddr, mop->size)) {
> > r = -EFAULT;
> > break;
> > }
> > - r = write_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
> > + r = write_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
> > + mop->size, mop->key);
> > break;
> > }
> >
>
> [...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 05/10] KVM: s390: Add optional storage key checking to MEMOP IOCTL
2022-02-10 9:29 ` Christian Borntraeger
2022-02-10 9:40 ` Janis Schoetterl-Glausch
@ 2022-02-11 8:53 ` Janosch Frank
1 sibling, 0 replies; 21+ messages in thread
From: Janosch Frank @ 2022-02-11 8:53 UTC (permalink / raw)
To: Christian Borntraeger, Janis Schoetterl-Glausch, Claudio Imbrenda
Cc: Alexander Gordeev, David Hildenbrand, Jonathan Corbet, kvm,
linux-doc, linux-kernel, linux-s390, Paolo Bonzini,
Sven Schnelle, Vasily Gorbik, Heiko Carstens
On 2/10/22 10:29, Christian Borntraeger wrote:
> Am 09.02.22 um 18:04 schrieb Janis Schoetterl-Glausch:
>> User space needs a mechanism to perform key checked accesses when
>> emulating instructions.
>>
>> The key can be passed as an additional argument.
>> Having an additional argument is flexible, as user space can
>> pass the guest PSW's key, in order to make an access the same way the
>> CPU would, or pass another key if necessary.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>> Acked-by: Janosch Frank <frankja@linux.ibm.com>
>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>
> Claudio, Janosch, can you confirm that this is still valid?
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
>
>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
>
> minor thing below
>> ---
>> arch/s390/kvm/kvm-s390.c | 30 ++++++++++++++++++++----------
>> include/uapi/linux/kvm.h | 6 +++++-
>> 2 files changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index cf347e1a4f17..85763ec7bc60 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -32,6 +32,7 @@
>> #include <linux/sched/signal.h>
>> #include <linux/string.h>
>> #include <linux/pgtable.h>
>> +#include <linux/bitfield.h>
>
> do we still need that after the changes?
>
>>
>> #include <asm/asm-offsets.h>
>> #include <asm/lowcore.h>
>> @@ -2359,6 +2360,11 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>> return r;
>> }
>>
>> +static bool access_key_invalid(u8 access_key)
>> +{
>> + return access_key > 0xf;
>> +}
>> +
>> long kvm_arch_vm_ioctl(struct file *filp,
>> unsigned int ioctl, unsigned long arg)
>> {
>> @@ -4690,17 +4696,19 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>> void *tmpbuf = NULL;
>> int r = 0;
>> const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION
>> - | KVM_S390_MEMOP_F_CHECK_ONLY;
>> + | KVM_S390_MEMOP_F_CHECK_ONLY
>> + | KVM_S390_MEMOP_F_SKEY_PROTECTION;
>>
>> if (mop->flags & ~supported_flags || mop->ar >= NUM_ACRS || !mop->size)
>> return -EINVAL;
>> -
>> if (mop->size > MEM_OP_MAX_SIZE)
>> return -E2BIG;
>> -
>> if (kvm_s390_pv_cpu_is_protected(vcpu))
>> return -EINVAL;
>> -
You want to make it look like the sida op?
Personally I'd leave it as is.
>> + if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
>> + if (access_key_invalid(mop->key))
>> + return -EINVAL;
>> + }
>> if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
>> tmpbuf = vmalloc(mop->size);
>> if (!tmpbuf)
>> @@ -4710,11 +4718,12 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>> switch (mop->op) {
>> case KVM_S390_MEMOP_LOGICAL_READ:
>> if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
>> - r = check_gva_range(vcpu, mop->gaddr, mop->ar,
>> - mop->size, GACC_FETCH, 0);
>> + r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
>> + GACC_FETCH, mop->key);
>> break;
>> }
>> - r = read_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
>> + r = read_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
>> + mop->size, mop->key);
>> if (r == 0) {
>> if (copy_to_user(uaddr, tmpbuf, mop->size))
>> r = -EFAULT;
>> @@ -4722,15 +4731,16 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>> break;
>> case KVM_S390_MEMOP_LOGICAL_WRITE:
>> if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
>> - r = check_gva_range(vcpu, mop->gaddr, mop->ar,
>> - mop->size, GACC_STORE, 0);
>> + r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size,
>> + GACC_STORE, mop->key);
>> break;
>> }
>> if (copy_from_user(tmpbuf, uaddr, mop->size)) {
>> r = -EFAULT;
>> break;
>> }
>> - r = write_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
>> + r = write_guest_with_key(vcpu, mop->gaddr, mop->ar, tmpbuf,
>> + mop->size, mop->key);
>> break;
>> }
>>
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index b46bcdb0cab1..44558cf4c52e 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -562,7 +562,10 @@ struct kvm_s390_mem_op {
>> __u32 op; /* type of operation */
>> __u64 buf; /* buffer in userspace */
>> union {
>> - __u8 ar; /* the access register number */
>> + struct {
>> + __u8 ar; /* the access register number */
>> + __u8 key; /* access key, ignored if flag unset */
>> + };
>> __u32 sida_offset; /* offset into the sida */
>> __u8 reserved[32]; /* should be set to 0 */
>> };
>> @@ -575,6 +578,7 @@ struct kvm_s390_mem_op {
>> /* flags for kvm_s390_mem_op->flags */
>> #define KVM_S390_MEMOP_F_CHECK_ONLY (1ULL << 0)
>> #define KVM_S390_MEMOP_F_INJECT_EXCEPTION (1ULL << 1)
>> +#define KVM_S390_MEMOP_F_SKEY_PROTECTION (1ULL << 2)
>>
>> /* for KVM_INTERRUPT */
>> struct kvm_interrupt {
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 09/10] KVM: s390: Update api documentation for memop ioctl
2022-02-09 17:04 ` [PATCH v3 09/10] KVM: s390: Update api documentation for memop ioctl Janis Schoetterl-Glausch
2022-02-10 14:41 ` Christian Borntraeger
@ 2022-02-11 9:17 ` Janosch Frank
1 sibling, 0 replies; 21+ messages in thread
From: Janosch Frank @ 2022-02-11 9:17 UTC (permalink / raw)
To: Janis Schoetterl-Glausch, Christian Borntraeger, Heiko Carstens
Cc: Alexander Gordeev, Claudio Imbrenda, David Hildenbrand,
Jonathan Corbet, kvm, linux-doc, linux-kernel, linux-s390,
Paolo Bonzini, Sven Schnelle, Vasily Gorbik
On 2/9/22 18:04, Janis Schoetterl-Glausch wrote:
> Document all currently existing operations, flags and explain under
> which circumstances they are available. Document the recently
> introduced absolute operations and the storage key protection flag,
> as well as the existing SIDA operations.
>
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Minor nits below
> ---
> Documentation/virt/kvm/api.rst | 112 ++++++++++++++++++++++++++-------
> include/uapi/linux/kvm.h | 2 +-
> 2 files changed, 91 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index a4267104db50..2d131af44576 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -3683,15 +3683,17 @@ The fields in each entry are defined as follows:
> 4.89 KVM_S390_MEM_OP
> --------------------
>
> -:Capability: KVM_CAP_S390_MEM_OP
> +:Capability: KVM_CAP_S390_MEM_OP, KVM_CAP_S390_PROTECTED, KVM_CAP_S390_MEM_OP_EXTENSION
> :Architectures: s390
> -:Type: vcpu ioctl
> +:Type: vm ioctl, vcpu ioctl
> :Parameters: struct kvm_s390_mem_op (in)
> :Returns: = 0 on success,
> < 0 on generic error (e.g. -EFAULT or -ENOMEM),
> > 0 if an exception occurred while walking the page tables
>
> -Read or write data from/to the logical (virtual) memory of a VCPU.
> +Read or write data from/to the VM's memory.
> +The KVM_CAP_S390_MEM_OP_EXTENSION capability specifies what functionality is
> +supported.
>
> Parameters are specified via the following structure::
>
> @@ -3701,33 +3703,99 @@ Parameters are specified via the following structure::
> __u32 size; /* amount of bytes */
> __u32 op; /* type of operation */
> __u64 buf; /* buffer in userspace */
> - __u8 ar; /* the access register number */
> - __u8 reserved[31]; /* should be set to 0 */
> + union {
> + struct {
> + __u8 ar; /* the access register number */
> + __u8 key; /* access key, ignored if flag unset */
> + };
> + __u32 sida_offset; /* offset into the sida */
> + __u8 reserved[32]; /* ignored */
> + };
> };
>
> -The type of operation is specified in the "op" field. It is either
> -KVM_S390_MEMOP_LOGICAL_READ for reading from logical memory space or
> -KVM_S390_MEMOP_LOGICAL_WRITE for writing to logical memory space. The
> -KVM_S390_MEMOP_F_CHECK_ONLY flag can be set in the "flags" field to check
> -whether the corresponding memory access would create an access exception
> -(without touching the data in the memory at the destination). In case an
> -access exception occurred while walking the MMU tables of the guest, the
> -ioctl returns a positive error number to indicate the type of exception.
> -This exception is also raised directly at the corresponding VCPU if the
> -flag KVM_S390_MEMOP_F_INJECT_EXCEPTION is set in the "flags" field.
> -
> The start address of the memory region has to be specified in the "gaddr"
> field, and the length of the region in the "size" field (which must not
> be 0). The maximum value for "size" can be obtained by checking the
> KVM_CAP_S390_MEM_OP capability. "buf" is the buffer supplied by the
> userspace application where the read data should be written to for
> -KVM_S390_MEMOP_LOGICAL_READ, or where the data that should be written is
> -stored for a KVM_S390_MEMOP_LOGICAL_WRITE. When KVM_S390_MEMOP_F_CHECK_ONLY
> -is specified, "buf" is unused and can be NULL. "ar" designates the access
> -register number to be used; the valid range is 0..15.
> +a read access, or where the data that should be written is stored for
> +a write access. The "reserved" field is meant for future extensions.
> +Reserved and unused values are ignored. Future extension that add members must
> +introduce new flags.
> +
> +The type of operation is specified in the "op" field. Flags modifying
> +their behavior can be set in the "flags" field. Undefined flag bits must
> +be set to 0.
> +
> +Possible operations are:
> + * ``KVM_S390_MEMOP_LOGICAL_READ``
> + * ``KVM_S390_MEMOP_LOGICAL_WRITE``
> + * ``KVM_S390_MEMOP_ABSOLUTE_READ``
> + * ``KVM_S390_MEMOP_ABSOLUTE_WRITE``
> + * ``KVM_S390_MEMOP_SIDA_READ``
> + * ``KVM_S390_MEMOP_SIDA_WRITE``
> +
> +Logical read/write:
> +^^^^^^^^^^^^^^^^^^^
> +
> +Access logical memory, i.e. translate the given guest address to an absolute
> +address given the state of the VCPU and use the absolute address as target of
> +the access. "ar" designates the access register number to be used; the valid
> +range is 0..15.
> +Logical accesses are permitted for the VCPU ioctl only.
> +Logical accesses are permitted for non secure guests only.
s/secure/protected/
> +
> +Supported flags:
> + * ``KVM_S390_MEMOP_F_CHECK_ONLY``
> + * ``KVM_S390_MEMOP_F_INJECT_EXCEPTION``
> + * ``KVM_S390_MEMOP_F_SKEY_PROTECTION``
> +
> +The KVM_S390_MEMOP_F_CHECK_ONLY flag can be set to check whether the
> +corresponding memory access would cause an access exception, without touching
I think the comma needs to be removed.
> +the data in memory at the destination.
> +In this case, "buf" is unused and can be NULL.
> +
> +In case an access exception occurred during the access (or would occur
> +in case of KVM_S390_MEMOP_F_CHECK_ONLY), the ioctl returns a positive
> +error number indicating the type of exception. This exception is also
> +raised directly at the corresponding VCPU if the flag
> +KVM_S390_MEMOP_F_INJECT_EXCEPTION is set.
> +
> +If the KVM_S390_MEMOP_F_SKEY_PROTECTION flag is set, storage key
> +protection is also in effect and may cause exceptions if accesses are
> +prohibited given the access key passed in "key".
> +KVM_S390_MEMOP_F_SKEY_PROTECTION is available if KVM_CAP_S390_MEM_OP_EXTENSION
> +is > 0.
> +
> +Absolute read/write:
> +^^^^^^^^^^^^^^^^^^^^
> +
> +Access absolute memory. This operation is intended to be used with the
> +KVM_S390_MEMOP_F_SKEY_PROTECTION flag, to allow accessing memory and performing
> +the checks required for storage key protection as one operation (as opposed to
> +user space getting the storage keys, performing the checks, and accessing
> +memory thereafter, which could lead to a delay between check and access).
> +Absolute accesses are permitted for the VM ioctl if KVM_CAP_S390_MEM_OP_EXTENSION
> +is > 0.
> +Currently absolute accesses are not permitted for VCPU ioctls.
> +Absolute accesses are permitted for non secure guests only.
s/secure/protected/
> +
> +Supported flags:
> + * ``KVM_S390_MEMOP_F_CHECK_ONLY``
> + * ``KVM_S390_MEMOP_F_SKEY_PROTECTION``
> +
> +The semantics of the flags are as for logical accesses.
> +
> +SIDA read/write:
> +^^^^^^^^^^^^^^^^
> +
> +Access the secure instruction data area which contains memory operands necessary
> +for instruction emulation for secure guests.
> +SIDA accesses are available if the KVM_CAP_S390_PROTECTED capability is available.
> +SIDA accesses are permitted for the VCPU ioctl only.
> +SIDA accesses are permitted for secure guests only.
s/secure/protected/
>
> -The "reserved" field is meant for future extensions. It is not used by
> -KVM with the currently defined set of flags.
> +No flags are supported.
>
> 4.90 KVM_S390_GET_SKEYS
> -----------------------
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 26bff414f1a0..fd01fe04a183 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -567,7 +567,7 @@ struct kvm_s390_mem_op {
> __u8 key; /* access key, ignored if flag unset */
> };
> __u32 sida_offset; /* offset into the sida */
> - __u8 reserved[32]; /* should be set to 0 */
> + __u8 reserved[32]; /* ignored */
> };
> };
> /* types for kvm_s390_mem_op->op */
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2022-02-11 9:17 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09 17:04 [PATCH v3 00/10] KVM: s390: Do storage key checking Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 01/10] s390/uaccess: Add copy_from/to_user_key functions Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 02/10] KVM: s390: Honor storage keys when accessing guest memory Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 03/10] KVM: s390: handle_tprot: Honor storage keys Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 04/10] KVM: s390: selftests: Test TEST PROTECTION emulation Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 05/10] KVM: s390: Add optional storage key checking to MEMOP IOCTL Janis Schoetterl-Glausch
2022-02-10 9:29 ` Christian Borntraeger
2022-02-10 9:40 ` Janis Schoetterl-Glausch
2022-02-10 9:43 ` Christian Borntraeger
2022-02-11 8:53 ` Janosch Frank
2022-02-10 11:59 ` Janis Schoetterl-Glausch
2022-02-10 16:33 ` Claudio Imbrenda
2022-02-09 17:04 ` [PATCH v3 06/10] KVM: s390: Add vm IOCTL for key checked guest absolute memory access Janis Schoetterl-Glausch
2022-02-10 14:40 ` Christian Borntraeger
2022-02-10 16:25 ` Claudio Imbrenda
2022-02-09 17:04 ` [PATCH v3 07/10] KVM: s390: Rename existing vcpu memop functions Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 08/10] KVM: s390: Add capability for storage key extension of MEM_OP IOCTL Janis Schoetterl-Glausch
2022-02-09 17:04 ` [PATCH v3 09/10] KVM: s390: Update api documentation for memop ioctl Janis Schoetterl-Glausch
2022-02-10 14:41 ` Christian Borntraeger
2022-02-11 9:17 ` Janosch Frank
2022-02-09 17:04 ` [PATCH v3 10/10] KVM: s390: selftests: Test memops with storage keys Janis Schoetterl-Glausch
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).