On Thu, Oct 12, 2017 at 12:41:20PM +0200, 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.
>
> Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
> ---
> arch/arm64/kvm/hyp/switch.c | 2 --
> virt/kvm/arm/hyp/timer-sr.c | 10 ++--------
> 2 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 2cedf12..b98b73b 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -336,7 +336,6 @@ int kvm_vcpu_run(struct kvm_vcpu *vcpu)
> __activate_vm(vcpu);
>
> __vgic_restore_state(vcpu);
> - __timer_enable_traps(vcpu);
>
> /*
> * We must restore the 32-bit state before the sysregs, thanks
> @@ -356,7 +355,6 @@ int kvm_vcpu_run(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 f398616..82c217e 100644
> --- a/virt/kvm/arm/hyp/timer-sr.c
> +++ b/virt/kvm/arm/hyp/timer-sr.c
> @@ -53,16 +53,10 @@ void __hyp_text disable_el1_phys_timer_access(void)
>
> void __hyp_text __timer_disable_traps(struct kvm_vcpu *vcpu)
> {
> - /*
> - * We don't need to do this for VHE since the host kernel runs in EL2
> - * with HCR_EL2.TGE ==1, which makes those bits have no impact.
> - */
I was about to suggest that we should move this comment, instead of remove
it, but it seems misleading anyway. We do call
enable/disable_el1_phys_timer_access on VHE, but at VCPU load/put time
instead of VM enter/exit time. So I guess removing it is best.
> - if (!has_vhe())
> - enable_el1_phys_timer_access();
> + enable_el1_phys_timer_access();
> }
>
> void __hyp_text __timer_enable_traps(struct kvm_vcpu *vcpu)
> {
> - if (!has_vhe())
> - disable_el1_phys_timer_access();
> + disable_el1_phys_timer_access();
> }
> --
> 2.9.0
>
Should we just call enable/disable_el1_phys_timer_access directly from
__kvm_vcpu_run for non-VHE and drop this extra function level?
Thanks,
drew
