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,

[email protected]

author

2014-12-10 17:47 UTC #2 lore raw

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

In reply to [email protected]

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 cyclictest avg latency by 50%.

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 +56 -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,59 @@ static void apic_timer_expired(struct kv
 
 	if (waitqueue_active(q))
 		wake_up_interruptible(q);
+
+	if (ktimer->timer_mode_mask == APIC_LVT_TIMER_TSCDEADLINE)
+		ktimer->expired_tscdeadline = ktimer->tscdeadline;
+}
+
+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_lvtt_tscdeadline(apic))
+		return;
+
+	if (!lapic_timer_int_injected(vcpu))
+		return;
+
+	tsc_deadline = apic->lapic_timer.expired_tscdeadline;
+	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);
+
+		ndelay(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;
+	struct kvm_arch *kvm_arch = &apic->vcpu->kvm->arch;
+
 	atomic_set(&apic->lapic_timer.pending, 0);
 
 	if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {

@@ -1137,6 +1187,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;

@@ -1149,10 +1200,14 @@ static void start_apic_timer(struct kvm_
 		now = apic->lapic_timer.timer.base->get_time();
 		guest_tsc = kvm_x86_ops->read_l1_tsc(vcpu, native_read_tsc());
 		if (likely(tscdeadline > guest_tsc)) {
+			u32 advance = kvm_arch->lapic_tscdeadline_advance_ns;
+
 			ns = (tscdeadline - guest_tsc) * 1000000ULL;
 			do_div(ns, this_tsc_khz);
+			expire = ktime_add_ns(now, ns);
+			expire = ktime_sub_ns(expire, advance);
 			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 +18 -0

Index: kvm/arch/x86/kvm/x86.c
===================================================================
@@ -2742,6 +2742,7 @@ int kvm_vm_ioctl_check_extension(struct
 	case KVM_CAP_ASSIGN_DEV_IRQ:
 	case KVM_CAP_PCI_2_3:
 #endif
+	case KVM_CAP_TSCDEADLINE_ADVANCE:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:

@@ -4079,6 +4080,22 @@ long kvm_arch_vm_ioctl(struct file *filp
 		r = 0;
 		break;
 	}
+	case KVM_SET_TSCDEADLINE_ADVANCE: {
+		struct kvm_tscdeadline_advance adv;
+
+		r = -EFAULT;
+		if (copy_from_user(&adv, argp, sizeof(adv)))
+			goto out;
+
+		/* cap at 50us to avoid spinning for too long */
+		r = -EINVAL;
+		if (adv.timer_advance > 50000)
+			goto out;
+
+		kvm->arch.lapic_tscdeadline_advance_ns = adv.timer_advance;
+		r = 0;
+		break;
+	}
 
 	default:
 		r = kvm_vm_ioctl_assigned_device(kvm, ioctl, arg);

@@ -6311,6 +6328,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/include/asm/kvm_host.h +2 -0

Index: kvm/arch/x86/include/asm/kvm_host.h
===================================================================
@@ -612,6 +612,8 @@ struct kvm_arch {
 	u64 hv_hypercall;
 	u64 hv_tsc_page;
 
+	u32 lapic_tscdeadline_advance_ns;
+
 	#ifdef CONFIG_KVM_MMU_AUDIT
 	int audit_point;
 	#endif

arch/x86/include/uapi/asm/kvm.h +5 -0

Index: kvm/arch/x86/include/uapi/asm/kvm.h
===================================================================
@@ -277,6 +277,11 @@ struct kvm_reinject_control {
 	__u8 reserved[31];
 };
 
+struct kvm_tscdeadline_advance {
+	__u32 timer_advance;
+	__u32 reserved[3];
+};
+
 /* When set in flags, include corresponding fields on KVM_SET_VCPU_EVENTS */
 #define KVM_VCPUEVENT_VALID_NMI_PENDING	0x00000001
 #define KVM_VCPUEVENT_VALID_SIPI_VECTOR	0x00000002

include/uapi/linux/kvm.h +3 -0

Index: kvm/include/uapi/linux/kvm.h
===================================================================
@@ -753,6 +753,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_PPC_FIXUP_HCALL 103
 #define KVM_CAP_PPC_ENABLE_HCALL 104
 #define KVM_CAP_CHECK_EXTENSION_VM 105
+#define KVM_CAP_TSCDEADLINE_ADVANCE 106
 
 #ifdef KVM_CAP_IRQ_ROUTING

@@ -1053,6 +1054,8 @@ struct kvm_s390_ucas_mapping {
 #define KVM_GET_DEVICE_ATTR	  _IOW(KVMIO,  0xe2, struct kvm_device_attr)
 #define KVM_HAS_DEVICE_ATTR	  _IOW(KVMIO,  0xe3, struct kvm_device_attr)
 
+#define KVM_SET_TSCDEADLINE_ADVANCE  _IOW(KVMIO,  0xe4, struct kvm_tscdeadline_advance)
+
 /*
  * ioctls for vcpu fds
  */

Paolo Bonzini

maintainer

2014-12-10 17:29 UTC #3 lore raw

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

Quoted 9 lines · On 10/12/2014 17:53, [email protected] wrote:

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 cyclictest avg latency by 50%.

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

What is the latency value that you find in practice, for both apic_timer_fn to vmentry? Or for apic_timer_fn to just before vmrun? Let's start with a kvm-unit-tests patch to measure this value.

We can then decide whether to hardcode a small default value (e.g. 1000-3000) and make it a module parameter? Or perhaps start with a higher value (twice what you find in practice?) and adjust it towards a target every time wait_lapic_expire is called. But in order to judge the correct approach, I need to see the numbers.

Paolo

Marcelo Tosatti

2014-12-10 17:34 UTC #4 lore raw

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

Quoted 18 lines · Paolo Bonzini

Quoted 15 lines · On 10/12/2014 17:53, [email protected] wrote:
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 cyclictest avg latency by 50%.
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.

Quoted 4 lines

Quoted 1 line
Signed-off-by Marcelo Tosatti <[email protected]>
What is the latency value that you find in practice, for both apic_timer_fn to vmentry? Or for apic_timer_fn to just before vmrun?

7us between apic_timer_fn and kvm_entry tracepoint.

Quoted 1 line

Let's start with a kvm-unit-tests patch to measure this value.

I can, but kvm-unit-test register state will not be similar to actual guest state (think host/guest state loading).

What is the advantage of using a kvm-unit-test test rather than cyclictest in the guest ?

Quoted 5 lines

We can then decide whether to hardcode a small default value (e.g. 1000-3000) and make it a module parameter? Or perhaps start with a higher value (twice what you find in practice?) and adjust it towards a target every time wait_lapic_expire is called. But in order to judge the correct approach, I need to see the numbers.

Problem with automatic adjustment is: what is the correct target?

You want faster instances of apic_timer_fn->vm-entry to spin a bit, and allow slow instances of apic_timer_fn->vm-entry to have an effective advancement.

Paolo Bonzini

maintainer

2014-12-10 17:53 UTC #5 lore raw

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

Quoted 3 lines · Marcelo Tosatti

Quoted 1 line
Quoted 1 line
Let's start with a kvm-unit-tests patch to measure this value.
I can, but kvm-unit-test register state will not be similar to actual guest state (think host/guest state loading).

7us is about 20000 clock cycles. A lightweight vmexit is an order of magnitude less expensive, and half of the vmexit overhead is the VMRUN instruction itself. All in all, the host/guest state loading should not matter (or should matter little).

Quoted 2 lines

What is the advantage of using a kvm-unit-test test rather than cyclictest in the guest ?

That it starts in <3 seconds, and that you can vary the timer frequency in order to measure jitter in addition to latency.

Quoted 7 lines

Quoted 5 lines
We can then decide whether to hardcode a small default value (e.g. 1000-3000) and make it a module parameter? Or perhaps start with a higher value (twice what you find in practice?) and adjust it towards a target every time wait_lapic_expire is called. But in order to judge the correct approach, I need to see the numbers.
Problem with automatic adjustment is: what is the correct target?

We cannot say without seeing the numbers, particularly the jitter. This is why I want to see numbers for varying frequencies (from 100us to 10ms per tick, say).

Quoted 3 lines

You want faster instances of apic_timer_fn->vm-entry to spin a bit, and allow slow instances of apic_timer_fn->vm-entry to have an effective advancement.

If it is small enogh, you can make the timer a little "early" (increase advance) by a small amount on every delivered interrupt. This prepares for a slow instance.

And you can make the timer less "early" (decrease advance) by some percentage of what you had to wait on every wait_lapic_expire, if you had to wait more than a given threshold. This avoids that you wait too much on consecutive fast instances.

Paolo