linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC V3 0/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support
@ 2016-10-13 11:34 Wanpeng Li
  2016-10-13 11:34 ` [PATCH RFC V3 1/6] KVM: LAPIC: extract start_sw_period() to handle periodic/oneshot mode Wanpeng Li
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Wanpeng Li @ 2016-10-13 11:34 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář, Yunhong Jiang, Wanpeng Li

Most windows guests which I have on hand currently still utilize APIC Timer 
periodic/oneshot mode instead of APIC Timer tsc-deadline mode:
- windows 2008 server r2
- windows 2012 server r2
- windows 7
- windows 10 

This patchset adds the APIC Timer periodic/oneshot mode VMX preemption 
timer support.

I test the patchset by modifying kvm-unit-test/apic.flat to test 10w times 
APIC timer operations (from guest writes LVTT to timer fire and return to 
guest).

The patchset reduces ~300+ clock cycles for each APIC timer oneshot mode 
operation virtualization. However, the performance of periodic mode is 
still bad, so this version is still a RFC. Your comments to improve the 
patchset is a great appreciated.

v2 -> v3:
 * remove kvm_lapic_hv_timer_in_use() check in apic_get_tmcc, replace
   the hritmer_get_remaining() by target_expiration - now
 * rename expired_period to target_expiration
 * introduce set_target_expiration() helper
 * move the checking of minimal period to set_target_expiration()
 * cleanup kvm_get_lapic_target_deadline_tsc()

v1 -> v2:
 * remember the timeout when setting up the timer to get a correct TMCCT
 * move apic->lapic_timer.period/tscdeadline caculations to start_apic_timer()

Wanpeng Li (6):
  KVM: LAPIC: extract start_sw_period() to handle periodic/oneshot mode
  KVM: LAPIC: guarantee the timer is in tsc-deadline mode when rdmsr MSR_IA32_TSCDEADLINE
  KVM: LAPIC: introduce set_target_expiration to set target expiration time
  KVM: LAPIC: introduce kvm_get_lapic_target_expiration_tsc() to get APIC Timer target deadline tsc
  KVM: LAPIC: rename start/cancel_hv_tscdeadline to start/cancel_hv_timer
  KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support

 arch/x86/kvm/lapic.c | 173 +++++++++++++++++++++++++++++++--------------------
 arch/x86/kvm/lapic.h |   2 +
 arch/x86/kvm/x86.c   |   2 +-
 3 files changed, 109 insertions(+), 68 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH RFC V3 1/6] KVM: LAPIC: extract start_sw_period() to handle periodic/oneshot mode
  2016-10-13 11:34 [PATCH RFC V3 0/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
@ 2016-10-13 11:34 ` Wanpeng Li
  2016-10-13 11:34 ` [PATCH RFC V3 2/6] KVM: LAPIC: guarantee the timer is in tsc-deadline mode when rdmsr MSR_IA32_TSCDEADLINE Wanpeng Li
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Wanpeng Li @ 2016-10-13 11:34 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] 12+ messages in thread

* [PATCH RFC V3 2/6] KVM: LAPIC: guarantee the timer is in tsc-deadline mode when rdmsr MSR_IA32_TSCDEADLINE
  2016-10-13 11:34 [PATCH RFC V3 0/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
  2016-10-13 11:34 ` [PATCH RFC V3 1/6] KVM: LAPIC: extract start_sw_period() to handle periodic/oneshot mode Wanpeng Li
@ 2016-10-13 11:34 ` Wanpeng Li
  2016-10-13 11:34 ` [PATCH RFC V3 3/6] KVM: LAPIC: introduce set_target_expiration() to set target expiration time Wanpeng Li
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Wanpeng Li @ 2016-10-13 11:34 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] 12+ messages in thread

* [PATCH RFC V3 3/6] KVM: LAPIC: introduce set_target_expiration() to set target expiration time
  2016-10-13 11:34 [PATCH RFC V3 0/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
  2016-10-13 11:34 ` [PATCH RFC V3 1/6] KVM: LAPIC: extract start_sw_period() to handle periodic/oneshot mode Wanpeng Li
  2016-10-13 11:34 ` [PATCH RFC V3 2/6] KVM: LAPIC: guarantee the timer is in tsc-deadline mode when rdmsr MSR_IA32_TSCDEADLINE Wanpeng Li
@ 2016-10-13 11:34 ` Wanpeng Li
  2016-10-13 11:34 ` [PATCH RFC V3 4/6] KVM: LAPIC: introduce kvm_get_lapic_target_expiration_tsc() to get APIC Timer target deadline tsc Wanpeng Li
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Wanpeng Li @ 2016-10-13 11:34 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář, Yunhong Jiang, Wanpeng Li

From: Wanpeng Li <wanpeng.li@hotmail.com>

Introduce set_target_expiration() to set target expiration time which is 
when LVTT current-count register counts down to zero and a timer interrupt 
is generated.

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 | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/lapic.h |  1 +
 2 files changed, 48 insertions(+)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index dce6c0b..09dad85 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1391,6 +1391,53 @@ static void start_sw_period(struct kvm_lapic *apic)
 				apic->lapic_timer.period)));
 }
 
+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;
+
+	if (!apic->lapic_timer.period)
+		return false;
+
+	/*
+	 * 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;
+		}
+	}
+
+	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)));
+
+	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)
 {
 	if (!lapic_in_kernel(vcpu))
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index f60d01c..889b504 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] 12+ messages in thread

* [PATCH RFC V3 4/6] KVM: LAPIC: introduce kvm_get_lapic_target_expiration_tsc() to get APIC Timer target deadline tsc
  2016-10-13 11:34 [PATCH RFC V3 0/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
                   ` (2 preceding siblings ...)
  2016-10-13 11:34 ` [PATCH RFC V3 3/6] KVM: LAPIC: introduce set_target_expiration() to set target expiration time Wanpeng Li
@ 2016-10-13 11:34 ` Wanpeng Li
  2016-10-13 11:34 ` [PATCH RFC V3 5/6] KVM: LAPIC: rename start/cancel_hv_tscdeadline to start/cancel_hv_timer Wanpeng Li
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Wanpeng Li @ 2016-10-13 11:34 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 09dad85..5eb83c6 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1753,6 +1753,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 889b504..66ce7b1 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -86,6 +86,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..2b85299 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] 12+ messages in thread

* [PATCH RFC V3 5/6] KVM: LAPIC: rename start/cancel_hv_tscdeadline to start/cancel_hv_timer
  2016-10-13 11:34 [PATCH RFC V3 0/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
                   ` (3 preceding siblings ...)
  2016-10-13 11:34 ` [PATCH RFC V3 4/6] KVM: LAPIC: introduce kvm_get_lapic_target_expiration_tsc() to get APIC Timer target deadline tsc Wanpeng Li
@ 2016-10-13 11:34 ` Wanpeng Li
  2016-10-13 11:34 ` [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
  2016-10-14 10:05 ` [PATCH RFC V3 0/6] " Wanpeng Li
  6 siblings, 0 replies; 12+ messages in thread
From: Wanpeng Li @ 2016-10-13 11:34 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 5eb83c6..e93e549 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1447,7 +1447,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;
@@ -1459,26 +1459,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);
@@ -1492,7 +1492,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);
 
@@ -1504,7 +1504,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;
@@ -1520,7 +1520,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] 12+ messages in thread

* [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support
  2016-10-13 11:34 [PATCH RFC V3 0/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
                   ` (4 preceding siblings ...)
  2016-10-13 11:34 ` [PATCH RFC V3 5/6] KVM: LAPIC: rename start/cancel_hv_tscdeadline to start/cancel_hv_timer Wanpeng Li
@ 2016-10-13 11:34 ` Wanpeng Li
  2016-10-13 12:35   ` Paolo Bonzini
  2016-10-14 10:05 ` [PATCH RFC V3 0/6] " Wanpeng Li
  6 siblings, 1 reply; 12+ messages in thread
From: Wanpeng Li @ 2016-10-13 11:34 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. 

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 | 100 ++++++++++++++++++++-------------------------------
 1 file changed, 39 insertions(+), 61 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index e93e549..7663246 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);
 
@@ -1349,46 +1350,9 @@ static void start_sw_tscdeadline(struct kvm_lapic *apic)
 
 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)));
+		apic->lapic_timer.target_expiration,
+		HRTIMER_MODE_ABS_PINNED);
 }
 
 static bool set_target_expiration(struct kvm_lapic *apic)
@@ -1453,22 +1417,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);
@@ -1477,7 +1431,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,
@@ -1485,14 +1440,29 @@ 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);
+	start_hv_timer(apic);
 }
 EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_hv_timer);
 
@@ -1509,7 +1479,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);
 
@@ -1517,9 +1490,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);
 	}
@@ -2052,8 +2027,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);
 	}
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support
  2016-10-13 11:34 ` [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
@ 2016-10-13 12:35   ` Paolo Bonzini
  2016-10-14  1:25     ` Wanpeng Li
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2016-10-13 12:35 UTC (permalink / raw)
  To: Wanpeng Li, linux-kernel, kvm
  Cc: Radim Krčmář, Yunhong Jiang, Wanpeng Li



On 13/10/2016 13:34, Wanpeng Li wrote:
> 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. 
> 
> 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 | 100 ++++++++++++++++++++-------------------------------
>  1 file changed, 39 insertions(+), 61 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index e93e549..7663246 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);
>  
> @@ -1349,46 +1350,9 @@ static void start_sw_tscdeadline(struct kvm_lapic *apic)
>  
>  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)));
> +		apic->lapic_timer.target_expiration,
> +		HRTIMER_MODE_ABS_PINNED);
>  }
>  
>  static bool set_target_expiration(struct kvm_lapic *apic)
> @@ -1453,22 +1417,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;

I think things would be simpler if you change this to:

	if (!kvm_x86_ops->set_hv_timer)
		return false;

	if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
		if (!set_target_expiration(apic))
			return true;
	}

	tscdeadline = apic->lapic_timer.tscdeadline;

You can also add a corresponding

static void start_sw_timer(struct kvm_lapic *apic)
{
	if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
		start_sw_period(apic);
	else
		start_sw_tscdeadline(apic);
}

so that the caller can be just this:

	if (!start_hv_timer(apic));
		start_sw_timer(apic);

or in the case of kvm_lapic_expired_hv_timer:

	if (apic_lvtt_period(apic) && !start_hv_timer(apic))
		start_sw_period(apic);

Independent of this, patch 3 should be squashed into this one.

Thanks,

Paolo

> -	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);
> @@ -1477,7 +1431,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,
> @@ -1485,14 +1440,29 @@ 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);
> +	start_hv_timer(apic);
>  }
>  EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_hv_timer);
>  
> @@ -1509,7 +1479,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);
>  
> @@ -1517,9 +1490,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);
>  	}
> @@ -2052,8 +2027,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);
>  	}
>  }
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support
  2016-10-13 12:35   ` Paolo Bonzini
@ 2016-10-14  1:25     ` Wanpeng Li
  0 siblings, 0 replies; 12+ messages in thread
From: Wanpeng Li @ 2016-10-14  1:25 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: linux-kernel, kvm, Radim Krčmář,
	Yunhong Jiang, Wanpeng Li

2016-10-13 20:35 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
>
> On 13/10/2016 13:34, Wanpeng Li wrote:
>> 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.
>>
>> 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 | 100 ++++++++++++++++++++-------------------------------
>>  1 file changed, 39 insertions(+), 61 deletions(-)
>>
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index e93e549..7663246 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);
>>
>> @@ -1349,46 +1350,9 @@ static void start_sw_tscdeadline(struct kvm_lapic *apic)
>>
>>  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)));
>> +             apic->lapic_timer.target_expiration,
>> +             HRTIMER_MODE_ABS_PINNED);
>>  }
>>
>>  static bool set_target_expiration(struct kvm_lapic *apic)
>> @@ -1453,22 +1417,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;
>
> I think things would be simpler if you change this to:
>
>         if (!kvm_x86_ops->set_hv_timer)
>                 return false;
>
>         if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
>                 if (!set_target_expiration(apic))
>                         return true;
>         }

set_target_expiration() in start_hv_timer() is not correct as pointed
out by Radim. https://lkml.org/lkml/2016/10/12/93

Regards,
Wanpeng Li

>
>         tscdeadline = apic->lapic_timer.tscdeadline;
>
> You can also add a corresponding
>
> static void start_sw_timer(struct kvm_lapic *apic)
> {
>         if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
>                 start_sw_period(apic);
>         else
>                 start_sw_tscdeadline(apic);
> }
>
> so that the caller can be just this:
>
>         if (!start_hv_timer(apic));
>                 start_sw_timer(apic);
>
> or in the case of kvm_lapic_expired_hv_timer:
>
>         if (apic_lvtt_period(apic) && !start_hv_timer(apic))
>                 start_sw_period(apic);
>
> Independent of this, patch 3 should be squashed into this one.
>
> Thanks,
>
> Paolo
>
>> -     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);
>> @@ -1477,7 +1431,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,
>> @@ -1485,14 +1440,29 @@ 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);
>> +     start_hv_timer(apic);
>>  }
>>  EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_hv_timer);
>>
>> @@ -1509,7 +1479,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);
>>
>> @@ -1517,9 +1490,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);
>>       }
>> @@ -2052,8 +2027,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);
>>       }
>>  }
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH RFC V3 0/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support
  2016-10-13 11:34 [PATCH RFC V3 0/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
                   ` (5 preceding siblings ...)
  2016-10-13 11:34 ` [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
@ 2016-10-14 10:05 ` Wanpeng Li
  2016-10-14 11:11   ` Paolo Bonzini
  6 siblings, 1 reply; 12+ messages in thread
From: Wanpeng Li @ 2016-10-14 10:05 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář, Yunhong Jiang, Wanpeng Li

2016-10-13 19:34 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
[...]
>
> The patchset reduces ~300+ clock cycles for each APIC timer oneshot mode
> operation virtualization. However, the performance of periodic mode is
> still bad, so this version is still a RFC. Your comments to improve the
> patchset is a great appreciated.

I observed that the clock cycles between the start of
kvm_lapic_expired_hv_timer() (periodic mode emulated through restart a
preemption timer after the last preemption timer vmexit) to
kvm_inject_apic_timer_irqs() is almost half of the clock cycles
between the start of apic_timer_fn() (periodic mode emulated through
hrtimer) to kvm_inject_apic_timers_irqs(), so the overhead of
preemption timer is lower in this path. Maybe something is still not
correct in other places which results in performance of periodic mode
emulated by VMX preemption timer is still not good. Any help is a
great appreciated. :)

Radim, Paolo, ping :)

Regards,
Wanpeng Li

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH RFC V3 0/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support
  2016-10-14 10:05 ` [PATCH RFC V3 0/6] " Wanpeng Li
@ 2016-10-14 11:11   ` Paolo Bonzini
       [not found]     ` <CANRm+CzCDfmNpz+u+pDwJ5Gsqrgtb50GZc0B-jVSNdV_w3Ubtg@mail.gmail.com>
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2016-10-14 11:11 UTC (permalink / raw)
  To: Wanpeng Li, linux-kernel, kvm
  Cc: Radim Krčmář, Yunhong Jiang, Wanpeng Li



On 14/10/2016 12:05, Wanpeng Li wrote:
> 2016-10-13 19:34 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
> [...]
>>
>> The patchset reduces ~300+ clock cycles for each APIC timer oneshot mode
>> operation virtualization. However, the performance of periodic mode is
>> still bad, so this version is still a RFC. Your comments to improve the
>> patchset is a great appreciated.
> 
> I observed that the clock cycles between the start of
> kvm_lapic_expired_hv_timer() (periodic mode emulated through restart a
> preemption timer after the last preemption timer vmexit) to
> kvm_inject_apic_timer_irqs() is almost half of the clock cycles
> between the start of apic_timer_fn() (periodic mode emulated through
> hrtimer) to kvm_inject_apic_timers_irqs(), so the overhead of
> preemption timer is lower in this path. Maybe something is still not
> correct in other places which results in performance of periodic mode
> emulated by VMX preemption timer is still not good. Any help is a
> great appreciated. :)
> 
> Radim, Paolo, ping :)

Just post the patches and we'll see. :)  Another useful thing to do is
to prepare testcases similar to tscdeadline_latency, but using the
periodic and oneshot modes.

Paolo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH RFC V3 0/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support
       [not found]     ` <CANRm+CzCDfmNpz+u+pDwJ5Gsqrgtb50GZc0B-jVSNdV_w3Ubtg@mail.gmail.com>
@ 2016-10-14 12:08       ` Paolo Bonzini
  0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2016-10-14 12:08 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: linux-kernel, kvm, Radim Krčmář,
	Yunhong Jiang, Wanpeng Li



On 14/10/2016 14:06, Wanpeng Li wrote:
>     thing to do is
>     to prepare testcases similar to tscdeadline_latency, but using the
>     periodic and oneshot modes.
> 
> 
>  I have already post a kvm-unit-test/ apic.flat patch which can test
> periodic mode latency. :)

That one runs for a very short time, it's different from
tscdeadline_latency.

Paolo

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-10-14 12:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-13 11:34 [PATCH RFC V3 0/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
2016-10-13 11:34 ` [PATCH RFC V3 1/6] KVM: LAPIC: extract start_sw_period() to handle periodic/oneshot mode Wanpeng Li
2016-10-13 11:34 ` [PATCH RFC V3 2/6] KVM: LAPIC: guarantee the timer is in tsc-deadline mode when rdmsr MSR_IA32_TSCDEADLINE Wanpeng Li
2016-10-13 11:34 ` [PATCH RFC V3 3/6] KVM: LAPIC: introduce set_target_expiration() to set target expiration time Wanpeng Li
2016-10-13 11:34 ` [PATCH RFC V3 4/6] KVM: LAPIC: introduce kvm_get_lapic_target_expiration_tsc() to get APIC Timer target deadline tsc Wanpeng Li
2016-10-13 11:34 ` [PATCH RFC V3 5/6] KVM: LAPIC: rename start/cancel_hv_tscdeadline to start/cancel_hv_timer Wanpeng Li
2016-10-13 11:34 ` [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
2016-10-13 12:35   ` Paolo Bonzini
2016-10-14  1:25     ` Wanpeng Li
2016-10-14 10:05 ` [PATCH RFC V3 0/6] " Wanpeng Li
2016-10-14 11:11   ` Paolo Bonzini
     [not found]     ` <CANRm+CzCDfmNpz+u+pDwJ5Gsqrgtb50GZc0B-jVSNdV_w3Ubtg@mail.gmail.com>
2016-10-14 12:08       ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).