* [PATCH 1/5] KVM: LAPIC: extract start_sw_period() to handle periodic/oneshot mode
2016-10-17 7:44 [PATCH 0/5] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
@ 2016-10-17 7:44 ` Wanpeng Li
2016-10-17 7:44 ` [PATCH 2/5] KVM: LAPIC: guarantee the timer is in tsc-deadline mode Wanpeng Li
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Wanpeng Li @ 2016-10-17 7:44 UTC (permalink / raw)
To: linux-kernel, kvm
Cc: Paolo Bonzini, Radim Krčmář, Yunhong Jiang,
Wanpeng Li
From: Wanpeng Li <wanpeng.li@hotmail.com>
Extract start_sw_period() to handle periodic/oneshot mode, it will be
used by later patch.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Yunhong Jiang <yunhong.jiang@intel.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
arch/x86/kvm/lapic.c | 89 +++++++++++++++++++++++++++-------------------------
1 file changed, 47 insertions(+), 42 deletions(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 23b99f3..dad743e 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1347,6 +1347,50 @@ static void start_sw_tscdeadline(struct kvm_lapic *apic)
local_irq_restore(flags);
}
+static void start_sw_period(struct kvm_lapic *apic)
+{
+ ktime_t now;
+
+ /* lapic timer in oneshot or periodic mode */
+ now = apic->lapic_timer.timer.base->get_time();
+ apic->lapic_timer.period = (u64)kvm_lapic_get_reg(apic, APIC_TMICT)
+ * APIC_BUS_CYCLE_NS * apic->divide_count;
+
+ if (!apic->lapic_timer.period)
+ return;
+ /*
+ * Do not allow the guest to program periodic timers with small
+ * interval, since the hrtimers are not throttled by the host
+ * scheduler.
+ */
+ if (apic_lvtt_period(apic)) {
+ s64 min_period = min_timer_period_us * 1000LL;
+
+ if (apic->lapic_timer.period < min_period) {
+ pr_info_ratelimited(
+ "kvm: vcpu %i: requested %lld ns "
+ "lapic timer period limited to %lld ns\n",
+ apic->vcpu->vcpu_id,
+ apic->lapic_timer.period, min_period);
+ apic->lapic_timer.period = min_period;
+ }
+ }
+
+ hrtimer_start(&apic->lapic_timer.timer,
+ ktime_add_ns(now, apic->lapic_timer.period),
+ HRTIMER_MODE_ABS_PINNED);
+
+ apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
+ PRIx64 ", "
+ "timer initial count 0x%x, period %lldns, "
+ "expire @ 0x%016" PRIx64 ".\n", __func__,
+ APIC_BUS_CYCLE_NS, ktime_to_ns(now),
+ kvm_lapic_get_reg(apic, APIC_TMICT),
+ apic->lapic_timer.period,
+ ktime_to_ns(ktime_add_ns(now,
+ apic->lapic_timer.period)));
+}
+
bool kvm_lapic_hv_timer_in_use(struct kvm_vcpu *vcpu)
{
if (!lapic_in_kernel(vcpu))
@@ -1424,50 +1468,11 @@ EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_sw_timer);
static void start_apic_timer(struct kvm_lapic *apic)
{
- ktime_t now;
-
atomic_set(&apic->lapic_timer.pending, 0);
- if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
- /* lapic timer in oneshot or periodic mode */
- now = apic->lapic_timer.timer.base->get_time();
- apic->lapic_timer.period = (u64)kvm_lapic_get_reg(apic, APIC_TMICT)
- * APIC_BUS_CYCLE_NS * apic->divide_count;
-
- if (!apic->lapic_timer.period)
- return;
- /*
- * Do not allow the guest to program periodic timers with small
- * interval, since the hrtimers are not throttled by the host
- * scheduler.
- */
- if (apic_lvtt_period(apic)) {
- s64 min_period = min_timer_period_us * 1000LL;
-
- if (apic->lapic_timer.period < min_period) {
- pr_info_ratelimited(
- "kvm: vcpu %i: requested %lld ns "
- "lapic timer period limited to %lld ns\n",
- apic->vcpu->vcpu_id,
- apic->lapic_timer.period, min_period);
- apic->lapic_timer.period = min_period;
- }
- }
-
- hrtimer_start(&apic->lapic_timer.timer,
- ktime_add_ns(now, apic->lapic_timer.period),
- HRTIMER_MODE_ABS_PINNED);
-
- apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
- PRIx64 ", "
- "timer initial count 0x%x, period %lldns, "
- "expire @ 0x%016" PRIx64 ".\n", __func__,
- APIC_BUS_CYCLE_NS, ktime_to_ns(now),
- kvm_lapic_get_reg(apic, APIC_TMICT),
- apic->lapic_timer.period,
- ktime_to_ns(ktime_add_ns(now,
- apic->lapic_timer.period)));
- } else if (apic_lvtt_tscdeadline(apic)) {
+ if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
+ start_sw_period(apic);
+ else if (apic_lvtt_tscdeadline(apic)) {
if (!(kvm_x86_ops->set_hv_timer && start_hv_tscdeadline(apic)))
start_sw_tscdeadline(apic);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/5] KVM: LAPIC: guarantee the timer is in tsc-deadline mode
2016-10-17 7:44 [PATCH 0/5] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
2016-10-17 7:44 ` [PATCH 1/5] KVM: LAPIC: extract start_sw_period() to handle periodic/oneshot mode Wanpeng Li
@ 2016-10-17 7:44 ` Wanpeng Li
2016-10-17 7:45 ` [PATCH 3/5] KVM: LAPIC: introduce kvm_get_lapic_target_expiration_tsc() Wanpeng Li
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Wanpeng Li @ 2016-10-17 7:44 UTC (permalink / raw)
To: linux-kernel, kvm
Cc: Paolo Bonzini, Radim Krčmář, Yunhong Jiang,
Wanpeng Li
From: Wanpeng Li <wanpeng.li@hotmail.com>
Check apic_lvtt_tscdeadline() mode directly instead of apic_lvtt_oneshot()
and apic_lvtt_period() to guarantee the timer is in tsc-deadline mode when
rdmsr MSR_IA32_TSCDEADLINE.
Suggsted-by: Radim Krčmář <rkrcmar@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Yunhong Jiang <yunhong.jiang@intel.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
arch/x86/kvm/lapic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index dad743e..dce6c0b 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1711,8 +1711,8 @@ u64 kvm_get_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu)
{
struct kvm_lapic *apic = vcpu->arch.apic;
- if (!lapic_in_kernel(vcpu) || apic_lvtt_oneshot(apic) ||
- apic_lvtt_period(apic))
+ if (!lapic_in_kernel(vcpu) ||
+ !apic_lvtt_tscdeadline(apic))
return 0;
return apic->lapic_timer.tscdeadline;
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/5] KVM: LAPIC: introduce kvm_get_lapic_target_expiration_tsc()
2016-10-17 7:44 [PATCH 0/5] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
2016-10-17 7:44 ` [PATCH 1/5] KVM: LAPIC: extract start_sw_period() to handle periodic/oneshot mode Wanpeng Li
2016-10-17 7:44 ` [PATCH 2/5] KVM: LAPIC: guarantee the timer is in tsc-deadline mode Wanpeng Li
@ 2016-10-17 7:45 ` Wanpeng Li
2016-10-17 7:45 ` [PATCH 4/5] KVM: LAPIC: rename start/cancel_hv_tscdeadline to start/cancel_hv_timer Wanpeng Li
2016-10-17 7:45 ` [PATCH 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
4 siblings, 0 replies; 11+ messages in thread
From: Wanpeng Li @ 2016-10-17 7:45 UTC (permalink / raw)
To: linux-kernel, kvm
Cc: Paolo Bonzini, Radim Krčmář, Yunhong Jiang,
Wanpeng Li
From: Wanpeng Li <wanpeng.li@hotmail.com>
Introdce kvm_get_lapic_target_expiration_tsc() to get APIC Timer target
deadline tsc.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Yunhong Jiang <yunhong.jiang@intel.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
arch/x86/kvm/lapic.c | 9 +++++++++
arch/x86/kvm/lapic.h | 1 +
arch/x86/kvm/x86.c | 2 +-
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index dce6c0b..531fc28 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1706,6 +1706,15 @@ void kvm_free_lapic(struct kvm_vcpu *vcpu)
* LAPIC interface
*----------------------------------------------------------------------
*/
+u64 kvm_get_lapic_target_expiration_tsc(struct kvm_vcpu *vcpu)
+{
+ struct kvm_lapic *apic = vcpu->arch.apic;
+
+ if (!lapic_in_kernel(vcpu))
+ return 0;
+
+ return apic->lapic_timer.tscdeadline;
+}
u64 kvm_get_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu)
{
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index f60d01c..031db26 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -85,6 +85,7 @@ int kvm_apic_get_state(struct kvm_vcpu *vcpu, struct kvm_lapic_state *s);
int kvm_apic_set_state(struct kvm_vcpu *vcpu, struct kvm_lapic_state *s);
int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu);
+u64 kvm_get_lapic_target_expiration_tsc(struct kvm_vcpu *vcpu);
u64 kvm_get_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu);
void kvm_set_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu, u64 data);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e375235..ec53c3e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2794,7 +2794,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
}
if (kvm_lapic_hv_timer_in_use(vcpu) &&
kvm_x86_ops->set_hv_timer(vcpu,
- kvm_get_lapic_tscdeadline_msr(vcpu)))
+ kvm_get_lapic_target_expiration_tsc(vcpu)))
kvm_lapic_switch_to_sw_timer(vcpu);
/*
* On a host with synchronized TSC, there is no need to update
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 4/5] KVM: LAPIC: rename start/cancel_hv_tscdeadline to start/cancel_hv_timer
2016-10-17 7:44 [PATCH 0/5] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
` (2 preceding siblings ...)
2016-10-17 7:45 ` [PATCH 3/5] KVM: LAPIC: introduce kvm_get_lapic_target_expiration_tsc() Wanpeng Li
@ 2016-10-17 7:45 ` Wanpeng Li
2016-10-17 7:45 ` [PATCH 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
4 siblings, 0 replies; 11+ messages in thread
From: Wanpeng Li @ 2016-10-17 7:45 UTC (permalink / raw)
To: linux-kernel, kvm
Cc: Paolo Bonzini, Radim Krčmář, Yunhong Jiang,
Wanpeng Li
From: Wanpeng Li <wanpeng.li@hotmail.com>
Rename start/cancel_hv_tscdeadline to start/cancel_hv_timer since
they will handle both APIC Timer periodic/oneshot mode and tsc-deadline
mode.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Yunhong Jiang <yunhong.jiang@intel.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
arch/x86/kvm/lapic.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 531fc28..0354a79 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1400,7 +1400,7 @@ bool kvm_lapic_hv_timer_in_use(struct kvm_vcpu *vcpu)
}
EXPORT_SYMBOL_GPL(kvm_lapic_hv_timer_in_use);
-static void cancel_hv_tscdeadline(struct kvm_lapic *apic)
+static void cancel_hv_timer(struct kvm_lapic *apic)
{
kvm_x86_ops->cancel_hv_timer(apic->vcpu);
apic->lapic_timer.hv_timer_in_use = false;
@@ -1412,26 +1412,26 @@ void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu)
WARN_ON(!apic->lapic_timer.hv_timer_in_use);
WARN_ON(swait_active(&vcpu->wq));
- cancel_hv_tscdeadline(apic);
+ cancel_hv_timer(apic);
apic_timer_expired(apic);
}
EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer);
-static bool start_hv_tscdeadline(struct kvm_lapic *apic)
+static bool start_hv_timer(struct kvm_lapic *apic)
{
u64 tscdeadline = apic->lapic_timer.tscdeadline;
if (atomic_read(&apic->lapic_timer.pending) ||
kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
if (apic->lapic_timer.hv_timer_in_use)
- cancel_hv_tscdeadline(apic);
+ cancel_hv_timer(apic);
} else {
apic->lapic_timer.hv_timer_in_use = true;
hrtimer_cancel(&apic->lapic_timer.timer);
/* In case the sw timer triggered in the window */
if (atomic_read(&apic->lapic_timer.pending))
- cancel_hv_tscdeadline(apic);
+ cancel_hv_timer(apic);
}
trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
apic->lapic_timer.hv_timer_in_use);
@@ -1445,7 +1445,7 @@ void kvm_lapic_switch_to_hv_timer(struct kvm_vcpu *vcpu)
WARN_ON(apic->lapic_timer.hv_timer_in_use);
if (apic_lvtt_tscdeadline(apic))
- start_hv_tscdeadline(apic);
+ start_hv_timer(apic);
}
EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_hv_timer);
@@ -1457,7 +1457,7 @@ void kvm_lapic_switch_to_sw_timer(struct kvm_vcpu *vcpu)
if (!apic->lapic_timer.hv_timer_in_use)
return;
- cancel_hv_tscdeadline(apic);
+ cancel_hv_timer(apic);
if (atomic_read(&apic->lapic_timer.pending))
return;
@@ -1473,7 +1473,7 @@ static void start_apic_timer(struct kvm_lapic *apic)
if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
start_sw_period(apic);
else if (apic_lvtt_tscdeadline(apic)) {
- if (!(kvm_x86_ops->set_hv_timer && start_hv_tscdeadline(apic)))
+ if (!(kvm_x86_ops->set_hv_timer && start_hv_timer(apic)))
start_sw_tscdeadline(apic);
}
}
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode VMX preemption timer support
2016-10-17 7:44 [PATCH 0/5] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
` (3 preceding siblings ...)
2016-10-17 7:45 ` [PATCH 4/5] KVM: LAPIC: rename start/cancel_hv_tscdeadline to start/cancel_hv_timer Wanpeng Li
@ 2016-10-17 7:45 ` Wanpeng Li
2016-10-17 8:45 ` Paolo Bonzini
2016-10-19 19:28 ` Radim Krčmář
4 siblings, 2 replies; 11+ messages in thread
From: Wanpeng Li @ 2016-10-17 7:45 UTC (permalink / raw)
To: linux-kernel, kvm
Cc: Paolo Bonzini, Radim Krčmář, Yunhong Jiang,
Wanpeng Li
From: Wanpeng Li <wanpeng.li@hotmail.com>
Most windows guests still utilize APIC Timer periodic/oneshot mode
instead of tsc-deadline mode, and the APIC Timer periodic/oneshot
mode are still emulated by high overhead hrtimer on host. This patch
converts the expected expire time of the periodic/oneshot mode to
guest deadline tsc in order to leverage VMX preemption timer logic
for APIC Timer tsc-deadline mode. After each preemption timer vmexit
preemption timer is restarted to emulate LVTT current-count register
is automatically reloaded from the initial-count register when the
count reaches 0. This patch reduces ~3800 cycles for each APIC Timer
periodic mode operation virtualization.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Yunhong Jiang <yunhong.jiang@intel.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
arch/x86/kvm/lapic.c | 103 ++++++++++++++++++++++++++++++++++++---------------
arch/x86/kvm/lapic.h | 1 +
2 files changed, 75 insertions(+), 29 deletions(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 0354a79..4e77d37 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1090,7 +1090,7 @@ static void apic_send_ipi(struct kvm_lapic *apic)
static u32 apic_get_tmcct(struct kvm_lapic *apic)
{
- ktime_t remaining;
+ ktime_t remaining, now;
s64 ns;
u32 tmcct;
@@ -1101,7 +1101,8 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
apic->lapic_timer.period == 0)
return 0;
- remaining = hrtimer_get_remaining(&apic->lapic_timer.timer);
+ now = apic->lapic_timer.timer.base->get_time();
+ remaining = ktime_sub(apic->lapic_timer.target_expiration, now);
if (ktime_to_ns(remaining) < 0)
remaining = ktime_set(0, 0);
@@ -1351,13 +1352,28 @@ static void start_sw_period(struct kvm_lapic *apic)
{
ktime_t now;
- /* lapic timer in oneshot or periodic mode */
+ now = apic->lapic_timer.timer.base->get_time();
+
+ if (likely(ktime_compare(apic->lapic_timer.target_expiration, now)))
+ hrtimer_start(&apic->lapic_timer.timer,
+ apic->lapic_timer.target_expiration,
+ HRTIMER_MODE_ABS_PINNED);
+ else
+ apic_timer_expired(apic);
+}
+
+static bool set_target_expiration(struct kvm_lapic *apic)
+{
+ ktime_t now;
+ u64 tscl = rdtsc();
+
now = apic->lapic_timer.timer.base->get_time();
apic->lapic_timer.period = (u64)kvm_lapic_get_reg(apic, APIC_TMICT)
- * APIC_BUS_CYCLE_NS * apic->divide_count;
+ * APIC_BUS_CYCLE_NS * apic->divide_count;
if (!apic->lapic_timer.period)
- return;
+ return false;
+
/*
* Do not allow the guest to program periodic timers with small
* interval, since the hrtimers are not throttled by the host
@@ -1376,10 +1392,6 @@ static void start_sw_period(struct kvm_lapic *apic)
}
}
- hrtimer_start(&apic->lapic_timer.timer,
- ktime_add_ns(now, apic->lapic_timer.period),
- HRTIMER_MODE_ABS_PINNED);
-
apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
PRIx64 ", "
"timer initial count 0x%x, period %lldns, "
@@ -1389,6 +1401,12 @@ static void start_sw_period(struct kvm_lapic *apic)
apic->lapic_timer.period,
ktime_to_ns(ktime_add_ns(now,
apic->lapic_timer.period)));
+
+ 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);
+
+ return true;
}
bool kvm_lapic_hv_timer_in_use(struct kvm_vcpu *vcpu)
@@ -1406,22 +1424,12 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
apic->lapic_timer.hv_timer_in_use = false;
}
-void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu)
-{
- struct kvm_lapic *apic = vcpu->arch.apic;
-
- WARN_ON(!apic->lapic_timer.hv_timer_in_use);
- WARN_ON(swait_active(&vcpu->wq));
- cancel_hv_timer(apic);
- apic_timer_expired(apic);
-}
-EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer);
-
static bool start_hv_timer(struct kvm_lapic *apic)
{
u64 tscdeadline = apic->lapic_timer.tscdeadline;
- if (atomic_read(&apic->lapic_timer.pending) ||
+ if ((atomic_read(&apic->lapic_timer.pending) &&
+ !apic_lvtt_period(apic)) ||
kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
if (apic->lapic_timer.hv_timer_in_use)
cancel_hv_timer(apic);
@@ -1430,7 +1438,8 @@ static bool start_hv_timer(struct kvm_lapic *apic)
hrtimer_cancel(&apic->lapic_timer.timer);
/* In case the sw timer triggered in the window */
- if (atomic_read(&apic->lapic_timer.pending))
+ if (atomic_read(&apic->lapic_timer.pending) &&
+ !apic_lvtt_period(apic))
cancel_hv_timer(apic);
}
trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
@@ -1438,14 +1447,42 @@ static bool start_hv_timer(struct kvm_lapic *apic)
return apic->lapic_timer.hv_timer_in_use;
}
+void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu)
+{
+ struct kvm_lapic *apic = vcpu->arch.apic;
+
+ WARN_ON(!apic->lapic_timer.hv_timer_in_use);
+ WARN_ON(swait_active(&vcpu->wq));
+ cancel_hv_timer(apic);
+ apic_timer_expired(apic);
+
+ if (apic_lvtt_period(apic) &&
+ set_target_expiration(apic) &&
+ !start_hv_timer(apic))
+ start_sw_period(apic);
+}
+EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer);
+
void kvm_lapic_switch_to_hv_timer(struct kvm_vcpu *vcpu)
{
struct kvm_lapic *apic = vcpu->arch.apic;
WARN_ON(apic->lapic_timer.hv_timer_in_use);
- if (apic_lvtt_tscdeadline(apic))
- start_hv_timer(apic);
+ if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
+ ktime_t remaining, now;
+ u64 tscl = rdtsc();
+
+ now = apic->lapic_timer.timer.base->get_time();
+ remaining = hrtimer_get_remaining(&apic->lapic_timer.timer);
+ if (ktime_to_ns(remaining) < 0)
+ remaining = ktime_set(0, 0);
+
+ apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
+ nsec_to_cycles(apic->vcpu, ktime_to_ns(remaining));
+ apic->lapic_timer.target_expiration = ktime_add_ns(now, ktime_to_ns(remaining));
+ }
+ start_hv_timer(apic);
}
EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_hv_timer);
@@ -1462,7 +1499,10 @@ void kvm_lapic_switch_to_sw_timer(struct kvm_vcpu *vcpu)
if (atomic_read(&apic->lapic_timer.pending))
return;
- start_sw_tscdeadline(apic);
+ if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
+ start_sw_period(apic);
+ else if (apic_lvtt_tscdeadline(apic))
+ start_sw_tscdeadline(apic);
}
EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_sw_timer);
@@ -1470,9 +1510,11 @@ static void start_apic_timer(struct kvm_lapic *apic)
{
atomic_set(&apic->lapic_timer.pending, 0);
- if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
- start_sw_period(apic);
- else if (apic_lvtt_tscdeadline(apic)) {
+ if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
+ if (set_target_expiration(apic) &&
+ !(kvm_x86_ops->set_hv_timer && start_hv_timer(apic)))
+ start_sw_period(apic);
+ } else if (apic_lvtt_tscdeadline(apic)) {
if (!(kvm_x86_ops->set_hv_timer && start_hv_timer(apic)))
start_sw_tscdeadline(apic);
}
@@ -2005,8 +2047,11 @@ void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
if (atomic_read(&apic->lapic_timer.pending) > 0) {
kvm_apic_local_deliver(apic, APIC_LVTT);
- if (apic_lvtt_tscdeadline(apic))
+ if (!(apic_lvtt_period(apic) &&
+ kvm_lapic_hv_timer_in_use(vcpu))) {
apic->lapic_timer.tscdeadline = 0;
+ apic->lapic_timer.target_expiration = ktime_set(0, 0);
+ }
atomic_set(&apic->lapic_timer.pending, 0);
}
}
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 031db26..e0c8023 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -15,6 +15,7 @@
struct kvm_timer {
struct hrtimer timer;
s64 period; /* unit: ns */
+ ktime_t target_expiration;
u32 timer_mode;
u32 timer_mode_mask;
u64 tscdeadline;
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode VMX preemption timer support
2016-10-17 7:45 ` [PATCH 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
@ 2016-10-17 8:45 ` Paolo Bonzini
2016-10-19 19:28 ` Radim Krčmář
1 sibling, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2016-10-17 8:45 UTC (permalink / raw)
To: Wanpeng Li, linux-kernel, kvm
Cc: Radim Krčmář, Yunhong Jiang, Wanpeng Li
On 17/10/2016 09:45, Wanpeng Li wrote:
> + remaining = ktime_sub(apic->lapic_timer.target_expiration, now);
> if (ktime_to_ns(remaining) < 0)
> remaining = ktime_set(0, 0);
>
> @@ -1351,13 +1352,28 @@ static void start_sw_period(struct kvm_lapic *apic)
> {
> ktime_t now;
>
> - /* lapic timer in oneshot or periodic mode */
> + now = apic->lapic_timer.timer.base->get_time();
> +
> + if (likely(ktime_compare(apic->lapic_timer.target_expiration, now)))
ktime_after, not ktime_compare. Can be fixed on commit, I guess.
Paolo
> + hrtimer_start(&apic->lapic_timer.timer,
> + apic->lapic_timer.target_expiration,
> + HRTIMER_MODE_ABS_PINNED);
> + else
> + apic_timer_expired(apic);
> +}
> +
> +static bool set_target_expiration(struct kvm_lapic *apic)
> +{
> + ktime_t now;
> + u64 tscl = rdtsc();
> +
> now = apic->lapic_timer.timer.base->get_time();
> apic->lapic_timer.period = (u64)kvm_lapic_get_reg(apic, APIC_TMICT)
> - * APIC_BUS_CYCLE_NS * apic->divide_count;
> + * APIC_BUS_CYCLE_NS * apic->divide_count;
>
> if (!apic->lapic_timer.period)
> - return;
> + return false;
> /*
> * Do not allow the guest to program periodic timers with small
> * interval, since the hrtimers are not throttled by the host
> @@ -1376,10 +1392,6 @@ static void start_sw_period(struct kvm_lapic *apic)
> }
> }
>
> - hrtimer_start(&apic->lapic_timer.timer,
> - ktime_add_ns(now, apic->lapic_timer.period),
> - HRTIMER_MODE_ABS_PINNED);
> -
> apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
> PRIx64 ", "
> "timer initial count 0x%x, period %lldns, "
> @@ -1389,6 +1401,12 @@ static void start_sw_period(struct kvm_lapic *apic)
> apic->lapic_timer.period,
> ktime_to_ns(ktime_add_ns(now,
> apic->lapic_timer.period)));
> +
> + 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);
> +
> + return true;
> }
>
> bool kvm_lapic_hv_timer_in_use(struct kvm_vcpu *vcpu)
> @@ -1406,22 +1424,12 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
> apic->lapic_timer.hv_timer_in_use = false;
> }
>
> -void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu)
> -{
> - struct kvm_lapic *apic = vcpu->arch.apic;
> -
> - WARN_ON(!apic->lapic_timer.hv_timer_in_use);
> - WARN_ON(swait_active(&vcpu->wq));
> - cancel_hv_timer(apic);
> - apic_timer_expired(apic);
> -}
> -EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer);
> -
> static bool start_hv_timer(struct kvm_lapic *apic)
> {
> u64 tscdeadline = apic->lapic_timer.tscdeadline;
>
> - if (atomic_read(&apic->lapic_timer.pending) ||
> + if ((atomic_read(&apic->lapic_timer.pending) &&
> + !apic_lvtt_period(apic)) ||
> kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
> if (apic->lapic_timer.hv_timer_in_use)
> cancel_hv_timer(apic);
> @@ -1430,7 +1438,8 @@ static bool start_hv_timer(struct kvm_lapic *apic)
> hrtimer_cancel(&apic->lapic_timer.timer);
>
> /* In case the sw timer triggered in the window */
> - if (atomic_read(&apic->lapic_timer.pending))
> + if (atomic_read(&apic->lapic_timer.pending) &&
> + !apic_lvtt_period(apic))
> cancel_hv_timer(apic);
> }
> trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
> @@ -1438,14 +1447,42 @@ static bool start_hv_timer(struct kvm_lapic *apic)
> return apic->lapic_timer.hv_timer_in_use;
> }
>
> +void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_lapic *apic = vcpu->arch.apic;
> +
> + WARN_ON(!apic->lapic_timer.hv_timer_in_use);
> + WARN_ON(swait_active(&vcpu->wq));
> + cancel_hv_timer(apic);
> + apic_timer_expired(apic);
> +
> + if (apic_lvtt_period(apic) &&
> + set_target_expiration(apic) &&
> + !start_hv_timer(apic))
> + start_sw_period(apic);
> +}
> +EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer);
> +
> void kvm_lapic_switch_to_hv_timer(struct kvm_vcpu *vcpu)
> {
> struct kvm_lapic *apic = vcpu->arch.apic;
>
> WARN_ON(apic->lapic_timer.hv_timer_in_use);
>
> - if (apic_lvtt_tscdeadline(apic))
> - start_hv_timer(apic);
> + if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
> + ktime_t remaining, now;
> + u64 tscl = rdtsc();
> +
> + now = apic->lapic_timer.timer.base->get_time();
> + remaining = hrtimer_get_remaining(&apic->lapic_timer.timer);
> + if (ktime_to_ns(remaining) < 0)
> + remaining = ktime_set(0, 0);
> +
> + apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
> + nsec_to_cycles(apic->vcpu, ktime_to_ns(remaining));
> + apic->lapic_timer.target_expiration = ktime_add_ns(now, ktime_to_ns(remaining));
> + }
> + start_hv_timer(apic);
> }
> EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_hv_timer);
>
> @@ -1462,7 +1499,10 @@ void kvm_lapic_switch_to_sw_timer(struct kvm_vcpu *vcpu)
> if (atomic_read(&apic->lapic_timer.pending))
> return;
>
> - start_sw_tscdeadline(apic);
> + if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
> + start_sw_period(apic);
> + else if (apic_lvtt_tscdeadline(apic))
> + start_sw_tscdeadline(apic);
> }
> EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_sw_timer);
>
> @@ -1470,9 +1510,11 @@ static void start_apic_timer(struct kvm_lapic *apic)
> {
> atomic_set(&apic->lapic_timer.pending, 0);
>
> - if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
> - start_sw_period(apic);
> - else if (apic_lvtt_tscdeadline(apic)) {
> + if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
> + if (set_target_expiration(apic) &&
> + !(kvm_x86_ops->set_hv_timer && start_hv_timer(apic)))
> + start_sw_period(apic);
> + } else if (apic_lvtt_tscdeadline(apic)) {
> if (!(kvm_x86_ops->set_hv_timer && start_hv_timer(apic)))
> start_sw_tscdeadline(apic);
> }
> @@ -2005,8 +2047,11 @@ void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
>
> if (atomic_read(&apic->lapic_timer.pending) > 0) {
> kvm_apic_local_deliver(apic, APIC_LVTT);
> - if (apic_lvtt_tscdeadline(apic))
> + if (!(apic_lvtt_period(apic) &&
> + kvm_lapic_hv_timer_in_use(vcpu))) {
> apic->lapic_timer.tscdeadline = 0;
> + apic->lapic_timer.target_expiration = ktime_set(0, 0);
> + }
> atomic_set(&apic->lapic_timer.pending, 0);
> }
> }
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index 031db26..e0c8023 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -15,6 +15,7 @@
> struct kvm_timer {
> struct hrtimer timer;
> s64 period; /* unit: ns */
> + ktime_t target_expiration;
> u32 timer_mode;
> u32 timer_mode_mask;
> u64 tscdeadline;
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode VMX preemption timer support
2016-10-17 7:45 ` [PATCH 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
2016-10-17 8:45 ` Paolo Bonzini
@ 2016-10-19 19:28 ` Radim Krčmář
2016-10-19 22:57 ` Wanpeng Li
1 sibling, 1 reply; 11+ messages in thread
From: Radim Krčmář @ 2016-10-19 19:28 UTC (permalink / raw)
To: Wanpeng Li; +Cc: linux-kernel, kvm, Paolo Bonzini, Yunhong Jiang, Wanpeng Li
2016-10-17 15:45+0800, Wanpeng Li:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
>
> Most windows guests still utilize APIC Timer periodic/oneshot mode
> instead of tsc-deadline mode, and the APIC Timer periodic/oneshot
> mode are still emulated by high overhead hrtimer on host. This patch
> converts the expected expire time of the periodic/oneshot mode to
> guest deadline tsc in order to leverage VMX preemption timer logic
> for APIC Timer tsc-deadline mode. After each preemption timer vmexit
> preemption timer is restarted to emulate LVTT current-count register
> is automatically reloaded from the initial-count register when the
> count reaches 0. This patch reduces ~3800 cycles for each APIC Timer
> periodic mode operation virtualization.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Yunhong Jiang <yunhong.jiang@intel.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> @@ -1090,7 +1090,7 @@ static void apic_send_ipi(struct kvm_lapic *apic)
>
> static u32 apic_get_tmcct(struct kvm_lapic *apic)
> {
> - ktime_t remaining;
> + ktime_t remaining, now;
> s64 ns;
> u32 tmcct;
>
> @@ -1101,7 +1101,8 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
> apic->lapic_timer.period == 0)
> return 0;
>
> - remaining = hrtimer_get_remaining(&apic->lapic_timer.timer);
> + now = apic->lapic_timer.timer.base->get_time();
> + remaining = ktime_sub(apic->lapic_timer.target_expiration, now);
Periodic timer does not advance apic->lapic_timer.target_expiration,
when rearming the hrtimer, so this would incorrectly return 0 in
subsequent periods.
> if (ktime_to_ns(remaining) < 0)
> remaining = ktime_set(0, 0);
>
> @@ -1438,14 +1447,42 @@ static bool start_hv_timer(struct kvm_lapic *apic)
> return apic->lapic_timer.hv_timer_in_use;
> }
>
> +void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_lapic *apic = vcpu->arch.apic;
> +
> + WARN_ON(!apic->lapic_timer.hv_timer_in_use);
> + WARN_ON(swait_active(&vcpu->wq));
> + cancel_hv_timer(apic);
> + apic_timer_expired(apic);
> +
> + if (apic_lvtt_period(apic) &&
> + set_target_expiration(apic) &&
> + !start_hv_timer(apic))
> + start_sw_period(apic);
> +}
> +EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer);
> +
> void kvm_lapic_switch_to_hv_timer(struct kvm_vcpu *vcpu)
> {
> struct kvm_lapic *apic = vcpu->arch.apic;
>
> WARN_ON(apic->lapic_timer.hv_timer_in_use);
>
> - if (apic_lvtt_tscdeadline(apic))
> - start_hv_timer(apic);
> + if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
> + ktime_t remaining, now;
> + u64 tscl = rdtsc();
> +
> + now = apic->lapic_timer.timer.base->get_time();
> + remaining = hrtimer_get_remaining(&apic->lapic_timer.timer);
> + if (ktime_to_ns(remaining) < 0)
> + remaining = ktime_set(0, 0);
> +
> + apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
> + nsec_to_cycles(apic->vcpu, ktime_to_ns(remaining));
> + apic->lapic_timer.target_expiration = ktime_add_ns(now, ktime_to_ns(remaining));
For oneshot timer, there is no need to hrtimer_get_remaining(), because
apic->lapic_timer.tscdeadline and apic->lapic_timer.target_expiration
are already correct, so we could just use them.
The same could be true for the periodic timer as well, but
apic->lapic_timer.target_expiration nor apic->lapic_timer.tscdeadline is
advanced in apic_timer_fn(), so they are soon incorrect.
I think it would be better to add a function to advance the periodic
timer and use it in kvm_lapic_expired_hv_timer() and in apic_timer_fn().
The function can be simpler than set_target_expiration(), because it
just adds the period to an existing timer. Periodic timer will also be
better then, because the period will not depend on KVM's latency when
rearming.
> + }
> + start_hv_timer(apic);
> }
> EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_hv_timer);
>
> @@ -1462,7 +1499,10 @@ void kvm_lapic_switch_to_sw_timer(struct kvm_vcpu *vcpu)
> if (atomic_read(&apic->lapic_timer.pending))
> return;
>
> - start_sw_tscdeadline(apic);
> + if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
> + start_sw_period(apic);
> + else if (apic_lvtt_tscdeadline(apic))
> + start_sw_tscdeadline(apic);
> }
> EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_sw_timer);
>
> @@ -1470,9 +1510,11 @@ static void start_apic_timer(struct kvm_lapic *apic)
> {
> atomic_set(&apic->lapic_timer.pending, 0);
>
> - if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
> - start_sw_period(apic);
> - else if (apic_lvtt_tscdeadline(apic)) {
> + if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
> + if (set_target_expiration(apic) &&
> + !(kvm_x86_ops->set_hv_timer && start_hv_timer(apic)))
> + start_sw_period(apic);
> + } else if (apic_lvtt_tscdeadline(apic)) {
> if (!(kvm_x86_ops->set_hv_timer && start_hv_timer(apic)))
> start_sw_tscdeadline(apic);
> }
> @@ -2005,8 +2047,11 @@ void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
>
> if (atomic_read(&apic->lapic_timer.pending) > 0) {
> kvm_apic_local_deliver(apic, APIC_LVTT);
> - if (apic_lvtt_tscdeadline(apic))
> + if (!(apic_lvtt_period(apic) &&
> + kvm_lapic_hv_timer_in_use(vcpu))) {
This would zero apic->lapic_timer.target_expiration of
apic_lvtt_period() when !kvm_lapic_hv_timer_in_use().
I think we don't want to ever do that, so we want
if (!(apic_lvtt_period(apic)) {
or maybe even better
if (apic_lvtt_tscdeadline(apic))
apic->lapic_timer.tscdeadline = 0;
if (apic_lvtt_oneshot(apic)) {
apic->lapic_timer.tscdeadline = 0;
apic->lapic_timer.target_expiration = ktime_set(0, 0);
}
> apic->lapic_timer.tscdeadline = 0;
> + apic->lapic_timer.target_expiration = ktime_set(0, 0);
> + }
> atomic_set(&apic->lapic_timer.pending, 0);
> }
> }
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode VMX preemption timer support
2016-10-19 19:28 ` Radim Krčmář
@ 2016-10-19 22:57 ` Wanpeng Li
2016-10-20 18:10 ` Radim Krčmář
0 siblings, 1 reply; 11+ messages in thread
From: Wanpeng Li @ 2016-10-19 22:57 UTC (permalink / raw)
To: Radim Krčmář
Cc: linux-kernel@vger.kernel.org, kvm, Paolo Bonzini, Yunhong Jiang,
Wanpeng Li
2016-10-20 3:28 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
> 2016-10-17 15:45+0800, Wanpeng Li:
>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>
>> Most windows guests still utilize APIC Timer periodic/oneshot mode
>> instead of tsc-deadline mode, and the APIC Timer periodic/oneshot
>> mode are still emulated by high overhead hrtimer on host. This patch
>> converts the expected expire time of the periodic/oneshot mode to
>> guest deadline tsc in order to leverage VMX preemption timer logic
>> for APIC Timer tsc-deadline mode. After each preemption timer vmexit
>> preemption timer is restarted to emulate LVTT current-count register
>> is automatically reloaded from the initial-count register when the
>> count reaches 0. This patch reduces ~3800 cycles for each APIC Timer
>> periodic mode operation virtualization.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: Yunhong Jiang <yunhong.jiang@intel.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> @@ -1090,7 +1090,7 @@ static void apic_send_ipi(struct kvm_lapic *apic)
>>
>> static u32 apic_get_tmcct(struct kvm_lapic *apic)
>> {
>> - ktime_t remaining;
>> + ktime_t remaining, now;
>> s64 ns;
>> u32 tmcct;
>>
>> @@ -1101,7 +1101,8 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
>> apic->lapic_timer.period == 0)
>> return 0;
>>
>> - remaining = hrtimer_get_remaining(&apic->lapic_timer.timer);
>> + now = apic->lapic_timer.timer.base->get_time();
>> + remaining = ktime_sub(apic->lapic_timer.target_expiration, now);
>
> Periodic timer does not advance apic->lapic_timer.target_expiration,
> when rearming the hrtimer, so this would incorrectly return 0 in
> subsequent periods.
Agreed.
>
>> if (ktime_to_ns(remaining) < 0)
>> remaining = ktime_set(0, 0);
>>
>> @@ -1438,14 +1447,42 @@ static bool start_hv_timer(struct kvm_lapic *apic)
>> return apic->lapic_timer.hv_timer_in_use;
>> }
>>
>> +void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu)
>> +{
>> + struct kvm_lapic *apic = vcpu->arch.apic;
>> +
>> + WARN_ON(!apic->lapic_timer.hv_timer_in_use);
>> + WARN_ON(swait_active(&vcpu->wq));
>> + cancel_hv_timer(apic);
>> + apic_timer_expired(apic);
>> +
>> + if (apic_lvtt_period(apic) &&
>> + set_target_expiration(apic) &&
>> + !start_hv_timer(apic))
>> + start_sw_period(apic);
>> +}
>> +EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer);
>> +
>> void kvm_lapic_switch_to_hv_timer(struct kvm_vcpu *vcpu)
>> {
>> struct kvm_lapic *apic = vcpu->arch.apic;
>>
>> WARN_ON(apic->lapic_timer.hv_timer_in_use);
>>
>> - if (apic_lvtt_tscdeadline(apic))
>> - start_hv_timer(apic);
>> + if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
>> + ktime_t remaining, now;
>> + u64 tscl = rdtsc();
>> +
>> + now = apic->lapic_timer.timer.base->get_time();
>> + remaining = hrtimer_get_remaining(&apic->lapic_timer.timer);
>> + if (ktime_to_ns(remaining) < 0)
>> + remaining = ktime_set(0, 0);
>> +
>> + apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
>> + nsec_to_cycles(apic->vcpu, ktime_to_ns(remaining));
>> + apic->lapic_timer.target_expiration = ktime_add_ns(now, ktime_to_ns(remaining));
>
> For oneshot timer, there is no need to hrtimer_get_remaining(), because
> apic->lapic_timer.tscdeadline and apic->lapic_timer.target_expiration
> are already correct, so we could just use them.
Agreed.
>
> The same could be true for the periodic timer as well, but
> apic->lapic_timer.target_expiration nor apic->lapic_timer.tscdeadline is
> advanced in apic_timer_fn(), so they are soon incorrect.
Yeah, I catch this when testing.
>
> I think it would be better to add a function to advance the periodic
> timer and use it in kvm_lapic_expired_hv_timer() and in apic_timer_fn().
>
> The function can be simpler than set_target_expiration(), because it
> just adds the period to an existing timer. Periodic timer will also be
> better then, because the period will not depend on KVM's latency when
> rearming.
Good point, what's the function name do you like? How about
advance_target_expiration()?
>
>> + }
>> + start_hv_timer(apic);
>> }
>> EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_hv_timer);
>>
>> @@ -1462,7 +1499,10 @@ void kvm_lapic_switch_to_sw_timer(struct kvm_vcpu *vcpu)
>> if (atomic_read(&apic->lapic_timer.pending))
>> return;
>>
>> - start_sw_tscdeadline(apic);
>> + if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
>> + start_sw_period(apic);
>> + else if (apic_lvtt_tscdeadline(apic))
>> + start_sw_tscdeadline(apic);
>> }
>> EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_sw_timer);
>>
>> @@ -1470,9 +1510,11 @@ static void start_apic_timer(struct kvm_lapic *apic)
>> {
>> atomic_set(&apic->lapic_timer.pending, 0);
>>
>> - if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
>> - start_sw_period(apic);
>> - else if (apic_lvtt_tscdeadline(apic)) {
>> + if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
>> + if (set_target_expiration(apic) &&
>> + !(kvm_x86_ops->set_hv_timer && start_hv_timer(apic)))
>> + start_sw_period(apic);
>> + } else if (apic_lvtt_tscdeadline(apic)) {
>> if (!(kvm_x86_ops->set_hv_timer && start_hv_timer(apic)))
>> start_sw_tscdeadline(apic);
>> }
>> @@ -2005,8 +2047,11 @@ void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
>>
>> if (atomic_read(&apic->lapic_timer.pending) > 0) {
>> kvm_apic_local_deliver(apic, APIC_LVTT);
>> - if (apic_lvtt_tscdeadline(apic))
>> + if (!(apic_lvtt_period(apic) &&
>> + kvm_lapic_hv_timer_in_use(vcpu))) {
>
> This would zero apic->lapic_timer.target_expiration of
> apic_lvtt_period() when !kvm_lapic_hv_timer_in_use().
>
> I think we don't want to ever do that, so we want
>
> if (!(apic_lvtt_period(apic)) {
>
> or maybe even better
>
> if (apic_lvtt_tscdeadline(apic))
> apic->lapic_timer.tscdeadline = 0;
>
> if (apic_lvtt_oneshot(apic)) {
> apic->lapic_timer.tscdeadline = 0;
> apic->lapic_timer.target_expiration = ktime_set(0, 0);
> }
>
>> apic->lapic_timer.tscdeadline = 0;
>> + apic->lapic_timer.target_expiration = ktime_set(0, 0);
>> + }
Agreed, thanks for your review. :)
Regards,
Wanpeng Li
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode VMX preemption timer support
2016-10-19 22:57 ` Wanpeng Li
@ 2016-10-20 18:10 ` Radim Krčmář
2016-10-21 13:38 ` Wanpeng Li
0 siblings, 1 reply; 11+ messages in thread
From: Radim Krčmář @ 2016-10-20 18:10 UTC (permalink / raw)
To: Wanpeng Li
Cc: linux-kernel@vger.kernel.org, kvm, Paolo Bonzini, Yunhong Jiang,
Wanpeng Li
2016-10-20 06:57+0800, Wanpeng Li:
> 2016-10-20 3:28 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
>> I think it would be better to add a function to advance the periodic
>> timer and use it in kvm_lapic_expired_hv_timer() and in apic_timer_fn().
>>
>> The function can be simpler than set_target_expiration(), because it
>> just adds the period to an existing timer. Periodic timer will also be
>> better then, because the period will not depend on KVM's latency when
>> rearming.
>
> Good point, what's the function name do you like? How about
> advance_target_expiration()?
Sounds good, I'd just slap periodic somewhere,
advance_periodic_target_expiration()?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode VMX preemption timer support
2016-10-20 18:10 ` Radim Krčmář
@ 2016-10-21 13:38 ` Wanpeng Li
0 siblings, 0 replies; 11+ messages in thread
From: Wanpeng Li @ 2016-10-21 13:38 UTC (permalink / raw)
To: Radim Krčmář
Cc: linux-kernel@vger.kernel.org, kvm, Paolo Bonzini, Yunhong Jiang,
Wanpeng Li
2016-10-21 2:10 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
> 2016-10-20 06:57+0800, Wanpeng Li:
>> 2016-10-20 3:28 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
>>> I think it would be better to add a function to advance the periodic
>>> timer and use it in kvm_lapic_expired_hv_timer() and in apic_timer_fn().
>>>
>>> The function can be simpler than set_target_expiration(), because it
>>> just adds the period to an existing timer. Periodic timer will also be
>>> better then, because the period will not depend on KVM's latency when
>>> rearming.
>>
>> Good point, what's the function name do you like? How about
>> advance_target_expiration()?
>
> Sounds good, I'd just slap periodic somewhere,
> advance_periodic_target_expiration()?
Cool, thanks for your review. :)
Regards,
Wanpeng Li
^ permalink raw reply [flat|nested] 11+ messages in thread