Re: [patch 3/3] PTP: add kvm PTP driver

[Radim Krcmar]

2017-01-18 12:50-0200, Marcelo Tosatti:
> On Wed, Jan 18, 2017 at 03:02:23PM +0100, Paolo Bonzini wrote:
>> 
>> 
>> On 18/01/2017 14:36, Miroslav Lichvar wrote:
>> > On Wed, Jan 18, 2017 at 01:46:58PM +0100, Paolo Bonzini wrote:
>> >> On 18/01/2017 13:24, Marcelo Tosatti wrote:
>> >>>> Testcase: run a guest and a loop sending SIGUSR1 to vcpu0 (emulating
>> >>>> intense interrupts). Follows results:
>> > 
>> >>>> Do you still want to drop it in favour of simplicity?
>> > 
>> >> It's just that it's not obvious why you get better results with biased
>> >> host timestamps.  What makes the biased host timestamp more precise?
>> >>
>> >> I'd rather use PTP_SYS_OFFSET_PRECISE instead, but unfortunately chrony
>> >> does not support it---but I would still prefer you to support
>> >> PTP_SYS_OFFSET_PRECISE as well.
>> > 
>> > Interesting. I wasn't aware that there is a new ioctl for measuring
>> > the HW-sys offset. Adding support to chrony shouldn't be difficult.
>> > 
>> > If I understand it correctly, PTP_SYS_OFFSET can be emulated on top of
>> > PTP_SYS_OFFSET_PRECISE simply by copying the sys_realtime and device
>> > fields to corresponding ts slots. The apparent delay will be zero, but
>> > that's ok if the conversion is really accurate.
>> 
>> Yes, for 1 sample only.  Otherwise you'd have the same issue as in
>> Marcelo's driver (the device aka guest timestamp from
>> PTP_SYS_OFFSET_PRECISE would not be halfway between the system aka host
>> timestamps), and your idea below could be applied.
>> 
>> > I'm not sure if trying to do that in the opposite direction is a good
>> > idea. An application using PTP_SYS_OFFSET_PRECISE may assume the
>> > conversion is accurate and not include any delay/dispersion in an
>> > estimate of the maximum error, which is needed in NTP for instance.
>> > 
>> > If we know the host timestamp ts[1] is not in the middle between the
>> > guests timestamps ts[0] and ts[2], but rather closer to ts[2], why not
>> > simply shift ts[1] by (ts[2]-ts[0])/2 ?
> 
> 
> 
>> 
>> Interesting idea!  For this to work, KVM needs to implement
>> getcrosstimestamp and ptp_chardev.c can then add an alternative
>> implementation of PTP_SYS_OFFSET, based on precise cross timestamps.
>> 
>> Something like
>> 
>>                 for (i = 0; i <= sysoff->n_samples; i++) {
>> 			// ... call getcrosststamp ...
>> 			sysns = ktime_to_ns(xtstamp.sys_realtime);
>> 			if (i > 0) {
>> 				devns = ktime_to_ns(xtstamp.device);
>> 				devns -= (sysns - prev_sysns) / 2;
>> 				devts = ns_to_timespec(devns);
>> 				pct->sec = devts.tv_sec;
>> 				pct->nsec = devts.tv_nsec;
>> 	                        pct++;
>> 			}
>> 			systs = ns_to_timespec(sysns);
>>                         pct->sec = ts.tv_sec;
>>                         pct->nsec = ts.tv_nsec;
>>                         pct++;
>> 			prev_sysns = sysns;
>>                 }

Nice.  PTP_SYS_OFFSET seems to be mandatory, so this hunk could be
upstreamable as a fallback for PTP devices that have getcrosststamp and
not gettime64.  KVM PTP would be the only driver using it so far.

>> Marcelo, can you give it a try?
> 
> Can convert fine, but problem is the simultaneous read
> of host and guest clocks.
> 
>> Thanks,
>> 
>> Paolo
> 
> It seems to me anything else other than using a single TSC read
> (for both host and guest clocks) is a poor PTP_SYS_OFFSET_PRECISE 
> implementation (because it would claim to be similar to ART, where
> the timestamps are simultaneous), but not be.

Yes, the guest should use the TSC returned by the hypercall to compute
the corresponding guest time, which will allow us to know the
host<->guest offset.

Is this impossible to do with the current API?

Thanks.