2017-09-28 18:04-0700, Wanpeng Li:
> From: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
>
> If we take TSC-deadline mode timer out of the picture, the Intel SDM
> does not say that the timer is disable when the timer mode is change,
> either from one-shot to periodic or vice versa.
I think it does, please see comment under [v2 1/4].
> After this patch, the timer is no longer disarmed on change of mode, so
> the counter (TMCCT) keeps counting down.
>
> So what does a write to LVTT changes ? On baremetal, the change of mode
> is probably taken into account only when the counter reach 0. When this
> happen, LVTT is use to figure out if the counter should restard counting
> down from TMICT (so periodic mode) or stop counting (if one-shot mode).
>
> This patch is based on observation of the behavior of the APIC timer on
> baremetal as well as check that they does not go against the description
> written in the Intel SDM.
>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: Radim Krčmář <rkrcmar@xxxxxxxxxx>
> Signed-off-by: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
> ---
> arch/x86/kvm/lapic.c | 40 ++++++++++++++++++++++++++++------------
> 1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index a739cbb..946c11b 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1301,7 +1301,7 @@ static void update_divide_count(struct kvm_lapic *apic)
> apic->divide_count);
> }
>
> -static void apic_update_lvtt(struct kvm_lapic *apic)
> +static bool apic_update_lvtt(struct kvm_lapic *apic)
> {
> u32 timer_mode = kvm_lapic_get_reg(apic, APIC_LVTT) &
> apic->lapic_timer.timer_mode_mask;
> @@ -1309,7 +1309,9 @@ static void apic_update_lvtt(struct kvm_lapic *apic)
> if (apic->lapic_timer.timer_mode != timer_mode) {
> apic->lapic_timer.timer_mode = timer_mode;
> hrtimer_cancel(&apic->lapic_timer.timer);
> + return true;
> }
> + return false;
> }
>
> static void apic_timer_expired(struct kvm_lapic *apic)
> @@ -1430,11 +1432,12 @@ static void start_sw_period(struct kvm_lapic *apic)
> HRTIMER_MODE_ABS_PINNED);
> }
>
> -static bool set_target_expiration(struct kvm_lapic *apic)
> +static bool set_target_expiration(struct kvm_lapic *apic, bool timer_update)
> {
> - ktime_t now;
> - u64 tscl = rdtsc();
> + ktime_t now, remaining;
> + u64 tscl = rdtsc(), delta;
>
> + /* Calculate the next time the timer should trigger an interrupt */
> now = ktime_get();
> apic->lapic_timer.period = (u64)kvm_lapic_get_reg(apic, APIC_TMICT)
> * APIC_BUS_CYCLE_NS * apic->divide_count;
> @@ -1470,9 +1473,21 @@ static bool set_target_expiration(struct kvm_lapic *apic)
> ktime_to_ns(ktime_add_ns(now,
> apic->lapic_timer.period)));
>
> + if (!timer_update)
> + delta = apic->lapic_timer.period;
> + else {
> + remaining = ktime_sub(apic->lapic_timer.target_expiration, now);
> + if (ktime_to_ns(remaining) < 0)
> + remaining = 0;
> + delta = mod_64(ktime_to_ns(remaining), apic->lapic_timer.period);
> + }
> +
> + if (!delta)
> + return false;
> +
> apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
> - nsec_to_cycles(apic->vcpu, apic->lapic_timer.period);
> - apic->lapic_timer.target_expiration = ktime_add_ns(now, apic->lapic_timer.period);
> + nsec_to_cycles(apic->vcpu, delta);
> + apic->lapic_timer.target_expiration = ktime_add_ns(now, delta);
>
> return true;
> }
> @@ -1609,12 +1624,12 @@ void kvm_lapic_restart_hv_timer(struct kvm_vcpu *vcpu)
> restart_apic_timer(apic);
> }
>
> -static void start_apic_timer(struct kvm_lapic *apic)
> +static void start_apic_timer(struct kvm_lapic *apic, bool timer_update)
> {
> atomic_set(&apic->lapic_timer.pending, 0);
>
> if ((apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
> - && !set_target_expiration(apic))
> + && !set_target_expiration(apic, timer_update))
> return;
>
> restart_apic_timer(apic);
> @@ -1729,7 +1744,8 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
> val |= APIC_LVT_MASKED;
> val &= (apic_lvt_mask[0] | apic->lapic_timer.timer_mode_mask);
> kvm_lapic_set_reg(apic, APIC_LVTT, val);
> - apic_update_lvtt(apic);
> + if (apic_update_lvtt(apic) && !apic_lvtt_tscdeadline(apic))
> + start_apic_timer(apic, true);
> break;
>
> case APIC_TMICT:
> @@ -1738,7 +1754,7 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>
> hrtimer_cancel(&apic->lapic_timer.timer);
> kvm_lapic_set_reg(apic, APIC_TMICT, val);
> - start_apic_timer(apic);
> + start_apic_timer(apic, false);
> break;
>
> case APIC_TDCR:
> @@ -1872,7 +1888,7 @@ void kvm_set_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu, u64 data)
>
> hrtimer_cancel(&apic->lapic_timer.timer);
> apic->lapic_timer.tscdeadline = data;
> - start_apic_timer(apic);
> + start_apic_timer(apic, false);
> }
>
> void kvm_lapic_set_tpr(struct kvm_vcpu *vcpu, unsigned long cr8)
> @@ -2238,7 +2254,7 @@ int kvm_apic_set_state(struct kvm_vcpu *vcpu, struct kvm_lapic_state *s)
> apic_update_lvtt(apic);
> apic_manage_nmi_watchdog(apic, kvm_lapic_get_reg(apic, APIC_LVT0));
> update_divide_count(apic);
> - start_apic_timer(apic);
> + start_apic_timer(apic, false);
> apic->irr_pending = true;
> apic->isr_count = vcpu->arch.apicv_active ?
> 1 : count_vectors(apic->regs + APIC_ISR);
> --
> 2.7.4
>
