Re: [RFC PATCH v1 05/10] KVM: s390: Add optional storage key checking to MEMOP IOCTL

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





Am 18.01.22 um 10:52 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 via the flags field.
As reserved flags need to be 0, and key 0 matches all storage keys,
by default no key checking is performed, as before.
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@xxxxxxxxxxxxx>
Acked-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
Reviewed-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
---
  arch/s390/kvm/kvm-s390.c | 21 ++++++++++++++-------
  include/uapi/linux/kvm.h |  1 +
  2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 38b304e81c57..c4acdb025ff1 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>
@@ -4727,9 +4728,11 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
  {
  	void __user *uaddr = (void __user *)mop->buf;
  	void *tmpbuf = NULL;
+	char access_key = 0;
  	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_SKEYS_ACC;

I think it would be better to just have a flag here (single bit) to check the key and
then embed the key value in the payload.

        union {
                __u8 ar;        /* the access register number */
                __u32 sida_offset; /* offset into the sida */
                __u8 reserved[32]; /* should be set to 0 */
        };

There are cases when we need both, AR and key so maybe an unname struct is the easiest.

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 1daa45268de2..e8fa1f82d472 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;       /* effective storage key */
+               };
                __u32 sida_offset; /* offset into the sida */
                __u8 reserved[32]; /* should be set to 0 */
        };

(or acc instead of key)


if (mop->flags & ~supported_flags || mop->ar >= NUM_ACRS || !mop->size)
  		return -EINVAL;
@@ -4746,14 +4749,17 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
  			return -ENOMEM;
  	}
+ access_key = FIELD_GET(KVM_S390_MEMOP_F_SKEYS_ACC, mop->flags);
+
  	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, access_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, access_key);
  		if (r == 0) {
  			if (copy_to_user(uaddr, tmpbuf, mop->size))
  				r = -EFAULT;
@@ -4761,15 +4767,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, access_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, access_key);
  		break;
  	}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 1daa45268de2..e3f450b2f346 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -575,6 +575,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_SKEYS_ACC		0x0f00ULL
/* for KVM_INTERRUPT */
  struct kvm_interrupt {



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux