On Fri, Feb 09, 2018 at 05:53:43PM +0000, Julien Grall wrote: > Hi Christoffer, > > On 01/12/2018 12:07 PM, Christoffer Dall wrote: > >The VHE switch function calls __timer_enable_traps and > >__timer_disable_traps which don't do anything on VHE systems. > >Therefore, simply remove these calls from the VHE switch function and > >make the functions non-conditional as they are now only called from the > >non-VHE switch path. > > > >Acked-by: Marc Zyngier <marc.zyngier@xxxxxxx> > >Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx> > >--- > > arch/arm64/kvm/hyp/switch.c | 2 -- > > virt/kvm/arm/hyp/timer-sr.c | 44 ++++++++++++++++++++++---------------------- > > 2 files changed, 22 insertions(+), 24 deletions(-) > > > >diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c > >index 9aadef6966bf..6175fcb33ed2 100644 > >--- a/arch/arm64/kvm/hyp/switch.c > >+++ b/arch/arm64/kvm/hyp/switch.c > >@@ -354,7 +354,6 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu) > > __activate_vm(vcpu->kvm); > > __vgic_restore_state(vcpu); > >- __timer_enable_traps(vcpu); > > /* > > * We must restore the 32-bit state before the sysregs, thanks > >@@ -373,7 +372,6 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu) > > __sysreg_save_guest_state(guest_ctxt); > > __sysreg32_save_state(vcpu); > >- __timer_disable_traps(vcpu); > > __vgic_save_state(vcpu); > > __deactivate_traps(vcpu); > >diff --git a/virt/kvm/arm/hyp/timer-sr.c b/virt/kvm/arm/hyp/timer-sr.c > >index f24404b3c8df..77754a62eb0c 100644 > >--- a/virt/kvm/arm/hyp/timer-sr.c > >+++ b/virt/kvm/arm/hyp/timer-sr.c > >@@ -27,34 +27,34 @@ void __hyp_text __kvm_timer_set_cntvoff(u32 cntvoff_low, u32 cntvoff_high) > > write_sysreg(cntvoff, cntvoff_el2); > > } > >+/* > >+ * Should only be called on non-VHE systems. > >+ * VHE systems use EL2 timers and configure EL1 timers in kvm_timer_init_vhe(). > >+ */ > > void __hyp_text __timer_disable_traps(struct kvm_vcpu *vcpu) > > Would it be worth to suffix the function with nvhe? So it would be clear > that it should not be called for VHE system? > Actually, I decided against this, because it's also called from the 32-bit code and it looks a little strange there, and it's not like we have an equivalent _vhe version. Thanks, -Christoffer
