On Sat, Sep 23 2017 at 2:42:03 am BST, Christoffer Dall <cdall@xxxxxxxxxx> wrote:
> When trapping on a guest access to one of the timer registers, we were
> messing with the internals of the timer state from the sysregs handling
> code, and that logic was about to receive more added complexity when
> optimizing the timer handling code.
>
> Therefore, since we already have timer register access functions (to
> access registers from userspace), reuse those for the timer register
> traps from a VM and let the timer code maintain its own consistency.
>
> Signed-off-by: Christoffer Dall <cdall@xxxxxxxxxx>
> ---
> arch/arm64/kvm/sys_regs.c | 41 ++++++++++++++---------------------------
> 1 file changed, 14 insertions(+), 27 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 2e070d3..bb0e41b 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -841,13 +841,16 @@ static bool access_cntp_tval(struct kvm_vcpu *vcpu,
> struct sys_reg_params *p,
> const struct sys_reg_desc *r)
> {
> - struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> u64 now = kvm_phys_timer_read();
> + u64 cval;
>
> - if (p->is_write)
> - ptimer->cnt_cval = p->regval + now;
> - else
> - p->regval = ptimer->cnt_cval - now;
> + if (p->is_write) {
> + kvm_arm_timer_set_reg(vcpu, KVM_REG_ARM_PTIMER_CVAL,
> + p->regval + now);
> + } else {
> + cval = kvm_arm_timer_get_reg(vcpu, KVM_REG_ARM_PTIMER_CVAL);
> + p->regval = cval - now;
> + }
>
> return true;
> }
> @@ -856,24 +859,10 @@ static bool access_cntp_ctl(struct kvm_vcpu *vcpu,
> struct sys_reg_params *p,
> const struct sys_reg_desc *r)
> {
> - struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> -
> - if (p->is_write) {
> - /* ISTATUS bit is read-only */
> - ptimer->cnt_ctl = p->regval & ~ARCH_TIMER_CTRL_IT_STAT;
> - } else {
> - u64 now = kvm_phys_timer_read();
> -
> - p->regval = ptimer->cnt_ctl;
> - /*
> - * Set ISTATUS bit if it's expired.
> - * Note that according to ARMv8 ARM Issue A.k, ISTATUS bit is
> - * UNKNOWN when ENABLE bit is 0, so we chose to set ISTATUS bit
> - * regardless of ENABLE bit for our implementation convenience.
> - */
> - if (ptimer->cnt_cval <= now)
> - p->regval |= ARCH_TIMER_CTRL_IT_STAT;
> - }
> + if (p->is_write)
> + kvm_arm_timer_set_reg(vcpu, KVM_REG_ARM_PTIMER_CTL, p->regval);
> + else
> + p->regval = kvm_arm_timer_get_reg(vcpu, KVM_REG_ARM_PTIMER_CTL);
>
> return true;
> }
> @@ -882,12 +871,10 @@ static bool access_cntp_cval(struct kvm_vcpu *vcpu,
> struct sys_reg_params *p,
> const struct sys_reg_desc *r)
> {
> - struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> -
> if (p->is_write)
> - ptimer->cnt_cval = p->regval;
> + kvm_arm_timer_set_reg(vcpu, KVM_REG_ARM_PTIMER_CVAL, p->regval);
> else
> - p->regval = ptimer->cnt_cval;
> + p->regval = kvm_arm_timer_get_reg(vcpu, KVM_REG_ARM_PTIMER_CVAL);
>
> return true;
> }
Acked-by: Marc Zyngier <marc.zyngier@xxxxxxx>
M.
--
Jazz is not dead, it just smell funny.
