[PATCH 0/3] Introduce new vcpu ioctls KVM_(GET|SET)_MANY_REGS

[PATCH 0/3] Introduce new vcpu ioctls KVM_(GET|SET)_MANY_REGS

[darkhan]

From: Darkhan Mukashov <darkhan@amazon.com>

The ultimate goal is to introduce new vcpu ioctls KVM_(GET|SET)_MANY_REGS.
To introduce these ioctls, implementations of KVM_(GET|SET)_ONE_REG have
to be refactored. Specifically, KVM_(GET|SET)_ONE_REG should be handled in
a generic kvm_vcpu_ioctl function.

New KVM APIs KVM_(GET|SET)_MANY_REGS make it possible to bulk read/write
vCPU registers at one ioctl call. These ioctls can be very useful when
vCPU state serialization/deserialization is required (e.g. live update of
kvm, live migration of guests), hence all registers have to be
saved/restored. KVM_(GET|SET)_MANY_REGS will help avoid performance
overhead associated with syscall (ioctl in our case) handling. Tests
conducted on AWS Graviton2 Processors (64-bit ARM Neoverse cores) show
that average save/restore time of all vCPU registers can be optimized
~3.5 times per vCPU with new ioctls. Test results can be found in Table 1.
+---------+-------------+---------------+
|         | kvm_one_reg | kvm_many_regs |
+---------+-------------+---------------+
| get all |   123 usec  |    33 usec    |
+---------+-------------+---------------+
| set all |   120 usec  |    36 usec    |
+---------+-------------+---------------+
	Table 1. Test results

The patches are based out of kvm/queue.

Darkhan Mukashov (3):
  Documentation: KVM: change description of vcpu ioctls
    KVM_(GET|SET)_ONE_REG
  KVM: handle vcpu ioctls KVM_(GET|SET)_ONE_REG in a generic function
  KVM: introduce new vcpu ioctls KVM_GET_MANY_REGS and KVM_SET_MANY_REGS

 Documentation/virt/kvm/api.rst     | 80 ++++++++++++++++++++++++++++--
 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 +++++++
 include/uapi/linux/kvm.h           | 11 ++++
 virt/kvm/kvm_main.c                | 62 +++++++++++++++++++++++
 14 files changed, 244 insertions(+), 79 deletions(-)

-- 
2.17.1

[PATCH 1/3] Documentation: KVM: change description of vcpu ioctls KVM_(GET|SET)_ONE_REG

[darkhan]

From: Darkhan Mukashov <darkhan@amazon.com>

KVM APIs KVM_GET_ONE_REG and KVM_SET_ONE_REG are not implemented
in x86. They are handled in architecture specific "kvm_arch_vcpu_ioctl"
functions. There are no handlers for KVM_GET_ONE_REG and KVM_SET_ONE_REG
in "kvm_arch_vcpu_ioctl" in x86. -EINVAL is returned when both are called.
Therefore, architectures supported by KVM_(GET|SET)_ONE_REG should be
"all except x86" rather than "all".

KVM_GET_ONE_REG accepts a struct kvm_one_reg and writes value of a register
indicated by 'id' field of the struct to the memory location pointed by
'addr' field of the struct. As nothing is written to the struct
kvm_one_reg, parameter type should be "in" rather than "in/out".

Signed-off-by: Darkhan Mukashov <darkhan@amazon.com>
---
 Documentation/virt/kvm/api.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 76317221d29f..6d6135c15729 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -2105,7 +2105,7 @@ prior to calling the KVM_RUN ioctl.
 --------------------
 
 :Capability: KVM_CAP_ONE_REG
-:Architectures: all
+:Architectures: all except x86
 :Type: vcpu ioctl
 :Parameters: struct kvm_one_reg (in)
 :Returns: 0 on success, negative value on failure
@@ -2544,9 +2544,9 @@ following id bit patterns::
 --------------------
 
 :Capability: KVM_CAP_ONE_REG
-:Architectures: all
+:Architectures: all except x86
 :Type: vcpu ioctl
-:Parameters: struct kvm_one_reg (in and out)
+:Parameters: struct kvm_one_reg (in)
 :Returns: 0 on success, negative value on failure
 
 Errors include:
-- 
2.17.1

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

[darkhan]

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

[PATCH 3/3] KVM: introduce new vcpu ioctls KVM_GET_MANY_REGS and KVM_SET_MANY_REGS

[darkhan]

From: Darkhan Mukashov <darkhan@amazon.com>

In order to read/write arbitrary number of registers, one has to call
KVM_(GET|SET)_ONE_REG ioctls multiple times. New KVM APIs
KVM_(GET|SET)_MANY_REGS make it possible to bulk read/write vCPU registers
at one ioctl call. These ioctls can be very useful when vCPU state
serialization/deserialization is required (e.g. live update of kvm, live
migration of guests), hence all registers have to be saved/restored.
KVM_(GET|SET)_MANY_REGS will help avoid performance overhead associated
with syscall (ioctl in our case) handling. Tests conducted on AWS Graviton2
Processors (64-bit ARM Neoverse cores) show that average save/restore time
of all vCPU registers can be optimized ~3.5 times per vCPU with new ioctls.
Test results can be found in Table 1.
+---------+-------------+---------------+
|         | kvm_one_reg | kvm_many_regs |
+---------+-------------+---------------+
| get all |   123 usec  |    33 usec    |
+---------+-------------+---------------+
| set all |   120 usec  |    36 usec    |
+---------+-------------+---------------+
	Table 1. Test results

KVM_(GET|SET)_MANY_REGS accept one argument
struct kvm_many_regs {
	/*
	 * number of regs to be got/set
	 */
	__u64 n;

	/*
	 * the same struct used by KVM_(GET|SET)_ONE_REG
	 */
	struct kvm_one_reg reg[0];
}

New ioctls allow to get/set values of multiple registers implemented in a
vcpu. Registers to get/set are indicated by the 'id' field of each
struct kvm_one_reg in the passed array. On success, KVM_GET_MANY_REGS
writes values of indicated registers to memory locations pointed by
the 'addr' field of each kvm_one_reg, KVM_SET_MANY_REGS changes values
of registers to values located at memory locations pointed by the 'addr'
field of each kvm_one_reg.

KVM_(GET|SET)_MANY_REGS rely on KVM_(GET|SET)_ONE_REG, which are not
implemented in x86. Therefore, new ioctls support all architectures except
x86.

Signed-off-by: Darkhan Mukashov <darkhan@amazon.com>
---
 Documentation/virt/kvm/api.rst | 74 ++++++++++++++++++++++++++++++++++
 include/uapi/linux/kvm.h       | 11 +++++
 virt/kvm/kvm_main.c            | 42 +++++++++++++++++++
 3 files changed, 127 insertions(+)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 6d6135c15729..9a229f7e182e 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -4808,6 +4808,80 @@ If a vCPU is in running state while this ioctl is invoked, the vCPU may
 experience inconsistent filtering behavior on MSR accesses.
 
 
+4.127 KVM_GET_MANY_REGS
+-----------------------
+
+:Capability: basic
+:Architectures: all except x86
+:Type: vcpu ioctl
+:Parameters: struct kvm_many_regs (in)
+:Returns: 0 on success, negative value on failure
+
+Errors:
+
+  ======   ============================================================
+  E2BIG    the passed array of kvm_one_regs is too big, its size should
+           not exceed KVM_MANY_REGS_MAX_SIZE
+  ======   ============================================================
+
+As this ioctl uses KVM_GET_ONE_REG, errors from KVM_GET_ONE_REG are propagated.
+
+(These error codes are indicative only: do not rely on a specific error
+code being returned in a specific situation.)
+
+::
+
+  struct kvm_many_regs {
+	  __u64 n;
+	  struct kvm_one_reg reg[0];
+  };
+
+This ioctl allows to receive values of multiple registers implemented
+in a vcpu. Registers to read are indicated by the 'id' field of each
+kvm_one_reg struct in the passed array. On success, values of indicated
+registers are written to memory locations pointed by the 'addr' field
+of each kvm_one_reg.
+
+In case of duplicate IDs in the 'reg' array, the register will be read as many
+times as it appears in the array.
+
+The list of registers accessible using this interface is identical to the
+list in 4.68.
+
+4.128 KVM_SET_MANY_REGS
+-----------------------
+
+:Capability: basic
+:Architectures: all except x86
+:Type: vcpu ioctl
+:Parameters: struct kvm_many_regs (in)
+:Returns: 0 on success, negative value on failure
+
+Errors:
+
+  ======   ============================================================
+  E2BIG    the passed array of kvm_one_regs is too big, its size should
+           not exceed KVM_MANY_REGS_MAX_SIZE
+  ======   ============================================================
+
+As this ioctl uses KVM_SET_ONE_REG, errors from KVM_SET_ONE_REG are propagated.
+
+(These error codes are indicative only: do not rely on a specific error
+code being returned in a specific situation.)
+
+This ioctl allows to set values of multiple registers implemented
+in a vcpu. Registers to set are indicated by the 'id' field of each
+kvm_one_reg struct in the passed array. On success, values of registers
+are changed to values located at memory locations pointed by the 'addr'
+field of each kvm_one_reg.
+
+In case of duplicate IDs in the 'reg' array, the register will be set as many
+times as it appears in the array.
+
+The list of registers accessible using this interface is identical to the
+list in 4.68.
+
+
 5. The kvm_run structure
 ========================
 
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index ca41220b40b8..87e799457fc3 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1223,6 +1223,14 @@ struct kvm_one_reg {
 	__u64 addr;
 };
 
+/* For KVM_(GET|SET)_MANY_REGS */
+#define KVM_MANY_REGS_MAX_SIZE	16384
+
+struct kvm_many_regs {
+	__u64 n;
+	struct kvm_one_reg reg[0];
+};
+
 #define KVM_MSI_VALID_DEVID	(1U << 0)
 struct kvm_msi {
 	__u32 address_lo;
@@ -1557,6 +1565,9 @@ struct kvm_pv_cmd {
 /* Available with KVM_CAP_X86_MSR_FILTER */
 #define KVM_X86_SET_MSR_FILTER	_IOW(KVMIO,  0xc6, struct kvm_msr_filter)
 
+#define KVM_GET_MANY_REGS            _IOW(KVMIO, 0xc7, struct kvm_many_regs)
+#define KVM_SET_MANY_REGS            _IOW(KVMIO, 0xc8, struct kvm_many_regs)
+
 /* Secure Encrypted Virtualization command */
 enum sev_cmd_id {
 	/* Guest initialization commands */
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index e14185f4977f..3bb618882d2c 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3255,6 +3255,48 @@ static long kvm_vcpu_ioctl(struct file *filp,
 		kvm_arch_after_reg_access(vcpu);
 		break;
 	}
+	case KVM_SET_MANY_REGS:
+	case KVM_GET_MANY_REGS: {
+		struct kvm_many_regs __user *user_regs = argp;
+		struct kvm_many_regs many_regs;
+		struct kvm_one_reg *reg;
+		unsigned int i, size;
+
+		r = kvm_arch_before_reg_access(vcpu);
+		if (r < 0)
+			break;
+
+		r = -EFAULT;
+		if (copy_from_user(&many_regs, user_regs, sizeof(many_regs)))
+			goto out_after_regs_access;
+
+		r = -E2BIG;
+		size = sizeof(struct kvm_one_reg) * many_regs.n;
+		if (size > KVM_MANY_REGS_MAX_SIZE)
+			goto out_after_regs_access;
+
+		r = -ENOMEM;
+		reg = (struct kvm_one_reg *)memdup_user(user_regs->reg, size);
+		if (IS_ERR(reg))
+			goto out_after_regs_access;
+
+		if (ioctl == KVM_GET_MANY_REGS)
+			for (i = 0; i < many_regs.n; i++) {
+				r = kvm_arch_vcpu_ioctl_get_one_reg(vcpu, &reg[i]);
+				if (r < 0)
+					break;
+			}
+		else
+			for (i = 0; i < many_regs.n; i++) {
+				r = kvm_arch_vcpu_ioctl_set_one_reg(vcpu, &reg[i]);
+				if (r < 0)
+					break;
+			}
+		kfree(reg);
+out_after_regs_access:
+		kvm_arch_after_reg_access(vcpu);
+		break;
+	}
 	case KVM_GET_REGS: {
 		struct kvm_regs *kvm_regs;
 
-- 
2.17.1

Re: [PATCH 0/3] Introduce new vcpu ioctls KVM_(GET|SET)_MANY_REGS

[Marc Zyngier]

On 2020-11-20 12:56, darkhan@amazon.com wrote:
> From: Darkhan Mukashov <darkhan@amazon.com>
> 
> The ultimate goal is to introduce new vcpu ioctls 
> KVM_(GET|SET)_MANY_REGS.
> To introduce these ioctls, implementations of KVM_(GET|SET)_ONE_REG 
> have
> to be refactored. Specifically, KVM_(GET|SET)_ONE_REG should be handled 
> in
> a generic kvm_vcpu_ioctl function.
> 
> New KVM APIs KVM_(GET|SET)_MANY_REGS make it possible to bulk 
> read/write
> vCPU registers at one ioctl call. These ioctls can be very useful when
> vCPU state serialization/deserialization is required (e.g. live update 
> of
> kvm, live migration of guests), hence all registers have to be
> saved/restored. KVM_(GET|SET)_MANY_REGS will help avoid performance
> overhead associated with syscall (ioctl in our case) handling. Tests
> conducted on AWS Graviton2 Processors (64-bit ARM Neoverse cores) show
> that average save/restore time of all vCPU registers can be optimized
> ~3.5 times per vCPU with new ioctls. Test results can be found in Table 
> 1.
> +---------+-------------+---------------+
> |         | kvm_one_reg | kvm_many_regs |
> +---------+-------------+---------------+
> | get all |   123 usec  |    33 usec    |
> +---------+-------------+---------------+
> | set all |   120 usec  |    36 usec    |
> +---------+-------------+---------------+
> 	Table 1. Test results

I have asked in private last week, and didn't get a satisfying answer:

We are talking about 90us over the time taken by enough state to be
transferred to the target so that it can be restarted. What proportion
does this represent? I'd expect userspace to do this from the vcpu
thread, and thus to be able to do everything in parallel, in which case
the gain doesn't scale with the number of vcpu.

One of the reasons for me being reluctant is that the userspace API
breaks extremely often, and that you are now adding yet another one
that can be used concurrently with the existing one.

So I would really like to see the whole picture, and not just this
very narrow "make it faster" approach. I also want to understand
why this is a "MANY" approach, and not "ALL".

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...