From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH] x86/vdso/pvclock: Protect STABLE check with the seqcount
Date: Thu, 7 Jan 2016 19:02:18 -0200
Message-ID: <20160107210216.GA19334@amt.cnet>
References: <20160104225904.GA4947@amt.cnet>
 <755dcedb17269e1d7ce12a9a713dea303835137e.1451949191.git.luto@kernel.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: X86 ML <x86@kernel.org>, Radim Krcmar <rkrcmar@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	Alexander Graf <agraf@suse.de>
To: Andy Lutomirski <luto@kernel.org>
Return-path: <linux-kernel-owner@vger.kernel.org>
Content-Disposition: inline
In-Reply-To: <755dcedb17269e1d7ce12a9a713dea303835137e.1451949191.git.luto@kernel.org>
Sender: linux-kernel-owner@vger.kernel.org
List-Id: kvm.vger.kernel.org

On Mon, Jan 04, 2016 at 03:14:28PM -0800, Andy Lutomirski wrote:
> If the clock becomes unstable while we're reading it, we need to
> bail.  We can do this by simply moving the check into the seqcount
> loop.
> 
> Reported-by: Marcelo Tosatti <mtosatti@redhat.com>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
> 
> Marcelo, how's this?
> 
> arch/x86/entry/vdso/vclock_gettime.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c
> index 8602f06c759f..1a50e09c945b 100644
> --- a/arch/x86/entry/vdso/vclock_gettime.c
> +++ b/arch/x86/entry/vdso/vclock_gettime.c
> @@ -126,23 +126,23 @@ static notrace cycle_t vread_pvclock(int *mode)
>  	 *
>  	 * On Xen, we don't appear to have that guarantee, but Xen still
>  	 * supplies a valid seqlock using the version field.
> -
> +	 *
>  	 * We only do pvclock vdso timing at all if
>  	 * PVCLOCK_TSC_STABLE_BIT is set, and we interpret that bit to
>  	 * mean that all vCPUs have matching pvti and that the TSC is
>  	 * synced, so we can just look at vCPU 0's pvti.
>  	 */
>  
> -	if (unlikely(!(pvti->flags & PVCLOCK_TSC_STABLE_BIT))) {
> -		*mode = VCLOCK_NONE;
> -		return 0;
> -	}
> -
>  	do {
>  		version = pvti->version;
>  
>  		smp_rmb();
>  
> +		if (unlikely(!(pvti->flags & PVCLOCK_TSC_STABLE_BIT))) {
> +			*mode = VCLOCK_NONE;
> +			return 0;
> +		}
> +
>  		tsc = rdtsc_ordered();
>  		pvti_tsc_to_system_mul = pvti->tsc_to_system_mul;
>  		pvti_tsc_shift = pvti->tsc_shift;
> -- 
> 2.4.3

Check it before returning the value (once cleared, it can't be set back 
to 1), similarly to what was in place before.