KVM: add option to advance tscdeadline hrtimer expiration (v4)

For the hrtimer which emulates the tscdeadline timer in the guest, add an option to advance expiration, and busy spin on VM-entry waiting for the actual expiration time to elapse.

This allows achieving low latencies in cyclictest (or any scenario which requires strict timing regarding timer expiration).

Reduces average cyclictest latency from 12us to 8us on Core i5 desktop.

Note: this option requires tuning to find the appropriate value for a particular hardware/guest combination. One method is to measure the average delay between apic_timer_fn and VM-entry.
Another method is to start with 1000ns, and increase the value in say 500ns increments until avg cyclictest numbers stop decreasing.

Signed-off-by Marcelo Tosatti <[email protected]>

Index: kvm/arch/x86/kvm/lapic.c
===================================================================

arch/x86/kvm/lapic.c +33 -0

@@ -33,6 +33,7 @@
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/apicdef.h>
+#include <asm/delay.h>
 #include <linux/atomic.h>
 #include <linux/jump_label.h>
 #include "kvm_cache_regs.h"

@@ -1073,6 +1074,7 @@ static void apic_timer_expired(struct kv
 {
 	struct kvm_vcpu *vcpu = apic->vcpu;
 	wait_queue_head_t *q = &vcpu->wq;
+	struct kvm_timer *ktimer = &apic->lapic_timer;
 
 	/*
 	 * Note: KVM_REQ_PENDING_TIMER is implicitly checked in

@@ -1087,11 +1089,64 @@ static void apic_timer_expired(struct kv
 
 	if (waitqueue_active(q))
 		wake_up_interruptible(q);
+
+	if (apic_lvtt_tscdeadline(apic))
+		ktimer->expired_tscdeadline = ktimer->tscdeadline;
+}
+
+/*
+ * On APICv, this test will cause a busy wait
+ * during a higher-priority task.
+ */
+
+static bool lapic_timer_int_injected(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->arch.apic;
+	u32 reg = kvm_apic_get_reg(apic, APIC_LVTT);
+
+	if (kvm_apic_hw_enabled(apic)) {
+		int vec = reg & APIC_VECTOR_MASK;
+
+		if (kvm_x86_ops->test_posted_interrupt)
+			return kvm_x86_ops->test_posted_interrupt(vcpu, vec);
+		else {
+			if (apic_test_vector(vec, apic->regs + APIC_ISR))
+				return true;
+		}
+	}
+	return false;
+}
+
+void wait_lapic_expire(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->arch.apic;

arch/x86/kvm/lapic.c +15 -1

+	tsc_deadline = apic->lapic_timer.expired_tscdeadline;
+	apic->lapic_timer.expired_tscdeadline = 0;
+	guest_tsc = kvm_x86_ops->read_l1_tsc(vcpu, native_read_tsc());
+
+	while (guest_tsc < tsc_deadline) {
+		int delay = min(tsc_deadline - guest_tsc, 1000ULL);
+
+		__delay(delay);
+		guest_tsc = kvm_x86_ops->read_l1_tsc(vcpu, native_read_tsc());
+	}
 }
 
 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)) {

@@ -1137,6 +1192,7 @@ static void start_apic_timer(struct kvm_
 		/* lapic timer in tsc deadline mode */
 		u64 guest_tsc, tscdeadline = apic->lapic_timer.tscdeadline;
 		u64 ns = 0;
+		ktime_t expire;
 		struct kvm_vcpu *vcpu = apic->vcpu;
 		unsigned long this_tsc_khz = vcpu->arch.virtual_tsc_khz;
 		unsigned long flags;

@@ -1151,8 +1207,10 @@ static void start_apic_timer(struct kvm_
 		if (likely(tscdeadline > guest_tsc)) {
 			ns = (tscdeadline - guest_tsc) * 1000000ULL;
 			do_div(ns, this_tsc_khz);
+			expire = ktime_add_ns(now, ns);
+			expire = ktime_sub_ns(expire, lapic_timer_advance_ns);
 			hrtimer_start(&apic->lapic_timer.timer,
-				ktime_add_ns(now, ns), HRTIMER_MODE_ABS);
+				      expire, HRTIMER_MODE_ABS);
 		} else
 			apic_timer_expired(apic);
 

Index: kvm/arch/x86/kvm/lapic.h
===================================================================

arch/x86/kvm/lapic.h +3 -0

@@ -14,6 +14,7 @@ struct kvm_timer {
 	u32 timer_mode;
 	u32 timer_mode_mask;
 	u64 tscdeadline;
+	u64 expired_tscdeadline;
 	atomic_t pending;			/* accumulated triggered timers */
 };
 

@@ -170,4 +171,6 @@ static inline bool kvm_apic_has_events(s
 
 bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector);
 
+void wait_lapic_expire(struct kvm_vcpu *vcpu);
+
 #endif

Index: kvm/arch/x86/kvm/x86.c
===================================================================

arch/x86/kvm/x86.c +5 -0

@@ -108,6 +108,10 @@ EXPORT_SYMBOL_GPL(kvm_max_guest_tsc_khz)
 static u32 tsc_tolerance_ppm = 250;
 module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR);
 
+/* lapic timer advance (tscdeadline mode only) in nanoseconds */
+unsigned int lapic_timer_advance_ns = 0;
+module_param(lapic_timer_advance_ns, uint, S_IRUGO | S_IWUSR);
+
 static bool backwards_tsc_observed = false;
 
 #define KVM_NR_SHARED_MSRS 16

@@ -6311,6 +6315,7 @@ static int vcpu_enter_guest(struct kvm_v
 	}
 
 	trace_kvm_entry(vcpu->vcpu_id);
+	wait_lapic_expire(vcpu);
 	kvm_x86_ops->run(vcpu);
 
 	/*

Index: kvm/arch/x86/kvm/x86.h
===================================================================

arch/x86/kvm/x86.h +2 -1

@@ -170,5 +170,7 @@ extern u64 kvm_supported_xcr0(void);
 
 extern unsigned int min_timer_period_us;
 
+extern unsigned int lapic_timer_advance_ns;
+
 extern struct static_key kvm_no_apic_vcpu;
 #endif
 
 
--

To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 73 lines · Marcelo Tosatti

For the hrtimer which emulates the tscdeadline timer in the guest, add an option to advance expiration, and busy spin on VM-entry waiting for the actual expiration time to elapse.

This allows achieving low latencies in cyclictest (or any scenario which requires strict timing regarding timer expiration).

Reduces average cyclictest latency from 12us to 8us on Core i5 desktop.

Note: this option requires tuning to find the appropriate value for a particular hardware/guest combination. One method is to measure the average delay between apic_timer_fn and VM-entry.
Another method is to start with 1000ns, and increase the value in say 500ns increments until avg cyclictest numbers stop decreasing.

Signed-off-by Marcelo Tosatti <[email protected]>

Index: kvm/arch/x86/kvm/lapic.c
===================================================================

arch/x86/kvm/lapic.c +33 -0

@@ -33,6 +33,7 @@
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/apicdef.h>
+#include <asm/delay.h>
 #include <linux/atomic.h>
 #include <linux/jump_label.h>
 #include "kvm_cache_regs.h"

@@ -1073,6 +1074,7 @@ static void apic_timer_expired(struct kv
 {
 	struct kvm_vcpu *vcpu = apic->vcpu;
 	wait_queue_head_t *q = &vcpu->wq;
+	struct kvm_timer *ktimer = &apic->lapic_timer;
 
 	/*
 	 * Note: KVM_REQ_PENDING_TIMER is implicitly checked in

@@ -1087,11 +1089,64 @@ static void apic_timer_expired(struct kv
 
 	if (waitqueue_active(q))
 		wake_up_interruptible(q);
+
+	if (apic_lvtt_tscdeadline(apic))
+		ktimer->expired_tscdeadline = ktimer->tscdeadline;
+}
+
+/*
+ * On APICv, this test will cause a busy wait
+ * during a higher-priority task.
+ */
+
+static bool lapic_timer_int_injected(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->arch.apic;
+	u32 reg = kvm_apic_get_reg(apic, APIC_LVTT);
+
+	if (kvm_apic_hw_enabled(apic)) {
+		int vec = reg & APIC_VECTOR_MASK;
+
+		if (kvm_x86_ops->test_posted_interrupt)
+			return kvm_x86_ops->test_posted_interrupt(vcpu, vec);
+		else {
+			if (apic_test_vector(vec, apic->regs + APIC_ISR))
+				return true;
+		}
+	}
+	return false;
+}
+
+void wait_lapic_expire(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->arch.apic;

arch/x86/kvm/lapic.c static void apic_timer_expired(struct kv · line 1132

Quoted 10 lines

arch/x86/kvm/lapic.c +10 -0

+	u64 guest_tsc, tsc_deadline;
+
+	if (!kvm_vcpu_has_lapic(vcpu))
+		return;
+
+	if (apic->lapic_timer.expired_tscdeadline == 0)
+		return;
+
+	if (!lapic_timer_int_injected(vcpu))
+		return;

By the time we get here, I think, if expired_tscdeadline != 0 we're sure that the interrupt has been injected. It may be in IRR rather than ISR, but at least on APICv the last test should be redundant.

So perhaps you can get rid of patch 1 and check
kvm_apic_vid_enabled(vcpu->kvm):

	if (k_a_v_e(vcpu->kvm)
		return true;
	if (apic_test_vector(vec, apic->regs + APIC_ISR))
		return true;

Does this sound correct?

Quoted 8 lines

Quoted 8 lines

So perhaps you can get rid of patch 1 and check
kvm_apic_vid_enabled(vcpu->kvm):

	if (k_a_v_e(vcpu->kvm)
		return true;
	if (apic_test_vector(vec, apic->regs + APIC_ISR))
		return true;

Quoted 1 line

Quoted 1 line

Does this sound correct?

* expired_tscdeadline != 0.
* APIC timer interrupt delivery masked at LVTT register.

Implies expired_tscdeadline != 0 and interrupt not injected.

Add tracepoint to wait_lapic_expire.

Signed-off-by Marcelo Tosatti <[email protected]>

Index: kvm/arch/x86/kvm/lapic.c
===================================================================

arch/x86/kvm/lapic.c +1 -0

@@ -1121,6 +1121,7 @@ void wait_lapic_expire(struct kvm_vcpu *
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 	u64 guest_tsc, tsc_deadline;
+	unsigned int total_delay = 0;
 
 	if (!kvm_vcpu_has_lapic(vcpu))
 		return;

arch/x86/kvm/lapic.c +0 -0

 }
 
 static void start_apic_timer(struct kvm_lapic *apic)

Index: kvm/arch/x86/kvm/trace.h
===================================================================

arch/x86/kvm/trace.h +19 -1

@@ -914,6 +914,25 @@ TRACE_EVENT(kvm_pvclock_update,
 		  __entry->flags)
 );
 
+TRACE_EVENT(kvm_wait_lapic_expire,
+	TP_PROTO(unsigned int vcpu_id, unsigned int total_delay),
+	TP_ARGS(vcpu_id, total_delay),
+
+	TP_STRUCT__entry(
+		__field(	unsigned int,	vcpu_id			)
+		__field(	unsigned int,	total_delay		)
+	),
+
+	TP_fast_assign(
+		__entry->vcpu_id	   = vcpu_id;
+		__entry->total_delay	   = total_delay;
+	),
+
+	TP_printk("vcpu %u: total_delay %u",
+		  __entry->vcpu_id,
+		  __entry->total_delay)
+);
+
 #endif /* _TRACE_KVM_H */
 
 #undef TRACE_INCLUDE_PATH
 
 
--

To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 17 lines · Marcelo Tosatti

Add tracepoint to wait_lapic_expire.

Signed-off-by Marcelo Tosatti <[email protected]>

Index: kvm/arch/x86/kvm/lapic.c
===================================================================

arch/x86/kvm/lapic.c +1 -0

@@ -1121,6 +1121,7 @@ void wait_lapic_expire(struct kvm_vcpu *
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 	u64 guest_tsc, tsc_deadline;
+	unsigned int total_delay = 0;
 
 	if (!kvm_vcpu_has_lapic(vcpu))
 		return;

arch/x86/kvm/lapic.c void wait_lapic_expire(struct kvm_vcpu * · line 1148

Quoted 10 lines

arch/x86/kvm/lapic.c +4 -0

@@ -1138,9 +1139,13 @@ void wait_lapic_expire(struct kvm_vcpu *
 	while (guest_tsc < tsc_deadline) {
 		int delay = min(tsc_deadline - guest_tsc, 1000ULL);
 
+		total_delay += delay;
+
 		__delay(delay);
 		guest_tsc = kvm_x86_ops->read_l1_tsc(vcpu, native_read_tsc());
 	}
+
+	trace_kvm_wait_lapic_expire(vcpu->vcpu_id, total_delay);

Let's add guest_tsc - tsc_deadline to the tracepoint. This should simplify the tuning of the parameter.

Paolo

Quoted 2 lines

Let's add guest_tsc - tsc_deadline to the tracepoint. This should simplify the tuning of the parameter.

total_delay is very close to that, except the summands are

    1000 + 1000 + ... + "remainder"

Yes?

BTW its very easy to tune the parameter with the kvm-unit-test test (the optimal value is clear). I'll write a document.