Paolo Bonzini <pbonzini@xxxxxxxxxx> writes: > On 30/03/21 16:44, Vitaly Kuznetsov wrote: >> We could've solved the problem by reducing the precision a bit and >> instead of doing >> >> now_ns = get_kvmclock_ns(kvm); >> >> in KVM_SET_CLOCK() handling we could do >> >> now_ns = ka->master_kernel_ns >> >> and that would make vcpu->hv_clock.system_time == 0 after >> kvm_guest_time_update() but it'd hurt 'normal' use-cases to fix the >> corner case. > > Marcelo is right, and I guess instead of a negative system time you > *should* have a slightly larger tsc_timestamp that corresponds to a zero > system_time. E.g. instead of -70 ns in the system time you'd have 210 > more clock cycles in the tsc_timestamp and 0 in the system time. > > In the end it's impossible to achieve absolute precision; does the > KVM_SET_CLOCK value refer to the nanosecond value before entering the > kernel, or after, or what? The difference is much more than these 70 > ns. So what you propose above seems to be really the best solution > within the constraints of the KVM_SET_CLOCK API (a better API, which > Maxim had started working on and I should revisit, would pass a > TSC+nanosecond pair). I'm leaning towards making a v3 and depending on how complex it's going to look like we can decide which way to go. > > On the other hand you'd have to take into account what happens if the > masterclock is not in use, which would make things a bit more complex > than what you sketched. (I really wish we establish a plan to get rid of !masterclock logic some day ... ) > If guests probably do not look at the > system_time and just add it blindly to the result, then treating > system_time as signed as in v2 is the easiest. -- Vitaly
