* [patch 1/3] KVM: x86: provide realtime host clock via vsyscall notifiers
2017-01-13 18:45 [patch 0/3] KVM virtual PTP driver (v2) Marcelo Tosatti
@ 2017-01-13 18:45 ` Marcelo Tosatti
2017-01-13 18:46 ` [patch 2/3] KVM: x86: add KVM_HC_CLOCK_OFFSET hypercall Marcelo Tosatti
2017-01-13 18:46 ` [patch 3/3] PTP: add kvm PTP driver Marcelo Tosatti
2 siblings, 0 replies; 9+ messages in thread
From: Marcelo Tosatti @ 2017-01-13 18:45 UTC (permalink / raw)
To: kvm, linux-kernel
Cc: Paolo Bonzini, Radim Krcmar, Richard Cochran, Miroslav Lichvar,
Marcelo Tosatti
[-- Attachment #1: kvm-do-realtime --]
[-- Type: text/plain, Size: 2010 bytes --]
Expose the realtime host clock and save the TSC value
used for the clock calculation.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
---
arch/x86/kvm/x86.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
v2: unify nsec_base (Radim)
Index: kvm-ptpdriver/arch/x86/kvm/x86.c
===================================================================
--- kvm-ptpdriver.orig/arch/x86/kvm/x86.c 2017-01-13 08:59:03.015895353 -0200
+++ kvm-ptpdriver/arch/x86/kvm/x86.c 2017-01-13 15:49:14.058347822 -0200
@@ -1139,6 +1139,7 @@
u64 boot_ns;
u64 nsec_base;
+ u64 wall_time_sec;
};
static struct pvclock_gtod_data pvclock_gtod_data;
@@ -1162,6 +1163,8 @@
vdata->boot_ns = boot_ns;
vdata->nsec_base = tk->tkr_mono.xtime_nsec;
+ vdata->wall_time_sec = tk->xtime_sec;
+
write_seqcount_end(&vdata->seq);
}
#endif
@@ -1623,6 +1626,28 @@
return mode;
}
+static int do_realtime(struct timespec *ts, cycle_t *cycle_now)
+{
+ struct pvclock_gtod_data *gtod = &pvclock_gtod_data;
+ unsigned long seq;
+ int mode;
+ u64 ns;
+
+ do {
+ seq = read_seqcount_begin(>od->seq);
+ mode = gtod->clock.vclock_mode;
+ ts->tv_sec = gtod->wall_time_sec;
+ ns = gtod->nsec_base;
+ ns += vgettsc(cycle_now);
+ ns >>= gtod->clock.shift;
+ } while (unlikely(read_seqcount_retry(>od->seq, seq)));
+
+ ts->tv_sec += __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
+ ts->tv_nsec = ns;
+
+ return mode;
+}
+
/* returns true if host is using tsc clocksource */
static bool kvm_get_time_and_clockread(s64 *kernel_ns, cycle_t *cycle_now)
{
@@ -1632,6 +1657,17 @@
return do_monotonic_boot(kernel_ns, cycle_now) == VCLOCK_TSC;
}
+
+/* returns true if host is using tsc clocksource */
+static bool kvm_get_walltime_and_clockread(struct timespec *ts,
+ cycle_t *cycle_now)
+{
+ /* checked again under seqlock below */
+ if (pvclock_gtod_data.clock.vclock_mode != VCLOCK_TSC)
+ return false;
+
+ return do_realtime(ts, cycle_now) == VCLOCK_TSC;
+}
#endif
/*
^ permalink raw reply [flat|nested] 9+ messages in thread* [patch 2/3] KVM: x86: add KVM_HC_CLOCK_OFFSET hypercall
2017-01-13 18:45 [patch 0/3] KVM virtual PTP driver (v2) Marcelo Tosatti
2017-01-13 18:45 ` [patch 1/3] KVM: x86: provide realtime host clock via vsyscall notifiers Marcelo Tosatti
@ 2017-01-13 18:46 ` Marcelo Tosatti
2017-01-13 18:46 ` [patch 3/3] PTP: add kvm PTP driver Marcelo Tosatti
2 siblings, 0 replies; 9+ messages in thread
From: Marcelo Tosatti @ 2017-01-13 18:46 UTC (permalink / raw)
To: kvm, linux-kernel
Cc: Paolo Bonzini, Radim Krcmar, Richard Cochran, Miroslav Lichvar,
Marcelo Tosatti
[-- Attachment #1: kvm-hv-clock-offset --]
[-- Type: text/plain, Size: 5057 bytes --]
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@redhat.com>
---
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 15:43:47.196867691 -0200
+++ kvm-ptpdriver/arch/x86/include/uapi/asm/kvm_para.h 2017-01-13 15:59:39.596193853 -0200
@@ -50,6 +50,15 @@
__u32 pad[11];
};
+#define KVM_CLOCK_PAIRING_WALLCLOCK 0
+struct kvm_clock_offset {
+ __s64 sec;
+ __s64 nsec;
+ __u64 tsc;
+ __u32 flags;
+ __u32 pad[9];
+};
+
#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 15:43:47.196867691 -0200
+++ kvm-ptpdriver/Documentation/virtual/kvm/hypercalls.txt 2017-01-13 15:59:26.676163231 -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 {
+ __s64 sec;
+ __s64 nsec;
+ __u64 tsc;
+ __u32 flags;
+ __u32 pad[9];
+ };
+
+ Where:
+ * sec: seconds from clock_type clock.
+ * nsec: nanoseconds from clock_type clock.
+ * tsc: TSC value used to calculate sec/nsec pair
+ (this hypercall only works when host uses TSC clocksource).
+ * flags: flags, unused (0) at the moment.
+
+Returns KVM_EOPNOTSUPP if the host does not use TSC clocksource,
+or if clock type is different than KVM_CLOCK_PAIRING_WALLCLOCK.
+
Index: kvm-ptpdriver/arch/x86/kvm/x86.c
===================================================================
--- kvm-ptpdriver.orig/arch/x86/kvm/x86.c 2017-01-13 15:49:14.058347822 -0200
+++ kvm-ptpdriver/arch/x86/kvm/x86.c 2017-01-13 16:00:56.964373208 -0200
@@ -6119,6 +6119,44 @@
}
EXPORT_SYMBOL_GPL(kvm_emulate_halt);
+static int kvm_pv_clock_pairing(struct kvm_vcpu *vcpu, gpa_t paddr,
+ unsigned long clock_type)
+{
+ struct kvm_clock_offset *clock_offset;
+ struct timespec ts;
+ cycle_t cycle;
+ int ret;
+
+ if (clock_type != KVM_CLOCK_PAIRING_WALLCLOCK)
+ return -KVM_EOPNOTSUPP;
+
+ if (pvclock_gtod_data.clock.vclock_mode != VCLOCK_TSC)
+ return -KVM_EOPNOTSUPP;
+
+ clock_offset = kzalloc(sizeof(struct kvm_clock_offset), GFP_KERNEL);
+ if (clock_offset == NULL)
+ return -KVM_ENOMEM;
+
+ if (kvm_get_walltime_and_clockread(&ts, &cycle) == false) {
+ kfree(clock_offset);
+ return -KVM_EOPNOTSUPP;
+ }
+
+ clock_offset->sec = ts.tv_sec;
+ clock_offset->nsec = ts.tv_nsec;
+ clock_offset->tsc = kvm_read_l1_tsc(vcpu, cycle);
+ clock_offset->flags = 0;
+
+ ret = 0;
+ if (kvm_write_guest(vcpu->kvm, paddr, clock_offset,
+ sizeof(struct kvm_clock_offset)))
+ ret = -KVM_EFAULT;
+
+ kfree(clock_offset);
+
+ return ret;
+}
+
/*
* kvm_pv_kick_cpu_op: Kick a vcpu.
*
@@ -6183,6 +6221,9 @@
kvm_pv_kick_cpu_op(vcpu->kvm, a0, a1);
ret = 0;
break;
+ case KVM_HC_CLOCK_PAIRING:
+ ret = kvm_pv_clock_pairing(vcpu, a0, a1);
+ break;
default:
ret = -KVM_ENOSYS;
break;
Index: kvm-ptpdriver/include/uapi/linux/kvm_para.h
===================================================================
--- kvm-ptpdriver.orig/include/uapi/linux/kvm_para.h 2017-01-13 15:43:47.196867691 -0200
+++ kvm-ptpdriver/include/uapi/linux/kvm_para.h 2017-01-13 16:01:09.290401187 -0200
@@ -14,6 +14,8 @@
#define KVM_EFAULT EFAULT
#define KVM_E2BIG E2BIG
#define KVM_EPERM EPERM
+#define KVM_EOPNOTSUPP EOPNOTSUPP
+#define KVM_ENOMEM ENOMEM
#define KVM_HC_VAPIC_POLL_IRQ 1
#define KVM_HC_MMU_OP 2
@@ -23,6 +25,7 @@
#define KVM_HC_MIPS_GET_CLOCK_FREQ 6
#define KVM_HC_MIPS_EXIT_VM 7
#define KVM_HC_MIPS_CONSOLE_OUTPUT 8
+#define KVM_HC_CLOCK_PAIRING 9
/*
* hypercalls use architecture specific
^ permalink raw reply [flat|nested] 9+ messages in thread* [patch 3/3] PTP: add kvm PTP driver
2017-01-13 18:45 [patch 0/3] KVM virtual PTP driver (v2) Marcelo Tosatti
2017-01-13 18:45 ` [patch 1/3] KVM: x86: provide realtime host clock via vsyscall notifiers Marcelo Tosatti
2017-01-13 18:46 ` [patch 2/3] KVM: x86: add KVM_HC_CLOCK_OFFSET hypercall Marcelo Tosatti
@ 2017-01-13 18:46 ` Marcelo Tosatti
2 siblings, 0 replies; 9+ messages in thread
From: Marcelo Tosatti @ 2017-01-13 18:46 UTC (permalink / raw)
To: kvm, linux-kernel
Cc: Paolo Bonzini, Radim Krcmar, Richard Cochran, Miroslav Lichvar,
Marcelo Tosatti
[-- Attachment #1: kvm-ptpdriver --]
[-- Type: text/plain, Size: 6662 bytes --]
Add a driver with gettime method returning hosts realtime clock.
This allows Chrony to synchronize host and guest clocks with
high precision (see results below).
chronyc> sources
MS Name/IP address Stratum Poll Reach LastRx Last sample
===============================================================================
#* PHC0 0 3 377 6 +4ns[ +4ns] +/- 3ns
To configure Chronyd to use PHC refclock, add the
following line to its configuration file:
refclock PHC /dev/ptpX poll 3 dpoll -2 offset 0
Where /dev/ptpX is the kvmclock PTP clock.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
---
drivers/ptp/Kconfig | 12 +++
drivers/ptp/Makefile | 1
drivers/ptp/ptp_kvm.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 192 insertions(+)
v2: check for kvmclock (Radim)
initialize global variables before device registration (Radim)
Index: kvm-ptpdriver/drivers/ptp/Kconfig
===================================================================
--- kvm-ptpdriver.orig/drivers/ptp/Kconfig 2017-01-13 16:43:04.808231054 -0200
+++ kvm-ptpdriver/drivers/ptp/Kconfig 2017-01-13 16:43:31.248266610 -0200
@@ -90,4 +90,16 @@
To compile this driver as a module, choose M here: the module
will be called ptp_pch.
+config PTP_1588_CLOCK_KVM
+ tristate "KVM virtual PTP clock"
+ depends on PTP_1588_CLOCK
+ depends on KVM_GUEST
+ default y
+ help
+ This driver adds support for using kvm infrastructure as a PTP
+ clock. This clock is only useful if you are using KVM guests.
+
+ To compile this driver as a module, choose M here: the module
+ will be called ptp_kvm.
+
endmenu
Index: kvm-ptpdriver/drivers/ptp/ptp_kvm.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ kvm-ptpdriver/drivers/ptp/ptp_kvm.c 2017-01-13 16:43:31.248266610 -0200
@@ -0,0 +1,179 @@
+/*
+ * Virtual PTP 1588 clock for use with KVM guests
+ *
+ * Copyright (C) 2017 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <uapi/linux/kvm_para.h>
+#include <asm/kvm_para.h>
+#include <asm/pvclock.h>
+#include <uapi/asm/kvm_para.h>
+
+#include <linux/ptp_clock_kernel.h>
+
+struct kvm_ptp_clock {
+ struct ptp_clock *ptp_clock;
+ struct ptp_clock_info caps;
+};
+
+DEFINE_SPINLOCK(kvm_ptp_lock);
+
+static struct pvclock_vsyscall_time_info *hv_clock;
+
+/*
+ * PTP clock operations
+ */
+
+static int ptp_kvm_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
+{
+ return -EOPNOTSUPP;
+}
+
+static int ptp_kvm_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+ return -EOPNOTSUPP;
+}
+
+static struct kvm_clock_offset clock_off;
+static phys_addr_t clock_off_gpa;
+
+static int ptp_kvm_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
+{
+ unsigned long ret;
+ struct timespec64 tspec;
+ u64 delta;
+ cycle_t offset;
+ unsigned version;
+ int cpu;
+ struct pvclock_vcpu_time_info *src;
+
+ preempt_disable_notrace();
+ cpu = smp_processor_id();
+ src = &hv_clock[cpu].pvti;
+
+ spin_lock(&kvm_ptp_lock);
+
+ do {
+ /*
+ * We are measuring the delay between
+ * kvm_hypercall and rdtsc using TSC,
+ * and converting that delta to
+ * tsc_to_system_mul and tsc_shift
+ * So any changes to tsc_to_system_mul
+ * and tsc_shift in this region
+ * invalidate the measurement.
+ */
+ version = pvclock_read_begin(src);
+
+ ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING,
+ clock_off_gpa,
+ KVM_CLOCK_PAIRING_WALLCLOCK);
+ if (ret != 0) {
+ pr_err("clock offset hypercall ret %lu\n", ret);
+ spin_unlock(&kvm_ptp_lock);
+ preempt_enable_notrace();
+ return -EOPNOTSUPP;
+ }
+
+ tspec.tv_sec = clock_off.sec;
+ tspec.tv_nsec = clock_off.nsec;
+
+ delta = rdtsc_ordered() - clock_off.tsc;
+
+ offset = pvclock_scale_delta(delta, src->tsc_to_system_mul,
+ src->tsc_shift);
+
+ } while (pvclock_read_retry(src, version));
+
+ preempt_enable_notrace();
+
+ tspec.tv_nsec = tspec.tv_nsec + offset;
+
+ spin_unlock(&kvm_ptp_lock);
+
+ if (tspec.tv_nsec >= NSEC_PER_SEC) {
+ u64 secs = tspec.tv_nsec;
+
+ tspec.tv_nsec = do_div(secs, NSEC_PER_SEC);
+ tspec.tv_sec += secs;
+ }
+
+ memcpy(ts, &tspec, sizeof(struct timespec64));
+
+ return 0;
+}
+
+static int ptp_kvm_settime(struct ptp_clock_info *ptp,
+ const struct timespec64 *ts)
+{
+ return -EOPNOTSUPP;
+}
+
+static int ptp_kvm_enable(struct ptp_clock_info *ptp,
+ struct ptp_clock_request *rq, int on)
+{
+ return -EOPNOTSUPP;
+}
+
+static struct ptp_clock_info ptp_kvm_caps = {
+ .owner = THIS_MODULE,
+ .name = "KVM virtual PTP",
+ .max_adj = 0,
+ .n_ext_ts = 0,
+ .n_pins = 0,
+ .pps = 0,
+ .adjfreq = ptp_kvm_adjfreq,
+ .adjtime = ptp_kvm_adjtime,
+ .gettime64 = ptp_kvm_gettime,
+ .settime64 = ptp_kvm_settime,
+ .enable = ptp_kvm_enable,
+};
+
+/* module operations */
+
+static struct kvm_ptp_clock kvm_ptp_clock;
+
+static void __exit ptp_kvm_exit(void)
+{
+ ptp_clock_unregister(kvm_ptp_clock.ptp_clock);
+}
+
+static int __init ptp_kvm_init(void)
+{
+ clock_off_gpa = slow_virt_to_phys(&clock_off);
+ hv_clock = pvclock_pvti_cpu0_va();
+
+ if (!hv_clock)
+ return -ENODEV;
+
+ kvm_ptp_clock.caps = ptp_kvm_caps;
+
+ kvm_ptp_clock.ptp_clock = ptp_clock_register(&kvm_ptp_clock.caps, NULL);
+
+ if (IS_ERR(kvm_ptp_clock.ptp_clock))
+ return PTR_ERR(kvm_ptp_clock.ptp_clock);
+
+ return 0;
+}
+
+module_init(ptp_kvm_init);
+module_exit(ptp_kvm_exit);
+
+MODULE_AUTHOR("Marcelo Tosatti <mtosatti@redhat.com>");
+MODULE_DESCRIPTION("PTP clock using KVMCLOCK");
+MODULE_LICENSE("GPL");
Index: kvm-ptpdriver/drivers/ptp/Makefile
===================================================================
--- kvm-ptpdriver.orig/drivers/ptp/Makefile 2017-01-13 16:43:04.808231054 -0200
+++ kvm-ptpdriver/drivers/ptp/Makefile 2017-01-13 16:43:31.248266610 -0200
@@ -6,3 +6,4 @@
obj-$(CONFIG_PTP_1588_CLOCK) += ptp.o
obj-$(CONFIG_PTP_1588_CLOCK_IXP46X) += ptp_ixp46x.o
obj-$(CONFIG_PTP_1588_CLOCK_PCH) += ptp_pch.o
+obj-$(CONFIG_PTP_1588_CLOCK_KVM) += ptp_kvm.o
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch 2/3] KVM: x86: add KVM_HC_CLOCK_OFFSET hypercall
2017-01-13 12:01 [patch 0/3] KVM virtual " Marcelo Tosatti
@ 2017-01-13 12:01 ` Marcelo Tosatti
2017-01-13 15:31 ` Radim Krcmar
0 siblings, 1 reply; 9+ messages in thread
From: Marcelo Tosatti @ 2017-01-13 12:01 UTC (permalink / raw)
To: kvm, linux-kernel
Cc: Paolo Bonzini, Radim Krcmar, Richard Cochran, Miroslav Lichvar,
Marcelo Tosatti
[-- Attachment #1: kvm-hv-clock-offset --]
[-- Type: text/plain, Size: 4788 bytes --]
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@redhat.com>
---
Documentation/virtual/kvm/hypercalls.txt | 30 ++++++++++++++++++++++
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, 83 insertions(+)
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 09:03:33.823305270 -0200
+++ kvm-ptpdriver/arch/x86/include/uapi/asm/kvm_para.h 2017-01-13 09:07:54.633699270 -0200
@@ -50,6 +50,15 @@
__u32 pad[11];
};
+#define KVM_CLOCK_OFFSET_WALLCLOCK 0
+struct kvm_clock_offset {
+ __s64 sec;
+ __s64 nsec;
+ __u64 tsc;
+ __u32 flags;
+ __u32 pad[1];
+};
+
#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 09:03:33.823305270 -0200
+++ kvm-ptpdriver/Documentation/virtual/kvm/hypercalls.txt 2017-01-13 09:15:32.971389949 -0200
@@ -81,3 +81,33 @@
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_OFFSET
+------------------------
+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_OFFSET_WALLCLOCK (0)
+is supported (hosts CLOCK_REALTIME clock).
+
+ struct kvm_clock_offset {
+ __s64 sec;
+ __s64 nsec;
+ __u64 tsc;
+ __u32 flags;
+ __u32 pad;
+ };
+
+ Where:
+ * sec: seconds from clock_type clock.
+ * nsec: nanoseconds from clock_type clock.
+ * tsc: TSC value used to calculate sec/nsec pair
+ (this hypercall only works when host uses TSC clocksource).
+ * flags: flags, unused at the moment.
+
Index: kvm-ptpdriver/arch/x86/kvm/x86.c
===================================================================
--- kvm-ptpdriver.orig/arch/x86/kvm/x86.c 2017-01-13 09:04:46.581415259 -0200
+++ kvm-ptpdriver/arch/x86/kvm/x86.c 2017-01-13 09:08:51.557785166 -0200
@@ -6121,6 +6121,44 @@
}
EXPORT_SYMBOL_GPL(kvm_emulate_halt);
+static int kvm_pv_clock_offset(struct kvm_vcpu *vcpu, gpa_t paddr,
+ unsigned long clock_type)
+{
+ struct kvm_clock_offset *clock_offset;
+ struct timespec ts;
+ cycle_t cycle;
+ int ret;
+
+ if (clock_type != KVM_CLOCK_OFFSET_WALLCLOCK)
+ return -KVM_EOPNOTSUPP;
+
+ if (pvclock_gtod_data.clock.vclock_mode != VCLOCK_TSC)
+ return -KVM_EOPNOTSUPP;
+
+ clock_offset = kzalloc(sizeof(struct kvm_clock_offset), GFP_KERNEL);
+ if (clock_offset == NULL)
+ return -KVM_ENOMEM;
+
+ if (kvm_get_walltime_and_clockread(&ts, &cycle) == false) {
+ kfree(clock_offset);
+ return -KVM_EOPNOTSUPP;
+ }
+
+ clock_offset->sec = ts.tv_sec;
+ clock_offset->nsec = ts.tv_nsec;
+ clock_offset->tsc = kvm_read_l1_tsc(vcpu, cycle);
+ clock_offset->flags = 0;
+
+ ret = 0;
+ if (kvm_write_guest(vcpu->kvm, paddr, clock_offset,
+ sizeof(struct kvm_clock_offset)))
+ ret = -KVM_EFAULT;
+
+ kfree(clock_offset);
+
+ return ret;
+}
+
/*
* kvm_pv_kick_cpu_op: Kick a vcpu.
*
@@ -6185,6 +6223,9 @@
kvm_pv_kick_cpu_op(vcpu->kvm, a0, a1);
ret = 0;
break;
+ case KVM_HC_CLOCK_OFFSET:
+ ret = kvm_pv_clock_offset(vcpu, a0, a1);
+ break;
default:
ret = -KVM_ENOSYS;
break;
Index: kvm-ptpdriver/include/uapi/linux/kvm_para.h
===================================================================
--- kvm-ptpdriver.orig/include/uapi/linux/kvm_para.h 2017-01-13 09:03:33.823305270 -0200
+++ kvm-ptpdriver/include/uapi/linux/kvm_para.h 2017-01-13 09:07:54.636699274 -0200
@@ -14,6 +14,8 @@
#define KVM_EFAULT EFAULT
#define KVM_E2BIG E2BIG
#define KVM_EPERM EPERM
+#define KVM_EOPNOTSUPP EOPNOTSUPP
+#define KVM_ENOMEM ENOMEM
#define KVM_HC_VAPIC_POLL_IRQ 1
#define KVM_HC_MMU_OP 2
@@ -23,6 +25,7 @@
#define KVM_HC_MIPS_GET_CLOCK_FREQ 6
#define KVM_HC_MIPS_EXIT_VM 7
#define KVM_HC_MIPS_CONSOLE_OUTPUT 8
+#define KVM_HC_CLOCK_OFFSET 9
/*
* hypercalls use architecture specific
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [patch 2/3] KVM: x86: add KVM_HC_CLOCK_OFFSET hypercall
2017-01-13 12:01 ` [patch 2/3] KVM: x86: add KVM_HC_CLOCK_OFFSET hypercall Marcelo Tosatti
@ 2017-01-13 15:31 ` Radim Krcmar
2017-01-13 15:43 ` Marcelo Tosatti
0 siblings, 1 reply; 9+ messages in thread
From: Radim Krcmar @ 2017-01-13 15:31 UTC (permalink / raw)
To: Marcelo Tosatti
Cc: kvm, linux-kernel, Paolo Bonzini, Richard Cochran, Miroslav Lichvar
2017-01-13 10:01-0200, Marcelo Tosatti:
> 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@redhat.com>
>
> ---
> Documentation/virtual/kvm/hypercalls.txt | 30 ++++++++++++++++++++++
> 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, 83 insertions(+)
>
> 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 09:03:33.823305270 -0200
> +++ kvm-ptpdriver/arch/x86/include/uapi/asm/kvm_para.h 2017-01-13 09:07:54.633699270 -0200
> @@ -50,6 +50,15 @@
> __u32 pad[11];
> };
>
> +#define KVM_CLOCK_OFFSET_WALLCLOCK 0
> +struct kvm_clock_offset {
> + __s64 sec;
> + __s64 nsec;
> + __u64 tsc;
> + __u32 flags;
> + __u32 pad[1];
> +};
> +
> #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 09:03:33.823305270 -0200
> +++ kvm-ptpdriver/Documentation/virtual/kvm/hypercalls.txt 2017-01-13 09:15:32.971389949 -0200
> @@ -81,3 +81,33 @@
> 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_OFFSET
> +------------------------
> +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_OFFSET_WALLCLOCK (0)
> +is supported (hosts CLOCK_REALTIME clock).
> +
> + struct kvm_clock_offset {
> + __s64 sec;
> + __s64 nsec;
Why is nsec:
1) signed -- it is a remainder after division by NSEC_PER_SEC
2) bigger than 32 bit -- NSEC_PER_SEC < 2^32
?
> + __u64 tsc;
> + __u32 flags;
> + __u32 pad;
> + };
> +
> + Where:
> + * sec: seconds from clock_type clock.
> + * nsec: nanoseconds from clock_type clock.
The important part of an offset is the starting point -- I assume it is
the the usual one, but documentation better be explicit.
> + * tsc: TSC value used to calculate sec/nsec pair
This hypercall could in theory work even when computing the offset with
a different clocksource on the host and the estimating TSC at that time.
> + (this hypercall only works when host uses TSC clocksource).
Describe that error as a return value.
> + * flags: flags, unused at the moment.
unused = 0.
> +
> Index: kvm-ptpdriver/arch/x86/kvm/x86.c
> ===================================================================
> --- kvm-ptpdriver.orig/arch/x86/kvm/x86.c 2017-01-13 09:04:46.581415259 -0200
> +++ kvm-ptpdriver/arch/x86/kvm/x86.c 2017-01-13 09:08:51.557785166 -0200
> @@ -6121,6 +6121,44 @@
> }
> EXPORT_SYMBOL_GPL(kvm_emulate_halt);
>
> +static int kvm_pv_clock_offset(struct kvm_vcpu *vcpu, gpa_t paddr,
> + unsigned long clock_type)
> +{
> + struct kvm_clock_offset *clock_offset;
> + struct timespec ts;
> + cycle_t cycle;
> + int ret;
> +
> + if (clock_type != KVM_CLOCK_OFFSET_WALLCLOCK)
> + return -KVM_EOPNOTSUPP;
> +
> + if (pvclock_gtod_data.clock.vclock_mode != VCLOCK_TSC)
> + return -KVM_EOPNOTSUPP;
> +
> + clock_offset = kzalloc(sizeof(struct kvm_clock_offset), GFP_KERNEL);
Can't clock_offset be on the stack?
> + if (clock_offset == NULL)
> + return -KVM_ENOMEM;
> +
> + if (kvm_get_walltime_and_clockread(&ts, &cycle) == false) {
> + kfree(clock_offset);
> + return -KVM_EOPNOTSUPP;
> + }
> +
> + clock_offset->sec = ts.tv_sec;
> + clock_offset->nsec = ts.tv_nsec;
> + clock_offset->tsc = kvm_read_l1_tsc(vcpu, cycle);
> + clock_offset->flags = 0;
> +
> + ret = 0;
> + if (kvm_write_guest(vcpu->kvm, paddr, clock_offset,
> + sizeof(struct kvm_clock_offset)))
> + ret = -KVM_EFAULT;
> +
> + kfree(clock_offset);
> +
> + return ret;
> +}
> +
> /*
> * kvm_pv_kick_cpu_op: Kick a vcpu.
> *
> @@ -6185,6 +6223,9 @@
> kvm_pv_kick_cpu_op(vcpu->kvm, a0, a1);
> ret = 0;
> break;
> + case KVM_HC_CLOCK_OFFSET:
> + ret = kvm_pv_clock_offset(vcpu, a0, a1);
> + break;
> default:
> ret = -KVM_ENOSYS;
> break;
> Index: kvm-ptpdriver/include/uapi/linux/kvm_para.h
> ===================================================================
> --- kvm-ptpdriver.orig/include/uapi/linux/kvm_para.h 2017-01-13 09:03:33.823305270 -0200
> +++ kvm-ptpdriver/include/uapi/linux/kvm_para.h 2017-01-13 09:07:54.636699274 -0200
> @@ -14,6 +14,8 @@
> #define KVM_EFAULT EFAULT
> #define KVM_E2BIG E2BIG
> #define KVM_EPERM EPERM
> +#define KVM_EOPNOTSUPP EOPNOTSUPP
> +#define KVM_ENOMEM ENOMEM
>
> #define KVM_HC_VAPIC_POLL_IRQ 1
> #define KVM_HC_MMU_OP 2
> @@ -23,6 +25,7 @@
> #define KVM_HC_MIPS_GET_CLOCK_FREQ 6
> #define KVM_HC_MIPS_EXIT_VM 7
> #define KVM_HC_MIPS_CONSOLE_OUTPUT 8
> +#define KVM_HC_CLOCK_OFFSET 9
>
> /*
> * hypercalls use architecture specific
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [patch 2/3] KVM: x86: add KVM_HC_CLOCK_OFFSET hypercall
2017-01-13 15:31 ` Radim Krcmar
@ 2017-01-13 15:43 ` Marcelo Tosatti
2017-01-13 17:07 ` Radim Krcmar
0 siblings, 1 reply; 9+ messages in thread
From: Marcelo Tosatti @ 2017-01-13 15:43 UTC (permalink / raw)
To: Radim Krcmar
Cc: kvm, linux-kernel, Paolo Bonzini, Richard Cochran, Miroslav Lichvar
On Fri, Jan 13, 2017 at 04:31:58PM +0100, Radim Krcmar wrote:
> 2017-01-13 10:01-0200, Marcelo Tosatti:
> > 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@redhat.com>
> >
> > ---
> > Documentation/virtual/kvm/hypercalls.txt | 30 ++++++++++++++++++++++
> > 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, 83 insertions(+)
> >
> > 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 09:03:33.823305270 -0200
> > +++ kvm-ptpdriver/arch/x86/include/uapi/asm/kvm_para.h 2017-01-13 09:07:54.633699270 -0200
> > @@ -50,6 +50,15 @@
> > __u32 pad[11];
> > };
> >
> > +#define KVM_CLOCK_OFFSET_WALLCLOCK 0
> > +struct kvm_clock_offset {
> > + __s64 sec;
> > + __s64 nsec;
> > + __u64 tsc;
> > + __u32 flags;
> > + __u32 pad[1];
> > +};
> > +
> > #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 09:03:33.823305270 -0200
> > +++ kvm-ptpdriver/Documentation/virtual/kvm/hypercalls.txt 2017-01-13 09:15:32.971389949 -0200
> > @@ -81,3 +81,33 @@
> > 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_OFFSET
> > +------------------------
> > +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_OFFSET_WALLCLOCK (0)
> > +is supported (hosts CLOCK_REALTIME clock).
> > +
> > + struct kvm_clock_offset {
> > + __s64 sec;
> > + __s64 nsec;
>
> Why is nsec:
> 1) signed -- it is a remainder after division by NSEC_PER_SEC
Because "struct timespec" is signed because it can be used for
time deltas (you won't actually get signed values for
kvm_get_walltime_and_clockread).
Just wanted to match "struct timespec".
> 2) bigger than 32 bit -- NSEC_PER_SEC < 2^32
> ?
Again matching struct timespec.
> > + __u64 tsc;
> > + __u32 flags;
> > + __u32 pad;
> > + };
> > +
> > + Where:
> > + * sec: seconds from clock_type clock.
> > + * nsec: nanoseconds from clock_type clock.
>
> The important part of an offset is the starting point -- I assume it is
> the the usual one, but documentation better be explicit.
Don't get what you mean? (the values have same meaning as hosts
clock_gettime(CLOCK_REALTIME), supposedly that is clear).
> > + * tsc: TSC value used to calculate sec/nsec pair
>
> This hypercall could in theory work even when computing the offset with
> a different clocksource on the host and the estimating TSC at that time.
Yes it could.
> > + (this hypercall only works when host uses TSC clocksource).
>
> Describe that error as a return value.
Fixed.
+
+ if (kvm_get_walltime_and_clockread(&ts, &cycle) == false) {
+ kfree(clock_offset);
+ return -KVM_EOPNOTSUPP;
+ }
>
> > + * flags: flags, unused at the moment.
>
> unused = 0.
Fixed.
> > +
> > Index: kvm-ptpdriver/arch/x86/kvm/x86.c
> > ===================================================================
> > --- kvm-ptpdriver.orig/arch/x86/kvm/x86.c 2017-01-13 09:04:46.581415259 -0200
> > +++ kvm-ptpdriver/arch/x86/kvm/x86.c 2017-01-13 09:08:51.557785166 -0200
> > @@ -6121,6 +6121,44 @@
> > }
> > EXPORT_SYMBOL_GPL(kvm_emulate_halt);
> >
> > +static int kvm_pv_clock_offset(struct kvm_vcpu *vcpu, gpa_t paddr,
> > + unsigned long clock_type)
> > +{
> > + struct kvm_clock_offset *clock_offset;
> > + struct timespec ts;
> > + cycle_t cycle;
> > + int ret;
> > +
> > + if (clock_type != KVM_CLOCK_OFFSET_WALLCLOCK)
> > + return -KVM_EOPNOTSUPP;
> > +
> > + if (pvclock_gtod_data.clock.vclock_mode != VCLOCK_TSC)
> > + return -KVM_EOPNOTSUPP;
> > +
> > + clock_offset = kzalloc(sizeof(struct kvm_clock_offset), GFP_KERNEL);
>
> Can't clock_offset be on the stack?
Well since the kernel stack is limited, thought it would be safer to
allocate it.
> > + if (clock_offset == NULL)
> > + return -KVM_ENOMEM;
> > +
> > + if (kvm_get_walltime_and_clockread(&ts, &cycle) == false) {
> > + kfree(clock_offset);
> > + return -KVM_EOPNOTSUPP;
> > + }
> > +
> > + clock_offset->sec = ts.tv_sec;
> > + clock_offset->nsec = ts.tv_nsec;
> > + clock_offset->tsc = kvm_read_l1_tsc(vcpu, cycle);
> > + clock_offset->flags = 0;
> > +
> > + ret = 0;
> > + if (kvm_write_guest(vcpu->kvm, paddr, clock_offset,
> > + sizeof(struct kvm_clock_offset)))
> > + ret = -KVM_EFAULT;
> > +
> > + kfree(clock_offset);
> > +
> > + return ret;
> > +}
> > +
> > /*
> > * kvm_pv_kick_cpu_op: Kick a vcpu.
> > *
> > @@ -6185,6 +6223,9 @@
> > kvm_pv_kick_cpu_op(vcpu->kvm, a0, a1);
> > ret = 0;
> > break;
> > + case KVM_HC_CLOCK_OFFSET:
> > + ret = kvm_pv_clock_offset(vcpu, a0, a1);
> > + break;
> > default:
> > ret = -KVM_ENOSYS;
> > break;
> > Index: kvm-ptpdriver/include/uapi/linux/kvm_para.h
> > ===================================================================
> > --- kvm-ptpdriver.orig/include/uapi/linux/kvm_para.h 2017-01-13 09:03:33.823305270 -0200
> > +++ kvm-ptpdriver/include/uapi/linux/kvm_para.h 2017-01-13 09:07:54.636699274 -0200
> > @@ -14,6 +14,8 @@
> > #define KVM_EFAULT EFAULT
> > #define KVM_E2BIG E2BIG
> > #define KVM_EPERM EPERM
> > +#define KVM_EOPNOTSUPP EOPNOTSUPP
> > +#define KVM_ENOMEM ENOMEM
> >
> > #define KVM_HC_VAPIC_POLL_IRQ 1
> > #define KVM_HC_MMU_OP 2
> > @@ -23,6 +25,7 @@
> > #define KVM_HC_MIPS_GET_CLOCK_FREQ 6
> > #define KVM_HC_MIPS_EXIT_VM 7
> > #define KVM_HC_MIPS_CONSOLE_OUTPUT 8
> > +#define KVM_HC_CLOCK_OFFSET 9
> >
> > /*
> > * hypercalls use architecture specific
> >
> >
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [patch 2/3] KVM: x86: add KVM_HC_CLOCK_OFFSET hypercall
2017-01-13 15:43 ` Marcelo Tosatti
@ 2017-01-13 17:07 ` Radim Krcmar
2017-01-13 17:57 ` Marcelo Tosatti
0 siblings, 1 reply; 9+ messages in thread
From: Radim Krcmar @ 2017-01-13 17:07 UTC (permalink / raw)
To: Marcelo Tosatti
Cc: kvm, linux-kernel, Paolo Bonzini, Richard Cochran, Miroslav Lichvar
2017-01-13 13:43-0200, Marcelo Tosatti:
> On Fri, Jan 13, 2017 at 04:31:58PM +0100, Radim Krcmar wrote:
>> 2017-01-13 10:01-0200, Marcelo Tosatti:
>> > 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@redhat.com>
>> >
>> > ---
>> > Index: kvm-ptpdriver/Documentation/virtual/kvm/hypercalls.txt
>> > @@ -81,3 +81,33 @@
>> > +6. KVM_HC_CLOCK_OFFSET
>> > +------------------------
>> > +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_OFFSET_WALLCLOCK (0)
>> > +is supported (hosts CLOCK_REALTIME clock).
>> > +
>> > + struct kvm_clock_offset {
>> > + __s64 sec;
>> > + __s64 nsec;
>>
>> Why is nsec:
>> 1) signed -- it is a remainder after division by NSEC_PER_SEC
>
> Because "struct timespec" is signed because it can be used for
> time deltas (you won't actually get signed values for
> kvm_get_walltime_and_clockread).
>
> Just wanted to match "struct timespec".
>
>> 2) bigger than 32 bit -- NSEC_PER_SEC < 2^32
>> ?
>
> Again matching struct timespec.
It is "long" in struct timespec, which could also be "s32" ...
I'd rather waste those 8 bytes inside padding -- its purpose is clear
there. :)
>> > + __u64 tsc;
>> > + __u32 flags;
>> > + __u32 pad;
>> > + };
>> > +
>> > + Where:
>> > + * sec: seconds from clock_type clock.
>> > + * nsec: nanoseconds from clock_type clock.
>>
>> The important part of an offset is the starting point -- I assume it is
>> the the usual one, but documentation better be explicit.
>
> Don't get what you mean? (the values have same meaning as hosts
> clock_gettime(CLOCK_REALTIME), supposedly that is clear).
Ah, I didn't understand that clock_type was refering to CLOCK_REALTIME.
I'd drop offset from the hypercall name. IIUC, all various clock types
would be compared to TSC, so we could name the hypercall somewhat like
KVM_HC_CLOCK_AT_TSC -- we want to know what was the time on the selected
clock when TSC was at value __u64 tsc.
The only offset is between sec+nsec and the beginning of time (and tsc
and 0), but we don't care about that offset by itself -- we care about
relation of sec+nsec to tsc, which isn't a simple offset as they flow
differently.
If we wanted to be more generic, then KVM_HC_CLOCK_PAIRING/SNAPSHOT/...
and argument 1 would contain clock_types, here REALTIME and TSC.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [patch 2/3] KVM: x86: add KVM_HC_CLOCK_OFFSET hypercall
2017-01-13 17:07 ` Radim Krcmar
@ 2017-01-13 17:57 ` Marcelo Tosatti
0 siblings, 0 replies; 9+ messages in thread
From: Marcelo Tosatti @ 2017-01-13 17:57 UTC (permalink / raw)
To: Radim Krcmar
Cc: kvm, linux-kernel, Paolo Bonzini, Richard Cochran, Miroslav Lichvar
On Fri, Jan 13, 2017 at 06:07:40PM +0100, Radim Krcmar wrote:
> 2017-01-13 13:43-0200, Marcelo Tosatti:
> > On Fri, Jan 13, 2017 at 04:31:58PM +0100, Radim Krcmar wrote:
> >> 2017-01-13 10:01-0200, Marcelo Tosatti:
> >> > 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@redhat.com>
> >> >
> >> > ---
> >> > Index: kvm-ptpdriver/Documentation/virtual/kvm/hypercalls.txt
> >> > @@ -81,3 +81,33 @@
> >> > +6. KVM_HC_CLOCK_OFFSET
> >> > +------------------------
> >> > +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_OFFSET_WALLCLOCK (0)
> >> > +is supported (hosts CLOCK_REALTIME clock).
> >> > +
> >> > + struct kvm_clock_offset {
> >> > + __s64 sec;
> >> > + __s64 nsec;
> >>
> >> Why is nsec:
> >> 1) signed -- it is a remainder after division by NSEC_PER_SEC
> >
> > Because "struct timespec" is signed because it can be used for
> > time deltas (you won't actually get signed values for
> > kvm_get_walltime_and_clockread).
> >
> > Just wanted to match "struct timespec".
> >
> >> 2) bigger than 32 bit -- NSEC_PER_SEC < 2^32
> >> ?
> >
> > Again matching struct timespec.
>
> It is "long" in struct timespec, which could also be "s32" ...
Which fits in a s64.
> I'd rather waste those 8 bytes inside padding -- its purpose is clear
> there. :)
The purpose of the s64 is to match timespec.
> >> > + __u64 tsc;
> >> > + __u32 flags;
> >> > + __u32 pad;
> >> > + };
> >> > +
> >> > + Where:
> >> > + * sec: seconds from clock_type clock.
> >> > + * nsec: nanoseconds from clock_type clock.
> >>
> >> The important part of an offset is the starting point -- I assume it is
> >> the the usual one, but documentation better be explicit.
> >
> > Don't get what you mean? (the values have same meaning as hosts
> > clock_gettime(CLOCK_REALTIME), supposedly that is clear).
>
> Ah, I didn't understand that clock_type was refering to CLOCK_REALTIME.
>
> I'd drop offset from the hypercall name. IIUC, all various clock types
> would be compared to TSC, so we could name the hypercall somewhat like
> KVM_HC_CLOCK_AT_TSC -- we want to know what was the time on the selected
> clock when TSC was at value __u64 tsc.
Well its the offset from TSC, so thats why "offset" in the name.
And as you say, the interface could be extended to support
other clocks (using the available data in pad, i'll make it larger BTW).
Which would render the "TSC" from the hypercall name invalid.
> The only offset is between sec+nsec and the beginning of time (and tsc
> and 0), but we don't care about that offset by itself -- we care about
> relation of sec+nsec to tsc, which isn't a simple offset as they flow
> differently.
>
> If we wanted to be more generic, then KVM_HC_CLOCK_PAIRING/SNAPSHOT/...
> and argument 1 would contain clock_types, here REALTIME and TSC.
Pairing is fine, changed.
^ permalink raw reply [flat|nested] 9+ messages in thread