[PATCH v7 0/4] Add RAS virtualization support

[PATCH v7 0/4] Add RAS virtualization support

[Dongjiu Geng]

In the firmware-first RAS solution, corrupt data is detected in a
memory location when guest OS application software executing at EL0
or guest OS kernel El1 software are reading from the memory. The
memory node records errors into an accessible system registers.

Because SCR_EL3.EA is 1, then CPU will trap to El3 firmware, EL3
firmware records the error to APEI table through reading system
register.

Because the error was taken from a lower Exception level, if the
exception is SEA/SEI and HCR_EL2.TEA/HCR_EL2.AMO is 1, firmware
sets ESR_EL2/FAR_EL2 to fake a exception trap to EL2, then
transfers to hypervisor.

For the synchronous external abort(SEA), Hypervisor calls the
handle_guest_sea() to deal with this error, which will reads 
the APEI table to get the error physical address, then call
memory_failure() to identify the this address to poisoned and
deliver SIGBUS signal to userspace. The advantage of using SIGBUS
signal to notify user space is that it can be compatible with
Non-Kvm users.

For the SError Interrupt(SEI), KVM firstly classified the error.
If the SError error comes from guest and is not propagated, then call
handle_guest_sei() to let host firstly handle it. If the address recorded
by APEI table is valid, then deliver SIGBUS signal to user space,
user space will record the address to guest APEI table. Otherwise, directly 
injects virtual SError, or panic if the error is fatal. Sometime the error
address recorded by APEI may be invalid(not accurate), and SIGBUS is not
delivered to userspace. For this case, to make sure notify guest, userspace
still inject virtual SError with specify syndrome to guest. The specify syndrome
will be set to the VSESR_EL2. VSESR_EL2 is a new ARMv8.2 RAS extensions register
which provides the syndrome value reported to software on taking a virtual
SError interrupt exception. By default specify this syndrome value 
to IMPLEMENTATION DEFINED, because all-zero  means 'RAS error: Uncategorized'
instead of 'no valid ISS'.


Dongjiu Geng (4):
  arm64: kvm: route synchronous external abort exceptions to EL2
  arm64: kvm: Introduce KVM_ARM_SET_SERROR_ESR ioctl
  arm64: kvm: Set Virtual SError Exception Syndrome for guest
  arm64: kvm: handle SEI notification for guest

 Documentation/virtual/kvm/api.txt    | 11 +++++++
 arch/arm/include/asm/kvm_host.h      |  1 +
 arch/arm/kvm/guest.c                 |  9 ++++++
 arch/arm64/include/asm/esr.h         | 10 ++++++
 arch/arm64/include/asm/kvm_arm.h     |  2 ++
 arch/arm64/include/asm/kvm_asm.h     |  1 +
 arch/arm64/include/asm/kvm_emulate.h | 17 ++++++++++
 arch/arm64/include/asm/kvm_host.h    | 11 +++++++
 arch/arm64/include/asm/sysreg.h      | 13 ++++++++
 arch/arm64/include/asm/system_misc.h |  2 +-
 arch/arm64/kvm/guest.c               | 14 +++++++++
 arch/arm64/kvm/handle_exit.c         | 61 +++++++++++++++++++++++++++++++++---
 arch/arm64/kvm/hyp/switch.c          | 15 +++++++++
 arch/arm64/kvm/inject_fault.c        | 13 +++++++-
 arch/arm64/kvm/reset.c               |  3 ++
 arch/arm64/kvm/sys_regs.c            | 40 +++++++++++++++++++++++
 arch/arm64/mm/fault.c                | 16 ++++++++++
 include/uapi/linux/kvm.h             |  3 ++
 virt/kvm/arm/arm.c                   |  7 +++++
 19 files changed, 243 insertions(+), 6 deletions(-)

-- 
2.10.1

[PATCH v7 1/4] arm64: kvm: route synchronous external abort exceptions to EL2

[Dongjiu Geng]

ARMv8.2 adds a new bit HCR_EL2.TEA which controls to
route synchronous external aborts to EL2, and adds a
trap control bit HCR_EL2.TERR which controls to
trap all Non-secure EL1&0 error record accesses to EL2.

This patch enables the two bits for the guest OS.
when an synchronous abort is generated in the guest OS,
it will trap to EL3 firmware, EL3 firmware will check the
HCR_EL2.TEA value to decide to jump to hypervisor or host
OS. Enabling HCR_EL2.TERR makes error record access
from guest trap to EL2.

Add some minimal emulation for RAS-Error-Record registers.
In the emulation, ERRIDR_EL1 and ERRSELR_EL1 are zero.
Then, the others ERX* registers are RAZ/WI.

Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
---
 arch/arm64/include/asm/kvm_arm.h     |  2 ++
 arch/arm64/include/asm/kvm_emulate.h |  7 +++++++
 arch/arm64/include/asm/kvm_host.h    |  2 ++
 arch/arm64/include/asm/sysreg.h      | 10 +++++++++
 arch/arm64/kvm/sys_regs.c            | 40 ++++++++++++++++++++++++++++++++++++
 5 files changed, 61 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 61d694c..1188272 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -23,6 +23,8 @@
 #include <asm/types.h>
 
 /* Hyp Configuration Register (HCR) bits */
+#define HCR_TEA		(UL(1) << 37)
+#define HCR_TERR	(UL(1) << 36)
 #define HCR_E2H		(UL(1) << 34)
 #define HCR_ID		(UL(1) << 33)
 #define HCR_CD		(UL(1) << 32)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index fe39e68..47983db 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -47,6 +47,13 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
 	vcpu->arch.hcr_el2 = HCR_GUEST_FLAGS;
 	if (is_kernel_in_hyp_mode())
 		vcpu->arch.hcr_el2 |= HCR_E2H;
+	if (cpus_have_const_cap(ARM64_HAS_RAS_EXTN)) {
+		/* route synchronous external abort exceptions to EL2 */
+		vcpu->arch.hcr_el2 |= HCR_TEA;
+		/* trap error record accesses */
+		vcpu->arch.hcr_el2 |= HCR_TERR;
+	}
+
 	if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features))
 		vcpu->arch.hcr_el2 &= ~HCR_RW;
 }
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index d686300..af55b3bc 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -105,6 +105,8 @@ enum vcpu_sysreg {
 	TTBR1_EL1,	/* Translation Table Base Register 1 */
 	TCR_EL1,	/* Translation Control Register */
 	ESR_EL1,	/* Exception Syndrome Register */
+	ERRIDR_EL1,     /* Error Record ID Register */
+	ERRSELR_EL1,    /* Error Record Select Register */
 	AFSR0_EL1,	/* Auxiliary Fault Status Register 0 */
 	AFSR1_EL1,	/* Auxiliary Fault Status Register 1 */
 	FAR_EL1,	/* Fault Address Register */
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 35b786b..bd11ca0 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -169,6 +169,16 @@
 #define SYS_AFSR0_EL1			sys_reg(3, 0, 5, 1, 0)
 #define SYS_AFSR1_EL1			sys_reg(3, 0, 5, 1, 1)
 #define SYS_ESR_EL1			sys_reg(3, 0, 5, 2, 0)
+
+#define SYS_ERRIDR_EL1			sys_reg(3, 0, 5, 3, 0)
+#define SYS_ERRSELR_EL1			sys_reg(3, 0, 5, 3, 1)
+#define SYS_ERXFR_EL1			sys_reg(3, 0, 5, 4, 0)
+#define SYS_ERXCTLR_EL1			sys_reg(3, 0, 5, 4, 1)
+#define SYS_ERXSTATUS_EL1		sys_reg(3, 0, 5, 4, 2)
+#define SYS_ERXADDR_EL1			sys_reg(3, 0, 5, 4, 3)
+#define SYS_ERXMISC0_EL1		sys_reg(3, 0, 5, 5, 0)
+#define SYS_ERXMISC1_EL1		sys_reg(3, 0, 5, 5, 1)
+
 #define SYS_FAR_EL1			sys_reg(3, 0, 6, 0, 0)
 #define SYS_PAR_EL1			sys_reg(3, 0, 7, 4, 0)
 
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 2e070d3..a74617b 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -775,6 +775,36 @@ static bool access_pmovs(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 	return true;
 }
 
+static bool access_error_reg(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+			 const struct sys_reg_desc *r)
+{
+	/* accessing ERRIDR_EL1 */
+	if (r->CRm == 3 && r->Op2 == 0) {
+		if (p->is_write)
+			vcpu_sys_reg(vcpu, ERRIDR_EL1) = 0;
+
+		return trap_raz_wi(vcpu, p, r);
+	}
+
+	/* accessing ERRSELR_EL1 */
+	if (r->CRm == 3 && r->Op2 == 1) {
+		if (p->is_write)
+			vcpu_sys_reg(vcpu, ERRSELR_EL1) = 0;
+
+		return trap_raz_wi(vcpu, p, r);
+	}
+
+	/*
+	 * If ERRSELR_EL1.SEL is greater than or equal to ERRIDR_EL1.NUM,
+	 * the ERX* registers are RAZ/WI.
+	 */
+	if ((vcpu_sys_reg(vcpu, ERRSELR_EL1) & 0xff) >=
+			(vcpu_sys_reg(vcpu, ERRIDR_EL1) && 0xff))
+		return trap_raz_wi(vcpu, p, r);
+
+	return true;
+}
+
 static bool access_pmswinc(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 			   const struct sys_reg_desc *r)
 {
@@ -953,6 +983,16 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_AFSR0_EL1), access_vm_reg, reset_unknown, AFSR0_EL1 },
 	{ SYS_DESC(SYS_AFSR1_EL1), access_vm_reg, reset_unknown, AFSR1_EL1 },
 	{ SYS_DESC(SYS_ESR_EL1), access_vm_reg, reset_unknown, ESR_EL1 },
+
+	{ SYS_DESC(SYS_ERRIDR_EL1), access_error_reg, reset_val, ERRIDR_EL1, 0 },
+	{ SYS_DESC(SYS_ERRSELR_EL1), access_error_reg, reset_val, ERRSELR_EL1, 0 },
+	{ SYS_DESC(SYS_ERXFR_EL1), access_error_reg },
+	{ SYS_DESC(SYS_ERXCTLR_EL1), access_error_reg },
+	{ SYS_DESC(SYS_ERXSTATUS_EL1), access_error_reg },
+	{ SYS_DESC(SYS_ERXADDR_EL1), access_error_reg },
+	{ SYS_DESC(SYS_ERXMISC0_EL1), access_error_reg },
+	{ SYS_DESC(SYS_ERXMISC1_EL1), access_error_reg },
+
 	{ SYS_DESC(SYS_FAR_EL1), access_vm_reg, reset_unknown, FAR_EL1 },
 	{ SYS_DESC(SYS_PAR_EL1), NULL, reset_unknown, PAR_EL1 },
 
-- 
2.10.1

[PATCH v7 2/4] arm64: kvm: Introduce KVM_ARM_SET_SERROR_ESR ioctl

[Dongjiu Geng]

On ARM64 SError syndrome value is specific to the model being
emulated for the guest and user space needs a way to tell the
kernel this value. userspace can specify different value to
affect guest OS error recovery behaviour.

We make this API ARM-specific as we haven't yet reached a consensus
for a generic API for all KVM architectures that will allow us to
do something like this.

Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
Signed-off-by: Quanming Wu <wuquanming@huawei.com>
---
 Documentation/virtual/kvm/api.txt | 11 +++++++++++
 arch/arm/include/asm/kvm_host.h   |  1 +
 arch/arm/kvm/guest.c              |  9 +++++++++
 arch/arm64/include/asm/kvm_host.h |  2 ++
 arch/arm64/kvm/guest.c            |  5 +++++
 arch/arm64/kvm/reset.c            |  3 +++
 include/uapi/linux/kvm.h          |  3 +++
 virt/kvm/arm/arm.c                |  7 +++++++
 8 files changed, 41 insertions(+)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index e63a35f..b076fc8 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -4347,3 +4347,14 @@ This capability indicates that userspace can load HV_X64_MSR_VP_INDEX msr.  Its
 value is used to denote the target vcpu for a SynIC interrupt.  For
 compatibilty, KVM initializes this msr to KVM's internal vcpu index.  When this
 capability is absent, userspace can still query this msr's value.
+
+8.13 KVM_CAP_ARM_SET_SERROR_ESR
+
+Architectures: arm, arm64
+
+This capability indicates that userspace can specify syndrome value reported to
+software on taking a virtual SError interrupt exception.
+Userspace can only specify the ISS field for the ESR syndrome, EC field is set
+by hardware when the virtual SError interrupt is taken. If this virtual SError
+is taken to EL1 using AArch64, this value will be reported in ESR_EL1, otherwise
+if it is taken to EL1 using AArch32, this value will be reported in DFSR.{AET, ExT}.
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 127e2dd..3723138 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -211,6 +211,7 @@ 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_set_sei_esr(struct kvm_vcpu *vcpu, u64 *syndrome);
 unsigned long kvm_call_hyp(void *hypfn, ...);
 void force_vm_exit(const cpumask_t *mask);
 
diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
index 1e0784e..309b236 100644
--- a/arch/arm/kvm/guest.c
+++ b/arch/arm/kvm/guest.c
@@ -248,6 +248,15 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 	return -EINVAL;
 }
 
+/*
+ * we only support SEI injection with specified synchronous
+ * in ARM64, not support it in ARM32.
+ */
+int kvm_arm_set_sei_esr(struct kvm_vcpu *vcpu, u64 *syndrome)
+{
+	return -EINVAL;
+}
+
 int __attribute_const__ kvm_target_cpu(void)
 {
 	switch (read_cpuid_part()) {
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index af55b3bc..dd4cb25 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -320,6 +320,8 @@ 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_set_sei_esr(struct kvm_vcpu *vcpu, u64 *syndrome);
+
 #define KVM_ARCH_WANT_MMU_NOTIFIER
 int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
 int kvm_unmap_hva_range(struct kvm *kvm,
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 5c7f657..0a08b05 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -277,6 +277,11 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 	return -EINVAL;
 }
 
+int kvm_arm_set_sei_esr(struct kvm_vcpu *vcpu, u64 *syndrome)
+{
+	return -EINVAL;
+}
+
 int __attribute_const__ kvm_target_cpu(void)
 {
 	unsigned long implementor = read_cpuid_implementor();
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 3256b92..9163628 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_SET_SERROR_ESR:
+		r = cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
+		break;
 	case KVM_CAP_SET_GUEST_DEBUG:
 	case KVM_CAP_VCPU_ATTRIBUTES:
 		r = 1;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 6cd63c1..aece611 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -929,6 +929,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_PPC_SMT_POSSIBLE 147
 #define KVM_CAP_HYPERV_SYNIC2 148
 #define KVM_CAP_HYPERV_VP_INDEX 149
+#define KVM_CAP_ARM_SET_SERROR_ESR 150
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -1355,6 +1356,8 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_S390_CMMA_MIGRATION */
 #define KVM_S390_GET_CMMA_BITS      _IOWR(KVMIO, 0xb8, struct kvm_s390_cmma_log)
 #define KVM_S390_SET_CMMA_BITS      _IOW(KVMIO, 0xb9, struct kvm_s390_cmma_log)
+/* Available with KVM_CAP_ARM_SET_SERROR_ESR */
+#define KVM_ARM_SET_SERROR_ESR       _IOW(KVMIO,  0xb10, __u64)
 
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index a39a1e1..2d8abeb 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1022,6 +1022,13 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 			return -EFAULT;
 		return kvm_arm_vcpu_has_attr(vcpu, &attr);
 	}
+	case KVM_ARM_SET_SERROR_ESR: {
+		u64 syndrome;
+
+		if (copy_from_user(&syndrome, argp, sizeof(syndrome)))
+			return -EFAULT;
+		return kvm_arm_set_sei_esr(vcpu, &syndrome);
+	}
 	default:
 		return -EINVAL;
 	}
-- 
2.10.1

[PATCH v7 3/4] arm64: kvm: Set Virtual SError Exception Syndrome for guest

[Dongjiu Geng]

RAS Extension add a VSESR_EL2 register which can provides
the syndrome value reported to software on taking a virtual
SError interrupt exception. This patch supports to specify
this Syndrome.

In the RAS Extensions we can not set all-zero syndrome value
for SError, which means 'RAS error: Uncategorized' instead of
'no valid ISS'. So set it to IMPLEMENTATION DEFINED syndrome
by default.

we also need to support userspace to specify a valid syndrome
value, Because in some case, the recovery is driven by userspace.
This patch can support that serspace can specify it.

In the guest/host world switch, restore this value to VSESR_EL2
only when HCR_EL2.VSE is set. This value no need to be saved
because it is stale vale when guest exit.

Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
Signed-off-by: Quanming Wu <wuquanming@huawei.com>
[Set an impdef ESR for Virtual-SError]
Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/arm64/include/asm/kvm_emulate.h | 10 ++++++++++
 arch/arm64/include/asm/kvm_host.h    |  1 +
 arch/arm64/include/asm/sysreg.h      |  3 +++
 arch/arm64/include/asm/system_misc.h |  1 -
 arch/arm64/kvm/guest.c               | 11 ++++++++++-
 arch/arm64/kvm/hyp/switch.c          | 15 +++++++++++++++
 arch/arm64/kvm/inject_fault.c        | 13 ++++++++++++-
 7 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 47983db..74213bd 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -155,6 +155,16 @@ static inline u32 kvm_vcpu_get_hsr(const struct kvm_vcpu *vcpu)
 	return vcpu->arch.fault.esr_el2;
 }
 
+static inline u32 kvm_vcpu_get_vsesr(const struct kvm_vcpu *vcpu)
+{
+	return vcpu->arch.fault.vsesr_el2;
+}
+
+static inline void kvm_vcpu_set_vsesr(struct kvm_vcpu *vcpu, unsigned long val)
+{
+	vcpu->arch.fault.vsesr_el2 = val;
+}
+
 static inline int kvm_vcpu_get_condition(const struct kvm_vcpu *vcpu)
 {
 	u32 esr = kvm_vcpu_get_hsr(vcpu);
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index dd4cb25..1a0c07e 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -88,6 +88,7 @@ struct kvm_vcpu_fault_info {
 	u32 esr_el2;		/* Hyp Syndrom Register */
 	u64 far_el2;		/* Hyp Fault Address Register */
 	u64 hpfar_el2;		/* Hyp IPA Fault Address Register */
+	u32 vsesr_el2;          /* Virtual SError Exception Syndrome Register */
 };
 
 /*
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index bd11ca0..e6c7fb8 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -86,6 +86,9 @@
 #define REG_PSTATE_PAN_IMM		sys_reg(0, 0, 4, 0, 4)
 #define REG_PSTATE_UAO_IMM		sys_reg(0, 0, 4, 0, 3)
 
+/* virtual SError exception syndrome register in armv8.2 */
+#define REG_VSESR_EL2                  sys_reg(3, 4, 5, 2, 3)
+
 #define SET_PSTATE_PAN(x) __emit_inst(0xd5000000 | REG_PSTATE_PAN_IMM |	\
 				      (!!x)<<8 | 0x1f)
 #define SET_PSTATE_UAO(x) __emit_inst(0xd5000000 | REG_PSTATE_UAO_IMM |	\
diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
index 07aa8e3..e70cb61 100644
--- a/arch/arm64/include/asm/system_misc.h
+++ b/arch/arm64/include/asm/system_misc.h
@@ -57,7 +57,6 @@ extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
 })
 
 int handle_guest_sea(phys_addr_t addr, unsigned int esr);
-
 #endif	/* __ASSEMBLY__ */
 
 #endif	/* __ASM_SYSTEM_MISC_H */
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 0a08b05..a9acbfd 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -279,7 +279,16 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 
 int kvm_arm_set_sei_esr(struct kvm_vcpu *vcpu, u64 *syndrome)
 {
-	return -EINVAL;
+	u64 reg = *syndrome;
+
+	/* inject virtual system Error or asynchronous abort */
+	kvm_inject_vabt(vcpu);
+
+	if (reg)
+		/* set vsesr_el2[24:0] with value that user space specified */
+		kvm_vcpu_set_vsesr(vcpu, reg & ESR_ELx_ISS_MASK);
+
+	return 0;
 }
 
 int __attribute_const__ kvm_target_cpu(void)
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index c6f17c7..7c0915e 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -67,6 +67,14 @@ static hyp_alternate_select(__activate_traps_arch,
 			    __activate_traps_nvhe, __activate_traps_vhe,
 			    ARM64_HAS_VIRT_HOST_EXTN);
 
+static void __hyp_text __sysreg_set_vsesr(struct kvm_vcpu *vcpu)
+{
+	if (cpus_have_const_cap(ARM64_HAS_RAS_EXTN) &&
+		(vcpu->arch.hcr_el2 & HCR_VSE))
+		write_sysreg_s(vcpu->arch.fault.vsesr_el2, REG_VSESR_EL2);
+}
+
+
 static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
 {
 	u64 val;
@@ -86,6 +94,13 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
 		isb();
 	}
 	write_sysreg(val, hcr_el2);
+
+	/*
+	 * If the virtual SError interrupt is taken to EL1 using AArch64,
+	 * then VSESR_EL2 provides the syndrome value reported in ESR_EL1.
+	 */
+	__sysreg_set_vsesr(vcpu);
+
 	/* Trap on AArch32 cp15 c15 accesses (EL1 or EL0) */
 	write_sysreg(1 << 15, hstr_el2);
 	/*
diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
index da6a8cf..2457222 100644
--- a/arch/arm64/kvm/inject_fault.c
+++ b/arch/arm64/kvm/inject_fault.c
@@ -232,14 +232,25 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
 		inject_undef64(vcpu);
 }
 
+static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
+{
+	kvm_vcpu_set_vsesr(vcpu, vcpu_get_hcr(vcpu) | HCR_VSE);
+	vcpu_set_hcr(vcpu, vcpu_get_hcr(vcpu) | HCR_VSE);
+}
+
 /**
  * kvm_inject_vabt - inject an async abort / SError into the guest
  * @vcpu: The VCPU to receive the exception
  *
  * It is assumed that this code is called from the VCPU thread and that the
  * VCPU therefore is not currently executing guest code.
+ *
+ * Systems with the RAS Extensions specify an imp-def ESR (ISV/IDS = 1) with
+ * the remaining ISS all-zeros so that this error is not interpreted as an
+ * uncatagorized RAS error. Without the RAS Extensions we can't specify an ESR
+ * value, so the CPU generates an imp-def value.
  */
 void kvm_inject_vabt(struct kvm_vcpu *vcpu)
 {
-	vcpu_set_hcr(vcpu, vcpu_get_hcr(vcpu) | HCR_VSE);
+	pend_guest_serror(vcpu, ESR_ELx_ISV);
 }
-- 
2.10.1

[PATCH v7 4/4] arm64: kvm: handle SEI notification for guest

[Dongjiu Geng]

when KVM received SError, it firstly classified the error
according to [ESR_ELx.AET]. If the SEI is Uncategorized or
IMPLEMENTATION DEFINED directly inject virtual SError with
IMPLEMENTATION DEFINED syndrome.

If the SError error is not propagated, then let host to handle
it. Because usually the address recorded by APEI table is not
accurate, so can not identify the address to hwpoison memory and
can not notify guest to do the recovery, so at the same time, let
user space specify a valid ESR and inject virtual SError.

Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
Signed-off-by: Quanming Wu <wuquanming@huawei.com>
---
 arch/arm64/include/asm/esr.h         | 15 +++++++++
 arch/arm64/include/asm/kvm_asm.h     |  1 +
 arch/arm64/include/asm/kvm_host.h    |  6 ++++
 arch/arm64/include/asm/system_misc.h |  1 +
 arch/arm64/kvm/handle_exit.c         | 62 +++++++++++++++++++++++++++++++++---
 arch/arm64/mm/fault.c                | 16 ++++++++++
 6 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index 8cabd57..e5d23a8f 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -95,6 +95,7 @@
 #define ESR_ELx_FSC_ACCESS	(0x08)
 #define ESR_ELx_FSC_FAULT	(0x04)
 #define ESR_ELx_FSC_PERM	(0x0C)
+#define ESR_ELx_FSC_SERROR	(0x11)
 
 /* ISS field definitions for Data Aborts */
 #define ESR_ELx_ISV		(UL(1) << 24)
@@ -107,6 +108,20 @@
 #define ESR_ELx_AR 		(UL(1) << 14)
 #define ESR_ELx_CM 		(UL(1) << 8)
 
+/* ISS field definitions for SError interrupt */
+#define ESR_ELx_AET_SHIFT	(10)
+#define ESR_ELx_AET		(UL(0x7) << ESR_ELx_AET_SHIFT)
+/* Uncontainable error */
+#define ESR_ELx_AET_UC		(UL(0) << ESR_ELx_AET_SHIFT)
+/* Unrecoverable error */
+#define ESR_ELx_AET_UEU		(UL(1) << ESR_ELx_AET_SHIFT)
+/* Restartable error */
+#define ESR_ELx_AET_UEO		(UL(2) << ESR_ELx_AET_SHIFT)
+/* Recoverable error */
+#define ESR_ELx_AET_UER		(UL(3) << ESR_ELx_AET_SHIFT)
+/* Corrected */
+#define ESR_ELx_AET_CE		(UL(6) << ESR_ELx_AET_SHIFT)
+
 /* ISS field definitions for exceptions taken in to Hyp */
 #define ESR_ELx_CV		(UL(1) << 24)
 #define ESR_ELx_COND_SHIFT	(20)
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index 26a64d0..d167ed3 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -27,6 +27,7 @@
 #define ARM_EXCEPTION_IRQ	  0
 #define ARM_EXCEPTION_EL1_SERROR  1
 #define ARM_EXCEPTION_TRAP	  2
+
 /* The hyp-stub will return this for any kvm_call_hyp() call */
 #define ARM_EXCEPTION_HYP_GONE	  HVC_STUB_ERR
 
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 1a0c07e..c0da350 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -395,4 +395,10 @@ static inline void __cpu_init_stage2(void)
 		  "PARange is %d bits, unsupported configuration!", parange);
 }
 
+enum {
+	KVM_SEI_SEV_RECOVERABLE = 1,
+	KVM_SEI_SEV_FATAL,
+	KVM_SEI_SEV_CORRECTED,
+};
+
 #endif /* __ARM64_KVM_HOST_H__ */
diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
index e70cb61..73b45fb 100644
--- a/arch/arm64/include/asm/system_misc.h
+++ b/arch/arm64/include/asm/system_misc.h
@@ -57,6 +57,7 @@ extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
 })
 
 int handle_guest_sea(phys_addr_t addr, unsigned int esr);
+int handle_guest_sei(unsigned int esr);
 #endif	/* __ASSEMBLY__ */
 
 #endif	/* __ASM_SYSTEM_MISC_H */
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 17d8a16..d546d55 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -28,6 +28,7 @@
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_mmu.h>
 #include <asm/kvm_psci.h>
+#include <asm/system_misc.h>
 
 #define CREATE_TRACE_POINTS
 #include "trace.h"
@@ -178,6 +179,61 @@ static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
 	return arm_exit_handlers[hsr_ec];
 }
 
+/**
+ * kvm_handle_guest_sei - handles SError interrupt or asynchronous aborts
+ * @vcpu:	the VCPU pointer
+ *
+ * For RAS SError interrupt, if the AET is [ESR_ELx_AET_UEU] or
+ * [ESR_ELx_AET_UER], then let host user space do the recovery,
+ * otherwise, directly inject virtual SError to guest or panic.
+ */
+static int kvm_handle_guest_sei(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+	unsigned int esr = kvm_vcpu_get_hsr(vcpu);
+	bool impdef_syndrome =  esr & ESR_ELx_ISV;	/* aka IDS */
+	unsigned int aet = esr & ESR_ELx_AET;
+
+	run->exit_reason = KVM_EXIT_EXCEPTION;
+	run->ex.exception = ARM_EXCEPTION_EL1_SERROR;
+
+	/*
+	 * In below three conditions, it will directly inject the
+	 * virtual SError:
+	 * 1. Not support RAS extension
+	 * 2. The Syndrome is IMPLEMENTATION DEFINED
+	 * 3. It is Uncategorized SEI
+	 */
+	if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN) || impdef_syndrome ||
+			((esr & ESR_ELx_FSC) != ESR_ELx_FSC_SERROR)) {
+		kvm_inject_vabt(vcpu);
+		return 1;
+	}
+
+	switch (aet) {
+	case ESR_ELx_AET_CE:	/* corrected error */
+	case ESR_ELx_AET_UEO:	/* restartable error, not yet consumed */
+		break;		/* continue processing the guest exit */
+	case ESR_ELx_AET_UEU:
+	case ESR_ELx_AET_UER:	/* The error has not been propagated */
+	/*
+	 * Only handle the guest SEI if the error has not been propagated
+	 */
+	if (!handle_guest_sei(kvm_vcpu_get_hsr(vcpu))) {
+		run->ex.error_code = KVM_SEI_SEV_RECOVERABLE;
+		return 0;
+	}
+	/* If SError handling is failed, continue run, panic */
+	default:
+		/*
+		 * Until now, the CPU supports RAS and SEI is fatal, or host
+		 * does not support to handle the SError.
+		 */
+		panic("This Asynchronous SError interrupt is dangerous, panic");
+	}
+
+	return 0;
+}
+
 /*
  * Return > 0 to return to guest, < 0 on error, 0 (and set exit_reason) on
  * proper exit to userspace.
@@ -201,8 +257,7 @@ int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
 			*vcpu_pc(vcpu) -= adj;
 		}
 
-		kvm_inject_vabt(vcpu);
-		return 1;
+		return kvm_handle_guest_sei(vcpu, run);
 	}
 
 	exception_index = ARM_EXCEPTION_CODE(exception_index);
@@ -211,8 +266,7 @@ int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	case ARM_EXCEPTION_IRQ:
 		return 1;
 	case ARM_EXCEPTION_EL1_SERROR:
-		kvm_inject_vabt(vcpu);
-		return 1;
+		return kvm_handle_guest_sei(vcpu, run);
 	case ARM_EXCEPTION_TRAP:
 		/*
 		 * See ARM ARM B1.14.1: "Hyp traps on instructions
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index c98c1b3..dce7ab9 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -688,6 +688,22 @@ int handle_guest_sea(phys_addr_t addr, unsigned int esr)
 }
 
 /*
+ * Handle SError interrupt that occurred in guest OS.
+ *
+ * The return value will be zero if the SEI was successfully handled
+ * and non-zero if handling is failed.
+ */
+int handle_guest_sei(unsigned int esr)
+{
+	int ret = -ENOENT;
+
+	if (IS_ENABLED(CONFIG_ACPI_APEI_SEI))
+		ret = ghes_notify_abort(ACPI_HEST_NOTIFY_SEI);
+
+	return ret;
+}
+
+/*
  * Dispatch a data abort to the relevant handler.
  */
 asmlinkage void __exception do_mem_abort(unsigned long addr, unsigned int esr,
-- 
2.10.1

[PATCH v7 1/4] arm64: kvm: route synchronous external abort exceptions to EL2

[James Morse]

Hi Dongjiu Geng,

On 17/10/17 15:14, Dongjiu Geng wrote:
> ARMv8.2 adds a new bit HCR_EL2.TEA which controls to
> route synchronous external aborts to EL2, and adds a
> trap control bit HCR_EL2.TERR which controls to
> trap all Non-secure EL1&0 error record accesses to EL2.

The bulk of this patch is about trap-and-emulating these ERR registers, but
that's not reflected in the title:
> KVM: arm64: Emulate RAS error registers and set HCR_EL2's TERR & TEA


> This patch enables the two bits for the guest OS.
> when an synchronous abort is generated in the guest OS,
> it will trap to EL3 firmware, EL3 firmware will check the

*buzz*
This depends on SCR_EL3.EA, which this patch doesn't touch and the normal-world
can't even know about. This is what your system does, the commit message should
be about the change to Linux.

(I've said this before)


> HCR_EL2.TEA value to decide to jump to hypervisor or host
> OS. Enabling HCR_EL2.TERR makes error record access
> from guest trap to EL2.
> 
> Add some minimal emulation for RAS-Error-Record registers.
> In the emulation, ERRIDR_EL1 and ERRSELR_EL1 are zero.
> Then, the others ERX* registers are RAZ/WI.

> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index fe39e68..47983db 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -47,6 +47,13 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
>  	vcpu->arch.hcr_el2 = HCR_GUEST_FLAGS;
>  	if (is_kernel_in_hyp_mode())
>  		vcpu->arch.hcr_el2 |= HCR_E2H;
> +	if (cpus_have_const_cap(ARM64_HAS_RAS_EXTN)) {

This ARM64_HAS_RAS_EXTN isn't in mainline, nor is it added by your series. I
know where it comes from, but other reviewers may not. If you have dependencies
on another series, please call them out in the cover letter.

This is the first cpus_have_const_cap() user in this header file, it probably needs:
#include <asm/cpufeature.h>


> +		/* route synchronous external abort exceptions to EL2 */
> +		vcpu->arch.hcr_el2 |= HCR_TEA;
> +		/* trap error record accesses */
> +		vcpu->arch.hcr_el2 |= HCR_TERR;
> +	}
> +
>  	if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features))
>  		vcpu->arch.hcr_el2 &= ~HCR_RW;
>  }
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index d686300..af55b3bc 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -105,6 +105,8 @@ enum vcpu_sysreg {
>  	TTBR1_EL1,	/* Translation Table Base Register 1 */
>  	TCR_EL1,	/* Translation Control Register */
>  	ESR_EL1,	/* Exception Syndrome Register */

> +	ERRIDR_EL1,     /* Error Record ID Register */

Page 39 of [0]: 'ERRIDR_EL1 is a 64-bit read-only ...'.


> +	ERRSELR_EL1,    /* Error Record Select Register */

We're emulating these as RAZ/WI, do we really need to allocate
vcpu->arch.ctxt.sys_regs space for them? If we always return 0 for ERRIDR, then
we don't need to keep ERRSELR as 'the value read back [..] is UNKNOWN'.

I think we only need space for these once their value needs to be migrated,
user-space doesn't need to know they exist until then.


>  	AFSR0_EL1,	/* Auxiliary Fault Status Register 0 */
>  	AFSR1_EL1,	/* Auxiliary Fault Status Register 1 */
>  	FAR_EL1,	/* Fault Address Register */

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 2e070d3..a74617b 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -775,6 +775,36 @@ static bool access_pmovs(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>  	return true;
>  }
>  
> +static bool access_error_reg(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> +			 const struct sys_reg_desc *r)
> +{
> +	/* accessing ERRIDR_EL1 */
> +	if (r->CRm == 3 && r->Op2 == 0) {
> +		if (p->is_write)
> +			vcpu_sys_reg(vcpu, ERRIDR_EL1) = 0;

As above, this register is read-only.


> +		return trap_raz_wi(vcpu, p, r);

If we can get rid of the vcpu storage this just becomes trap_raz_wi() .


> +	}
> +
> +	/* accessing ERRSELR_EL1 */
> +	if (r->CRm == 3 && r->Op2 == 1) {
> +		if (p->is_write)
> +			vcpu_sys_reg(vcpu, ERRSELR_EL1) = 0;
> +
> +		return trap_raz_wi(vcpu, p, r);
> +	}

Same here.


> +
> +	/*
> +	 * If ERRSELR_EL1.SEL is greater than or equal to ERRIDR_EL1.NUM,
> +	 * the ERX* registers are RAZ/WI.
> +	 */
> +	if ((vcpu_sys_reg(vcpu, ERRSELR_EL1) & 0xff) >=
> +			(vcpu_sys_reg(vcpu, ERRIDR_EL1) && 0xff))
> +		return trap_raz_wi(vcpu, p, r);

The trick here is ERRIDR_EL1 is read only, KVM can choose how many records it
emulates. Let's pick zero:
> Zero indicates no records can be accessed through the Error Record System
> registers.

Which lets us collapse this entire function down to trap_raz_wi().


I agree we will need this code once we need to allow user-space to populate
values for these registers. (and I bet no-one will want to do that until we get
kernel-first support)


> +	return true;
> +}
> +
>  static bool access_pmswinc(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>  			   const struct sys_reg_desc *r)
>  {
> @@ -953,6 +983,16 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  	{ SYS_DESC(SYS_AFSR0_EL1), access_vm_reg, reset_unknown, AFSR0_EL1 },
>  	{ SYS_DESC(SYS_AFSR1_EL1), access_vm_reg, reset_unknown, AFSR1_EL1 },
>  	{ SYS_DESC(SYS_ESR_EL1), access_vm_reg, reset_unknown, ESR_EL1 },
> +
> +	{ SYS_DESC(SYS_ERRIDR_EL1), access_error_reg, reset_val, ERRIDR_EL1, 0 },
> +	{ SYS_DESC(SYS_ERRSELR_EL1), access_error_reg, reset_val, ERRSELR_EL1, 0 },
> +	{ SYS_DESC(SYS_ERXFR_EL1), access_error_reg },
> +	{ SYS_DESC(SYS_ERXCTLR_EL1), access_error_reg },
> +	{ SYS_DESC(SYS_ERXSTATUS_EL1), access_error_reg },
> +	{ SYS_DESC(SYS_ERXADDR_EL1), access_error_reg },
> +	{ SYS_DESC(SYS_ERXMISC0_EL1), access_error_reg },
> +	{ SYS_DESC(SYS_ERXMISC1_EL1), access_error_reg },

Why can't we just make all these trap_raz_wi()?


>  	{ SYS_DESC(SYS_FAR_EL1), access_vm_reg, reset_unknown, FAR_EL1 },
>  	{ SYS_DESC(SYS_PAR_EL1), NULL, reset_unknown, PAR_EL1 },


I'll make these changes and then repost this as part of the RAS+IESB series it
depends on.


Thanks!

James


[0]
https://static.docs.arm.com/ddi0587/a/RAS%20Extension-release%20candidate_march_29.pdf

[PATCH v7 1/4] arm64: kvm: route synchronous external abort exceptions to EL2

[gengdongjiu]

Hi james,
  Thanks for the mail and sorry for my late response.


2017-10-19 1:21 GMT+08:00, James Morse <james.morse@arm.com>:
> Hi Dongjiu Geng,
>
> On 17/10/17 15:14, Dongjiu Geng wrote:
>> ARMv8.2 adds a new bit HCR_EL2.TEA which controls to
>> route synchronous external aborts to EL2, and adds a
>> trap control bit HCR_EL2.TERR which controls to
>> trap all Non-secure EL1&0 error record accesses to EL2.
>
> The bulk of this patch is about trap-and-emulating these ERR registers, but
> that's not reflected in the title:
>> KVM: arm64: Emulate RAS error registers and set HCR_EL2's TERR & TEA
>
>
>> This patch enables the two bits for the guest OS.
>> when an synchronous abort is generated in the guest OS,
>> it will trap to EL3 firmware, EL3 firmware will check the
>
> *buzz*
> This depends on SCR_EL3.EA, which this patch doesn't touch and the
> normal-world
> can't even know about. This is what your system does, the commit message
> should
> be about the change to Linux.
>
> (I've said this before)

Thanks for the point out, I make this series in a hurry(you are
waiting this patch), forget to check again your comments before.

>
>
>> HCR_EL2.TEA value to decide to jump to hypervisor or host
>> OS. Enabling HCR_EL2.TERR makes error record access
>> from guest trap to EL2.
>>
>> Add some minimal emulation for RAS-Error-Record registers.
>> In the emulation, ERRIDR_EL1 and ERRSELR_EL1 are zero.
>> Then, the others ERX* registers are RAZ/WI.
>
>> diff --git a/arch/arm64/include/asm/kvm_emulate.h
>> b/arch/arm64/include/asm/kvm_emulate.h
>> index fe39e68..47983db 100644
>> --- a/arch/arm64/include/asm/kvm_emulate.h
>> +++ b/arch/arm64/include/asm/kvm_emulate.h
>> @@ -47,6 +47,13 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu
>> *vcpu)
>>  	vcpu->arch.hcr_el2 = HCR_GUEST_FLAGS;
>>  	if (is_kernel_in_hyp_mode())
>>  		vcpu->arch.hcr_el2 |= HCR_E2H;
>> +	if (cpus_have_const_cap(ARM64_HAS_RAS_EXTN)) {
>
> This ARM64_HAS_RAS_EXTN isn't in mainline, nor is it added by your series.
> I
> know where it comes from, but other reviewers may not. If you have
> dependencies
> on another series, please call them out in the cover letter.

yes, thanks for the point out.

>
> This is the first cpus_have_const_cap() user in this header file, it
> probably needs:
> #include <asm/cpufeature.h>

OK

>
>
>> +		/* route synchronous external abort exceptions to EL2 */
>> +		vcpu->arch.hcr_el2 |= HCR_TEA;
>> +		/* trap error record accesses */
>> +		vcpu->arch.hcr_el2 |= HCR_TERR;
>> +	}
>> +
>>  	if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features))
>>  		vcpu->arch.hcr_el2 &= ~HCR_RW;
>>  }
>> diff --git a/arch/arm64/include/asm/kvm_host.h
>> b/arch/arm64/include/asm/kvm_host.h
>> index d686300..af55b3bc 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -105,6 +105,8 @@ enum vcpu_sysreg {
>>  	TTBR1_EL1,	/* Translation Table Base Register 1 */
>>  	TCR_EL1,	/* Translation Control Register */
>>  	ESR_EL1,	/* Exception Syndrome Register */
>
>> +	ERRIDR_EL1,     /* Error Record ID Register */
>
> Page 39 of [0]: 'ERRIDR_EL1 is a 64-bit read-only ...'.

yes, it is read-only.

>
>
>> +	ERRSELR_EL1,    /* Error Record Select Register */
>
> We're emulating these as RAZ/WI, do we really need to allocate
> vcpu->arch.ctxt.sys_regs space for them? If we always return 0 for ERRIDR,
> then
> we don't need to keep ERRSELR as 'the value read back [..] is UNKNOWN'.

https://lists.cs.columbia.edu/pipermail/kvmarm/2017-September/027176.html

" 'If ERRSELR_EL1.SEL is
[>=]  ERRIDR_EL1.NUM' that makes the ERX* registers RAZ/WI"

This is because I want to make above simulation as you suggested, if
want to make above simulation, it needs set "vcpu->arch.ctxt.sys_regs"
to 0, instead of reading from system register.

so need a space to store it

>
> I think we only need space for these once their value needs to be migrated,
> user-space doesn't need to know they exist until then.
>
>
>>  	AFSR0_EL1,	/* Auxiliary Fault Status Register 0 */
>>  	AFSR1_EL1,	/* Auxiliary Fault Status Register 1 */
>>  	FAR_EL1,	/* Fault Address Register */
>
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index 2e070d3..a74617b 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -775,6 +775,36 @@ static bool access_pmovs(struct kvm_vcpu *vcpu,
>> struct sys_reg_params *p,
>>  	return true;
>>  }
>>
>> +static bool access_error_reg(struct kvm_vcpu *vcpu, struct sys_reg_params
>> *p,
>> +			 const struct sys_reg_desc *r)
>> +{
>> +	/* accessing ERRIDR_EL1 */
>> +	if (r->CRm == 3 && r->Op2 == 0) {
>> +		if (p->is_write)
>> +			vcpu_sys_reg(vcpu, ERRIDR_EL1) = 0;
>
> As above, this register is read-only.

yes, it is my mistake.

>
>
>> +		return trap_raz_wi(vcpu, p, r);
>
> If we can get rid of the vcpu storage this just becomes trap_raz_wi() .

yes

>
>
>> +	}
>> +
>> +	/* accessing ERRSELR_EL1 */
>> +	if (r->CRm == 3 && r->Op2 == 1) {
>> +		if (p->is_write)
>> +			vcpu_sys_reg(vcpu, ERRSELR_EL1) = 0;
>> +
>> +		return trap_raz_wi(vcpu, p, r);
>> +	}
>
> Same here.
>
>
>> +
>> +	/*
>> +	 * If ERRSELR_EL1.SEL is greater than or equal to ERRIDR_EL1.NUM,
>> +	 * the ERX* registers are RAZ/WI.
>> +	 */
>> +	if ((vcpu_sys_reg(vcpu, ERRSELR_EL1) & 0xff) >=
>> +			(vcpu_sys_reg(vcpu, ERRIDR_EL1) && 0xff))
>> +		return trap_raz_wi(vcpu, p, r);
>
> The trick here is ERRIDR_EL1 is read only, KVM can choose how many records
> it
> emulates. Let's pick zero:

yes, if it is read only, just pick zero.


>> Zero indicates no records can be accessed through the Error Record System
>> registers.
>
> Which lets us collapse this entire function down to trap_raz_wi().
>
>
> I agree we will need this code once we need to allow user-space to populate
> values for these registers. (and I bet no-one will want to do that until we
> get
> kernel-first support)
>

yes.

>
>> +	return true;
>> +}
>> +
>>  static bool access_pmswinc(struct kvm_vcpu *vcpu, struct sys_reg_params
>> *p,
>>  			   const struct sys_reg_desc *r)
>>  {
>> @@ -953,6 +983,16 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>>  	{ SYS_DESC(SYS_AFSR0_EL1), access_vm_reg, reset_unknown, AFSR0_EL1 },
>>  	{ SYS_DESC(SYS_AFSR1_EL1), access_vm_reg, reset_unknown, AFSR1_EL1 },
>>  	{ SYS_DESC(SYS_ESR_EL1), access_vm_reg, reset_unknown, ESR_EL1 },
>> +
>> +	{ SYS_DESC(SYS_ERRIDR_EL1), access_error_reg, reset_val, ERRIDR_EL1, 0
>> },
>> +	{ SYS_DESC(SYS_ERRSELR_EL1), access_error_reg, reset_val, ERRSELR_EL1, 0
>> },
>> +	{ SYS_DESC(SYS_ERXFR_EL1), access_error_reg },
>> +	{ SYS_DESC(SYS_ERXCTLR_EL1), access_error_reg },
>> +	{ SYS_DESC(SYS_ERXSTATUS_EL1), access_error_reg },
>> +	{ SYS_DESC(SYS_ERXADDR_EL1), access_error_reg },
>> +	{ SYS_DESC(SYS_ERXMISC0_EL1), access_error_reg },
>> +	{ SYS_DESC(SYS_ERXMISC1_EL1), access_error_reg },
>
> Why can't we just make all these trap_raz_wi()?

https://lists.cs.columbia.edu/pipermail/kvmarm/2017-September/027176.html

you ever have below suggestion, so use a function "access_error_reg" to do it.
'If ERRSELR_EL1.SEL is
[>=]  ERRIDR_EL1.NUM' that makes the ERX* registers RAZ/WI too.

make all  to  trap_raz_wi() is simple.

>
>
>>  	{ SYS_DESC(SYS_FAR_EL1), access_vm_reg, reset_unknown, FAR_EL1 },
>>  	{ SYS_DESC(SYS_PAR_EL1), NULL, reset_unknown, PAR_EL1 },
>
>
> I'll make these changes and then repost this as part of the RAS+IESB series
> it
> depends on.

Ok,  thanks

>
>
> Thanks!
>
> James
>
>
> [0]
> https://static.docs.arm.com/ddi0587/a/RAS%20Extension-release%20candidate_march_29.pdf
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
>