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

Groups > linux.kernel > #1500193 > unrolled thread

[PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support

Started byWanpeng Li <kernellwp@gmail.com>
First post2016-10-13 13:40 +0200
Last post2016-10-14 03:30 +0200
Articles 3 — 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 V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li <kernellwp@gmail.com> - 2016-10-13 13:40 +0200
    Re: [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot  mode VMX preemption timer support Paolo Bonzini <pbonzini@redhat.com> - 2016-10-13 14:40 +0200
      Re: [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot  mode VMX preemption timer support Wanpeng Li <kernellwp@gmail.com> - 2016-10-14 03:30 +0200

#1500193 — [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support

FromWanpeng Li <kernellwp@gmail.com>
Date2016-10-13 13:40 +0200
Subject[PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support
Message-ID<srMwF-2X1-7@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. 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

[toc] | [next] | [standalone]

#1500242 — Re: [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support

FromPaolo Bonzini <pbonzini@redhat.com>
Date2016-10-13 14:40 +0200
SubjectRe: [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support
Message-ID<srNsJ-3wv-19@gated-at.bofh.it>
In reply to#1500193

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);
>  	}
>  }
>

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

#1500664 — Re: [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support

FromWanpeng Li <kernellwp@gmail.com>
Date2016-10-14 03:30 +0200
SubjectRe: [PATCH RFC V3 6/6] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support
Message-ID<srZtT-2Sz-5@gated-at.bofh.it>
In reply to#1500242
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);
>>       }
>>  }
>>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web