LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v1 0/2] support to set VSESR_EL2 by user space
@ 2018-05-31 13:08 Dongjiu Geng
  2018-05-31 13:08 ` [PATCH v1 1/2] arm64: KVM: export the capability to set guest SError syndrome Dongjiu Geng
  2018-05-31 13:08 ` [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS Dongjiu Geng
  0 siblings, 2 replies; 9+ messages in thread
From: Dongjiu Geng @ 2018-05-31 13:08 UTC (permalink / raw)
  To: rkrcmar, corbet, christoffer.dall, marc.zyngier, linux,
	catalin.marinas, will.deacon, kvm, linux-doc, james.morse,
	gengdongjiu, linux-arm-kernel, linux-kernel, linux-acpi

This series patch is separated from https://www.spinics.net/lists/kvm/msg168917.html

1. Detect whether KVM can set set guest SError syndrome
2. Support to Set VSESR_EL2 and inject SError by user space.
3. Support live migration to keep SError pending state and VSESR_EL2 value

The user space patch is here: https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06965.html

Dongjiu Geng (2):
  arm64: KVM: export the capability to set guest SError syndrome
  arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS

 Documentation/virtual/kvm/api.txt    | 42 +++++++++++++++++++++++++++++++++---
 arch/arm/include/asm/kvm_host.h      |  6 ++++++
 arch/arm/kvm/guest.c                 | 12 +++++++++++
 arch/arm64/include/asm/kvm_emulate.h |  5 +++++
 arch/arm64/include/asm/kvm_host.h    |  7 ++++++
 arch/arm64/include/uapi/asm/kvm.h    | 13 +++++++++++
 arch/arm64/kvm/guest.c               | 36 +++++++++++++++++++++++++++++++
 arch/arm64/kvm/inject_fault.c        |  7 +++++-
 arch/arm64/kvm/reset.c               |  4 ++++
 include/uapi/linux/kvm.h             |  1 +
 virt/kvm/arm/arm.c                   | 21 ++++++++++++++++++
 11 files changed, 150 insertions(+), 4 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 1/2] arm64: KVM: export the capability to set guest SError syndrome
  2018-05-31 13:08 [PATCH v1 0/2] support to set VSESR_EL2 by user space Dongjiu Geng
@ 2018-05-31 13:08 ` Dongjiu Geng
  2018-05-31 13:08 ` [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS Dongjiu Geng
  1 sibling, 0 replies; 9+ messages in thread
From: Dongjiu Geng @ 2018-05-31 13:08 UTC (permalink / raw)
  To: rkrcmar, corbet, christoffer.dall, marc.zyngier, linux,
	catalin.marinas, will.deacon, kvm, linux-doc, james.morse,
	gengdongjiu, linux-arm-kernel, linux-kernel, linux-acpi

For the arm64 RAS Extension, user space can inject a virtual-SError
with specified ESR. So user space needs to know whether KVM support
to inject such SError, this interface adds this query for this capability.

KVM will check whether system support RAS Extension, if supported, KVM
returns true to user space, otherwise returns false.

Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
Reviewed-by: James Morse <james.morse@arm.com>
---
 Documentation/virtual/kvm/api.txt | 11 +++++++++++
 arch/arm64/kvm/reset.c            |  3 +++
 include/uapi/linux/kvm.h          |  1 +
 3 files changed, 15 insertions(+)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 758bf40..fdac969 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -4603,3 +4603,14 @@ Architectures: s390
 This capability indicates that kvm will implement the interfaces to handle
 reset, migration and nested KVM for branch prediction blocking. The stfle
 facility 82 should not be provided to the guest without this capability.
+
+8.14 KVM_CAP_ARM_SET_SERROR_ESR
+
+Architectures: arm, arm64
+
+This capability indicates that userspace can specify the syndrome value reported
+to the guest OS when guest takes a virtual SError interrupt exception.
+If KVM has this capability, userspace can only specify the ISS field for the ESR
+syndrome, it can not specify the EC field which is not under control by KVM.
+If this virtual SError is taken to EL1 using AArch64, this value will be reported
+in ISS filed of ESR_EL1.
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 3256b92..38c8a64 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -77,6 +77,9 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_ARM_PMU_V3:
 		r = kvm_arm_support_pmu_v3();
 		break;
+	case KVM_CAP_ARM_INJECT_SERROR_ESR:
+		r = cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
+		break;
 	case KVM_CAP_SET_GUEST_DEBUG:
 	case KVM_CAP_VCPU_ATTRIBUTES:
 		r = 1;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index b02c41e..e88f976 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -948,6 +948,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_S390_BPB 152
 #define KVM_CAP_GET_MSR_FEATURES 153
 #define KVM_CAP_HYPERV_EVENTFD 154
+#define KVM_CAP_ARM_INJECT_SERROR_ESR 155
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
  2018-05-31 13:08 [PATCH v1 0/2] support to set VSESR_EL2 by user space Dongjiu Geng
  2018-05-31 13:08 ` [PATCH v1 1/2] arm64: KVM: export the capability to set guest SError syndrome Dongjiu Geng
@ 2018-05-31 13:08 ` Dongjiu Geng
  2018-06-01  9:17   ` Marc Zyngier
                     ` (3 more replies)
  1 sibling, 4 replies; 9+ messages in thread
From: Dongjiu Geng @ 2018-05-31 13:08 UTC (permalink / raw)
  To: rkrcmar, corbet, christoffer.dall, marc.zyngier, linux,
	catalin.marinas, will.deacon, kvm, linux-doc, james.morse,
	gengdongjiu, linux-arm-kernel, linux-kernel, linux-acpi

For the migrating VMs, user space may need to know the exception
state. For example, in the machine A, KVM make an SError pending,
when migrate to B, KVM also needs to pend an SError.

This new IOCTL exports user-invisible states related to SError.
Together with appropriate user space changes, user space can get/set
the SError exception state to do migrate/snapshot/suspend.

Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
--
this series patch is separated from https://www.spinics.net/lists/kvm/msg168917.html
change since V12:
1. change (vcpu->arch.hcr_el2 & HCR_VSE) to !!(vcpu->arch.hcr_el2 & HCR_VSE) in kvm_arm_vcpu_get_events()

Change since V11:
Address James's comments, thanks James
1. Align the struct of kvm_vcpu_events to 64 bytes
2. Avoid exposing the stale ESR value in the kvm_arm_vcpu_get_events()
3. Change variables 'injected' name to 'serror_pending' in the kvm_arm_vcpu_set_events()
4. Change to sizeof(events) from sizeof(struct kvm_vcpu_events) in kvm_arch_vcpu_ioctl()

Change since V10:
Address James's comments, thanks James
1. Merge the helper function with the user.
2. Move the ISS_MASK into pend_guest_serror() to clear top bits
3. Make kvm_vcpu_events struct align to 4 bytes
4. Add something check in the kvm_arm_vcpu_set_events()
5. Check kvm_arm_vcpu_get/set_events()'s return value.
6. Initialise kvm_vcpu_events to 0 so that padding transferred to user-space doesn't
   contain kernel stack.
---
 Documentation/virtual/kvm/api.txt    | 31 ++++++++++++++++++++++++++++---
 arch/arm/include/asm/kvm_host.h      |  6 ++++++
 arch/arm/kvm/guest.c                 | 12 ++++++++++++
 arch/arm64/include/asm/kvm_emulate.h |  5 +++++
 arch/arm64/include/asm/kvm_host.h    |  7 +++++++
 arch/arm64/include/uapi/asm/kvm.h    | 13 +++++++++++++
 arch/arm64/kvm/guest.c               | 36 ++++++++++++++++++++++++++++++++++++
 arch/arm64/kvm/inject_fault.c        |  7 ++++++-
 arch/arm64/kvm/reset.c               |  1 +
 virt/kvm/arm/arm.c                   | 21 +++++++++++++++++++++
 10 files changed, 135 insertions(+), 4 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index fdac969..8896737 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -835,11 +835,13 @@ struct kvm_clock_data {
 
 Capability: KVM_CAP_VCPU_EVENTS
 Extended by: KVM_CAP_INTR_SHADOW
-Architectures: x86
+Architectures: x86, arm, arm64
 Type: vm ioctl
 Parameters: struct kvm_vcpu_event (out)
 Returns: 0 on success, -1 on error
 
+X86:
+
 Gets currently pending exceptions, interrupts, and NMIs as well as related
 states of the vcpu.
 
@@ -881,15 +883,32 @@ Only two fields are defined in the flags field:
 - KVM_VCPUEVENT_VALID_SMM may be set in the flags field to signal that
   smi contains a valid state.
 
+ARM, ARM64:
+
+Gets currently pending SError exceptions as well as related states of the vcpu.
+
+struct kvm_vcpu_events {
+	struct {
+		__u8 serror_pending;
+		__u8 serror_has_esr;
+		/* Align it to 8 bytes */
+		__u8 pad[6];
+		__u64 serror_esr;
+	} exception;
+	__u32 reserved[12];
+};
+
 4.32 KVM_SET_VCPU_EVENTS
 
-Capability: KVM_CAP_VCPU_EVENTS
+Capebility: KVM_CAP_VCPU_EVENTS
 Extended by: KVM_CAP_INTR_SHADOW
-Architectures: x86
+Architectures: x86, arm, arm64
 Type: vm ioctl
 Parameters: struct kvm_vcpu_event (in)
 Returns: 0 on success, -1 on error
 
+X86:
+
 Set pending exceptions, interrupts, and NMIs as well as related states of the
 vcpu.
 
@@ -910,6 +929,12 @@ shall be written into the VCPU.
 
 KVM_VCPUEVENT_VALID_SMM can only be set if KVM_CAP_X86_SMM is available.
 
+ARM, ARM64:
+
+Set pending SError exceptions as well as related states of the vcpu.
+
+See KVM_GET_VCPU_EVENTS for the data structure.
+
 
 4.33 KVM_GET_DEBUGREGS
 
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index c7c28c8..39f9901 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -213,6 +213,12 @@ 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);
+
+int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
+			struct kvm_vcpu_events *events);
+
 unsigned long kvm_call_hyp(void *hypfn, ...);
 void force_vm_exit(const cpumask_t *mask);
 
diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
index a18f33e..c685f0e 100644
--- a/arch/arm/kvm/guest.c
+++ b/arch/arm/kvm/guest.c
@@ -261,6 +261,18 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 	return -EINVAL;
 }
 
+int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
+			struct kvm_vcpu_events *events)
+{
+	return -EINVAL;
+}
+
+int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
+			struct kvm_vcpu_events *events)
+{
+	return -EINVAL;
+}
+
 int __attribute_const__ kvm_target_cpu(void)
 {
 	switch (read_cpuid_part()) {
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 1dab3a9..18f61ff 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -81,6 +81,11 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
 	return (unsigned long *)&vcpu->arch.hcr_el2;
 }
 
+static inline unsigned long vcpu_get_vsesr(struct kvm_vcpu *vcpu)
+{
+	return vcpu->arch.vsesr_el2;
+}
+
 static inline void vcpu_set_vsesr(struct kvm_vcpu *vcpu, u64 vsesr)
 {
 	vcpu->arch.vsesr_el2 = vsesr;
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 469de8a..357304a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -335,6 +335,11 @@ 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);
+
+int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
+			struct kvm_vcpu_events *events);
 
 #define KVM_ARCH_WANT_MMU_NOTIFIER
 int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
@@ -363,6 +368,8 @@ void handle_exit_early(struct kvm_vcpu *vcpu, struct kvm_run *run,
 int kvm_perf_init(void);
 int kvm_perf_teardown(void);
 
+void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
+
 struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
 
 void __kvm_set_tpidr_el2(u64 tpidr_el2);
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 04b3256..df4faee 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -39,6 +39,7 @@
 #define __KVM_HAVE_GUEST_DEBUG
 #define __KVM_HAVE_IRQ_LINE
 #define __KVM_HAVE_READONLY_MEM
+#define __KVM_HAVE_VCPU_EVENTS
 
 #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
 
@@ -153,6 +154,18 @@ struct kvm_sync_regs {
 struct kvm_arch_memory_slot {
 };
 
+/* for KVM_GET/SET_VCPU_EVENTS */
+struct kvm_vcpu_events {
+	struct {
+		__u8 serror_pending;
+		__u8 serror_has_esr;
+		/* Align it to 8 bytes */
+		__u8 pad[6];
+		__u64 serror_esr;
+	} exception;
+	__u32 reserved[12];
+};
+
 /* If you need to interpret the index values, here is the key: */
 #define KVM_REG_ARM_COPROC_MASK		0x000000000FFF0000
 #define KVM_REG_ARM_COPROC_SHIFT	16
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 56a0260..71d3841 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -289,6 +289,42 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 	return -EINVAL;
 }
 
+int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
+			struct kvm_vcpu_events *events)
+{
+	events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
+	events->exception.serror_has_esr =
+			cpus_have_const_cap(ARM64_HAS_RAS_EXTN) &&
+					(!!vcpu_get_vsesr(vcpu));
+
+	if (events->exception.serror_pending &&
+		events->exception.serror_has_esr)
+		events->exception.serror_esr = vcpu_get_vsesr(vcpu);
+	else
+		events->exception.serror_esr = 0;
+
+	return 0;
+}
+
+int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
+			struct kvm_vcpu_events *events)
+{
+	bool serror_pending = events->exception.serror_pending;
+	bool has_esr = events->exception.serror_has_esr;
+
+	if (serror_pending && has_esr) {
+		if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
+			return -EINVAL;
+
+		kvm_set_sei_esr(vcpu, events->exception.serror_esr);
+
+	} else if (serror_pending) {
+		kvm_inject_vabt(vcpu);
+	}
+
+	return 0;
+}
+
 int __attribute_const__ kvm_target_cpu(void)
 {
 	unsigned long implementor = read_cpuid_implementor();
diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
index d8e7165..9e0ca56 100644
--- a/arch/arm64/kvm/inject_fault.c
+++ b/arch/arm64/kvm/inject_fault.c
@@ -166,7 +166,7 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
 
 static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
 {
-	vcpu_set_vsesr(vcpu, esr);
+	vcpu_set_vsesr(vcpu, esr & ESR_ELx_ISS_MASK);
 	*vcpu_hcr(vcpu) |= HCR_VSE;
 }
 
@@ -186,3 +186,8 @@ void kvm_inject_vabt(struct kvm_vcpu *vcpu)
 {
 	pend_guest_serror(vcpu, ESR_ELx_ISV);
 }
+
+void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome)
+{
+	pend_guest_serror(vcpu, syndrome);
+}
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 38c8a64..20e919a 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -82,6 +82,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
 		break;
 	case KVM_CAP_SET_GUEST_DEBUG:
 	case KVM_CAP_VCPU_ATTRIBUTES:
+	case KVM_CAP_VCPU_EVENTS:
 		r = 1;
 		break;
 	default:
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index a4c1b76..8b43968 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1107,6 +1107,27 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = kvm_arm_vcpu_has_attr(vcpu, &attr);
 		break;
 	}
+	case KVM_GET_VCPU_EVENTS: {
+		struct kvm_vcpu_events events;
+
+		memset(&events, 0, sizeof(events));
+		if (kvm_arm_vcpu_get_events(vcpu, &events))
+			return -EINVAL;
+
+		if (copy_to_user(argp, &events, sizeof(events)))
+			return -EFAULT;
+
+		return 0;
+	}
+	case KVM_SET_VCPU_EVENTS: {
+		struct kvm_vcpu_events events;
+
+		if (copy_from_user(&events, argp,
+				sizeof(struct kvm_vcpu_events)))
+			return -EFAULT;
+
+		return kvm_arm_vcpu_set_events(vcpu, &events);
+	}
 	default:
 		r = -EINVAL;
 	}
-- 
2.7.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
  2018-05-31 13:08 ` [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS Dongjiu Geng
@ 2018-06-01  9:17   ` Marc Zyngier
  2018-06-01 21:22   ` Eric Northup
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Marc Zyngier @ 2018-06-01  9:17 UTC (permalink / raw)
  To: Dongjiu Geng, rkrcmar, corbet, christoffer.dall, linux,
	catalin.marinas, will.deacon, kvm, linux-doc, james.morse,
	linux-arm-kernel, linux-kernel, linux-acpi

On 31/05/18 14:08, Dongjiu Geng wrote:
> For the migrating VMs, user space may need to know the exception
> state. For example, in the machine A, KVM make an SError pending,
> when migrate to B, KVM also needs to pend an SError.
> 
> This new IOCTL exports user-invisible states related to SError.
> Together with appropriate user space changes, user space can get/set
> the SError exception state to do migrate/snapshot/suspend.
> 
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> --
> this series patch is separated from https://www.spinics.net/lists/kvm/msg168917.html
> change since V12:
> 1. change (vcpu->arch.hcr_el2 & HCR_VSE) to !!(vcpu->arch.hcr_el2 & HCR_VSE) in kvm_arm_vcpu_get_events()
> 
> Change since V11:
> Address James's comments, thanks James
> 1. Align the struct of kvm_vcpu_events to 64 bytes
> 2. Avoid exposing the stale ESR value in the kvm_arm_vcpu_get_events()
> 3. Change variables 'injected' name to 'serror_pending' in the kvm_arm_vcpu_set_events()
> 4. Change to sizeof(events) from sizeof(struct kvm_vcpu_events) in kvm_arch_vcpu_ioctl()
> 
> Change since V10:
> Address James's comments, thanks James
> 1. Merge the helper function with the user.
> 2. Move the ISS_MASK into pend_guest_serror() to clear top bits
> 3. Make kvm_vcpu_events struct align to 4 bytes
> 4. Add something check in the kvm_arm_vcpu_set_events()
> 5. Check kvm_arm_vcpu_get/set_events()'s return value.
> 6. Initialise kvm_vcpu_events to 0 so that padding transferred to user-space doesn't
>    contain kernel stack.
> ---
>  Documentation/virtual/kvm/api.txt    | 31 ++++++++++++++++++++++++++++---
>  arch/arm/include/asm/kvm_host.h      |  6 ++++++
>  arch/arm/kvm/guest.c                 | 12 ++++++++++++
>  arch/arm64/include/asm/kvm_emulate.h |  5 +++++
>  arch/arm64/include/asm/kvm_host.h    |  7 +++++++
>  arch/arm64/include/uapi/asm/kvm.h    | 13 +++++++++++++
>  arch/arm64/kvm/guest.c               | 36 ++++++++++++++++++++++++++++++++++++
>  arch/arm64/kvm/inject_fault.c        |  7 ++++++-
>  arch/arm64/kvm/reset.c               |  1 +
>  virt/kvm/arm/arm.c                   | 21 +++++++++++++++++++++
>  10 files changed, 135 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index fdac969..8896737 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -835,11 +835,13 @@ struct kvm_clock_data {
>  
>  Capability: KVM_CAP_VCPU_EVENTS
>  Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
>  Type: vm ioctl
>  Parameters: struct kvm_vcpu_event (out)
>  Returns: 0 on success, -1 on error
>  
> +X86:
> +
>  Gets currently pending exceptions, interrupts, and NMIs as well as related
>  states of the vcpu.
>  
> @@ -881,15 +883,32 @@ Only two fields are defined in the flags field:
>  - KVM_VCPUEVENT_VALID_SMM may be set in the flags field to signal that
>    smi contains a valid state.
>  
> +ARM, ARM64:
> +
> +Gets currently pending SError exceptions as well as related states of the vcpu.
> +
> +struct kvm_vcpu_events {
> +	struct {
> +		__u8 serror_pending;
> +		__u8 serror_has_esr;
> +		/* Align it to 8 bytes */
> +		__u8 pad[6];
> +		__u64 serror_esr;
> +	} exception;
> +	__u32 reserved[12];
> +};
> +
>  4.32 KVM_SET_VCPU_EVENTS
>  
> -Capability: KVM_CAP_VCPU_EVENTS
> +Capebility: KVM_CAP_VCPU_EVENTS
>  Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
>  Type: vm ioctl
>  Parameters: struct kvm_vcpu_event (in)
>  Returns: 0 on success, -1 on error
>  
> +X86:
> +
>  Set pending exceptions, interrupts, and NMIs as well as related states of the
>  vcpu.
>  
> @@ -910,6 +929,12 @@ shall be written into the VCPU.
>  
>  KVM_VCPUEVENT_VALID_SMM can only be set if KVM_CAP_X86_SMM is available.
>  
> +ARM, ARM64:
> +
> +Set pending SError exceptions as well as related states of the vcpu.
> +
> +See KVM_GET_VCPU_EVENTS for the data structure.
> +
>  
>  4.33 KVM_GET_DEBUGREGS
>  
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index c7c28c8..39f9901 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -213,6 +213,12 @@ 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);
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events);
> +
>  unsigned long kvm_call_hyp(void *hypfn, ...);
>  void force_vm_exit(const cpumask_t *mask);
>  
> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> index a18f33e..c685f0e 100644
> --- a/arch/arm/kvm/guest.c
> +++ b/arch/arm/kvm/guest.c
> @@ -261,6 +261,18 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>  	return -EINVAL;
>  }
>  
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events)
> +{
> +	return -EINVAL;
> +}
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events)
> +{
> +	return -EINVAL;
> +}
> +
>  int __attribute_const__ kvm_target_cpu(void)
>  {
>  	switch (read_cpuid_part()) {
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 1dab3a9..18f61ff 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -81,6 +81,11 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
>  	return (unsigned long *)&vcpu->arch.hcr_el2;
>  }
>  
> +static inline unsigned long vcpu_get_vsesr(struct kvm_vcpu *vcpu)
> +{
> +	return vcpu->arch.vsesr_el2;
> +}
> +
>  static inline void vcpu_set_vsesr(struct kvm_vcpu *vcpu, u64 vsesr)
>  {
>  	vcpu->arch.vsesr_el2 = vsesr;
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 469de8a..357304a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -335,6 +335,11 @@ 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);
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events);
>  
>  #define KVM_ARCH_WANT_MMU_NOTIFIER
>  int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
> @@ -363,6 +368,8 @@ void handle_exit_early(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  int kvm_perf_init(void);
>  int kvm_perf_teardown(void);
>  
> +void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
> +
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
>  void __kvm_set_tpidr_el2(u64 tpidr_el2);
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 04b3256..df4faee 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -39,6 +39,7 @@
>  #define __KVM_HAVE_GUEST_DEBUG
>  #define __KVM_HAVE_IRQ_LINE
>  #define __KVM_HAVE_READONLY_MEM
> +#define __KVM_HAVE_VCPU_EVENTS
>  
>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>  
> @@ -153,6 +154,18 @@ struct kvm_sync_regs {
>  struct kvm_arch_memory_slot {
>  };
>  
> +/* for KVM_GET/SET_VCPU_EVENTS */
> +struct kvm_vcpu_events {
> +	struct {
> +		__u8 serror_pending;
> +		__u8 serror_has_esr;
> +		/* Align it to 8 bytes */
> +		__u8 pad[6];
> +		__u64 serror_esr;
> +	} exception;
> +	__u32 reserved[12];
> +};
> +
>  /* If you need to interpret the index values, here is the key: */
>  #define KVM_REG_ARM_COPROC_MASK		0x000000000FFF0000
>  #define KVM_REG_ARM_COPROC_SHIFT	16
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 56a0260..71d3841 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -289,6 +289,42 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>  	return -EINVAL;
>  }
>  
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events)
> +{
> +	events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
> +	events->exception.serror_has_esr =
> +			cpus_have_const_cap(ARM64_HAS_RAS_EXTN) &&
> +					(!!vcpu_get_vsesr(vcpu));

This is odd. Isn't VSESR==0 a valid value? And isn't
serror_has_esr always true when ARM64_HAS_RAS_EXTN is set?

> +
> +	if (events->exception.serror_pending &&
> +		events->exception.serror_has_esr)
> +		events->exception.serror_esr = vcpu_get_vsesr(vcpu);
> +	else
> +		events->exception.serror_esr = 0;
> +
> +	return 0;
> +}
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events)
> +{
> +	bool serror_pending = events->exception.serror_pending;
> +	bool has_esr = events->exception.serror_has_esr;
> +
> +	if (serror_pending && has_esr) {
> +		if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
> +			return -EINVAL;
> +
> +		kvm_set_sei_esr(vcpu, events->exception.serror_esr);
> +

Spurious blank line

> +	} else if (serror_pending) {
> +		kvm_inject_vabt(vcpu);
> +	}
> +
> +	return 0;
> +}
> +
>  int __attribute_const__ kvm_target_cpu(void)
>  {
>  	unsigned long implementor = read_cpuid_implementor();
> diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> index d8e7165..9e0ca56 100644
> --- a/arch/arm64/kvm/inject_fault.c
> +++ b/arch/arm64/kvm/inject_fault.c
> @@ -166,7 +166,7 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
>  
>  static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
>  {
> -	vcpu_set_vsesr(vcpu, esr);
> +	vcpu_set_vsesr(vcpu, esr & ESR_ELx_ISS_MASK);
>  	*vcpu_hcr(vcpu) |= HCR_VSE;
>  }
>  
> @@ -186,3 +186,8 @@ void kvm_inject_vabt(struct kvm_vcpu *vcpu)
>  {
>  	pend_guest_serror(vcpu, ESR_ELx_ISV);
>  }
> +
> +void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome)
> +{
> +	pend_guest_serror(vcpu, syndrome);
> +}

I think it'd make more sense to rename pend_guest_serror to
kvm_set_sei_esr and be done with it.

> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 38c8a64..20e919a 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -82,6 +82,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
>  		break;
>  	case KVM_CAP_SET_GUEST_DEBUG:
>  	case KVM_CAP_VCPU_ATTRIBUTES:
> +	case KVM_CAP_VCPU_EVENTS:
>  		r = 1;
>  		break;
>  	default:
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index a4c1b76..8b43968 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1107,6 +1107,27 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>  		r = kvm_arm_vcpu_has_attr(vcpu, &attr);
>  		break;
>  	}
> +	case KVM_GET_VCPU_EVENTS: {
> +		struct kvm_vcpu_events events;
> +
> +		memset(&events, 0, sizeof(events));

You could write this as

		struct kvm_cpu_events events = { };

but it'd make more sense if kvm_arm_vcpu_get_events() did all the work
rather than having this split responsibility.

> +		if (kvm_arm_vcpu_get_events(vcpu, &events))
> +			return -EINVAL;
> +
> +		if (copy_to_user(argp, &events, sizeof(events)))
> +			return -EFAULT;
> +
> +		return 0;
> +	}
> +	case KVM_SET_VCPU_EVENTS: {
> +		struct kvm_vcpu_events events;
> +
> +		if (copy_from_user(&events, argp,
> +				sizeof(struct kvm_vcpu_events)))

Prefer using sizeof(events) instead.

> +			return -EFAULT;
> +
> +		return kvm_arm_vcpu_set_events(vcpu, &events);
> +	}
>  	default:
>  		r = -EINVAL;
>  	}
> 

Thanks,

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
  2018-05-31 13:08 ` [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS Dongjiu Geng
  2018-06-01  9:17   ` Marc Zyngier
@ 2018-06-01 21:22   ` Eric Northup
  2018-06-04  9:53     ` gengdongjiu
  2018-06-04 19:06   ` kbuild test robot
  2018-06-04 20:03   ` kbuild test robot
  3 siblings, 1 reply; 9+ messages in thread
From: Eric Northup @ 2018-06-01 21:22 UTC (permalink / raw)
  To: gengdongjiu
  Cc: rkrcmar, corbet, christoffer.dall, Marc Zyngier, linux,
	Catalin Marinas, will.deacon, KVM, linux-doc, james.morse,
	linux-arm-kernel, Linux Kernel Mailing List, linux-acpi

On Wed, May 30, 2018 at 10:04 PM Dongjiu Geng <gengdongjiu@huawei.com> wrote:
>
> For the migrating VMs, user space may need to know the exception
> state. For example, in the machine A, KVM make an SError pending,
> when migrate to B, KVM also needs to pend an SError.
>
> This new IOCTL exports user-invisible states related to SError.
> Together with appropriate user space changes, user space can get/set
> the SError exception state to do migrate/snapshot/suspend.
>
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> --
> this series patch is separated from https://www.spinics.net/lists/kvm/msg168917.html
> change since V12:
> 1. change (vcpu->arch.hcr_el2 & HCR_VSE) to !!(vcpu->arch.hcr_el2 & HCR_VSE) in kvm_arm_vcpu_get_events()
>
> Change since V11:
> Address James's comments, thanks James
> 1. Align the struct of kvm_vcpu_events to 64 bytes
> 2. Avoid exposing the stale ESR value in the kvm_arm_vcpu_get_events()
> 3. Change variables 'injected' name to 'serror_pending' in the kvm_arm_vcpu_set_events()
> 4. Change to sizeof(events) from sizeof(struct kvm_vcpu_events) in kvm_arch_vcpu_ioctl()
>
> Change since V10:
> Address James's comments, thanks James
> 1. Merge the helper function with the user.
> 2. Move the ISS_MASK into pend_guest_serror() to clear top bits
> 3. Make kvm_vcpu_events struct align to 4 bytes
> 4. Add something check in the kvm_arm_vcpu_set_events()
> 5. Check kvm_arm_vcpu_get/set_events()'s return value.
> 6. Initialise kvm_vcpu_events to 0 so that padding transferred to user-space doesn't
>    contain kernel stack.
> ---
>  Documentation/virtual/kvm/api.txt    | 31 ++++++++++++++++++++++++++++---
>  arch/arm/include/asm/kvm_host.h      |  6 ++++++
>  arch/arm/kvm/guest.c                 | 12 ++++++++++++
>  arch/arm64/include/asm/kvm_emulate.h |  5 +++++
>  arch/arm64/include/asm/kvm_host.h    |  7 +++++++
>  arch/arm64/include/uapi/asm/kvm.h    | 13 +++++++++++++
>  arch/arm64/kvm/guest.c               | 36 ++++++++++++++++++++++++++++++++++++
>  arch/arm64/kvm/inject_fault.c        |  7 ++++++-
>  arch/arm64/kvm/reset.c               |  1 +
>  virt/kvm/arm/arm.c                   | 21 +++++++++++++++++++++
>  10 files changed, 135 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index fdac969..8896737 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -835,11 +835,13 @@ struct kvm_clock_data {
>
>  Capability: KVM_CAP_VCPU_EVENTS
>  Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
>  Type: vm ioctl
>  Parameters: struct kvm_vcpu_event (out)
>  Returns: 0 on success, -1 on error
>
> +X86:
> +
>  Gets currently pending exceptions, interrupts, and NMIs as well as related
>  states of the vcpu.
>
> @@ -881,15 +883,32 @@ Only two fields are defined in the flags field:
>  - KVM_VCPUEVENT_VALID_SMM may be set in the flags field to signal that
>    smi contains a valid state.
>
> +ARM, ARM64:
> +
> +Gets currently pending SError exceptions as well as related states of the vcpu.
> +
> +struct kvm_vcpu_events {
> +       struct {
> +               __u8 serror_pending;
> +               __u8 serror_has_esr;
> +               /* Align it to 8 bytes */
> +               __u8 pad[6];
> +               __u64 serror_esr;
> +       } exception;
> +       __u32 reserved[12];
>
> +};
> +
>  4.32 KVM_SET_VCPU_EVENTS
>
> -Capability: KVM_CAP_VCPU_EVENTS
> +Capebility: KVM_CAP_VCPU_EVENTS
>  Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
>  Type: vm ioctl
>  Parameters: struct kvm_vcpu_event (in)
>  Returns: 0 on success, -1 on error
>
> +X86:
> +
>  Set pending exceptions, interrupts, and NMIs as well as related states of the
>  vcpu.
>
> @@ -910,6 +929,12 @@ shall be written into the VCPU.
>
>  KVM_VCPUEVENT_VALID_SMM can only be set if KVM_CAP_X86_SMM is available.
>
> +ARM, ARM64:
> +
> +Set pending SError exceptions as well as related states of the vcpu.
> +
> +See KVM_GET_VCPU_EVENTS for the data structure.
> +
>
>  4.33 KVM_GET_DEBUGREGS
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index c7c28c8..39f9901 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -213,6 +213,12 @@ 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);
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +                       struct kvm_vcpu_events *events);
> +
>  unsigned long kvm_call_hyp(void *hypfn, ...);
>  void force_vm_exit(const cpumask_t *mask);
>
> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> index a18f33e..c685f0e 100644
> --- a/arch/arm/kvm/guest.c
> +++ b/arch/arm/kvm/guest.c
> @@ -261,6 +261,18 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>         return -EINVAL;
>  }
>
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +                       struct kvm_vcpu_events *events)
> +{
> +       return -EINVAL;
> +}
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +                       struct kvm_vcpu_events *events)
> +{
> +       return -EINVAL;
> +}
> +
>  int __attribute_const__ kvm_target_cpu(void)
>  {
>         switch (read_cpuid_part()) {
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 1dab3a9..18f61ff 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -81,6 +81,11 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
>         return (unsigned long *)&vcpu->arch.hcr_el2;
>  }
>
> +static inline unsigned long vcpu_get_vsesr(struct kvm_vcpu *vcpu)
> +{
> +       return vcpu->arch.vsesr_el2;
> +}
> +
>  static inline void vcpu_set_vsesr(struct kvm_vcpu *vcpu, u64 vsesr)
>  {
>         vcpu->arch.vsesr_el2 = vsesr;
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 469de8a..357304a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -335,6 +335,11 @@ 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);
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +                       struct kvm_vcpu_events *events);
>
>  #define KVM_ARCH_WANT_MMU_NOTIFIER
>  int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
> @@ -363,6 +368,8 @@ void handle_exit_early(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  int kvm_perf_init(void);
>  int kvm_perf_teardown(void);
>
> +void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
> +
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>
>  void __kvm_set_tpidr_el2(u64 tpidr_el2);
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 04b3256..df4faee 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -39,6 +39,7 @@
>  #define __KVM_HAVE_GUEST_DEBUG
>  #define __KVM_HAVE_IRQ_LINE
>  #define __KVM_HAVE_READONLY_MEM
> +#define __KVM_HAVE_VCPU_EVENTS
>
>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>
> @@ -153,6 +154,18 @@ struct kvm_sync_regs {
>  struct kvm_arch_memory_slot {
>  };
>
> +/* for KVM_GET/SET_VCPU_EVENTS */
> +struct kvm_vcpu_events {
> +       struct {
> +               __u8 serror_pending;
> +               __u8 serror_has_esr;
> +               /* Align it to 8 bytes */
> +               __u8 pad[6];
> +               __u64 serror_esr;
> +       } exception;
> +       __u32 reserved[12];

It will be easier to re-purpose this in the future if the field is
reserved and is checked that it must be zero.  SET_VCPU_EVENTS would
return EINVAL if reserved fields get used until a later meaning is
defined for them.

> +};
> +
>  /* If you need to interpret the index values, here is the key: */
>  #define KVM_REG_ARM_COPROC_MASK                0x000000000FFF0000
>  #define KVM_REG_ARM_COPROC_SHIFT       16
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 56a0260..71d3841 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -289,6 +289,42 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>         return -EINVAL;
>  }
>
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +                       struct kvm_vcpu_events *events)
> +{
> +       events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
> +       events->exception.serror_has_esr =
> +                       cpus_have_const_cap(ARM64_HAS_RAS_EXTN) &&
> +                                       (!!vcpu_get_vsesr(vcpu));
> +
> +       if (events->exception.serror_pending &&
> +               events->exception.serror_has_esr)
> +               events->exception.serror_esr = vcpu_get_vsesr(vcpu);
> +       else
> +               events->exception.serror_esr = 0;
> +
> +       return 0;
> +}
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +                       struct kvm_vcpu_events *events)
> +{
> +       bool serror_pending = events->exception.serror_pending;
> +       bool has_esr = events->exception.serror_has_esr;
> +
> +       if (serror_pending && has_esr) {
> +               if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
> +                       return -EINVAL;
> +
> +               kvm_set_sei_esr(vcpu, events->exception.serror_esr);
> +
> +       } else if (serror_pending) {
> +               kvm_inject_vabt(vcpu);
> +       }
> +
> +       return 0;
> +}
> +
>  int __attribute_const__ kvm_target_cpu(void)
>  {
>         unsigned long implementor = read_cpuid_implementor();
> diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> index d8e7165..9e0ca56 100644
> --- a/arch/arm64/kvm/inject_fault.c
> +++ b/arch/arm64/kvm/inject_fault.c
> @@ -166,7 +166,7 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
>
>  static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
>  {
> -       vcpu_set_vsesr(vcpu, esr);
> +       vcpu_set_vsesr(vcpu, esr & ESR_ELx_ISS_MASK);
>         *vcpu_hcr(vcpu) |= HCR_VSE;
>  }
>
> @@ -186,3 +186,8 @@ void kvm_inject_vabt(struct kvm_vcpu *vcpu)
>  {
>         pend_guest_serror(vcpu, ESR_ELx_ISV);
>  }
> +
> +void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome)
> +{
> +       pend_guest_serror(vcpu, syndrome);
> +}
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 38c8a64..20e919a 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -82,6 +82,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
>                 break;
>         case KVM_CAP_SET_GUEST_DEBUG:
>         case KVM_CAP_VCPU_ATTRIBUTES:
> +       case KVM_CAP_VCPU_EVENTS:
>                 r = 1;
>                 break;
>         default:
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index a4c1b76..8b43968 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1107,6 +1107,27 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>                 r = kvm_arm_vcpu_has_attr(vcpu, &attr);
>                 break;
>         }
> +       case KVM_GET_VCPU_EVENTS: {
> +               struct kvm_vcpu_events events;
> +
> +               memset(&events, 0, sizeof(events));
> +               if (kvm_arm_vcpu_get_events(vcpu, &events))
> +                       return -EINVAL;
> +
> +               if (copy_to_user(argp, &events, sizeof(events)))
> +                       return -EFAULT;
> +
> +               return 0;
> +       }
> +       case KVM_SET_VCPU_EVENTS: {
> +               struct kvm_vcpu_events events;
> +
> +               if (copy_from_user(&events, argp,
> +                               sizeof(struct kvm_vcpu_events)))
> +                       return -EFAULT;
> +
> +               return kvm_arm_vcpu_set_events(vcpu, &events);
> +       }
>         default:
>                 r = -EINVAL;
>         }
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
  2018-06-01 21:22   ` Eric Northup
@ 2018-06-04  9:53     ` gengdongjiu
  0 siblings, 0 replies; 9+ messages in thread
From: gengdongjiu @ 2018-06-04  9:53 UTC (permalink / raw)
  To: Eric Northup
  Cc: rkrcmar, corbet, christoffer.dall, Marc Zyngier, linux,
	Catalin Marinas, will.deacon, KVM, linux-doc, james.morse,
	linux-arm-kernel, Linux Kernel Mailing List, linux-acpi


On 2018/6/2 5:22, Eric Northup wrote:
> On Wed, May 30, 2018 at 10:04 PM Dongjiu Geng <gengdongjiu@huawei.com> wrote:
>>
>> For the migrating VMs, user space may need to know the exception

[...]

>> +               __u8 pad[6];
>> +               __u64 serror_esr;
>> +       } exception;
>> +       __u32 reserved[12];
> 
> It will be easier to re-purpose this in the future if the field is
> reserved and is checked that it must be zero.  SET_VCPU_EVENTS would
> return EINVAL if reserved fields get used until a later meaning is
> defined for them.
Ok, thanks. I will check the reserved fields when calling SET_VCPU_EVENTS.

> 
>> +};
>> +
>>  /* If you need to interpret the index values, here is the key: */
>>  #define KVM_REG_ARM_COPROC_MASK                0x000000000FFF0000
>>  #define KVM_REG_ARM_COPROC_SHIFT       16
>> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
>> index 56a0260..71d3841 100644
>> --- a/arch/arm64/kvm/guest.c
>> +++ b/arch/arm64/kvm/guest.c
>> @@ -289,6 +289,42 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>>         return -EINVAL;
>>  }
>>
[...]
>> --
>> 2.7.4
>>
> 
> .
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
  2018-05-31 13:08 ` [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS Dongjiu Geng
  2018-06-01  9:17   ` Marc Zyngier
  2018-06-01 21:22   ` Eric Northup
@ 2018-06-04 19:06   ` kbuild test robot
  2018-06-04 20:03   ` kbuild test robot
  3 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2018-06-04 19:06 UTC (permalink / raw)
  To: Dongjiu Geng
  Cc: kbuild-all, rkrcmar, corbet, christoffer.dall, marc.zyngier,
	linux, catalin.marinas, will.deacon, kvm, linux-doc, james.morse,
	gengdongjiu, linux-arm-kernel, linux-kernel, linux-acpi

[-- Attachment #1: Type: text/plain, Size: 2167 bytes --]

Hi Dongjiu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on v4.17 next-20180601]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dongjiu-Geng/arm64-KVM-export-the-capability-to-set-guest-SError-syndrome/20180602-175846
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm-axm55xx_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kvm_host.h:37:0,
                    from arch/arm/kernel/asm-offsets.c:18:
>> arch/arm/include/asm/kvm_host.h:217:11: warning: 'struct kvm_vcpu_events' declared inside parameter list will not be visible outside of this definition or declaration
       struct kvm_vcpu_events *events);
              ^~~~~~~~~~~~~~~
   arch/arm/include/asm/kvm_host.h:220:11: warning: 'struct kvm_vcpu_events' declared inside parameter list will not be visible outside of this definition or declaration
       struct kvm_vcpu_events *events);
              ^~~~~~~~~~~~~~~

vim +217 arch/arm/include/asm/kvm_host.h

   210	
   211	int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init);
   212	unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
   213	int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
   214	int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
   215	int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
   216	int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
 > 217				struct kvm_vcpu_events *events);
   218	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19855 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
  2018-05-31 13:08 ` [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS Dongjiu Geng
                     ` (2 preceding siblings ...)
  2018-06-04 19:06   ` kbuild test robot
@ 2018-06-04 20:03   ` kbuild test robot
  3 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2018-06-04 20:03 UTC (permalink / raw)
  To: Dongjiu Geng
  Cc: kbuild-all, rkrcmar, corbet, christoffer.dall, marc.zyngier,
	linux, catalin.marinas, will.deacon, kvm, linux-doc, james.morse,
	gengdongjiu, linux-arm-kernel, linux-kernel, linux-acpi

[-- Attachment #1: Type: text/plain, Size: 10800 bytes --]

Hi Dongjiu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on v4.17 next-20180601]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dongjiu-Geng/arm64-KVM-export-the-capability-to-set-guest-SError-syndrome/20180602-175846
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm-axm55xx_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/kvm_host.h:37:0,
                    from arch/arm/kvm/guest.c:21:
   arch/arm/include/asm/kvm_host.h:217:11: warning: 'struct kvm_vcpu_events' declared inside parameter list will not be visible outside of this definition or declaration
       struct kvm_vcpu_events *events);
              ^~~~~~~~~~~~~~~
   arch/arm/include/asm/kvm_host.h:220:11: warning: 'struct kvm_vcpu_events' declared inside parameter list will not be visible outside of this definition or declaration
       struct kvm_vcpu_events *events);
              ^~~~~~~~~~~~~~~
>> arch/arm/kvm/guest.c:265:11: warning: 'struct kvm_vcpu_events' declared inside parameter list will not be visible outside of this definition or declaration
       struct kvm_vcpu_events *events)
              ^~~~~~~~~~~~~~~
>> arch/arm/kvm/guest.c:264:5: error: conflicting types for 'kvm_arm_vcpu_get_events'
    int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
        ^~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/kvm_host.h:37:0,
                    from arch/arm/kvm/guest.c:21:
   arch/arm/include/asm/kvm_host.h:216:5: note: previous declaration of 'kvm_arm_vcpu_get_events' was here
    int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
        ^~~~~~~~~~~~~~~~~~~~~~~
   arch/arm/kvm/guest.c:271:11: warning: 'struct kvm_vcpu_events' declared inside parameter list will not be visible outside of this definition or declaration
       struct kvm_vcpu_events *events)
              ^~~~~~~~~~~~~~~
>> arch/arm/kvm/guest.c:270:5: error: conflicting types for 'kvm_arm_vcpu_set_events'
    int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
        ^~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/kvm_host.h:37:0,
                    from arch/arm/kvm/guest.c:21:
   arch/arm/include/asm/kvm_host.h:219:5: note: previous declaration of 'kvm_arm_vcpu_set_events' was here
    int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
        ^~~~~~~~~~~~~~~~~~~~~~~
--
   In file included from include/linux/kvm_host.h:37:0,
                    from arch/arm/kvm/../../../virt/kvm/arm/arm.c:22:
   arch/arm/include/asm/kvm_host.h:217:11: warning: 'struct kvm_vcpu_events' declared inside parameter list will not be visible outside of this definition or declaration
       struct kvm_vcpu_events *events);
              ^~~~~~~~~~~~~~~
   arch/arm/include/asm/kvm_host.h:220:11: warning: 'struct kvm_vcpu_events' declared inside parameter list will not be visible outside of this definition or declaration
       struct kvm_vcpu_events *events);
              ^~~~~~~~~~~~~~~
   In file included from include/asm-generic/ioctl.h:5:0,
                    from ./arch/arm/include/generated/uapi/asm/ioctl.h:1,
                    from include/uapi/linux/ioctl.h:5,
                    from include/uapi/linux/fs.h:14,
                    from include/linux/fs.h:41,
                    from include/linux/huge_mm.h:7,
                    from include/linux/mm.h:472,
                    from include/linux/kvm_host.h:17,
                    from arch/arm/kvm/../../../virt/kvm/arm/arm.c:22:
   arch/arm/kvm/../../../virt/kvm/arm/arm.c: In function 'kvm_arch_vcpu_ioctl':
>> include/uapi/linux/kvm.h:1339:54: error: invalid application of 'sizeof' to incomplete type 'struct kvm_vcpu_events'
    #define KVM_GET_VCPU_EVENTS       _IOR(KVMIO,  0x9f, struct kvm_vcpu_events)
                                                         ^
   include/uapi/asm-generic/ioctl.h:73:5: note: in definition of macro '_IOC'
      ((size) << _IOC_SIZESHIFT))
        ^~~~
>> include/uapi/asm-generic/ioctl.h:86:56: note: in expansion of macro '_IOC_TYPECHECK'
    #define _IOR(type,nr,size) _IOC(_IOC_READ,(type),(nr),(_IOC_TYPECHECK(size)))
                                                           ^~~~~~~~~~~~~~
>> include/uapi/linux/kvm.h:1339:35: note: in expansion of macro '_IOR'
    #define KVM_GET_VCPU_EVENTS       _IOR(KVMIO,  0x9f, struct kvm_vcpu_events)
                                      ^~~~
>> arch/arm/kvm/../../../virt/kvm/arm/arm.c:1110:7: note: in expansion of macro 'KVM_GET_VCPU_EVENTS'
     case KVM_GET_VCPU_EVENTS: {
          ^~~~~~~~~~~~~~~~~~~
>> include/asm-generic/ioctl.h:13:25: error: array type has incomplete element type 'struct kvm_vcpu_events'
     ((sizeof(t) == sizeof(t[1]) && \
                            ^
   include/uapi/asm-generic/ioctl.h:73:5: note: in definition of macro '_IOC'
      ((size) << _IOC_SIZESHIFT))
        ^~~~
>> include/uapi/asm-generic/ioctl.h:86:56: note: in expansion of macro '_IOC_TYPECHECK'
    #define _IOR(type,nr,size) _IOC(_IOC_READ,(type),(nr),(_IOC_TYPECHECK(size)))
                                                           ^~~~~~~~~~~~~~
>> include/uapi/linux/kvm.h:1339:35: note: in expansion of macro '_IOR'
    #define KVM_GET_VCPU_EVENTS       _IOR(KVMIO,  0x9f, struct kvm_vcpu_events)
                                      ^~~~
>> arch/arm/kvm/../../../virt/kvm/arm/arm.c:1110:7: note: in expansion of macro 'KVM_GET_VCPU_EVENTS'
     case KVM_GET_VCPU_EVENTS: {
          ^~~~~~~~~~~~~~~~~~~
>> include/uapi/linux/kvm.h:1339:54: error: invalid application of 'sizeof' to incomplete type 'struct kvm_vcpu_events'
    #define KVM_GET_VCPU_EVENTS       _IOR(KVMIO,  0x9f, struct kvm_vcpu_events)
                                                         ^
   include/uapi/asm-generic/ioctl.h:73:5: note: in definition of macro '_IOC'
      ((size) << _IOC_SIZESHIFT))
        ^~~~
>> include/uapi/asm-generic/ioctl.h:86:56: note: in expansion of macro '_IOC_TYPECHECK'
    #define _IOR(type,nr,size) _IOC(_IOC_READ,(type),(nr),(_IOC_TYPECHECK(size)))
                                                           ^~~~~~~~~~~~~~
>> include/uapi/linux/kvm.h:1339:35: note: in expansion of macro '_IOR'
    #define KVM_GET_VCPU_EVENTS       _IOR(KVMIO,  0x9f, struct kvm_vcpu_events)
                                      ^~~~
>> arch/arm/kvm/../../../virt/kvm/arm/arm.c:1110:7: note: in expansion of macro 'KVM_GET_VCPU_EVENTS'
     case KVM_GET_VCPU_EVENTS: {
          ^~~~~~~~~~~~~~~~~~~
>> include/uapi/linux/kvm.h:1339:54: error: invalid application of 'sizeof' to incomplete type 'struct kvm_vcpu_events'
    #define KVM_GET_VCPU_EVENTS       _IOR(KVMIO,  0x9f, struct kvm_vcpu_events)
                                                         ^
   include/uapi/asm-generic/ioctl.h:73:5: note: in definition of macro '_IOC'
      ((size) << _IOC_SIZESHIFT))
        ^~~~
>> include/uapi/asm-generic/ioctl.h:86:56: note: in expansion of macro '_IOC_TYPECHECK'
    #define _IOR(type,nr,size) _IOC(_IOC_READ,(type),(nr),(_IOC_TYPECHECK(size)))
                                                           ^~~~~~~~~~~~~~
>> include/uapi/linux/kvm.h:1339:35: note: in expansion of macro '_IOR'
    #define KVM_GET_VCPU_EVENTS       _IOR(KVMIO,  0x9f, struct kvm_vcpu_events)
                                      ^~~~
>> arch/arm/kvm/../../../virt/kvm/arm/arm.c:1110:7: note: in expansion of macro 'KVM_GET_VCPU_EVENTS'
     case KVM_GET_VCPU_EVENTS: {
          ^~~~~~~~~~~~~~~~~~~
>> arch/arm/kvm/../../../virt/kvm/arm/arm.c:1111:26: error: storage size of 'events' isn't known
      struct kvm_vcpu_events events;
                             ^~~~~~
   arch/arm/kvm/../../../virt/kvm/arm/arm.c:1111:26: warning: unused variable 'events' [-Wunused-variable]
   In file included from include/asm-generic/ioctl.h:5:0,
                    from ./arch/arm/include/generated/uapi/asm/ioctl.h:1,
                    from include/uapi/linux/ioctl.h:5,
                    from include/uapi/linux/fs.h:14,
                    from include/linux/fs.h:41,
                    from include/linux/huge_mm.h:7,
                    from include/linux/mm.h:472,
                    from include/linux/kvm_host.h:17,
                    from arch/arm/kvm/../../../virt/kvm/arm/arm.c:22:
   include/uapi/linux/kvm.h:1340:54: error: invalid application of 'sizeof' to incomplete type 'struct kvm_vcpu_events'
    #define KVM_SET_VCPU_EVENTS       _IOW(KVMIO,  0xa0, struct kvm_vcpu_events)
                                                         ^
   include/uapi/asm-generic/ioctl.h:73:5: note: in definition of macro '_IOC'
      ((size) << _IOC_SIZESHIFT))
        ^~~~
   include/uapi/asm-generic/ioctl.h:87:57: note: in expansion of macro '_IOC_TYPECHECK'
    #define _IOW(type,nr,size) _IOC(_IOC_WRITE,(type),(nr),(_IOC_TYPECHECK(size)))
                                                            ^~~~~~~~~~~~~~
>> include/uapi/linux/kvm.h:1340:35: note: in expansion of macro '_IOW'
    #define KVM_SET_VCPU_EVENTS       _IOW(KVMIO,  0xa0, struct kvm_vcpu_events)
                                      ^~~~
>> arch/arm/kvm/../../../virt/kvm/arm/arm.c:1122:7: note: in expansion of macro 'KVM_SET_VCPU_EVENTS'
     case KVM_SET_VCPU_EVENTS: {
          ^~~~~~~~~~~~~~~~~~~
>> include/asm-generic/ioctl.h:13:25: error: array type has incomplete element type 'struct kvm_vcpu_events'
     ((sizeof(t) == sizeof(t[1]) && \
                            ^
   include/uapi/asm-generic/ioctl.h:73:5: note: in definition of macro '_IOC'
      ((size) << _IOC_SIZESHIFT))
        ^~~~
   include/uapi/asm-generic/ioctl.h:87:57: note: in expansion of macro '_IOC_TYPECHECK'
    #define _IOW(type,nr,size) _IOC(_IOC_WRITE,(type),(nr),(_IOC_TYPECHECK(size)))
                                                            ^~~~~~~~~~~~~~

vim +/kvm_arm_vcpu_get_events +264 arch/arm/kvm/guest.c

   263	
 > 264	int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
 > 265				struct kvm_vcpu_events *events)
   266	{
   267		return -EINVAL;
   268	}
   269	
 > 270	int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
   271				struct kvm_vcpu_events *events)
   272	{
   273		return -EINVAL;
   274	}
   275	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19855 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
@ 2018-06-01 15:04 gengdongjiu
  0 siblings, 0 replies; 9+ messages in thread
From: gengdongjiu @ 2018-06-01 15:04 UTC (permalink / raw)
  To: Marc Zyngier, rkrcmar, corbet, christoffer.dall, linux,
	catalin.marinas, will.deacon, kvm, linux-doc, james.morse,
	linux-arm-kernel, linux-kernel, linux-acpi

Hi Marc,

> 
> On 31/05/18 14:08, Dongjiu Geng wrote:
> > For the migrating VMs, user space may need to know the exception
> > state. For example, in the machine A, KVM make an SError pending, when
> > migrate to B, KVM also needs to pend an SError.
> >
> > This new IOCTL exports user-invisible states related to SError.
> > Together with appropriate user space changes, user space can get/set
> > the SError exception state to do migrate/snapshot/suspend.
> >
> > Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> > --
> > this series patch is separated from
> > https://www.spinics.net/lists/kvm/msg168917.html
> > change since V12:
> > 1. change (vcpu->arch.hcr_el2 & HCR_VSE) to !!(vcpu->arch.hcr_el2 &
> > HCR_VSE) in kvm_arm_vcpu_get_events()
> >
> > Change since V11:
> > Address James's comments, thanks James 1. Align the struct of
> > kvm_vcpu_events to 64 bytes 2. Avoid exposing the stale ESR value in
> > the kvm_arm_vcpu_get_events() 3. Change variables 'injected' name to
> > 'serror_pending' in the kvm_arm_vcpu_set_events() 4. Change to
> > sizeof(events) from sizeof(struct kvm_vcpu_events) in
> > kvm_arch_vcpu_ioctl()
> >
> > Change since V10:
> > Address James's comments, thanks James 1. Merge the helper function
> > with the user.
> > 2. Move the ISS_MASK into pend_guest_serror() to clear top bits 3.
> > Make kvm_vcpu_events struct align to 4 bytes 4. Add something check in
> > the kvm_arm_vcpu_set_events() 5. Check kvm_arm_vcpu_get/set_events()'s
> > return value.
> > 6. Initialise kvm_vcpu_events to 0 so that padding transferred to user-space doesn't
> >    contain kernel stack.
> > ---
> >  Documentation/virtual/kvm/api.txt    | 31 ++++++++++++++++++++++++++++---
> >  arch/arm/include/asm/kvm_host.h      |  6 ++++++
> >  arch/arm/kvm/guest.c                 | 12 ++++++++++++
> >  arch/arm64/include/asm/kvm_emulate.h |  5 +++++
> >  arch/arm64/include/asm/kvm_host.h    |  7 +++++++
> >  arch/arm64/include/uapi/asm/kvm.h    | 13 +++++++++++++
> >  arch/arm64/kvm/guest.c               | 36 ++++++++++++++++++++++++++++++++++++
> >  arch/arm64/kvm/inject_fault.c        |  7 ++++++-
> >  arch/arm64/kvm/reset.c               |  1 +
> >  virt/kvm/arm/arm.c                   | 21 +++++++++++++++++++++
> >  10 files changed, 135 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/virtual/kvm/api.txt
> > b/Documentation/virtual/kvm/api.txt
> > index fdac969..8896737 100644
> > --- a/Documentation/virtual/kvm/api.txt
> > +++ b/Documentation/virtual/kvm/api.txt
> > @@ -835,11 +835,13 @@ struct kvm_clock_data {
> >
> >  Capability: KVM_CAP_VCPU_EVENTS
> >  Extended by: KVM_CAP_INTR_SHADOW
> > -Architectures: x86
> > +Architectures: x86, arm, arm64
> >  Type: vm ioctl
> >  Parameters: struct kvm_vcpu_event (out)
> >  Returns: 0 on success, -1 on error
> >
> > +X86:
> > +
> >  Gets currently pending exceptions, interrupts, and NMIs as well as
> > related  states of the vcpu.
> >
> > @@ -881,15 +883,32 @@ Only two fields are defined in the flags field:
> >  - KVM_VCPUEVENT_VALID_SMM may be set in the flags field to signal that
> >    smi contains a valid state.
> >
> > +ARM, ARM64:
> > +
> > +Gets currently pending SError exceptions as well as related states of the vcpu.
> > +
> > +struct kvm_vcpu_events {
> > +	struct {
> > +		__u8 serror_pending;
> > +		__u8 serror_has_esr;
> > +		/* Align it to 8 bytes */
> > +		__u8 pad[6];
> > +		__u64 serror_esr;
> > +	} exception;
> > +	__u32 reserved[12];
> > +};
> > +
> >  4.32 KVM_SET_VCPU_EVENTS
> >
> > -Capability: KVM_CAP_VCPU_EVENTS
> > +Capebility: KVM_CAP_VCPU_EVENTS
> >  Extended by: KVM_CAP_INTR_SHADOW
> > -Architectures: x86
> > +Architectures: x86, arm, arm64
> >  Type: vm ioctl
> >  Parameters: struct kvm_vcpu_event (in)
> >  Returns: 0 on success, -1 on error
> >
> > +X86:
> > +
> >  Set pending exceptions, interrupts, and NMIs as well as related
> > states of the  vcpu.
> >
> > @@ -910,6 +929,12 @@ shall be written into the VCPU.
> >
> >  KVM_VCPUEVENT_VALID_SMM can only be set if KVM_CAP_X86_SMM is available.
> >
> > +ARM, ARM64:
> > +
> > +Set pending SError exceptions as well as related states of the vcpu.
> > +
> > +See KVM_GET_VCPU_EVENTS for the data structure.
> > +
> >
> >  4.33 KVM_GET_DEBUGREGS
> >
> > diff --git a/arch/arm/include/asm/kvm_host.h
> > b/arch/arm/include/asm/kvm_host.h index c7c28c8..39f9901 100644
> > --- a/arch/arm/include/asm/kvm_host.h
> > +++ b/arch/arm/include/asm/kvm_host.h
> > @@ -213,6 +213,12 @@ 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);
> > +
> > +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> > +			struct kvm_vcpu_events *events);
> > +
> >  unsigned long kvm_call_hyp(void *hypfn, ...);  void
> > force_vm_exit(const cpumask_t *mask);
> >
> > diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c index
> > a18f33e..c685f0e 100644
> > --- a/arch/arm/kvm/guest.c
> > +++ b/arch/arm/kvm/guest.c
> > @@ -261,6 +261,18 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
> >  	return -EINVAL;
> >  }
> >
> > +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> > +			struct kvm_vcpu_events *events)
> > +{
> > +	return -EINVAL;
> > +}
> > +
> > +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> > +			struct kvm_vcpu_events *events)
> > +{
> > +	return -EINVAL;
> > +}
> > +
> >  int __attribute_const__ kvm_target_cpu(void)  {
> >  	switch (read_cpuid_part()) {
> > diff --git a/arch/arm64/include/asm/kvm_emulate.h
> > b/arch/arm64/include/asm/kvm_emulate.h
> > index 1dab3a9..18f61ff 100644
> > --- a/arch/arm64/include/asm/kvm_emulate.h
> > +++ b/arch/arm64/include/asm/kvm_emulate.h
> > @@ -81,6 +81,11 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
> >  	return (unsigned long *)&vcpu->arch.hcr_el2;  }
> >
> > +static inline unsigned long vcpu_get_vsesr(struct kvm_vcpu *vcpu) {
> > +	return vcpu->arch.vsesr_el2;
> > +}
> > +
> >  static inline void vcpu_set_vsesr(struct kvm_vcpu *vcpu, u64 vsesr)
> > {
> >  	vcpu->arch.vsesr_el2 = vsesr;
> > diff --git a/arch/arm64/include/asm/kvm_host.h
> > b/arch/arm64/include/asm/kvm_host.h
> > index 469de8a..357304a 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -335,6 +335,11 @@ 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);
> > +
> > +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> > +			struct kvm_vcpu_events *events);
> >
> >  #define KVM_ARCH_WANT_MMU_NOTIFIER
> >  int kvm_unmap_hva(struct kvm *kvm, unsigned long hva); @@ -363,6
> > +368,8 @@ void handle_exit_early(struct kvm_vcpu *vcpu, struct kvm_run
> > *run,  int kvm_perf_init(void);  int kvm_perf_teardown(void);
> >
> > +void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
> > +
> >  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long
> > mpidr);
> >
> >  void __kvm_set_tpidr_el2(u64 tpidr_el2); diff --git
> > a/arch/arm64/include/uapi/asm/kvm.h
> > b/arch/arm64/include/uapi/asm/kvm.h
> > index 04b3256..df4faee 100644
> > --- a/arch/arm64/include/uapi/asm/kvm.h
> > +++ b/arch/arm64/include/uapi/asm/kvm.h
> > @@ -39,6 +39,7 @@
> >  #define __KVM_HAVE_GUEST_DEBUG
> >  #define __KVM_HAVE_IRQ_LINE
> >  #define __KVM_HAVE_READONLY_MEM
> > +#define __KVM_HAVE_VCPU_EVENTS
> >
> >  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
> >
> > @@ -153,6 +154,18 @@ struct kvm_sync_regs {  struct
> > kvm_arch_memory_slot {  };
> >
> > +/* for KVM_GET/SET_VCPU_EVENTS */
> > +struct kvm_vcpu_events {
> > +	struct {
> > +		__u8 serror_pending;
> > +		__u8 serror_has_esr;
> > +		/* Align it to 8 bytes */
> > +		__u8 pad[6];
> > +		__u64 serror_esr;
> > +	} exception;
> > +	__u32 reserved[12];
> > +};
> > +
> >  /* If you need to interpret the index values, here is the key: */
> >  #define KVM_REG_ARM_COPROC_MASK		0x000000000FFF0000
> >  #define KVM_REG_ARM_COPROC_SHIFT	16
> > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index
> > 56a0260..71d3841 100644
> > --- a/arch/arm64/kvm/guest.c
> > +++ b/arch/arm64/kvm/guest.c
> > @@ -289,6 +289,42 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
> >  	return -EINVAL;
> >  }
> >
> > +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> > +			struct kvm_vcpu_events *events)
> > +{
> > +	events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
> > +	events->exception.serror_has_esr =
> > +			cpus_have_const_cap(ARM64_HAS_RAS_EXTN) &&
> > +					(!!vcpu_get_vsesr(vcpu));
> 
> This is odd. Isn't VSESR==0 a valid value? And isn't serror_has_esr always true when ARM64_HAS_RAS_EXTN is set?

An all-zero SError ESR now means: 'RAS error: Uncategorized' instead of 'no valid ISS'. yes, I think the VSESR can be 0. Thanks for the pointing out. So it is better to write as shown blow:

events->exception.serror_has_esr = cpus_have_const_cap(ARM64_HAS_RAS_EXTN);

> > +
> > +	if (events->exception.serror_pending &&
> > +		events->exception.serror_has_esr)
> > +		events->exception.serror_esr = vcpu_get_vsesr(vcpu);
> > +	else
> > +		events->exception.serror_esr = 0;
> > +
> > +	return 0;
> > +}
> > +
> > +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> > +			struct kvm_vcpu_events *events)
> > +{
> > +	bool serror_pending = events->exception.serror_pending;
> > +	bool has_esr = events->exception.serror_has_esr;
> > +
> > +	if (serror_pending && has_esr) {
> > +		if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
> > +			return -EINVAL;
> > +
> > +		kvm_set_sei_esr(vcpu, events->exception.serror_esr);
> > +
> 
> Spurious blank line

I will remove this blank line

> 
> > +	} else if (serror_pending) {
> > +		kvm_inject_vabt(vcpu);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  int __attribute_const__ kvm_target_cpu(void)  {
> >  	unsigned long implementor = read_cpuid_implementor(); diff --git
> > a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c index
> > d8e7165..9e0ca56 100644
> > --- a/arch/arm64/kvm/inject_fault.c
> > +++ b/arch/arm64/kvm/inject_fault.c
> > @@ -166,7 +166,7 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
> >
> >  static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)  {
> > -	vcpu_set_vsesr(vcpu, esr);
> > +	vcpu_set_vsesr(vcpu, esr & ESR_ELx_ISS_MASK);
> >  	*vcpu_hcr(vcpu) |= HCR_VSE;
> >  }
> >
> > @@ -186,3 +186,8 @@ void kvm_inject_vabt(struct kvm_vcpu *vcpu)  {
> >  	pend_guest_serror(vcpu, ESR_ELx_ISV);  }
> > +
> > +void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome) {
> > +	pend_guest_serror(vcpu, syndrome);
> > +}
> 
> I think it'd make more sense to rename pend_guest_serror to kvm_set_sei_esr and be done with it.

Ok, got it.

> 
> > diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c index
> > 38c8a64..20e919a 100644
> > --- a/arch/arm64/kvm/reset.c
> > +++ b/arch/arm64/kvm/reset.c
> > @@ -82,6 +82,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
> >  		break;
> >  	case KVM_CAP_SET_GUEST_DEBUG:
> >  	case KVM_CAP_VCPU_ATTRIBUTES:
> > +	case KVM_CAP_VCPU_EVENTS:
> >  		r = 1;
> >  		break;
> >  	default:
> > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c index
> > a4c1b76..8b43968 100644
> > --- a/virt/kvm/arm/arm.c
> > +++ b/virt/kvm/arm/arm.c
> > @@ -1107,6 +1107,27 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> >  		r = kvm_arm_vcpu_has_attr(vcpu, &attr);
> >  		break;
> >  	}
> > +	case KVM_GET_VCPU_EVENTS: {
> > +		struct kvm_vcpu_events events;
> > +
> > +		memset(&events, 0, sizeof(events));
> 
> You could write this as
> 
> 		struct kvm_cpu_events events = { };
> 
> but it'd make more sense if kvm_arm_vcpu_get_events() did all the work rather than having this split responsibility.

Ok, thanks for the good suggestion.

> 
> > +		if (kvm_arm_vcpu_get_events(vcpu, &events))
> > +			return -EINVAL;
> > +
> > +		if (copy_to_user(argp, &events, sizeof(events)))
> > +			return -EFAULT;
> > +
> > +		return 0;
> > +	}
> > +	case KVM_SET_VCPU_EVENTS: {
> > +		struct kvm_vcpu_events events;
> > +
> > +		if (copy_from_user(&events, argp,
> > +				sizeof(struct kvm_vcpu_events)))
> 
> Prefer using sizeof(events) instead.

Yes, it should, It is my careless. Thanks for the pointing out. 

> 
> > +			return -EFAULT;
> > +
> > +		return kvm_arm_vcpu_set_events(vcpu, &events);
> > +	}
> >  	default:
> >  		r = -EINVAL;
> >  	}
> >
> 
> Thanks,
> 
> 	M.
> --
> Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-06-04 20:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-31 13:08 [PATCH v1 0/2] support to set VSESR_EL2 by user space Dongjiu Geng
2018-05-31 13:08 ` [PATCH v1 1/2] arm64: KVM: export the capability to set guest SError syndrome Dongjiu Geng
2018-05-31 13:08 ` [PATCH v1 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS Dongjiu Geng
2018-06-01  9:17   ` Marc Zyngier
2018-06-01 21:22   ` Eric Northup
2018-06-04  9:53     ` gengdongjiu
2018-06-04 19:06   ` kbuild test robot
2018-06-04 20:03   ` kbuild test robot
2018-06-01 15:04 gengdongjiu

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).