Given that this series[0] has languished in my Inbox for the best of the
past two years, and in an effort to eventually get it merged, I've
taken the liberty to pick it up and do the changes I wanted to see
instead of waiting to go through yet another round.
All the patches have a link to their original counterpart (though I
have squashed a couple of them where it made sense). Tested both 64
and 32bit guests for a good measure. Of course, I claim full
responsibility for any bug introduced here.
Unless someone screams now, this is going in 5.13, because I'm frankly
fed up with it! ;-)
* From v18 [2]
- Fix kvm_hypercall2() return type
- Rebased on top of 5.12-rc3
- Added RBs
* From v17 [1]:
- Fixed compilation issue on 32bit systems not selecting
CONFIG_HAVE_ARM_SMCCC_DISCOVERY
- Fixed KVM service discovery not properly parsing the reply
from the hypervisor
* From v16 [0]:
- Moved the KVM service discovery to its own file, plugged it into
PSCI instead of the arch code, dropped the inlining, made use of
asm/hypervisor.h.
- Tidied-up the namespacing
- Cleanup the hypercall handler
- De-duplicate the guest code
- Tidied-up arm64-specific documentation
- Dropped the generic PTP documentation as it needs a new location,
and some cleanup
- Squashed hypercall documentation and capability into the
main KVM patch
- Rebased on top of 5.11-rc4
[0] https://lore.kernel.org/r/[email protected]
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
Jianyong Wu (4):
ptp: Reorganize ptp_kvm.c to make it arch-independent
clocksource: Add clocksource id for arm arch counter
KVM: arm64: Add support for the KVM PTP service
ptp: arm/arm64: Enable ptp_kvm for arm/arm64
Thomas Gleixner (1):
time: Add mechanism to recognize clocksource in time_get_snapshot
Will Deacon (2):
arm/arm64: Probe for the presence of KVM hypervisor
KVM: arm64: Advertise KVM UID to guests via SMCCC
Documentation/virt/kvm/api.rst | 10 +++
Documentation/virt/kvm/arm/index.rst | 1 +
Documentation/virt/kvm/arm/ptp_kvm.rst | 25 ++++++
arch/arm/include/asm/hypervisor.h | 3 +
arch/arm64/include/asm/hypervisor.h | 3 +
arch/arm64/kvm/arm.c | 1 +
arch/arm64/kvm/hypercalls.c | 80 +++++++++++++++--
drivers/clocksource/arm_arch_timer.c | 36 ++++++++
drivers/firmware/psci/psci.c | 2 +
drivers/firmware/smccc/Makefile | 2 +-
drivers/firmware/smccc/kvm_guest.c | 50 +++++++++++
drivers/firmware/smccc/smccc.c | 1 +
drivers/ptp/Kconfig | 2 +-
drivers/ptp/Makefile | 2 +
drivers/ptp/ptp_kvm_arm.c | 28 ++++++
drivers/ptp/{ptp_kvm.c => ptp_kvm_common.c} | 84 +++++-------------
drivers/ptp/ptp_kvm_x86.c | 97 +++++++++++++++++++++
include/linux/arm-smccc.h | 41 +++++++++
include/linux/clocksource.h | 6 ++
include/linux/clocksource_ids.h | 12 +++
include/linux/ptp_kvm.h | 19 ++++
include/linux/timekeeping.h | 12 +--
include/uapi/linux/kvm.h | 1 +
kernel/time/clocksource.c | 2 +
kernel/time/timekeeping.c | 1 +
25 files changed, 443 insertions(+), 78 deletions(-)
create mode 100644 Documentation/virt/kvm/arm/ptp_kvm.rst
create mode 100644 drivers/firmware/smccc/kvm_guest.c
create mode 100644 drivers/ptp/ptp_kvm_arm.c
rename drivers/ptp/{ptp_kvm.c => ptp_kvm_common.c} (60%)
create mode 100644 drivers/ptp/ptp_kvm_x86.c
create mode 100644 include/linux/clocksource_ids.h
create mode 100644 include/linux/ptp_kvm.h
--
2.29.2
From: Will Deacon <[email protected]>
We can advertise ourselves to guests as KVM and provide a basic features
bitmap for discoverability of future hypervisor services.
Cc: Marc Zyngier <[email protected]>
Signed-off-by: Will Deacon <[email protected]>
Signed-off-by: Jianyong Wu <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/arm64/kvm/hypercalls.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index ead21b98b620..78d32c34d49c 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -12,13 +12,13 @@
int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
{
u32 func_id = smccc_get_function(vcpu);
- long val = SMCCC_RET_NOT_SUPPORTED;
+ u64 val[4] = {SMCCC_RET_NOT_SUPPORTED};
u32 feature;
gpa_t gpa;
switch (func_id) {
case ARM_SMCCC_VERSION_FUNC_ID:
- val = ARM_SMCCC_VERSION_1_1;
+ val[0] = ARM_SMCCC_VERSION_1_1;
break;
case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
feature = smccc_get_arg1(vcpu);
@@ -28,10 +28,10 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
case SPECTRE_VULNERABLE:
break;
case SPECTRE_MITIGATED:
- val = SMCCC_RET_SUCCESS;
+ val[0] = SMCCC_RET_SUCCESS;
break;
case SPECTRE_UNAFFECTED:
- val = SMCCC_ARCH_WORKAROUND_RET_UNAFFECTED;
+ val[0] = SMCCC_ARCH_WORKAROUND_RET_UNAFFECTED;
break;
}
break;
@@ -54,22 +54,31 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
break;
fallthrough;
case SPECTRE_UNAFFECTED:
- val = SMCCC_RET_NOT_REQUIRED;
+ val[0] = SMCCC_RET_NOT_REQUIRED;
break;
}
break;
case ARM_SMCCC_HV_PV_TIME_FEATURES:
- val = SMCCC_RET_SUCCESS;
+ val[0] = SMCCC_RET_SUCCESS;
break;
}
break;
case ARM_SMCCC_HV_PV_TIME_FEATURES:
- val = kvm_hypercall_pv_features(vcpu);
+ val[0] = kvm_hypercall_pv_features(vcpu);
break;
case ARM_SMCCC_HV_PV_TIME_ST:
gpa = kvm_init_stolen_time(vcpu);
if (gpa != GPA_INVALID)
- val = gpa;
+ val[0] = gpa;
+ break;
+ case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
+ val[0] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0;
+ val[1] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1;
+ val[2] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2;
+ val[3] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3;
+ break;
+ case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
+ val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
break;
case ARM_SMCCC_TRNG_VERSION:
case ARM_SMCCC_TRNG_FEATURES:
@@ -81,6 +90,6 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
return kvm_psci_call(vcpu);
}
- smccc_set_retval(vcpu, val, 0, 0, 0);
+ smccc_set_retval(vcpu, val[0], val[1], val[2], val[3]);
return 1;
}
--
2.29.2
From: Jianyong Wu <[email protected]>
Currently, the ptp_kvm module contains a lot of x86-specific code.
Let's move this code into a new arch-specific file in the same directory,
and rename the arch-independent file to ptp_kvm_common.c.
Reviewed-by: Andre Przywara <[email protected]>
Signed-off-by: Jianyong Wu <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/ptp/Makefile | 1 +
drivers/ptp/{ptp_kvm.c => ptp_kvm_common.c} | 84 +++++-------------
drivers/ptp/ptp_kvm_x86.c | 97 +++++++++++++++++++++
include/linux/ptp_kvm.h | 19 ++++
4 files changed, 139 insertions(+), 62 deletions(-)
rename drivers/ptp/{ptp_kvm.c => ptp_kvm_common.c} (60%)
create mode 100644 drivers/ptp/ptp_kvm_x86.c
create mode 100644 include/linux/ptp_kvm.h
diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
index db5aef3bddc6..d11eeb5811d1 100644
--- a/drivers/ptp/Makefile
+++ b/drivers/ptp/Makefile
@@ -4,6 +4,7 @@
#
ptp-y := ptp_clock.o ptp_chardev.o ptp_sysfs.o
+ptp_kvm-$(CONFIG_X86) := ptp_kvm_x86.o ptp_kvm_common.o
obj-$(CONFIG_PTP_1588_CLOCK) += ptp.o
obj-$(CONFIG_PTP_1588_CLOCK_DTE) += ptp_dte.o
obj-$(CONFIG_PTP_1588_CLOCK_INES) += ptp_ines.o
diff --git a/drivers/ptp/ptp_kvm.c b/drivers/ptp/ptp_kvm_common.c
similarity index 60%
rename from drivers/ptp/ptp_kvm.c
rename to drivers/ptp/ptp_kvm_common.c
index 658d33fc3195..721ddcede5e1 100644
--- a/drivers/ptp/ptp_kvm.c
+++ b/drivers/ptp/ptp_kvm_common.c
@@ -8,11 +8,11 @@
#include <linux/err.h>
#include <linux/init.h>
#include <linux/kernel.h>
+#include <linux/slab.h>
#include <linux/module.h>
+#include <linux/ptp_kvm.h>
#include <uapi/linux/kvm_para.h>
#include <asm/kvm_para.h>
-#include <asm/pvclock.h>
-#include <asm/kvmclock.h>
#include <uapi/asm/kvm_para.h>
#include <linux/ptp_clock_kernel.h>
@@ -24,56 +24,29 @@ struct kvm_ptp_clock {
static DEFINE_SPINLOCK(kvm_ptp_lock);
-static struct pvclock_vsyscall_time_info *hv_clock;
-
-static struct kvm_clock_pairing clock_pair;
-static phys_addr_t clock_pair_gpa;
-
static int ptp_kvm_get_time_fn(ktime_t *device_time,
struct system_counterval_t *system_counter,
void *ctx)
{
- unsigned long ret;
+ long ret;
+ u64 cycle;
struct timespec64 tspec;
- unsigned version;
- int cpu;
- struct pvclock_vcpu_time_info *src;
+ struct clocksource *cs;
spin_lock(&kvm_ptp_lock);
preempt_disable_notrace();
- cpu = smp_processor_id();
- src = &hv_clock[cpu].pvti;
-
- do {
- /*
- * We are using a TSC value read in the hosts
- * kvm_hc_clock_pairing handling.
- * So any changes to tsc_to_system_mul
- * and tsc_shift or any other pvclock
- * data invalidate that measurement.
- */
- version = pvclock_read_begin(src);
-
- ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING,
- clock_pair_gpa,
- KVM_CLOCK_PAIRING_WALLCLOCK);
- if (ret != 0) {
- pr_err_ratelimited("clock pairing hypercall ret %lu\n", ret);
- spin_unlock(&kvm_ptp_lock);
- preempt_enable_notrace();
- return -EOPNOTSUPP;
- }
-
- tspec.tv_sec = clock_pair.sec;
- tspec.tv_nsec = clock_pair.nsec;
- ret = __pvclock_read_cycles(src, clock_pair.tsc);
- } while (pvclock_read_retry(src, version));
+ ret = kvm_arch_ptp_get_crosststamp(&cycle, &tspec, &cs);
+ if (ret) {
+ spin_unlock(&kvm_ptp_lock);
+ preempt_enable_notrace();
+ return ret;
+ }
preempt_enable_notrace();
- system_counter->cycles = ret;
- system_counter->cs = &kvm_clock;
+ system_counter->cycles = cycle;
+ system_counter->cs = cs;
*device_time = timespec64_to_ktime(tspec);
@@ -111,22 +84,17 @@ static int ptp_kvm_settime(struct ptp_clock_info *ptp,
static int ptp_kvm_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
{
- unsigned long ret;
+ long ret;
struct timespec64 tspec;
spin_lock(&kvm_ptp_lock);
- ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING,
- clock_pair_gpa,
- KVM_CLOCK_PAIRING_WALLCLOCK);
- if (ret != 0) {
- pr_err_ratelimited("clock offset hypercall ret %lu\n", ret);
+ ret = kvm_arch_ptp_get_clock(&tspec);
+ if (ret) {
spin_unlock(&kvm_ptp_lock);
- return -EOPNOTSUPP;
+ return ret;
}
- tspec.tv_sec = clock_pair.sec;
- tspec.tv_nsec = clock_pair.nsec;
spin_unlock(&kvm_ptp_lock);
memcpy(ts, &tspec, sizeof(struct timespec64));
@@ -168,19 +136,11 @@ static int __init ptp_kvm_init(void)
{
long ret;
- if (!kvm_para_available())
- return -ENODEV;
-
- clock_pair_gpa = slow_virt_to_phys(&clock_pair);
- hv_clock = pvclock_get_pvti_cpu0_va();
-
- if (!hv_clock)
- return -ENODEV;
-
- ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING, clock_pair_gpa,
- KVM_CLOCK_PAIRING_WALLCLOCK);
- if (ret == -KVM_ENOSYS || ret == -KVM_EOPNOTSUPP)
- return -ENODEV;
+ ret = kvm_arch_ptp_init();
+ if (ret) {
+ pr_err("fail to initialize ptp_kvm");
+ return ret;
+ }
kvm_ptp_clock.caps = ptp_kvm_caps;
diff --git a/drivers/ptp/ptp_kvm_x86.c b/drivers/ptp/ptp_kvm_x86.c
new file mode 100644
index 000000000000..3dd519dfc473
--- /dev/null
+++ b/drivers/ptp/ptp_kvm_x86.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Virtual PTP 1588 clock for use with KVM guests
+ *
+ * Copyright (C) 2017 Red Hat Inc.
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <asm/pvclock.h>
+#include <asm/kvmclock.h>
+#include <linux/module.h>
+#include <uapi/asm/kvm_para.h>
+#include <uapi/linux/kvm_para.h>
+#include <linux/ptp_clock_kernel.h>
+#include <linux/ptp_kvm.h>
+
+struct pvclock_vsyscall_time_info *hv_clock;
+
+static phys_addr_t clock_pair_gpa;
+static struct kvm_clock_pairing clock_pair;
+
+int kvm_arch_ptp_init(void)
+{
+ long ret;
+
+ if (!kvm_para_available())
+ return -ENODEV;
+
+ clock_pair_gpa = slow_virt_to_phys(&clock_pair);
+ hv_clock = pvclock_get_pvti_cpu0_va();
+ if (!hv_clock)
+ return -ENODEV;
+
+ ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING, clock_pair_gpa,
+ KVM_CLOCK_PAIRING_WALLCLOCK);
+ if (ret == -KVM_ENOSYS || ret == -KVM_EOPNOTSUPP)
+ return -ENODEV;
+
+ return 0;
+}
+
+int kvm_arch_ptp_get_clock(struct timespec64 *ts)
+{
+ long ret;
+
+ ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING,
+ clock_pair_gpa,
+ KVM_CLOCK_PAIRING_WALLCLOCK);
+ if (ret != 0) {
+ pr_err_ratelimited("clock offset hypercall ret %lu\n", ret);
+ return -EOPNOTSUPP;
+ }
+
+ ts->tv_sec = clock_pair.sec;
+ ts->tv_nsec = clock_pair.nsec;
+
+ return 0;
+}
+
+int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *tspec,
+ struct clocksource **cs)
+{
+ struct pvclock_vcpu_time_info *src;
+ unsigned int version;
+ long ret;
+ int cpu;
+
+ cpu = smp_processor_id();
+ src = &hv_clock[cpu].pvti;
+
+ do {
+ /*
+ * We are using a TSC value read in the hosts
+ * kvm_hc_clock_pairing handling.
+ * So any changes to tsc_to_system_mul
+ * and tsc_shift or any other pvclock
+ * data invalidate that measurement.
+ */
+ version = pvclock_read_begin(src);
+
+ ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING,
+ clock_pair_gpa,
+ KVM_CLOCK_PAIRING_WALLCLOCK);
+ if (ret != 0) {
+ pr_err_ratelimited("clock pairing hypercall ret %lu\n", ret);
+ return -EOPNOTSUPP;
+ }
+ tspec->tv_sec = clock_pair.sec;
+ tspec->tv_nsec = clock_pair.nsec;
+ *cycle = __pvclock_read_cycles(src, clock_pair.tsc);
+ } while (pvclock_read_retry(src, version));
+
+ *cs = &kvm_clock;
+
+ return 0;
+}
diff --git a/include/linux/ptp_kvm.h b/include/linux/ptp_kvm.h
new file mode 100644
index 000000000000..f960a719f0d5
--- /dev/null
+++ b/include/linux/ptp_kvm.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Virtual PTP 1588 clock for use with KVM guests
+ *
+ * Copyright (C) 2017 Red Hat Inc.
+ */
+
+#ifndef _PTP_KVM_H_
+#define _PTP_KVM_H_
+
+struct timespec64;
+struct clocksource;
+
+int kvm_arch_ptp_init(void);
+int kvm_arch_ptp_get_clock(struct timespec64 *ts);
+int kvm_arch_ptp_get_crosststamp(u64 *cycle,
+ struct timespec64 *tspec, struct clocksource **cs);
+
+#endif /* _PTP_KVM_H_ */
--
2.29.2
From: Thomas Gleixner <[email protected]>
System time snapshots are not conveying information about the current
clocksource which was used, but callers like the PTP KVM guest
implementation have the requirement to evaluate the clocksource type to
select the appropriate mechanism.
Introduce a clocksource id field in struct clocksource which is by default
set to CSID_GENERIC (0). Clocksource implementations can set that field to
a value which allows to identify the clocksource.
Store the clocksource id of the current clocksource in the
system_time_snapshot so callers can evaluate which clocksource was used to
take the snapshot and act accordingly.
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Jianyong Wu <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
include/linux/clocksource.h | 6 ++++++
include/linux/clocksource_ids.h | 11 +++++++++++
include/linux/timekeeping.h | 12 +++++++-----
kernel/time/clocksource.c | 2 ++
kernel/time/timekeeping.c | 1 +
5 files changed, 27 insertions(+), 5 deletions(-)
create mode 100644 include/linux/clocksource_ids.h
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 86d143db6523..1290d0dce840 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -17,6 +17,7 @@
#include <linux/timer.h>
#include <linux/init.h>
#include <linux/of.h>
+#include <linux/clocksource_ids.h>
#include <asm/div64.h>
#include <asm/io.h>
@@ -62,6 +63,10 @@ struct module;
* 400-499: Perfect
* The ideal clocksource. A must-use where
* available.
+ * @id: Defaults to CSID_GENERIC. The id value is captured
+ * in certain snapshot functions to allow callers to
+ * validate the clocksource from which the snapshot was
+ * taken.
* @flags: Flags describing special properties
* @enable: Optional function to enable the clocksource
* @disable: Optional function to disable the clocksource
@@ -100,6 +105,7 @@ struct clocksource {
const char *name;
struct list_head list;
int rating;
+ enum clocksource_ids id;
enum vdso_clock_mode vdso_clock_mode;
unsigned long flags;
diff --git a/include/linux/clocksource_ids.h b/include/linux/clocksource_ids.h
new file mode 100644
index 000000000000..4d8e19e05328
--- /dev/null
+++ b/include/linux/clocksource_ids.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_CLOCKSOURCE_IDS_H
+#define _LINUX_CLOCKSOURCE_IDS_H
+
+/* Enum to give clocksources a unique identifier */
+enum clocksource_ids {
+ CSID_GENERIC = 0,
+ CSID_MAX,
+};
+
+#endif
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index c6792cf01bc7..78a98bdff76d 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -3,6 +3,7 @@
#define _LINUX_TIMEKEEPING_H
#include <linux/errno.h>
+#include <linux/clocksource_ids.h>
/* Included from linux/ktime.h */
@@ -243,11 +244,12 @@ struct ktime_timestamps {
* @cs_was_changed_seq: The sequence number of clocksource change events
*/
struct system_time_snapshot {
- u64 cycles;
- ktime_t real;
- ktime_t raw;
- unsigned int clock_was_set_seq;
- u8 cs_was_changed_seq;
+ u64 cycles;
+ ktime_t real;
+ ktime_t raw;
+ enum clocksource_ids cs_id;
+ unsigned int clock_was_set_seq;
+ u8 cs_was_changed_seq;
};
/**
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index cce484a2cc7c..4fe1df894ee5 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -920,6 +920,8 @@ int __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq)
clocksource_arch_init(cs);
+ if (WARN_ON_ONCE((unsigned int)cs->id >= CSID_MAX))
+ cs->id = CSID_GENERIC;
if (cs->vdso_clock_mode < 0 ||
cs->vdso_clock_mode >= VDSO_CLOCKMODE_MAX) {
pr_warn("clocksource %s registered with invalid VDSO mode %d. Disabling VDSO support.\n",
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 6aee5768c86f..06f55f9258bf 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1048,6 +1048,7 @@ void ktime_get_snapshot(struct system_time_snapshot *systime_snapshot)
do {
seq = read_seqcount_begin(&tk_core.seq);
now = tk_clock_read(&tk->tkr_mono);
+ systime_snapshot->cs_id = tk->tkr_mono.clock->id;
systime_snapshot->cs_was_changed_seq = tk->cs_was_changed_seq;
systime_snapshot->clock_was_set_seq = tk->clock_was_set_seq;
base_real = ktime_add(tk->tkr_mono.base,
--
2.29.2
From: Jianyong Wu <[email protected]>
Currently, there is no mechanism to keep time sync between guest and host
in arm/arm64 virtualization environment. Time in guest will drift compared
with host after boot up as they may both use third party time sources
to correct their time respectively. The time deviation will be in order
of milliseconds. But in some scenarios,like in cloud environment, we ask
for higher time precision.
kvm ptp clock, which chooses the host clock source as a reference
clock to sync time between guest and host, has been adopted by x86
which takes the time sync order from milliseconds to nanoseconds.
This patch enables kvm ptp clock for arm/arm64 and improves clock sync precision
significantly.
Test result comparisons between with kvm ptp clock and without it in arm/arm64
are as follows. This test derived from the result of command 'chronyc
sources'. we should take more care of the last sample column which shows
the offset between the local clock and the source at the last measurement.
no kvm ptp in guest:
MS Name/IP address Stratum Poll Reach LastRx Last sample
========================================================================
^* dns1.synet.edu.cn 2 6 377 13 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 21 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 29 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 37 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 45 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 53 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 61 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 4 -130us[ +796us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 12 -130us[ +796us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 20 -130us[ +796us] +/- 21ms
in host:
MS Name/IP address Stratum Poll Reach LastRx Last sample
========================================================================
^* 120.25.115.20 2 7 377 72 -470us[ -603us] +/- 18ms
^* 120.25.115.20 2 7 377 92 -470us[ -603us] +/- 18ms
^* 120.25.115.20 2 7 377 112 -470us[ -603us] +/- 18ms
^* 120.25.115.20 2 7 377 2 +872ns[-6808ns] +/- 17ms
^* 120.25.115.20 2 7 377 22 +872ns[-6808ns] +/- 17ms
^* 120.25.115.20 2 7 377 43 +872ns[-6808ns] +/- 17ms
^* 120.25.115.20 2 7 377 63 +872ns[-6808ns] +/- 17ms
^* 120.25.115.20 2 7 377 83 +872ns[-6808ns] +/- 17ms
^* 120.25.115.20 2 7 377 103 +872ns[-6808ns] +/- 17ms
^* 120.25.115.20 2 7 377 123 +872ns[-6808ns] +/- 17ms
The dns1.synet.edu.cn is the network reference clock for guest and
120.25.115.20 is the network reference clock for host. we can't get the
clock error between guest and host directly, but a roughly estimated value
will be in order of hundreds of us to ms.
with kvm ptp in guest:
chrony has been disabled in host to remove the disturb by network clock.
MS Name/IP address Stratum Poll Reach LastRx Last sample
========================================================================
* PHC0 0 3 377 8 -7ns[ +1ns] +/- 3ns
* PHC0 0 3 377 8 +1ns[ +16ns] +/- 3ns
* PHC0 0 3 377 6 -4ns[ -0ns] +/- 6ns
* PHC0 0 3 377 6 -8ns[ -12ns] +/- 5ns
* PHC0 0 3 377 5 +2ns[ +4ns] +/- 4ns
* PHC0 0 3 377 13 +2ns[ +4ns] +/- 4ns
* PHC0 0 3 377 12 -4ns[ -6ns] +/- 4ns
* PHC0 0 3 377 11 -8ns[ -11ns] +/- 6ns
* PHC0 0 3 377 10 -14ns[ -20ns] +/- 4ns
* PHC0 0 3 377 8 +4ns[ +5ns] +/- 4ns
The PHC0 is the ptp clock which choose the host clock as its source
clock. So we can see that the clock difference between host and guest
is in order of ns.
Cc: Mark Rutland <[email protected]>
Signed-off-by: Jianyong Wu <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/clocksource/arm_arch_timer.c | 34 ++++++++++++++++++++++++++++
drivers/ptp/Kconfig | 2 +-
drivers/ptp/Makefile | 1 +
drivers/ptp/ptp_kvm_arm.c | 28 +++++++++++++++++++++++
4 files changed, 64 insertions(+), 1 deletion(-)
create mode 100644 drivers/ptp/ptp_kvm_arm.c
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 8f12e223703f..e0f167e5e792 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -25,6 +25,8 @@
#include <linux/sched/clock.h>
#include <linux/sched_clock.h>
#include <linux/acpi.h>
+#include <linux/arm-smccc.h>
+#include <linux/ptp_kvm.h>
#include <asm/arch_timer.h>
#include <asm/virt.h>
@@ -1659,3 +1661,35 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)
}
TIMER_ACPI_DECLARE(arch_timer, ACPI_SIG_GTDT, arch_timer_acpi_init);
#endif
+
+int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *ts,
+ struct clocksource **cs)
+{
+ struct arm_smccc_res hvc_res;
+ u32 ptp_counter;
+ ktime_t ktime;
+
+ if (!IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY))
+ return -EOPNOTSUPP;
+
+ if (arch_timer_uses_ppi == ARCH_TIMER_VIRT_PPI)
+ ptp_counter = KVM_PTP_VIRT_COUNTER;
+ else
+ ptp_counter = KVM_PTP_PHYS_COUNTER;
+
+ arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID,
+ ptp_counter, &hvc_res);
+
+ if ((int)(hvc_res.a0) < 0)
+ return -EOPNOTSUPP;
+
+ ktime = (u64)hvc_res.a0 << 32 | hvc_res.a1;
+ *ts = ktime_to_timespec64(ktime);
+ if (cycle)
+ *cycle = (u64)hvc_res.a2 << 32 | hvc_res.a3;
+ if (cs)
+ *cs = &clocksource_counter;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(kvm_arch_ptp_get_crosststamp);
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index f2edef0df40f..8c20e524e9ad 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
config PTP_1588_CLOCK_KVM
tristate "KVM virtual PTP clock"
depends on PTP_1588_CLOCK
- depends on KVM_GUEST && X86
+ depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER)
default y
help
This driver adds support for using kvm infrastructure as a PTP
diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
index d11eeb5811d1..8673d1743faa 100644
--- a/drivers/ptp/Makefile
+++ b/drivers/ptp/Makefile
@@ -5,6 +5,7 @@
ptp-y := ptp_clock.o ptp_chardev.o ptp_sysfs.o
ptp_kvm-$(CONFIG_X86) := ptp_kvm_x86.o ptp_kvm_common.o
+ptp_kvm-$(CONFIG_HAVE_ARM_SMCCC) := ptp_kvm_arm.o ptp_kvm_common.o
obj-$(CONFIG_PTP_1588_CLOCK) += ptp.o
obj-$(CONFIG_PTP_1588_CLOCK_DTE) += ptp_dte.o
obj-$(CONFIG_PTP_1588_CLOCK_INES) += ptp_ines.o
diff --git a/drivers/ptp/ptp_kvm_arm.c b/drivers/ptp/ptp_kvm_arm.c
new file mode 100644
index 000000000000..b7d28c8dfb84
--- /dev/null
+++ b/drivers/ptp/ptp_kvm_arm.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Virtual PTP 1588 clock for use with KVM guests
+ * Copyright (C) 2019 ARM Ltd.
+ * All Rights Reserved
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/ptp_kvm.h>
+
+#include <asm/arch_timer.h>
+#include <asm/hypervisor.h>
+
+int kvm_arch_ptp_init(void)
+{
+ int ret;
+
+ ret = kvm_arm_hyp_service_available(ARM_SMCCC_KVM_FUNC_PTP);
+ if (ret <= 0)
+ return -EOPNOTSUPP;
+
+ return 0;
+}
+
+int kvm_arch_ptp_get_clock(struct timespec64 *ts)
+{
+ return kvm_arch_ptp_get_crosststamp(NULL, ts, NULL);
+}
--
2.29.2
From: Jianyong Wu <[email protected]>
Add clocksource id to the ARM generic counter so that it can be easily
identified from callers such as ptp_kvm.
Cc: Mark Rutland <[email protected]>
Reviewed-by: Andre Przywara <[email protected]>
Signed-off-by: Jianyong Wu <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/clocksource/arm_arch_timer.c | 2 ++
include/linux/clocksource_ids.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index d0177824c518..8f12e223703f 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -16,6 +16,7 @@
#include <linux/cpu_pm.h>
#include <linux/clockchips.h>
#include <linux/clocksource.h>
+#include <linux/clocksource_ids.h>
#include <linux/interrupt.h>
#include <linux/of_irq.h>
#include <linux/of_address.h>
@@ -191,6 +192,7 @@ static u64 arch_counter_read_cc(const struct cyclecounter *cc)
static struct clocksource clocksource_counter = {
.name = "arch_sys_counter",
+ .id = CSID_ARM_ARCH_COUNTER,
.rating = 400,
.read = arch_counter_read,
.mask = CLOCKSOURCE_MASK(56),
diff --git a/include/linux/clocksource_ids.h b/include/linux/clocksource_ids.h
index 4d8e19e05328..16775d7d8f8d 100644
--- a/include/linux/clocksource_ids.h
+++ b/include/linux/clocksource_ids.h
@@ -5,6 +5,7 @@
/* Enum to give clocksources a unique identifier */
enum clocksource_ids {
CSID_GENERIC = 0,
+ CSID_ARM_ARCH_COUNTER,
CSID_MAX,
};
--
2.29.2
From: Jianyong Wu <[email protected]>
Implement the hypervisor side of the KVM PTP interface.
The service offers wall time and cycle count from host to guest.
The caller must specify whether they want the host's view of
either the virtual or physical counter.
Signed-off-by: Jianyong Wu <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
Documentation/virt/kvm/api.rst | 10 +++++
Documentation/virt/kvm/arm/index.rst | 1 +
Documentation/virt/kvm/arm/ptp_kvm.rst | 25 ++++++++++++
arch/arm64/kvm/arm.c | 1 +
arch/arm64/kvm/hypercalls.c | 53 ++++++++++++++++++++++++++
include/linux/arm-smccc.h | 16 ++++++++
include/uapi/linux/kvm.h | 1 +
7 files changed, 107 insertions(+)
create mode 100644 Documentation/virt/kvm/arm/ptp_kvm.rst
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 38e327d4b479..987d99e39887 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6724,3 +6724,13 @@ vcpu_info is set.
The KVM_XEN_HVM_CONFIG_RUNSTATE flag indicates that the runstate-related
features KVM_XEN_VCPU_ATTR_TYPE_RUNSTATE_ADDR/_CURRENT/_DATA/_ADJUST are
supported by the KVM_XEN_VCPU_SET_ATTR/KVM_XEN_VCPU_GET_ATTR ioctls.
+
+8.31 KVM_CAP_PTP_KVM
+--------------------
+
+:Architectures: arm64
+
+This capability indicates that the KVM virtual PTP service is
+supported in the host. A VMM can check whether the service is
+available to the guest on migration.
+
diff --git a/Documentation/virt/kvm/arm/index.rst b/Documentation/virt/kvm/arm/index.rst
index 3e2b2aba90fc..78a9b670aafe 100644
--- a/Documentation/virt/kvm/arm/index.rst
+++ b/Documentation/virt/kvm/arm/index.rst
@@ -10,3 +10,4 @@ ARM
hyp-abi
psci
pvtime
+ ptp_kvm
diff --git a/Documentation/virt/kvm/arm/ptp_kvm.rst b/Documentation/virt/kvm/arm/ptp_kvm.rst
new file mode 100644
index 000000000000..68cffb50d8bf
--- /dev/null
+++ b/Documentation/virt/kvm/arm/ptp_kvm.rst
@@ -0,0 +1,25 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+PTP_KVM support for arm/arm64
+=============================
+
+PTP_KVM is used for high precision time sync between host and guests.
+It relies on transferring the wall clock and counter value from the
+host to the guest using a KVM-specific hypercall.
+
+* ARM_SMCCC_HYP_KVM_PTP_FUNC_ID: 0x86000001
+
+This hypercall uses the SMC32/HVC32 calling convention:
+
+ARM_SMCCC_HYP_KVM_PTP_FUNC_ID
+ ============= ========== ==========
+ Function ID: (uint32) 0x86000001
+ Arguments: (uint32) KVM_PTP_VIRT_COUNTER(0)
+ KVM_PTP_PHYS_COUNTER(1)
+ Return Values: (int32) NOT_SUPPORTED(-1) on error, or
+ (uint32) Upper 32 bits of wall clock time (r0)
+ (uint32) Lower 32 bits of wall clock time (r1)
+ (uint32) Upper 32 bits of counter (r2)
+ (uint32) Lower 32 bits of counter (r3)
+ Endianness: No Restrictions.
+ ============= ========== ==========
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 7f06ba76698d..46401798c644 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -206,6 +206,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_ARM_INJECT_EXT_DABT:
case KVM_CAP_SET_GUEST_DEBUG:
case KVM_CAP_VCPU_ATTRIBUTES:
+ case KVM_CAP_PTP_KVM:
r = 1;
break;
case KVM_CAP_ARM_SET_DEVICE_ADDR:
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 78d32c34d49c..30da78f72b3b 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -9,6 +9,55 @@
#include <kvm/arm_hypercalls.h>
#include <kvm/arm_psci.h>
+static void kvm_ptp_get_time(struct kvm_vcpu *vcpu, u64 *val)
+{
+ struct system_time_snapshot systime_snapshot;
+ u64 cycles = ~0UL;
+ u32 feature;
+
+ /*
+ * system time and counter value must captured at the same
+ * time to keep consistency and precision.
+ */
+ ktime_get_snapshot(&systime_snapshot);
+
+ /*
+ * This is only valid if the current clocksource is the
+ * architected counter, as this is the only one the guest
+ * can see.
+ */
+ if (systime_snapshot.cs_id != CSID_ARM_ARCH_COUNTER)
+ return;
+
+ /*
+ * The guest selects one of the two reference counters
+ * (virtual or physical) with the first argument of the SMCCC
+ * call. In case the identifier is not supported, error out.
+ */
+ feature = smccc_get_arg1(vcpu);
+ switch (feature) {
+ case KVM_PTP_VIRT_COUNTER:
+ cycles = systime_snapshot.cycles - vcpu_read_sys_reg(vcpu, CNTVOFF_EL2);
+ break;
+ case KVM_PTP_PHYS_COUNTER:
+ cycles = systime_snapshot.cycles;
+ break;
+ default:
+ return;
+ }
+
+ /*
+ * This relies on the top bit of val[0] never being set for
+ * valid values of system time, because that is *really* far
+ * in the future (about 292 years from 1970, and at that stage
+ * nobody will give a damn about it).
+ */
+ val[0] = upper_32_bits(systime_snapshot.real);
+ val[1] = lower_32_bits(systime_snapshot.real);
+ val[2] = upper_32_bits(cycles);
+ val[3] = lower_32_bits(cycles);
+}
+
int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
{
u32 func_id = smccc_get_function(vcpu);
@@ -79,6 +128,10 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
break;
case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
+ val[0] |= BIT(ARM_SMCCC_KVM_FUNC_PTP);
+ break;
+ case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
+ kvm_ptp_get_time(vcpu, val);
break;
case ARM_SMCCC_TRNG_VERSION:
case ARM_SMCCC_TRNG_FEATURES:
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 1a27bd9493fe..6861489a1890 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -103,6 +103,7 @@
/* KVM "vendor specific" services */
#define ARM_SMCCC_KVM_FUNC_FEATURES 0
+#define ARM_SMCCC_KVM_FUNC_PTP 1
#define ARM_SMCCC_KVM_FUNC_FEATURES_2 127
#define ARM_SMCCC_KVM_NUM_FUNCS 128
@@ -114,6 +115,21 @@
#define SMCCC_ARCH_WORKAROUND_RET_UNAFFECTED 1
+/*
+ * ptp_kvm is a feature used for time sync between vm and host.
+ * ptp_kvm module in guest kernel will get service from host using
+ * this hypercall ID.
+ */
+#define ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID \
+ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+ ARM_SMCCC_SMC_32, \
+ ARM_SMCCC_OWNER_VENDOR_HYP, \
+ ARM_SMCCC_KVM_FUNC_PTP)
+
+/* ptp_kvm counter type ID */
+#define KVM_PTP_VIRT_COUNTER 0
+#define KVM_PTP_PHYS_COUNTER 1
+
/* Paravirtualised time calls (defined by ARM DEN0057A) */
#define ARM_SMCCC_HV_PV_TIME_FEATURES \
ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index f6afee209620..0e0f70c0d0dc 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1078,6 +1078,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_DIRTY_LOG_RING 192
#define KVM_CAP_X86_BUS_LOCK_EXIT 193
#define KVM_CAP_PPC_DAWR1 194
+#define KVM_CAP_PTP_KVM 195
#ifdef KVM_CAP_IRQ_ROUTING
--
2.29.2
On Tue, 30 Mar 2021 15:54:26 +0100,
Marc Zyngier <[email protected]> wrote:
>
> From: Jianyong Wu <[email protected]>
>
> Currently, the ptp_kvm module contains a lot of x86-specific code.
> Let's move this code into a new arch-specific file in the same directory,
> and rename the arch-independent file to ptp_kvm_common.c.
>
> Reviewed-by: Andre Przywara <[email protected]>
> Signed-off-by: Jianyong Wu <[email protected]>
> Signed-off-by: Marc Zyngier <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
Richard, Paolo,
Can I get an Ack on this and patch #7? We're getting pretty close to
the next merge window, and this series has been going on for a couple
of years now...
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On Wed, Apr 07, 2021 at 10:28:44AM +0100, Marc Zyngier wrote:
> On Tue, 30 Mar 2021 15:54:26 +0100,
> Marc Zyngier <[email protected]> wrote:
> >
> > From: Jianyong Wu <[email protected]>
> >
> > Currently, the ptp_kvm module contains a lot of x86-specific code.
> > Let's move this code into a new arch-specific file in the same directory,
> > and rename the arch-independent file to ptp_kvm_common.c.
> >
> > Reviewed-by: Andre Przywara <[email protected]>
> > Signed-off-by: Jianyong Wu <[email protected]>
> > Signed-off-by: Marc Zyngier <[email protected]>
> > Link: https://lore.kernel.org/r/[email protected]
>
> Richard, Paolo,
>
> Can I get an Ack on this and patch #7? We're getting pretty close to
> the next merge window, and this series has been going on for a couple
> of years now...
For both patches:
Acked-by: Richard Cochran <[email protected]>
On Wed, 07 Apr 2021 16:13:34 +0100,
Richard Cochran <[email protected]> wrote:
>
> On Wed, Apr 07, 2021 at 10:28:44AM +0100, Marc Zyngier wrote:
> > On Tue, 30 Mar 2021 15:54:26 +0100,
> > Marc Zyngier <[email protected]> wrote:
> > >
> > > From: Jianyong Wu <[email protected]>
> > >
> > > Currently, the ptp_kvm module contains a lot of x86-specific code.
> > > Let's move this code into a new arch-specific file in the same directory,
> > > and rename the arch-independent file to ptp_kvm_common.c.
> > >
> > > Reviewed-by: Andre Przywara <[email protected]>
> > > Signed-off-by: Jianyong Wu <[email protected]>
> > > Signed-off-by: Marc Zyngier <[email protected]>
> > > Link: https://lore.kernel.org/r/[email protected]
> >
> > Richard, Paolo,
> >
> > Can I get an Ack on this and patch #7? We're getting pretty close to
> > the next merge window, and this series has been going on for a couple
> > of years now...
>
> For both patches:
>
> Acked-by: Richard Cochran <[email protected]>
Cheers Richard, much appreciated.
M.
--
Without deviation from the norm, progress is not possible.
On 2021/3/30 22:54, Marc Zyngier wrote:
> +int kvm_arch_ptp_init(void)
> +{
> + int ret;
> +
> + ret = kvm_arm_hyp_service_available(ARM_SMCCC_KVM_FUNC_PTP);
> + if (ret <= 0)
kvm_arm_hyp_service_available() returns boolean. Maybe write as ?
bool ret;
ret = kvm_arm_hyp_service_available();
if (!ret)
return -ENODEV;
> + return -EOPNOTSUPP;
> +
> + return 0;
> +}
On 2021/3/30 22:54, Marc Zyngier wrote:
> +PTP_KVM support for arm/arm64
> +=============================
> +
> +PTP_KVM is used for high precision time sync between host and guests.
> +It relies on transferring the wall clock and counter value from the
> +host to the guest using a KVM-specific hypercall.
> +
> +* ARM_SMCCC_HYP_KVM_PTP_FUNC_ID: 0x86000001
Per include/linux/arm-smccc.h, this should be
ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID.
> +
> +This hypercall uses the SMC32/HVC32 calling convention:
> +
> +ARM_SMCCC_HYP_KVM_PTP_FUNC_ID
ditto
> + ============= ========== ==========
> + Function ID: (uint32) 0x86000001
> + Arguments: (uint32) KVM_PTP_VIRT_COUNTER(0)
> + KVM_PTP_PHYS_COUNTER(1)
On Sat, 17 Apr 2021 09:42:37 +0100,
Zenghui Yu <[email protected]> wrote:
>
> On 2021/3/30 22:54, Marc Zyngier wrote:
> > +int kvm_arch_ptp_init(void)
> > +{
> > + int ret;
> > +
> > + ret = kvm_arm_hyp_service_available(ARM_SMCCC_KVM_FUNC_PTP);
> > + if (ret <= 0)
>
> kvm_arm_hyp_service_available() returns boolean. Maybe write as ?
>
> bool ret;
>
> ret = kvm_arm_hyp_service_available();
> if (!ret)
> return -ENODEV;
Fixed in 300bb1fe7671, as previously reported by Dan Carpenter in [1].
Thanks,
M.
https://lore.kernel.org/r/20210331043704.GG2065@kadam
--
Without deviation from the norm, progress is not possible.
On Sat, 17 Apr 2021 09:59:39 +0100,
Zenghui Yu <[email protected]> wrote:
>
> On 2021/3/30 22:54, Marc Zyngier wrote:
> > +PTP_KVM support for arm/arm64
> > +=============================
> > +
> > +PTP_KVM is used for high precision time sync between host and guests.
> > +It relies on transferring the wall clock and counter value from the
> > +host to the guest using a KVM-specific hypercall.
> > +
> > +* ARM_SMCCC_HYP_KVM_PTP_FUNC_ID: 0x86000001
>
> Per include/linux/arm-smccc.h, this should be
> ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID.
Well spotted. Care to send a patch on top of my kvm-arm64/ptp branch?
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On 2021/3/30 22:54, Marc Zyngier wrote:
> - u64 cycles;
> - ktime_t real;
> - ktime_t raw;
> - unsigned int clock_was_set_seq;
> - u8 cs_was_changed_seq;
> + u64 cycles;
> + ktime_t real;
> + ktime_t raw;
> + enum clocksource_ids cs_id;
nit: worth adding a description for it (on top of the structure)?
> + unsigned int clock_was_set_seq;
> + u8 cs_was_changed_seq;
On 2021/4/17 17:10, Marc Zyngier wrote:
> On Sat, 17 Apr 2021 09:59:39 +0100,
> Zenghui Yu <[email protected]> wrote:
>>
>> On 2021/3/30 22:54, Marc Zyngier wrote:
>>> +PTP_KVM support for arm/arm64
>>> +=============================
>>> +
>>> +PTP_KVM is used for high precision time sync between host and guests.
>>> +It relies on transferring the wall clock and counter value from the
>>> +host to the guest using a KVM-specific hypercall.
>>> +
>>> +* ARM_SMCCC_HYP_KVM_PTP_FUNC_ID: 0x86000001
>>
>> Per include/linux/arm-smccc.h, this should be
>> ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID.
>
> Well spotted. Care to send a patch on top of my kvm-arm64/ptp branch?
Okay. I'll have a patch for that.
Zenghui
Hi Marc, Jianyong,
On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <[email protected]> wrote:
> From: Jianyong Wu <[email protected]>
>
> Currently, there is no mechanism to keep time sync between guest and host
> in arm/arm64 virtualization environment. Time in guest will drift compared
> with host after boot up as they may both use third party time sources
> to correct their time respectively. The time deviation will be in order
> of milliseconds. But in some scenarios,like in cloud environment, we ask
> for higher time precision.
>
> kvm ptp clock, which chooses the host clock source as a reference
> clock to sync time between guest and host, has been adopted by x86
> which takes the time sync order from milliseconds to nanoseconds.
>
> This patch enables kvm ptp clock for arm/arm64 and improves clock sync precision
> significantly.
> --- a/drivers/ptp/Kconfig
> +++ b/drivers/ptp/Kconfig
> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
> config PTP_1588_CLOCK_KVM
> tristate "KVM virtual PTP clock"
> depends on PTP_1588_CLOCK
> - depends on KVM_GUEST && X86
> + depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER)
Why does this not depend on KVM_GUEST on ARM?
I.e. shouldn't the dependency be:
KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))
?
> default y
> help
> This driver adds support for using kvm infrastructure as a PTP
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
On 2021-05-11 10:07, Geert Uytterhoeven wrote:
> Hi Marc, Jianyong,
>
> On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <[email protected]> wrote:
>> From: Jianyong Wu <[email protected]>
>>
>> Currently, there is no mechanism to keep time sync between guest and
>> host
>> in arm/arm64 virtualization environment. Time in guest will drift
>> compared
>> with host after boot up as they may both use third party time sources
>> to correct their time respectively. The time deviation will be in
>> order
>> of milliseconds. But in some scenarios,like in cloud environment, we
>> ask
>> for higher time precision.
>>
>> kvm ptp clock, which chooses the host clock source as a reference
>> clock to sync time between guest and host, has been adopted by x86
>> which takes the time sync order from milliseconds to nanoseconds.
>>
>> This patch enables kvm ptp clock for arm/arm64 and improves clock sync
>> precision
>> significantly.
>
>> --- a/drivers/ptp/Kconfig
>> +++ b/drivers/ptp/Kconfig
>> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
>> config PTP_1588_CLOCK_KVM
>> tristate "KVM virtual PTP clock"
>> depends on PTP_1588_CLOCK
>> - depends on KVM_GUEST && X86
>> + depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY &&
>> ARM_ARCH_TIMER)
>
> Why does this not depend on KVM_GUEST on ARM?
> I.e. shouldn't the dependency be:
>
> KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))
>
> ?
arm/arm64 do not select KVM_GUEST. Any kernel can be used for a guest,
and KVM/arm64 doesn't know about this configuration symbol.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
Hi Geert,
On Wed, 26 May 2021 08:52:42 +0100,
Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Marc,
>
> On Tue, May 11, 2021 at 11:13 AM Marc Zyngier <[email protected]> wrote:
> > On 2021-05-11 10:07, Geert Uytterhoeven wrote:
> > > On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <[email protected]> wrote:
> > >> From: Jianyong Wu <[email protected]>
> > >
> > >> --- a/drivers/ptp/Kconfig
> > >> +++ b/drivers/ptp/Kconfig
> > >> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
> > >> config PTP_1588_CLOCK_KVM
> > >> tristate "KVM virtual PTP clock"
> > >> depends on PTP_1588_CLOCK
> > >> - depends on KVM_GUEST && X86
> > >> + depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY &&
> > >> ARM_ARCH_TIMER)
> > >
> > > Why does this not depend on KVM_GUEST on ARM?
> > > I.e. shouldn't the dependency be:
> > >
> > > KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))
> > >
> > > ?
> >
> > arm/arm64 do not select KVM_GUEST. Any kernel can be used for a guest,
> > and KVM/arm64 doesn't know about this configuration symbol.
>
> OK.
>
> Does PTP_1588_CLOCK_KVM need to default to yes?
> Perhaps only on X86, to maintain the status quo?
I think I don't really understand the problem you are trying to
solve. Is it that 'make oldconfig' now asks you about this new driver?
Why is that an issue?
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Hi Marc,
On Wed, May 26, 2021 at 10:01 AM Marc Zyngier <[email protected]> wrote:
> On Wed, 26 May 2021 08:52:42 +0100,
> Geert Uytterhoeven <[email protected]> wrote:
> > On Tue, May 11, 2021 at 11:13 AM Marc Zyngier <[email protected]> wrote:
> > > On 2021-05-11 10:07, Geert Uytterhoeven wrote:
> > > > On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <[email protected]> wrote:
> > > >> From: Jianyong Wu <[email protected]>
> > > >
> > > >> --- a/drivers/ptp/Kconfig
> > > >> +++ b/drivers/ptp/Kconfig
> > > >> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
> > > >> config PTP_1588_CLOCK_KVM
> > > >> tristate "KVM virtual PTP clock"
> > > >> depends on PTP_1588_CLOCK
> > > >> - depends on KVM_GUEST && X86
> > > >> + depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY &&
> > > >> ARM_ARCH_TIMER)
> > > >
> > > > Why does this not depend on KVM_GUEST on ARM?
> > > > I.e. shouldn't the dependency be:
> > > >
> > > > KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))
> > > >
> > > > ?
> > >
> > > arm/arm64 do not select KVM_GUEST. Any kernel can be used for a guest,
> > > and KVM/arm64 doesn't know about this configuration symbol.
> >
> > OK.
> >
> > Does PTP_1588_CLOCK_KVM need to default to yes?
> > Perhaps only on X86, to maintain the status quo?
>
> I think I don't really understand the problem you are trying to
> solve. Is it that 'make oldconfig' now asks you about this new driver?
> Why is that an issue?
My first "problem" was that it asked about this new driver on
arm/arm64, while I assumed there were some missing dependencies
(configuring a kernel should not ask useless questions). That turned
out to be a wrong assumption, so there is no such problem here.
The second problem is "default y": code that is not critical should
not be enabled by default. Hence my last question.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Marc,
On Wed, May 26, 2021 at 10:32 AM Marc Zyngier <[email protected]> wrote:
> On Wed, 26 May 2021 09:18:27 +0100,
> Geert Uytterhoeven <[email protected]> wrote:
> > On Wed, May 26, 2021 at 10:01 AM Marc Zyngier <[email protected]> wrote:
> > > On Wed, 26 May 2021 08:52:42 +0100,
> > > Geert Uytterhoeven <[email protected]> wrote:
> > > > On Tue, May 11, 2021 at 11:13 AM Marc Zyngier <[email protected]> wrote:
> > > > > On 2021-05-11 10:07, Geert Uytterhoeven wrote:
> > > > > > On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <[email protected]> wrote:
> > > > > >> From: Jianyong Wu <[email protected]>
> > > > > >
> > > > > >> --- a/drivers/ptp/Kconfig
> > > > > >> +++ b/drivers/ptp/Kconfig
> > > > > >> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
> > > > > >> config PTP_1588_CLOCK_KVM
> > > > > >> tristate "KVM virtual PTP clock"
> > > > > >> depends on PTP_1588_CLOCK
> > > > > >> - depends on KVM_GUEST && X86
> > > > > >> + depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY &&
> > > > > >> ARM_ARCH_TIMER)
> > > > > >
> > > > > > Why does this not depend on KVM_GUEST on ARM?
> > > > > > I.e. shouldn't the dependency be:
> > > > > >
> > > > > > KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))
> > > > > >
> > > > > > ?
> > > > >
> > > > > arm/arm64 do not select KVM_GUEST. Any kernel can be used for a guest,
> > > > > and KVM/arm64 doesn't know about this configuration symbol.
> > > >
> > > > OK.
> > > >
> > > > Does PTP_1588_CLOCK_KVM need to default to yes?
> > > > Perhaps only on X86, to maintain the status quo?
> > >
> > > I think I don't really understand the problem you are trying to
> > > solve. Is it that 'make oldconfig' now asks you about this new driver?
> > > Why is that an issue?
> >
> > My first "problem" was that it asked about this new driver on
> > arm/arm64, while I assumed there were some missing dependencies
> > (configuring a kernel should not ask useless questions). That turned
> > out to be a wrong assumption, so there is no such problem here.
> >
> > The second problem is "default y": code that is not critical should
> > not be enabled by default. Hence my last question.
>
> I think consistency between architectures is important. Certainly,
> distributions depend on that, and we otherwise end-up with distro
> kernels missing functionalities.
>
> The notion of "critical" is also pretty relative. defconfig contains a
I'm not talking about defconfig, but about "default y" in defconfig.
> gazillion of things that are not critical to most people, for example,
> and yet misses a bunch of things that are needed to boot on some of my
> systems.
Perhaps those should be added, so those systems can be tested using
defconfig? At least for arm64, I think that's aligned with the
arm64 defconfig policy.
> That's just to say that I find it difficult to make that choice from
> the PoV of a kernel hacker. I'm personally more inclined to leave
> things enabled and let people *disable* things if they want to reduce
> the footprint of their kernel.
The standard question to respond to w.r.t. "default y" is: "Why is
your feature so special that it needs to be enabled by default?",
which implies "default y" is the exception, not the rule.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Wed, 26 May 2021 09:18:27 +0100,
Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Marc,
>
> On Wed, May 26, 2021 at 10:01 AM Marc Zyngier <[email protected]> wrote:
> > On Wed, 26 May 2021 08:52:42 +0100,
> > Geert Uytterhoeven <[email protected]> wrote:
> > > On Tue, May 11, 2021 at 11:13 AM Marc Zyngier <[email protected]> wrote:
> > > > On 2021-05-11 10:07, Geert Uytterhoeven wrote:
> > > > > On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <[email protected]> wrote:
> > > > >> From: Jianyong Wu <[email protected]>
> > > > >
> > > > >> --- a/drivers/ptp/Kconfig
> > > > >> +++ b/drivers/ptp/Kconfig
> > > > >> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
> > > > >> config PTP_1588_CLOCK_KVM
> > > > >> tristate "KVM virtual PTP clock"
> > > > >> depends on PTP_1588_CLOCK
> > > > >> - depends on KVM_GUEST && X86
> > > > >> + depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY &&
> > > > >> ARM_ARCH_TIMER)
> > > > >
> > > > > Why does this not depend on KVM_GUEST on ARM?
> > > > > I.e. shouldn't the dependency be:
> > > > >
> > > > > KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))
> > > > >
> > > > > ?
> > > >
> > > > arm/arm64 do not select KVM_GUEST. Any kernel can be used for a guest,
> > > > and KVM/arm64 doesn't know about this configuration symbol.
> > >
> > > OK.
> > >
> > > Does PTP_1588_CLOCK_KVM need to default to yes?
> > > Perhaps only on X86, to maintain the status quo?
> >
> > I think I don't really understand the problem you are trying to
> > solve. Is it that 'make oldconfig' now asks you about this new driver?
> > Why is that an issue?
>
> My first "problem" was that it asked about this new driver on
> arm/arm64, while I assumed there were some missing dependencies
> (configuring a kernel should not ask useless questions). That turned
> out to be a wrong assumption, so there is no such problem here.
>
> The second problem is "default y": code that is not critical should
> not be enabled by default. Hence my last question.
I think consistency between architectures is important. Certainly,
distributions depend on that, and we otherwise end-up with distro
kernels missing functionalities.
The notion of "critical" is also pretty relative. defconfig contains a
gazillion of things that are not critical to most people, for example,
and yet misses a bunch of things that are needed to boot on some of my
systems.
That's just to say that I find it difficult to make that choice from
the PoV of a kernel hacker. I'm personally more inclined to leave
things enabled and let people *disable* things if they want to reduce
the footprint of their kernel.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Hi Marc,
On Tue, May 11, 2021 at 11:13 AM Marc Zyngier <[email protected]> wrote:
> On 2021-05-11 10:07, Geert Uytterhoeven wrote:
> > On Tue, Mar 30, 2021 at 4:56 PM Marc Zyngier <[email protected]> wrote:
> >> From: Jianyong Wu <[email protected]>
> >>
> >> Currently, there is no mechanism to keep time sync between guest and
> >> host
> >> in arm/arm64 virtualization environment. Time in guest will drift
> >> compared
> >> with host after boot up as they may both use third party time sources
> >> to correct their time respectively. The time deviation will be in
> >> order
> >> of milliseconds. But in some scenarios,like in cloud environment, we
> >> ask
> >> for higher time precision.
> >>
> >> kvm ptp clock, which chooses the host clock source as a reference
> >> clock to sync time between guest and host, has been adopted by x86
> >> which takes the time sync order from milliseconds to nanoseconds.
> >>
> >> This patch enables kvm ptp clock for arm/arm64 and improves clock sync
> >> precision
> >> significantly.
> >
> >> --- a/drivers/ptp/Kconfig
> >> +++ b/drivers/ptp/Kconfig
> >> @@ -108,7 +108,7 @@ config PTP_1588_CLOCK_PCH
> >> config PTP_1588_CLOCK_KVM
> >> tristate "KVM virtual PTP clock"
> >> depends on PTP_1588_CLOCK
> >> - depends on KVM_GUEST && X86
> >> + depends on (KVM_GUEST && X86) || (HAVE_ARM_SMCCC_DISCOVERY &&
> >> ARM_ARCH_TIMER)
> >
> > Why does this not depend on KVM_GUEST on ARM?
> > I.e. shouldn't the dependency be:
> >
> > KVM_GUEST && (X86 || (HAVE_ARM_SMCCC_DISCOVERY && ARM_ARCH_TIMER))
> >
> > ?
>
> arm/arm64 do not select KVM_GUEST. Any kernel can be used for a guest,
> and KVM/arm64 doesn't know about this configuration symbol.
OK.
Does PTP_1588_CLOCK_KVM need to default to yes?
Perhaps only on X86, to maintain the status quo?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds