lkml.org 
[lkml]   [2020]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH v14 08/10] ptp: arm64: Enable ptp_kvm for arm64
    Date
    Hi Marc,

    > -----Original Message-----
    > From: Marc Zyngier <maz@kernel.org>
    > Sent: Saturday, September 5, 2020 7:33 PM
    > To: Jianyong Wu <Jianyong.Wu@arm.com>
    > Cc: netdev@vger.kernel.org; yangbo.lu@nxp.com; john.stultz@linaro.org;
    > tglx@linutronix.de; pbonzini@redhat.com; sean.j.christopherson@intel.com;
    > richardcochran@gmail.com; Mark Rutland <Mark.Rutland@arm.com>;
    > will@kernel.org; Suzuki Poulose <Suzuki.Poulose@arm.com>; Steven Price
    > <Steven.Price@arm.com>; 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>; Justin He
    > <Justin.He@arm.com>; nd <nd@arm.com>
    > Subject: Re: [PATCH v14 08/10] ptp: arm64: Enable ptp_kvm for arm64
    >
    > On Fri, 04 Sep 2020 10:27:42 +0100,
    > Jianyong Wu <jianyong.wu@arm.com> wrote:
    > >
    > > Currently, there is no mechanism to keep time sync between guest and
    > > host in 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
    > > envirenment, we ask for higher time precision.
    > >
    > > kvm ptp clock, which choose the host clock source as a reference clock
    > > to sync time between guest and host, has been adopted by x86 which
    > > makes the time sync order from milliseconds to nanoseconds.
    > >
    > > This patch enables kvm ptp clock for arm64 and improve clock sync
    > > precison significantly.
    > >
    > > Test result comparisons between with kvm ptp clock and without it in
    > > 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.
    > >
    > > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
    > > ---
    > > drivers/clocksource/arm_arch_timer.c | 24 +++++++++++++
    > > drivers/ptp/Kconfig | 2 +-
    > > drivers/ptp/ptp_kvm_arm64.c | 53
    > ++++++++++++++++++++++++++++
    > > 3 files changed, 78 insertions(+), 1 deletion(-) create mode 100644
    > > drivers/ptp/ptp_kvm_arm64.c
    >
    > And I missed that one earlier:
    >
    > > diff --git a/drivers/clocksource/arm_arch_timer.c
    > > b/drivers/clocksource/arm_arch_timer.c
    > > index d55acffb0b90..aaf286e90092 100644
    > > --- a/drivers/clocksource/arm_arch_timer.c
    > > +++ b/drivers/clocksource/arm_arch_timer.c
    > > @@ -1650,3 +1650,27 @@ 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>
    >
    > No conditional includes, please.
    >
    Ok.

    > > +int kvm_arch_ptp_get_crosststamp(unsigned long *cycle, struct
    > timespec64 *ts,
    > > + struct clocksource **cs)
    > > +{
    > > + struct arm_smccc_res hvc_res;
    > > + ktime_t ktime;
    > > +
    > > + /* Currently, linux guest will always use the virtual counter */
    > > +
    > arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_PTP_FU
    > NC_ID,
    > > + ARM_PTP_VIRT_COUNTER, &hvc_res);
    >
    > You don't need to assume anything. This driver already provides you with the
    > information you need to tell the hypervisor which counter to
    > use:
    >
    > if (arch_timer_uses_ppi == ARCH_TIMER_VIRT_PPI)
    > ptp_counter = ARM_PTP_VIRT_COUNTER;
    > else
    > ptp_counter = ARM_PTP_PHYS_COUNTER;
    > arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_PTP_FU
    > NC_ID,
    > ptp_counter, &hvc_res);
    >
    > At least, this is vaguely future proof.
    >
    > The hypervisor will still have to discriminate between a call between a call
    > from vEL1 or vEL2 to decide whether to subtract the offset from the counter
    > value, but that's out of scope for now.

    Very kind of you!

    Thanks
    Jianyong
    >
    > M.
    >
    > --
    > Without deviation from the norm, progress is not possible.

    \
     
     \ /
      Last update: 2020-09-07 10:52    [W:4.817 / U:0.100 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site