On Wed, Jan 25, 2017 at 01:59:34PM +0100, Paolo Bonzini wrote:
>
>
> On 25/01/2017 13:55, Marcelo Tosatti wrote:
> > On Wed, Jan 25, 2017 at 01:30:49PM +0100, Paolo Bonzini wrote:
> >>
> >>
> >> On 24/01/2017 18:09, Marcelo Tosatti wrote:
> >>> Add a hypercall to retrieve the host realtime clock
> >>> and the TSC value used to calculate that clock read.
> >>>
> >>> Used to implement clock synchronization between
> >>> host and guest.
> >>>
> >>> Signed-off-by: Marcelo Tosatti <mtosatti@xxxxxxxxxx>
> >>>
> >>> ---
> >>> Documentation/virtual/kvm/hypercalls.txt | 33 ++++++++++++++++++++++++
> >>> arch/x86/include/uapi/asm/kvm_para.h | 9 ++++++
> >>> arch/x86/kvm/x86.c | 41 +++++++++++++++++++++++++++++++
> >>> include/uapi/linux/kvm_para.h | 3 ++
> >>> 4 files changed, 86 insertions(+)
> >>>
> >>> v2: improve documentation (Radim)
> >>> change hypercall name to KVM_HC_CLOCK_PAIRING (Radim)
> >>> increase padding size
> >>>
> >>> Index: kvm-ptpdriver/arch/x86/include/uapi/asm/kvm_para.h
> >>> ===================================================================
> >>> --- kvm-ptpdriver.orig/arch/x86/include/uapi/asm/kvm_para.h 2017-01-13 16:43:11.947240575 -0200
> >>> +++ kvm-ptpdriver/arch/x86/include/uapi/asm/kvm_para.h 2017-01-13 16:43:25.038258187 -0200
> >>> @@ -50,6 +50,15 @@
> >>> __u32 pad[11];
> >>> };
> >>>
> >>> +#define KVM_CLOCK_PAIRING_WALLCLOCK 0
> >>> +struct kvm_clock_offset {
> >>
> >> The struct still has the old name.
> >>
> >>> + __s64 sec;
> >>> + __s64 nsec;
> >>> + __u64 tsc;
> >>> + __u32 flags;
> >>> + __u32 pad[9];
> >>> +};
> >
> > That was on purpose: "to pair" is the operation.
> >
> > "struct kvm_clock_pairing" makes no sense to me
> > (because pairing is an "action").
> >
> > (if you'd prefer that name, let me know).
>
> A "pairing" is a union of two things, in this case a sec/nsec value and
> a TSC value. (I had to look it up in the dictionary though :)).
>
> >>> #define KVM_STEAL_ALIGNMENT_BITS 5
> >>> #define KVM_STEAL_VALID_BITS ((-1ULL << (KVM_STEAL_ALIGNMENT_BITS + 1)))
> >>> #define KVM_STEAL_RESERVED_MASK (((1 << KVM_STEAL_ALIGNMENT_BITS) - 1 ) << 1)
> >>> Index: kvm-ptpdriver/Documentation/virtual/kvm/hypercalls.txt
> >>> ===================================================================
> >>> --- kvm-ptpdriver.orig/Documentation/virtual/kvm/hypercalls.txt 2017-01-13 16:43:11.947240575 -0200
> >>> +++ kvm-ptpdriver/Documentation/virtual/kvm/hypercalls.txt 2017-01-13 16:43:25.038258187 -0200
> >>> @@ -81,3 +81,36 @@
> >>> same guest can wakeup the sleeping vcpu by issuing KVM_HC_KICK_CPU hypercall,
> >>> specifying APIC ID (a1) of the vcpu to be woken up. An additional argument (a0)
> >>> is used in the hypercall for future use.
> >>> +
> >>> +
> >>> +6. KVM_HC_CLOCK_PAIRING
> >>> +------------------------
> >>> +Architecture: x86
> >>> +Status: active
> >>> +Purpose: Hypercall used to synchronize host and guest clocks.
> >>> +Usage:
> >>> +
> >>> +a0: guest physical address where host copies
> >>> +"struct kvm_clock_offset" structure.
> >>> +
> >>> +a1: clock_type, ATM only KVM_CLOCK_PAIRING_WALLCLOCK (0)
> >>> +is supported (hosts CLOCK_REALTIME clock).
> >>> +
> >>> + struct kvm_clock_offset {
> >>
> >> Name of the struct not adjusted.
> >>> + __s64 sec;
> >>> + __s64 nsec;
> >>> + __u64 tsc;
> >>> + __u32 flags;
> >>> + __u32 pad[9];
> >>
> >> I'd just leave one element of padding and put the struct on the stack,
> >> since we have no reason to think we'll need a lot of padding.
> >
> > Well you don't know: we could implement it in case of HPET clocksource
> > hosts.
>
> But then the guest wouldn't have a TSC value to use, would it?
>
> Also, how would it use more padding?
I don't know, maybe pass the TSC and host HPET read in the structure?
(after exposing the HPET to the guest).
Or maybe you want to sync CLOCK_MONOTONIC and that requires other data.
Not sure.
Just some space if new features are necessary.
