Re: [PATCH v2 2/7] KVM: lapic: Convert guest TSC to host time domain when delaying

From: Liran Alon <liran.alon@oracle.com>
To: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Radim Krčmář" <rkrcmar@redhat.com>,
	kvm@vger.kernel.org, "Wanpeng Li" <wanpengli@tencent.com>
Subject: Re: [PATCH v2 2/7] KVM: lapic: Convert guest TSC to host time domain when delaying
Date: Tue, 16 Apr 2019 21:35:29 +0300	[thread overview]
Message-ID: <75E0B1AD-A287-467D-A954-D51412C7E442@oracle.com> (raw)
In-Reply-To: <20190416174857.506-3-sean.j.christopherson@intel.com>


> On 16 Apr 2019, at 20:48, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> 
> To minimize the latency of timer interrupts as observed by the guest,
> KVM adjusts the values it programs into the host timers to account for
> the host's overhead of programming and handling the timer event.  In
> the event that the adjustments are too aggressive, i.e. the timer fires
> earlier than the guest expects, KVM busy waits immediately prior to
> entering the guest.
> 
> Currently, KVM manually converts the delay from nanoseconds to clock
> cycles.  But, the conversion is done in the guest's time domain, while
> the delay occurs in the host's time domain, i.e. the delay may not be
> accurate and could wait too little or too long.
> 
> Convert the delay from guest clock cycles to host nanoseconds and use
> ndelay() instead of __delay() to provide more accurate timing.  This
> also avoids the need to convert lapic_timer_advance_ns, which is used
> to cap the delay, to guest clock cycles.
> 
> Cc: Liran Alon <liran.alon@oracle.com>
> Cc: Wanpeng Li <wanpengli@tencent.com>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>

Reviewed-by: Liran Alon <liran.alon@oracle.com>

> ---
> arch/x86/kvm/lapic.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 92446cba9b24..5891c0badfa6 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1502,10 +1502,12 @@ void wait_lapic_expire(struct kvm_vcpu *vcpu)
> 	guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc());
> 	trace_kvm_wait_lapic_expire(vcpu->vcpu_id, guest_tsc - tsc_deadline);
> 
> -	/* __delay is delay_tsc whenever the hardware has TSC, thus always.  */
> -	if (guest_tsc < tsc_deadline)
> -		__delay(min(tsc_deadline - guest_tsc,
> -			nsec_to_cycles(vcpu, lapic_timer_advance_ns)));
> +	/* ndelay uses delay_tsc whenever the hardware has TSC, thus always. */
> +	if (guest_tsc < tsc_deadline) {
> +		ns = (tsc_deadline - guest_tsc) * 1000000ULL;
> +		do_div(ns, vcpu->arch.virtual_tsc_khz);
> +		ndelay(min_t(u32, ns, lapic_timer_advance_ns));
> +	}
> 
> 	if (!lapic_timer_advance_adjust_done) {
> 		/* too early */
> -- 
> 2.21.0
>