All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 0/4] support exception state migration and set VSESR_EL2 by user space
@ 2018-07-13 15:47 ` James Morse
  0 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-13 15:47 UTC (permalink / raw)
  To: kvmarm; +Cc: kvm, Marc Zyngier, linuxarm, gengdongjiu, linux-arm-kernel

Hi guys,

This is GengDongjiu's get/set events series[0], with additional plumbing for
32bit.

Changes since v7: removed the temporary #ifdefs and updated the documentation
in patch 4.

This series adds the get/set events API to KVM for arm/arm64 so that
user-space can migrate pending SError, (which KVM may have made pending
as part of its device emulation), and for RAS make a new SError with a
specific ESR value pending. (Only systems with the v8.2 RAS Extensions
can specify an ESR, KVM advertises this capability with a new cap:
KVM_CAP_ARM_INJECT_SERROR_ESR)

User-space can use this for emulation of the v8.2 CPU RAS features
(using error nodes backed by mmio) or as part of NOTIFY_SEI which
extends a v8.2 RAS SError to also have firmware-first CPER records.


Thanks,

James


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

James Morse (2):
  KVM: arm64: Share the parts of get/set events useful to 32bit
  KVM: arm: Add 32bit get/set events support

 Documentation/virtual/kvm/api.txt    | 44 ++++++++++++++++++++++++---
 arch/arm/include/asm/kvm_host.h      |  5 ++++
 arch/arm/include/uapi/asm/kvm.h      | 13 ++++++++
 arch/arm/kvm/guest.c                 | 23 ++++++++++++++
 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               | 33 ++++++++++++++++++++
 arch/arm64/kvm/inject_fault.c        |  6 ++--
 arch/arm64/kvm/reset.c               |  4 +++
 include/uapi/linux/kvm.h             |  1 +
 virt/kvm/arm/arm.c                   | 45 ++++++++++++++++++++++++++++
 12 files changed, 192 insertions(+), 7 deletions(-)

-- 
2.18.0

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

* [PATCH v8 0/4] support exception state migration and set VSESR_EL2 by user space
@ 2018-07-13 15:47 ` James Morse
  0 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-13 15:47 UTC (permalink / raw)
  To: linux-arm-kernel

Hi guys,

This is GengDongjiu's get/set events series[0], with additional plumbing for
32bit.

Changes since v7: removed the temporary #ifdefs and updated the documentation
in patch 4.

This series adds the get/set events API to KVM for arm/arm64 so that
user-space can migrate pending SError, (which KVM may have made pending
as part of its device emulation), and for RAS make a new SError with a
specific ESR value pending. (Only systems with the v8.2 RAS Extensions
can specify an ESR, KVM advertises this capability with a new cap:
KVM_CAP_ARM_INJECT_SERROR_ESR)

User-space can use this for emulation of the v8.2 CPU RAS features
(using error nodes backed by mmio) or as part of NOTIFY_SEI which
extends a v8.2 RAS SError to also have firmware-first CPER records.


Thanks,

James


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

James Morse (2):
  KVM: arm64: Share the parts of get/set events useful to 32bit
  KVM: arm: Add 32bit get/set events support

 Documentation/virtual/kvm/api.txt    | 44 ++++++++++++++++++++++++---
 arch/arm/include/asm/kvm_host.h      |  5 ++++
 arch/arm/include/uapi/asm/kvm.h      | 13 ++++++++
 arch/arm/kvm/guest.c                 | 23 ++++++++++++++
 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               | 33 ++++++++++++++++++++
 arch/arm64/kvm/inject_fault.c        |  6 ++--
 arch/arm64/kvm/reset.c               |  4 +++
 include/uapi/linux/kvm.h             |  1 +
 virt/kvm/arm/arm.c                   | 45 ++++++++++++++++++++++++++++
 12 files changed, 192 insertions(+), 7 deletions(-)

-- 
2.18.0

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

* [PATCH v8 1/4] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
  2018-07-13 15:47 ` James Morse
@ 2018-07-13 15:47   ` James Morse
  -1 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-13 15:47 UTC (permalink / raw)
  To: kvmarm; +Cc: kvm, Marc Zyngier, linuxarm, gengdongjiu, linux-arm-kernel

From: Dongjiu Geng <gengdongjiu@huawei.com>

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>
Reviewed-by: James Morse <james.morse@arm.com>
Signed-off-by: James Morse <james.morse@arm.com>
---
 Documentation/virtual/kvm/api.txt    | 33 +++++++++++++++++---
 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               | 46 ++++++++++++++++++++++++++++
 arch/arm64/kvm/inject_fault.c        |  6 ++--
 arch/arm64/kvm/reset.c               |  1 +
 virt/kvm/arm/arm.c                   | 21 +++++++++++++
 8 files changed, 125 insertions(+), 7 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index d10944e619d3..e3940f8715a5 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
-Type: vm ioctl
+Architectures: x86, arm64
+Type: vcpu 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.
 
+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
 Extended by: KVM_CAP_INTR_SHADOW
-Architectures: x86
-Type: vm ioctl
+Architectures: x86, arm64
+Type: vcpu 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.
 
+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/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 1dab3a984608..18f61fff2cf9 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 fe8777b12f86..45a384b0a78a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -350,6 +350,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);
@@ -378,6 +383,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 4e76630dd655..97c3478ee6e7 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
 
@@ -154,6 +155,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 56a0260ceb11..dd05be96d981 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -289,6 +289,52 @@ 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)
+{
+	memset(events, 0, sizeof(*events));
+
+	events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
+	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);
+
+	return 0;
+}
+
+int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
+			struct kvm_vcpu_events *events)
+{
+	int i;
+	bool serror_pending = events->exception.serror_pending;
+	bool has_esr = events->exception.serror_has_esr;
+
+	/* check whether the reserved field is zero */
+	for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
+		if (events->reserved[i])
+			return -EINVAL;
+
+	/* check whether the pad field is zero */
+	for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
+		if (events->exception.pad[i])
+			return -EINVAL;
+
+	if (serror_pending && has_esr) {
+		if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
+			return -EINVAL;
+
+		if (!((events->exception.serror_esr) & ~ESR_ELx_ISS_MASK))
+			kvm_set_sei_esr(vcpu, events->exception.serror_esr);
+		else
+			return -EINVAL;
+	} 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 d8e71659ba7e..a55e91dfcf8f 100644
--- a/arch/arm64/kvm/inject_fault.c
+++ b/arch/arm64/kvm/inject_fault.c
@@ -164,9 +164,9 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
 		inject_undef64(vcpu);
 }
 
-static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
+void kvm_set_sei_esr(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;
 }
 
@@ -184,5 +184,5 @@ static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
  */
 void kvm_inject_vabt(struct kvm_vcpu *vcpu)
 {
-	pend_guest_serror(vcpu, ESR_ELx_ISV);
+	kvm_set_sei_esr(vcpu, ESR_ELx_ISV);
 }
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index a74311beda35..a3db01a28062 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -79,6 +79,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 04e554cae3a2..a94eab71e5c7 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1124,6 +1124,27 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = kvm_arm_vcpu_has_attr(vcpu, &attr);
 		break;
 	}
+#ifdef __KVM_HAVE_VCPU_EVENTS
+	case KVM_GET_VCPU_EVENTS: {
+		struct kvm_vcpu_events 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(events)))
+			return -EFAULT;
+
+		return kvm_arm_vcpu_set_events(vcpu, &events);
+	}
+#endif
 	default:
 		r = -EINVAL;
 	}
-- 
2.18.0

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

* [PATCH v8 1/4] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
@ 2018-07-13 15:47   ` James Morse
  0 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-13 15:47 UTC (permalink / raw)
  To: linux-arm-kernel

From: Dongjiu Geng <gengdongjiu@huawei.com>

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>
Reviewed-by: James Morse <james.morse@arm.com>
Signed-off-by: James Morse <james.morse@arm.com>
---
 Documentation/virtual/kvm/api.txt    | 33 +++++++++++++++++---
 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               | 46 ++++++++++++++++++++++++++++
 arch/arm64/kvm/inject_fault.c        |  6 ++--
 arch/arm64/kvm/reset.c               |  1 +
 virt/kvm/arm/arm.c                   | 21 +++++++++++++
 8 files changed, 125 insertions(+), 7 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index d10944e619d3..e3940f8715a5 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
-Type: vm ioctl
+Architectures: x86, arm64
+Type: vcpu 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.
 
+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
 Extended by: KVM_CAP_INTR_SHADOW
-Architectures: x86
-Type: vm ioctl
+Architectures: x86, arm64
+Type: vcpu 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.
 
+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/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 1dab3a984608..18f61fff2cf9 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 fe8777b12f86..45a384b0a78a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -350,6 +350,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);
@@ -378,6 +383,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 4e76630dd655..97c3478ee6e7 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
 
@@ -154,6 +155,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 56a0260ceb11..dd05be96d981 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -289,6 +289,52 @@ 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)
+{
+	memset(events, 0, sizeof(*events));
+
+	events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
+	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);
+
+	return 0;
+}
+
+int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
+			struct kvm_vcpu_events *events)
+{
+	int i;
+	bool serror_pending = events->exception.serror_pending;
+	bool has_esr = events->exception.serror_has_esr;
+
+	/* check whether the reserved field is zero */
+	for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
+		if (events->reserved[i])
+			return -EINVAL;
+
+	/* check whether the pad field is zero */
+	for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
+		if (events->exception.pad[i])
+			return -EINVAL;
+
+	if (serror_pending && has_esr) {
+		if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
+			return -EINVAL;
+
+		if (!((events->exception.serror_esr) & ~ESR_ELx_ISS_MASK))
+			kvm_set_sei_esr(vcpu, events->exception.serror_esr);
+		else
+			return -EINVAL;
+	} 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 d8e71659ba7e..a55e91dfcf8f 100644
--- a/arch/arm64/kvm/inject_fault.c
+++ b/arch/arm64/kvm/inject_fault.c
@@ -164,9 +164,9 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
 		inject_undef64(vcpu);
 }
 
-static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
+void kvm_set_sei_esr(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;
 }
 
@@ -184,5 +184,5 @@ static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
  */
 void kvm_inject_vabt(struct kvm_vcpu *vcpu)
 {
-	pend_guest_serror(vcpu, ESR_ELx_ISV);
+	kvm_set_sei_esr(vcpu, ESR_ELx_ISV);
 }
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index a74311beda35..a3db01a28062 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -79,6 +79,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 04e554cae3a2..a94eab71e5c7 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1124,6 +1124,27 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = kvm_arm_vcpu_has_attr(vcpu, &attr);
 		break;
 	}
+#ifdef __KVM_HAVE_VCPU_EVENTS
+	case KVM_GET_VCPU_EVENTS: {
+		struct kvm_vcpu_events 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(events)))
+			return -EFAULT;
+
+		return kvm_arm_vcpu_set_events(vcpu, &events);
+	}
+#endif
 	default:
 		r = -EINVAL;
 	}
-- 
2.18.0

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

* [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
  2018-07-13 15:47 ` James Morse
@ 2018-07-13 15:47   ` James Morse
  -1 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-13 15:47 UTC (permalink / raw)
  To: kvmarm; +Cc: kvm, Marc Zyngier, linuxarm, gengdongjiu, linux-arm-kernel

From: Dongjiu Geng <gengdongjiu@huawei.com>

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>
Signed-off-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 e3940f8715a5..480fa64eb52a 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -4643,3 +4643,14 @@ This capability indicates that KVM supports paravirtualized Hyper-V TLB Flush
 hypercalls:
 HvFlushVirtualAddressSpace, HvFlushVirtualAddressSpaceEx,
 HvFlushVirtualAddressList, HvFlushVirtualAddressListEx.
+
+8.19 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 a3db01a28062..067c6ba969bd 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:
 	case KVM_CAP_VCPU_EVENTS:
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index b6270a3b38e9..a7d9bc4e4068 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -949,6 +949,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_GET_MSR_FEATURES 153
 #define KVM_CAP_HYPERV_EVENTFD 154
 #define KVM_CAP_HYPERV_TLBFLUSH 155
+#define KVM_CAP_ARM_INJECT_SERROR_ESR 156
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.18.0

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

* [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
@ 2018-07-13 15:47   ` James Morse
  0 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-13 15:47 UTC (permalink / raw)
  To: linux-arm-kernel

From: Dongjiu Geng <gengdongjiu@huawei.com>

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>
Signed-off-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 e3940f8715a5..480fa64eb52a 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -4643,3 +4643,14 @@ This capability indicates that KVM supports paravirtualized Hyper-V TLB Flush
 hypercalls:
 HvFlushVirtualAddressSpace, HvFlushVirtualAddressSpaceEx,
 HvFlushVirtualAddressList, HvFlushVirtualAddressListEx.
+
+8.19 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 a3db01a28062..067c6ba969bd 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:
 	case KVM_CAP_VCPU_EVENTS:
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index b6270a3b38e9..a7d9bc4e4068 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -949,6 +949,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_GET_MSR_FEATURES 153
 #define KVM_CAP_HYPERV_EVENTFD 154
 #define KVM_CAP_HYPERV_TLBFLUSH 155
+#define KVM_CAP_ARM_INJECT_SERROR_ESR 156
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.18.0

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

* [PATCH v8 3/4] KVM: arm64: Share the parts of get/set events useful to 32bit
  2018-07-13 15:47 ` James Morse
@ 2018-07-13 15:47   ` James Morse
  -1 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-13 15:47 UTC (permalink / raw)
  To: kvmarm; +Cc: kvm, Marc Zyngier, linuxarm, gengdongjiu, linux-arm-kernel

The get/set events helpers do some work to check reserved
and padding fields are zero. This is useful on 32bit too.

Move this code into virt/kvm/arm/arm.c, and give the arch
code some underscores.

This is temporarily hidden behind __KVM_HAVE_VCPU_EVENTS until
32bit is wired up.

Signed-off-by: James Morse <james.morse@arm.com>
CC: Dongjiu Geng <gengdongjiu@huawei.com>
---
 arch/arm64/include/asm/kvm_host.h |  8 ++++----
 arch/arm64/kvm/guest.c            | 21 ++++-----------------
 virt/kvm/arm/arm.c                | 28 ++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 45a384b0a78a..66d09b44ebd8 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -350,11 +350,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_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);
+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);
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index dd05be96d981..725c7545e91a 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -289,11 +289,9 @@ 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)
+int __kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
+			      struct kvm_vcpu_events *events)
 {
-	memset(events, 0, sizeof(*events));
-
 	events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
 	events->exception.serror_has_esr = cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
 
@@ -303,23 +301,12 @@ int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
 	return 0;
 }
 
-int kvm_arm_vcpu_set_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)
 {
-	int i;
 	bool serror_pending = events->exception.serror_pending;
 	bool has_esr = events->exception.serror_has_esr;
 
-	/* check whether the reserved field is zero */
-	for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
-		if (events->reserved[i])
-			return -EINVAL;
-
-	/* check whether the pad field is zero */
-	for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
-		if (events->exception.pad[i])
-			return -EINVAL;
-
 	if (serror_pending && has_esr) {
 		if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
 			return -EINVAL;
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index a94eab71e5c7..f70d24e1751d 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1044,6 +1044,34 @@ static int kvm_arm_vcpu_has_attr(struct kvm_vcpu *vcpu,
 	return ret;
 }
 
+#ifdef __KVM_HAVE_VCPU_EVENTS	/* temporary: until 32bit is wired up */
+static int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
+				   struct kvm_vcpu_events *events)
+{
+	memset(events, 0, sizeof(*events));
+
+	return __kvm_arm_vcpu_get_events(vcpu, events);
+}
+
+static int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
+				   struct kvm_vcpu_events *events)
+{
+	int i;
+
+	/* check whether the reserved field is zero */
+	for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
+		if (events->reserved[i])
+			return -EINVAL;
+
+	/* check whether the pad field is zero */
+	for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
+		if (events->exception.pad[i])
+			return -EINVAL;
+
+	return __kvm_arm_vcpu_set_events(vcpu, events);
+}
+#endif /* __KVM_HAVE_VCPU_EVENTS */
+
 long kvm_arch_vcpu_ioctl(struct file *filp,
 			 unsigned int ioctl, unsigned long arg)
 {
-- 
2.18.0

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

* [PATCH v8 3/4] KVM: arm64: Share the parts of get/set events useful to 32bit
@ 2018-07-13 15:47   ` James Morse
  0 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-13 15:47 UTC (permalink / raw)
  To: linux-arm-kernel

The get/set events helpers do some work to check reserved
and padding fields are zero. This is useful on 32bit too.

Move this code into virt/kvm/arm/arm.c, and give the arch
code some underscores.

This is temporarily hidden behind __KVM_HAVE_VCPU_EVENTS until
32bit is wired up.

Signed-off-by: James Morse <james.morse@arm.com>
CC: Dongjiu Geng <gengdongjiu@huawei.com>
---
 arch/arm64/include/asm/kvm_host.h |  8 ++++----
 arch/arm64/kvm/guest.c            | 21 ++++-----------------
 virt/kvm/arm/arm.c                | 28 ++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 45a384b0a78a..66d09b44ebd8 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -350,11 +350,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_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);
+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);
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index dd05be96d981..725c7545e91a 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -289,11 +289,9 @@ 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)
+int __kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
+			      struct kvm_vcpu_events *events)
 {
-	memset(events, 0, sizeof(*events));
-
 	events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
 	events->exception.serror_has_esr = cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
 
@@ -303,23 +301,12 @@ int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
 	return 0;
 }
 
-int kvm_arm_vcpu_set_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)
 {
-	int i;
 	bool serror_pending = events->exception.serror_pending;
 	bool has_esr = events->exception.serror_has_esr;
 
-	/* check whether the reserved field is zero */
-	for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
-		if (events->reserved[i])
-			return -EINVAL;
-
-	/* check whether the pad field is zero */
-	for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
-		if (events->exception.pad[i])
-			return -EINVAL;
-
 	if (serror_pending && has_esr) {
 		if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
 			return -EINVAL;
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index a94eab71e5c7..f70d24e1751d 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1044,6 +1044,34 @@ static int kvm_arm_vcpu_has_attr(struct kvm_vcpu *vcpu,
 	return ret;
 }
 
+#ifdef __KVM_HAVE_VCPU_EVENTS	/* temporary: until 32bit is wired up */
+static int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
+				   struct kvm_vcpu_events *events)
+{
+	memset(events, 0, sizeof(*events));
+
+	return __kvm_arm_vcpu_get_events(vcpu, events);
+}
+
+static int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
+				   struct kvm_vcpu_events *events)
+{
+	int i;
+
+	/* check whether the reserved field is zero */
+	for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
+		if (events->reserved[i])
+			return -EINVAL;
+
+	/* check whether the pad field is zero */
+	for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
+		if (events->exception.pad[i])
+			return -EINVAL;
+
+	return __kvm_arm_vcpu_set_events(vcpu, events);
+}
+#endif /* __KVM_HAVE_VCPU_EVENTS */
+
 long kvm_arch_vcpu_ioctl(struct file *filp,
 			 unsigned int ioctl, unsigned long arg)
 {
-- 
2.18.0

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

* [PATCH v8 4/4] KVM: arm: Add 32bit get/set events support
  2018-07-13 15:47 ` James Morse
@ 2018-07-13 15:47   ` James Morse
  -1 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-13 15:47 UTC (permalink / raw)
  To: kvmarm; +Cc: kvm, Marc Zyngier, linuxarm, gengdongjiu, linux-arm-kernel

arm64's new use of KVMs get_events/set_events API calls isn't just
for RAS, it allows an SError that has been made pending by KVM as
part of its device emulation to be migrated.

Wire this up for 32bit too.

We only need to read/write the HCR_VA bit, and check that no esr has
been provided, as we don't yet support VDFSR.

Signed-off-by: James Morse <james.morse@arm.com>
CC: Dongjiu Geng <gengdongjiu@huawei.com>
---
Changes since v7:
 * Removed temporary #ifdefs
 * Added 'and 32bit' to the the documentation.
---
 Documentation/virtual/kvm/api.txt |  8 ++++----
 arch/arm/include/asm/kvm_host.h   |  5 +++++
 arch/arm/include/uapi/asm/kvm.h   | 13 +++++++++++++
 arch/arm/kvm/guest.c              | 23 +++++++++++++++++++++++
 virt/kvm/arm/arm.c                |  4 ----
 5 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 480fa64eb52a..209151554547 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -835,7 +835,7 @@ struct kvm_clock_data {
 
 Capability: KVM_CAP_VCPU_EVENTS
 Extended by: KVM_CAP_INTR_SHADOW
-Architectures: x86, arm64
+Architectures: x86, arm, arm64
 Type: vcpu ioctl
 Parameters: struct kvm_vcpu_event (out)
 Returns: 0 on success, -1 on error
@@ -883,7 +883,7 @@ 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.
 
-ARM64:
+ARM/ARM64:
 
 Gets currently pending SError exceptions as well as related states of the vcpu.
 
@@ -902,7 +902,7 @@ struct kvm_vcpu_events {
 
 Capability: KVM_CAP_VCPU_EVENTS
 Extended by: KVM_CAP_INTR_SHADOW
-Architectures: x86, arm64
+Architectures: x86, arm, arm64
 Type: vcpu ioctl
 Parameters: struct kvm_vcpu_event (in)
 Returns: 0 on success, -1 on error
@@ -929,7 +929,7 @@ shall be written into the VCPU.
 
 KVM_VCPUEVENT_VALID_SMM can only be set if KVM_CAP_X86_SMM is available.
 
-ARM64:
+ARM/ARM64:
 
 Set pending SError exceptions as well as related states of the vcpu.
 
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 1f1fe4109b02..79906cecb091 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -216,6 +216,11 @@ 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);
 unsigned long kvm_call_hyp(void *hypfn, ...);
 void force_vm_exit(const cpumask_t *mask);
+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);
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index 16e006f708ca..4602464ebdfb 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -27,6 +27,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
 
@@ -125,6 +126,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/arm/kvm/guest.c b/arch/arm/kvm/guest.c
index a18f33edc471..2b8de885b2bf 100644
--- a/arch/arm/kvm/guest.c
+++ b/arch/arm/kvm/guest.c
@@ -261,6 +261,29 @@ 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_hcr(vcpu) & HCR_VA);
+
+	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)
+		return -EINVAL;
+	else if (serror_pending)
+		kvm_inject_vabt(vcpu);
+
+	return 0;
+}
+
 int __attribute_const__ kvm_target_cpu(void)
 {
 	switch (read_cpuid_part()) {
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index f70d24e1751d..da7cabf8980e 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1044,7 +1044,6 @@ static int kvm_arm_vcpu_has_attr(struct kvm_vcpu *vcpu,
 	return ret;
 }
 
-#ifdef __KVM_HAVE_VCPU_EVENTS	/* temporary: until 32bit is wired up */
 static int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
 				   struct kvm_vcpu_events *events)
 {
@@ -1070,7 +1069,6 @@ static int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
 
 	return __kvm_arm_vcpu_set_events(vcpu, events);
 }
-#endif /* __KVM_HAVE_VCPU_EVENTS */
 
 long kvm_arch_vcpu_ioctl(struct file *filp,
 			 unsigned int ioctl, unsigned long arg)
@@ -1152,7 +1150,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = kvm_arm_vcpu_has_attr(vcpu, &attr);
 		break;
 	}
-#ifdef __KVM_HAVE_VCPU_EVENTS
 	case KVM_GET_VCPU_EVENTS: {
 		struct kvm_vcpu_events events;
 
@@ -1172,7 +1169,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 
 		return kvm_arm_vcpu_set_events(vcpu, &events);
 	}
-#endif
 	default:
 		r = -EINVAL;
 	}
-- 
2.18.0

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

* [PATCH v8 4/4] KVM: arm: Add 32bit get/set events support
@ 2018-07-13 15:47   ` James Morse
  0 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-13 15:47 UTC (permalink / raw)
  To: linux-arm-kernel

arm64's new use of KVMs get_events/set_events API calls isn't just
for RAS, it allows an SError that has been made pending by KVM as
part of its device emulation to be migrated.

Wire this up for 32bit too.

We only need to read/write the HCR_VA bit, and check that no esr has
been provided, as we don't yet support VDFSR.

Signed-off-by: James Morse <james.morse@arm.com>
CC: Dongjiu Geng <gengdongjiu@huawei.com>
---
Changes since v7:
 * Removed temporary #ifdefs
 * Added 'and 32bit' to the the documentation.
---
 Documentation/virtual/kvm/api.txt |  8 ++++----
 arch/arm/include/asm/kvm_host.h   |  5 +++++
 arch/arm/include/uapi/asm/kvm.h   | 13 +++++++++++++
 arch/arm/kvm/guest.c              | 23 +++++++++++++++++++++++
 virt/kvm/arm/arm.c                |  4 ----
 5 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 480fa64eb52a..209151554547 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -835,7 +835,7 @@ struct kvm_clock_data {
 
 Capability: KVM_CAP_VCPU_EVENTS
 Extended by: KVM_CAP_INTR_SHADOW
-Architectures: x86, arm64
+Architectures: x86, arm, arm64
 Type: vcpu ioctl
 Parameters: struct kvm_vcpu_event (out)
 Returns: 0 on success, -1 on error
@@ -883,7 +883,7 @@ 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.
 
-ARM64:
+ARM/ARM64:
 
 Gets currently pending SError exceptions as well as related states of the vcpu.
 
@@ -902,7 +902,7 @@ struct kvm_vcpu_events {
 
 Capability: KVM_CAP_VCPU_EVENTS
 Extended by: KVM_CAP_INTR_SHADOW
-Architectures: x86, arm64
+Architectures: x86, arm, arm64
 Type: vcpu ioctl
 Parameters: struct kvm_vcpu_event (in)
 Returns: 0 on success, -1 on error
@@ -929,7 +929,7 @@ shall be written into the VCPU.
 
 KVM_VCPUEVENT_VALID_SMM can only be set if KVM_CAP_X86_SMM is available.
 
-ARM64:
+ARM/ARM64:
 
 Set pending SError exceptions as well as related states of the vcpu.
 
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 1f1fe4109b02..79906cecb091 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -216,6 +216,11 @@ 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);
 unsigned long kvm_call_hyp(void *hypfn, ...);
 void force_vm_exit(const cpumask_t *mask);
+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);
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index 16e006f708ca..4602464ebdfb 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -27,6 +27,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
 
@@ -125,6 +126,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/arm/kvm/guest.c b/arch/arm/kvm/guest.c
index a18f33edc471..2b8de885b2bf 100644
--- a/arch/arm/kvm/guest.c
+++ b/arch/arm/kvm/guest.c
@@ -261,6 +261,29 @@ 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_hcr(vcpu) & HCR_VA);
+
+	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)
+		return -EINVAL;
+	else if (serror_pending)
+		kvm_inject_vabt(vcpu);
+
+	return 0;
+}
+
 int __attribute_const__ kvm_target_cpu(void)
 {
 	switch (read_cpuid_part()) {
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index f70d24e1751d..da7cabf8980e 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1044,7 +1044,6 @@ static int kvm_arm_vcpu_has_attr(struct kvm_vcpu *vcpu,
 	return ret;
 }
 
-#ifdef __KVM_HAVE_VCPU_EVENTS	/* temporary: until 32bit is wired up */
 static int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
 				   struct kvm_vcpu_events *events)
 {
@@ -1070,7 +1069,6 @@ static int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
 
 	return __kvm_arm_vcpu_set_events(vcpu, events);
 }
-#endif /* __KVM_HAVE_VCPU_EVENTS */
 
 long kvm_arch_vcpu_ioctl(struct file *filp,
 			 unsigned int ioctl, unsigned long arg)
@@ -1152,7 +1150,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = kvm_arm_vcpu_has_attr(vcpu, &attr);
 		break;
 	}
-#ifdef __KVM_HAVE_VCPU_EVENTS
 	case KVM_GET_VCPU_EVENTS: {
 		struct kvm_vcpu_events events;
 
@@ -1172,7 +1169,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 
 		return kvm_arm_vcpu_set_events(vcpu, &events);
 	}
-#endif
 	default:
 		r = -EINVAL;
 	}
-- 
2.18.0

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

* Re: [PATCH v8 3/4] KVM: arm64: Share the parts of get/set events useful to 32bit
  2018-07-13 15:47   ` James Morse
  (?)
@ 2018-07-16  2:50     ` gengdongjiu
  -1 siblings, 0 replies; 36+ messages in thread
From: gengdongjiu @ 2018-07-16  2:50 UTC (permalink / raw)
  To: James Morse, kvmarm; +Cc: Marc Zyngier, linuxarm, linux-arm-kernel, kvm

On 2018/7/13 23:47, James Morse wrote:
> The get/set events helpers do some work to check reserved
> and padding fields are zero. This is useful on 32bit too.
> 
> Move this code into virt/kvm/arm/arm.c, and give the arch
> code some underscores.
> 
> This is temporarily hidden behind __KVM_HAVE_VCPU_EVENTS until
> 32bit is wired up.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> CC: Dongjiu Geng <gengdongjiu@huawei.com>

thanks!
Reviewed-by: Dongjiu Geng <gengdongjiu@huawei.com>

> ---
>  arch/arm64/include/asm/kvm_host.h |  8 ++++----
>  arch/arm64/kvm/guest.c            | 21 ++++-----------------
>  virt/kvm/arm/arm.c                | 28 ++++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 45a384b0a78a..66d09b44ebd8 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -350,11 +350,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_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);
> +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);
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index dd05be96d981..725c7545e91a 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -289,11 +289,9 @@ 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)
> +int __kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +			      struct kvm_vcpu_events *events)
>  {
> -	memset(events, 0, sizeof(*events));
> -
>  	events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
>  	events->exception.serror_has_esr = cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
>  
> @@ -303,23 +301,12 @@ int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
>  	return 0;
>  }
>  
> -int kvm_arm_vcpu_set_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)
>  {
> -	int i;
>  	bool serror_pending = events->exception.serror_pending;
>  	bool has_esr = events->exception.serror_has_esr;
>  
> -	/* check whether the reserved field is zero */
> -	for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
> -		if (events->reserved[i])
> -			return -EINVAL;
> -
> -	/* check whether the pad field is zero */
> -	for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
> -		if (events->exception.pad[i])
> -			return -EINVAL;
> -
>  	if (serror_pending && has_esr) {
>  		if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
>  			return -EINVAL;
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index a94eab71e5c7..f70d24e1751d 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1044,6 +1044,34 @@ static int kvm_arm_vcpu_has_attr(struct kvm_vcpu *vcpu,
>  	return ret;
>  }
>  
> +#ifdef __KVM_HAVE_VCPU_EVENTS	/* temporary: until 32bit is wired up */
> +static int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +				   struct kvm_vcpu_events *events)
> +{
> +	memset(events, 0, sizeof(*events));
> +
> +	return __kvm_arm_vcpu_get_events(vcpu, events);
> +}
> +
> +static int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +				   struct kvm_vcpu_events *events)
> +{
> +	int i;
> +
> +	/* check whether the reserved field is zero */
> +	for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
> +		if (events->reserved[i])
> +			return -EINVAL;
> +
> +	/* check whether the pad field is zero */
> +	for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
> +		if (events->exception.pad[i])
> +			return -EINVAL;
> +
> +	return __kvm_arm_vcpu_set_events(vcpu, events);
> +}
> +#endif /* __KVM_HAVE_VCPU_EVENTS */


> +
>  long kvm_arch_vcpu_ioctl(struct file *filp,
>  			 unsigned int ioctl, unsigned long arg)
>  {
> 

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

* Re: [PATCH v8 3/4] KVM: arm64: Share the parts of get/set events useful to 32bit
@ 2018-07-16  2:50     ` gengdongjiu
  0 siblings, 0 replies; 36+ messages in thread
From: gengdongjiu @ 2018-07-16  2:50 UTC (permalink / raw)
  To: James Morse, kvmarm; +Cc: Marc Zyngier, linuxarm, linux-arm-kernel, kvm

On 2018/7/13 23:47, James Morse wrote:
> The get/set events helpers do some work to check reserved
> and padding fields are zero. This is useful on 32bit too.
> 
> Move this code into virt/kvm/arm/arm.c, and give the arch
> code some underscores.
> 
> This is temporarily hidden behind __KVM_HAVE_VCPU_EVENTS until
> 32bit is wired up.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> CC: Dongjiu Geng <gengdongjiu@huawei.com>

thanks!
Reviewed-by: Dongjiu Geng <gengdongjiu@huawei.com>

> ---
>  arch/arm64/include/asm/kvm_host.h |  8 ++++----
>  arch/arm64/kvm/guest.c            | 21 ++++-----------------
>  virt/kvm/arm/arm.c                | 28 ++++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 45a384b0a78a..66d09b44ebd8 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -350,11 +350,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_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);
> +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);
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index dd05be96d981..725c7545e91a 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -289,11 +289,9 @@ 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)
> +int __kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +			      struct kvm_vcpu_events *events)
>  {
> -	memset(events, 0, sizeof(*events));
> -
>  	events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
>  	events->exception.serror_has_esr = cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
>  
> @@ -303,23 +301,12 @@ int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
>  	return 0;
>  }
>  
> -int kvm_arm_vcpu_set_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)
>  {
> -	int i;
>  	bool serror_pending = events->exception.serror_pending;
>  	bool has_esr = events->exception.serror_has_esr;
>  
> -	/* check whether the reserved field is zero */
> -	for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
> -		if (events->reserved[i])
> -			return -EINVAL;
> -
> -	/* check whether the pad field is zero */
> -	for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
> -		if (events->exception.pad[i])
> -			return -EINVAL;
> -
>  	if (serror_pending && has_esr) {
>  		if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
>  			return -EINVAL;
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index a94eab71e5c7..f70d24e1751d 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1044,6 +1044,34 @@ static int kvm_arm_vcpu_has_attr(struct kvm_vcpu *vcpu,
>  	return ret;
>  }
>  
> +#ifdef __KVM_HAVE_VCPU_EVENTS	/* temporary: until 32bit is wired up */
> +static int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +				   struct kvm_vcpu_events *events)
> +{
> +	memset(events, 0, sizeof(*events));
> +
> +	return __kvm_arm_vcpu_get_events(vcpu, events);
> +}
> +
> +static int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +				   struct kvm_vcpu_events *events)
> +{
> +	int i;
> +
> +	/* check whether the reserved field is zero */
> +	for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
> +		if (events->reserved[i])
> +			return -EINVAL;
> +
> +	/* check whether the pad field is zero */
> +	for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
> +		if (events->exception.pad[i])
> +			return -EINVAL;
> +
> +	return __kvm_arm_vcpu_set_events(vcpu, events);
> +}
> +#endif /* __KVM_HAVE_VCPU_EVENTS */


> +
>  long kvm_arch_vcpu_ioctl(struct file *filp,
>  			 unsigned int ioctl, unsigned long arg)
>  {
> 

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

* [PATCH v8 3/4] KVM: arm64: Share the parts of get/set events useful to 32bit
@ 2018-07-16  2:50     ` gengdongjiu
  0 siblings, 0 replies; 36+ messages in thread
From: gengdongjiu @ 2018-07-16  2:50 UTC (permalink / raw)
  To: linux-arm-kernel

On 2018/7/13 23:47, James Morse wrote:
> The get/set events helpers do some work to check reserved
> and padding fields are zero. This is useful on 32bit too.
> 
> Move this code into virt/kvm/arm/arm.c, and give the arch
> code some underscores.
> 
> This is temporarily hidden behind __KVM_HAVE_VCPU_EVENTS until
> 32bit is wired up.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> CC: Dongjiu Geng <gengdongjiu@huawei.com>

thanks!
Reviewed-by: Dongjiu Geng <gengdongjiu@huawei.com>

> ---
>  arch/arm64/include/asm/kvm_host.h |  8 ++++----
>  arch/arm64/kvm/guest.c            | 21 ++++-----------------
>  virt/kvm/arm/arm.c                | 28 ++++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 45a384b0a78a..66d09b44ebd8 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -350,11 +350,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_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);
> +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);
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index dd05be96d981..725c7545e91a 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -289,11 +289,9 @@ 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)
> +int __kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +			      struct kvm_vcpu_events *events)
>  {
> -	memset(events, 0, sizeof(*events));
> -
>  	events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
>  	events->exception.serror_has_esr = cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
>  
> @@ -303,23 +301,12 @@ int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
>  	return 0;
>  }
>  
> -int kvm_arm_vcpu_set_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)
>  {
> -	int i;
>  	bool serror_pending = events->exception.serror_pending;
>  	bool has_esr = events->exception.serror_has_esr;
>  
> -	/* check whether the reserved field is zero */
> -	for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
> -		if (events->reserved[i])
> -			return -EINVAL;
> -
> -	/* check whether the pad field is zero */
> -	for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
> -		if (events->exception.pad[i])
> -			return -EINVAL;
> -
>  	if (serror_pending && has_esr) {
>  		if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
>  			return -EINVAL;
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index a94eab71e5c7..f70d24e1751d 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1044,6 +1044,34 @@ static int kvm_arm_vcpu_has_attr(struct kvm_vcpu *vcpu,
>  	return ret;
>  }
>  
> +#ifdef __KVM_HAVE_VCPU_EVENTS	/* temporary: until 32bit is wired up */
> +static int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +				   struct kvm_vcpu_events *events)
> +{
> +	memset(events, 0, sizeof(*events));
> +
> +	return __kvm_arm_vcpu_get_events(vcpu, events);
> +}
> +
> +static int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +				   struct kvm_vcpu_events *events)
> +{
> +	int i;
> +
> +	/* check whether the reserved field is zero */
> +	for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
> +		if (events->reserved[i])
> +			return -EINVAL;
> +
> +	/* check whether the pad field is zero */
> +	for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
> +		if (events->exception.pad[i])
> +			return -EINVAL;
> +
> +	return __kvm_arm_vcpu_set_events(vcpu, events);
> +}
> +#endif /* __KVM_HAVE_VCPU_EVENTS */


> +
>  long kvm_arch_vcpu_ioctl(struct file *filp,
>  			 unsigned int ioctl, unsigned long arg)
>  {
> 

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

* Re: [PATCH v8 4/4] KVM: arm: Add 32bit get/set events support
  2018-07-13 15:47   ` James Morse
  (?)
@ 2018-07-16  3:00     ` gengdongjiu
  -1 siblings, 0 replies; 36+ messages in thread
From: gengdongjiu @ 2018-07-16  3:00 UTC (permalink / raw)
  To: James Morse, kvmarm; +Cc: Marc Zyngier, linuxarm, linux-arm-kernel, kvm



On 2018/7/13 23:47, James Morse wrote:
> arm64's new use of KVMs get_events/set_events API calls isn't just
> for RAS, it allows an SError that has been made pending by KVM as
> part of its device emulation to be migrated.
> 
> Wire this up for 32bit too.
> 
> We only need to read/write the HCR_VA bit, and check that no esr has
> been provided, as we don't yet support VDFSR.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> CC: Dongjiu Geng <gengdongjiu@huawei.com>

Thanks!
Reviewed-by: Dongjiu Geng <gengdongjiu@huawei.com>

> ---
> Changes since v7:
>  * Removed temporary #ifdefs
>  * Added 'and 32bit' to the the documentation.
> ---
>  Documentation/virtual/kvm/api.txt |  8 ++++----
>  arch/arm/include/asm/kvm_host.h   |  5 +++++

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

* Re: [PATCH v8 4/4] KVM: arm: Add 32bit get/set events support
@ 2018-07-16  3:00     ` gengdongjiu
  0 siblings, 0 replies; 36+ messages in thread
From: gengdongjiu @ 2018-07-16  3:00 UTC (permalink / raw)
  To: James Morse, kvmarm; +Cc: Marc Zyngier, linuxarm, linux-arm-kernel, kvm



On 2018/7/13 23:47, James Morse wrote:
> arm64's new use of KVMs get_events/set_events API calls isn't just
> for RAS, it allows an SError that has been made pending by KVM as
> part of its device emulation to be migrated.
> 
> Wire this up for 32bit too.
> 
> We only need to read/write the HCR_VA bit, and check that no esr has
> been provided, as we don't yet support VDFSR.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> CC: Dongjiu Geng <gengdongjiu@huawei.com>

Thanks!
Reviewed-by: Dongjiu Geng <gengdongjiu@huawei.com>

> ---
> Changes since v7:
>  * Removed temporary #ifdefs
>  * Added 'and 32bit' to the the documentation.
> ---
>  Documentation/virtual/kvm/api.txt |  8 ++++----
>  arch/arm/include/asm/kvm_host.h   |  5 +++++

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

* [PATCH v8 4/4] KVM: arm: Add 32bit get/set events support
@ 2018-07-16  3:00     ` gengdongjiu
  0 siblings, 0 replies; 36+ messages in thread
From: gengdongjiu @ 2018-07-16  3:00 UTC (permalink / raw)
  To: linux-arm-kernel



On 2018/7/13 23:47, James Morse wrote:
> arm64's new use of KVMs get_events/set_events API calls isn't just
> for RAS, it allows an SError that has been made pending by KVM as
> part of its device emulation to be migrated.
> 
> Wire this up for 32bit too.
> 
> We only need to read/write the HCR_VA bit, and check that no esr has
> been provided, as we don't yet support VDFSR.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> CC: Dongjiu Geng <gengdongjiu@huawei.com>

Thanks!
Reviewed-by: Dongjiu Geng <gengdongjiu@huawei.com>

> ---
> Changes since v7:
>  * Removed temporary #ifdefs
>  * Added 'and 32bit' to the the documentation.
> ---
>  Documentation/virtual/kvm/api.txt |  8 ++++----
>  arch/arm/include/asm/kvm_host.h   |  5 +++++

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

* Re: [PATCH v8 1/4] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
  2018-07-13 15:47   ` James Morse
@ 2018-07-17 16:33     ` Peter Maydell
  -1 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2018-07-17 16:33 UTC (permalink / raw)
  To: James Morse
  Cc: kvm-devel, Marc Zyngier, Linuxarm, gengdongjiu, kvmarm, arm-mail-list

On 13 July 2018 at 16:47, James Morse <james.morse@arm.com> wrote:
> From: Dongjiu Geng <gengdongjiu@huawei.com>
>
> 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>
> Reviewed-by: James Morse <james.morse@arm.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  Documentation/virtual/kvm/api.txt    | 33 +++++++++++++++++---
>  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               | 46 ++++++++++++++++++++++++++++
>  arch/arm64/kvm/inject_fault.c        |  6 ++--
>  arch/arm64/kvm/reset.c               |  1 +
>  virt/kvm/arm/arm.c                   | 21 +++++++++++++
>  8 files changed, 125 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index d10944e619d3..e3940f8715a5 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
> -Type: vm ioctl
> +Architectures: x86, arm64
> +Type: vcpu 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.
>
> +ARM64:
> +
> +Gets currently pending SError exceptions as well as related states of the vcpu.

Any chance of expanding this to explain what this state actually is?
(ie something you could use to implement or review the userspace
code which has to manipulate it).

> +
> +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

thanks
-- PMM

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

* [PATCH v8 1/4] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
@ 2018-07-17 16:33     ` Peter Maydell
  0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2018-07-17 16:33 UTC (permalink / raw)
  To: linux-arm-kernel

On 13 July 2018 at 16:47, James Morse <james.morse@arm.com> wrote:
> From: Dongjiu Geng <gengdongjiu@huawei.com>
>
> 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>
> Reviewed-by: James Morse <james.morse@arm.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  Documentation/virtual/kvm/api.txt    | 33 +++++++++++++++++---
>  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               | 46 ++++++++++++++++++++++++++++
>  arch/arm64/kvm/inject_fault.c        |  6 ++--
>  arch/arm64/kvm/reset.c               |  1 +
>  virt/kvm/arm/arm.c                   | 21 +++++++++++++
>  8 files changed, 125 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index d10944e619d3..e3940f8715a5 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
> -Type: vm ioctl
> +Architectures: x86, arm64
> +Type: vcpu 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.
>
> +ARM64:
> +
> +Gets currently pending SError exceptions as well as related states of the vcpu.

Any chance of expanding this to explain what this state actually is?
(ie something you could use to implement or review the userspace
code which has to manipulate it).

> +
> +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

thanks
-- PMM

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

* Re: [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
  2018-07-13 15:47   ` James Morse
@ 2018-07-17 16:36     ` Peter Maydell
  -1 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2018-07-17 16:36 UTC (permalink / raw)
  To: James Morse
  Cc: kvm-devel, Marc Zyngier, Linuxarm, gengdongjiu, kvmarm, arm-mail-list

On 13 July 2018 at 16:47, James Morse <james.morse@arm.com> wrote:
> From: Dongjiu Geng <gengdongjiu@huawei.com>
>
> 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>
> Signed-off-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 e3940f8715a5..480fa64eb52a 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -4643,3 +4643,14 @@ This capability indicates that KVM supports paravirtualized Hyper-V TLB Flush
>  hypercalls:
>  HvFlushVirtualAddressSpace, HvFlushVirtualAddressSpaceEx,
>  HvFlushVirtualAddressList, HvFlushVirtualAddressListEx.
> +
> +8.19 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.

"the guest"

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

Full stop or semicolon, not comma.
Not sure what the "which is not under control by KVM" part means.

> +If this virtual SError is taken to EL1 using AArch64, this value will be reported
> +in ISS filed of ESR_EL1.

"the ISS field".

How does this capability interact with the KVM_CAP_VCPU_EVENTS
API in patch 1? Is it saying "you can specify the ESR via
that API", or talking about something else?

What happens if userspace does try to specify the EC field value ?
Is this an error, or is it ignored ?

> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index a3db01a28062..067c6ba969bd 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:
>         case KVM_CAP_VCPU_EVENTS:
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index b6270a3b38e9..a7d9bc4e4068 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -949,6 +949,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_GET_MSR_FEATURES 153
>  #define KVM_CAP_HYPERV_EVENTFD 154
>  #define KVM_CAP_HYPERV_TLBFLUSH 155
> +#define KVM_CAP_ARM_INJECT_SERROR_ESR 156
>
>  #ifdef KVM_CAP_IRQ_ROUTING
>
> --
> 2.18.0
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

thanks
-- PMM

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

* [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
@ 2018-07-17 16:36     ` Peter Maydell
  0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2018-07-17 16:36 UTC (permalink / raw)
  To: linux-arm-kernel

On 13 July 2018 at 16:47, James Morse <james.morse@arm.com> wrote:
> From: Dongjiu Geng <gengdongjiu@huawei.com>
>
> 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>
> Signed-off-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 e3940f8715a5..480fa64eb52a 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -4643,3 +4643,14 @@ This capability indicates that KVM supports paravirtualized Hyper-V TLB Flush
>  hypercalls:
>  HvFlushVirtualAddressSpace, HvFlushVirtualAddressSpaceEx,
>  HvFlushVirtualAddressList, HvFlushVirtualAddressListEx.
> +
> +8.19 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.

"the guest"

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

Full stop or semicolon, not comma.
Not sure what the "which is not under control by KVM" part means.

> +If this virtual SError is taken to EL1 using AArch64, this value will be reported
> +in ISS filed of ESR_EL1.

"the ISS field".

How does this capability interact with the KVM_CAP_VCPU_EVENTS
API in patch 1? Is it saying "you can specify the ESR via
that API", or talking about something else?

What happens if userspace does try to specify the EC field value ?
Is this an error, or is it ignored ?

> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index a3db01a28062..067c6ba969bd 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:
>         case KVM_CAP_VCPU_EVENTS:
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index b6270a3b38e9..a7d9bc4e4068 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -949,6 +949,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_GET_MSR_FEATURES 153
>  #define KVM_CAP_HYPERV_EVENTFD 154
>  #define KVM_CAP_HYPERV_TLBFLUSH 155
> +#define KVM_CAP_ARM_INJECT_SERROR_ESR 156
>
>  #ifdef KVM_CAP_IRQ_ROUTING
>
> --
> 2.18.0
>
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

thanks
-- PMM

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

* Re: [PATCH v8 1/4] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
  2018-07-17 16:33     ` Peter Maydell
@ 2018-07-19  7:39       ` gengdongjiu
  -1 siblings, 0 replies; 36+ messages in thread
From: gengdongjiu @ 2018-07-19  7:39 UTC (permalink / raw)
  To: Peter Maydell, James Morse
  Cc: Marc Zyngier, arm-mail-list, kvmarm, kvm-devel, Linuxarm

On 2018/7/18 0:33, Peter Maydell wrote:
> On 13 July 2018 at 16:47, James Morse <james.morse@arm.com> wrote:
>> From: Dongjiu Geng <gengdongjiu@huawei.com>
>>
>> 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>
>> Reviewed-by: James Morse <james.morse@arm.com>
>> Signed-off-by: James Morse <james.morse@arm.com>
>> ---
>>  Documentation/virtual/kvm/api.txt    | 33 +++++++++++++++++---
>>  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               | 46 ++++++++++++++++++++++++++++
>>  arch/arm64/kvm/inject_fault.c        |  6 ++--
>>  arch/arm64/kvm/reset.c               |  1 +
>>  virt/kvm/arm/arm.c                   | 21 +++++++++++++
>>  8 files changed, 125 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index d10944e619d3..e3940f8715a5 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
>> -Type: vm ioctl
>> +Architectures: x86, arm64
>> +Type: vcpu 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.
>>
>> +ARM64:
>> +
>> +Gets currently pending SError exceptions as well as related states of the vcpu.
> 
> Any chance of expanding this to explain what this state actually is?
> (ie something you could use to implement or review the userspace
> code which has to manipulate it)
sure, can expand it.

serror_pending means whether KVM is pending Serror for guest.
serror_has_esr means whether KVM or user space can set the SError syndrome for guest.
serror_esr means the value of SError syndrome.

Anyway, I will expand this to explain it.

> 
>> +
>> +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
> 
> thanks
> -- PMM
> 
> .
> 

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

* [PATCH v8 1/4] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
@ 2018-07-19  7:39       ` gengdongjiu
  0 siblings, 0 replies; 36+ messages in thread
From: gengdongjiu @ 2018-07-19  7:39 UTC (permalink / raw)
  To: linux-arm-kernel

On 2018/7/18 0:33, Peter Maydell wrote:
> On 13 July 2018 at 16:47, James Morse <james.morse@arm.com> wrote:
>> From: Dongjiu Geng <gengdongjiu@huawei.com>
>>
>> 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>
>> Reviewed-by: James Morse <james.morse@arm.com>
>> Signed-off-by: James Morse <james.morse@arm.com>
>> ---
>>  Documentation/virtual/kvm/api.txt    | 33 +++++++++++++++++---
>>  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               | 46 ++++++++++++++++++++++++++++
>>  arch/arm64/kvm/inject_fault.c        |  6 ++--
>>  arch/arm64/kvm/reset.c               |  1 +
>>  virt/kvm/arm/arm.c                   | 21 +++++++++++++
>>  8 files changed, 125 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index d10944e619d3..e3940f8715a5 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
>> -Type: vm ioctl
>> +Architectures: x86, arm64
>> +Type: vcpu 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.
>>
>> +ARM64:
>> +
>> +Gets currently pending SError exceptions as well as related states of the vcpu.
> 
> Any chance of expanding this to explain what this state actually is?
> (ie something you could use to implement or review the userspace
> code which has to manipulate it)
sure, can expand it.

serror_pending means whether KVM is pending Serror for guest.
serror_has_esr means whether KVM or user space can set the SError syndrome for guest.
serror_esr means the value of SError syndrome.

Anyway, I will expand this to explain it.

> 
>> +
>> +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
> 
> thanks
> -- PMM
> 
> .
> 

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

* Re: [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
  2018-07-17 16:36     ` Peter Maydell
@ 2018-07-19  8:54       ` gengdongjiu
  -1 siblings, 0 replies; 36+ messages in thread
From: gengdongjiu @ 2018-07-19  8:54 UTC (permalink / raw)
  To: Peter Maydell, James Morse
  Cc: Marc Zyngier, arm-mail-list, kvmarm, kvm-devel, Linuxarm

Hi peter,
  Thanks for the review.

On 2018/7/18 0:36, Peter Maydell wrote:
> On 13 July 2018 at 16:47, James Morse <james.morse@arm.com> wrote:
>> From: Dongjiu Geng <gengdongjiu@huawei.com>
>>
>> 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>
>> Signed-off-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 e3940f8715a5..480fa64eb52a 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -4643,3 +4643,14 @@ This capability indicates that KVM supports paravirtualized Hyper-V TLB Flush
>>  hypercalls:
>>  HvFlushVirtualAddressSpace, HvFlushVirtualAddressSpaceEx,
>>  HvFlushVirtualAddressList, HvFlushVirtualAddressListEx.
>> +
>> +8.19 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.
> 
> "the guest"
  got it, thanks

> 
>> +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.
> 
> Full stop or semicolon, not comma.
> Not sure what the "which is not under control by KVM" part means.
> 
>> +If this virtual SError is taken to EL1 using AArch64, this value will be reported
>> +in ISS filed of ESR_EL1.
> 
> "the ISS field".
  got it, thanks

> 
> How does this capability interact with the KVM_CAP_VCPU_EVENTS
> API in patch 1? Is it saying "you can specify the ESR via
> that API", or talking about something else?

The user space will firstly check the  KVM_CAP_VCPU_EVENTS through kvm_has_vcpu_events() to know whether KVM can support get/set vcpu events,
If KVM supports get/set vcpu events, it will check KVM_CAP_ARM_SET_SERROR_ESR to know whether can set guest SError ESR, as shown the user space code in [1].

[1]:
+static int kvm_put_vcpu_events(ARMCPU *cpu)
+{
+    CPUARMState *env = &cpu->env;
+    struct kvm_vcpu_events events = {};
+
+    if (!kvm_has_vcpu_events()) {
+        return 0;
+    }
+
+    memset(&events, 0, sizeof(events));
+    events.exception.serror_pending = env->serror.pending;
+
+    if (arm_feature(env, ARM_FEATURE_RAS)) {
+        events.exception.serror_has_esr = env->serror.has_esr;
+        events.exception.serror_esr = env->serror.esr;
+    }
+
+    return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_VCPU_EVENTS, &events);
+}

> 
> What happens if userspace does try to specify the EC field value ?
> Is this an error, or is it ignored ?
if userspace try to specify the EC field value, KVM will return error to userspace, do not be ignored,


> 
>> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
>> index a3db01a28062..067c6ba969bd 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:
>>         case KVM_CAP_VCPU_EVENTS:
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index b6270a3b38e9..a7d9bc4e4068 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -949,6 +949,7 @@ struct kvm_ppc_resize_hpt {
>>  #define KVM_CAP_GET_MSR_FEATURES 153
>>  #define KVM_CAP_HYPERV_EVENTFD 154
>>  #define KVM_CAP_HYPERV_TLBFLUSH 155
>> +#define KVM_CAP_ARM_INJECT_SERROR_ESR 156
>>
>>  #ifdef KVM_CAP_IRQ_ROUTING
>>
>> --
>> 2.18.0
>>
>> _______________________________________________
>> kvmarm mailing list
>> kvmarm@lists.cs.columbia.edu
>> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
> 
> thanks
> -- PMM
> 
> .
> 

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

* [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
@ 2018-07-19  8:54       ` gengdongjiu
  0 siblings, 0 replies; 36+ messages in thread
From: gengdongjiu @ 2018-07-19  8:54 UTC (permalink / raw)
  To: linux-arm-kernel

Hi peter,
  Thanks for the review.

On 2018/7/18 0:36, Peter Maydell wrote:
> On 13 July 2018 at 16:47, James Morse <james.morse@arm.com> wrote:
>> From: Dongjiu Geng <gengdongjiu@huawei.com>
>>
>> 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>
>> Signed-off-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 e3940f8715a5..480fa64eb52a 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -4643,3 +4643,14 @@ This capability indicates that KVM supports paravirtualized Hyper-V TLB Flush
>>  hypercalls:
>>  HvFlushVirtualAddressSpace, HvFlushVirtualAddressSpaceEx,
>>  HvFlushVirtualAddressList, HvFlushVirtualAddressListEx.
>> +
>> +8.19 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.
> 
> "the guest"
  got it, thanks

> 
>> +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.
> 
> Full stop or semicolon, not comma.
> Not sure what the "which is not under control by KVM" part means.
> 
>> +If this virtual SError is taken to EL1 using AArch64, this value will be reported
>> +in ISS filed of ESR_EL1.
> 
> "the ISS field".
  got it, thanks

> 
> How does this capability interact with the KVM_CAP_VCPU_EVENTS
> API in patch 1? Is it saying "you can specify the ESR via
> that API", or talking about something else?

The user space will firstly check the  KVM_CAP_VCPU_EVENTS through kvm_has_vcpu_events() to know whether KVM can support get/set vcpu events,
If KVM supports get/set vcpu events, it will check KVM_CAP_ARM_SET_SERROR_ESR to know whether can set guest SError ESR, as shown the user space code in [1].

[1]:
+static int kvm_put_vcpu_events(ARMCPU *cpu)
+{
+    CPUARMState *env = &cpu->env;
+    struct kvm_vcpu_events events = {};
+
+    if (!kvm_has_vcpu_events()) {
+        return 0;
+    }
+
+    memset(&events, 0, sizeof(events));
+    events.exception.serror_pending = env->serror.pending;
+
+    if (arm_feature(env, ARM_FEATURE_RAS)) {
+        events.exception.serror_has_esr = env->serror.has_esr;
+        events.exception.serror_esr = env->serror.esr;
+    }
+
+    return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_VCPU_EVENTS, &events);
+}

> 
> What happens if userspace does try to specify the EC field value ?
> Is this an error, or is it ignored ?
if userspace try to specify the EC field value, KVM will return error to userspace, do not be ignored,


> 
>> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
>> index a3db01a28062..067c6ba969bd 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:
>>         case KVM_CAP_VCPU_EVENTS:
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index b6270a3b38e9..a7d9bc4e4068 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -949,6 +949,7 @@ struct kvm_ppc_resize_hpt {
>>  #define KVM_CAP_GET_MSR_FEATURES 153
>>  #define KVM_CAP_HYPERV_EVENTFD 154
>>  #define KVM_CAP_HYPERV_TLBFLUSH 155
>> +#define KVM_CAP_ARM_INJECT_SERROR_ESR 156
>>
>>  #ifdef KVM_CAP_IRQ_ROUTING
>>
>> --
>> 2.18.0
>>
>> _______________________________________________
>> kvmarm mailing list
>> kvmarm at lists.cs.columbia.edu
>> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
> 
> thanks
> -- PMM
> 
> .
> 

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

* Re: [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
  2018-07-19  8:54       ` gengdongjiu
@ 2018-07-19  9:10         ` Peter Maydell
  -1 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2018-07-19  9:10 UTC (permalink / raw)
  To: gengdongjiu; +Cc: kvm-devel, Marc Zyngier, Linuxarm, kvmarm, arm-mail-list

On 19 July 2018 at 09:54, gengdongjiu <gengdongjiu@huawei.com> wrote:
> Hi peter,
>   Thanks for the review.
>
> On 2018/7/18 0:36, Peter Maydell wrote:
>> How does this capability interact with the KVM_CAP_VCPU_EVENTS
>> API in patch 1? Is it saying "you can specify the ESR via
>> that API", or talking about something else?
>
> The user space will firstly check the  KVM_CAP_VCPU_EVENTS through kvm_has_vcpu_events() to know whether KVM can support get/set vcpu events,
> If KVM supports get/set vcpu events, it will check KVM_CAP_ARM_SET_SERROR_ESR to know whether can set guest SError ESR, as shown the user space code in [1].

OK, so this capability affects the behaviour of
KVM_SET_VCPU_EVENTS. Can you make sure the documentation
of KVM_SET_VCPU_EVENTS clearly explains what happens
and the effect of whether this capability is present,
please? People trying to understand the behaviour of
that ioctl are not going to know that they need to
look here as well.

thanks
-- PMM

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

* [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
@ 2018-07-19  9:10         ` Peter Maydell
  0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2018-07-19  9:10 UTC (permalink / raw)
  To: linux-arm-kernel

On 19 July 2018 at 09:54, gengdongjiu <gengdongjiu@huawei.com> wrote:
> Hi peter,
>   Thanks for the review.
>
> On 2018/7/18 0:36, Peter Maydell wrote:
>> How does this capability interact with the KVM_CAP_VCPU_EVENTS
>> API in patch 1? Is it saying "you can specify the ESR via
>> that API", or talking about something else?
>
> The user space will firstly check the  KVM_CAP_VCPU_EVENTS through kvm_has_vcpu_events() to know whether KVM can support get/set vcpu events,
> If KVM supports get/set vcpu events, it will check KVM_CAP_ARM_SET_SERROR_ESR to know whether can set guest SError ESR, as shown the user space code in [1].

OK, so this capability affects the behaviour of
KVM_SET_VCPU_EVENTS. Can you make sure the documentation
of KVM_SET_VCPU_EVENTS clearly explains what happens
and the effect of whether this capability is present,
please? People trying to understand the behaviour of
that ioctl are not going to know that they need to
look here as well.

thanks
-- PMM

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

* Re: [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
  2018-07-19  9:10         ` Peter Maydell
@ 2018-07-19  9:29           ` gengdongjiu
  -1 siblings, 0 replies; 36+ messages in thread
From: gengdongjiu @ 2018-07-19  9:29 UTC (permalink / raw)
  To: Peter Maydell; +Cc: kvm-devel, Marc Zyngier, Linuxarm, kvmarm, arm-mail-list

On 2018/7/19 17:10, Peter Maydell wrote:
>> The user space will firstly check the  KVM_CAP_VCPU_EVENTS through kvm_has_vcpu_events() to know whether KVM can support get/set vcpu events,
>> If KVM supports get/set vcpu events, it will check KVM_CAP_ARM_SET_SERROR_ESR to know whether can set guest SError ESR, as shown the user space code in [1].
> OK, so this capability affects the behaviour of
> KVM_SET_VCPU_EVENTS. Can you make sure the documentation
> of KVM_SET_VCPU_EVENTS clearly explains what happens
> and the effect of whether this capability is present,
> please? People trying to understand the behaviour of
> that ioctl are not going to know that they need to
> look here as well.
sure, I will. Thanks peter's reminder and comments.

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

* [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
@ 2018-07-19  9:29           ` gengdongjiu
  0 siblings, 0 replies; 36+ messages in thread
From: gengdongjiu @ 2018-07-19  9:29 UTC (permalink / raw)
  To: linux-arm-kernel

On 2018/7/19 17:10, Peter Maydell wrote:
>> The user space will firstly check the  KVM_CAP_VCPU_EVENTS through kvm_has_vcpu_events() to know whether KVM can support get/set vcpu events,
>> If KVM supports get/set vcpu events, it will check KVM_CAP_ARM_SET_SERROR_ESR to know whether can set guest SError ESR, as shown the user space code in [1].
> OK, so this capability affects the behaviour of
> KVM_SET_VCPU_EVENTS. Can you make sure the documentation
> of KVM_SET_VCPU_EVENTS clearly explains what happens
> and the effect of whether this capability is present,
> please? People trying to understand the behaviour of
> that ioctl are not going to know that they need to
> look here as well.
sure, I will. Thanks peter's reminder and comments.

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

* Re: [PATCH v8 1/4] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
  2018-07-19  7:39       ` gengdongjiu
@ 2018-07-19 14:15         ` James Morse
  -1 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-19 14:15 UTC (permalink / raw)
  To: gengdongjiu, Peter Maydell
  Cc: Marc Zyngier, arm-mail-list, kvmarm, kvm-devel, Linuxarm

Hi guys,

On 19/07/18 08:39, gengdongjiu wrote:
> On 2018/7/18 0:33, Peter Maydell wrote:
>> On 13 July 2018 at 16:47, James Morse <james.morse@arm.com> wrote:
>>> From: Dongjiu Geng <gengdongjiu@huawei.com>
>>>
>>> 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.

>>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>>> index d10944e619d3..e3940f8715a5 100644
>>> --- a/Documentation/virtual/kvm/api.txt
>>> +++ b/Documentation/virtual/kvm/api.txt


>>> +Gets currently pending SError exceptions as well as related states of the vcpu.
>>
>> Any chance of expanding this to explain what this state actually is?
>> (ie something you could use to implement or review the userspace
>> code which has to manipulate it)
> sure, can expand it.
> 
> serror_pending means whether KVM is pending Serror for guest.
> serror_has_esr means whether KVM or user space can set the SError syndrome for guest.
> serror_esr means the value of SError syndrome.

I've had a go at something more verbose for this section:
------------------%<------------------
If the guest accesses a device that is being emulated by the host kernel in such
a way that a real device would generate a physical SError, KVM may make a
virtual SError pending for that VCPU. This system error interrupt remains
pending until the guest takes the exception by unmasking PSTATE.A.

Running the VCPU may cause it to take a pending SError, or make an access that
causes an SError to become pending. The event's description is only valid while
the VPCU is not running.

This API provides a way to read and write the pending 'event' state that is not
visible to the guest. To save, restore or migrate a VCPU the struct representing
the state can be read then written using this GET/SET API, along with the other
guest-visible registers. It is not possible to 'cancel' an SError that has been
made pending.

A device being emulated in user-space may also wish to generate an SError. To do
this the events structure can be populated by user-space. The current state
should be read first, to ensure no existing SError is pending. If an existing
SError is pending, the architectures 'Multiple SError interrupts' rules should
be followed. (2.5.3 of DDI0587.a "ARM Reliability, Availability, and
Serviceability (RAS) Specification").
------------------%<------------------


Thanks,

James

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

* [PATCH v8 1/4] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
@ 2018-07-19 14:15         ` James Morse
  0 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-19 14:15 UTC (permalink / raw)
  To: linux-arm-kernel

Hi guys,

On 19/07/18 08:39, gengdongjiu wrote:
> On 2018/7/18 0:33, Peter Maydell wrote:
>> On 13 July 2018 at 16:47, James Morse <james.morse@arm.com> wrote:
>>> From: Dongjiu Geng <gengdongjiu@huawei.com>
>>>
>>> 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.

>>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>>> index d10944e619d3..e3940f8715a5 100644
>>> --- a/Documentation/virtual/kvm/api.txt
>>> +++ b/Documentation/virtual/kvm/api.txt


>>> +Gets currently pending SError exceptions as well as related states of the vcpu.
>>
>> Any chance of expanding this to explain what this state actually is?
>> (ie something you could use to implement or review the userspace
>> code which has to manipulate it)
> sure, can expand it.
> 
> serror_pending means whether KVM is pending Serror for guest.
> serror_has_esr means whether KVM or user space can set the SError syndrome for guest.
> serror_esr means the value of SError syndrome.

I've had a go at something more verbose for this section:
------------------%<------------------
If the guest accesses a device that is being emulated by the host kernel in such
a way that a real device would generate a physical SError, KVM may make a
virtual SError pending for that VCPU. This system error interrupt remains
pending until the guest takes the exception by unmasking PSTATE.A.

Running the VCPU may cause it to take a pending SError, or make an access that
causes an SError to become pending. The event's description is only valid while
the VPCU is not running.

This API provides a way to read and write the pending 'event' state that is not
visible to the guest. To save, restore or migrate a VCPU the struct representing
the state can be read then written using this GET/SET API, along with the other
guest-visible registers. It is not possible to 'cancel' an SError that has been
made pending.

A device being emulated in user-space may also wish to generate an SError. To do
this the events structure can be populated by user-space. The current state
should be read first, to ensure no existing SError is pending. If an existing
SError is pending, the architectures 'Multiple SError interrupts' rules should
be followed. (2.5.3 of DDI0587.a "ARM Reliability, Availability, and
Serviceability (RAS) Specification").
------------------%<------------------


Thanks,

James

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

* Re: [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
  2018-07-19  9:29           ` gengdongjiu
@ 2018-07-19 14:15             ` James Morse
  -1 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-19 14:15 UTC (permalink / raw)
  To: gengdongjiu, Peter Maydell
  Cc: Marc Zyngier, arm-mail-list, kvmarm, kvm-devel, Linuxarm

Hi guys,

On 19/07/18 10:29, gengdongjiu wrote:
> On 2018/7/19 17:10, Peter Maydell wrote:
>>> The user space will firstly check the  KVM_CAP_VCPU_EVENTS through kvm_has_vcpu_events() to know whether KVM can support get/set vcpu events,
>>> If KVM supports get/set vcpu events, it will check KVM_CAP_ARM_SET_SERROR_ESR to know whether can set guest SError ESR, as shown the user space code in [1].
>> OK, so this capability affects the behaviour of
>> KVM_SET_VCPU_EVENTS. Can you make sure the documentation
>> of KVM_SET_VCPU_EVENTS clearly explains what happens
>> and the effect of whether this capability is present,
>> please? People trying to understand the behaviour of
>> that ioctl are not going to know that they need to
>> look here as well.

> sure, I will. Thanks peter's reminder and comments.

I've collected these changes, this description currently reads:
------------------%<------------------
This capability indicates that userspace can specify the syndrome value reported
to the guest when it takes a virtual SError interrupt exception.
If KVM advertises this capability, userspace can only specify the ISS field for
the ESR syndrome. Other parts of the ESR, such as the EC are generated by the
CPU when the exception is taken. If this virtual SError is taken to EL1 using
AArch64, this value will be reported in the ISS field of ESR_ELx.

See KVM_CAP_VCPU_EVENTS for more details.
------------------%<------------------

and this patch adds to the GET/SET API section with:
------------------%<------------------
SError exceptions always have an ESR value. Some CPUs have the ability to
specify what the virtual SError's ESR value should be. These systems will
advertise KVM_CAP_ARM_SET_SERROR_ESR. In this case exception.has_esr will
always have a non-zero value when read, and the agent making an SError
pending should specify the ISS field in the lower 24 bits of
exception.serror_esr. If the system supports KVM_CAP_ARM_SET_SERROR_ESR, but
user-space sets the events with exception.has_esr as zero, KVM will choose an ESR.

Specifying exception.has_esr on a system that does not support it will return an
error. Settings anything other than the lower 24bits of exception.serror_esr
will return an error.
------------------%<------------------

because as Peter points out, this is where people will expect to find this.

I'll kick this out as a v9 later today.

Thanks!

James

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

* [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
@ 2018-07-19 14:15             ` James Morse
  0 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2018-07-19 14:15 UTC (permalink / raw)
  To: linux-arm-kernel

Hi guys,

On 19/07/18 10:29, gengdongjiu wrote:
> On 2018/7/19 17:10, Peter Maydell wrote:
>>> The user space will firstly check the  KVM_CAP_VCPU_EVENTS through kvm_has_vcpu_events() to know whether KVM can support get/set vcpu events,
>>> If KVM supports get/set vcpu events, it will check KVM_CAP_ARM_SET_SERROR_ESR to know whether can set guest SError ESR, as shown the user space code in [1].
>> OK, so this capability affects the behaviour of
>> KVM_SET_VCPU_EVENTS. Can you make sure the documentation
>> of KVM_SET_VCPU_EVENTS clearly explains what happens
>> and the effect of whether this capability is present,
>> please? People trying to understand the behaviour of
>> that ioctl are not going to know that they need to
>> look here as well.

> sure, I will. Thanks peter's reminder and comments.

I've collected these changes, this description currently reads:
------------------%<------------------
This capability indicates that userspace can specify the syndrome value reported
to the guest when it takes a virtual SError interrupt exception.
If KVM advertises this capability, userspace can only specify the ISS field for
the ESR syndrome. Other parts of the ESR, such as the EC are generated by the
CPU when the exception is taken. If this virtual SError is taken to EL1 using
AArch64, this value will be reported in the ISS field of ESR_ELx.

See KVM_CAP_VCPU_EVENTS for more details.
------------------%<------------------

and this patch adds to the GET/SET API section with:
------------------%<------------------
SError exceptions always have an ESR value. Some CPUs have the ability to
specify what the virtual SError's ESR value should be. These systems will
advertise KVM_CAP_ARM_SET_SERROR_ESR. In this case exception.has_esr will
always have a non-zero value when read, and the agent making an SError
pending should specify the ISS field in the lower 24 bits of
exception.serror_esr. If the system supports KVM_CAP_ARM_SET_SERROR_ESR, but
user-space sets the events with exception.has_esr as zero, KVM will choose an ESR.

Specifying exception.has_esr on a system that does not support it will return an
error. Settings anything other than the lower 24bits of exception.serror_esr
will return an error.
------------------%<------------------

because as Peter points out, this is where people will expect to find this.

I'll kick this out as a v9 later today.

Thanks!

James

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

* Re: [PATCH v8 1/4] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
  2018-07-19 14:15         ` James Morse
@ 2018-07-19 14:34           ` Peter Maydell
  -1 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2018-07-19 14:34 UTC (permalink / raw)
  To: James Morse
  Cc: kvm-devel, Marc Zyngier, Linuxarm, gengdongjiu, kvmarm, arm-mail-list

On 19 July 2018 at 15:15, James Morse <james.morse@arm.com> wrote:
> Hi guys,
>
> On 19/07/18 08:39, gengdongjiu wrote:
>> On 2018/7/18 0:33, Peter Maydell wrote:
>>> On 13 July 2018 at 16:47, James Morse <james.morse@arm.com> wrote:
>>>> From: Dongjiu Geng <gengdongjiu@huawei.com>
>>>>
>>>> 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.
>
>>>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>>>> index d10944e619d3..e3940f8715a5 100644
>>>> --- a/Documentation/virtual/kvm/api.txt
>>>> +++ b/Documentation/virtual/kvm/api.txt
>
>
>>>> +Gets currently pending SError exceptions as well as related states of the vcpu.
>>>
>>> Any chance of expanding this to explain what this state actually is?
>>> (ie something you could use to implement or review the userspace
>>> code which has to manipulate it)
>> sure, can expand it.
>>
>> serror_pending means whether KVM is pending Serror for guest.
>> serror_has_esr means whether KVM or user space can set the SError syndrome for guest.
>> serror_esr means the value of SError syndrome.
>
> I've had a go at something more verbose for this section:
> ------------------%<------------------
> If the guest accesses a device that is being emulated by the host kernel in such
> a way that a real device would generate a physical SError, KVM may make a
> virtual SError pending for that VCPU. This system error interrupt remains
> pending until the guest takes the exception by unmasking PSTATE.A.
>
> Running the VCPU may cause it to take a pending SError, or make an access that
> causes an SError to become pending. The event's description is only valid while
> the VPCU is not running.
>
> This API provides a way to read and write the pending 'event' state that is not
> visible to the guest. To save, restore or migrate a VCPU the struct representing
> the state can be read then written using this GET/SET API, along with the other
> guest-visible registers. It is not possible to 'cancel' an SError that has been
> made pending.
>
> A device being emulated in user-space may also wish to generate an SError. To do
> this the events structure can be populated by user-space. The current state
> should be read first, to ensure no existing SError is pending. If an existing
> SError is pending, the architectures 'Multiple SError interrupts' rules should

"architecture's"

> be followed. (2.5.3 of DDI0587.a "ARM Reliability, Availability, and
> Serviceability (RAS) Specification").
> ------------------%<------------------

Otherwise looks good.

thanks
-- PMM

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

* [PATCH v8 1/4] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
@ 2018-07-19 14:34           ` Peter Maydell
  0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2018-07-19 14:34 UTC (permalink / raw)
  To: linux-arm-kernel

On 19 July 2018 at 15:15, James Morse <james.morse@arm.com> wrote:
> Hi guys,
>
> On 19/07/18 08:39, gengdongjiu wrote:
>> On 2018/7/18 0:33, Peter Maydell wrote:
>>> On 13 July 2018 at 16:47, James Morse <james.morse@arm.com> wrote:
>>>> From: Dongjiu Geng <gengdongjiu@huawei.com>
>>>>
>>>> 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.
>
>>>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>>>> index d10944e619d3..e3940f8715a5 100644
>>>> --- a/Documentation/virtual/kvm/api.txt
>>>> +++ b/Documentation/virtual/kvm/api.txt
>
>
>>>> +Gets currently pending SError exceptions as well as related states of the vcpu.
>>>
>>> Any chance of expanding this to explain what this state actually is?
>>> (ie something you could use to implement or review the userspace
>>> code which has to manipulate it)
>> sure, can expand it.
>>
>> serror_pending means whether KVM is pending Serror for guest.
>> serror_has_esr means whether KVM or user space can set the SError syndrome for guest.
>> serror_esr means the value of SError syndrome.
>
> I've had a go at something more verbose for this section:
> ------------------%<------------------
> If the guest accesses a device that is being emulated by the host kernel in such
> a way that a real device would generate a physical SError, KVM may make a
> virtual SError pending for that VCPU. This system error interrupt remains
> pending until the guest takes the exception by unmasking PSTATE.A.
>
> Running the VCPU may cause it to take a pending SError, or make an access that
> causes an SError to become pending. The event's description is only valid while
> the VPCU is not running.
>
> This API provides a way to read and write the pending 'event' state that is not
> visible to the guest. To save, restore or migrate a VCPU the struct representing
> the state can be read then written using this GET/SET API, along with the other
> guest-visible registers. It is not possible to 'cancel' an SError that has been
> made pending.
>
> A device being emulated in user-space may also wish to generate an SError. To do
> this the events structure can be populated by user-space. The current state
> should be read first, to ensure no existing SError is pending. If an existing
> SError is pending, the architectures 'Multiple SError interrupts' rules should

"architecture's"

> be followed. (2.5.3 of DDI0587.a "ARM Reliability, Availability, and
> Serviceability (RAS) Specification").
> ------------------%<------------------

Otherwise looks good.

thanks
-- PMM

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

* Re: [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
  2018-07-19 14:15             ` James Morse
@ 2018-07-19 14:37               ` Peter Maydell
  -1 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2018-07-19 14:37 UTC (permalink / raw)
  To: James Morse
  Cc: kvm-devel, Marc Zyngier, Linuxarm, gengdongjiu, kvmarm, arm-mail-list

On 19 July 2018 at 15:15, James Morse <james.morse@arm.com> wrote:
> Hi guys,
>
> On 19/07/18 10:29, gengdongjiu wrote:
>> On 2018/7/19 17:10, Peter Maydell wrote:
>>>> The user space will firstly check the  KVM_CAP_VCPU_EVENTS through kvm_has_vcpu_events() to know whether KVM can support get/set vcpu events,
>>>> If KVM supports get/set vcpu events, it will check KVM_CAP_ARM_SET_SERROR_ESR to know whether can set guest SError ESR, as shown the user space code in [1].
>>> OK, so this capability affects the behaviour of
>>> KVM_SET_VCPU_EVENTS. Can you make sure the documentation
>>> of KVM_SET_VCPU_EVENTS clearly explains what happens
>>> and the effect of whether this capability is present,
>>> please? People trying to understand the behaviour of
>>> that ioctl are not going to know that they need to
>>> look here as well.
>
>> sure, I will. Thanks peter's reminder and comments.
>
> I've collected these changes, this description currently reads:
> ------------------%<------------------
> This capability indicates that userspace can specify the syndrome value reported
> to the guest when it takes a virtual SError interrupt exception.

Perhaps "specify (via the KVM_SET_VCPU_EVENTS ioctl)" ?

> If KVM advertises this capability, userspace can only specify the ISS field for
> the ESR syndrome. Other parts of the ESR, such as the EC are generated by the
> CPU when the exception is taken. If this virtual SError is taken to EL1 using
> AArch64, this value will be reported in the ISS field of ESR_ELx.
>
> See KVM_CAP_VCPU_EVENTS for more details.
> ------------------%<------------------
>
> and this patch adds to the GET/SET API section with:
> ------------------%<------------------
> SError exceptions always have an ESR value. Some CPUs have the ability to
> specify what the virtual SError's ESR value should be. These systems will
> advertise KVM_CAP_ARM_SET_SERROR_ESR. In this case exception.has_esr will
> always have a non-zero value when read, and the agent making an SError
> pending should specify the ISS field in the lower 24 bits of
> exception.serror_esr. If the system supports KVM_CAP_ARM_SET_SERROR_ESR, but
> user-space sets the events with exception.has_esr as zero, KVM will choose an ESR.
>
> Specifying exception.has_esr on a system that does not support it will return an
> error. Settings anything other than the lower 24bits of exception.serror_esr

"Setting"

> will return an error.

I think you should state which error (presumably EINVAL?)

> ------------------%<------------------
>
> because as Peter points out, this is where people will expect to find this.

thanks
-- PMM

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

* [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome
@ 2018-07-19 14:37               ` Peter Maydell
  0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2018-07-19 14:37 UTC (permalink / raw)
  To: linux-arm-kernel

On 19 July 2018 at 15:15, James Morse <james.morse@arm.com> wrote:
> Hi guys,
>
> On 19/07/18 10:29, gengdongjiu wrote:
>> On 2018/7/19 17:10, Peter Maydell wrote:
>>>> The user space will firstly check the  KVM_CAP_VCPU_EVENTS through kvm_has_vcpu_events() to know whether KVM can support get/set vcpu events,
>>>> If KVM supports get/set vcpu events, it will check KVM_CAP_ARM_SET_SERROR_ESR to know whether can set guest SError ESR, as shown the user space code in [1].
>>> OK, so this capability affects the behaviour of
>>> KVM_SET_VCPU_EVENTS. Can you make sure the documentation
>>> of KVM_SET_VCPU_EVENTS clearly explains what happens
>>> and the effect of whether this capability is present,
>>> please? People trying to understand the behaviour of
>>> that ioctl are not going to know that they need to
>>> look here as well.
>
>> sure, I will. Thanks peter's reminder and comments.
>
> I've collected these changes, this description currently reads:
> ------------------%<------------------
> This capability indicates that userspace can specify the syndrome value reported
> to the guest when it takes a virtual SError interrupt exception.

Perhaps "specify (via the KVM_SET_VCPU_EVENTS ioctl)" ?

> If KVM advertises this capability, userspace can only specify the ISS field for
> the ESR syndrome. Other parts of the ESR, such as the EC are generated by the
> CPU when the exception is taken. If this virtual SError is taken to EL1 using
> AArch64, this value will be reported in the ISS field of ESR_ELx.
>
> See KVM_CAP_VCPU_EVENTS for more details.
> ------------------%<------------------
>
> and this patch adds to the GET/SET API section with:
> ------------------%<------------------
> SError exceptions always have an ESR value. Some CPUs have the ability to
> specify what the virtual SError's ESR value should be. These systems will
> advertise KVM_CAP_ARM_SET_SERROR_ESR. In this case exception.has_esr will
> always have a non-zero value when read, and the agent making an SError
> pending should specify the ISS field in the lower 24 bits of
> exception.serror_esr. If the system supports KVM_CAP_ARM_SET_SERROR_ESR, but
> user-space sets the events with exception.has_esr as zero, KVM will choose an ESR.
>
> Specifying exception.has_esr on a system that does not support it will return an
> error. Settings anything other than the lower 24bits of exception.serror_esr

"Setting"

> will return an error.

I think you should state which error (presumably EINVAL?)

> ------------------%<------------------
>
> because as Peter points out, this is where people will expect to find this.

thanks
-- PMM

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

end of thread, other threads:[~2018-07-19 14:37 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-13 15:47 [PATCH v8 0/4] support exception state migration and set VSESR_EL2 by user space James Morse
2018-07-13 15:47 ` James Morse
2018-07-13 15:47 ` [PATCH v8 1/4] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS James Morse
2018-07-13 15:47   ` James Morse
2018-07-17 16:33   ` Peter Maydell
2018-07-17 16:33     ` Peter Maydell
2018-07-19  7:39     ` gengdongjiu
2018-07-19  7:39       ` gengdongjiu
2018-07-19 14:15       ` James Morse
2018-07-19 14:15         ` James Morse
2018-07-19 14:34         ` Peter Maydell
2018-07-19 14:34           ` Peter Maydell
2018-07-13 15:47 ` [PATCH v8 2/4] arm64: KVM: export the capability to set guest SError syndrome James Morse
2018-07-13 15:47   ` James Morse
2018-07-17 16:36   ` Peter Maydell
2018-07-17 16:36     ` Peter Maydell
2018-07-19  8:54     ` gengdongjiu
2018-07-19  8:54       ` gengdongjiu
2018-07-19  9:10       ` Peter Maydell
2018-07-19  9:10         ` Peter Maydell
2018-07-19  9:29         ` gengdongjiu
2018-07-19  9:29           ` gengdongjiu
2018-07-19 14:15           ` James Morse
2018-07-19 14:15             ` James Morse
2018-07-19 14:37             ` Peter Maydell
2018-07-19 14:37               ` Peter Maydell
2018-07-13 15:47 ` [PATCH v8 3/4] KVM: arm64: Share the parts of get/set events useful to 32bit James Morse
2018-07-13 15:47   ` James Morse
2018-07-16  2:50   ` gengdongjiu
2018-07-16  2:50     ` gengdongjiu
2018-07-16  2:50     ` gengdongjiu
2018-07-13 15:47 ` [PATCH v8 4/4] KVM: arm: Add 32bit get/set events support James Morse
2018-07-13 15:47   ` James Morse
2018-07-16  3:00   ` gengdongjiu
2018-07-16  3:00     ` gengdongjiu
2018-07-16  3:00     ` gengdongjiu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.