From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [RFC PATCH V3 3/5] Utilize the vmx preemption timer
Date: Mon, 6 Jun 2016 14:37:47 +0200
Message-ID: <00a2da92-e504-41a1-a0a0-65836989051f@redhat.com>
References: <1465000951-13343-1-git-send-email-yunhong.jiang@linux.intel.com>
 <1465000951-13343-4-git-send-email-yunhong.jiang@linux.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
Cc: mtosatti@redhat.com, rkrcmar@redhat.com, kernellwp@gmail.com
To: Yunhong Jiang <yunhong.jiang@linux.intel.com>, kvm@vger.kernel.org
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-wm0-f68.google.com ([74.125.82.68]:36601 "EHLO
	mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1750891AbcFFMhw (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 6 Jun 2016 08:37:52 -0400
Received: by mail-wm0-f68.google.com with SMTP id m124so15052751wme.3
        for <kvm@vger.kernel.org>; Mon, 06 Jun 2016 05:37:52 -0700 (PDT)
In-Reply-To: <1465000951-13343-4-git-send-email-yunhong.jiang@linux.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>



On 04/06/2016 02:42, Yunhong Jiang wrote:
> +static int vmx_set_hv_timer(struct kvm_vcpu *vcpu, u64 guest_deadline_tsc)
> +{
> +	u64 tscl = rdtsc(), delta_tsc;
> +
> +	delta_tsc = guest_deadline_tsc - kvm_read_l1_tsc(vcpu, tscl);
> +
> +	/* Convert to host delta tsc if tsc scaling is enabled */
> +	if (vcpu->arch.tsc_scaling_ratio &&
> +			u64_shl_div_u64(delta_tsc,
> +				kvm_tsc_scaling_ratio_frac_bits,
> +				vcpu->arch.tsc_scaling_ratio,
> +				&delta_tsc))
> +		return -1;

Please return -EOVERFLOW or -ERANGE.  It's just aesthetic, but usually
in Linux functions do not return -1.

Thanks,

Paolo

> +	/*
> +	 * If the delta tsc can't be fit in the 32 bit after the multi shift,
> +	 * we can't use the preemption timer.
> +	 * It's possible that it can be fit when vmentry happens late, but
> +	 * checking on every vmentry is costly, so fail earilier.
> +	 */
> +	if (delta_tsc >> (cpu_preemption_timer_multi + 32))
> +		return -1;
> +
> +	vcpu->arch.hv_orig_tsc = tscl;
> +	vcpu->arch.hv_deadline_tsc = tscl + delta_tsc;
> +	vmcs_set_bits(PIN_BASED_VM_EXEC_CONTROL,
> +			PIN_BASED_VMX_PREEMPTION_TIMER);
> +	return 0;
> +}