KVM/arm64: Early PSTATE evaluation fixes
Marc Zyngier
maintainercover KVM/arm64: Early PSTATE evaluation fixes
There are a number of cases where we evaluate PSTATE early on guest exit. Nothing wrong with that. Except that we actually synchronise KVM's view of PSTATE pretty late, way after we needed it. Oopsie boo.
Thankfully, there are only two paths that require it: GICv3 emulation for 32bit guests, and trap handling of 32bit guests in protected mode. There is no known need of the former (though you could enable it on the command line), and the latter is still a work in progress. In any case, this needs fixing.
Funnily enough, this is something that I had already solved on NV, so the solution isn't that different from what I have there.
Unless someone shouts, I intend to merge these as fixes.
Marc Zyngier (2):
KVM: arm64: Save PSTATE early on exit
KVM: arm64: Move pkvm's special 32bit handling into a generic
infrastructure
arch/arm64/kvm/hyp/include/hyp/switch.h | 14 ++++++++++++++ arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 7 ++++++- arch/arm64/kvm/hyp/nvhe/switch.c | 8 +------- arch/arm64/kvm/hyp/vhe/switch.c | 4 ++++ 4 files changed, 25 insertions(+), 8 deletions(-)
Signature · 1 line
2.30.2
Marc Zyngier
maintainerPATCH 1/2 KVM: arm64: Save PSTATE early on exit
In order to be able to use promitives such as vcpu_mode_is_32bit(), we need to synchronize the guest PSTATE. However, this is currently done deep imto the bowels of the world-switch code, and we do have helpers evaluating this much earlier (__vgic_v3_perform_cpuif_access and handle_aarch32_guest, for example).
Move the saving of the guest pstate into the early fixups, which cures the first issue. The second one will be addressed separately.
--- arch/arm64/kvm/hyp/include/hyp/switch.h | 6 ++++++ arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 7 ++++++- 2 files changed, 12 insertions(+), 1 deletion(-)
arch/arm64/kvm/hyp/include/hyp/switch.h +6 -0
@@ -429,6 +429,12 @@ static inline bool kvm_hyp_handle_exit(struct kvm_vcpu *vcpu, u64 *exit_code)*/ static inline bool fixup_guest_exit (struct kvm_vcpu *vcpu,u64 *exit_code) { +/* +* Save PSTATE early so that we can evaluate the vcpu mode +* early on. +*/ + vcpu->arch.ctxt.regs.pstate =read_sysreg_el2 (SYS_SPSR ); +if (ARM_EXCEPTION_CODE (*exit_code) !=ARM_EXCEPTION_IRQ ) vcpu->arch.fault.esr_el2 =read_sysreg_el2 (SYS_ESR );
arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h +6 -1
@@ -70,7 +70,12 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)static inline void __sysreg_save_el2_return_state (struct kvm_cpu_context *ctxt) { ctxt->regs.pc =read_sysreg_el2 (SYS_ELR ); - ctxt->regs.pstate =read_sysreg_el2 (SYS_SPSR ); +/* +* Guest PSTATE gets saved at guest fixup time in all +* cases. We still need to handle the nVHE host side here. +*/ +if (!has_vhe () && ctxt->__hyp_running_vcpu) + ctxt->regs.pstate =read_sysreg_el2 (SYS_SPSR );if (cpus_have_final_cap (ARM64_HAS_RAS_EXTN ))ctxt_sys_reg (ctxt,DISR_EL1 ) =read_sysreg_s (SYS_VDISR_EL2 );
Signature · 1 line
2.30.2
Re: [PATCH 1/2] KVM: arm64: Save PSTATE early on exit
Hi Marc,
Quoted 6 lines · Marc Zyngier
In order to be able to use promitives such as
vcpu_mode_is_32bit(), we need to synchronize the guest PSTATE. However, this is currently done deep imto the bowels of the world-switch code, and we do have helpers evaluating this much earlier (__vgic_v3_perform_cpuif_accessandhandle_aarch32_guest, for example).
Couple of nits:
s/promitives/primitives
s/imto/into
Quoted 46 lines
Move the saving of the guest pstate into the early fixups, which cures the first issue. The second one will be addressed separately.
Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/hyp/include/hyp/switch.h | 6 ++++++ arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 7 ++++++- 2 files changed, 12 insertions(+), 1 deletion(-)arch/arm64/kvm/hyp/include/hyp/switch.h +6
-0@@ -429,6 +429,12 @@ static inline bool kvm_hyp_handle_exit(struct kvm_vcpu *vcpu, u64 *exit_code)*/ static inline bool fixup_guest_exit (struct kvm_vcpu *vcpu,u64 *exit_code) { +/* +* Save PSTATE early so that we can evaluate the vcpu mode +* early on. +*/ + vcpu->arch.ctxt.regs.pstate =read_sysreg_el2 (SYS_SPSR ); +if (ARM_EXCEPTION_CODE (*exit_code) !=ARM_EXCEPTION_IRQ ) vcpu->arch.fault.esr_el2 =read_sysreg_el2 (SYS_ESR );arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h +6
-2@@ -70,7 +70,12 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)static inline void __sysreg_save_el2_return_state (struct kvm_cpu_context *ctxt) { ctxt->regs.pc =read_sysreg_el2 (SYS_ELR ); - ctxt->regs.pstate =read_sysreg_el2 (SYS_SPSR ); +/* +* Guest PSTATE gets saved at guest fixup time in all +* cases. We still need to handle the nVHE host side here. +*/ +if (!has_vhe () && ctxt->__hyp_running_vcpu) + ctxt->regs.pstate =read_sysreg_el2 (SYS_SPSR );if (cpus_have_final_cap (ARM64_HAS_RAS_EXTN ))ctxt_sys_reg (ctxt,DISR_EL1 ) =read_sysreg_s (SYS_VDISR_EL2 ); --
Quoted 2 lines
2.30.2
I see that now that you're storing pstate early at the guest exit, and therefore no need for vhe path to check for it for the guest when saving the return state. Going through the various possibilities, I think that all cases are covered.
I tested this code as well and it ran fine.
Thanks,
/fuad
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Marc Zyngier
maintainerPATCH 2/2 KVM: arm64: Move pkvm's special 32bit handling into a generic infrastructure
In reply to Marc ZyngierProtected KVM is trying to turn AArch32 exceptions into an illegal exception entry. Unfortunately, it does that it a way that is a bit abrupt, and too early for PSTATE to be available.
Instead, move it to the fixup code, which is a more reasonable place for it. This will also be useful for the NV code.
--- arch/arm64/kvm/hyp/include/hyp/switch.h | 8 ++++++++ arch/arm64/kvm/hyp/nvhe/switch.c | 8 +------- arch/arm64/kvm/hyp/vhe/switch.c | 4 ++++ 3 files changed, 13 insertions(+), 7 deletions(-)
arch/arm64/kvm/hyp/include/hyp/switch.h +8 -0
@@ -403,6 +403,8 @@ typedef bool (*exit_handler_fn)(struct kvm_vcpu *, u64 *);static const exit_handler_fn *kvm_get_exit_handler_array (struct kvm_vcpu *vcpu); +static void early_exit_filter (struct kvm_vcpu *vcpu,u64 *exit_code); +/* * Allow the hypervisor to handle the exit with an exit handler if it has one. *
@@ -435,6 +437,12 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)*/ vcpu->arch.ctxt.regs.pstate =read_sysreg_el2 (SYS_SPSR ); +/* +* Check whether we want to repaint the state one way or +* another. +*/ +early_exit_filter (vcpu, exit_code); +if (ARM_EXCEPTION_CODE (*exit_code) !=ARM_EXCEPTION_IRQ ) vcpu->arch.fault.esr_el2 =read_sysreg_el2 (SYS_ESR );
arch/arm64/kvm/hyp/nvhe/switch.c +1 -7
@@ -233,7 +233,7 @@ static const exit_handler_fn *kvm_get_exit_handler_array(struct kvm_vcpu *vcpu)* Returns false if the guest ran in AArch32 when it shouldn't have, and * thus should exit to the host, or true if a the guest run loop can continue. */ -static bool handle_aarch32_guest (struct kvm_vcpu *vcpu,u64 *exit_code) +static void early_exit_filter (struct kvm_vcpu *vcpu,u64 *exit_code) {struct kvm *kvm =kern_hyp_va (vcpu->kvm);
@@ -248,10 +248,7 @@ static bool handle_aarch32_guest(struct kvm_vcpu *vcpu, u64 *exit_code) vcpu->arch.target =-1 ; *exit_code &=BIT (ARM_EXIT_WITH_SERROR_BIT ); *exit_code |=ARM_EXCEPTION_IL ; -return false; } - -return true; }/* Switch to the guest for legacy non-VHE systems */
@@ -316,9 +313,6 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)/* Jump in the fire! */ exit_code =__guest_enter (vcpu); -if (unlikely (!handle_aarch32_guest (vcpu, &exit_code))) -break ; -/* And we're baaack! */ }while (fixup_guest_exit (vcpu, &exit_code));
arch/arm64/kvm/hyp/vhe/switch.c +4 -0
@@ -112,6 +112,10 @@ static const exit_handler_fn *kvm_get_exit_handler_array(struct kvm_vcpu *vcpu)return hyp_exit_handlers; } +static void early_exit_filter (struct kvm_vcpu *vcpu,u64 *exit_code) +{ +} +/* Switch to the guest for VHE systems running in EL2 */ static int __kvm_vcpu_run_vhe (struct kvm_vcpu *vcpu) {
Signature · 1 line
2.30.2
Re: [PATCH 2/2] KVM: arm64: Move pkvm's special 32bit handling into a generic infrastructure
Hi Marc,
Quoted 3 lines · Marc Zyngier
Protected KVM is trying to turn AArch32 exceptions into an illegal exception entry. Unfortunately, it does that it a way that is a bit
Small nit: s/it/in
Quoted 4 lines
abrupt, and too early for PSTATE to be available.
Instead, move it to the fixup code, which is a more reasonable place for it. This will also be useful for the NV code.
This approach seems to be easier to generalize for other cases than the previous one.
Cheers,
/fuad
Quoted 84 lines
Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/hyp/include/hyp/switch.h | 8 ++++++++ arch/arm64/kvm/hyp/nvhe/switch.c | 8 +------- arch/arm64/kvm/hyp/vhe/switch.c | 4 ++++ 3 files changed, 13 insertions(+), 7 deletions(-)arch/arm64/kvm/hyp/include/hyp/switch.h +8
-0@@ -403,6 +403,8 @@ typedef bool (*exit_handler_fn)(struct kvm_vcpu *, u64 *);static const exit_handler_fn *kvm_get_exit_handler_array (struct kvm_vcpu *vcpu); +static void early_exit_filter (struct kvm_vcpu *vcpu,u64 *exit_code); +/* * Allow the hypervisor to handle the exit with an exit handler if it has one. * @@ -435,6 +437,12 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)*/ vcpu->arch.ctxt.regs.pstate =read_sysreg_el2 (SYS_SPSR ); +/* +* Check whether we want to repaint the state one way or +* another. +*/ +early_exit_filter (vcpu, exit_code); +if (ARM_EXCEPTION_CODE (*exit_code) !=ARM_EXCEPTION_IRQ ) vcpu->arch.fault.esr_el2 =read_sysreg_el2 (SYS_ESR );arch/arm64/kvm/hyp/nvhe/switch.c +1
-7@@ -233,7 +233,7 @@ static const exit_handler_fn *kvm_get_exit_handler_array(struct kvm_vcpu *vcpu)* Returns false if the guest ran in AArch32 when it shouldn't have, and * thus should exit to the host, or true if a the guest run loop can continue. */ -static bool handle_aarch32_guest (struct kvm_vcpu *vcpu,u64 *exit_code) +static void early_exit_filter (struct kvm_vcpu *vcpu,u64 *exit_code) {struct kvm *kvm =kern_hyp_va (vcpu->kvm);@@ -248,10 +248,7 @@ static bool handle_aarch32_guest(struct kvm_vcpu *vcpu, u64 *exit_code) vcpu->arch.target =-1 ; *exit_code &=BIT (ARM_EXIT_WITH_SERROR_BIT ); *exit_code |=ARM_EXCEPTION_IL ; -return false; } - -return true; }/* Switch to the guest for legacy non-VHE systems */ @@ -316,9 +313,6 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)/* Jump in the fire! */ exit_code =__guest_enter (vcpu); -if (unlikely (!handle_aarch32_guest (vcpu, &exit_code))) -break ; -/* And we're baaack! */ }while (fixup_guest_exit (vcpu, &exit_code));arch/arm64/kvm/hyp/vhe/switch.c +4
-1@@ -112,6 +112,10 @@ static const exit_handler_fn *kvm_get_exit_handler_array(struct kvm_vcpu *vcpu)return hyp_exit_handlers; } +static void early_exit_filter (struct kvm_vcpu *vcpu,u64 *exit_code) +{ +} +/* Switch to the guest for VHE systems running in EL2 */ static int __kvm_vcpu_run_vhe (struct kvm_vcpu *vcpu) { --
Quoted 2 lines
2.30.2
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm