Re: [patch 02/16] x86: pvclock: make sure rdtsc doesnt speculate out of region

[Marcelo Tosatti <mtosatti@redhat.com>]

On Thu, Nov 01, 2012 at 05:49:51PM +0400, Glauber Costa wrote:
> 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.

17.13 TIME-STAMP COUNTER

"The RDTSC instruction is not serializing or ordered with other
instructions. It does not necessarily wait until all previous
instructions have been executed before reading the counter. Similarly,
subsequent instructions may begin execution before the RDTSC instruction
operation is performed."

Both instructions are TSC barriers.