Re: [PATCH] KVM: selftests: Compare wall time from xen shinfo against KVM_GET_CLOCK

[David Woodhouse]

Sorry for delayed response.

On Thu, 2024-01-11 at 14:59 +0100, Vitaly Kuznetsov wrote:
> 
> +       vm_ioctl(vm, KVM_GET_CLOCK, &kcdata);
> +       delta = (wc->sec * NSEC_PER_SEC + wc->nsec) - (kcdata.realtime - kcdata.clock);

I think you need to check for KVM_CLOCK_REALTIME in the flags here,
don't you? It might not always be set.

And also, nobody should ever be using KVM_CLOCK_REALTIME as it stands
at the moment. It's fundamentally broken because it should always have
used CLOCK_TAI not CLOCK_REALTIME.

I'm in the process of fixing that up as an incremental ABI change,
putting the TAI offset into one of the spare pad fields in the
kvm_clock_data and adding a new KVM_CLOCK_TAI_OFFSET flag.

> +       TEST_ASSERT(llabs(delta) < 100,
> +                   "Guest's epoch from shinfo %d.%09d differs from KVM_GET_CLOCK %lld.%lld",
> +                   wc->sec, wc->nsec, (kcdata.realtime - kcdata.clock) / NSEC_PER_SEC,
> +                   (kcdata.realtime - kcdata.clock) % NSEC_PER_SEC);
> 

> Replace the check with comparing wall clock data from shinfo to
> KVM_GET_CLOCK. The later gives both realtime and kvmclock so guest's epoch
> can be calculated by subtraction. Note, the computed epoch may still differ
> a few nanoseconds from shinfo as different TSC is used and there are
> rounding errors but 100 nanoseconds margin should be enough to cover
> it (famous last words).

Aren't those just bugs? Surely this *should* be cycle-perfect, if the
host has a stable TSC?

But I suppose this isn't the test case for that. This is just testing
that the Xen shinfo is doing basically the right thing.

And we're going to need a few more fixes like this WIP before we get
get to cycle perfection from the horrid mess that is our kvmclock code:
https://git.infradead.org/?p=users/dwmw2/linux.git;a=commitdiff;h=bc557e5ee4

Attachment: smime.p7s
Description: S/MIME cryptographic signature