| Message ID | 20191015104822.13890-6-jianyong.wu@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable ptp_kvm for arm64 | expand |
On 15/10/19 12:48, Jianyong Wu wrote: > +int kvm_arch_ptp_get_clock_generic(struct timespec64 *ts, > + struct arm_smccc_res *hvc_res) > +{ > + u64 ns; > + ktime_t ktime_overall; > + > + arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID, > + hvc_res); > + if ((long)(hvc_res->a0) < 0) > + return -EOPNOTSUPP; > + > + ktime_overall = hvc_res->a0 << 32 | hvc_res->a1; > + *ts = ktime_to_timespec64(ktime_overall); > + > + return 0; > +} > + This seems wrong, who uses kvm_arch_ptp_get_clock_fn? Paolo
Hi Paolo, > -----Original Message----- > From: Paolo Bonzini <pbonzini@redhat.com> > Sent: Wednesday, October 16, 2019 12:39 AM > To: Jianyong Wu (Arm Technology China) <Jianyong.Wu@arm.com>; > netdev@vger.kernel.org; yangbo.lu@nxp.com; john.stultz@linaro.org; > tglx@linutronix.de; sean.j.christopherson@intel.com; maz@kernel.org; > richardcochran@gmail.com; Mark Rutland <Mark.Rutland@arm.com>; > will@kernel.org; Suzuki Poulose <Suzuki.Poulose@arm.com> > Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org; Steve Capper > <Steve.Capper@arm.com>; Kaly Xin (Arm Technology China) > <Kaly.Xin@arm.com>; Justin He (Arm Technology China) > <Justin.He@arm.com>; nd <nd@arm.com> > Subject: Re: [PATCH v5 5/6] ptp: arm64: Enable ptp_kvm for arm64 > > On 15/10/19 12:48, Jianyong Wu wrote: > > +int kvm_arch_ptp_get_clock_generic(struct timespec64 *ts, > > + struct arm_smccc_res *hvc_res) { > > + u64 ns; > > + ktime_t ktime_overall; > > + > > + > arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_PTP_FU > NC_ID, > > + hvc_res); > > + if ((long)(hvc_res->a0) < 0) > > + return -EOPNOTSUPP; > > + > > + ktime_overall = hvc_res->a0 << 32 | hvc_res->a1; > > + *ts = ktime_to_timespec64(ktime_overall); > > + > > + return 0; > > +} > > + > > This seems wrong, who uses kvm_arch_ptp_get_clock_fn? > This func used only by kvm_arch_ptp_get_clock and nothing to do with kvm_arch_ptp_get_clock_fn. Also it can be merged into kvm_arch_ptp_get_clock. > Paolo
On 16/10/19 05:52, Jianyong Wu (Arm Technology China) wrote: > This func used only by kvm_arch_ptp_get_clock and nothing to do with > kvm_arch_ptp_get_clock_fn. Also it can be merged into > kvm_arch_ptp_get_clock. > Your patches also have no user for kvm_arch_ptp_get_clock, so you can remove it. Paolo
On 16/10/19 09:10, Paolo Bonzini wrote: > On 16/10/19 05:52, Jianyong Wu (Arm Technology China) wrote: >> This func used only by kvm_arch_ptp_get_clock and nothing to do with >> kvm_arch_ptp_get_clock_fn. Also it can be merged into >> kvm_arch_ptp_get_clock. >> > > Your patches also have no user for kvm_arch_ptp_get_clock, so you can > remove it. Nevermind. I misread patch 2. However, to remove the confusion, can you rename kvm_arch_ptp_get_clock_fn to kvm_arch_ptp_get_crosststamp? Paolo
Hi Paolo, > -----Original Message----- > From: Paolo Bonzini <pbonzini@redhat.com> > Sent: Wednesday, October 16, 2019 3:14 PM > To: Jianyong Wu (Arm Technology China) <Jianyong.Wu@arm.com>; > netdev@vger.kernel.org; yangbo.lu@nxp.com; john.stultz@linaro.org; > tglx@linutronix.de; sean.j.christopherson@intel.com; maz@kernel.org; > richardcochran@gmail.com; Mark Rutland <Mark.Rutland@arm.com>; > will@kernel.org; Suzuki Poulose <Suzuki.Poulose@arm.com> > Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org; Steve Capper > <Steve.Capper@arm.com>; Kaly Xin (Arm Technology China) > <Kaly.Xin@arm.com>; Justin He (Arm Technology China) > <Justin.He@arm.com>; nd <nd@arm.com> > Subject: Re: [PATCH v5 5/6] ptp: arm64: Enable ptp_kvm for arm64 > > On 16/10/19 09:10, Paolo Bonzini wrote: > > On 16/10/19 05:52, Jianyong Wu (Arm Technology China) wrote: > >> This func used only by kvm_arch_ptp_get_clock and nothing to do with > >> kvm_arch_ptp_get_clock_fn. Also it can be merged into > >> kvm_arch_ptp_get_clock. > >> > > > > Your patches also have no user for kvm_arch_ptp_get_clock, so you can > > remove it. > > Nevermind. I misread patch 2. However, to remove the confusion, can you > rename kvm_arch_ptp_get_clock_fn to kvm_arch_ptp_get_crosststamp? > the suffix of this function name is reserved from its arch-independent caller ptp_kvm_get_time_fn, so if I change this function name It will be better change them whole. I think "ptp_get_crosststamp" is a better suffix. Thanks Jianyong > Paolo
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 3597f1f27b10..3e1fd794451f 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -1639,3 +1639,25 @@ bool is_arm_arch_counter(void *cs) { return (struct clocksource *)cs == &clocksource_counter; } + +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK_KVM) +#include <linux/arm-smccc.h> +int kvm_arch_ptp_get_clock_fn(unsigned long *cycle, struct timespec64 *ts, + struct clocksource **cs) +{ + struct arm_smccc_res hvc_res; + ktime_t ktime_overall; + + arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID, &hvc_res); + if ((long)(hvc_res.a0) < 0) + return -EOPNOTSUPP; + + ktime_overall = hvc_res.a0 << 32 | hvc_res.a1; + *ts = ktime_to_timespec64(ktime_overall); + *cycle = hvc_res.a2 << 32 | hvc_res.a3; + *cs = &clocksource_counter; + + return 0; +} +EXPORT_SYMBOL_GPL(kvm_arch_ptp_get_clock_fn); +#endif diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig index 9b8fee5178e8..3c31ff8eb05f 100644 --- a/drivers/ptp/Kconfig +++ b/drivers/ptp/Kconfig @@ -110,7 +110,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 || ARM64 && ARM_ARCH_TIMER default y help This driver adds support for using kvm infrastructure as a PTP diff --git a/drivers/ptp/ptp_kvm_arm64.c b/drivers/ptp/ptp_kvm_arm64.c new file mode 100644 index 000000000000..7c697f20904f --- /dev/null +++ b/drivers/ptp/ptp_kvm_arm64.c @@ -0,0 +1,54 @@ +// 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/kernel.h> +#include <linux/err.h> +#include <asm/hypervisor.h> +#include <linux/module.h> +#include <linux/psci.h> +#include <linux/arm-smccc.h> +#include <linux/timecounter.h> +#include <linux/sched/clock.h> +#include <asm/arch_timer.h> + +int kvm_arch_ptp_init(void) +{ + struct arm_smccc_res hvc_res; + + arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID, + &hvc_res); + if ((long)(hvc_res.a0) < 0) + return -EOPNOTSUPP; + + return 0; +} + +int kvm_arch_ptp_get_clock_generic(struct timespec64 *ts, + struct arm_smccc_res *hvc_res) +{ + u64 ns; + ktime_t ktime_overall; + + arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID, + hvc_res); + if ((long)(hvc_res->a0) < 0) + return -EOPNOTSUPP; + + ktime_overall = hvc_res->a0 << 32 | hvc_res->a1; + *ts = ktime_to_timespec64(ktime_overall); + + return 0; +} + +int kvm_arch_ptp_get_clock(struct timespec64 *ts) +{ + struct arm_smccc_res hvc_res; + + kvm_arch_ptp_get_clock_generic(ts, &hvc_res); + + return 0; +} diff --git a/drivers/ptp/ptp_kvm_common.c b/drivers/ptp/ptp_kvm_common.c index d8f215186904..c0b445fa6144 100644 --- a/drivers/ptp/ptp_kvm_common.c +++ b/drivers/ptp/ptp_kvm_common.c @@ -138,7 +138,7 @@ static int __init ptp_kvm_init(void) int ret; ret = kvm_arch_ptp_init(); - if (!ret) + if (ret) return -EOPNOTSUPP; kvm_ptp_clock.caps = ptp_kvm_caps;
Currently in arm64 virtualization environment, there is no mechanism to keep time sync between guest and host. 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 some scenarios ask for higher time precision, like in cloud envirenment, we want all the VMs running in the host aquire the same level accuracy from host clock. Use of kvm ptp clock, which choose the host clock source clock as a reference clock to sync time clock between guest and host has been adopted by x86 which makes the time sync order from milliseconds to nanoseconds. This patch enable kvm ptp on arm64 and we get the similar clock drift as found with x86 with kvm ptp. Test result comparison between with kvm ptp and without it in arm64 are as follows. This test derived from the result of command 'chronyc sources'. we should take more cure 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 be sure to say that the clock error between host and guest is in order of ns. Signed-off-by: Jianyong Wu <jianyong.wu@arm.com> --- drivers/clocksource/arm_arch_timer.c | 22 ++++++++++++ drivers/ptp/Kconfig | 2 +- drivers/ptp/ptp_kvm_arm64.c | 54 ++++++++++++++++++++++++++++ drivers/ptp/ptp_kvm_common.c | 2 +- 4 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 drivers/ptp/ptp_kvm_arm64.c