| Message ID | 1498481156-23572-1-git-send-email-gengdongjiu@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
correct the commit message: In the firmware-first RAS solution, OS receives an synchronous external abort, then trapped to EL3 by SCR_EL3.EA. Firmware inspects the HCR_EL2.TEA and chooses the target to send APEI's SEA notification. If the SCR_EL3.EA is set, delegates the error exception to the hypervisor, otherwise it delegates to the host OS kernel On 2017/6/26 20:45, Dongjiu Geng wrote: > In the firmware-first RAS solution, guest OS receives an synchronous > external abort, then trapped to EL3 by SCR_EL3.EA. Firmware inspects > the HCR_EL2.TEA and chooses the target to send APEI's SEA notification. > If the SCR_EL3.EA is set, delegates the error exception to the hypervisor, > otherwise it delegates to the guest OS kernel > > Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com> > --- > arch/arm64/include/asm/kvm_arm.h | 2 ++ > arch/arm64/include/asm/kvm_emulate.h | 7 +++++++ > 2 files changed, 9 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 f5ea0ba..5f64ab2 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; > } >
On Tue, Jun 27, 2017 at 08:15:49PM +0800, gengdongjiu wrote: > correct the commit message: > > In the firmware-first RAS solution, OS receives an synchronous > external abort, then trapped to EL3 by SCR_EL3.EA. Firmware inspects > the HCR_EL2.TEA and chooses the target to send APEI's SEA notification. > If the SCR_EL3.EA is set, delegates the error exception to the hypervisor, > otherwise it delegates to the host OS kernel This commit text has nothing (directly) to do with the content of the patch. Whether or not seting these bits are used by firmware to emulate injecting an exception or by the CPU raising a an exception is not the core of the issue. Please describe your change, then provide rationale. Thanks, -Christoffer > > > On 2017/6/26 20:45, Dongjiu Geng wrote: > > In the firmware-first RAS solution, guest OS receives an synchronous > > external abort, then trapped to EL3 by SCR_EL3.EA. Firmware inspects > > the HCR_EL2.TEA and chooses the target to send APEI's SEA notification. > > If the SCR_EL3.EA is set, delegates the error exception to the hypervisor, > > otherwise it delegates to the guest OS kernel > > > > Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com> > > --- > > arch/arm64/include/asm/kvm_arm.h | 2 ++ > > arch/arm64/include/asm/kvm_emulate.h | 7 +++++++ > > 2 files changed, 9 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 f5ea0ba..5f64ab2 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; > > } > > >
Hi Christoffer, On 2017/7/3 16:23, Christoffer Dall wrote: > On Tue, Jun 27, 2017 at 08:15:49PM +0800, gengdongjiu wrote: >> correct the commit message: >> >> In the firmware-first RAS solution, OS receives an synchronous >> external abort, then trapped to EL3 by SCR_EL3.EA. Firmware inspects >> the HCR_EL2.TEA and chooses the target to send APEI's SEA notification. >> If the SCR_EL3.EA is set, delegates the error exception to the hypervisor, >> otherwise it delegates to the host OS kernel > > This commit text has nothing (directly) to do with the content of the > patch. Whether or not seting these bits are used by firmware to emulate > injecting an exception or by the CPU raising a an exception is not the > core of the issue. > > Please describe your change, then provide rationale. (1)Below hcr_el2.TEA/TERR two field is introduced by armv8.2, RAS extension. TEA, bit [37] Route synchronous External Abort exceptions to EL2. The possible values of this bit are: 0 Do not route synchronous External Abort exceptions from Non-secure EL0 and EL1 to EL2. 1 Route synchronous External Abort exceptions from Non-secure EL0 and EL1 to EL2, if not routed to EL3. This bit is RES0 if the RAS extension is not implemented. TERR, bit [36] Trap Error record accesses. The possible values of this bit are: 0 Do not trap accesses to error record registers from Non-secure EL1 to EL2. 1 Accesses to the ER* registers from Non-secure EL1 generate a Trap exception to EL2. This bit is RES0 if the RAS extension is not implemented. (2) when synchronous External Abort(SEA) OS happen SEA, it trap to EL3 firmware. then the firmware needs to do by faking an exception entry to hypervisor EL2; or by faking an exception entry to EL1 so if the hcr_el2.TEA is set, firmware will eret to EL2; otherwise, eret to EL1. hcr_el2.TEA is only set for the guest OS. not set for the host OS. (3) setting hcr_el2.HCR_TERR want to trap the EL1 error record access to EL2. > > Thanks, > -Christoffer > > >> >> >> On 2017/6/26 20:45, Dongjiu Geng wrote: >>> In the firmware-first RAS solution, guest OS receives an synchronous >>> external abort, then trapped to EL3 by SCR_EL3.EA. Firmware inspects >>> the HCR_EL2.TEA and chooses the target to send APEI's SEA notification. >>> If the SCR_EL3.EA is set, delegates the error exception to the hypervisor, >>> otherwise it delegates to the guest OS kernel >>> >>> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com> >>> --- >>> arch/arm64/include/asm/kvm_arm.h | 2 ++ >>> arch/arm64/include/asm/kvm_emulate.h | 7 +++++++ >>> 2 files changed, 9 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 f5ea0ba..5f64ab2 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; >>> } >>> >> > > . >
Hi Dongjiu, On Tue, Jul 04, 2017 at 02:30:21PM +0800, gengdongjiu wrote: > Hi Christoffer, > > On 2017/7/3 16:23, Christoffer Dall wrote: > > On Tue, Jun 27, 2017 at 08:15:49PM +0800, gengdongjiu wrote: > >> correct the commit message: > >> > >> In the firmware-first RAS solution, OS receives an synchronous > >> external abort, then trapped to EL3 by SCR_EL3.EA. Firmware inspects > >> the HCR_EL2.TEA and chooses the target to send APEI's SEA notification. > >> If the SCR_EL3.EA is set, delegates the error exception to the hypervisor, > >> otherwise it delegates to the host OS kernel > > > > This commit text has nothing (directly) to do with the content of the > > patch. Whether or not seting these bits are used by firmware to emulate > > injecting an exception or by the CPU raising a an exception is not the > > core of the issue. > > > > Please describe your change, then provide rationale. > > (1)Below hcr_el2.TEA/TERR two field is introduced by armv8.2, RAS extension. > > TEA, bit [37] > Route synchronous External Abort exceptions to EL2. The possible values of this bit are: > 0 Do not route synchronous External Abort exceptions from Non-secure EL0 and EL1 to EL2. > 1 Route synchronous External Abort exceptions from Non-secure EL0 and EL1 to EL2, if not routed > to EL3. > This bit is RES0 if the RAS extension is not implemented. > TERR, bit [36] > Trap Error record accesses. The possible values of this bit are: > 0 Do not trap accesses to error record registers from Non-secure EL1 to EL2. > 1 Accesses to the ER* registers from Non-secure EL1 generate a Trap exception to EL2. > This bit is RES0 if the RAS extension is not implemented. > > (2) when synchronous External Abort(SEA) OS happen SEA, it trap to EL3 firmware. > then the firmware needs to do by faking an exception entry to hypervisor EL2; or > by faking an exception entry to EL1 > so if the hcr_el2.TEA is set, firmware will eret to EL2; otherwise, eret to EL1. > hcr_el2.TEA is only set for the guest OS. > not set for the host OS. > > > (3) setting hcr_el2.HCR_TERR want to trap the EL1 error record access to EL2. So again, I was more asking for a new commit message for the next version of the patch rather than an explanation here. But I the commit message should not mention assumptions about how EL3 firmware works, but try to stay within the semantics laid out by the ARM architecture. For example: ARMv8.2 introduces two new bits, TEA and TERR, which [...]. When RAS is available, we set these bits on the HCR_EL2, because we want to take external aborts to EL2 in order for [...]. Thanks, -Christoffer > > > > > > Thanks, > > -Christoffer > > > > > >> > >> > >> On 2017/6/26 20:45, Dongjiu Geng wrote: > >>> In the firmware-first RAS solution, guest OS receives an synchronous > >>> external abort, then trapped to EL3 by SCR_EL3.EA. Firmware inspects > >>> the HCR_EL2.TEA and chooses the target to send APEI's SEA notification. > >>> If the SCR_EL3.EA is set, delegates the error exception to the hypervisor, > >>> otherwise it delegates to the guest OS kernel > >>> > >>> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com> > >>> --- > >>> arch/arm64/include/asm/kvm_arm.h | 2 ++ > >>> arch/arm64/include/asm/kvm_emulate.h | 7 +++++++ > >>> 2 files changed, 9 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 f5ea0ba..5f64ab2 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; > >>> } > >>> > >> > > > > . > > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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 f5ea0ba..5f64ab2 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; }
In the firmware-first RAS solution, guest OS receives an synchronous external abort, then trapped to EL3 by SCR_EL3.EA. Firmware inspects the HCR_EL2.TEA and chooses the target to send APEI's SEA notification. If the SCR_EL3.EA is set, delegates the error exception to the hypervisor, otherwise it delegates to the guest OS kernel Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com> --- arch/arm64/include/asm/kvm_arm.h | 2 ++ arch/arm64/include/asm/kvm_emulate.h | 7 +++++++ 2 files changed, 9 insertions(+)