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

PATCH In review Marcelo Tosatti 3 participants 8 messages Dec 23, 2014 – Jan 8, 2015

Subsystems (3)

x86 KVM KVM x86

lore

Share & tools

lore

Marcelo Tosatti

author

2014-12-23 21:01 UTC #0 lore raw

cover KVM: add option to advance tscdeadline hrtimer expiration (v6)

See patches for details.

v2:
- fix email address.

v3:
- use module parameter for configuration of value (Paolo/Radim)

v4:
- fix check for tscdeadline mode while waiting for expiration (Paolo)
- use proper delay function (Radim)
- fix LVTT tscdeadline mode check in hrtimer interrupt handler (Radim)
- add comment regarding PPR and APICv (Paolo)

v5:
- use tscdeadline expiration and guest tsc difference in tracepoint (Paolo)

v6:
- Remove unnecessary delay loop (Radim)
- Disable feature if __delay is not TSC based (Paolo)

Marcelo Tosatti

author

2014-12-23 21:01 UTC #1 lore raw

PATCH 1/3 KVM: x86: add method to test PIR bitmap vector

kvm_x86_ops->test_posted_interrupt() returns true/false depending whether 'vector' is set.

Next patch makes use of this interface.

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

arch/x86/include/asm/kvm_host.h +1 -0

Index: kvm/arch/x86/include/asm/kvm_host.h
===================================================================
@@ -743,6 +743,7 @@ struct kvm_x86_ops {
 	void (*set_virtual_x2apic_mode)(struct kvm_vcpu *vcpu, bool set);
 	void (*set_apic_access_page_addr)(struct kvm_vcpu *vcpu, hpa_t hpa);
 	void (*deliver_posted_interrupt)(struct kvm_vcpu *vcpu, int vector);
+	bool (*test_posted_interrupt)(struct kvm_vcpu *vcpu, int vector);
 	void (*sync_pir_to_irr)(struct kvm_vcpu *vcpu);
 	int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
 	int (*get_tdp_level)(void);

arch/x86/kvm/vmx.c +14 -0

Index: kvm/arch/x86/kvm/vmx.c
===================================================================
@@ -435,6 +435,11 @@ static int pi_test_and_set_pir(int vecto
 	return test_and_set_bit(vector, (unsigned long *)pi_desc->pir);
 }
 
+static int pi_test_pir(int vector, struct pi_desc *pi_desc)
+{
+	return test_bit(vector, (unsigned long *)pi_desc->pir);
+}
+
 struct vcpu_vmx {
 	struct kvm_vcpu       vcpu;
 	unsigned long         host_rsp;

@@ -5939,6 +5944,7 @@ static __init int hardware_setup(void)
 	else {
 		kvm_x86_ops->hwapic_irr_update = NULL;
 		kvm_x86_ops->deliver_posted_interrupt = NULL;
+		kvm_x86_ops->test_posted_interrupt = NULL;
 		kvm_x86_ops->sync_pir_to_irr = vmx_sync_pir_to_irr_dummy;
 	}

@@ -6960,6 +6966,13 @@ static int handle_invvpid(struct kvm_vcp
 	return 1;
 }
 
+static bool vmx_test_pir(struct kvm_vcpu *vcpu, int vector)
+{
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
+
+	return pi_test_pir(vector, &vmx->pi_desc);
+}
+
 /*
  * The exit handlers return 1 if the exit was handled fully and guest execution
  * may resume.  Otherwise they set the kvm_run parameter to indicate what needs

@@ -9374,6 +9387,7 @@ static struct kvm_x86_ops vmx_x86_ops =
 	.hwapic_isr_update = vmx_hwapic_isr_update,
 	.sync_pir_to_irr = vmx_sync_pir_to_irr,
 	.deliver_posted_interrupt = vmx_deliver_posted_interrupt,
+	.test_posted_interrupt = vmx_test_pir,
 
 	.set_tss_addr = vmx_set_tss_addr,
 	.get_tdp_level = get_ept_level,

Marcelo Tosatti

author

2014-12-23 21:01 UTC #2 lore raw

PATCH 2/3 KVM: x86: add option to advance tscdeadline hrtimer expiration

In reply to 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]>

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

Index: kvm/arch/x86/kvm/lapic.c
===================================================================
@@ -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,60 @@ 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;
+	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;
+
+	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());
+
+	if (guest_tsc < tsc_deadline)
+		__delay(tsc_deadline - guest_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 +1188,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 +1203,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);

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

Index: kvm/arch/x86/kvm/lapic.h
===================================================================
@@ -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

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

Index: kvm/arch/x86/kvm/x86.c
===================================================================
@@ -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

@@ -5625,6 +5629,10 @@ static void kvm_timer_init(void)
 	__register_hotcpu_notifier(&kvmclock_cpu_notifier_block);
 	cpu_notifier_register_done();
 
+	if (check_tsc_unstable() && lapic_timer_advance_ns) {
+		pr_info("kvm: unstable TSC, disabling lapic_timer_advance_ns\n");
+		lapic_timer_advance_ns = 0;
+	}
 }
 
 static DEFINE_PER_CPU(struct kvm_vcpu *, current_vcpu);

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

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

Index: kvm/arch/x86/kvm/x86.h
===================================================================
@@ -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

Radim Krcmar

Reviewed-by on v1 2/3

2015-01-05 18:12 UTC #3 lore raw

Re: [patch 2/3] KVM: x86: add option to advance tscdeadline hrtimer expiration

2014-12-23 15:58-0500, Marcelo Tosatti:

Quoted 10 lines

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.

Quoted 7 lines

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]>

Reviewed-by Radim Krčmář <[email protected]>

(Other patches weren't touched, so my previous Reviewed-by holds.)

Quoted 13 lines

+++ kvm/arch/x86/kvm/x86.c

arch/x86/kvm/x86.c +4 -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

arch/x86/kvm/x86.c static void kvm_timer_init(void) · line 5634

Quoted 6 lines

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

@@ -5625,6 +5629,10 @@ static void kvm_timer_init(void)
 	__register_hotcpu_notifier(&kvmclock_cpu_notifier_block);
 	cpu_notifier_register_done();
 
+	if (check_tsc_unstable() && lapic_timer_advance_ns) {
+		pr_info("kvm: unstable TSC, disabling lapic_timer_advance_ns\n");
+		lapic_timer_advance_ns = 0;

Does unstable TSC invalidate this feature? (lapic_timer_advance_ns can be overridden, so we don't differentiate
workflows that calibrate after starting with 0.)

And cover letter is a bit misleading: The condition does nothing to guarantee TSC based __delay() loop. (Right now, __delay() = delay_tsc() whenever the hardware has TSC, regardless of stability, thus always.)

Radim Krcmar

2015-01-05 18:20 UTC #4 lore raw

Re: [patch 2/3] KVM: x86: add option to advance tscdeadline hrtimer expiration

2015-01-05 19:12+0100, Radim Krcmar:

Quoted 2 lines

                                     (Right now, __delay() = delay_tsc()
whenever the hardware has TSC, regardless of stability, thus always.)

(For quantifiers' sake, there also is 'tsc_disabled' variable.)

Marcelo Tosatti

author

2015-01-08 17:41 UTC #5 lore raw

Re: [patch 2/3] KVM: x86: add option to advance tscdeadline hrtimer expiration

In reply to Radim Krcmar

Quoted 39 lines · Radim Krcmar

2014-12-23 15:58-0500, Marcelo Tosatti:
Quoted 10 lines
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.
Quoted 7 lines
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]>
Reviewed-by Radim Krčmář <[email protected]>
(Other patches weren't touched, so my previous Reviewed-by holds.)
Quoted 16 lines
+++ kvm/arch/x86/kvm/x86.c
arch/x86/kvm/x86.c +4 -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
@@ -5625,6 +5629,10 @@ static void kvm_timer_init(void)
 	__register_hotcpu_notifier(&kvmclock_cpu_notifier_block);
 	cpu_notifier_register_done();
 

Quoted 7 lines

arch/x86/kvm/x86.c static void kvm_timer_init(void) · line 5634
Quoted 3 lines
arch/x86/kvm/x86.c +3 -0
+	if (check_tsc_unstable() && lapic_timer_advance_ns) {
+		pr_info("kvm: unstable TSC, disabling lapic_timer_advance_ns\n");
+		lapic_timer_advance_ns = 0;
Does unstable TSC invalidate this feature? (lapic_timer_advance_ns can be overridden, so we don't differentiate
workflows that calibrate after starting with 0.)

Hum, i don't see why. Paolo?

Quoted 3 lines

And cover letter is a bit misleading: The condition does nothing to guarantee TSC based __delay() loop. (Right now, __delay() = delay_tsc() whenever the hardware has TSC, regardless of stability, thus always.)

OK.

Paolo Bonzini

maintainer

2015-01-08 21:44 UTC #6 lore raw

Re: [patch 2/3] KVM: x86: add option to advance tscdeadline hrtimer expiration

Quoted 7 lines · Marcelo Tosatti

Paolo?
Quoted 3 lines
And cover letter is a bit misleading: The condition does nothing to guarantee TSC based __delay() loop. (Right now, __delay() = delay_tsc() whenever the hardware has TSC, regardless of stability, thus always.)
OK.

Yes, because of this it is unnecessary to check for stable TSC. I'll remove that.

Paolo

Marcelo Tosatti

author

2014-12-23 21:01 UTC #7 lore raw

PATCH 3/3 KVM: x86: add tracepoint to wait_lapic_expire

In reply to Marcelo Tosatti

Add tracepoint to wait_lapic_expire.

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

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

Index: kvm/arch/x86/kvm/lapic.c
===================================================================
@@ -1137,6 +1137,8 @@ void wait_lapic_expire(struct kvm_vcpu *
 
 	if (guest_tsc < tsc_deadline)
 		__delay(tsc_deadline - guest_tsc);
+
+	trace_kvm_wait_lapic_expire(vcpu->vcpu_id, guest_tsc - tsc_deadline);
 }
 
 static void start_apic_timer(struct kvm_lapic *apic)

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

Index: kvm/arch/x86/kvm/trace.h
===================================================================
@@ -914,6 +914,25 @@ TRACE_EVENT(kvm_pvclock_update,
 		  __entry->flags)
 );
 
+TRACE_EVENT(kvm_wait_lapic_expire,
+	TP_PROTO(unsigned int vcpu_id, s64 delta),
+	TP_ARGS(vcpu_id, delta),
+
+	TP_STRUCT__entry(
+		__field(	unsigned int,	vcpu_id		)
+		__field(	s64,		delta		)
+	),
+
+	TP_fast_assign(
+		__entry->vcpu_id	   = vcpu_id;
+		__entry->delta             = delta;
+	),
+
+	TP_printk("vcpu %u: delta %lld",
+		  __entry->vcpu_id,
+		  __entry->delta)
+);
+
 #endif /* _TRACE_KVM_H */
 
 #undef TRACE_INCLUDE_PATH