Re: [PATCH] KVM: x86: fix kvmclock breakage from timers branch merge

Chris J Arges
2014-09-04T20:37:30Z

On 09/04/2014 02:42 PM, Paolo Bonzini wrote:
> Il 04/09/2014 21:15, Paolo Bonzini ha scritto:
>> Il 04/09/2014 20:16, Chris J Arges ha scritto:
>>>> + boot_ns = timespec_to_ns(&tk->total_sleep_time)
>>>> + + tk->wall_to_monotonic.tv_sec * (u64)NSEC_PER_SEC
>>>> + + tk->wall_to_monotonic.tv_nsec
>>>> + + tk->xtime_sec * (u64)NSEC_PER_SEC;
>>
>> So this means that the above 3.16-based code is not the same as
>>
>> boot_ns = ktime_to_ns(ktime_add(tk->tkr.base_mono, tk->offs_boot));
>>
>> in 3.17. Everything else in the patch you tested is the same as the
>> code that is in 3.17, so that's a start.
>>
>> Paolo
>>
>
> Based on commit 02cba1598a2a3b689e79ad6dad2532521f638271 we have:
>
> offs_real - offs_boot = wall_to_monotonic + total_sleep_time
>
> The patch I posted this morning separates tk->xtime_sec out of boot_ns, so
> all that is missing should be a change in boot_ns from base_mono + offs_boot
> to offs_real - offs_boot.
>
> Chris, can you try this patch on top of the previous one:
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 92493e10937c..811eecc43fe8 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1031,7 +1031,7 @@ static void update_pvclock_gtod(struct timekeeper *tk)
> struct pvclock_gtod_data *vdata = &pvclock_gtod_data;
> u64 boot_ns;
>
> - boot_ns = ktime_to_ns(ktime_add(tk->tkr.base_mono, tk->offs_boot));
> + boot_ns = ktime_to_ns(ktime_sub(tk->offs_real, tk->offs_boot));
>
> write_seqcount_begin(&vdata->seq);
>
> If it doesn't work, then commit 02cba1598a2a3b689e79ad6dad2532521f638271
> is also broken.
>
> Paolo
>

Paolo,
That modification do your additional patch didn't work. However I was
able to modify the code as follows to get this test case working. The
only additional modification was:
+ vdata->nsec_base = tk->xtime_sec * (u64)NSEC_PER_SEC
+ - boot_ns;

--chris j arges

--

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7b25aa2..60c0a9b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1023,8 +1023,8 @@ struct pvclock_gtod_data {
u32 shift;
} clock;

- u64 boot_ns;
u64 nsec_base;
+ u64 snsec_base;
};

static struct pvclock_gtod_data pvclock_gtod_data;
@@ -1034,7 +1034,7 @@ static void update_pvclock_gtod(struct timekeeper *tk)
struct pvclock_gtod_data *vdata = &pvclock_gtod_data;
u64 boot_ns;

- boot_ns = ktime_to_ns(ktime_add(tk->tkr.base_mono, tk->offs_boot));
+ boot_ns = ktime_to_ns(ktime_sub(tk->offs_real, tk->offs_boot));

write_seqcount_begin(&vdata->seq);

@@ -1045,8 +1045,9 @@ static void update_pvclock_gtod(struct timekeeper *tk)
vdata->clock.mult = tk->tkr.mult;
vdata->clock.shift = tk->tkr.shift;

- vdata->boot_ns = boot_ns;
- vdata->nsec_base = tk->tkr.xtime_nsec;
+ vdata->nsec_base = tk->xtime_sec * (u64)NSEC_PER_SEC
+ - boot_ns;
+ vdata->snsec_base = tk->tkr.xtime_nsec;

write_seqcount_end(&vdata->seq);
}
@@ -1416,10 +1417,10 @@ static int do_monotonic_boot(s64 *t, cycle_t
*cycle_now)
do {
seq = read_seqcount_begin(&gtod->seq);
mode = gtod->clock.vclock_mode;
- ns = gtod->nsec_base;
+ ns = gtod->snsec_base;
ns += vgettsc(cycle_now);
ns >>= gtod->clock.shift;
- ns += gtod->boot_ns;
+ ns += gtod->nsec_base;
} while (unlikely(read_seqcount_retry(&gtod->seq, seq)));
*t = ns;