KVM/arm64 fixes for NV PAC support
Marc Zyngier
maintainercover KVM/arm64 fixes for NV PAC support
Here's a small series of fixes following the introduction of NV PAuth support in 6.10:
- address the relative priorities of Instruction abort, Illegal Execution state, and PAC failure (already posted)
- Expose BTI to a NV guest now that we have PAC up and running
Unless someone shouts, I'll take these patches in the next batch of fixes.
M.
Marc Zyngier (2):
KVM: arm64: nv: Fix relative priorities of exceptions generated by
ERETAx
KVM: arm64: nv: Expose BTI and CSV_frac to a guest hypervisor
arch/arm64/kvm/emulate-nested.c | 21 +++++++++++---------- arch/arm64/kvm/nested.c | 6 ++++-- 2 files changed, 15 insertions(+), 12 deletions(-)
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH 1/2 KVM: arm64: nv: Fix relative priorities of exceptions generated by ERETAx
ERETAx can fail in multiple ways:
(1) ELR_EL2 points lalaland
(2) we get a PAC failure
(3) SPSR_EL2 has the wrong mode
(1) is easy, as we just let the CPU do its thing and deliver an Instruction Abort. However, (2) and (3) are interesting, because the PAC failure priority is way below that of the Illegal Execution State exception.
Which means that if we have detected a PAC failure (and that we have FPACCOMBINE), we must be careful to give priority to the Illegal Execution State exception, should one be pending.
Solving this involves hoisting the SPSR calculation earlier and testing for the IL bit before injecting the FPAC exception.
In the extreme case of a ERETAx returning to an invalid mode *and* failing its PAC check, we end up with an Instruction Abort (due to the new PC being mangled by the failed Auth) *and* PSTATE.IL being set. Which matches the requirements of the architecture.
Whilst we're at it, remove a stale comment that states the obvious and only confuses the reader.
--- arch/arm64/kvm/emulate-nested.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
arch/arm64/kvm/emulate-nested.c +11 -10
@@ -2181,16 +2181,23 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)if (forward_traps (vcpu,HCR_NV ))return ; + spsr =vcpu_read_sys_reg (vcpu,SPSR_EL2 ); + spsr =kvm_check_illegal_exception_return (vcpu, spsr); +/* Check for an ERETAx */ esr =kvm_vcpu_get_esr (vcpu);if (esr_iss_is_eretax (esr) && !kvm_auth_eretax (vcpu, &elr)) {/* -* Oh no, ERETAx failed to authenticate. If we have -* FPACCOMBINE, deliver an exception right away. If we -* don't, then let the mangled ELR value trickle down the +* Oh no, ERETAx failed to authenticate. +* +* If we have FPACCOMBINE and we don't have a pending +* Illegal Execution State exception (which has priority +* over FPAC), deliver an exception right away. +* +* Otherwise, let the mangled ELR value trickle down the * ERET handling, and the guest will have a little surprise. */ -if (kvm_has_pauth (vcpu->kvm,FPACCOMBINE )) { +if (kvm_has_pauth (vcpu->kvm,FPACCOMBINE ) && !(spsr &PSR_IL_BIT )) { esr &= ESR_ELx_ERET_ISS_ERETA; esr |=FIELD_PREP (ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC);kvm_inject_nested_sync (vcpu, esr);
@@ -2201,17 +2208,11 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)preempt_disable ();kvm_arch_vcpu_put (vcpu); - spsr =__vcpu_sys_reg (vcpu,SPSR_EL2 ); - spsr =kvm_check_illegal_exception_return (vcpu, spsr);if (!esr_iss_is_eretax (esr)) elr =__vcpu_sys_reg (vcpu,ELR_EL2 );trace_kvm_nested_eret (vcpu, elr, spsr); -/* -* Note that the current exception level is always the virtual EL2, -* since we set HCR_EL2.NV bit only when entering the virtual EL2. -*/ *vcpu_pc (vcpu) = elr; *vcpu_cpsr (vcpu) = spsr;
Signature · 1 line
2.39.2
Re: [PATCH 1/2] KVM: arm64: nv: Fix relative priorities of exceptions generated by ERETAx
Hi Marc,
These references are from ARM DDI 0487 J.a.
Quoted 6 lines · Marc Zyngier
ERETAx can fail in multiple ways:
(1)
ELR_EL2points lalaland
(2) we get a PAC failure
(3)SPSR_EL2has the wrong mode
Quoted 8 lines
(1) is easy, as we just let the CPU do its thing and deliver an Instruction Abort. However, (2) and (3) are interesting, because the PAC failure priority is way below that of the Illegal Execution State exception.
Which means that if we have detected a PAC failure (and that we have FPACCOMBINE), we must be careful to give priority to the Illegal Execution State exception, should one be pending.
This is IZFGJP Prioritization of Synchronous exceptions taken to AArch64 state.
Quoted 8 lines
Solving this involves hoisting the SPSR calculation earlier and testing for the IL bit before injecting the FPAC exception.
In the extreme case of a ERETAx returning to an invalid mode *and* failing its PAC check, we end up with an Instruction Abort (due to the new PC being mangled by the failed Auth) *and* PSTATE.IL being set. Which matches the requirements of the architecture.
And this is IGPPXQ, which says "which causes the next instruction
to generate an Illegal State exception. The exception return instruction does not generate the exception."
Which matches since Instruction Abort has a higher priority than Illegal Exception.
Quoted 50 lines
Whilst we're at it, remove a stale comment that states the obvious and only confuses the reader.
Fixes 213b3d1ea161 ("KVM: arm64: nv: Handle ERETA[AB] instructions")Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/emulate-nested.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)arch/arm64/kvm/emulate-nested.c +11
-6@@ -2181,16 +2181,23 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)if (forward_traps (vcpu,HCR_NV ))return ; + spsr =vcpu_read_sys_reg (vcpu,SPSR_EL2 ); + spsr =kvm_check_illegal_exception_return (vcpu, spsr); +/* Check for an ERETAx */ esr =kvm_vcpu_get_esr (vcpu);if (esr_iss_is_eretax (esr) && !kvm_auth_eretax (vcpu, &elr)) {/* -* Oh no, ERETAx failed to authenticate. If we have -* FPACCOMBINE, deliver an exception right away. If we -* don't, then let the mangled ELR value trickle down the +* Oh no, ERETAx failed to authenticate. +* +* If we have FPACCOMBINE and we don't have a pending +* Illegal Execution State exception (which has priority +* over FPAC), deliver an exception right away. +* +* Otherwise, let the mangled ELR value trickle down the * ERET handling, and the guest will have a little surprise. */ -if (kvm_has_pauth (vcpu->kvm,FPACCOMBINE )) { +if (kvm_has_pauth (vcpu->kvm,FPACCOMBINE ) && !(spsr &PSR_IL_BIT )) { esr &= ESR_ELx_ERET_ISS_ERETA; esr |=FIELD_PREP (ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC);kvm_inject_nested_sync (vcpu, esr);@@ -2201,17 +2208,11 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)preempt_disable ();kvm_arch_vcpu_put (vcpu); - spsr =__vcpu_sys_reg (vcpu,SPSR_EL2 ); - spsr =kvm_check_illegal_exception_return (vcpu, spsr);if (!esr_iss_is_eretax (esr)) elr =__vcpu_sys_reg (vcpu,ELR_EL2 );
arch/arm64/kvm/emulate-nested.c void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu) · line 2218
Quoted 10 lines
arch/arm64/kvm/emulate-nested.c +0
-4trace_kvm_nested_eret (vcpu, elr, spsr); -/* -* Note that the current exception level is always the virtual EL2, -* since we set HCR_EL2.NV bit only when entering the virtual EL2. -*/ *vcpu_pc (vcpu) = elr; *vcpu_cpsr (vcpu) = spsr;
Those references were just me checking it, not suggesting you edit the commit message.
Thanks,
Joey
Marc Zyngier
maintainerRe: [PATCH 1/2] KVM: arm64: nv: Fix relative priorities of exceptions generated by ERETAx
Hi Joey,
Quoted 4 lines · Joey Gouly
Hi Marc,
These references are from ARM DDI 0487 J.a.
Ah, you have an upgrade pending! ;-)
Quoted 13 lines · Marc Zyngier
Quoted 11 lines
ERETAx can fail in multiple ways:
(1)
ELR_EL2points lalaland
(2) we get a PAC failure
(3)SPSR_EL2has the wrong mode(1) is easy, as we just let the CPU do its thing and deliver an Instruction Abort. However, (2) and (3) are interesting, because the PAC failure priority is way below that of the Illegal Execution State exception.
Quoted 7 lines
Quoted 3 lines
Which means that if we have detected a PAC failure (and that we have FPACCOMBINE), we must be careful to give priority to the Illegal Execution State exception, should one be pending.
This is IZFGJP Prioritization of Synchronous exceptions taken to AArch64 state.
Indeed.
Quoted 9 lines
Quoted 8 lines
Solving this involves hoisting the SPSR calculation earlier and testing for the IL bit before injecting the FPAC exception.
In the extreme case of a ERETAx returning to an invalid mode *and* failing its PAC check, we end up with an Instruction Abort (due to the new PC being mangled by the failed Auth) *and* PSTATE.IL being set. Which matches the requirements of the architecture.
Quoted 6 lines
And this is IGPPXQ, which says "which causes the next instruction to generate an Illegal State exception. The exception return instruction does not generate the exception."
Which matches since Instruction Abort has a higher priority than Illegal Exception.
Spot on. Although it is a bit surprising that in all cases, the CPU has to attempt fetching the next instruction before delivering the IL exception so that it can prioritise the Abort. I guess it simplifies some HW implementations, but makes SW suffer a bit.
Quoted 54 lines
Quoted 54 lines
Whilst we're at it, remove a stale comment that states the obvious and only confuses the reader.
Fixes 213b3d1ea161 ("KVM: arm64: nv: Handle ERETA[AB] instructions")Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/emulate-nested.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)arch/arm64/kvm/emulate-nested.c +11
-6@@ -2181,16 +2181,23 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)if (forward_traps (vcpu,HCR_NV ))return ; + spsr =vcpu_read_sys_reg (vcpu,SPSR_EL2 ); + spsr =kvm_check_illegal_exception_return (vcpu, spsr); +/* Check for an ERETAx */ esr =kvm_vcpu_get_esr (vcpu);if (esr_iss_is_eretax (esr) && !kvm_auth_eretax (vcpu, &elr)) {/* -* Oh no, ERETAx failed to authenticate. If we have -* FPACCOMBINE, deliver an exception right away. If we -* don't, then let the mangled ELR value trickle down the +* Oh no, ERETAx failed to authenticate. +* +* If we have FPACCOMBINE and we don't have a pending +* Illegal Execution State exception (which has priority +* over FPAC), deliver an exception right away. +* +* Otherwise, let the mangled ELR value trickle down the * ERET handling, and the guest will have a little surprise. */ -if (kvm_has_pauth (vcpu->kvm,FPACCOMBINE )) { +if (kvm_has_pauth (vcpu->kvm,FPACCOMBINE ) && !(spsr &PSR_IL_BIT )) { esr &= ESR_ELx_ERET_ISS_ERETA; esr |=FIELD_PREP (ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC);kvm_inject_nested_sync (vcpu, esr);@@ -2201,17 +2208,11 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)preempt_disable ();kvm_arch_vcpu_put (vcpu); - spsr =__vcpu_sys_reg (vcpu,SPSR_EL2 ); - spsr =kvm_check_illegal_exception_return (vcpu, spsr);if (!esr_iss_is_eretax (esr)) elr =__vcpu_sys_reg (vcpu,ELR_EL2 );trace_kvm_nested_eret (vcpu, elr, spsr);
Quoted 10 lines
arch/arm64/kvm/emulate-nested.c void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu) · line 2218
Quoted 7 lines
arch/arm64/kvm/emulate-nested.c +0
-4-/* -* Note that the current exception level is always the virtual EL2, -* since we set HCR_EL2.NV bit only when entering the virtual EL2. -*/ *vcpu_pc (vcpu) = elr; *vcpu_cpsr (vcpu) = spsr;Those references were just me checking it, not suggesting you edit the commit message.
Well, that's excellent detective work!
Quoted 2 lines
Reviewed-by Joey Gouly <[email protected]>
Thanks again,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerPATCH 2/2 KVM: arm64: nv: Expose BTI and CSV_frac to a guest hypervisor
In reply to Marc ZyngierNow that we expose PAC to NV guests, we can also expose BTI (as the two as joined at the hip, due to some of the PAC instructions being landing pads).
While we're at it, also propagate CSV_frac, which requires no particular emulation.
--- arch/arm64/kvm/nested.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm64/kvm/nested.c +4 -2
@@ -58,8 +58,10 @@ static u64 limit_nv_id_reg(u32 id, u64 val)break ;case SYS_ID_AA64PFR1_EL1 : -/* Only support SSBS */ - val &=NV_FTR (PFR1 ,SSBS ); +/* Only support BTI, SSBS, CSV2_frac */ + val &= (NV_FTR (PFR1 ,BT ) | +NV_FTR (PFR1 ,SSBS ) | +NV_FTR (PFR1 , CSV2_frac));break ;case SYS_ID_AA64MMFR0_EL1 :
Signature · 1 line
2.39.2
Quoted 11 lines · Marc Zyngier
Here's a small series of fixes following the introduction of NV PAuth support in 6.10:
- address the relative priorities of Instruction abort, Illegal Execution state, and PAC failure (already posted)
- Expose BTI to a NV guest now that we have PAC up and running
Unless someone shouts, I'll take these patches in the next batch of fixes.
Signature · 2 lines
Thanks, Oliver
Quoted 9 lines · Marc Zyngier
Here's a small series of fixes following the introduction of NV PAuth support in 6.10:
- address the relative priorities of Instruction abort, Illegal Execution state, and PAC failure (already posted)
- Expose BTI to a NV guest now that we have PAC up and running
[...]
Applied to fixes, thanks!
[1/2] KVM: arm64: nv: Fix relative priorities of exceptions generated by ERETAx
commit: 41011e2de3480f9adb6420b35b67628b2d903355
[2/2] KVM: arm64: nv: Expose BTI and CSV_frac to a guest hypervisor
commit: 47eb2d68d10208e6a9e89d10b66018e8d6ca0623Cheers,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.