* [PATCH v3 1/3] KVM: LAPIC: Fix lapic timer mode transition
2017-10-05 10:53 [PATCH v3 0/3] KVM: LAPIC: Rework lapic timer to behave more like real-hardware Wanpeng Li
@ 2017-10-05 10:53 ` Wanpeng Li
2017-10-05 10:53 ` [PATCH v3 2/3] KVM: LAPIC: Keep timer running when switching between one-shot and periodic mode Wanpeng Li
2017-10-05 10:53 ` [PATCH v3 3/3] KVM: LAPIC: Apply change to TDCR right away to the timer Wanpeng Li
2 siblings, 0 replies; 6+ messages in thread
From: Wanpeng Li @ 2017-10-05 10:53 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li
From: Wanpeng Li <wanpeng.li@hotmail.com>
SDM 10.5.4.1 TSC-Deadline Mode mentioned that "Transitioning between TSC-Deadline
mode and other timer modes also disarms the timer". So the APIC Timer Initial Count
Register for one-shot/periodic mode should be reset. This patch do it.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
arch/x86/include/asm/apicdef.h | 1 +
arch/x86/kvm/lapic.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/arch/x86/include/asm/apicdef.h b/arch/x86/include/asm/apicdef.h
index c46bb99..7fde8e9 100644
--- a/arch/x86/include/asm/apicdef.h
+++ b/arch/x86/include/asm/apicdef.h
@@ -100,6 +100,7 @@
#define APIC_TIMER_BASE_CLKIN 0x0
#define APIC_TIMER_BASE_TMBASE 0x1
#define APIC_TIMER_BASE_DIV 0x2
+#define APIC_LVT_TIMER_MASK (3 << 17)
#define APIC_LVT_TIMER_ONESHOT (0 << 17)
#define APIC_LVT_TIMER_PERIODIC (1 << 17)
#define APIC_LVT_TIMER_TSCDEADLINE (2 << 17)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 69c5612..6723e2c 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1307,6 +1307,9 @@ static void apic_update_lvtt(struct kvm_lapic *apic)
apic->lapic_timer.timer_mode_mask;
if (apic->lapic_timer.timer_mode != timer_mode) {
+ if (apic_lvtt_tscdeadline(apic) != (timer_mode ==
+ APIC_LVT_TIMER_TSCDEADLINE))
+ kvm_lapic_set_reg(apic, APIC_TMICT, 0);
apic->lapic_timer.timer_mode = timer_mode;
hrtimer_cancel(&apic->lapic_timer.timer);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 2/3] KVM: LAPIC: Keep timer running when switching between one-shot and periodic mode
2017-10-05 10:53 [PATCH v3 0/3] KVM: LAPIC: Rework lapic timer to behave more like real-hardware Wanpeng Li
2017-10-05 10:53 ` [PATCH v3 1/3] KVM: LAPIC: Fix lapic timer mode transition Wanpeng Li
@ 2017-10-05 10:53 ` Wanpeng Li
2017-10-05 12:40 ` Radim Krčmář
2017-10-05 10:53 ` [PATCH v3 3/3] KVM: LAPIC: Apply change to TDCR right away to the timer Wanpeng Li
2 siblings, 1 reply; 6+ messages in thread
From: Wanpeng Li @ 2017-10-05 10:53 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li
From: Wanpeng Li <wanpeng.li@hotmail.com>
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@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
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
index 6723e2c..df31048 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1301,18 +1301,21 @@ 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;
if (apic->lapic_timer.timer_mode != timer_mode) {
if (apic_lvtt_tscdeadline(apic) != (timer_mode ==
- APIC_LVT_TIMER_TSCDEADLINE))
+ APIC_LVT_TIMER_TSCDEADLINE)) {
kvm_lapic_set_reg(apic, APIC_TMICT, 0);
+ hrtimer_cancel(&apic->lapic_timer.timer);
+ }
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)
@@ -1433,11 +1436,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;
@@ -1473,9 +1477,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;
}
@@ -1612,12 +1628,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 +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);
break;
case APIC_TMICT:
@@ -1738,7 +1755,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 +1889,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 +2255,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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3 2/3] KVM: LAPIC: Keep timer running when switching between one-shot and periodic mode
2017-10-05 10:53 ` [PATCH v3 2/3] KVM: LAPIC: Keep timer running when switching between one-shot and periodic mode Wanpeng Li
@ 2017-10-05 12:40 ` Radim Krčmář
2017-10-05 12:54 ` Wanpeng Li
0 siblings, 1 reply; 6+ messages in thread
From: Radim Krčmář @ 2017-10-05 12:40 UTC (permalink / raw)
To: Wanpeng Li; +Cc: linux-kernel, kvm, Paolo Bonzini, Wanpeng Li
2017-10-05 03:53-0700, Wanpeng Li:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
>
> 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@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
> 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().
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/3] KVM: LAPIC: Keep timer running when switching between one-shot and periodic mode
2017-10-05 12:40 ` Radim Krčmář
@ 2017-10-05 12:54 ` Wanpeng Li
0 siblings, 0 replies; 6+ messages in thread
From: Wanpeng Li @ 2017-10-05 12:54 UTC (permalink / raw)
To: Radim Krčmář
Cc: linux-kernel@vger.kernel.org, kvm, Paolo Bonzini, Wanpeng Li
2017-10-05 20:40 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
> 2017-10-05 03:53-0700, Wanpeng Li:
>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>
>> 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@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] KVM: LAPIC: Apply change to TDCR right away to the timer
2017-10-05 10:53 [PATCH v3 0/3] KVM: LAPIC: Rework lapic timer to behave more like real-hardware Wanpeng Li
2017-10-05 10:53 ` [PATCH v3 1/3] KVM: LAPIC: Fix lapic timer mode transition Wanpeng Li
2017-10-05 10:53 ` [PATCH v3 2/3] KVM: LAPIC: Keep timer running when switching between one-shot and periodic mode Wanpeng Li
@ 2017-10-05 10:53 ` Wanpeng Li
2 siblings, 0 replies; 6+ messages in thread
From: Wanpeng Li @ 2017-10-05 10:53 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li
From: Wanpeng Li <wanpeng.li@hotmail.com>
The description in the Intel SDM of how the divide configuration
register is used: "The APIC timer frequency will be the processor's bus
clock or core crystal clock frequency divided by the value specified in
the divide configuration register."
Observation of baremetal shown that when the TDCR is change, the TMCCT
does not change or make a big jump in value, but the rate at which it
count down change.
The patch update the emulation to APIC timer to so that a change to the
divide configuration would be reflected in the value of the counter and
when the next interrupt is triggered.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
arch/x86/kvm/lapic.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index df31048..5a3d6ba 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1436,7 +1436,7 @@ static void start_sw_period(struct kvm_lapic *apic)
HRTIMER_MODE_ABS_PINNED);
}
-static bool set_target_expiration(struct kvm_lapic *apic, bool timer_update)
+static bool set_target_expiration(struct kvm_lapic *apic, bool timer_update, uint32_t old_divisor)
{
ktime_t now, remaining;
u64 tscl = rdtsc(), delta;
@@ -1444,7 +1444,7 @@ static bool set_target_expiration(struct kvm_lapic *apic, bool timer_update)
/* 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;
+ * APIC_BUS_CYCLE_NS * old_divisor;
if (!apic->lapic_timer.period)
return false;
@@ -1489,6 +1489,12 @@ static bool set_target_expiration(struct kvm_lapic *apic, bool timer_update)
if (!delta)
return false;
+ if (apic->divide_count != old_divisor) {
+ apic->lapic_timer.period = (u64)kvm_lapic_get_reg(apic, APIC_TMICT)
+ * APIC_BUS_CYCLE_NS * apic->divide_count;
+ delta = delta * apic->divide_count / old_divisor;
+ }
+
apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
nsec_to_cycles(apic->vcpu, delta);
apic->lapic_timer.target_expiration = ktime_add_ns(now, delta);
@@ -1628,12 +1634,13 @@ void kvm_lapic_restart_hv_timer(struct kvm_vcpu *vcpu)
restart_apic_timer(apic);
}
-static void start_apic_timer(struct kvm_lapic *apic, bool timer_update)
+static void start_apic_timer(struct kvm_lapic *apic, bool timer_update,
+ uint32_t old_divisor)
{
atomic_set(&apic->lapic_timer.pending, 0);
if ((apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
- && !set_target_expiration(apic, timer_update))
+ && !set_target_expiration(apic, timer_update, old_divisor))
return;
restart_apic_timer(apic);
@@ -1746,7 +1753,7 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
val &= (apic_lvt_mask[0] | apic->lapic_timer.timer_mode_mask);
kvm_lapic_set_reg(apic, APIC_LVTT, val);
if (apic_update_lvtt(apic) && !apic_lvtt_tscdeadline(apic))
- start_apic_timer(apic, true);
+ start_apic_timer(apic, true, apic->divide_count);
break;
case APIC_TMICT:
@@ -1755,16 +1762,20 @@ 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, false);
+ start_apic_timer(apic, false, apic->divide_count);
break;
- case APIC_TDCR:
+ case APIC_TDCR: {
+ uint32_t current_divisor = apic->divide_count;
+
if (val & 4)
apic_debug("KVM_WRITE:TDCR %x\n", val);
kvm_lapic_set_reg(apic, APIC_TDCR, val);
update_divide_count(apic);
+ hrtimer_cancel(&apic->lapic_timer.timer);
+ start_apic_timer(apic, true, current_divisor);
break;
-
+ }
case APIC_ESR:
if (apic_x2apic_mode(apic) && val != 0) {
apic_debug("KVM_WRITE:ESR not zero %x\n", val);
@@ -1889,7 +1900,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, false);
+ start_apic_timer(apic, false, apic->divide_count);
}
void kvm_lapic_set_tpr(struct kvm_vcpu *vcpu, unsigned long cr8)
@@ -2255,7 +2266,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, false);
+ start_apic_timer(apic, false, apic->divide_count);
apic->irr_pending = true;
apic->isr_count = vcpu->arch.apicv_active ?
1 : count_vectors(apic->regs + APIC_ISR);
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread