From mboxrd@z Thu Jan  1 00:00:00 1970
From: Glauber Costa <glommer@parallels.com>
Subject: Re: [patch 02/16] x86: pvclock: make sure rdtsc doesnt speculate
 out of region
Date: Thu, 1 Nov 2012 17:49:51 +0400
Message-ID: <50927DFF.4000502@parallels.com>
References: <20121031224656.417434866@redhat.com> <20121031224823.907464421@redhat.com> <20121101114807.GQ25650@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="ISO-8859-1"
Content-Transfer-Encoding: 7bit
Cc: Marcelo Tosatti <mtosatti@redhat.com>, <kvm@vger.kernel.org>,
	<johnstul@us.ibm.com>, <jeremy@goop.org>, <zamsden@gmail.com>,
	<avi@redhat.com>, <pbonzini@redhat.com>
To: Gleb Natapov <gleb@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.parallels.com ([64.131.90.16]:45023 "EHLO mx2.parallels.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1758176Ab2KANt7 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 1 Nov 2012 09:49:59 -0400
In-Reply-To: <20121101114807.GQ25650@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 11/01/2012 03:48 PM, Gleb Natapov wrote:
> On Wed, Oct 31, 2012 at 08:46:58PM -0200, Marcelo Tosatti wrote:
>> Originally from Jeremy Fitzhardinge.
>>
>> pvclock_get_time_values, which contains the memory barriers
>> will be removed by next patch.
>>
>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>
>> Index: vsyscall/arch/x86/kernel/pvclock.c
>> ===================================================================
>> --- vsyscall.orig/arch/x86/kernel/pvclock.c
>> +++ vsyscall/arch/x86/kernel/pvclock.c
>> @@ -97,10 +97,10 @@ cycle_t pvclock_clocksource_read(struct 
>>  
>>  	do {
>>  		version = pvclock_get_time_values(&shadow, src);
>> -		barrier();
>> +		rdtsc_barrier();
>>  		offset = pvclock_get_nsec_offset(&shadow);
>>  		ret = shadow.system_timestamp + offset;
>> -		barrier();
>> +		rdtsc_barrier();
>>  	} while (version != src->version);
>>  
>>  	if ((valid_flags & PVCLOCK_TSC_STABLE_BIT) &&
>>
> On a guest without SSE2 rdtsc_barrier() will be nop while rmb() will
> be "lock; addl $0,0(%%esp)". I doubt pvclock will work correctly either
> way though.
> 
> --
> 			Gleb.
> 
Actually it shouldn't matter for KVM, since the page is only updated by
the vcpu, and the guest is never running while it happens. If Jeremy is
fine with this, so should I.