[RFC,v9,7/8] ptp: arm64: Enable ptp_kvm for arm64

Commit Message

Jianyong Wu Dec. 10, 2019, 3:40 a.m. UTC

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 <[e-mail]>
---
 drivers/clocksource/arm_arch_timer.c | 22 ++++++++++++
 drivers/ptp/Kconfig                  |  2 +-
 drivers/ptp/ptp_kvm_arm64.c          | 53 ++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 drivers/ptp/ptp_kvm_arm64.c

Comments

Marc Zyngier Jan. 7, 2020, 9:29 a.m. UTC | #1

On 2019-12-10 03:40, Jianyong Wu wrote:
> 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 <[e-mail]>
> ---
>  drivers/clocksource/arm_arch_timer.c | 22 ++++++++++++
>  drivers/ptp/Kconfig                  |  2 +-
>  drivers/ptp/ptp_kvm_arm64.c          | 53 ++++++++++++++++++++++++++++
>  3 files changed, 76 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/ptp/ptp_kvm_arm64.c
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c
> b/drivers/clocksource/arm_arch_timer.c
> index 277846decd33..72260b66f02e 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -1636,3 +1636,25 @@ 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
> +
> +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK_KVM)
> +#include <linux/arm-smccc.h>
> +int kvm_arch_ptp_get_crosststamp(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;

So why isn't that just a read of the virtual counter, given that what 
you do
in the hypervisor seems to be "cntpct - cntvoff"?

What am I missing here?

> +	*cs = &clocksource_counter;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(kvm_arch_ptp_get_crosststamp);
> +#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..f3f957117865
> --- /dev/null
> +++ b/drivers/ptp/ptp_kvm_arm64.c
> @@ -0,0 +1,53 @@
> +// 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)
> +{
> +	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;
> +}

I also wonder why this is all arm64 specific, while everything
should also work just fine on 32bit.

         M.

Jianyong Wu Jan. 9, 2020, 5:59 a.m. UTC | #2

Hi Marc,

> -----Original Message-----
> From: Marc Zyngier <[e-mail]>
> Sent: Tuesday, January 7, 2020 5:29 PM
> To: Jianyong Wu <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; Mark Rutland <[e-mail]>;
> [e-mail]; Suzuki Poulose <[e-mail]>; Steven Price
> <[e-mail]>; [e-mail]; linux-arm-
> [e-mail]; [e-mail];
> [e-mail]; Steve Capper <[e-mail]>; Kaly Xin
> <[e-mail]>; Justin He <[e-mail]>; nd <[e-mail]>
> Subject: Re: [RFC PATCH v9 7/8] ptp: arm64: Enable ptp_kvm for arm64
> 
> On 2019-12-10 03:40, Jianyong Wu wrote:
> > 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 <[e-mail]>
> > ---
> >  drivers/clocksource/arm_arch_timer.c | 22 ++++++++++++
> >  drivers/ptp/Kconfig                  |  2 +-
> >  drivers/ptp/ptp_kvm_arm64.c          | 53
> ++++++++++++++++++++++++++++
> >  3 files changed, 76 insertions(+), 1 deletion(-)  create mode 100644
> > drivers/ptp/ptp_kvm_arm64.c
> >
> > diff --git a/drivers/clocksource/arm_arch_timer.c
> > b/drivers/clocksource/arm_arch_timer.c
> > index 277846decd33..72260b66f02e 100644
> > --- a/drivers/clocksource/arm_arch_timer.c
> > +++ b/drivers/clocksource/arm_arch_timer.c
> > @@ -1636,3 +1636,25 @@ 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
> > +
> > +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK_KVM)
> > +#include <linux/arm-smccc.h>
> > +int kvm_arch_ptp_get_crosststamp(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_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);
> > +	*cycle = hvc_res.a2 << 32 | hvc_res.a3;
> 
> So why isn't that just a read of the virtual counter, given that what you do in
> the hypervisor seems to be "cntpct - cntvoff"?
> 
> What am I missing here?
> 
We need get clock time and counter cycle at the same time, so we can't just read virtual counter
at guest and must get it from host.

> > +	*cs = &clocksource_counter;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(kvm_arch_ptp_get_crosststamp);
> > +#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..f3f957117865
> > --- /dev/null
> > +++ b/drivers/ptp/ptp_kvm_arm64.c
> > @@ -0,0 +1,53 @@
> > +// 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_FU
> NC_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) {
> > +	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;
> > +}
> > +
> > +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;
> > +}
> 
> I also wonder why this is all arm64 specific, while everything should also work
> just fine on 32bit.
>
ptp_kvm is a feature for cloud computing to keep time consistency from container to container and to host on server,
So we focus it on arm64. Also I have never tested it on arm32 machine ( we lack of arm32 machine)
Do you think it's necessary to enable ptp_kvm on arm32? If so, I can do that.
 
Thanks
Jianyong

>          M.
> --
> Jazz is not dead. It just smells funny...

Marc Zyngier Jan. 9, 2020, 9:24 a.m. UTC | #3

On 2020-01-09 05:59, Jianyong Wu wrote:
> Hi Marc,
> 
>> -----Original Message-----
>> From: Marc Zyngier <[e-mail]>
>> Sent: Tuesday, January 7, 2020 5:29 PM
>> To: Jianyong Wu <[e-mail]>
>> Cc: [e-mail]; [e-mail]; [e-mail];
>> [e-mail]; [e-mail]; 
>> [e-mail];
>> [e-mail]; Mark Rutland <[e-mail]>;
>> [e-mail]; Suzuki Poulose <[e-mail]>; Steven Price
>> <[e-mail]>; [e-mail]; linux-arm-
>> [e-mail]; [e-mail];
>> [e-mail]; Steve Capper <[e-mail]>; Kaly Xin
>> <[e-mail]>; Justin He <[e-mail]>; nd <[e-mail]>
>> Subject: Re: [RFC PATCH v9 7/8] ptp: arm64: Enable ptp_kvm for arm64
>> 
>> On 2019-12-10 03:40, Jianyong Wu wrote:
>> > 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 <[e-mail]>
>> > ---
>> >  drivers/clocksource/arm_arch_timer.c | 22 ++++++++++++
>> >  drivers/ptp/Kconfig                  |  2 +-
>> >  drivers/ptp/ptp_kvm_arm64.c          | 53
>> ++++++++++++++++++++++++++++
>> >  3 files changed, 76 insertions(+), 1 deletion(-)  create mode 100644
>> > drivers/ptp/ptp_kvm_arm64.c
>> >
>> > diff --git a/drivers/clocksource/arm_arch_timer.c
>> > b/drivers/clocksource/arm_arch_timer.c
>> > index 277846decd33..72260b66f02e 100644
>> > --- a/drivers/clocksource/arm_arch_timer.c
>> > +++ b/drivers/clocksource/arm_arch_timer.c
>> > @@ -1636,3 +1636,25 @@ 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
>> > +
>> > +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK_KVM)
>> > +#include <linux/arm-smccc.h>
>> > +int kvm_arch_ptp_get_crosststamp(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_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);
>> > +	*cycle = hvc_res.a2 << 32 | hvc_res.a3;
>> 
>> So why isn't that just a read of the virtual counter, given that what 
>> you do in
>> the hypervisor seems to be "cntpct - cntvoff"?
>> 
>> What am I missing here?
>> 
> We need get clock time and counter cycle at the same time, so we can't
> just read virtual counter
> at guest and must get it from host.

See my comment in my reply to patch #6: *Must* seems like a very strong 
word,
and you don't explain *why* that's better than just computing the total 
hypercall
cost. Hint: given the frequency of the counter (in the few MHz range) vs 
the
frequency of a CPU (in the multiple GHz range, and with an IPC close 
enough to 1),
I doubt that you'll see the counter making much progress across a 
hypercall.

> 
>> > +	*cs = &clocksource_counter;
>> > +
>> > +	return 0;
>> > +}
>> > +EXPORT_SYMBOL_GPL(kvm_arch_ptp_get_crosststamp);
>> > +#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..f3f957117865
>> > --- /dev/null
>> > +++ b/drivers/ptp/ptp_kvm_arm64.c
>> > @@ -0,0 +1,53 @@
>> > +// 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_FU
>> NC_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) {
>> > +	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;
>> > +}
>> > +
>> > +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;
>> > +}
>> 
>> I also wonder why this is all arm64 specific, while everything should 
>> also work
>> just fine on 32bit.
>> 
> ptp_kvm is a feature for cloud computing to keep time consistency from
> container to container and to host on server,

Cloud computing? Never heard of that. Will probably never catch on.

> So we focus it on arm64. Also I have never tested it on arm32 machine
> ( we lack of arm32 machine)

I'm sure your employer can provide you with such a box. I can probably
even tell you which cupboard they are stored in... ;-)

> Do you think it's necessary to enable ptp_kvm on arm32? If so, I can do 
> that.

I can't see why we wouldn't, given that it should be a zero effort task
(none of the code here is arch specific).

Thanks,

         M.

Marc Zyngier Jan. 9, 2020, 9:46 a.m. UTC | #4

On 2020-01-09 09:24, Marc Zyngier wrote:
> On 2020-01-09 05:59, Jianyong Wu wrote:

[...]

>> So we focus it on arm64. Also I have never tested it on arm32 machine
>> ( we lack of arm32 machine)
> 
> I'm sure your employer can provide you with such a box. I can probably
> even tell you which cupboard they are stored in... ;-)
> 
>> Do you think it's necessary to enable ptp_kvm on arm32? If so, I can 
>> do that.
> 
> I can't see why we wouldn't, given that it should be a zero effort task
> (none of the code here is arch specific).

To be clear, what I'm after is support for 32bit *guests*. I don't 
expect any
issue with a 32bit host (it's all common code), but you should be able 
to test
32bit guests pretty easily (most ARMv8.0 CPUs support 32bit EL1).

         M.

Jianyong Wu Jan. 10, 2020, 10:15 a.m. UTC | #5

Hi Marc,

> -----Original Message-----
> From: Marc Zyngier <[e-mail]>
> Sent: Thursday, January 9, 2020 5:24 PM
> To: Jianyong Wu <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; Mark Rutland <[e-mail]>;
> [e-mail]; Suzuki Poulose <[e-mail]>; Steven Price
> <[e-mail]>; [e-mail]; linux-arm-
> [e-mail]; [e-mail];
> [e-mail]; Steve Capper <[e-mail]>; Kaly Xin
> <[e-mail]>; Justin He <[e-mail]>; nd <[e-mail]>
> Subject: Re: [RFC PATCH v9 7/8] ptp: arm64: Enable ptp_kvm for arm64
> 
> On 2020-01-09 05:59, Jianyong Wu wrote:
> > Hi Marc,
> >
> >> -----Original Message-----
> >> From: Marc Zyngier <[e-mail]>
> >> Sent: Tuesday, January 7, 2020 5:29 PM
> >> To: Jianyong Wu <[e-mail]>
> >> Cc: [e-mail]; [e-mail];
> >> [e-mail]; [e-mail]; [e-mail];
> >> [e-mail]; [e-mail]; Mark
> >> Rutland <[e-mail]>; [e-mail]; Suzuki Poulose
> >> <[e-mail]>; Steven Price <[e-mail]>;
> >> [e-mail]; linux-arm- [e-mail];
> >> [e-mail]; [e-mail]; Steve Capper
> >> <[e-mail]>; Kaly Xin <[e-mail]>; Justin He
> >> <[e-mail]>; nd <[e-mail]>
> >> Subject: Re: [RFC PATCH v9 7/8] ptp: arm64: Enable ptp_kvm for arm64
> >>
> >> On 2019-12-10 03:40, Jianyong Wu wrote:
> >> > 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 <[e-mail]>
> >> > ---
> >> >  drivers/clocksource/arm_arch_timer.c | 22 ++++++++++++
> >> >  drivers/ptp/Kconfig                  |  2 +-
> >> >  drivers/ptp/ptp_kvm_arm64.c          | 53
> >> ++++++++++++++++++++++++++++
> >> >  3 files changed, 76 insertions(+), 1 deletion(-)  create mode
> >> > 100644 drivers/ptp/ptp_kvm_arm64.c
> >> >
> >> > diff --git a/drivers/clocksource/arm_arch_timer.c
> >> > b/drivers/clocksource/arm_arch_timer.c
> >> > index 277846decd33..72260b66f02e 100644
> >> > --- a/drivers/clocksource/arm_arch_timer.c
> >> > +++ b/drivers/clocksource/arm_arch_timer.c
> >> > @@ -1636,3 +1636,25 @@ 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
> >> > +
> >> > +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK_KVM)
> >> > +#include <linux/arm-smccc.h>
> >> > +int kvm_arch_ptp_get_crosststamp(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_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);
> >> > +	*cycle = hvc_res.a2 << 32 | hvc_res.a3;
> >>
> >> So why isn't that just a read of the virtual counter, given that what
> >> you do in the hypervisor seems to be "cntpct - cntvoff"?
> >>
> >> What am I missing here?
> >>
> > We need get clock time and counter cycle at the same time, so we can't
> > just read virtual counter at guest and must get it from host.
> 
> See my comment in my reply to patch #6: *Must* seems like a very strong
> word, and you don't explain *why* that's better than just computing the
> total hypercall cost. Hint: given the frequency of the counter (in the few MHz
> range) vs the frequency of a CPU (in the multiple GHz range, and with an IPC
> close enough to 1), I doubt that you'll see the counter making much progress
> across a hypercall.
> 
Sorry, I will avoid to use those strong words.

It's really the case that the hypercall won't across cycle in general. But sometimes, kernel preempt 
may happen in the middle of the hypercall which we can't assume how long before schedule back. so it's better capture them
together at the same time.

> >
> >> > +	*cs = &clocksource_counter;
> >> > +
> >> > +	return 0;
> >> > +}
> >> > +EXPORT_SYMBOL_GPL(kvm_arch_ptp_get_crosststamp);
> >> > +#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..f3f957117865
> >> > --- /dev/null
> >> > +++ b/drivers/ptp/ptp_kvm_arm64.c
> >> > @@ -0,0 +1,53 @@
> >> > +// 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_FU
> >> NC_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) {
> >> > +	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;
> >> > +}
> >> > +
> >> > +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;
> >> > +}
> >>
> >> I also wonder why this is all arm64 specific, while everything should
> >> also work just fine on 32bit.
> >>
> > ptp_kvm is a feature for cloud computing to keep time consistency from
> > container to container and to host on server,
> 
> Cloud computing? Never heard of that. Will probably never catch on.
> 
Eh...

> > So we focus it on arm64. Also I have never tested it on arm32 machine
> > ( we lack of arm32 machine)
> 
> I'm sure your employer can provide you with such a box. I can probably even
> tell you which cupboard they are stored in... ;-)
> 
I'm working on building environment for arm32 from now.

> > Do you think it's necessary to enable ptp_kvm on arm32? If so, I can
> > do that.
> 
> I can't see why we wouldn't, given that it should be a zero effort task (none
> of the code here is arch specific).

Ok, I will test it on arm32.
By the way, does nested virtualization diff between arm64 and arm32?

Thanks
Jianyong
> 
> Thanks,
> 
>          M.
> --
> Jazz is not dead. It just smells funny...

Jianyong Wu Jan. 10, 2020, 10:15 a.m. UTC | #6

Hi Marc,

> -----Original Message-----
> From: Marc Zyngier <[e-mail]>
> Sent: Thursday, January 9, 2020 5:46 PM
> To: Jianyong Wu <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; Mark Rutland <[e-mail]>;
> [e-mail]; Suzuki Poulose <[e-mail]>; Steven Price
> <[e-mail]>; [e-mail]; linux-arm-
> [e-mail]; [e-mail];
> [e-mail]; Steve Capper <[e-mail]>; Kaly Xin
> <[e-mail]>; Justin He <[e-mail]>; nd <[e-mail]>;
> [e-mail]
> Subject: Re: [RFC PATCH v9 7/8] ptp: arm64: Enable ptp_kvm for arm64
> 
> On 2020-01-09 09:24, Marc Zyngier wrote:
> > On 2020-01-09 05:59, Jianyong Wu wrote:
> 
> [...]
> 
> >> So we focus it on arm64. Also I have never tested it on arm32 machine
> >> ( we lack of arm32 machine)
> >
> > I'm sure your employer can provide you with such a box. I can probably
> > even tell you which cupboard they are stored in... ;-)
> >
> >> Do you think it's necessary to enable ptp_kvm on arm32? If so, I can
> >> do that.
> >
> > I can't see why we wouldn't, given that it should be a zero effort
> > task (none of the code here is arch specific).
> 
> To be clear, what I'm after is support for 32bit *guests*. I don't expect any
> issue with a 32bit host (it's all common code), but you should be able to test
> 32bit guests pretty easily (most ARMv8.0 CPUs support 32bit EL1).
> 
Get it

Thanks
Jianyong 

>          M.
> --
> Jazz is not dead. It just smells funny...

Marc Zyngier Jan. 10, 2020, 10:35 a.m. UTC | #7

Hi Jianyong,

On 2020-01-10 10:15, Jianyong Wu wrote:
> Hi Marc,

[...]

>> >> > +	ktime_overall = hvc_res.a0 << 32 | hvc_res.a1;
>> >> > +	*ts = ktime_to_timespec64(ktime_overall);
>> >> > +	*cycle = hvc_res.a2 << 32 | hvc_res.a3;
>> >>
>> >> So why isn't that just a read of the virtual counter, given that what
>> >> you do in the hypervisor seems to be "cntpct - cntvoff"?
>> >>
>> >> What am I missing here?
>> >>
>> > We need get clock time and counter cycle at the same time, so we can't
>> > just read virtual counter at guest and must get it from host.
>> 
>> See my comment in my reply to patch #6: *Must* seems like a very 
>> strong
>> word, and you don't explain *why* that's better than just computing 
>> the
>> total hypercall cost. Hint: given the frequency of the counter (in the 
>> few MHz
>> range) vs the frequency of a CPU (in the multiple GHz range, and with 
>> an IPC
>> close enough to 1), I doubt that you'll see the counter making much 
>> progress
>> across a hypercall.
>> 
> Sorry, I will avoid to use those strong words.
> 
> It's really the case that the hypercall won't across cycle in general.
> But sometimes, kernel preempt
> may happen in the middle of the hypercall which we can't assume how
> long before schedule back. so it's better capture them
> together at the same time.

Fair enough. Please document the rational, as I guess others will ask
the same questions.

Then the problem to solve is that of the reference counter, as you so 
far
assume the virtual counter. I guess you need to be able to let the guest
select the reference counter when calling the PTP service.

[...]

> By the way, does nested virtualization diff between arm64 and arm32?

There is no nested virt for 32bit (it is explicitly forbidden by the
architecture).

         M.

Jianyong Wu Jan. 13, 2020, 10:37 a.m. UTC | #8

Hi Marc,

> -----Original Message-----
> From: Marc Zyngier <[e-mail]>
> Sent: Friday, January 10, 2020 6:35 PM
> To: Jianyong Wu <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; Mark Rutland <[e-mail]>;
> [e-mail]; Suzuki Poulose <[e-mail]>; Steven Price
> <[e-mail]>; [e-mail]; linux-arm-
> [e-mail]; [e-mail];
> [e-mail]; Steve Capper <[e-mail]>; Kaly Xin
> <[e-mail]>; Justin He <[e-mail]>; nd <[e-mail]>
> Subject: Re: [RFC PATCH v9 7/8] ptp: arm64: Enable ptp_kvm for arm64
> 
> Hi Jianyong,
> 
> On 2020-01-10 10:15, Jianyong Wu wrote:
> > Hi Marc,
> 
> [...]
> 
> >> >> > +	ktime_overall = hvc_res.a0 << 32 | hvc_res.a1;
> >> >> > +	*ts = ktime_to_timespec64(ktime_overall);
> >> >> > +	*cycle = hvc_res.a2 << 32 | hvc_res.a3;
> >> >>
> >> >> So why isn't that just a read of the virtual counter, given that
> >> >> what you do in the hypervisor seems to be "cntpct - cntvoff"?
> >> >>
> >> >> What am I missing here?
> >> >>
> >> > We need get clock time and counter cycle at the same time, so we
> >> > can't just read virtual counter at guest and must get it from host.
> >>
> >> See my comment in my reply to patch #6: *Must* seems like a very
> >> strong word, and you don't explain *why* that's better than just
> >> computing the total hypercall cost. Hint: given the frequency of the
> >> counter (in the few MHz
> >> range) vs the frequency of a CPU (in the multiple GHz range, and with
> >> an IPC close enough to 1), I doubt that you'll see the counter making
> >> much progress across a hypercall.
> >>
> > Sorry, I will avoid to use those strong words.
> >
> > It's really the case that the hypercall won't across cycle in general.
> > But sometimes, kernel preempt
> > may happen in the middle of the hypercall which we can't assume how
> > long before schedule back. so it's better capture them together at the
> > same time.
> 
> Fair enough. Please document the rational, as I guess others will ask the
> same questions.
> 
Ok

> Then the problem to solve is that of the reference counter, as you so far
> assume the virtual counter. I guess you need to be able to let the guest
> select the reference counter when calling the PTP service.
> 
I could not come up with an idea about the point where the guest give this info of counter value.
Where we give that interface to ptp service, as it's not a user space application.

> [...]
> 
> > By the way, does nested virtualization diff between arm64 and arm32?
> 
> There is no nested virt for 32bit (it is explicitly forbidden by the architecture).
>  
Ok, thanks

Thanks
Jianyong 

>          M.
> --
> Jazz is not dead. It just smells funny...

Marc Zyngier Jan. 13, 2020, 11:21 a.m. UTC | #9

On 2020-01-13 10:37, Jianyong Wu wrote:
> Hi Marc,
> 
>> -----Original Message-----
>> From: Marc Zyngier <[e-mail]>
>> Sent: Friday, January 10, 2020 6:35 PM
>> To: Jianyong Wu <[e-mail]>
>> Cc: [e-mail]; [e-mail]; [e-mail];
>> [e-mail]; [e-mail]; 
>> [e-mail];
>> [e-mail]; Mark Rutland <[e-mail]>;
>> [e-mail]; Suzuki Poulose <[e-mail]>; Steven Price
>> <[e-mail]>; [e-mail]; linux-arm-
>> [e-mail]; [e-mail];
>> [e-mail]; Steve Capper <[e-mail]>; Kaly Xin
>> <[e-mail]>; Justin He <[e-mail]>; nd <[e-mail]>
>> Subject: Re: [RFC PATCH v9 7/8] ptp: arm64: Enable ptp_kvm for arm64
>> 
>> Hi Jianyong,
>> 
>> On 2020-01-10 10:15, Jianyong Wu wrote:
>> > Hi Marc,
>> 
>> [...]
>> 
>> >> >> > +	ktime_overall = hvc_res.a0 << 32 | hvc_res.a1;
>> >> >> > +	*ts = ktime_to_timespec64(ktime_overall);
>> >> >> > +	*cycle = hvc_res.a2 << 32 | hvc_res.a3;
>> >> >>
>> >> >> So why isn't that just a read of the virtual counter, given that
>> >> >> what you do in the hypervisor seems to be "cntpct - cntvoff"?
>> >> >>
>> >> >> What am I missing here?
>> >> >>
>> >> > We need get clock time and counter cycle at the same time, so we
>> >> > can't just read virtual counter at guest and must get it from host.
>> >>
>> >> See my comment in my reply to patch #6: *Must* seems like a very
>> >> strong word, and you don't explain *why* that's better than just
>> >> computing the total hypercall cost. Hint: given the frequency of the
>> >> counter (in the few MHz
>> >> range) vs the frequency of a CPU (in the multiple GHz range, and with
>> >> an IPC close enough to 1), I doubt that you'll see the counter making
>> >> much progress across a hypercall.
>> >>
>> > Sorry, I will avoid to use those strong words.
>> >
>> > It's really the case that the hypercall won't across cycle in general.
>> > But sometimes, kernel preempt
>> > may happen in the middle of the hypercall which we can't assume how
>> > long before schedule back. so it's better capture them together at the
>> > same time.
>> 
>> Fair enough. Please document the rational, as I guess others will ask 
>> the
>> same questions.
>> 
> Ok
> 
>> Then the problem to solve is that of the reference counter, as you so 
>> far
>> assume the virtual counter. I guess you need to be able to let the 
>> guest
>> select the reference counter when calling the PTP service.
>> 
> I could not come up with an idea about the point where the guest give
> this info of counter value.
> Where we give that interface to ptp service, as it's not a user space
> application.

Again: why don't you let the guest ask for the counter it wants as part
of the SMC call? What is preventing this?

         M.

Jianyong Wu Jan. 14, 2020, 10:22 a.m. UTC | #10

Hi Marc,

> -----Original Message-----
> From: Marc Zyngier <[e-mail]>
> Sent: Monday, January 13, 2020 7:21 PM
> To: Jianyong Wu <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; Mark Rutland <[e-mail]>;
> [e-mail]; Suzuki Poulose <[e-mail]>; Steven Price
> <[e-mail]>; [e-mail]; linux-arm-
> [e-mail]; [e-mail];
> [e-mail]; Steve Capper <[e-mail]>; Kaly Xin
> <[e-mail]>; Justin He <[e-mail]>; nd <[e-mail]>
> Subject: Re: [RFC PATCH v9 7/8] ptp: arm64: Enable ptp_kvm for arm64
> 
> On 2020-01-13 10:37, Jianyong Wu wrote:
> > Hi Marc,
> >
> >> -----Original Message-----
> >> From: Marc Zyngier <[e-mail]>
> >> Sent: Friday, January 10, 2020 6:35 PM
> >> To: Jianyong Wu <[e-mail]>
> >> Cc: [e-mail]; [e-mail];
> >> [e-mail]; [e-mail]; [e-mail];
> >> [e-mail]; [e-mail]; Mark
> >> Rutland <[e-mail]>; [e-mail]; Suzuki Poulose
> >> <[e-mail]>; Steven Price <[e-mail]>;
> >> [e-mail]; linux-arm- [e-mail];
> >> [e-mail]; [e-mail]; Steve Capper
> >> <[e-mail]>; Kaly Xin <[e-mail]>; Justin He
> >> <[e-mail]>; nd <[e-mail]>
> >> Subject: Re: [RFC PATCH v9 7/8] ptp: arm64: Enable ptp_kvm for arm64
> >>
> >> Hi Jianyong,
> >>
> >> On 2020-01-10 10:15, Jianyong Wu wrote:
> >> > Hi Marc,
> >>
> >> [...]
> >>
> >> >> >> > +	ktime_overall = hvc_res.a0 << 32 | hvc_res.a1;
> >> >> >> > +	*ts = ktime_to_timespec64(ktime_overall);
> >> >> >> > +	*cycle = hvc_res.a2 << 32 | hvc_res.a3;
> >> >> >>
> >> >> >> So why isn't that just a read of the virtual counter, given
> >> >> >> that what you do in the hypervisor seems to be "cntpct - cntvoff"?
> >> >> >>
> >> >> >> What am I missing here?
> >> >> >>
> >> >> > We need get clock time and counter cycle at the same time, so we
> >> >> > can't just read virtual counter at guest and must get it from host.
> >> >>
> >> >> See my comment in my reply to patch #6: *Must* seems like a very
> >> >> strong word, and you don't explain *why* that's better than just
> >> >> computing the total hypercall cost. Hint: given the frequency of
> >> >> the counter (in the few MHz
> >> >> range) vs the frequency of a CPU (in the multiple GHz range, and
> >> >> with an IPC close enough to 1), I doubt that you'll see the
> >> >> counter making much progress across a hypercall.
> >> >>
> >> > Sorry, I will avoid to use those strong words.
> >> >
> >> > It's really the case that the hypercall won't across cycle in general.
> >> > But sometimes, kernel preempt
> >> > may happen in the middle of the hypercall which we can't assume how
> >> > long before schedule back. so it's better capture them together at
> >> > the same time.
> >>
> >> Fair enough. Please document the rational, as I guess others will ask
> >> the same questions.
> >>
> > Ok
> >
> >> Then the problem to solve is that of the reference counter, as you so
> >> far assume the virtual counter. I guess you need to be able to let
> >> the guest select the reference counter when calling the PTP service.
> >>
> > I could not come up with an idea about the point where the guest give
> > this info of counter value.
> > Where we give that interface to ptp service, as it's not a user space
> > application.
> 
> Again: why don't you let the guest ask for the counter it wants as part of the
> SMC call? What is preventing this?
> 
Ok, let me try it

Thanks

>          M.
> --
> Jazz is not dead. It just smells funny...

Patch

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 277846decd33..72260b66f02e 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -1636,3 +1636,25 @@  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
+
+#if IS_ENABLED(CONFIG_PTP_1588_CLOCK_KVM)
+#include <linux/arm-smccc.h>
+int kvm_arch_ptp_get_crosststamp(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_crosststamp);
+#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..f3f957117865
--- /dev/null
+++ b/drivers/ptp/ptp_kvm_arm64.c
@@ -0,0 +1,53 @@ 
+// 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)
+{
+	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;
+}