6th generation Intel platforms will have an Always Running
Timer (ART) that always runs when the system is powered and
is available to both the CPU and various on-board devices.
Initially, those devices include audio and network. The
ART will give these devices the capability of precisely
cross timestamping their local device clock with the system
clock. The ART is precisely related to the TSC by a ratio
read from CPUID leaf 0x15.
A device (such as the network controller) produces cross timestamps in
terms of the ART and the local device clock. The ART value on its
own isn't useful.
The first two patches enable translation of ART to system time.
The first patch adds the correlated clocksource concept which is
an auxiliary clock directly relate-able to a clock registered as
a clocksource. The second patch adds the Intel specific ART
correlated clocksource.
The last two patches modify the PTP character driver to call a
cross timestamp function (getsynctime()) in the driver when
available and perform the cross timestamp in the e1000e driver.
The patches taken together enable sub-microsecond cross timestamps
between the system clock and network device clock
Changelog since v2:
Split out x86 architecture specific code from common timekeeping code
additions
Split ART initialization between early TSC initialization and TSC
frequency refinement. Now, cpu_has_art can be used in
driver initialization code
Added e1000e PTP init code that detects presence of ART/spt disabling
cross timestamp if they're not available
Added additional commenting in TSC/ART init code, minor renaming of
functions and variables for greater clarity
Fixed a few formatting problems in e1000e driver patch
Christopher Hall (2):
Add support for driver cross-timestamp to PTP_SYS_OFFSET ioctl
Enabling hardware supported PTP system/device crosstimestamping
Christopher S. Hall (2):
Add correlated clocksource deriving system time from an auxiliary
clocksource
Added ART correlated clocksource and ART CPU feature
Documentation/ptp/testptp.c | 6 +-
arch/x86/include/asm/cpufeature.h | 3 +-
arch/x86/include/asm/tsc.h | 2 +
arch/x86/kernel/tsc.c | 54 ++++++++++++++++++
drivers/net/ethernet/intel/e1000e/defines.h | 5 ++
drivers/net/ethernet/intel/e1000e/ptp.c | 88 +++++++++++++++++++++++++++++
drivers/net/ethernet/intel/e1000e/regs.h | 4 ++
drivers/ptp/ptp_chardev.c | 29 +++++++---
include/linux/clocksource.h | 33 +++++++++++
include/linux/ptp_clock_kernel.h | 7 +++
include/linux/timekeeping.h | 4 ++
include/uapi/linux/ptp_clock.h | 4 +-
kernel/time/timekeeping.c | 65 +++++++++++++++++++++
13 files changed, 292 insertions(+), 12 deletions(-)
--
2.1.4
Add struct correlated_cs with pointer to original clocksource and
function pointer to convert correlated clocksource to the original
Add get_correlated_timestamp() function which given specific correlated_cs
and correlated_ts convert correlated counter value to system time
Signed-off-by: Christopher S. Hall <[email protected]>
---
include/linux/clocksource.h | 33 +++++++++++++++++++++++
include/linux/timekeeping.h | 4 +++
kernel/time/timekeeping.c | 65 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 102 insertions(+)
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 278dd27..4bedadb 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -258,4 +258,37 @@ void acpi_generic_timer_init(void);
static inline void acpi_generic_timer_init(void) { }
#endif
+/*
+ * struct correlated_cs - Descriptor for a clocksource correlated to another
+ * clocksource
+ * @related_cs: Pointer to the related timekeeping clocksource
+ * @convert: Conversion function to convert a timestamp from
+ * the correlated clocksource to cycles of the related
+ * timekeeping clocksource
+ */
+struct correlated_cs {
+ struct clocksource *related_cs;
+ u64 (*convert)(struct correlated_cs *cs,
+ u64 cycles);
+};
+
+struct correlated_ts;
+
+/**
+ * struct correlated_ts - Descriptor for taking a correlated time stamp
+ * @get_ts: Function to read out a synced system and device
+ * timestamp
+ * @system_ts: The raw system clock timestamp
+ * @device_ts: The raw device timestamp
+ * @system_real: @system_ts converted to CLOCK_REALTIME
+ * @system_raw: @system_ts converted to CLOCK_MONOTONIC_RAW
+ */
+struct correlated_ts {
+ int (*get_ts)(struct correlated_ts *ts);
+ u64 system_ts;
+ u64 device_ts;
+ ktime_t system_real;
+ ktime_t system_raw;
+ void *private;
+};
#endif /* _LINUX_CLOCKSOURCE_H */
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 6e191e4..a9e1a2d 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -258,6 +258,10 @@ extern void timekeeping_inject_sleeptime64(struct timespec64 *delta);
*/
extern void getnstime_raw_and_real(struct timespec *ts_raw,
struct timespec *ts_real);
+struct correlated_ts;
+struct correlated_cs;
+extern int get_correlated_timestamp(struct correlated_ts *crt,
+ struct correlated_cs *crs);
/*
* Persistent clock related interfaces
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index bca3667..90a7c6f 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -312,6 +312,19 @@ static inline s64 timekeeping_get_ns(struct tk_read_base *tkr)
return nsec + arch_gettimeoffset();
}
+static inline s64 timekeeping_convert_to_ns(struct tk_read_base *tkr,
+ cycle_t cycles)
+{
+ cycle_t delta;
+ s64 nsec;
+
+ /* calculate the delta since the last update_wall_time */
+ delta = clocksource_delta(cycles, tkr->cycle_last, tkr->mask);
+
+ nsec = delta * tkr->mult + tkr->xtime_nsec;
+ return nsec >> tkr->shift;
+}
+
/**
* update_fast_timekeeper - Update the fast and NMI safe monotonic timekeeper.
* @tkr: Timekeeping readout base from which we take the update
@@ -885,6 +898,58 @@ EXPORT_SYMBOL(getnstime_raw_and_real);
#endif /* CONFIG_NTP_PPS */
/**
+ * get_correlated_timestamp - Get a correlated timestamp
+ *
+ * Reads a timestamp from a device and correlates it to system time
+ */
+int get_correlated_timestamp(struct correlated_ts *crt,
+ struct correlated_cs *crs)
+{
+ struct timekeeper *tk = &tk_core.timekeeper;
+ unsigned long seq;
+ cycles_t cycles;
+ ktime_t base;
+ s64 nsecs;
+ int ret;
+
+ do {
+ seq = read_seqcount_begin(&tk_core.seq);
+ /*
+ * Verify that the correlated clocksoure is related to
+ * the currently installed timekeeper clocksoure
+ */
+ if (tk->tkr_mono.clock != crs->related_cs)
+ return -ENODEV;
+
+ /*
+ * Try to get a timestamp from the device.
+ */
+ ret = crt->get_ts(crt);
+ if (ret)
+ return ret;
+
+ /*
+ * Convert the timestamp to timekeeper clock cycles
+ */
+ cycles = crs->convert(crs, crt->system_ts);
+
+ /* Convert to clock realtime */
+ base = ktime_add(tk->tkr_mono.base,
+ tk_core.timekeeper.offs_real);
+ nsecs = timekeeping_convert_to_ns(&tk->tkr_mono, cycles);
+ crt->system_real = ktime_add_ns(base, nsecs);
+
+ /* Convert to clock raw monotonic */
+ base = tk->tkr_raw.base;
+ nsecs = timekeeping_convert_to_ns(&tk->tkr_raw, cycles);
+ crt->system_raw = ktime_add_ns(base, nsecs);
+
+ } while (read_seqcount_retry(&tk_core.seq, seq));
+ return 0;
+}
+EXPORT_SYMBOL(get_correlated_timestamp);
+
+/**
* do_gettimeofday - Returns the time of day in a timeval
* @tv: pointer to the timeval to be set
*
--
2.1.4
Add detect_art() call to early TSC initialization which reads ART->TSC
numerator/denominator and sets CPU feature if present
Add convert_art_to_tsc() function performing conversion ART to TSC
Add art_timestamp referencing art_to_tsc() and clocksource_tsc enabling
driver conversion of ART to TSC
Signed-off-by: Christopher S. Hall <[email protected]>
---
arch/x86/include/asm/cpufeature.h | 3 ++-
arch/x86/include/asm/tsc.h | 2 ++
arch/x86/kernel/tsc.c | 54 +++++++++++++++++++++++++++++++++++++++
3 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 3d6606f..a9322e5 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -85,7 +85,7 @@
#define X86_FEATURE_P4 ( 3*32+ 7) /* "" P4 */
#define X86_FEATURE_CONSTANT_TSC ( 3*32+ 8) /* TSC ticks at a constant rate */
#define X86_FEATURE_UP ( 3*32+ 9) /* smp kernel running on up */
-/* free, was #define X86_FEATURE_FXSAVE_LEAK ( 3*32+10) * "" FXSAVE leaks FOP/FIP/FOP */
+#define X86_FEATURE_ART (3*32+10) /* Platform has always running timer (ART) */
#define X86_FEATURE_ARCH_PERFMON ( 3*32+11) /* Intel Architectural PerfMon */
#define X86_FEATURE_PEBS ( 3*32+12) /* Precise-Event Based Sampling */
#define X86_FEATURE_BTS ( 3*32+13) /* Branch Trace Store */
@@ -352,6 +352,7 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
#define cpu_has_de boot_cpu_has(X86_FEATURE_DE)
#define cpu_has_pse boot_cpu_has(X86_FEATURE_PSE)
#define cpu_has_tsc boot_cpu_has(X86_FEATURE_TSC)
+#define cpu_has_art boot_cpu_has(X86_FEATURE_ART)
#define cpu_has_pge boot_cpu_has(X86_FEATURE_PGE)
#define cpu_has_apic boot_cpu_has(X86_FEATURE_APIC)
#define cpu_has_sep boot_cpu_has(X86_FEATURE_SEP)
diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index 94605c0..8d52d91 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -45,6 +45,8 @@ static __always_inline cycles_t vget_cycles(void)
return (cycles_t)__native_read_tsc();
}
+extern struct correlated_cs art_timestamper;
+
extern void tsc_init(void);
extern void mark_tsc_unstable(char *reason);
extern int unsynchronized_tsc(void);
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 7437b41..13f12e0 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -939,10 +939,36 @@ static struct notifier_block time_cpufreq_notifier_block = {
.notifier_call = time_cpufreq_notifier
};
+#define ART_CPUID_LEAF (0x15)
+#define ART_MIN_DENOMINATOR (2)
+
+static u32 art_to_tsc_numerator;
+static u32 art_to_tsc_denominator;
+
+/*
+ * If ART is present detect the numberator:denominator to convert to TSC
+ */
+void detect_art(void)
+{
+ unsigned int unused[2];
+
+ if (boot_cpu_data.cpuid_level >= ART_CPUID_LEAF) {
+ cpuid(ART_CPUID_LEAF, &art_to_tsc_denominator,
+ &art_to_tsc_numerator, unused, unused+1);
+
+ if (art_to_tsc_denominator >= ART_MIN_DENOMINATOR) {
+ set_cpu_cap(&boot_cpu_data, X86_FEATURE_ART);
+ }
+ }
+}
+
static int __init cpufreq_tsc(void)
{
if (!cpu_has_tsc)
return 0;
+
+ detect_art();
+
if (boot_cpu_has(X86_FEATURE_CONSTANT_TSC))
return 0;
cpufreq_register_notifier(&time_cpufreq_notifier_block,
@@ -1059,6 +1085,32 @@ int unsynchronized_tsc(void)
return 0;
}
+/*
+ * Convert ART to TSC given numerator/denominator found in detect_art()
+ */
+static u64 convert_art_to_tsc(struct correlated_cs *cs, u64 cycles)
+{
+ u64 tmp, res;
+
+ switch (art_to_tsc_denominator) {
+ default:
+ res = (cycles / art_to_tsc_denominator) * art_to_tsc_numerator;
+ tmp = (cycles % art_to_tsc_denominator) * art_to_tsc_numerator;
+ res += tmp / art_to_tsc_denominator;
+ break;
+ case 2:
+ res = (cycles >> 1) * art_to_tsc_numerator;
+ tmp = (cycles & 0x1) * art_to_tsc_numerator;
+ res += tmp >> 1;
+ break;
+ }
+ return res;
+}
+
+struct correlated_cs art_timestamper = {
+ .convert = convert_art_to_tsc,
+};
+EXPORT_SYMBOL(art_timestamper);
static void tsc_refine_calibration_work(struct work_struct *work);
static DECLARE_DELAYED_WORK(tsc_irqwork, tsc_refine_calibration_work);
@@ -1130,6 +1182,8 @@ static void tsc_refine_calibration_work(struct work_struct *work)
(unsigned long)tsc_khz % 1000);
out:
+ if (cpu_has_art)
+ art_timestamper.related_cs = &clocksource_tsc;
clocksource_register_khz(&clocksource_tsc, tsc_khz);
}
--
2.1.4
From: Christopher Hall <[email protected]>
This patch allows system and device time ("cross-timestamp") to be
performed by the driver. Currently, the cross-timestamping is performed
in the PTP_SYS_OFFSET ioctl. The PTP clock driver reads gettimeofday()
and the gettime64() callback provided by the driver. The cross-timestamp
is best effort where the latency between the capture of system time
(getnstimeofday()) and the device time (driver callback) may be
significant.
This patch adds an additional callback getsynctime64(). Which will be
called when the driver is able to perform a more accurate, implementation
specific cross-timestamping. For example, future network devices that
implement PCIE PTM will be able to precisely correlate the device clock
with the system clock with virtually zero latency between captures.
This added callback can be used by the driver to expose this functionality.
The callback, getsynctime64(), will only be called when defined and
n_samples == 1 because the driver returns only 1 cross-timestamp where
multiple samples cannot be chained together.
This patch also adds to the capabilities ioctl (PTP_CLOCK_GETCAPS),
allowing applications to query whether or not drivers implement the
getsynctime callback, providing more precise cross timestamping.
Commit Details:
Added additional callback to ptp_clock_info:
* getsynctime64()
This takes 2 arguments referring to system and device time
With this callback drivers may provide both system time and device time
to ensure precise correlation
Modified PTP_SYS_OFFSET ioctl in PTP clock driver to use the above
callback if it's available
Added capability (PTP_CLOCK_GETCAPS) for checking whether driver supports
cross timestamping
Added check for cross timestamping flag to testptp.c
Signed-off-by: Christopher S. Hall <[email protected]>
---
Documentation/ptp/testptp.c | 6 ++++--
drivers/ptp/ptp_chardev.c | 29 +++++++++++++++++++++--------
include/linux/ptp_clock_kernel.h | 7 +++++++
include/uapi/linux/ptp_clock.h | 4 +++-
4 files changed, 35 insertions(+), 11 deletions(-)
diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c
index 2bc8abc..8004efd 100644
--- a/Documentation/ptp/testptp.c
+++ b/Documentation/ptp/testptp.c
@@ -276,13 +276,15 @@ int main(int argc, char *argv[])
" %d external time stamp channels\n"
" %d programmable periodic signals\n"
" %d pulse per second\n"
- " %d programmable pins\n",
+ " %d programmable pins\n"
+ " %d cross timestamping\n",
caps.max_adj,
caps.n_alarm,
caps.n_ext_ts,
caps.n_per_out,
caps.pps,
- caps.n_pins);
+ caps.n_pins,
+ caps.cross_timestamping);
}
}
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index da7bae9..392ccfa 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -124,7 +124,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
struct ptp_clock_info *ops = ptp->info;
struct ptp_clock_time *pct;
- struct timespec64 ts;
+ struct timespec64 ts, systs;
int enable, err = 0;
unsigned int i, pin_index;
@@ -138,6 +138,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
caps.n_per_out = ptp->info->n_per_out;
caps.pps = ptp->info->pps;
caps.n_pins = ptp->info->n_pins;
+ caps.cross_timestamping = ptp->info->getsynctime64 != NULL;
if (copy_to_user((void __user *)arg, &caps, sizeof(caps)))
err = -EFAULT;
break;
@@ -196,19 +197,31 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
break;
}
pct = &sysoff->ts[0];
- for (i = 0; i < sysoff->n_samples; i++) {
- getnstimeofday64(&ts);
+ if (ptp->info->getsynctime64 && sysoff->n_samples == 1 &&
+ ptp->info->getsynctime64(ptp->info, &ts, &systs) == 0) {
+ pct->sec = systs.tv_sec;
+ pct->nsec = systs.tv_nsec;
+ pct++;
pct->sec = ts.tv_sec;
pct->nsec = ts.tv_nsec;
pct++;
- ptp->info->gettime64(ptp->info, &ts);
+ pct->sec = systs.tv_sec;
+ pct->nsec = systs.tv_nsec;
+ } else {
+ for (i = 0; i < sysoff->n_samples; i++) {
+ getnstimeofday64(&ts);
+ pct->sec = ts.tv_sec;
+ pct->nsec = ts.tv_nsec;
+ pct++;
+ ptp->info->gettime64(ptp->info, &ts);
+ pct->sec = ts.tv_sec;
+ pct->nsec = ts.tv_nsec;
+ pct++;
+ }
+ getnstimeofday64(&ts);
pct->sec = ts.tv_sec;
pct->nsec = ts.tv_nsec;
- pct++;
}
- getnstimeofday64(&ts);
- pct->sec = ts.tv_sec;
- pct->nsec = ts.tv_nsec;
if (copy_to_user((void __user *)arg, sysoff, sizeof(*sysoff)))
err = -EFAULT;
break;
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index b8b7306..8c43345 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -67,6 +67,11 @@ struct ptp_clock_request {
* @gettime64: Reads the current time from the hardware clock.
* parameter ts: Holds the result.
*
+ * @getsynctime64: Reads the current time from the hardware clock and system
+ * clock simultaneously.
+ * parameter dev: Holds the device time
+ * parameter sys: Holds the system time
+ *
* @settime64: Set the current time on the hardware clock.
* parameter ts: Time value to set.
*
@@ -105,6 +110,8 @@ struct ptp_clock_info {
int (*adjfreq)(struct ptp_clock_info *ptp, s32 delta);
int (*adjtime)(struct ptp_clock_info *ptp, s64 delta);
int (*gettime64)(struct ptp_clock_info *ptp, struct timespec64 *ts);
+ int (*getsynctime64)(struct ptp_clock_info *ptp, struct timespec64 *dev,
+ struct timespec64 *sys);
int (*settime64)(struct ptp_clock_info *p, const struct timespec64 *ts);
int (*enable)(struct ptp_clock_info *ptp,
struct ptp_clock_request *request, int on);
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index f0b7bfe..ffb2635 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -51,7 +51,9 @@ struct ptp_clock_caps {
int n_per_out; /* Number of programmable periodic signals. */
int pps; /* Whether the clock supports a PPS callback. */
int n_pins; /* Number of input/output pins. */
- int rsv[14]; /* Reserved for future use. */
+ /* Whether the clock supports precise system-device cross timestamps */
+ int cross_timestamping;
+ int rsv[13]; /* Reserved for future use. */
};
struct ptp_extts_request {
--
2.1.4
From: Christopher Hall <[email protected]>
Add getsynctime() PTP device callback to cross timestamp system device
clock using ART translation depends on platform being >= SPT
and having ART
getsynctime() reads ART (TSC-derived)/device cross timestamp and
converts to realtime/device time reporting cross timestamp to
PTP driver
Signed-off-by: Christopher S. Hall <[email protected]>
---
drivers/net/ethernet/intel/e1000e/defines.h | 5 ++
drivers/net/ethernet/intel/e1000e/ptp.c | 88 +++++++++++++++++++++++++++++
drivers/net/ethernet/intel/e1000e/regs.h | 4 ++
3 files changed, 97 insertions(+)
diff --git a/drivers/net/ethernet/intel/e1000e/defines.h b/drivers/net/ethernet/intel/e1000e/defines.h
index 133d407..13cff75 100644
--- a/drivers/net/ethernet/intel/e1000e/defines.h
+++ b/drivers/net/ethernet/intel/e1000e/defines.h
@@ -527,6 +527,11 @@
#define E1000_RXCW_C 0x20000000 /* Receive config */
#define E1000_RXCW_SYNCH 0x40000000 /* Receive config synch */
+/* HH Time Sync */
+#define E1000_TSYNCTXCTL_MAX_ALLOWED_DLY_MASK 0x0000F000 /* max delay */
+#define E1000_TSYNCTXCTL_SYNC_COMP 0x40000000 /* sync complete */
+#define E1000_TSYNCTXCTL_START_SYNC 0x80000000 /* initiate sync */
+
#define E1000_TSYNCTXCTL_VALID 0x00000001 /* Tx timestamp valid */
#define E1000_TSYNCTXCTL_ENABLED 0x00000010 /* enable Tx timestamping */
diff --git a/drivers/net/ethernet/intel/e1000e/ptp.c b/drivers/net/ethernet/intel/e1000e/ptp.c
index 25a0ad5..228f3f3 100644
--- a/drivers/net/ethernet/intel/e1000e/ptp.c
+++ b/drivers/net/ethernet/intel/e1000e/ptp.c
@@ -25,6 +25,8 @@
*/
#include "e1000.h"
+#include <asm/tsc.h>
+#include <linux/timekeeping.h>
/**
* e1000e_phc_adjfreq - adjust the frequency of the hardware clock
@@ -98,6 +100,87 @@ static int e1000e_phc_adjtime(struct ptp_clock_info *ptp, s64 delta)
return 0;
}
+#define MAX_HW_WAIT_COUNT (3)
+
+static int e1000e_phc_get_ts(struct correlated_ts *cts)
+{
+ struct e1000_adapter *adapter = (struct e1000_adapter *)cts->private;
+ struct e1000_hw *hw = &adapter->hw;
+ int i;
+ u32 tsync_ctrl;
+ int ret;
+
+ tsync_ctrl = er32(TSYNCTXCTL);
+ tsync_ctrl |= E1000_TSYNCTXCTL_START_SYNC |
+ E1000_TSYNCTXCTL_MAX_ALLOWED_DLY_MASK;
+ ew32(TSYNCTXCTL, tsync_ctrl);
+ for (i = 0; i < MAX_HW_WAIT_COUNT; ++i) {
+ udelay(1);
+ tsync_ctrl = er32(TSYNCTXCTL);
+ if (tsync_ctrl & E1000_TSYNCTXCTL_SYNC_COMP)
+ break;
+ }
+
+ if (i == MAX_HW_WAIT_COUNT) {
+ ret = -ETIMEDOUT;
+ } else {
+ ret = 0;
+ cts->system_ts = er32(PLTSTMPH);
+ cts->system_ts <<= 32;
+ cts->system_ts |= er32(PLTSTMPL);
+ cts->device_ts = er32(SYSSTMPH);
+ cts->device_ts <<= 32;
+ cts->device_ts |= er32(SYSSTMPL);
+ }
+
+ return ret;
+}
+
+/**
+ * e1000e_phc_getsynctime - Reads the current time from the hardware clock and
+ * correlated system time
+ * @ptp: ptp clock structure
+ * @devts: timespec structure to hold the current device time value
+ * @systs: timespec structure to hold the current system time value
+ *
+ * Read device and system (ART) clock simultaneously and return the correct
+ * clock values in ns after converting into a struct timespec.
+ **/
+static int e1000e_phc_getsynctime(struct ptp_clock_info *ptp,
+ struct timespec64 *devts,
+ struct timespec64 *systs)
+{
+ struct e1000_adapter *adapter = container_of(ptp, struct e1000_adapter,
+ ptp_clock_info);
+ unsigned long flags;
+ u32 remainder;
+ struct correlated_ts art_correlated_ts;
+ u64 device_time;
+ int ret;
+
+ art_correlated_ts.get_ts = e1000e_phc_get_ts;
+ art_correlated_ts.private = adapter;
+ ret = get_correlated_timestamp(&art_correlated_ts,
+ &art_timestamper);
+ if (ret != 0)
+ goto bail;
+
+ systs->tv_sec =
+ div_u64_rem(art_correlated_ts.system_real.tv64,
+ NSEC_PER_SEC, &remainder);
+ systs->tv_nsec = remainder;
+ spin_lock_irqsave(&adapter->systim_lock, flags);
+ device_time = timecounter_cyc2time(&adapter->tc,
+ art_correlated_ts.device_ts);
+ spin_unlock_irqrestore(&adapter->systim_lock, flags);
+ devts->tv_sec =
+ div_u64_rem(device_time, NSEC_PER_SEC, &remainder);
+ devts->tv_nsec = remainder;
+
+bail:
+ return ret;
+}
+
/**
* e1000e_phc_gettime - Reads the current time from the hardware clock
* @ptp: ptp clock structure
@@ -190,6 +273,7 @@ static const struct ptp_clock_info e1000e_ptp_clock_info = {
.adjfreq = e1000e_phc_adjfreq,
.adjtime = e1000e_phc_adjtime,
.gettime64 = e1000e_phc_gettime,
+ .getsynctime64 = e1000e_phc_getsynctime,
.settime64 = e1000e_phc_settime,
.enable = e1000e_phc_enable,
};
@@ -236,6 +320,10 @@ void e1000e_ptp_init(struct e1000_adapter *adapter)
break;
}
+ /* CPU must have ART and GBe must be from Sunrise Point or greater */
+ if (hw->mac.type < e1000_pch_spt || !cpu_has_art)
+ adapter->ptp_clock_info.getsynctime64 = NULL;
+
INIT_DELAYED_WORK(&adapter->systim_overflow_work,
e1000e_systim_overflow_work);
diff --git a/drivers/net/ethernet/intel/e1000e/regs.h b/drivers/net/ethernet/intel/e1000e/regs.h
index b24e5fe..4dd5b54 100644
--- a/drivers/net/ethernet/intel/e1000e/regs.h
+++ b/drivers/net/ethernet/intel/e1000e/regs.h
@@ -246,6 +246,10 @@
#define E1000_SYSTIML 0x0B600 /* System time register Low - RO */
#define E1000_SYSTIMH 0x0B604 /* System time register High - RO */
#define E1000_TIMINCA 0x0B608 /* Increment attributes register - RW */
+#define E1000_SYSSTMPL 0x0B648 /* HH Timesync system stamp low register */
+#define E1000_SYSSTMPH 0x0B64C /* HH Timesync system stamp hi register */
+#define E1000_PLTSTMPL 0x0B640 /* HH Timesync platform stamp low register */
+#define E1000_PLTSTMPH 0x0B644 /* HH Timesync platform stamp hi register */
#define E1000_RXMTRL 0x0B634 /* Time sync Rx EtherType and Msg Type - RW */
#define E1000_RXUDP 0x0B638 /* Time Sync Rx UDP Port - RW */
--
2.1.4
On Fri, 21 Aug 2015, Christopher S. Hall wrote:
> Add struct correlated_cs with pointer to original clocksource and
> function pointer to convert correlated clocksource to the original
>
> Add get_correlated_timestamp() function which given specific correlated_cs
> and correlated_ts convert correlated counter value to system time
This is not a proper changelog.
1) The subject line lacks a subsystem prefix
timekeeping:
Is the proper choice here
2) The subject line should be short and precise
timekeeping: Add mechanism to gather correlated timestamps
Might be an informative one.
3) The changelog itself should describe the reason why we want this
change, the purpose of the change etc.
Add foo
Add bar
Is pointless because we can see that from the patch itself.
What the patch cannot not explain is the WHY. That's what the
changelog is for.
4) You dropped the authorship
The proper way to do this is to add a 'FROM: author' at the top of
the changelog body.
As I wrote the patch, so I give you a changelog along with it:
<---
Subject: timekeeping: Add mechanism to gather correlated timestamps
From: Thomas Gleixner <[email protected]>
Modern Intel hardware provides the so called Always Running Timer
(ART). The TSC which is usually used for timekeeping is derived from
ART and runs with a fixed frequency ratio to it. ART is routed to
devices and allows to take atomic timestamp samples from the device
clock and the ART. One use case is PTP timestamps on network cards. We
want to utilize this feature as it allows us to better correlate the
PTP timestamp to the system time.
In order to gather precise timestamps we need to make sure that the
conversion from ART to TSC and the following conversion from TSC to
clock realtime happens synchronized with the ongoing timekeeping
updates. Otherwise we might convert an ART timestamp from point A in
time with the conversion factors of point B in time. These conversion
factors can differ due to NTP/PTP frequency adjustments and therefor
the resulting clock realtime timestamp would be slightly off, which is
contrary to the whole purpose of synchronized hardware timestamps.
Provide data structures which describe the correlation between two
clocksources and a function to gather correlated and convert
timestamps from a device. The function is as any other timekeeping
function protected against current timekeeper updates via the
timekeeper sequence lock. It calls the device function to gather the
hardware timestamps and converts them to clock real time and clock
monotonic raw.
Signed-off-by: Thomas Gleixner <[email protected]>
---->
Can you see the difference?
> Signed-off-by: Christopher S. Hall <[email protected]>
> ---
> include/linux/clocksource.h | 33 +++++++++++++++++++++++
> include/linux/timekeeping.h | 4 +++
> kernel/time/timekeeping.c | 65 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 102 insertions(+)
>
> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> index 278dd27..4bedadb 100644
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -258,4 +258,37 @@ void acpi_generic_timer_init(void);
> static inline void acpi_generic_timer_init(void) { }
> #endif
>
> +/*
> + * struct correlated_cs - Descriptor for a clocksource correlated to another
> + * clocksource
Don't believe checkpatch here. KernelDoc requires that this is one
line, 80 char limit or not.
> /**
> + * get_correlated_timestamp - Get a correlated timestamp
> + *
Lacks the parameter documentation:
* @crt: Pointer to a correlated timestamp structure which provides
* the device specific timestamp function and is used to store
* the raw and the correlated timestamps.
* @crs: Pointer to a correlated clocksource structure which describes
* the correlated clocksource and provides a conversion function
* to the timekeeping clocksource
> + return 0;
> +}
> +EXPORT_SYMBOL(get_correlated_timestamp);
EXPORT_SYMBOL_GPL please.
Thanks,
tglx
On Fri, 21 Aug 2015, Christopher S. Hall wrote:
> Add detect_art() call to early TSC initialization which reads ART->TSC
> numerator/denominator and sets CPU feature if present
>
> Add convert_art_to_tsc() function performing conversion ART to TSC
>
> Add art_timestamp referencing art_to_tsc() and clocksource_tsc enabling
> driver conversion of ART to TSC
That changelog needs a rewrite. See patch 1/4
> @@ -352,6 +352,7 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
> #define cpu_has_de boot_cpu_has(X86_FEATURE_DE)
> #define cpu_has_pse boot_cpu_has(X86_FEATURE_PSE)
> #define cpu_has_tsc boot_cpu_has(X86_FEATURE_TSC)
> +#define cpu_has_art boot_cpu_has(X86_FEATURE_ART)
Please do not add more cpu_has macros. There is nothing wrong to write
boot_cpu_has(X86_FEATURE_ART) in the code.
> +#define ART_CPUID_LEAF (0x15)
> +#define ART_MIN_DENOMINATOR (2)
#define ART_CPUID_LEAF 0x15
#define ART_MIN_DENOMINATOR 2
Why is the minimum denominator 2? That wants a comment.
> +static u32 art_to_tsc_numerator;
> +static u32 art_to_tsc_denominator;
Both want to be read_mostly
> +/*
> + * If ART is present detect the numberator:denominator to convert to TSC
> + */
> +void detect_art(void)
> +{
> + unsigned int unused[2];
> +
> + if (boot_cpu_data.cpuid_level >= ART_CPUID_LEAF) {
> + cpuid(ART_CPUID_LEAF, &art_to_tsc_denominator,
> + &art_to_tsc_numerator, unused, unused+1);
> +
> + if (art_to_tsc_denominator >= ART_MIN_DENOMINATOR) {
> + set_cpu_cap(&boot_cpu_data, X86_FEATURE_ART);
> + }
No parentheses around one liners please.
> + }
> +}
> +
> static int __init cpufreq_tsc(void)
> {
> if (!cpu_has_tsc)
> return 0;
> +
> + detect_art();
> +
> if (boot_cpu_has(X86_FEATURE_CONSTANT_TSC))
> return 0;
> cpufreq_register_notifier(&time_cpufreq_notifier_block,
> @@ -1059,6 +1085,32 @@ int unsynchronized_tsc(void)
> return 0;
> }
>
> +/*
> + * Convert ART to TSC given numerator/denominator found in detect_art()
> + */
> +static u64 convert_art_to_tsc(struct correlated_cs *cs, u64 cycles)
> +{
> + u64 tmp, res;
> +
> + switch (art_to_tsc_denominator) {
> + default:
> + res = (cycles / art_to_tsc_denominator) * art_to_tsc_numerator;
> + tmp = (cycles % art_to_tsc_denominator) * art_to_tsc_numerator;
> + res += tmp / art_to_tsc_denominator;
> + break;
> + case 2:
> + res = (cycles >> 1) * art_to_tsc_numerator;
> + tmp = (cycles & 0x1) * art_to_tsc_numerator;
> + res += tmp >> 1;
> + break;
Is it really worth do do this optimization? And if we do it we
shouldn't special case it for 2. You can check at ART detection time
whether the denominator is a power of two and have a flag which
selects a div/mod base or a shift based conversion.
Thanks,
tglx
On Fri, 21 Aug 2015, Christopher S. Hall wrote:
> From: Christopher Hall <[email protected]>
>
> This patch allows system and device time ("cross-timestamp") to be
> performed by the driver. Currently, the cross-timestamping is performed
> in the PTP_SYS_OFFSET ioctl. The PTP clock driver reads gettimeofday()
> and the gettime64() callback provided by the driver. The cross-timestamp
> is best effort where the latency between the capture of system time
> (getnstimeofday()) and the device time (driver callback) may be
> significant.
>
> This patch adds an additional callback getsynctime64(). Which will be
> called when the driver is able to perform a more accurate, implementation
> specific cross-timestamping. For example, future network devices that
> implement PCIE PTM will be able to precisely correlate the device clock
> with the system clock with virtually zero latency between captures.
> This added callback can be used by the driver to expose this functionality.
>
> The callback, getsynctime64(), will only be called when defined and
> n_samples == 1 because the driver returns only 1 cross-timestamp where
> multiple samples cannot be chained together.
>
> This patch also adds to the capabilities ioctl (PTP_CLOCK_GETCAPS),
> allowing applications to query whether or not drivers implement the
> getsynctime callback, providing more precise cross timestamping.
That looks close to a proper changelog. A few nitpicks though.
Please avoid 'This patch does ...' phrases. We already know that this
is a patch.
> Commit Details:
Please get rid of this. It's useless noise.
> Added additional callback to ptp_clock_info:
>
> * getsynctime64()
> @@ -196,19 +197,31 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
> break;
> }
> pct = &sysoff->ts[0];
> - for (i = 0; i < sysoff->n_samples; i++) {
> - getnstimeofday64(&ts);
> + if (ptp->info->getsynctime64 && sysoff->n_samples == 1 &&
The number of samples should be irrelevant for this sampling method.
> + ptp->info->getsynctime64(ptp->info, &ts, &systs) == 0) {
Why is this function taking struct timespec64 pointers? Just so every
driver which implements the callback needs to convert from u64 to
struct timespec64? That's simply wrong. Use u64 for both and do the
conversion in the ioctl.
Thanks,
tglx
On Fri, 21 Aug 2015, Christopher S. Hall wrote:
> From: Christopher Hall <[email protected]>
>
> Add getsynctime() PTP device callback to cross timestamp system device
> clock using ART translation depends on platform being >= SPT
> and having ART
>
> getsynctime() reads ART (TSC-derived)/device cross timestamp and
> converts to realtime/device time reporting cross timestamp to
> PTP driver
See patch 1/4
> index 25a0ad5..228f3f3 100644
> --- a/drivers/net/ethernet/intel/e1000e/ptp.c
> +++ b/drivers/net/ethernet/intel/e1000e/ptp.c
> @@ -25,6 +25,8 @@
> */
>
> #include "e1000.h"
> +#include <asm/tsc.h>
> +#include <linux/timekeeping.h>
The usual way to order includes is:
#include <linux/timekeeping.h>
#include <asm/tsc.h>
#include "e1000.h"
> +/**
> + * e1000e_phc_getsynctime - Reads the current time from the hardware clock and
> + * correlated system time
> + * @ptp: ptp clock structure
> + * @devts: timespec structure to hold the current device time value
> + * @systs: timespec structure to hold the current system time value
> + *
> + * Read device and system (ART) clock simultaneously and return the correct
> + * clock values in ns after converting into a struct timespec.
> + **/
> +static int e1000e_phc_getsynctime(struct ptp_clock_info *ptp,
> + struct timespec64 *devts,
> + struct timespec64 *systs)
> +{
> + struct e1000_adapter *adapter = container_of(ptp, struct e1000_adapter,
> + ptp_clock_info);
> + unsigned long flags;
> + u32 remainder;
> + struct correlated_ts art_correlated_ts;
> + u64 device_time;
> + int ret;
> +
> + art_correlated_ts.get_ts = e1000e_phc_get_ts;
> + art_correlated_ts.private = adapter;
> + ret = get_correlated_timestamp(&art_correlated_ts,
> + &art_timestamper);
Pointless line break
> + if (ret != 0)
> + goto bail;
What's the purpose of this goto?
if (ret)
return ret;
is completely sufficient.
> +
> + systs->tv_sec =
> + div_u64_rem(art_correlated_ts.system_real.tv64,
> + NSEC_PER_SEC, &remainder);
> + systs->tv_nsec = remainder;
ktime_to_timespec64() perhaps?
And please move that conversion to the ptp ioctl
> + spin_lock_irqsave(&adapter->systim_lock, flags);
> + device_time = timecounter_cyc2time(&adapter->tc,
> + art_correlated_ts.device_ts);
....
> + /* CPU must have ART and GBe must be from Sunrise Point or greater */
> + if (hw->mac.type < e1000_pch_spt || !cpu_has_art)
> + adapter->ptp_clock_info.getsynctime64 = NULL;
We do it the other way round. We leave the default NULL and update it
if we detect the feature.
Thanks,
tglx
On Sat, Aug 22, 2015 at 10:33:48PM +0200, Thomas Gleixner wrote:
> > @@ -196,19 +197,31 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
> > break;
> > }
> > pct = &sysoff->ts[0];
> > - for (i = 0; i < sysoff->n_samples; i++) {
> > - getnstimeofday64(&ts);
> > + if (ptp->info->getsynctime64 && sysoff->n_samples == 1 &&
>
> The number of samples should be irrelevant for this sampling method.
Chris had send me a preview of this before he posted, so I can explain
that test for one sample.
User space requests N (1 to 25) samples of the two clocks. The kernel
is supposed to deliver that many samples. This has always been the
documented behavior. From ptp_clock.h:
struct ptp_sys_offset {
unsigned int n_samples; /* Desired number of measurements. */
unsigned int rsv[3]; /* Reserved for future use. */
/*
* Array of interleaved system/phc time stamps. The kernel
* will provide 2*n_samples + 1 time stamps, with the last
* one as a system time stamp.
*/
struct ptp_clock_time ts[2 * PTP_MAX_SAMPLES + 1];
};
So the kernel cannot simply change n_samples to 1.
I would prefer to have a new system call that compares any two posix
clock_t, but that is of course more work.
Allowing n_samples=1 as a special case is a kind of overloading of the
ioctl to support the new capability. At least it preserves the
behavior of the interface from the user's perspective.
Thanks,
Richard
On Sat, 22 Aug 2015, Richard Cochran wrote:
> On Sat, Aug 22, 2015 at 10:33:48PM +0200, Thomas Gleixner wrote:
> > > @@ -196,19 +197,31 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
> > > break;
> > > }
> > > pct = &sysoff->ts[0];
> > > - for (i = 0; i < sysoff->n_samples; i++) {
> > > - getnstimeofday64(&ts);
> > > + if (ptp->info->getsynctime64 && sysoff->n_samples == 1 &&
> >
> > The number of samples should be irrelevant for this sampling method.
>
> Chris had send me a preview of this before he posted, so I can explain
> that test for one sample.
>
> User space requests N (1 to 25) samples of the two clocks. The kernel
> is supposed to deliver that many samples. This has always been the
> documented behavior. From ptp_clock.h:
>
> struct ptp_sys_offset {
> unsigned int n_samples; /* Desired number of measurements. */
> unsigned int rsv[3]; /* Reserved for future use. */
> /*
> * Array of interleaved system/phc time stamps. The kernel
> * will provide 2*n_samples + 1 time stamps, with the last
> * one as a system time stamp.
> */
> struct ptp_clock_time ts[2 * PTP_MAX_SAMPLES + 1];
> };
>
> So the kernel cannot simply change n_samples to 1.
>
> I would prefer to have a new system call that compares any two posix
> clock_t, but that is of course more work.
>
> Allowing n_samples=1 as a special case is a kind of overloading of the
> ioctl to support the new capability. At least it preserves the
> behavior of the interface from the user's perspective.
So why can't you take N samples from the synced hardware? It does not
make any sense to me to switch to the imprecise mode if nsamples > 1.
You can also provide a new IOCTL PTP_SYS_OFFSET_PRECISE which returns
-ENOSYS if hardware timestamping is not available and avoid the whole
nsamples dance for the case where we can get precise timestamps.
Thanks,
tglx
On Sun, Aug 23, 2015 at 10:15:00AM +0200, Thomas Gleixner wrote:
> So why can't you take N samples from the synced hardware? It does not
> make any sense to me to switch to the imprecise mode if nsamples > 1.
Ok, then I prefer to leave this "imprecise" method in place and ...
> You can also provide a new IOCTL PTP_SYS_OFFSET_PRECISE which returns
> -ENOSYS if hardware timestamping is not available and avoid the whole
> nsamples dance for the case where we can get precise timestamps.
have this for the new way.
By keeping the imprecise method, we will be able to run both methods
on the new hardware. That will help to quantify how imprecise the old
method is.
Thanks,
Richard
> -----Original Message-----
> From: Richard Cochran [mailto:[email protected]]
> Sent: Sunday, August 23, 2015 4:26 AM
> To: Thomas Gleixner
> Cc: Hall, Christopher S; Kirsher, Jeffrey T; [email protected];
> [email protected]; [email protected]; [email protected]; linux-
> [email protected]; [email protected]; intel-wired-
> [email protected]; [email protected]
> Subject: Re: [PATCH v3 3/4] Add support for driver cross-timestamp to
> PTP_SYS_OFFSET ioctl
>
> On Sun, Aug 23, 2015 at 10:15:00AM +0200, Thomas Gleixner wrote:
> > So why can't you take N samples from the synced hardware? It does not
> > make any sense to me to switch to the imprecise mode if nsamples > 1.
>
> Ok, then I prefer to leave this "imprecise" method in place and ...
>
> > You can also provide a new IOCTL PTP_SYS_OFFSET_PRECISE which returns
> > -ENOSYS if hardware timestamping is not available and avoid the whole
> > nsamples dance for the case where we can get precise timestamps.
>
> have this for the new way.
>
> By keeping the imprecise method, we will be able to run both methods
> on the new hardware. That will help to quantify how imprecise the old
> method is.
This means: remove code changes from the PTP_SYS_OFFSET ioctl and call getsynctime64() from a new ioctl PTP_SYS_OFFSET_PRECISE. Right?
And use the same type (struct ptp_sys_offset) for the new ioctl? Or should a new simplified struct be used? Such as:
struct precise_ptp_sys_offset {
struct ptp_clock_time device;
struct ptp_clock_time system;
};
Does it make sense to keep the "cross-timestamp" capabilities flag as-is?
>
> Thanks,
> Richard
On Mon, Aug 24, 2015 at 08:16:51PM +0000, Hall, Christopher S wrote:
>
> This means: remove code changes from the PTP_SYS_OFFSET ioctl and call getsynctime64() from a new ioctl PTP_SYS_OFFSET_PRECISE. Right?
Yes.
> And use the same type (struct ptp_sys_offset) for the new ioctl? Or should a new simplified struct be used? Such as:
>
> struct precise_ptp_sys_offset {
> struct ptp_clock_time device;
> struct ptp_clock_time system;
> };
I don't have a strong preference either way. I would not mind reusing
the existing struct.
> Does it make sense to keep the "cross-timestamp" capabilities flag as-is?
Yes, indeed.
Thanks,
Richard