Re: [PATCH v2 0/3] x86/vdso: Add Hyper-V TSC page clocksource support

[Thomas Gleixner <tglx@linutronix.de>]

On Fri, 17 Feb 2017, Vitaly Kuznetsov wrote:
> Thomas Gleixner <tglx@linutronix.de> writes:
> > On Wed, 15 Feb 2017, Vitaly Kuznetsov wrote:
> >> Actually, we already have an implementation of TSC page update in KVM
> >> (see arch/x86/kvm/hyperv.c, kvm_hv_setup_tsc_page()) and the update does
> >> the following:
> >> 
> >> 0) stash seq into seq_prev
> >> 1) seq = 0 making all reads from the page invalid
> >> 2) smp_wmb()
> >> 3) update tsc_scale, tsc_offset
> >> 4) smp_wmb()
> >> 5) set seq = seq_prev + 1
> >
> > I hope they handle the case where seq_prev overflows and becomes 0 :)
> >
> >> As far as I understand this helps with situations you described above as
> >> guest will notice either invalid value of 0 or seq change. In case the
> >> implementation in real Hyper-V is the same we're safe with compile
> >> barriers only.
> >
> > On x86 that's correct. smp_rmb() resolves to barrier(), but you certainly
> > need the smp_wmb() on the writer side.
> >
> > Now looking at the above your reader side code is bogus:
> >
> > +       while (1) {
> > +               sequence = tsc_pg->tsc_sequence;
> > +               if (!sequence)
> > +                       break;
> >
> > Why would you break out of the loop when seq is 0? The 0 is just telling
> > you that there is an update in progress.
> 
> Not only. As far as I understand (and I *think* K. Y. pointed this out)
> when VM is migrating to another host TSC page clocksource is disabled for
> extended period of time so we're better off reading from MSR than
> looping here. With regards to VDSO this means reverting to doing normal
> syscall.

If you migrate to another host and the VM is using the TSC page, then the
TSC page on the new host _must_ be available and accessible _before_ the VM
resumes there. So that extended period of time does not make any sense at
all. Voodoo programming is the only explanation which come to my mind.

Thanks,

	tglx