* [PATCH v2 0/2] KVM: arm64: Fixup PC updates on exit to userspace
@ 2021-05-14 10:40 ` Marc Zyngier
0 siblings, 0 replies; 21+ messages in thread
From: Marc Zyngier @ 2021-05-14 10:40 UTC (permalink / raw)
To: kvm, kvmarm, linux-arm-kernel; +Cc: kernel-team
We recently moved anything related to PC updates into the guest entry
code to help with the protected KVM effort. However, I missed a
critical point: userspace needs to be able to observe state changes
when the vcpu exits. Otherwise, live migration is a bit broken and
vcpu reset can fail (as reported by Zenghui). Not good.
These two patches aim at fixing the above, and carry a Cc stable.
* From v1:
- Sanitized flag checking
- Added comment about relying on __kvm_adjust_pc() being preempt-safe
- Dropped superfluous includes
Marc Zyngier (2):
KVM: arm64: Move __adjust_pc out of line
KVM: arm64: Commit pending PC adjustemnts before returning to
userspace
arch/arm64/include/asm/kvm_asm.h | 3 +++
arch/arm64/kvm/arm.c | 10 ++++++++++
arch/arm64/kvm/hyp/exception.c | 18 +++++++++++++++++-
arch/arm64/kvm/hyp/include/hyp/adjust_pc.h | 18 ------------------
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++
arch/arm64/kvm/hyp/nvhe/switch.c | 3 +--
arch/arm64/kvm/hyp/vhe/switch.c | 3 +--
7 files changed, 40 insertions(+), 23 deletions(-)
--
2.29.2
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH v2 0/2] KVM: arm64: Fixup PC updates on exit to userspace @ 2021-05-14 10:40 ` Marc Zyngier 0 siblings, 0 replies; 21+ messages in thread From: Marc Zyngier @ 2021-05-14 10:40 UTC (permalink / raw) To: kvm, kvmarm, linux-arm-kernel Cc: Zenghui Yu, Fuad Tabba, James Morse, Suzuki K Poulose, Alexandru Elisei, kernel-team We recently moved anything related to PC updates into the guest entry code to help with the protected KVM effort. However, I missed a critical point: userspace needs to be able to observe state changes when the vcpu exits. Otherwise, live migration is a bit broken and vcpu reset can fail (as reported by Zenghui). Not good. These two patches aim at fixing the above, and carry a Cc stable. * From v1: - Sanitized flag checking - Added comment about relying on __kvm_adjust_pc() being preempt-safe - Dropped superfluous includes Marc Zyngier (2): KVM: arm64: Move __adjust_pc out of line KVM: arm64: Commit pending PC adjustemnts before returning to userspace arch/arm64/include/asm/kvm_asm.h | 3 +++ arch/arm64/kvm/arm.c | 10 ++++++++++ arch/arm64/kvm/hyp/exception.c | 18 +++++++++++++++++- arch/arm64/kvm/hyp/include/hyp/adjust_pc.h | 18 ------------------ arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++ arch/arm64/kvm/hyp/nvhe/switch.c | 3 +-- arch/arm64/kvm/hyp/vhe/switch.c | 3 +-- 7 files changed, 40 insertions(+), 23 deletions(-) -- 2.29.2 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 0/2] KVM: arm64: Fixup PC updates on exit to userspace @ 2021-05-14 10:40 ` Marc Zyngier 0 siblings, 0 replies; 21+ messages in thread From: Marc Zyngier @ 2021-05-14 10:40 UTC (permalink / raw) To: kvm, kvmarm, linux-arm-kernel Cc: Zenghui Yu, Fuad Tabba, James Morse, Suzuki K Poulose, Alexandru Elisei, kernel-team We recently moved anything related to PC updates into the guest entry code to help with the protected KVM effort. However, I missed a critical point: userspace needs to be able to observe state changes when the vcpu exits. Otherwise, live migration is a bit broken and vcpu reset can fail (as reported by Zenghui). Not good. These two patches aim at fixing the above, and carry a Cc stable. * From v1: - Sanitized flag checking - Added comment about relying on __kvm_adjust_pc() being preempt-safe - Dropped superfluous includes Marc Zyngier (2): KVM: arm64: Move __adjust_pc out of line KVM: arm64: Commit pending PC adjustemnts before returning to userspace arch/arm64/include/asm/kvm_asm.h | 3 +++ arch/arm64/kvm/arm.c | 10 ++++++++++ arch/arm64/kvm/hyp/exception.c | 18 +++++++++++++++++- arch/arm64/kvm/hyp/include/hyp/adjust_pc.h | 18 ------------------ arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++ arch/arm64/kvm/hyp/nvhe/switch.c | 3 +-- arch/arm64/kvm/hyp/vhe/switch.c | 3 +-- 7 files changed, 40 insertions(+), 23 deletions(-) -- 2.29.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 1/2] KVM: arm64: Move __adjust_pc out of line 2021-05-14 10:40 ` Marc Zyngier (?) @ 2021-05-14 10:40 ` Marc Zyngier -1 siblings, 0 replies; 21+ messages in thread From: Marc Zyngier @ 2021-05-14 10:40 UTC (permalink / raw) To: kvm, kvmarm, linux-arm-kernel; +Cc: kernel-team, stable In order to make it easy to call __adjust_pc() from the EL1 code (in the case of nVHE), rename it to __kvm_adjust_pc() and move it out of line. No expected functional change. Signed-off-by: Marc Zyngier <maz@kernel.org> Cc: stable@vger.kernel.org # 5.11 --- arch/arm64/include/asm/kvm_asm.h | 2 ++ arch/arm64/kvm/hyp/exception.c | 18 +++++++++++++++++- arch/arm64/kvm/hyp/include/hyp/adjust_pc.h | 18 ------------------ arch/arm64/kvm/hyp/nvhe/switch.c | 3 +-- arch/arm64/kvm/hyp/vhe/switch.c | 3 +-- 5 files changed, 21 insertions(+), 23 deletions(-) diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index cf8df032b9c3..d5b11037401d 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -201,6 +201,8 @@ extern void __kvm_timer_set_cntvoff(u64 cntvoff); extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu); +extern void __kvm_adjust_pc(struct kvm_vcpu *vcpu); + extern u64 __vgic_v3_get_gic_config(void); extern u64 __vgic_v3_read_vmcr(void); extern void __vgic_v3_write_vmcr(u32 vmcr); diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c index 73629094f903..0812a496725f 100644 --- a/arch/arm64/kvm/hyp/exception.c +++ b/arch/arm64/kvm/hyp/exception.c @@ -296,7 +296,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset) *vcpu_pc(vcpu) = vect_offset; } -void kvm_inject_exception(struct kvm_vcpu *vcpu) +static void kvm_inject_exception(struct kvm_vcpu *vcpu) { if (vcpu_el1_is_32bit(vcpu)) { switch (vcpu->arch.flags & KVM_ARM64_EXCEPT_MASK) { @@ -329,3 +329,19 @@ void kvm_inject_exception(struct kvm_vcpu *vcpu) } } } + +/* + * Adjust the guest PC on entry, depending on flags provided by EL1 + * for the purpose of emulation (MMIO, sysreg) or exception injection. + */ +void __kvm_adjust_pc(struct kvm_vcpu *vcpu) +{ + if (vcpu->arch.flags & KVM_ARM64_PENDING_EXCEPTION) { + kvm_inject_exception(vcpu); + vcpu->arch.flags &= ~(KVM_ARM64_PENDING_EXCEPTION | + KVM_ARM64_EXCEPT_MASK); + } else if (vcpu->arch.flags & KVM_ARM64_INCREMENT_PC) { + kvm_skip_instr(vcpu); + vcpu->arch.flags &= ~KVM_ARM64_INCREMENT_PC; + } +} diff --git a/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h b/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h index 61716359035d..4fdfeabefeb4 100644 --- a/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h +++ b/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h @@ -13,8 +13,6 @@ #include <asm/kvm_emulate.h> #include <asm/kvm_host.h> -void kvm_inject_exception(struct kvm_vcpu *vcpu); - static inline void kvm_skip_instr(struct kvm_vcpu *vcpu) { if (vcpu_mode_is_32bit(vcpu)) { @@ -43,22 +41,6 @@ static inline void __kvm_skip_instr(struct kvm_vcpu *vcpu) write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); } -/* - * Adjust the guest PC on entry, depending on flags provided by EL1 - * for the purpose of emulation (MMIO, sysreg) or exception injection. - */ -static inline void __adjust_pc(struct kvm_vcpu *vcpu) -{ - if (vcpu->arch.flags & KVM_ARM64_PENDING_EXCEPTION) { - kvm_inject_exception(vcpu); - vcpu->arch.flags &= ~(KVM_ARM64_PENDING_EXCEPTION | - KVM_ARM64_EXCEPT_MASK); - } else if (vcpu->arch.flags & KVM_ARM64_INCREMENT_PC) { - kvm_skip_instr(vcpu); - vcpu->arch.flags &= ~KVM_ARM64_INCREMENT_PC; - } -} - /* * Skip an instruction while host sysregs are live. * Assumes host is always 64-bit. diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c index e9f6ea704d07..f7af9688c1f7 100644 --- a/arch/arm64/kvm/hyp/nvhe/switch.c +++ b/arch/arm64/kvm/hyp/nvhe/switch.c @@ -4,7 +4,6 @@ * Author: Marc Zyngier <marc.zyngier@arm.com> */ -#include <hyp/adjust_pc.h> #include <hyp/switch.h> #include <hyp/sysreg-sr.h> @@ -201,7 +200,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu) */ __debug_save_host_buffers_nvhe(vcpu); - __adjust_pc(vcpu); + __kvm_adjust_pc(vcpu); /* * We must restore the 32-bit state before the sysregs, thanks diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c index 7b8f7db5c1ed..b3229924d243 100644 --- a/arch/arm64/kvm/hyp/vhe/switch.c +++ b/arch/arm64/kvm/hyp/vhe/switch.c @@ -4,7 +4,6 @@ * Author: Marc Zyngier <marc.zyngier@arm.com> */ -#include <hyp/adjust_pc.h> #include <hyp/switch.h> #include <linux/arm-smccc.h> @@ -132,7 +131,7 @@ static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu) __load_guest_stage2(vcpu->arch.hw_mmu); __activate_traps(vcpu); - __adjust_pc(vcpu); + __kvm_adjust_pc(vcpu); sysreg_restore_guest_state_vhe(guest_ctxt); __debug_switch_to_guest(vcpu); -- 2.29.2 _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 1/2] KVM: arm64: Move __adjust_pc out of line @ 2021-05-14 10:40 ` Marc Zyngier 0 siblings, 0 replies; 21+ messages in thread From: Marc Zyngier @ 2021-05-14 10:40 UTC (permalink / raw) To: kvm, kvmarm, linux-arm-kernel Cc: Zenghui Yu, Fuad Tabba, James Morse, Suzuki K Poulose, Alexandru Elisei, kernel-team, stable In order to make it easy to call __adjust_pc() from the EL1 code (in the case of nVHE), rename it to __kvm_adjust_pc() and move it out of line. No expected functional change. Signed-off-by: Marc Zyngier <maz@kernel.org> Cc: stable@vger.kernel.org # 5.11 --- arch/arm64/include/asm/kvm_asm.h | 2 ++ arch/arm64/kvm/hyp/exception.c | 18 +++++++++++++++++- arch/arm64/kvm/hyp/include/hyp/adjust_pc.h | 18 ------------------ arch/arm64/kvm/hyp/nvhe/switch.c | 3 +-- arch/arm64/kvm/hyp/vhe/switch.c | 3 +-- 5 files changed, 21 insertions(+), 23 deletions(-) diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index cf8df032b9c3..d5b11037401d 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -201,6 +201,8 @@ extern void __kvm_timer_set_cntvoff(u64 cntvoff); extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu); +extern void __kvm_adjust_pc(struct kvm_vcpu *vcpu); + extern u64 __vgic_v3_get_gic_config(void); extern u64 __vgic_v3_read_vmcr(void); extern void __vgic_v3_write_vmcr(u32 vmcr); diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c index 73629094f903..0812a496725f 100644 --- a/arch/arm64/kvm/hyp/exception.c +++ b/arch/arm64/kvm/hyp/exception.c @@ -296,7 +296,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset) *vcpu_pc(vcpu) = vect_offset; } -void kvm_inject_exception(struct kvm_vcpu *vcpu) +static void kvm_inject_exception(struct kvm_vcpu *vcpu) { if (vcpu_el1_is_32bit(vcpu)) { switch (vcpu->arch.flags & KVM_ARM64_EXCEPT_MASK) { @@ -329,3 +329,19 @@ void kvm_inject_exception(struct kvm_vcpu *vcpu) } } } + +/* + * Adjust the guest PC on entry, depending on flags provided by EL1 + * for the purpose of emulation (MMIO, sysreg) or exception injection. + */ +void __kvm_adjust_pc(struct kvm_vcpu *vcpu) +{ + if (vcpu->arch.flags & KVM_ARM64_PENDING_EXCEPTION) { + kvm_inject_exception(vcpu); + vcpu->arch.flags &= ~(KVM_ARM64_PENDING_EXCEPTION | + KVM_ARM64_EXCEPT_MASK); + } else if (vcpu->arch.flags & KVM_ARM64_INCREMENT_PC) { + kvm_skip_instr(vcpu); + vcpu->arch.flags &= ~KVM_ARM64_INCREMENT_PC; + } +} diff --git a/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h b/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h index 61716359035d..4fdfeabefeb4 100644 --- a/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h +++ b/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h @@ -13,8 +13,6 @@ #include <asm/kvm_emulate.h> #include <asm/kvm_host.h> -void kvm_inject_exception(struct kvm_vcpu *vcpu); - static inline void kvm_skip_instr(struct kvm_vcpu *vcpu) { if (vcpu_mode_is_32bit(vcpu)) { @@ -43,22 +41,6 @@ static inline void __kvm_skip_instr(struct kvm_vcpu *vcpu) write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); } -/* - * Adjust the guest PC on entry, depending on flags provided by EL1 - * for the purpose of emulation (MMIO, sysreg) or exception injection. - */ -static inline void __adjust_pc(struct kvm_vcpu *vcpu) -{ - if (vcpu->arch.flags & KVM_ARM64_PENDING_EXCEPTION) { - kvm_inject_exception(vcpu); - vcpu->arch.flags &= ~(KVM_ARM64_PENDING_EXCEPTION | - KVM_ARM64_EXCEPT_MASK); - } else if (vcpu->arch.flags & KVM_ARM64_INCREMENT_PC) { - kvm_skip_instr(vcpu); - vcpu->arch.flags &= ~KVM_ARM64_INCREMENT_PC; - } -} - /* * Skip an instruction while host sysregs are live. * Assumes host is always 64-bit. diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c index e9f6ea704d07..f7af9688c1f7 100644 --- a/arch/arm64/kvm/hyp/nvhe/switch.c +++ b/arch/arm64/kvm/hyp/nvhe/switch.c @@ -4,7 +4,6 @@ * Author: Marc Zyngier <marc.zyngier@arm.com> */ -#include <hyp/adjust_pc.h> #include <hyp/switch.h> #include <hyp/sysreg-sr.h> @@ -201,7 +200,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu) */ __debug_save_host_buffers_nvhe(vcpu); - __adjust_pc(vcpu); + __kvm_adjust_pc(vcpu); /* * We must restore the 32-bit state before the sysregs, thanks diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c index 7b8f7db5c1ed..b3229924d243 100644 --- a/arch/arm64/kvm/hyp/vhe/switch.c +++ b/arch/arm64/kvm/hyp/vhe/switch.c @@ -4,7 +4,6 @@ * Author: Marc Zyngier <marc.zyngier@arm.com> */ -#include <hyp/adjust_pc.h> #include <hyp/switch.h> #include <linux/arm-smccc.h> @@ -132,7 +131,7 @@ static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu) __load_guest_stage2(vcpu->arch.hw_mmu); __activate_traps(vcpu); - __adjust_pc(vcpu); + __kvm_adjust_pc(vcpu); sysreg_restore_guest_state_vhe(guest_ctxt); __debug_switch_to_guest(vcpu); -- 2.29.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 1/2] KVM: arm64: Move __adjust_pc out of line @ 2021-05-14 10:40 ` Marc Zyngier 0 siblings, 0 replies; 21+ messages in thread From: Marc Zyngier @ 2021-05-14 10:40 UTC (permalink / raw) To: kvm, kvmarm, linux-arm-kernel Cc: Zenghui Yu, Fuad Tabba, James Morse, Suzuki K Poulose, Alexandru Elisei, kernel-team, stable In order to make it easy to call __adjust_pc() from the EL1 code (in the case of nVHE), rename it to __kvm_adjust_pc() and move it out of line. No expected functional change. Signed-off-by: Marc Zyngier <maz@kernel.org> Cc: stable@vger.kernel.org # 5.11 --- arch/arm64/include/asm/kvm_asm.h | 2 ++ arch/arm64/kvm/hyp/exception.c | 18 +++++++++++++++++- arch/arm64/kvm/hyp/include/hyp/adjust_pc.h | 18 ------------------ arch/arm64/kvm/hyp/nvhe/switch.c | 3 +-- arch/arm64/kvm/hyp/vhe/switch.c | 3 +-- 5 files changed, 21 insertions(+), 23 deletions(-) diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index cf8df032b9c3..d5b11037401d 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -201,6 +201,8 @@ extern void __kvm_timer_set_cntvoff(u64 cntvoff); extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu); +extern void __kvm_adjust_pc(struct kvm_vcpu *vcpu); + extern u64 __vgic_v3_get_gic_config(void); extern u64 __vgic_v3_read_vmcr(void); extern void __vgic_v3_write_vmcr(u32 vmcr); diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c index 73629094f903..0812a496725f 100644 --- a/arch/arm64/kvm/hyp/exception.c +++ b/arch/arm64/kvm/hyp/exception.c @@ -296,7 +296,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset) *vcpu_pc(vcpu) = vect_offset; } -void kvm_inject_exception(struct kvm_vcpu *vcpu) +static void kvm_inject_exception(struct kvm_vcpu *vcpu) { if (vcpu_el1_is_32bit(vcpu)) { switch (vcpu->arch.flags & KVM_ARM64_EXCEPT_MASK) { @@ -329,3 +329,19 @@ void kvm_inject_exception(struct kvm_vcpu *vcpu) } } } + +/* + * Adjust the guest PC on entry, depending on flags provided by EL1 + * for the purpose of emulation (MMIO, sysreg) or exception injection. + */ +void __kvm_adjust_pc(struct kvm_vcpu *vcpu) +{ + if (vcpu->arch.flags & KVM_ARM64_PENDING_EXCEPTION) { + kvm_inject_exception(vcpu); + vcpu->arch.flags &= ~(KVM_ARM64_PENDING_EXCEPTION | + KVM_ARM64_EXCEPT_MASK); + } else if (vcpu->arch.flags & KVM_ARM64_INCREMENT_PC) { + kvm_skip_instr(vcpu); + vcpu->arch.flags &= ~KVM_ARM64_INCREMENT_PC; + } +} diff --git a/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h b/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h index 61716359035d..4fdfeabefeb4 100644 --- a/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h +++ b/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h @@ -13,8 +13,6 @@ #include <asm/kvm_emulate.h> #include <asm/kvm_host.h> -void kvm_inject_exception(struct kvm_vcpu *vcpu); - static inline void kvm_skip_instr(struct kvm_vcpu *vcpu) { if (vcpu_mode_is_32bit(vcpu)) { @@ -43,22 +41,6 @@ static inline void __kvm_skip_instr(struct kvm_vcpu *vcpu) write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); } -/* - * Adjust the guest PC on entry, depending on flags provided by EL1 - * for the purpose of emulation (MMIO, sysreg) or exception injection. - */ -static inline void __adjust_pc(struct kvm_vcpu *vcpu) -{ - if (vcpu->arch.flags & KVM_ARM64_PENDING_EXCEPTION) { - kvm_inject_exception(vcpu); - vcpu->arch.flags &= ~(KVM_ARM64_PENDING_EXCEPTION | - KVM_ARM64_EXCEPT_MASK); - } else if (vcpu->arch.flags & KVM_ARM64_INCREMENT_PC) { - kvm_skip_instr(vcpu); - vcpu->arch.flags &= ~KVM_ARM64_INCREMENT_PC; - } -} - /* * Skip an instruction while host sysregs are live. * Assumes host is always 64-bit. diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c index e9f6ea704d07..f7af9688c1f7 100644 --- a/arch/arm64/kvm/hyp/nvhe/switch.c +++ b/arch/arm64/kvm/hyp/nvhe/switch.c @@ -4,7 +4,6 @@ * Author: Marc Zyngier <marc.zyngier@arm.com> */ -#include <hyp/adjust_pc.h> #include <hyp/switch.h> #include <hyp/sysreg-sr.h> @@ -201,7 +200,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu) */ __debug_save_host_buffers_nvhe(vcpu); - __adjust_pc(vcpu); + __kvm_adjust_pc(vcpu); /* * We must restore the 32-bit state before the sysregs, thanks diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c index 7b8f7db5c1ed..b3229924d243 100644 --- a/arch/arm64/kvm/hyp/vhe/switch.c +++ b/arch/arm64/kvm/hyp/vhe/switch.c @@ -4,7 +4,6 @@ * Author: Marc Zyngier <marc.zyngier@arm.com> */ -#include <hyp/adjust_pc.h> #include <hyp/switch.h> #include <linux/arm-smccc.h> @@ -132,7 +131,7 @@ static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu) __load_guest_stage2(vcpu->arch.hw_mmu); __activate_traps(vcpu); - __adjust_pc(vcpu); + __kvm_adjust_pc(vcpu); sysreg_restore_guest_state_vhe(guest_ctxt); __debug_switch_to_guest(vcpu); -- 2.29.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/2] KVM: arm64: Move __adjust_pc out of line 2021-05-14 10:40 ` Marc Zyngier (?) @ 2021-05-14 14:07 ` Alexandru Elisei -1 siblings, 0 replies; 21+ messages in thread From: Alexandru Elisei @ 2021-05-14 14:07 UTC (permalink / raw) To: Marc Zyngier, kvm, kvmarm, linux-arm-kernel; +Cc: stable, kernel-team Hi Marc, On 5/14/21 11:40 AM, Marc Zyngier wrote: > In order to make it easy to call __adjust_pc() from the EL1 code > (in the case of nVHE), rename it to __kvm_adjust_pc() and move > it out of line. > > No expected functional change. Looks good to me. Ran the kvm-unit-tests test selftest-vectors-kernel, which goes out of its way to trigger an undefined exception, compiled for arm and arm64, under VHE, nVHE and protected modes on an odroid-c4, everything worked as expected: Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> Thanks, Alex > > Signed-off-by: Marc Zyngier <maz@kernel.org> > Cc: stable@vger.kernel.org # 5.11 > --- > arch/arm64/include/asm/kvm_asm.h | 2 ++ > arch/arm64/kvm/hyp/exception.c | 18 +++++++++++++++++- > arch/arm64/kvm/hyp/include/hyp/adjust_pc.h | 18 ------------------ > arch/arm64/kvm/hyp/nvhe/switch.c | 3 +-- > arch/arm64/kvm/hyp/vhe/switch.c | 3 +-- > 5 files changed, 21 insertions(+), 23 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h > index cf8df032b9c3..d5b11037401d 100644 > --- a/arch/arm64/include/asm/kvm_asm.h > +++ b/arch/arm64/include/asm/kvm_asm.h > @@ -201,6 +201,8 @@ extern void __kvm_timer_set_cntvoff(u64 cntvoff); > > extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu); > > +extern void __kvm_adjust_pc(struct kvm_vcpu *vcpu); > + > extern u64 __vgic_v3_get_gic_config(void); > extern u64 __vgic_v3_read_vmcr(void); > extern void __vgic_v3_write_vmcr(u32 vmcr); > diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c > index 73629094f903..0812a496725f 100644 > --- a/arch/arm64/kvm/hyp/exception.c > +++ b/arch/arm64/kvm/hyp/exception.c > @@ -296,7 +296,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset) > *vcpu_pc(vcpu) = vect_offset; > } > > -void kvm_inject_exception(struct kvm_vcpu *vcpu) > +static void kvm_inject_exception(struct kvm_vcpu *vcpu) > { > if (vcpu_el1_is_32bit(vcpu)) { > switch (vcpu->arch.flags & KVM_ARM64_EXCEPT_MASK) { > @@ -329,3 +329,19 @@ void kvm_inject_exception(struct kvm_vcpu *vcpu) > } > } > } > + > +/* > + * Adjust the guest PC on entry, depending on flags provided by EL1 > + * for the purpose of emulation (MMIO, sysreg) or exception injection. > + */ > +void __kvm_adjust_pc(struct kvm_vcpu *vcpu) > +{ > + if (vcpu->arch.flags & KVM_ARM64_PENDING_EXCEPTION) { > + kvm_inject_exception(vcpu); > + vcpu->arch.flags &= ~(KVM_ARM64_PENDING_EXCEPTION | > + KVM_ARM64_EXCEPT_MASK); > + } else if (vcpu->arch.flags & KVM_ARM64_INCREMENT_PC) { > + kvm_skip_instr(vcpu); > + vcpu->arch.flags &= ~KVM_ARM64_INCREMENT_PC; > + } > +} > diff --git a/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h b/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h > index 61716359035d..4fdfeabefeb4 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h > +++ b/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h > @@ -13,8 +13,6 @@ > #include <asm/kvm_emulate.h> > #include <asm/kvm_host.h> > > -void kvm_inject_exception(struct kvm_vcpu *vcpu); > - > static inline void kvm_skip_instr(struct kvm_vcpu *vcpu) > { > if (vcpu_mode_is_32bit(vcpu)) { > @@ -43,22 +41,6 @@ static inline void __kvm_skip_instr(struct kvm_vcpu *vcpu) > write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); > } > > -/* > - * Adjust the guest PC on entry, depending on flags provided by EL1 > - * for the purpose of emulation (MMIO, sysreg) or exception injection. > - */ > -static inline void __adjust_pc(struct kvm_vcpu *vcpu) > -{ > - if (vcpu->arch.flags & KVM_ARM64_PENDING_EXCEPTION) { > - kvm_inject_exception(vcpu); > - vcpu->arch.flags &= ~(KVM_ARM64_PENDING_EXCEPTION | > - KVM_ARM64_EXCEPT_MASK); > - } else if (vcpu->arch.flags & KVM_ARM64_INCREMENT_PC) { > - kvm_skip_instr(vcpu); > - vcpu->arch.flags &= ~KVM_ARM64_INCREMENT_PC; > - } > -} > - > /* > * Skip an instruction while host sysregs are live. > * Assumes host is always 64-bit. > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c > index e9f6ea704d07..f7af9688c1f7 100644 > --- a/arch/arm64/kvm/hyp/nvhe/switch.c > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c > @@ -4,7 +4,6 @@ > * Author: Marc Zyngier <marc.zyngier@arm.com> > */ > > -#include <hyp/adjust_pc.h> > #include <hyp/switch.h> > #include <hyp/sysreg-sr.h> > > @@ -201,7 +200,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu) > */ > __debug_save_host_buffers_nvhe(vcpu); > > - __adjust_pc(vcpu); > + __kvm_adjust_pc(vcpu); > > /* > * We must restore the 32-bit state before the sysregs, thanks > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c > index 7b8f7db5c1ed..b3229924d243 100644 > --- a/arch/arm64/kvm/hyp/vhe/switch.c > +++ b/arch/arm64/kvm/hyp/vhe/switch.c > @@ -4,7 +4,6 @@ > * Author: Marc Zyngier <marc.zyngier@arm.com> > */ > > -#include <hyp/adjust_pc.h> > #include <hyp/switch.h> > > #include <linux/arm-smccc.h> > @@ -132,7 +131,7 @@ static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu) > __load_guest_stage2(vcpu->arch.hw_mmu); > __activate_traps(vcpu); > > - __adjust_pc(vcpu); > + __kvm_adjust_pc(vcpu); > > sysreg_restore_guest_state_vhe(guest_ctxt); > __debug_switch_to_guest(vcpu); _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/2] KVM: arm64: Move __adjust_pc out of line @ 2021-05-14 14:07 ` Alexandru Elisei 0 siblings, 0 replies; 21+ messages in thread From: Alexandru Elisei @ 2021-05-14 14:07 UTC (permalink / raw) To: Marc Zyngier, kvm, kvmarm, linux-arm-kernel Cc: Zenghui Yu, Fuad Tabba, James Morse, Suzuki K Poulose, kernel-team, stable Hi Marc, On 5/14/21 11:40 AM, Marc Zyngier wrote: > In order to make it easy to call __adjust_pc() from the EL1 code > (in the case of nVHE), rename it to __kvm_adjust_pc() and move > it out of line. > > No expected functional change. Looks good to me. Ran the kvm-unit-tests test selftest-vectors-kernel, which goes out of its way to trigger an undefined exception, compiled for arm and arm64, under VHE, nVHE and protected modes on an odroid-c4, everything worked as expected: Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> Thanks, Alex > > Signed-off-by: Marc Zyngier <maz@kernel.org> > Cc: stable@vger.kernel.org # 5.11 > --- > arch/arm64/include/asm/kvm_asm.h | 2 ++ > arch/arm64/kvm/hyp/exception.c | 18 +++++++++++++++++- > arch/arm64/kvm/hyp/include/hyp/adjust_pc.h | 18 ------------------ > arch/arm64/kvm/hyp/nvhe/switch.c | 3 +-- > arch/arm64/kvm/hyp/vhe/switch.c | 3 +-- > 5 files changed, 21 insertions(+), 23 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h > index cf8df032b9c3..d5b11037401d 100644 > --- a/arch/arm64/include/asm/kvm_asm.h > +++ b/arch/arm64/include/asm/kvm_asm.h > @@ -201,6 +201,8 @@ extern void __kvm_timer_set_cntvoff(u64 cntvoff); > > extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu); > > +extern void __kvm_adjust_pc(struct kvm_vcpu *vcpu); > + > extern u64 __vgic_v3_get_gic_config(void); > extern u64 __vgic_v3_read_vmcr(void); > extern void __vgic_v3_write_vmcr(u32 vmcr); > diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c > index 73629094f903..0812a496725f 100644 > --- a/arch/arm64/kvm/hyp/exception.c > +++ b/arch/arm64/kvm/hyp/exception.c > @@ -296,7 +296,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset) > *vcpu_pc(vcpu) = vect_offset; > } > > -void kvm_inject_exception(struct kvm_vcpu *vcpu) > +static void kvm_inject_exception(struct kvm_vcpu *vcpu) > { > if (vcpu_el1_is_32bit(vcpu)) { > switch (vcpu->arch.flags & KVM_ARM64_EXCEPT_MASK) { > @@ -329,3 +329,19 @@ void kvm_inject_exception(struct kvm_vcpu *vcpu) > } > } > } > + > +/* > + * Adjust the guest PC on entry, depending on flags provided by EL1 > + * for the purpose of emulation (MMIO, sysreg) or exception injection. > + */ > +void __kvm_adjust_pc(struct kvm_vcpu *vcpu) > +{ > + if (vcpu->arch.flags & KVM_ARM64_PENDING_EXCEPTION) { > + kvm_inject_exception(vcpu); > + vcpu->arch.flags &= ~(KVM_ARM64_PENDING_EXCEPTION | > + KVM_ARM64_EXCEPT_MASK); > + } else if (vcpu->arch.flags & KVM_ARM64_INCREMENT_PC) { > + kvm_skip_instr(vcpu); > + vcpu->arch.flags &= ~KVM_ARM64_INCREMENT_PC; > + } > +} > diff --git a/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h b/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h > index 61716359035d..4fdfeabefeb4 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h > +++ b/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h > @@ -13,8 +13,6 @@ > #include <asm/kvm_emulate.h> > #include <asm/kvm_host.h> > > -void kvm_inject_exception(struct kvm_vcpu *vcpu); > - > static inline void kvm_skip_instr(struct kvm_vcpu *vcpu) > { > if (vcpu_mode_is_32bit(vcpu)) { > @@ -43,22 +41,6 @@ static inline void __kvm_skip_instr(struct kvm_vcpu *vcpu) > write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); > } > > -/* > - * Adjust the guest PC on entry, depending on flags provided by EL1 > - * for the purpose of emulation (MMIO, sysreg) or exception injection. > - */ > -static inline void __adjust_pc(struct kvm_vcpu *vcpu) > -{ > - if (vcpu->arch.flags & KVM_ARM64_PENDING_EXCEPTION) { > - kvm_inject_exception(vcpu); > - vcpu->arch.flags &= ~(KVM_ARM64_PENDING_EXCEPTION | > - KVM_ARM64_EXCEPT_MASK); > - } else if (vcpu->arch.flags & KVM_ARM64_INCREMENT_PC) { > - kvm_skip_instr(vcpu); > - vcpu->arch.flags &= ~KVM_ARM64_INCREMENT_PC; > - } > -} > - > /* > * Skip an instruction while host sysregs are live. > * Assumes host is always 64-bit. > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c > index e9f6ea704d07..f7af9688c1f7 100644 > --- a/arch/arm64/kvm/hyp/nvhe/switch.c > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c > @@ -4,7 +4,6 @@ > * Author: Marc Zyngier <marc.zyngier@arm.com> > */ > > -#include <hyp/adjust_pc.h> > #include <hyp/switch.h> > #include <hyp/sysreg-sr.h> > > @@ -201,7 +200,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu) > */ > __debug_save_host_buffers_nvhe(vcpu); > > - __adjust_pc(vcpu); > + __kvm_adjust_pc(vcpu); > > /* > * We must restore the 32-bit state before the sysregs, thanks > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c > index 7b8f7db5c1ed..b3229924d243 100644 > --- a/arch/arm64/kvm/hyp/vhe/switch.c > +++ b/arch/arm64/kvm/hyp/vhe/switch.c > @@ -4,7 +4,6 @@ > * Author: Marc Zyngier <marc.zyngier@arm.com> > */ > > -#include <hyp/adjust_pc.h> > #include <hyp/switch.h> > > #include <linux/arm-smccc.h> > @@ -132,7 +131,7 @@ static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu) > __load_guest_stage2(vcpu->arch.hw_mmu); > __activate_traps(vcpu); > > - __adjust_pc(vcpu); > + __kvm_adjust_pc(vcpu); > > sysreg_restore_guest_state_vhe(guest_ctxt); > __debug_switch_to_guest(vcpu); ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/2] KVM: arm64: Move __adjust_pc out of line @ 2021-05-14 14:07 ` Alexandru Elisei 0 siblings, 0 replies; 21+ messages in thread From: Alexandru Elisei @ 2021-05-14 14:07 UTC (permalink / raw) To: Marc Zyngier, kvm, kvmarm, linux-arm-kernel Cc: Zenghui Yu, Fuad Tabba, James Morse, Suzuki K Poulose, kernel-team, stable Hi Marc, On 5/14/21 11:40 AM, Marc Zyngier wrote: > In order to make it easy to call __adjust_pc() from the EL1 code > (in the case of nVHE), rename it to __kvm_adjust_pc() and move > it out of line. > > No expected functional change. Looks good to me. Ran the kvm-unit-tests test selftest-vectors-kernel, which goes out of its way to trigger an undefined exception, compiled for arm and arm64, under VHE, nVHE and protected modes on an odroid-c4, everything worked as expected: Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> Thanks, Alex > > Signed-off-by: Marc Zyngier <maz@kernel.org> > Cc: stable@vger.kernel.org # 5.11 > --- > arch/arm64/include/asm/kvm_asm.h | 2 ++ > arch/arm64/kvm/hyp/exception.c | 18 +++++++++++++++++- > arch/arm64/kvm/hyp/include/hyp/adjust_pc.h | 18 ------------------ > arch/arm64/kvm/hyp/nvhe/switch.c | 3 +-- > arch/arm64/kvm/hyp/vhe/switch.c | 3 +-- > 5 files changed, 21 insertions(+), 23 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h > index cf8df032b9c3..d5b11037401d 100644 > --- a/arch/arm64/include/asm/kvm_asm.h > +++ b/arch/arm64/include/asm/kvm_asm.h > @@ -201,6 +201,8 @@ extern void __kvm_timer_set_cntvoff(u64 cntvoff); > > extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu); > > +extern void __kvm_adjust_pc(struct kvm_vcpu *vcpu); > + > extern u64 __vgic_v3_get_gic_config(void); > extern u64 __vgic_v3_read_vmcr(void); > extern void __vgic_v3_write_vmcr(u32 vmcr); > diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c > index 73629094f903..0812a496725f 100644 > --- a/arch/arm64/kvm/hyp/exception.c > +++ b/arch/arm64/kvm/hyp/exception.c > @@ -296,7 +296,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset) > *vcpu_pc(vcpu) = vect_offset; > } > > -void kvm_inject_exception(struct kvm_vcpu *vcpu) > +static void kvm_inject_exception(struct kvm_vcpu *vcpu) > { > if (vcpu_el1_is_32bit(vcpu)) { > switch (vcpu->arch.flags & KVM_ARM64_EXCEPT_MASK) { > @@ -329,3 +329,19 @@ void kvm_inject_exception(struct kvm_vcpu *vcpu) > } > } > } > + > +/* > + * Adjust the guest PC on entry, depending on flags provided by EL1 > + * for the purpose of emulation (MMIO, sysreg) or exception injection. > + */ > +void __kvm_adjust_pc(struct kvm_vcpu *vcpu) > +{ > + if (vcpu->arch.flags & KVM_ARM64_PENDING_EXCEPTION) { > + kvm_inject_exception(vcpu); > + vcpu->arch.flags &= ~(KVM_ARM64_PENDING_EXCEPTION | > + KVM_ARM64_EXCEPT_MASK); > + } else if (vcpu->arch.flags & KVM_ARM64_INCREMENT_PC) { > + kvm_skip_instr(vcpu); > + vcpu->arch.flags &= ~KVM_ARM64_INCREMENT_PC; > + } > +} > diff --git a/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h b/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h > index 61716359035d..4fdfeabefeb4 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h > +++ b/arch/arm64/kvm/hyp/include/hyp/adjust_pc.h > @@ -13,8 +13,6 @@ > #include <asm/kvm_emulate.h> > #include <asm/kvm_host.h> > > -void kvm_inject_exception(struct kvm_vcpu *vcpu); > - > static inline void kvm_skip_instr(struct kvm_vcpu *vcpu) > { > if (vcpu_mode_is_32bit(vcpu)) { > @@ -43,22 +41,6 @@ static inline void __kvm_skip_instr(struct kvm_vcpu *vcpu) > write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); > } > > -/* > - * Adjust the guest PC on entry, depending on flags provided by EL1 > - * for the purpose of emulation (MMIO, sysreg) or exception injection. > - */ > -static inline void __adjust_pc(struct kvm_vcpu *vcpu) > -{ > - if (vcpu->arch.flags & KVM_ARM64_PENDING_EXCEPTION) { > - kvm_inject_exception(vcpu); > - vcpu->arch.flags &= ~(KVM_ARM64_PENDING_EXCEPTION | > - KVM_ARM64_EXCEPT_MASK); > - } else if (vcpu->arch.flags & KVM_ARM64_INCREMENT_PC) { > - kvm_skip_instr(vcpu); > - vcpu->arch.flags &= ~KVM_ARM64_INCREMENT_PC; > - } > -} > - > /* > * Skip an instruction while host sysregs are live. > * Assumes host is always 64-bit. > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c > index e9f6ea704d07..f7af9688c1f7 100644 > --- a/arch/arm64/kvm/hyp/nvhe/switch.c > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c > @@ -4,7 +4,6 @@ > * Author: Marc Zyngier <marc.zyngier@arm.com> > */ > > -#include <hyp/adjust_pc.h> > #include <hyp/switch.h> > #include <hyp/sysreg-sr.h> > > @@ -201,7 +200,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu) > */ > __debug_save_host_buffers_nvhe(vcpu); > > - __adjust_pc(vcpu); > + __kvm_adjust_pc(vcpu); > > /* > * We must restore the 32-bit state before the sysregs, thanks > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c > index 7b8f7db5c1ed..b3229924d243 100644 > --- a/arch/arm64/kvm/hyp/vhe/switch.c > +++ b/arch/arm64/kvm/hyp/vhe/switch.c > @@ -4,7 +4,6 @@ > * Author: Marc Zyngier <marc.zyngier@arm.com> > */ > > -#include <hyp/adjust_pc.h> > #include <hyp/switch.h> > > #include <linux/arm-smccc.h> > @@ -132,7 +131,7 @@ static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu) > __load_guest_stage2(vcpu->arch.hw_mmu); > __activate_traps(vcpu); > > - __adjust_pc(vcpu); > + __kvm_adjust_pc(vcpu); > > sysreg_restore_guest_state_vhe(guest_ctxt); > __debug_switch_to_guest(vcpu); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 2/2] KVM: arm64: Commit pending PC adjustemnts before returning to userspace 2021-05-14 10:40 ` Marc Zyngier (?) @ 2021-05-14 10:40 ` Marc Zyngier -1 siblings, 0 replies; 21+ messages in thread From: Marc Zyngier @ 2021-05-14 10:40 UTC (permalink / raw) To: kvm, kvmarm, linux-arm-kernel; +Cc: kernel-team, stable KVM currently updates PC (and the corresponding exception state) using a two phase approach: first by setting a set of flags, then by converting these flags into a state update when the vcpu is about to enter the guest. However, this creates a disconnect with userspace if the vcpu thread returns there with any exception/PC flag set. In this case, the exposed context is wrong, as userpsace doesn't have access to these flags (they aren't architectural). It also means that these flags are preserved across a reset, which isn't expected. To solve this problem, force an explicit synchronisation of the exception state on vcpu exit to userspace. As an optimisation for nVHE systems, only perform this when there is something pending. Reported-by: Zenghui Yu <yuzenghui@huawei.com> Signed-off-by: Marc Zyngier <maz@kernel.org> Cc: stable@vger.kernel.org # 5.11 --- arch/arm64/include/asm/kvm_asm.h | 1 + arch/arm64/kvm/arm.c | 10 ++++++++++ arch/arm64/kvm/hyp/exception.c | 4 ++-- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++ 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index d5b11037401d..5e9b33cbac51 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -63,6 +63,7 @@ #define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18 #define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19 #define __KVM_HOST_SMCCC_FUNC___pkvm_mark_hyp 20 +#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 21 #ifndef __ASSEMBLY__ diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 1cb39c0803a4..c4fe2b71f429 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -897,6 +897,16 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) kvm_sigset_deactivate(vcpu); + /* + * In the unlikely event that we are returning to userspace + * with pending exceptions or PC adjustment, commit these + * adjustments in order to give userspace a consistent view of + * the vcpu state. + */ + if (unlikely(vcpu->arch.flags & (KVM_ARM64_PENDING_EXCEPTION | + KVM_ARM64_INCREMENT_PC))) + kvm_call_hyp(__kvm_adjust_pc, vcpu); + vcpu_put(vcpu); return ret; } diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c index 0812a496725f..11541b94b328 100644 --- a/arch/arm64/kvm/hyp/exception.c +++ b/arch/arm64/kvm/hyp/exception.c @@ -331,8 +331,8 @@ static void kvm_inject_exception(struct kvm_vcpu *vcpu) } /* - * Adjust the guest PC on entry, depending on flags provided by EL1 - * for the purpose of emulation (MMIO, sysreg) or exception injection. + * Adjust the guest PC (and potentially exception state) depending on + * flags provided by the emulation code. */ void __kvm_adjust_pc(struct kvm_vcpu *vcpu) { diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c index f36420a80474..1632f001f4ed 100644 --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c @@ -28,6 +28,13 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt) cpu_reg(host_ctxt, 1) = __kvm_vcpu_run(kern_hyp_va(vcpu)); } +static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt) +{ + DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1); + + __kvm_adjust_pc(kern_hyp_va(vcpu)); +} + static void handle___kvm_flush_vm_context(struct kvm_cpu_context *host_ctxt) { __kvm_flush_vm_context(); @@ -170,6 +177,7 @@ typedef void (*hcall_t)(struct kvm_cpu_context *); static const hcall_t host_hcall[] = { HANDLE_FUNC(__kvm_vcpu_run), + HANDLE_FUNC(__kvm_adjust_pc), HANDLE_FUNC(__kvm_flush_vm_context), HANDLE_FUNC(__kvm_tlb_flush_vmid_ipa), HANDLE_FUNC(__kvm_tlb_flush_vmid), -- 2.29.2 _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 2/2] KVM: arm64: Commit pending PC adjustemnts before returning to userspace @ 2021-05-14 10:40 ` Marc Zyngier 0 siblings, 0 replies; 21+ messages in thread From: Marc Zyngier @ 2021-05-14 10:40 UTC (permalink / raw) To: kvm, kvmarm, linux-arm-kernel Cc: Zenghui Yu, Fuad Tabba, James Morse, Suzuki K Poulose, Alexandru Elisei, kernel-team, stable KVM currently updates PC (and the corresponding exception state) using a two phase approach: first by setting a set of flags, then by converting these flags into a state update when the vcpu is about to enter the guest. However, this creates a disconnect with userspace if the vcpu thread returns there with any exception/PC flag set. In this case, the exposed context is wrong, as userpsace doesn't have access to these flags (they aren't architectural). It also means that these flags are preserved across a reset, which isn't expected. To solve this problem, force an explicit synchronisation of the exception state on vcpu exit to userspace. As an optimisation for nVHE systems, only perform this when there is something pending. Reported-by: Zenghui Yu <yuzenghui@huawei.com> Signed-off-by: Marc Zyngier <maz@kernel.org> Cc: stable@vger.kernel.org # 5.11 --- arch/arm64/include/asm/kvm_asm.h | 1 + arch/arm64/kvm/arm.c | 10 ++++++++++ arch/arm64/kvm/hyp/exception.c | 4 ++-- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++ 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index d5b11037401d..5e9b33cbac51 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -63,6 +63,7 @@ #define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18 #define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19 #define __KVM_HOST_SMCCC_FUNC___pkvm_mark_hyp 20 +#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 21 #ifndef __ASSEMBLY__ diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 1cb39c0803a4..c4fe2b71f429 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -897,6 +897,16 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) kvm_sigset_deactivate(vcpu); + /* + * In the unlikely event that we are returning to userspace + * with pending exceptions or PC adjustment, commit these + * adjustments in order to give userspace a consistent view of + * the vcpu state. + */ + if (unlikely(vcpu->arch.flags & (KVM_ARM64_PENDING_EXCEPTION | + KVM_ARM64_INCREMENT_PC))) + kvm_call_hyp(__kvm_adjust_pc, vcpu); + vcpu_put(vcpu); return ret; } diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c index 0812a496725f..11541b94b328 100644 --- a/arch/arm64/kvm/hyp/exception.c +++ b/arch/arm64/kvm/hyp/exception.c @@ -331,8 +331,8 @@ static void kvm_inject_exception(struct kvm_vcpu *vcpu) } /* - * Adjust the guest PC on entry, depending on flags provided by EL1 - * for the purpose of emulation (MMIO, sysreg) or exception injection. + * Adjust the guest PC (and potentially exception state) depending on + * flags provided by the emulation code. */ void __kvm_adjust_pc(struct kvm_vcpu *vcpu) { diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c index f36420a80474..1632f001f4ed 100644 --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c @@ -28,6 +28,13 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt) cpu_reg(host_ctxt, 1) = __kvm_vcpu_run(kern_hyp_va(vcpu)); } +static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt) +{ + DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1); + + __kvm_adjust_pc(kern_hyp_va(vcpu)); +} + static void handle___kvm_flush_vm_context(struct kvm_cpu_context *host_ctxt) { __kvm_flush_vm_context(); @@ -170,6 +177,7 @@ typedef void (*hcall_t)(struct kvm_cpu_context *); static const hcall_t host_hcall[] = { HANDLE_FUNC(__kvm_vcpu_run), + HANDLE_FUNC(__kvm_adjust_pc), HANDLE_FUNC(__kvm_flush_vm_context), HANDLE_FUNC(__kvm_tlb_flush_vmid_ipa), HANDLE_FUNC(__kvm_tlb_flush_vmid), -- 2.29.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 2/2] KVM: arm64: Commit pending PC adjustemnts before returning to userspace @ 2021-05-14 10:40 ` Marc Zyngier 0 siblings, 0 replies; 21+ messages in thread From: Marc Zyngier @ 2021-05-14 10:40 UTC (permalink / raw) To: kvm, kvmarm, linux-arm-kernel Cc: Zenghui Yu, Fuad Tabba, James Morse, Suzuki K Poulose, Alexandru Elisei, kernel-team, stable KVM currently updates PC (and the corresponding exception state) using a two phase approach: first by setting a set of flags, then by converting these flags into a state update when the vcpu is about to enter the guest. However, this creates a disconnect with userspace if the vcpu thread returns there with any exception/PC flag set. In this case, the exposed context is wrong, as userpsace doesn't have access to these flags (they aren't architectural). It also means that these flags are preserved across a reset, which isn't expected. To solve this problem, force an explicit synchronisation of the exception state on vcpu exit to userspace. As an optimisation for nVHE systems, only perform this when there is something pending. Reported-by: Zenghui Yu <yuzenghui@huawei.com> Signed-off-by: Marc Zyngier <maz@kernel.org> Cc: stable@vger.kernel.org # 5.11 --- arch/arm64/include/asm/kvm_asm.h | 1 + arch/arm64/kvm/arm.c | 10 ++++++++++ arch/arm64/kvm/hyp/exception.c | 4 ++-- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++ 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index d5b11037401d..5e9b33cbac51 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -63,6 +63,7 @@ #define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18 #define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19 #define __KVM_HOST_SMCCC_FUNC___pkvm_mark_hyp 20 +#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 21 #ifndef __ASSEMBLY__ diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 1cb39c0803a4..c4fe2b71f429 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -897,6 +897,16 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) kvm_sigset_deactivate(vcpu); + /* + * In the unlikely event that we are returning to userspace + * with pending exceptions or PC adjustment, commit these + * adjustments in order to give userspace a consistent view of + * the vcpu state. + */ + if (unlikely(vcpu->arch.flags & (KVM_ARM64_PENDING_EXCEPTION | + KVM_ARM64_INCREMENT_PC))) + kvm_call_hyp(__kvm_adjust_pc, vcpu); + vcpu_put(vcpu); return ret; } diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c index 0812a496725f..11541b94b328 100644 --- a/arch/arm64/kvm/hyp/exception.c +++ b/arch/arm64/kvm/hyp/exception.c @@ -331,8 +331,8 @@ static void kvm_inject_exception(struct kvm_vcpu *vcpu) } /* - * Adjust the guest PC on entry, depending on flags provided by EL1 - * for the purpose of emulation (MMIO, sysreg) or exception injection. + * Adjust the guest PC (and potentially exception state) depending on + * flags provided by the emulation code. */ void __kvm_adjust_pc(struct kvm_vcpu *vcpu) { diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c index f36420a80474..1632f001f4ed 100644 --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c @@ -28,6 +28,13 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt) cpu_reg(host_ctxt, 1) = __kvm_vcpu_run(kern_hyp_va(vcpu)); } +static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt) +{ + DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1); + + __kvm_adjust_pc(kern_hyp_va(vcpu)); +} + static void handle___kvm_flush_vm_context(struct kvm_cpu_context *host_ctxt) { __kvm_flush_vm_context(); @@ -170,6 +177,7 @@ typedef void (*hcall_t)(struct kvm_cpu_context *); static const hcall_t host_hcall[] = { HANDLE_FUNC(__kvm_vcpu_run), + HANDLE_FUNC(__kvm_adjust_pc), HANDLE_FUNC(__kvm_flush_vm_context), HANDLE_FUNC(__kvm_tlb_flush_vmid_ipa), HANDLE_FUNC(__kvm_tlb_flush_vmid), -- 2.29.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] KVM: arm64: Commit pending PC adjustemnts before returning to userspace 2021-05-14 10:40 ` Marc Zyngier (?) @ 2021-05-14 14:08 ` Alexandru Elisei -1 siblings, 0 replies; 21+ messages in thread From: Alexandru Elisei @ 2021-05-14 14:08 UTC (permalink / raw) To: Marc Zyngier, kvm, kvmarm, linux-arm-kernel; +Cc: stable, kernel-team Hi Marc, On 5/14/21 11:40 AM, Marc Zyngier wrote: > KVM currently updates PC (and the corresponding exception state) > using a two phase approach: first by setting a set of flags, > then by converting these flags into a state update when the vcpu > is about to enter the guest. > > However, this creates a disconnect with userspace if the vcpu thread > returns there with any exception/PC flag set. In this case, the exposed > context is wrong, as userpsace doesn't have access to these flags Nitpick: s/userpsace/userspace > (they aren't architectural). It also means that these flags are > preserved across a reset, which isn't expected. > > To solve this problem, force an explicit synchronisation of the > exception state on vcpu exit to userspace. As an optimisation > for nVHE systems, only perform this when there is something pending. > > Reported-by: Zenghui Yu <yuzenghui@huawei.com> > Signed-off-by: Marc Zyngier <maz@kernel.org> > Cc: stable@vger.kernel.org # 5.11 > --- > arch/arm64/include/asm/kvm_asm.h | 1 + > arch/arm64/kvm/arm.c | 10 ++++++++++ > arch/arm64/kvm/hyp/exception.c | 4 ++-- > arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++ > 4 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h > index d5b11037401d..5e9b33cbac51 100644 > --- a/arch/arm64/include/asm/kvm_asm.h > +++ b/arch/arm64/include/asm/kvm_asm.h > @@ -63,6 +63,7 @@ > #define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18 > #define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19 > #define __KVM_HOST_SMCCC_FUNC___pkvm_mark_hyp 20 > +#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 21 > > #ifndef __ASSEMBLY__ > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 1cb39c0803a4..c4fe2b71f429 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -897,6 +897,16 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) > > kvm_sigset_deactivate(vcpu); > > + /* > + * In the unlikely event that we are returning to userspace > + * with pending exceptions or PC adjustment, commit these > + * adjustments in order to give userspace a consistent view of > + * the vcpu state. > + */ > + if (unlikely(vcpu->arch.flags & (KVM_ARM64_PENDING_EXCEPTION | > + KVM_ARM64_INCREMENT_PC))) > + kvm_call_hyp(__kvm_adjust_pc, vcpu); > + > vcpu_put(vcpu); > return ret; > } > diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c > index 0812a496725f..11541b94b328 100644 > --- a/arch/arm64/kvm/hyp/exception.c > +++ b/arch/arm64/kvm/hyp/exception.c > @@ -331,8 +331,8 @@ static void kvm_inject_exception(struct kvm_vcpu *vcpu) > } > > /* > - * Adjust the guest PC on entry, depending on flags provided by EL1 > - * for the purpose of emulation (MMIO, sysreg) or exception injection. > + * Adjust the guest PC (and potentially exception state) depending on > + * flags provided by the emulation code. > */ > void __kvm_adjust_pc(struct kvm_vcpu *vcpu) > { > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > index f36420a80474..1632f001f4ed 100644 > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > @@ -28,6 +28,13 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt) > cpu_reg(host_ctxt, 1) = __kvm_vcpu_run(kern_hyp_va(vcpu)); > } > > +static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt) > +{ > + DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1); > + > + __kvm_adjust_pc(kern_hyp_va(vcpu)); > +} > + > static void handle___kvm_flush_vm_context(struct kvm_cpu_context *host_ctxt) > { > __kvm_flush_vm_context(); > @@ -170,6 +177,7 @@ typedef void (*hcall_t)(struct kvm_cpu_context *); > > static const hcall_t host_hcall[] = { > HANDLE_FUNC(__kvm_vcpu_run), > + HANDLE_FUNC(__kvm_adjust_pc), > HANDLE_FUNC(__kvm_flush_vm_context), > HANDLE_FUNC(__kvm_tlb_flush_vmid_ipa), > HANDLE_FUNC(__kvm_tlb_flush_vmid), I'm guessing that the comment mentioned in the cover letter should be in this patch, right? Or is the changelog from the cover letter stale? Regardless, I trust your judgement regarding the comment and the patch looks correct: Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> Thanks, Alex _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] KVM: arm64: Commit pending PC adjustemnts before returning to userspace @ 2021-05-14 14:08 ` Alexandru Elisei 0 siblings, 0 replies; 21+ messages in thread From: Alexandru Elisei @ 2021-05-14 14:08 UTC (permalink / raw) To: Marc Zyngier, kvm, kvmarm, linux-arm-kernel Cc: Zenghui Yu, Fuad Tabba, James Morse, Suzuki K Poulose, kernel-team, stable Hi Marc, On 5/14/21 11:40 AM, Marc Zyngier wrote: > KVM currently updates PC (and the corresponding exception state) > using a two phase approach: first by setting a set of flags, > then by converting these flags into a state update when the vcpu > is about to enter the guest. > > However, this creates a disconnect with userspace if the vcpu thread > returns there with any exception/PC flag set. In this case, the exposed > context is wrong, as userpsace doesn't have access to these flags Nitpick: s/userpsace/userspace > (they aren't architectural). It also means that these flags are > preserved across a reset, which isn't expected. > > To solve this problem, force an explicit synchronisation of the > exception state on vcpu exit to userspace. As an optimisation > for nVHE systems, only perform this when there is something pending. > > Reported-by: Zenghui Yu <yuzenghui@huawei.com> > Signed-off-by: Marc Zyngier <maz@kernel.org> > Cc: stable@vger.kernel.org # 5.11 > --- > arch/arm64/include/asm/kvm_asm.h | 1 + > arch/arm64/kvm/arm.c | 10 ++++++++++ > arch/arm64/kvm/hyp/exception.c | 4 ++-- > arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++ > 4 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h > index d5b11037401d..5e9b33cbac51 100644 > --- a/arch/arm64/include/asm/kvm_asm.h > +++ b/arch/arm64/include/asm/kvm_asm.h > @@ -63,6 +63,7 @@ > #define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18 > #define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19 > #define __KVM_HOST_SMCCC_FUNC___pkvm_mark_hyp 20 > +#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 21 > > #ifndef __ASSEMBLY__ > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 1cb39c0803a4..c4fe2b71f429 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -897,6 +897,16 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) > > kvm_sigset_deactivate(vcpu); > > + /* > + * In the unlikely event that we are returning to userspace > + * with pending exceptions or PC adjustment, commit these > + * adjustments in order to give userspace a consistent view of > + * the vcpu state. > + */ > + if (unlikely(vcpu->arch.flags & (KVM_ARM64_PENDING_EXCEPTION | > + KVM_ARM64_INCREMENT_PC))) > + kvm_call_hyp(__kvm_adjust_pc, vcpu); > + > vcpu_put(vcpu); > return ret; > } > diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c > index 0812a496725f..11541b94b328 100644 > --- a/arch/arm64/kvm/hyp/exception.c > +++ b/arch/arm64/kvm/hyp/exception.c > @@ -331,8 +331,8 @@ static void kvm_inject_exception(struct kvm_vcpu *vcpu) > } > > /* > - * Adjust the guest PC on entry, depending on flags provided by EL1 > - * for the purpose of emulation (MMIO, sysreg) or exception injection. > + * Adjust the guest PC (and potentially exception state) depending on > + * flags provided by the emulation code. > */ > void __kvm_adjust_pc(struct kvm_vcpu *vcpu) > { > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > index f36420a80474..1632f001f4ed 100644 > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > @@ -28,6 +28,13 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt) > cpu_reg(host_ctxt, 1) = __kvm_vcpu_run(kern_hyp_va(vcpu)); > } > > +static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt) > +{ > + DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1); > + > + __kvm_adjust_pc(kern_hyp_va(vcpu)); > +} > + > static void handle___kvm_flush_vm_context(struct kvm_cpu_context *host_ctxt) > { > __kvm_flush_vm_context(); > @@ -170,6 +177,7 @@ typedef void (*hcall_t)(struct kvm_cpu_context *); > > static const hcall_t host_hcall[] = { > HANDLE_FUNC(__kvm_vcpu_run), > + HANDLE_FUNC(__kvm_adjust_pc), > HANDLE_FUNC(__kvm_flush_vm_context), > HANDLE_FUNC(__kvm_tlb_flush_vmid_ipa), > HANDLE_FUNC(__kvm_tlb_flush_vmid), I'm guessing that the comment mentioned in the cover letter should be in this patch, right? Or is the changelog from the cover letter stale? Regardless, I trust your judgement regarding the comment and the patch looks correct: Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> Thanks, Alex ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] KVM: arm64: Commit pending PC adjustemnts before returning to userspace @ 2021-05-14 14:08 ` Alexandru Elisei 0 siblings, 0 replies; 21+ messages in thread From: Alexandru Elisei @ 2021-05-14 14:08 UTC (permalink / raw) To: Marc Zyngier, kvm, kvmarm, linux-arm-kernel Cc: Zenghui Yu, Fuad Tabba, James Morse, Suzuki K Poulose, kernel-team, stable Hi Marc, On 5/14/21 11:40 AM, Marc Zyngier wrote: > KVM currently updates PC (and the corresponding exception state) > using a two phase approach: first by setting a set of flags, > then by converting these flags into a state update when the vcpu > is about to enter the guest. > > However, this creates a disconnect with userspace if the vcpu thread > returns there with any exception/PC flag set. In this case, the exposed > context is wrong, as userpsace doesn't have access to these flags Nitpick: s/userpsace/userspace > (they aren't architectural). It also means that these flags are > preserved across a reset, which isn't expected. > > To solve this problem, force an explicit synchronisation of the > exception state on vcpu exit to userspace. As an optimisation > for nVHE systems, only perform this when there is something pending. > > Reported-by: Zenghui Yu <yuzenghui@huawei.com> > Signed-off-by: Marc Zyngier <maz@kernel.org> > Cc: stable@vger.kernel.org # 5.11 > --- > arch/arm64/include/asm/kvm_asm.h | 1 + > arch/arm64/kvm/arm.c | 10 ++++++++++ > arch/arm64/kvm/hyp/exception.c | 4 ++-- > arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++ > 4 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h > index d5b11037401d..5e9b33cbac51 100644 > --- a/arch/arm64/include/asm/kvm_asm.h > +++ b/arch/arm64/include/asm/kvm_asm.h > @@ -63,6 +63,7 @@ > #define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18 > #define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19 > #define __KVM_HOST_SMCCC_FUNC___pkvm_mark_hyp 20 > +#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 21 > > #ifndef __ASSEMBLY__ > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 1cb39c0803a4..c4fe2b71f429 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -897,6 +897,16 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) > > kvm_sigset_deactivate(vcpu); > > + /* > + * In the unlikely event that we are returning to userspace > + * with pending exceptions or PC adjustment, commit these > + * adjustments in order to give userspace a consistent view of > + * the vcpu state. > + */ > + if (unlikely(vcpu->arch.flags & (KVM_ARM64_PENDING_EXCEPTION | > + KVM_ARM64_INCREMENT_PC))) > + kvm_call_hyp(__kvm_adjust_pc, vcpu); > + > vcpu_put(vcpu); > return ret; > } > diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c > index 0812a496725f..11541b94b328 100644 > --- a/arch/arm64/kvm/hyp/exception.c > +++ b/arch/arm64/kvm/hyp/exception.c > @@ -331,8 +331,8 @@ static void kvm_inject_exception(struct kvm_vcpu *vcpu) > } > > /* > - * Adjust the guest PC on entry, depending on flags provided by EL1 > - * for the purpose of emulation (MMIO, sysreg) or exception injection. > + * Adjust the guest PC (and potentially exception state) depending on > + * flags provided by the emulation code. > */ > void __kvm_adjust_pc(struct kvm_vcpu *vcpu) > { > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > index f36420a80474..1632f001f4ed 100644 > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > @@ -28,6 +28,13 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt) > cpu_reg(host_ctxt, 1) = __kvm_vcpu_run(kern_hyp_va(vcpu)); > } > > +static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt) > +{ > + DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1); > + > + __kvm_adjust_pc(kern_hyp_va(vcpu)); > +} > + > static void handle___kvm_flush_vm_context(struct kvm_cpu_context *host_ctxt) > { > __kvm_flush_vm_context(); > @@ -170,6 +177,7 @@ typedef void (*hcall_t)(struct kvm_cpu_context *); > > static const hcall_t host_hcall[] = { > HANDLE_FUNC(__kvm_vcpu_run), > + HANDLE_FUNC(__kvm_adjust_pc), > HANDLE_FUNC(__kvm_flush_vm_context), > HANDLE_FUNC(__kvm_tlb_flush_vmid_ipa), > HANDLE_FUNC(__kvm_tlb_flush_vmid), I'm guessing that the comment mentioned in the cover letter should be in this patch, right? Or is the changelog from the cover letter stale? Regardless, I trust your judgement regarding the comment and the patch looks correct: Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> Thanks, Alex _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] KVM: arm64: Commit pending PC adjustemnts before returning to userspace 2021-05-14 14:08 ` Alexandru Elisei (?) @ 2021-05-14 14:21 ` Marc Zyngier -1 siblings, 0 replies; 21+ messages in thread From: Marc Zyngier @ 2021-05-14 14:21 UTC (permalink / raw) To: Alexandru Elisei; +Cc: kvm, stable, kernel-team, kvmarm, linux-arm-kernel On 2021-05-14 15:08, Alexandru Elisei wrote: > Hi Marc, > > On 5/14/21 11:40 AM, Marc Zyngier wrote: >> KVM currently updates PC (and the corresponding exception state) >> using a two phase approach: first by setting a set of flags, >> then by converting these flags into a state update when the vcpu >> is about to enter the guest. >> >> However, this creates a disconnect with userspace if the vcpu thread >> returns there with any exception/PC flag set. In this case, the >> exposed >> context is wrong, as userpsace doesn't have access to these flags > > Nitpick: s/userpsace/userspace Ah, forgot that one. > >> (they aren't architectural). It also means that these flags are >> preserved across a reset, which isn't expected. >> >> To solve this problem, force an explicit synchronisation of the >> exception state on vcpu exit to userspace. As an optimisation >> for nVHE systems, only perform this when there is something pending. >> >> Reported-by: Zenghui Yu <yuzenghui@huawei.com> >> Signed-off-by: Marc Zyngier <maz@kernel.org> >> Cc: stable@vger.kernel.org # 5.11 >> --- >> arch/arm64/include/asm/kvm_asm.h | 1 + >> arch/arm64/kvm/arm.c | 10 ++++++++++ >> arch/arm64/kvm/hyp/exception.c | 4 ++-- >> arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++ >> 4 files changed, 21 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/include/asm/kvm_asm.h >> b/arch/arm64/include/asm/kvm_asm.h >> index d5b11037401d..5e9b33cbac51 100644 >> --- a/arch/arm64/include/asm/kvm_asm.h >> +++ b/arch/arm64/include/asm/kvm_asm.h >> @@ -63,6 +63,7 @@ >> #define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18 >> #define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19 >> #define __KVM_HOST_SMCCC_FUNC___pkvm_mark_hyp 20 >> +#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 21 >> >> #ifndef __ASSEMBLY__ >> >> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c >> index 1cb39c0803a4..c4fe2b71f429 100644 >> --- a/arch/arm64/kvm/arm.c >> +++ b/arch/arm64/kvm/arm.c >> @@ -897,6 +897,16 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu >> *vcpu) >> >> kvm_sigset_deactivate(vcpu); >> >> + /* >> + * In the unlikely event that we are returning to userspace >> + * with pending exceptions or PC adjustment, commit these >> + * adjustments in order to give userspace a consistent view of >> + * the vcpu state. I didn't managed to commit this: diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index c4fe2b71f429..1126eae27400 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -901,7 +901,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) * In the unlikely event that we are returning to userspace * with pending exceptions or PC adjustment, commit these * adjustments in order to give userspace a consistent view of - * the vcpu state. + * the vcpu state. Note that this relies on __kvm_adjust_pc() + * being preempt-safe on VHE. */ if (unlikely(vcpu->arch.flags & (KVM_ARM64_PENDING_EXCEPTION | KVM_ARM64_INCREMENT_PC))) I promise I'll fold that in! >> + */ >> + if (unlikely(vcpu->arch.flags & (KVM_ARM64_PENDING_EXCEPTION | >> + KVM_ARM64_INCREMENT_PC))) >> + kvm_call_hyp(__kvm_adjust_pc, vcpu); >> + >> vcpu_put(vcpu); >> return ret; >> } >> diff --git a/arch/arm64/kvm/hyp/exception.c >> b/arch/arm64/kvm/hyp/exception.c >> index 0812a496725f..11541b94b328 100644 >> --- a/arch/arm64/kvm/hyp/exception.c >> +++ b/arch/arm64/kvm/hyp/exception.c >> @@ -331,8 +331,8 @@ static void kvm_inject_exception(struct kvm_vcpu >> *vcpu) >> } >> >> /* >> - * Adjust the guest PC on entry, depending on flags provided by EL1 >> - * for the purpose of emulation (MMIO, sysreg) or exception >> injection. >> + * Adjust the guest PC (and potentially exception state) depending on >> + * flags provided by the emulation code. >> */ >> void __kvm_adjust_pc(struct kvm_vcpu *vcpu) >> { >> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c >> b/arch/arm64/kvm/hyp/nvhe/hyp-main.c >> index f36420a80474..1632f001f4ed 100644 >> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c >> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c >> @@ -28,6 +28,13 @@ static void handle___kvm_vcpu_run(struct >> kvm_cpu_context *host_ctxt) >> cpu_reg(host_ctxt, 1) = __kvm_vcpu_run(kern_hyp_va(vcpu)); >> } >> >> +static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt) >> +{ >> + DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1); >> + >> + __kvm_adjust_pc(kern_hyp_va(vcpu)); >> +} >> + >> static void handle___kvm_flush_vm_context(struct kvm_cpu_context >> *host_ctxt) >> { >> __kvm_flush_vm_context(); >> @@ -170,6 +177,7 @@ typedef void (*hcall_t)(struct kvm_cpu_context *); >> >> static const hcall_t host_hcall[] = { >> HANDLE_FUNC(__kvm_vcpu_run), >> + HANDLE_FUNC(__kvm_adjust_pc), >> HANDLE_FUNC(__kvm_flush_vm_context), >> HANDLE_FUNC(__kvm_tlb_flush_vmid_ipa), >> HANDLE_FUNC(__kvm_tlb_flush_vmid), > > I'm guessing that the comment mentioned in the cover letter should be > in this > patch, right? Or is the changelog from the cover letter stale? > > Regardless, I trust your judgement regarding the comment and the patch > looks correct: > > Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> Thanks, M. -- Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] KVM: arm64: Commit pending PC adjustemnts before returning to userspace @ 2021-05-14 14:21 ` Marc Zyngier 0 siblings, 0 replies; 21+ messages in thread From: Marc Zyngier @ 2021-05-14 14:21 UTC (permalink / raw) To: Alexandru Elisei Cc: kvm, kvmarm, linux-arm-kernel, Zenghui Yu, Fuad Tabba, James Morse, Suzuki K Poulose, kernel-team, stable On 2021-05-14 15:08, Alexandru Elisei wrote: > Hi Marc, > > On 5/14/21 11:40 AM, Marc Zyngier wrote: >> KVM currently updates PC (and the corresponding exception state) >> using a two phase approach: first by setting a set of flags, >> then by converting these flags into a state update when the vcpu >> is about to enter the guest. >> >> However, this creates a disconnect with userspace if the vcpu thread >> returns there with any exception/PC flag set. In this case, the >> exposed >> context is wrong, as userpsace doesn't have access to these flags > > Nitpick: s/userpsace/userspace Ah, forgot that one. > >> (they aren't architectural). It also means that these flags are >> preserved across a reset, which isn't expected. >> >> To solve this problem, force an explicit synchronisation of the >> exception state on vcpu exit to userspace. As an optimisation >> for nVHE systems, only perform this when there is something pending. >> >> Reported-by: Zenghui Yu <yuzenghui@huawei.com> >> Signed-off-by: Marc Zyngier <maz@kernel.org> >> Cc: stable@vger.kernel.org # 5.11 >> --- >> arch/arm64/include/asm/kvm_asm.h | 1 + >> arch/arm64/kvm/arm.c | 10 ++++++++++ >> arch/arm64/kvm/hyp/exception.c | 4 ++-- >> arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++ >> 4 files changed, 21 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/include/asm/kvm_asm.h >> b/arch/arm64/include/asm/kvm_asm.h >> index d5b11037401d..5e9b33cbac51 100644 >> --- a/arch/arm64/include/asm/kvm_asm.h >> +++ b/arch/arm64/include/asm/kvm_asm.h >> @@ -63,6 +63,7 @@ >> #define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18 >> #define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19 >> #define __KVM_HOST_SMCCC_FUNC___pkvm_mark_hyp 20 >> +#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 21 >> >> #ifndef __ASSEMBLY__ >> >> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c >> index 1cb39c0803a4..c4fe2b71f429 100644 >> --- a/arch/arm64/kvm/arm.c >> +++ b/arch/arm64/kvm/arm.c >> @@ -897,6 +897,16 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu >> *vcpu) >> >> kvm_sigset_deactivate(vcpu); >> >> + /* >> + * In the unlikely event that we are returning to userspace >> + * with pending exceptions or PC adjustment, commit these >> + * adjustments in order to give userspace a consistent view of >> + * the vcpu state. I didn't managed to commit this: diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index c4fe2b71f429..1126eae27400 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -901,7 +901,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) * In the unlikely event that we are returning to userspace * with pending exceptions or PC adjustment, commit these * adjustments in order to give userspace a consistent view of - * the vcpu state. + * the vcpu state. Note that this relies on __kvm_adjust_pc() + * being preempt-safe on VHE. */ if (unlikely(vcpu->arch.flags & (KVM_ARM64_PENDING_EXCEPTION | KVM_ARM64_INCREMENT_PC))) I promise I'll fold that in! >> + */ >> + if (unlikely(vcpu->arch.flags & (KVM_ARM64_PENDING_EXCEPTION | >> + KVM_ARM64_INCREMENT_PC))) >> + kvm_call_hyp(__kvm_adjust_pc, vcpu); >> + >> vcpu_put(vcpu); >> return ret; >> } >> diff --git a/arch/arm64/kvm/hyp/exception.c >> b/arch/arm64/kvm/hyp/exception.c >> index 0812a496725f..11541b94b328 100644 >> --- a/arch/arm64/kvm/hyp/exception.c >> +++ b/arch/arm64/kvm/hyp/exception.c >> @@ -331,8 +331,8 @@ static void kvm_inject_exception(struct kvm_vcpu >> *vcpu) >> } >> >> /* >> - * Adjust the guest PC on entry, depending on flags provided by EL1 >> - * for the purpose of emulation (MMIO, sysreg) or exception >> injection. >> + * Adjust the guest PC (and potentially exception state) depending on >> + * flags provided by the emulation code. >> */ >> void __kvm_adjust_pc(struct kvm_vcpu *vcpu) >> { >> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c >> b/arch/arm64/kvm/hyp/nvhe/hyp-main.c >> index f36420a80474..1632f001f4ed 100644 >> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c >> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c >> @@ -28,6 +28,13 @@ static void handle___kvm_vcpu_run(struct >> kvm_cpu_context *host_ctxt) >> cpu_reg(host_ctxt, 1) = __kvm_vcpu_run(kern_hyp_va(vcpu)); >> } >> >> +static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt) >> +{ >> + DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1); >> + >> + __kvm_adjust_pc(kern_hyp_va(vcpu)); >> +} >> + >> static void handle___kvm_flush_vm_context(struct kvm_cpu_context >> *host_ctxt) >> { >> __kvm_flush_vm_context(); >> @@ -170,6 +177,7 @@ typedef void (*hcall_t)(struct kvm_cpu_context *); >> >> static const hcall_t host_hcall[] = { >> HANDLE_FUNC(__kvm_vcpu_run), >> + HANDLE_FUNC(__kvm_adjust_pc), >> HANDLE_FUNC(__kvm_flush_vm_context), >> HANDLE_FUNC(__kvm_tlb_flush_vmid_ipa), >> HANDLE_FUNC(__kvm_tlb_flush_vmid), > > I'm guessing that the comment mentioned in the cover letter should be > in this > patch, right? Or is the changelog from the cover letter stale? > > Regardless, I trust your judgement regarding the comment and the patch > looks correct: > > Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] KVM: arm64: Commit pending PC adjustemnts before returning to userspace @ 2021-05-14 14:21 ` Marc Zyngier 0 siblings, 0 replies; 21+ messages in thread From: Marc Zyngier @ 2021-05-14 14:21 UTC (permalink / raw) To: Alexandru Elisei Cc: kvm, kvmarm, linux-arm-kernel, Zenghui Yu, Fuad Tabba, James Morse, Suzuki K Poulose, kernel-team, stable On 2021-05-14 15:08, Alexandru Elisei wrote: > Hi Marc, > > On 5/14/21 11:40 AM, Marc Zyngier wrote: >> KVM currently updates PC (and the corresponding exception state) >> using a two phase approach: first by setting a set of flags, >> then by converting these flags into a state update when the vcpu >> is about to enter the guest. >> >> However, this creates a disconnect with userspace if the vcpu thread >> returns there with any exception/PC flag set. In this case, the >> exposed >> context is wrong, as userpsace doesn't have access to these flags > > Nitpick: s/userpsace/userspace Ah, forgot that one. > >> (they aren't architectural). It also means that these flags are >> preserved across a reset, which isn't expected. >> >> To solve this problem, force an explicit synchronisation of the >> exception state on vcpu exit to userspace. As an optimisation >> for nVHE systems, only perform this when there is something pending. >> >> Reported-by: Zenghui Yu <yuzenghui@huawei.com> >> Signed-off-by: Marc Zyngier <maz@kernel.org> >> Cc: stable@vger.kernel.org # 5.11 >> --- >> arch/arm64/include/asm/kvm_asm.h | 1 + >> arch/arm64/kvm/arm.c | 10 ++++++++++ >> arch/arm64/kvm/hyp/exception.c | 4 ++-- >> arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++ >> 4 files changed, 21 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/include/asm/kvm_asm.h >> b/arch/arm64/include/asm/kvm_asm.h >> index d5b11037401d..5e9b33cbac51 100644 >> --- a/arch/arm64/include/asm/kvm_asm.h >> +++ b/arch/arm64/include/asm/kvm_asm.h >> @@ -63,6 +63,7 @@ >> #define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18 >> #define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19 >> #define __KVM_HOST_SMCCC_FUNC___pkvm_mark_hyp 20 >> +#define __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc 21 >> >> #ifndef __ASSEMBLY__ >> >> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c >> index 1cb39c0803a4..c4fe2b71f429 100644 >> --- a/arch/arm64/kvm/arm.c >> +++ b/arch/arm64/kvm/arm.c >> @@ -897,6 +897,16 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu >> *vcpu) >> >> kvm_sigset_deactivate(vcpu); >> >> + /* >> + * In the unlikely event that we are returning to userspace >> + * with pending exceptions or PC adjustment, commit these >> + * adjustments in order to give userspace a consistent view of >> + * the vcpu state. I didn't managed to commit this: diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index c4fe2b71f429..1126eae27400 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -901,7 +901,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) * In the unlikely event that we are returning to userspace * with pending exceptions or PC adjustment, commit these * adjustments in order to give userspace a consistent view of - * the vcpu state. + * the vcpu state. Note that this relies on __kvm_adjust_pc() + * being preempt-safe on VHE. */ if (unlikely(vcpu->arch.flags & (KVM_ARM64_PENDING_EXCEPTION | KVM_ARM64_INCREMENT_PC))) I promise I'll fold that in! >> + */ >> + if (unlikely(vcpu->arch.flags & (KVM_ARM64_PENDING_EXCEPTION | >> + KVM_ARM64_INCREMENT_PC))) >> + kvm_call_hyp(__kvm_adjust_pc, vcpu); >> + >> vcpu_put(vcpu); >> return ret; >> } >> diff --git a/arch/arm64/kvm/hyp/exception.c >> b/arch/arm64/kvm/hyp/exception.c >> index 0812a496725f..11541b94b328 100644 >> --- a/arch/arm64/kvm/hyp/exception.c >> +++ b/arch/arm64/kvm/hyp/exception.c >> @@ -331,8 +331,8 @@ static void kvm_inject_exception(struct kvm_vcpu >> *vcpu) >> } >> >> /* >> - * Adjust the guest PC on entry, depending on flags provided by EL1 >> - * for the purpose of emulation (MMIO, sysreg) or exception >> injection. >> + * Adjust the guest PC (and potentially exception state) depending on >> + * flags provided by the emulation code. >> */ >> void __kvm_adjust_pc(struct kvm_vcpu *vcpu) >> { >> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c >> b/arch/arm64/kvm/hyp/nvhe/hyp-main.c >> index f36420a80474..1632f001f4ed 100644 >> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c >> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c >> @@ -28,6 +28,13 @@ static void handle___kvm_vcpu_run(struct >> kvm_cpu_context *host_ctxt) >> cpu_reg(host_ctxt, 1) = __kvm_vcpu_run(kern_hyp_va(vcpu)); >> } >> >> +static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt) >> +{ >> + DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1); >> + >> + __kvm_adjust_pc(kern_hyp_va(vcpu)); >> +} >> + >> static void handle___kvm_flush_vm_context(struct kvm_cpu_context >> *host_ctxt) >> { >> __kvm_flush_vm_context(); >> @@ -170,6 +177,7 @@ typedef void (*hcall_t)(struct kvm_cpu_context *); >> >> static const hcall_t host_hcall[] = { >> HANDLE_FUNC(__kvm_vcpu_run), >> + HANDLE_FUNC(__kvm_adjust_pc), >> HANDLE_FUNC(__kvm_flush_vm_context), >> HANDLE_FUNC(__kvm_tlb_flush_vmid_ipa), >> HANDLE_FUNC(__kvm_tlb_flush_vmid), > > I'm guessing that the comment mentioned in the cover letter should be > in this > patch, right? Or is the changelog from the cover letter stale? > > Regardless, I trust your judgement regarding the comment and the patch > looks correct: > > Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> Thanks, M. -- Jazz is not dead. It just smells funny... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 0/2] KVM: arm64: Fixup PC updates on exit to userspace 2021-05-14 10:40 ` Marc Zyngier (?) @ 2021-05-14 12:32 ` Zenghui Yu -1 siblings, 0 replies; 21+ messages in thread From: Zenghui Yu @ 2021-05-14 12:32 UTC (permalink / raw) To: Marc Zyngier; +Cc: kvm, kernel-team, kvmarm, linux-arm-kernel Hi Marc, On 2021/5/14 18:40, Marc Zyngier wrote: > We recently moved anything related to PC updates into the guest entry > code to help with the protected KVM effort. However, I missed a > critical point: userspace needs to be able to observe state changes > when the vcpu exits. Otherwise, live migration is a bit broken and > vcpu reset can fail (as reported by Zenghui). Not good. > > These two patches aim at fixing the above, and carry a Cc stable. > > * From v1: > - Sanitized flag checking > - Added comment about relying on __kvm_adjust_pc() being preempt-safe I had a try but failed to find the added comment ;-). Regardless, Reviewed-by: Zenghui Yu <yuzenghui@huawei.com> Tested-by: Zenghui Yu <yuzenghui@huawei.com> _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 0/2] KVM: arm64: Fixup PC updates on exit to userspace @ 2021-05-14 12:32 ` Zenghui Yu 0 siblings, 0 replies; 21+ messages in thread From: Zenghui Yu @ 2021-05-14 12:32 UTC (permalink / raw) To: Marc Zyngier Cc: kvm, kvmarm, linux-arm-kernel, Fuad Tabba, James Morse, Suzuki K Poulose, Alexandru Elisei, kernel-team Hi Marc, On 2021/5/14 18:40, Marc Zyngier wrote: > We recently moved anything related to PC updates into the guest entry > code to help with the protected KVM effort. However, I missed a > critical point: userspace needs to be able to observe state changes > when the vcpu exits. Otherwise, live migration is a bit broken and > vcpu reset can fail (as reported by Zenghui). Not good. > > These two patches aim at fixing the above, and carry a Cc stable. > > * From v1: > - Sanitized flag checking > - Added comment about relying on __kvm_adjust_pc() being preempt-safe I had a try but failed to find the added comment ;-). Regardless, Reviewed-by: Zenghui Yu <yuzenghui@huawei.com> Tested-by: Zenghui Yu <yuzenghui@huawei.com> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 0/2] KVM: arm64: Fixup PC updates on exit to userspace @ 2021-05-14 12:32 ` Zenghui Yu 0 siblings, 0 replies; 21+ messages in thread From: Zenghui Yu @ 2021-05-14 12:32 UTC (permalink / raw) To: Marc Zyngier Cc: kvm, kvmarm, linux-arm-kernel, Fuad Tabba, James Morse, Suzuki K Poulose, Alexandru Elisei, kernel-team Hi Marc, On 2021/5/14 18:40, Marc Zyngier wrote: > We recently moved anything related to PC updates into the guest entry > code to help with the protected KVM effort. However, I missed a > critical point: userspace needs to be able to observe state changes > when the vcpu exits. Otherwise, live migration is a bit broken and > vcpu reset can fail (as reported by Zenghui). Not good. > > These two patches aim at fixing the above, and carry a Cc stable. > > * From v1: > - Sanitized flag checking > - Added comment about relying on __kvm_adjust_pc() being preempt-safe I had a try but failed to find the added comment ;-). Regardless, Reviewed-by: Zenghui Yu <yuzenghui@huawei.com> Tested-by: Zenghui Yu <yuzenghui@huawei.com> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2021-05-14 14:23 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-05-14 10:40 [PATCH v2 0/2] KVM: arm64: Fixup PC updates on exit to userspace Marc Zyngier 2021-05-14 10:40 ` Marc Zyngier 2021-05-14 10:40 ` Marc Zyngier 2021-05-14 10:40 ` [PATCH v2 1/2] KVM: arm64: Move __adjust_pc out of line Marc Zyngier 2021-05-14 10:40 ` Marc Zyngier 2021-05-14 10:40 ` Marc Zyngier 2021-05-14 14:07 ` Alexandru Elisei 2021-05-14 14:07 ` Alexandru Elisei 2021-05-14 14:07 ` Alexandru Elisei 2021-05-14 10:40 ` [PATCH v2 2/2] KVM: arm64: Commit pending PC adjustemnts before returning to userspace Marc Zyngier 2021-05-14 10:40 ` Marc Zyngier 2021-05-14 10:40 ` Marc Zyngier 2021-05-14 14:08 ` Alexandru Elisei 2021-05-14 14:08 ` Alexandru Elisei 2021-05-14 14:08 ` Alexandru Elisei 2021-05-14 14:21 ` Marc Zyngier 2021-05-14 14:21 ` Marc Zyngier 2021-05-14 14:21 ` Marc Zyngier 2021-05-14 12:32 ` [PATCH v2 0/2] KVM: arm64: Fixup PC updates on exit " Zenghui Yu 2021-05-14 12:32 ` Zenghui Yu 2021-05-14 12:32 ` Zenghui Yu
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.