[PATCH 2/3] KVM: handle vcpu ioctls KVM_(GET|SET)_ONE_REG in a generic function

[<darkhan@amazon.com>]

From: Darkhan Mukashov <darkhan@amazon.com>

KVM APIs KVM_GET_ONE_REG and KVM_SET_ONE_REG are handled in arch specific
kvm_arch_vcpu_ioctl functions. These ioctls are supported by all archs
except x86. Implementations for arm64, mips, powerpc, s390 are almost the
same: copy_from_user a kvm_one_reg and pass it to arch specific get/set
function. Therefore, KVM_(GET|SET)_ONE_REG can be handled in a generic
kvm_vcpu_ioctl function. On top of that, this is the first step towards
implementating new vcpu ioctls KVM_GET_MANY_REGS & KVM_SET_MANY_REGS that
can be used to get/set arbitrary number of registers at one ioctl call.
As new ioctls will rely on KVM_(GET|SET)_ONE_REG, this refactoring is
needed for their implementation.

Although implementations of KVM_(GET|SET)_ONE_REG in arch specific
kvm_arch_vcpu_ioctl functions are similar, there are some differences.
These differences stem from differences in implementations of
kvm_arch_vcpu_ioctl functions. For example, vcpu_load and vcpu_put
functions are called in kvm_arch_vcpu_ioctl functions of mips and s390,
but not in arm64 and ppc. In arm64 and s390, there are arch specific
conditions that have to checked before register get/set. All arch specific
checks and operations are summarized in Table 1 below.
+-------+------------------------------+-----------------------+
|  arch |    before register access    | after register access |
+-------+------------------------------+-----------------------+
| arm64 |     kvm_vcpu_initialized     |                       |
+-------+------------------------------+-----------------------+
|  mips |           vcpu_load          |        vcpu_put       |
+-------+------------------------------+-----------------------+
|  ppc  |                              |                       |
+-------+------------------------------+-----------------------+
|  s390 |           vcpu_load          |        vcpu_put       |
|       | kvm_s390_pv_cpu_is_protected |                       |
+-------+------------------------------+-----------------------+
|  x86  |                              |                       |
+-------+------------------------------+-----------------------+
Table 1. Arch specific checks/operations before/after reg get/set

These differences have to be taken into account while KVM_(GET|SET)_ONE_REG
are moved into generic kvm_vcpu_ioctl function. New arch specific functions
kvm_arch_before_reg_access and kvm_arch_after_reg_access are introduced to
handle this issue. They are called before and after register get/set as
their names suggest. These functions have generic "empty" implementations,
but can be overridden by #defining __KVM_HAVE_ARCH_BEFORE_REG_ACCESS and
__KVM_HAVE_ARCH_AFTER_REG_ACCESS and implementing respective functions.

Signed-off-by: Darkhan Mukashov <darkhan@amazon.com>
---
 arch/arm64/include/asm/kvm_host.h  |  5 ++--
 arch/arm64/kvm/arm.c               | 25 ++++++--------------
 arch/arm64/kvm/guest.c             |  6 +++--
 arch/mips/include/asm/kvm_host.h   |  6 +++++
 arch/mips/kvm/mips.c               | 32 ++++++++++++-------------
 arch/powerpc/include/asm/kvm_ppc.h |  2 --
 arch/powerpc/kvm/powerpc.c         | 20 ++++------------
 arch/s390/include/asm/kvm_host.h   |  6 +++++
 arch/s390/kvm/kvm-s390.c           | 38 +++++++++++++++---------------
 arch/x86/kvm/x86.c                 | 12 ++++++++++
 include/linux/kvm_host.h           | 18 ++++++++++++++
 virt/kvm/kvm_main.c                | 20 ++++++++++++++++
 12 files changed, 114 insertions(+), 76 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 0aecbab6a7fb..090488079300 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -468,8 +468,6 @@ struct kvm_vcpu_stat {
 int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init);
 unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
 int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
-int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
-int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
 int __kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
 			      struct kvm_vcpu_events *events);
 
@@ -639,6 +637,9 @@ void kvm_vcpu_put_sysregs_vhe(struct kvm_vcpu *vcpu);
 
 int kvm_set_ipa_limit(void);
 
+#define __KVM_HAVE_ARCH_BEFORE_REG_ACCESS
+int kvm_arch_before_reg_access(struct kvm_vcpu *vcpu);
+
 #define __KVM_HAVE_ARCH_VM_ALLOC
 struct kvm *kvm_arch_alloc_vm(void);
 void kvm_arch_free_vm(struct kvm *kvm);
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index f56122eedffc..2f9c0a2000bd 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -633,6 +633,13 @@ static void check_vcpu_requests(struct kvm_vcpu *vcpu)
 	}
 }
 
+int kvm_arch_before_reg_access(struct kvm_vcpu *vcpu)
+{
+	if (unlikely(!kvm_vcpu_initialized(vcpu)))
+		return -ENOEXEC;
+	return 0;
+}
+
 /**
  * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
  * @vcpu:	The VCPU pointer
@@ -1089,24 +1096,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = kvm_arch_vcpu_ioctl_vcpu_init(vcpu, &init);
 		break;
 	}
-	case KVM_SET_ONE_REG:
-	case KVM_GET_ONE_REG: {
-		struct kvm_one_reg reg;
-
-		r = -ENOEXEC;
-		if (unlikely(!kvm_vcpu_initialized(vcpu)))
-			break;
-
-		r = -EFAULT;
-		if (copy_from_user(&reg, argp, sizeof(reg)))
-			break;
-
-		if (ioctl == KVM_SET_ONE_REG)
-			r = kvm_arm_set_reg(vcpu, &reg);
-		else
-			r = kvm_arm_get_reg(vcpu, &reg);
-		break;
-	}
 	case KVM_GET_REG_LIST: {
 		struct kvm_reg_list __user *user_list = argp;
 		struct kvm_reg_list reg_list;
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index dfb5218137ca..3eb0457fb139 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -710,7 +710,8 @@ int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
 	return kvm_arm_copy_sys_reg_indices(vcpu, uindices);
 }
 
-int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
+int kvm_arch_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
+				    struct kvm_one_reg *reg)
 {
 	/* We currently use nothing arch-specific in upper 32 bits */
 	if ((reg->id & ~KVM_REG_SIZE_MASK) >> 32 != KVM_REG_ARM64 >> 32)
@@ -728,7 +729,8 @@ int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
 	return kvm_arm_sys_reg_get_reg(vcpu, reg);
 }
 
-int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
+int kvm_arch_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
+				    struct kvm_one_reg *reg)
 {
 	/* We currently use nothing arch-specific in upper 32 bits */
 	if ((reg->id & ~KVM_REG_SIZE_MASK) >> 32 != KVM_REG_ARM64 >> 32)
diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 24f3d0f9996b..ed7ba09dfd57 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -700,6 +700,12 @@ static inline void kvm_change_##name1(struct mips_coproc *cop0,		\
 
 #endif
 
+#define __KVM_HAVE_ARCH_BEFORE_REG_ACCESS
+int kvm_arch_before_reg_access(struct kvm_vcpu *vcpu);
+
+#define __KVM_HAVE_ARCH_AFTER_REG_ACCESS
+void kvm_arch_after_reg_access(struct kvm_vcpu *vcpu);
+
 /*
  * Define accessors for CP0 registers that are accessible to the guest. These
  * are primarily used by common emulation code, which may need to access the
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 3d6a7f5827b1..de9985c31f5d 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -661,8 +661,19 @@ static int kvm_mips_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices)
 	return kvm_mips_callbacks->copy_reg_indices(vcpu, indices);
 }
 
-static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
-			    const struct kvm_one_reg *reg)
+int kvm_arch_before_reg_access(struct kvm_vcpu *vcpu)
+{
+	vcpu_load(vcpu);
+	return 0;
+}
+
+void kvm_arch_after_reg_access(struct kvm_vcpu *vcpu)
+{
+	vcpu_put(vcpu);
+}
+
+int kvm_arch_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
+					struct kvm_one_reg *reg)
 {
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
 	struct mips_fpu_struct *fpu = &vcpu->arch.fpu;
@@ -773,8 +784,8 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
 	}
 }
 
-static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
-			    const struct kvm_one_reg *reg)
+int kvm_arch_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
+					struct kvm_one_reg *reg)
 {
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
 	struct mips_fpu_struct *fpu = &vcpu->arch.fpu;
@@ -943,19 +954,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
 	vcpu_load(vcpu);
 
 	switch (ioctl) {
-	case KVM_SET_ONE_REG:
-	case KVM_GET_ONE_REG: {
-		struct kvm_one_reg reg;
-
-		r = -EFAULT;
-		if (copy_from_user(&reg, argp, sizeof(reg)))
-			break;
-		if (ioctl == KVM_SET_ONE_REG)
-			r = kvm_mips_set_reg(vcpu, &reg);
-		else
-			r = kvm_mips_get_reg(vcpu, &reg);
-		break;
-	}
 	case KVM_GET_REG_LIST: {
 		struct kvm_reg_list __user *user_list = argp;
 		struct kvm_reg_list reg_list;
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index 0a056c64c317..76f202bc608e 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -412,8 +412,6 @@ int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
 int kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
 int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
 
-int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg);
-int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg);
 int kvmppc_get_one_reg(struct kvm_vcpu *vcpu, u64 id, union kvmppc_one_reg *);
 int kvmppc_set_one_reg(struct kvm_vcpu *vcpu, u64 id, union kvmppc_one_reg *);
 
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 13999123b735..cd330c7d50b8 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -1673,7 +1673,8 @@ static int kvmppc_emulate_mmio_vmx_loadstore(struct kvm_vcpu *vcpu)
 }
 #endif /* CONFIG_ALTIVEC */
 
-int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
+int kvm_arch_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
+				    struct kvm_one_reg *reg)
 {
 	int r = 0;
 	union kvmppc_one_reg val;
@@ -1721,7 +1722,8 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
 	return r;
 }
 
-int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
+int kvm_arch_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
+				    struct kvm_one_reg *reg)
 {
 	int r;
 	union kvmppc_one_reg val;
@@ -2049,20 +2051,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		break;
 	}
 
-	case KVM_SET_ONE_REG:
-	case KVM_GET_ONE_REG:
-	{
-		struct kvm_one_reg reg;
-		r = -EFAULT;
-		if (copy_from_user(&reg, argp, sizeof(reg)))
-			goto out;
-		if (ioctl == KVM_SET_ONE_REG)
-			r = kvm_vcpu_ioctl_set_one_reg(vcpu, &reg);
-		else
-			r = kvm_vcpu_ioctl_get_one_reg(vcpu, &reg);
-		break;
-	}
-
 #if defined(CONFIG_KVM_E500V2) || defined(CONFIG_KVM_E500MC)
 	case KVM_DIRTY_TLB: {
 		struct kvm_dirty_tlb dirty;
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 463c24e26000..c21fb33a2dfe 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -970,6 +970,12 @@ static inline bool kvm_is_error_hva(unsigned long addr)
 	return IS_ERR_VALUE(addr);
 }
 
+#define __KVM_HAVE_ARCH_BEFORE_REG_ACCESS
+int kvm_arch_before_reg_access(struct kvm_vcpu *vcpu);
+
+#define __KVM_HAVE_ARCH_AFTER_REG_ACCESS
+void kvm_arch_after_reg_access(struct kvm_vcpu *vcpu);
+
 #define ASYNC_PF_PER_VCPU	64
 struct kvm_arch_async_pf {
 	unsigned long pfault_token;
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 6b74b92c1a58..a092f7ef91b4 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3409,8 +3409,23 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-static int kvm_arch_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
-					   struct kvm_one_reg *reg)
+int kvm_arch_before_reg_access(struct kvm_vcpu *vcpu)
+{
+	vcpu_load(vcpu);
+	if (kvm_s390_pv_cpu_is_protected(vcpu)) {
+		vcpu_put(vcpu);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+void kvm_arch_after_reg_access(struct kvm_vcpu *vcpu)
+{
+	vcpu_put(vcpu);
+}
+
+int kvm_arch_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
+				    struct kvm_one_reg *reg)
 {
 	int r = -EINVAL;
 
@@ -3458,8 +3473,8 @@ static int kvm_arch_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
 	return r;
 }
 
-static int kvm_arch_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
-					   struct kvm_one_reg *reg)
+int kvm_arch_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
+				    struct kvm_one_reg *reg)
 {
 	int r = -EINVAL;
 	__u64 val;
@@ -4834,21 +4849,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 				   rc, rrc);
 		}
 		break;
-	case KVM_SET_ONE_REG:
-	case KVM_GET_ONE_REG: {
-		struct kvm_one_reg reg;
-		r = -EINVAL;
-		if (kvm_s390_pv_cpu_is_protected(vcpu))
-			break;
-		r = -EFAULT;
-		if (copy_from_user(&reg, argp, sizeof(reg)))
-			break;
-		if (ioctl == KVM_SET_ONE_REG)
-			r = kvm_arch_vcpu_ioctl_set_one_reg(vcpu, &reg);
-		else
-			r = kvm_arch_vcpu_ioctl_get_one_reg(vcpu, &reg);
-		break;
-	}
 #ifdef CONFIG_KVM_S390_UCONTROL
 	case KVM_S390_UCAS_MAP: {
 		struct kvm_s390_ucas_mapping ucasmap;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 397f599b20e5..794e981d6136 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9279,6 +9279,18 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 	return r;
 }
 
+int kvm_arch_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
+				    struct kvm_one_reg *reg)
+{
+	return -EINVAL;
+}
+
+int kvm_arch_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
+				    struct kvm_one_reg *reg)
+{
+	return -EINVAL;
+}
+
 static void __get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
 {
 	if (vcpu->arch.emulate_regs_need_sync_to_vcpu) {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 7f2e2a09ebbd..53ca4dd1890f 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -886,6 +886,24 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 long kvm_arch_vm_ioctl(struct file *filp,
 		       unsigned int ioctl, unsigned long arg);
 
+#ifndef __KVM_HAVE_ARCH_BEFORE_REG_ACCESS
+static inline int kvm_arch_before_reg_access(struct kvm_vcpu *vcpu)
+{
+	return 0;
+}
+#endif
+
+#ifndef __KVM_HAVE_ARCH_AFTER_REG_ACCESS
+static inline void kvm_arch_after_reg_access(struct kvm_vcpu *vcpu)
+{
+}
+#endif
+
+int kvm_arch_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
+				    struct kvm_one_reg *reg);
+int kvm_arch_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
+				    struct kvm_one_reg *reg);
+
 int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu);
 int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu);
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2541a17ff1c4..e14185f4977f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3235,6 +3235,26 @@ static long kvm_vcpu_ioctl(struct file *filp,
 		trace_kvm_userspace_exit(vcpu->run->exit_reason, r);
 		break;
 	}
+	case KVM_GET_ONE_REG:
+	case KVM_SET_ONE_REG: {
+		struct kvm_one_reg reg;
+
+		r = kvm_arch_before_reg_access(vcpu);
+		if (r < 0)
+			break;
+
+		r = -EFAULT;
+		if (copy_from_user(&reg, argp, sizeof(reg)))
+			goto out_after_reg_access;
+
+		if (ioctl == KVM_GET_ONE_REG)
+			r = kvm_arch_vcpu_ioctl_get_one_reg(vcpu, &reg);
+		else
+			r = kvm_arch_vcpu_ioctl_set_one_reg(vcpu, &reg);
+out_after_reg_access:
+		kvm_arch_after_reg_access(vcpu);
+		break;
+	}
 	case KVM_GET_REGS: {
 		struct kvm_regs *kvm_regs;
 
-- 
2.17.1