2017-10-05 20:40 GMT+08:00 Radim Krčmář <rkrcmar@xxxxxxxxxx>: > 2017-10-05 03:53-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. >> >> 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 | 45 +++++++++++++++++++++++++++++++-------------- >> 1 file changed, 31 insertions(+), 14 deletions(-) >> >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c >> @@ -1729,7 +1745,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); > > start_apic_timer() is not needed here: when switching from > apic_lvtt_tscdeadline(), we have TMICT = 0, so the timer is disabled. > When switching between one-shot and periodic, the timer is running with > a correct expiration time. > > This will bring us close to bare-metal behavior wrt. races and also > allows us to get rid of apic_update_lvtt() return value and an argument > to start_apic_timer(). I see, what this patch should do is to skip hrtimer_cancel() in apic_update_lvtt() when either from one-shot mode to periodic or vice versa. Regards, Wanpeng Li
