From mboxrd@z Thu Jan 1 00:00:00 1970 From: Glauber Costa Subject: Re: [patch 13/18] KVM: x86: pass host_tsc to read_l1_tsc Date: Tue, 30 Oct 2012 11:55:27 +0400 Message-ID: <508F87EF.4020501@parallels.com> References: <20121024131340.742340256@redhat.com> <20121024131621.890469944@redhat.com> <508E9B1B.20902@parallels.com> <20121029184541.GD30422@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Cc: , , , , , , To: Marcelo Tosatti Return-path: Received: from mx2.parallels.com ([64.131.90.16]:37625 "EHLO mx2.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755300Ab2J3Hzn (ORCPT ); Tue, 30 Oct 2012 03:55:43 -0400 In-Reply-To: <20121029184541.GD30422@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: On 10/29/2012 10:45 PM, Marcelo Tosatti wrote: > On Mon, Oct 29, 2012 at 07:04:59PM +0400, Glauber Costa wrote: >> On 10/24/2012 05:13 PM, Marcelo Tosatti wrote: >>> Allow the caller to pass host tsc value to kvm_x86_ops->read_l1_tsc(). >>> >>> Signed-off-by: Marcelo Tosatti >> >> Would you mind explaining why? >> >> it seems to me that rdtscll() here would be perfectly safe: the only >> case in which they wouldn't, is in a nested-vm environment running >> paravirt-linux with a paravirt tsc. In this case, it is quite likely >> that we'll want rdtscll *anyway*, instead of going to tsc directly. > > Its something different (from a future patch): > > "KVM added a global variable to guarantee monotonicity in the guest. > One of the reasons for that is that the time between > > 1. ktime_get_ts(×pec); > 2. rdtscll(tsc); > > Is variable. That is, given a host with stable TSC, suppose that > two VCPUs read the same time via ktime_get_ts() above. > > The time required to execute 2. is not the same on those two instances > executing in different VCPUS (cache misses, interrupts...)." > > > Think step 1. returning the same value on both vcpus (to read the > explanation above). > This still doesn't go the core of the question. You are replacing rdtscll with native_read_tsc(). They are equivalent most of the time for the host (except in the case I outlined). So the logic you exposed, is valid for both. But the real problem is another one: Although I am not very skilled in C, I rock in the Logo programming language (http://en.wikipedia.org/wiki/Logo_(programming_language) ), and with that knowledge, I can understand your C code - with a bit of effort. After reading it, it becomes very clear that what you do is "Allow the caller to pass host tsc value to kvm_x86_ops->read_l1_tsc", so your changelog doesn't really add any data. It would be great if it explained us, readers, why instead of what!