Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1499386 > unrolled thread

[PATCH RFC V2 2/2] KVM: x86: Support using the VMX preemption timer for APIC Timer periodic/oneshot mode

Started byWanpeng Li <kernellwp@gmail.com>
First post2016-10-12 09:00 +0200
Last post2016-10-13 14:00 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH RFC V2 2/2] KVM: x86: Support using the VMX preemption timer for APIC Timer periodic/oneshot mode Wanpeng Li <kernellwp@gmail.com> - 2016-10-12 09:00 +0200
    Re: [PATCH RFC V2 2/2] KVM: x86: Support using the VMX preemption  timer for APIC Timer periodic/oneshot mode Radim Krčmář <rkrcmar@redhat.com> - 2016-10-12 19:50 +0200
      Re: [PATCH RFC V2 2/2] KVM: x86: Support using the VMX preemption  timer for APIC Timer periodic/oneshot mode Wanpeng Li <kernellwp@gmail.com> - 2016-10-13 13:40 +0200
        Re: [PATCH RFC V2 2/2] KVM: x86: Support using the VMX preemption  timer for APIC Timer periodic/oneshot mode Wanpeng Li <kernellwp@gmail.com> - 2016-10-13 14:00 +0200

#1499386 — [PATCH RFC V2 2/2] KVM: x86: Support using the VMX preemption timer for APIC Timer periodic/oneshot mode

FromWanpeng Li <kernellwp@gmail.com>
Date2016-10-12 09:00 +0200
Subject[PATCH RFC V2 2/2] KVM: x86: Support using the VMX preemption timer for APIC Timer periodic/oneshot mode
Message-ID<srlGa-Ku-17@gated-at.bofh.it>
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.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Yunhong Jiang <yunhong.jiang@intel.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/kvm/lapic.c | 103 +++++++++++++++++++++++++++++++++++----------------
 arch/x86/kvm/lapic.h |   1 +
 2 files changed, 73 insertions(+), 31 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index dad743e..f5a03ae 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1101,7 +1101,14 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
 		apic->lapic_timer.period == 0)
 		return 0;
 
-	remaining = hrtimer_get_remaining(&apic->lapic_timer.timer);
+	if (kvm_lapic_hv_timer_in_use(apic->vcpu)) {
+		ktime_t now;
+
+		now = apic->lapic_timer.timer.base->get_time();
+		remaining = ktime_sub(apic->lapic_timer.expired_period, now);
+	} else
+		remaining = hrtimer_get_remaining(&apic->lapic_timer.timer);
+
 	if (ktime_to_ns(remaining) < 0)
 		remaining = ktime_set(0, 0);
 
@@ -1353,8 +1360,6 @@ static void start_sw_period(struct kvm_lapic *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;
@@ -1377,7 +1382,7 @@ static void start_sw_period(struct kvm_lapic *apic)
 	}
 
 	hrtimer_start(&apic->lapic_timer.timer,
-		      ktime_add_ns(now, apic->lapic_timer.period),
+		      apic->lapic_timer.expired_period,
 		      HRTIMER_MODE_ABS_PINNED);
 
 	apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
@@ -1400,52 +1405,71 @@ 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;
 }
 
-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_tscdeadline(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) ||
+	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_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);
+		if (atomic_read(&apic->lapic_timer.pending) &&
+			!apic_lvtt_period(apic))
+			cancel_hv_timer(apic);
 	}
 	trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
 			apic->lapic_timer.hv_timer_in_use);
 	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)) {
+		ktime_t now;
+		u64 tscl = rdtsc();
+
+		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;
+
+		apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
+			nsec_to_cycles(apic->vcpu, apic->lapic_timer.period);
+		now = apic->lapic_timer.timer.base->get_time();
+		apic->lapic_timer.expired_period = ktime_add_ns(now, apic->lapic_timer.period);
+
+		start_hv_timer(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_tscdeadline(apic);
+	start_hv_timer(apic);
 }
 EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_hv_timer);
 
@@ -1457,12 +1481,15 @@ 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;
 
-	start_sw_tscdeadline(apic);
+	if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic))
+		start_sw_period(apic);
+	else if (apic_lvtt_tscdeadline(apic))
+		start_sw_tscdeadline(apic);
 }
 EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_sw_timer);
 
@@ -1470,10 +1497,25 @@ 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 (!(kvm_x86_ops->set_hv_timer && start_hv_tscdeadline(apic)))
+	if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
+		ktime_t now;
+		u64 tscl = rdtsc();
+
+		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;
+
+		apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
+			nsec_to_cycles(apic->vcpu, apic->lapic_timer.period);
+		now = apic->lapic_timer.timer.base->get_time();
+		apic->lapic_timer.expired_period = ktime_add_ns(now, apic->lapic_timer.period);
+
+		if (!(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);
 	}
 }
@@ -1711,8 +1753,7 @@ 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))
 		return 0;
 
 	return apic->lapic_timer.tscdeadline;
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index f60d01c..fea7602 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 expired_period;
 	u32 timer_mode;
 	u32 timer_mode_mask;
 	u64 tscdeadline;
-- 
1.9.1

[toc] | [next] | [standalone]

#1499851 — Re: [PATCH RFC V2 2/2] KVM: x86: Support using the VMX preemption timer for APIC Timer periodic/oneshot mode

FromRadim Krčmář <rkrcmar@redhat.com>
Date2016-10-12 19:50 +0200
SubjectRe: [PATCH RFC V2 2/2] KVM: x86: Support using the VMX preemption timer for APIC Timer periodic/oneshot mode
Message-ID<srvPb-7zM-13@gated-at.bofh.it>
In reply to#1499386
2016-10-12 14:52+0800, Wanpeng Li:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> Most windows guests still utilize APIC Timer periodic/oneshot mode 
> instead of tsc-deadline mode, and the APIC Timer periodic/oneshot 
> mode are still emulated by high overhead hrtimer on host. This patch 
> converts the expected expire time of the periodic/oneshot mode to
> guest deadline tsc in order to leverage VMX preemption timer logic 
> for APIC Timer tsc-deadline mode.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Yunhong Jiang <yunhong.jiang@intel.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> @@ -1101,7 +1101,14 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
>  		apic->lapic_timer.period == 0)
>  		return 0;
>  
> -	remaining = hrtimer_get_remaining(&apic->lapic_timer.timer);
> +	if (kvm_lapic_hv_timer_in_use(apic->vcpu)) {

apic->lapic_timer.expired_period is set unconditionally, so the
condition seems superfluous.

> +		ktime_t now;
> +
> +		now = apic->lapic_timer.timer.base->get_time();
> +		remaining = ktime_sub(apic->lapic_timer.expired_period, now);

The name expired_period is weird for something that can be in the future
... maybe target_ns?

> +	} else
> +		remaining = hrtimer_get_remaining(&apic->lapic_timer.timer);
> +
>  	if (ktime_to_ns(remaining) < 0)
>  		remaining = ktime_set(0, 0);
>  
> @@ -1353,8 +1360,6 @@ static void start_sw_period(struct kvm_lapic *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;

I would gut start_sw_period() a bit more: the checking of minimal period
is a bit misplaced, because it uses now() when the timer switched to
hrtimers, but that could have been long after the timer was set, so we'd
delay for no good reason.

>  	if (!apic->lapic_timer.period)
>  		return;

Can this happen?  (WARN_ONCE() if it is only a sanity check.)

> @@ -1400,52 +1405,71 @@ bool kvm_lapic_hv_timer_in_use(struct kvm_vcpu *vcpu)
> +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)) {
> +		ktime_t now;
> +		u64 tscl = rdtsc();
> +
> +		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;
> +
> +		apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
> +			nsec_to_cycles(apic->vcpu, apic->lapic_timer.period);
> +		now = apic->lapic_timer.timer.base->get_time();
> +		apic->lapic_timer.expired_period = ktime_add_ns(now, apic->lapic_timer.period);
> +
> +		start_hv_timer(apic);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer);
> +
> @@ -1470,10 +1497,25 @@ 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 (!(kvm_x86_ops->set_hv_timer && start_hv_tscdeadline(apic)))
> +	if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
> +		ktime_t now;
> +		u64 tscl = rdtsc();
> +
> +		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;
> +		apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
> +			nsec_to_cycles(apic->vcpu, apic->lapic_timer.period);
> +		now = apic->lapic_timer.timer.base->get_time();
> +		apic->lapic_timer.expired_period = ktime_add_ns(now, apic->lapic_timer.period);

This is the same code as in kvm_lapic_expired_hv_timer(), a helper
function is in order.

> +
> +		if (!(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);
>  	}
>  }
> @@ -1711,8 +1753,7 @@ 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))

rdmsr MSR_IA32_TSCDEADLINE has to return 0 when the timer is not in tsc
deadline mode, so the condition should stay.  (and check for
apic_lvtt_tscdeadline(), but that is a unrelated cleanup.)

> +	if (!lapic_in_kernel(vcpu))
>  		return 0;
>  
>  	return apic->lapic_timer.tscdeadline;

Thanks.

[toc] | [prev] | [next] | [standalone]

#1500200 — Re: [PATCH RFC V2 2/2] KVM: x86: Support using the VMX preemption timer for APIC Timer periodic/oneshot mode

FromWanpeng Li <kernellwp@gmail.com>
Date2016-10-13 13:40 +0200
SubjectRe: [PATCH RFC V2 2/2] KVM: x86: Support using the VMX preemption timer for APIC Timer periodic/oneshot mode
Message-ID<srMwG-2X1-29@gated-at.bofh.it>
In reply to#1499851
2016-10-13 1:41 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
> 2016-10-12 14:52+0800, Wanpeng Li:
>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>
>> Most windows guests still utilize APIC Timer periodic/oneshot mode
>> instead of tsc-deadline mode, and the APIC Timer periodic/oneshot
>> mode are still emulated by high overhead hrtimer on host. This patch
>> converts the expected expire time of the periodic/oneshot mode to
>> guest deadline tsc in order to leverage VMX preemption timer logic
>> for APIC Timer tsc-deadline mode.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: Yunhong Jiang <yunhong.jiang@intel.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> @@ -1101,7 +1101,14 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
>>               apic->lapic_timer.period == 0)
>>               return 0;
>>
>> -     remaining = hrtimer_get_remaining(&apic->lapic_timer.timer);
>> +     if (kvm_lapic_hv_timer_in_use(apic->vcpu)) {
>
> apic->lapic_timer.expired_period is set unconditionally, so the
> condition seems superfluous.

Agreed.

>
>> +             ktime_t now;
>> +
>> +             now = apic->lapic_timer.timer.base->get_time();
>> +             remaining = ktime_sub(apic->lapic_timer.expired_period, now);
>
> The name expired_period is weird for something that can be in the future
> ... maybe target_ns?

How about target_expiration? It should be ktime_t as the second
parameter of hrtimer_start().

>
>> +     } else
>> +             remaining = hrtimer_get_remaining(&apic->lapic_timer.timer);
>> +
>>       if (ktime_to_ns(remaining) < 0)
>>               remaining = ktime_set(0, 0);
>>
>> @@ -1353,8 +1360,6 @@ static void start_sw_period(struct kvm_lapic *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;
>
> I would gut start_sw_period() a bit more: the checking of minimal period
> is a bit misplaced, because it uses now() when the timer switched to
> hrtimers, but that could have been long after the timer was set, so we'd
> delay for no good reason.

Agreed.

>
>>       if (!apic->lapic_timer.period)
>>               return;
>
> Can this happen?  (WARN_ONCE() if it is only a sanity check.)

Actually I catch several apic->lapic_timer.period == 0 here, in
addition, the oneshot mode test of kvm-unit-tests/apic.flat will fail
if change it to WARN_ONCE() instead of return directly.

>
>> @@ -1400,52 +1405,71 @@ bool kvm_lapic_hv_timer_in_use(struct kvm_vcpu *vcpu)
>> +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)) {
>> +             ktime_t now;
>> +             u64 tscl = rdtsc();
>> +
>> +             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;
>> +
>> +             apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
>> +                     nsec_to_cycles(apic->vcpu, apic->lapic_timer.period);
>> +             now = apic->lapic_timer.timer.base->get_time();
>> +             apic->lapic_timer.expired_period = ktime_add_ns(now, apic->lapic_timer.period);
>> +
>> +             start_hv_timer(apic);
>> +     }
>> +}
>> +EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer);
>> +
>> @@ -1470,10 +1497,25 @@ 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 (!(kvm_x86_ops->set_hv_timer && start_hv_tscdeadline(apic)))
>> +     if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
>> +             ktime_t now;
>> +             u64 tscl = rdtsc();
>> +
>> +             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;
>> +             apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
>> +                     nsec_to_cycles(apic->vcpu, apic->lapic_timer.period);
>> +             now = apic->lapic_timer.timer.base->get_time();
>> +             apic->lapic_timer.expired_period = ktime_add_ns(now, apic->lapic_timer.period);
>
> This is the same code as in kvm_lapic_expired_hv_timer(), a helper
> function is in order.

Agreed.

>
>> +
>> +             if (!(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);
>>       }
>>  }
>> @@ -1711,8 +1753,7 @@ 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))
>
> rdmsr MSR_IA32_TSCDEADLINE has to return 0 when the timer is not in tsc
> deadline mode, so the condition should stay.  (and check for
> apic_lvtt_tscdeadline(), but that is a unrelated cleanup.)

Agreed. I  just sent out RFC V3 to handle these comments, thanks for
your active review, Radim. It is really helpful. :)

Regards,
Wanpeng Li

[toc] | [prev] | [next] | [standalone]

#1500218 — Re: [PATCH RFC V2 2/2] KVM: x86: Support using the VMX preemption timer for APIC Timer periodic/oneshot mode

FromWanpeng Li <kernellwp@gmail.com>
Date2016-10-13 14:00 +0200
SubjectRe: [PATCH RFC V2 2/2] KVM: x86: Support using the VMX preemption timer for APIC Timer periodic/oneshot mode
Message-ID<srMQ1-341-11@gated-at.bofh.it>
In reply to#1500200
2016-10-13 19:38 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
> 2016-10-13 1:41 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
>> 2016-10-12 14:52+0800, Wanpeng Li:

[...]

>>> @@ -1711,8 +1753,7 @@ 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))
>>
>> rdmsr MSR_IA32_TSCDEADLINE has to return 0 when the timer is not in tsc
>> deadline mode, so the condition should stay.  (and check for
>> apic_lvtt_tscdeadline(), but that is a unrelated cleanup.)

Actually I modify this function to get apic->lapic_timer.tscdeadline
in kvm_arch_vcpu_load() for periodic/oneshot mode, I introduce another
function to handle this in order to avoid break rdmsr
MSR_IA32_TSCDEADLINE sanity check in RFC V3.

Regards,
Wanpeng Li

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web