2015-08-07 23:00:48

by Hall, Christopher S

[permalink] [raw]
Subject: [PATCH v2 0/4] Patchset enabling hardware based cross-timestamps for next gen Intel platforms

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.

A system clock value like TSC or ART is not useful
unless translated to system time. The first *two* patches
enable this by changing the timekeeping code to return a
system time given a system clock value and translating ART
to TSC.

The last two patches modify the PTP driver to call a
cross timestamp function in the driver when available and
perform the cross timestamp in the e1000e driver.

Given the precise relationship between the network device
clock and system time enables better synchronization of
events on multiple network connected devices.

Changelog:

* The PTP portion of the patch set was posted 7/8/2015 (v3)
and rejected because of there wasn't a driver that
implemented the new API. Now, the driver patch is added
and the PTP patch operation is modified to revert to
previous behavior when cross timestamp can't be
completed. This is indicated by the driver returning a
non-zero value.

* v2 re-submit based on tglx provided correlated clocksource patch. This has
been included verbatim as the first patch in the series. Additions and
modifications are in the second patch. The PTP driver patch is unchanged
and the e1000e driver patch uses the *new* correlated clocksource interface
but is otherwise (in terms of hardware and PTP driver) unchanged.

* ART is removed as a compile option

* ART is added as an X86_FEATURE

Christopher Hall (4):
Add generic correlated clocksource code and ART to TSC conversion code
Add ART initialization code
Add support for driver cross-timestamp to PTP_SYS_OFFSET ioctl
Added getsynctime64() callback

Documentation/ptp/testptp.c | 6 +-
arch/x86/include/asm/cpufeature.h | 3 +-
arch/x86/include/asm/tsc.h | 1 +
arch/x86/kernel/tsc.c | 45 +++++++++++++++
drivers/net/ethernet/intel/e1000e/defines.h | 7 +++
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 | 32 +++++++++++
include/linux/ptp_clock_kernel.h | 7 +++
include/linux/timekeeping.h | 4 ++
include/uapi/linux/ptp_clock.h | 4 +-
kernel/time/timekeeping.c | 64 +++++++++++++++++++++
13 files changed, 282 insertions(+), 12 deletions(-)

--
1.9.1


2015-08-07 23:02:18

by Hall, Christopher S

[permalink] [raw]
Subject: [PATCH v2 1/4] Add generic correlated clocksource code and ART to TSC conversion code

Original patch description:

Subject: ptp: Get sync timestamps
From: Thomas Gleixner <[email protected]>
Date: Wed, 29 Jul 2015 10:52:06 +0200

The ART stuff wants to be splitted out.

======== Changes =======

Add struct correlated_cs (clocksource) with pointer to original clocksource
and function pointer to convert correlated clocksource to the original

Add struct correlated_ts (timestamp) with function pointer to read correlated
clocksource, device and system (in terms of correlated clocksource)
counter values (input) with resulting converted real and monotonic raw
system times (output)

Add get_correlated_timestamp() function which given specific correlated_cs
and correlated_ts convert correlated counter value to system time

Add art_to_tsc conversion function translated Always Running Timer (ART) to
TSC value
---
arch/x86/kernel/tsc.c | 31 ++++++++++++++++++++++
include/linux/clocksource.h | 30 +++++++++++++++++++++
include/linux/timekeeping.h | 4 +++
kernel/time/timekeeping.c | 63 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 128 insertions(+)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 7437b41..a90aa6a 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1059,6 +1059,27 @@ int unsynchronized_tsc(void)
return 0;
}

+static u32 tsc_numerator;
+static u32 tsc_denominator;
+/*
+ * CHECKME: Do we need the adjust value? It should be 0, but if we run
+ * in a VM this might be a different story.
+ */
+static u64 tsc_adjust;
+
+static u64 art_to_tsc(u64 cycles)
+{
+ u64 tmp, res = tsc_adjust;
+
+ res += (cycles / tsc_denominator) * tsc_numerator;
+ tmp = (cycles % tsc_denominator) * tsc_numerator;
+ res += tmp / tsc_denominator;
+ return res;
+}
+
+struct correlated_cs art_timestamper = {
+ .convert = art_to_tsc,
+};

static void tsc_refine_calibration_work(struct work_struct *work);
static DECLARE_DELAYED_WORK(tsc_irqwork, tsc_refine_calibration_work);
@@ -1129,6 +1150,16 @@ static void tsc_refine_calibration_work(struct work_struct *work)
(unsigned long)tsc_khz / 1000,
(unsigned long)tsc_khz % 1000);

+ /*
+ * TODO:
+ *
+ * If the system has ART, initialize the art_to_tsc conversion
+ * and set: art_timestamp.related_cs = &tsc_clocksource.
+ *
+ * Before that point a call to get_correlated_timestamp will
+ * fail the clocksource match check.
+ */
+
out:
clocksource_register_khz(&clocksource_tsc, tsc_khz);
}
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 278dd27..2ed3d0c 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -258,4 +258,34 @@ 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)(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;
+ u64 system_real;
+ u64 system_raw;
+};
#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..769a04b 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,56 @@ 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;
+}
+
+/**
* do_gettimeofday - Returns the time of day in a timeval
* @tv: pointer to the timeval to be set
*
--
1.9.1

2015-08-07 23:00:50

by Hall, Christopher S

[permalink] [raw]
Subject: [PATCH v2 2/4] Add ART initialization code

add private struct correlated_ts member used by get_ts() code

added EXPORTs making get_correlated_timestamp() function and
art_timestamper accessible

Add special case for denominator of 2 (art_to_tsc())
---
arch/x86/include/asm/cpufeature.h | 3 ++-
arch/x86/include/asm/tsc.h | 1 +
arch/x86/kernel/tsc.c | 42 ++++++++++++++++++++++++++-------------
include/linux/clocksource.h | 8 +++++---
kernel/time/timekeeping.c | 1 +
5 files changed, 37 insertions(+), 18 deletions(-)

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..0089991 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -53,6 +53,7 @@ extern int check_tsc_disabled(void);
extern unsigned long native_calibrate_tsc(void);

extern int tsc_clocksource_reliable;
+extern struct correlated_cs art_timestamper;

/*
* Boot-time check whether the TSCs are synchronized across
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index a90aa6a..0a2f336 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1059,6 +1059,9 @@ int unsynchronized_tsc(void)
return 0;
}

+#define ART_CPUID_LEAF (0x15)
+#define ART_MIN_DENOMINATOR (2)
+
static u32 tsc_numerator;
static u32 tsc_denominator;
/*
@@ -1067,19 +1070,29 @@ static u32 tsc_denominator;
*/
static u64 tsc_adjust;

-static u64 art_to_tsc(u64 cycles)
+static u64 art_to_tsc(struct correlated_cs *cs, u64 cycles)
{
u64 tmp, res = tsc_adjust;

- res += (cycles / tsc_denominator) * tsc_numerator;
- tmp = (cycles % tsc_denominator) * tsc_numerator;
- res += tmp / tsc_denominator;
+ switch (tsc_denominator) {
+ default:
+ res += (cycles / tsc_denominator) * tsc_numerator;
+ tmp = (cycles % tsc_denominator) * tsc_numerator;
+ res += tmp / tsc_denominator;
+ break;
+ case 2:
+ res += (cycles >> 1) * tsc_numerator;
+ tmp = (cycles & 0x1) * tsc_numerator;
+ res += tmp >> 1;
+ break;
+ }
return res;
}

struct correlated_cs art_timestamper = {
.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);
@@ -1103,6 +1116,7 @@ static void tsc_refine_calibration_work(struct work_struct *work)
static int hpet;
u64 tsc_stop, ref_stop, delta;
unsigned long freq;
+ unsigned int unused[2];

/* Don't bother refining TSC on unstable systems */
if (check_tsc_unstable())
@@ -1150,17 +1164,17 @@ static void tsc_refine_calibration_work(struct work_struct *work)
(unsigned long)tsc_khz / 1000,
(unsigned long)tsc_khz % 1000);

- /*
- * TODO:
- *
- * If the system has ART, initialize the art_to_tsc conversion
- * and set: art_timestamp.related_cs = &tsc_clocksource.
- *
- * Before that point a call to get_correlated_timestamp will
- * fail the clocksource match check.
- */
-
out:
+ if (boot_cpu_data.cpuid_level >= ART_CPUID_LEAF) {
+ cpuid(ART_CPUID_LEAF, &tsc_denominator, &tsc_numerator, unused,
+ unused+1);
+
+ if (tsc_denominator >= ART_MIN_DENOMINATOR) {
+ set_cpu_cap(&boot_cpu_data, X86_FEATURE_ART);
+ art_timestamper.related_cs = &clocksource_tsc;
+ }
+ }
+
clocksource_register_khz(&clocksource_tsc, tsc_khz);
}

diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 2ed3d0c..46ce54b 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -267,7 +267,8 @@ static inline void acpi_generic_timer_init(void) { }
*/
struct correlated_cs {
struct clocksource *related_cs;
- u64 (*convert)(u64 cycles);
+ u64 (*convert)(struct correlated_cs *cs,
+ u64 cycles);
};

struct correlated_ts;
@@ -285,7 +286,8 @@ struct correlated_ts {
int (*get_ts)(struct correlated_ts *ts);
u64 system_ts;
u64 device_ts;
- u64 system_real;
- u64 system_raw;
+ ktime_t system_real;
+ ktime_t system_raw;
+ void *private;
};
#endif /* _LINUX_CLOCKSOURCE_H */
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 769a04b..abf8aec 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -946,6 +946,7 @@ int get_correlated_timestamp(struct correlated_ts *crt,
} 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
--
1.9.1

2015-08-07 23:00:51

by Hall, Christopher S

[permalink] [raw]
Subject: [PATCH v2 3/4] Add support for driver cross-timestamp to PTP_SYS_OFFSET ioctl

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
---
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 {
--
1.9.1

2015-08-07 23:01:37

by Hall, Christopher S

[permalink] [raw]
Subject: [PATCH v2 4/4] Added getsynctime64() callback

Reads ART (TSC correlated clocksource), converts to realtime clock, and
reports cross timestamp to PTP driver
---
drivers/net/ethernet/intel/e1000e/defines.h | 7 +++
drivers/net/ethernet/intel/e1000e/ptp.c | 88 +++++++++++++++++++++++++++++
drivers/net/ethernet/intel/e1000e/regs.h | 4 ++
3 files changed, 99 insertions(+)

diff --git a/drivers/net/ethernet/intel/e1000e/defines.h b/drivers/net/ethernet/intel/e1000e/defines.h
index 133d407..9f16269 100644
--- a/drivers/net/ethernet/intel/e1000e/defines.h
+++ b/drivers/net/ethernet/intel/e1000e/defines.h
@@ -527,6 +527,13 @@
#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..c3d80c4 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,91 @@ static int e1000e_phc_adjtime(struct ptp_clock_info *ptp, s64 delta)
return 0;
}

+#define HW_WAIT_COUNT (2)
+#define HW_RETRY_COUNT (2)
+
+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, j;
+ u32 tsync_ctrl;
+ int ret;
+
+ if (hw->mac.type < e1000_pch_spt)
+ return -EOPNOTSUPP;
+
+ for (j = 0; j < HW_RETRY_COUNT; ++j) {
+ tsync_ctrl = er32(TSYNCTXCTL);
+ tsync_ctrl |= E1000_TSYNCTXCTL_START_SYNC |
+ E1000_TSYNCTXCTL_MAX_ALLOWED_DLY_MASK;
+ ew32(TSYNCTXCTL, tsync_ctrl);
+ ret = 0;
+ for (i = 0; i < HW_WAIT_COUNT; ++i) {
+ udelay(2);
+ tsync_ctrl = er32(TSYNCTXCTL);
+ if (tsync_ctrl & E1000_TSYNCTXCTL_SYNC_COMP)
+ break;
+ }
+
+ if (i == HW_WAIT_COUNT) {
+ ret = -ETIMEDOUT;
+ } else if (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);
+ break;
+ }
+ }
+
+ return ret;
+}
+
+#define SYNCTIME_RETRY_COUNT (2)
+
+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 i, ret;
+
+ if (!cpu_has_art)
+ return -EOPNOTSUPP;
+
+ for (i = 0; i < SYNCTIME_RETRY_COUNT; ++i) {
+ 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)
+ continue;
+
+ 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;
+ break;
+ }
+
+ return ret;
+}
+
/**
* e1000e_phc_gettime - Reads the current time from the hardware clock
* @ptp: ptp clock structure
@@ -190,6 +277,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,
};
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 */

--
1.9.1

2015-08-07 23:44:41

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] Add generic correlated clocksource code and ART to TSC conversion code

On 08/07/2015 04:01 PM, Christopher Hall wrote:
> Original patch description:
>
> Subject: ptp: Get sync timestamps
> From: Thomas Gleixner <[email protected]>
> Date: Wed, 29 Jul 2015 10:52:06 +0200
>
> The ART stuff wants to be splitted out.
>
> ======== Changes =======
>
> Add struct correlated_cs (clocksource) with pointer to original clocksource
> and function pointer to convert correlated clocksource to the original
>
> Add struct correlated_ts (timestamp) with function pointer to read correlated
> clocksource, device and system (in terms of correlated clocksource)
> counter values (input) with resulting converted real and monotonic raw
> system times (output)
>
> Add get_correlated_timestamp() function which given specific correlated_cs
> and correlated_ts convert correlated counter value to system time
>
> Add art_to_tsc conversion function translated Always Running Timer (ART) to
> TSC value
> ---
> arch/x86/kernel/tsc.c | 31 ++++++++++++++++++++++
> include/linux/clocksource.h | 30 +++++++++++++++++++++
> include/linux/timekeeping.h | 4 +++
> kernel/time/timekeeping.c | 63 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 128 insertions(+)
>
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 7437b41..a90aa6a 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -1059,6 +1059,27 @@ int unsynchronized_tsc(void)
> return 0;
> }
>
> +static u32 tsc_numerator;
> +static u32 tsc_denominator;
> +/*
> + * CHECKME: Do we need the adjust value? It should be 0, but if we run
> + * in a VM this might be a different story.
> + */
> +static u64 tsc_adjust;
> +
> +static u64 art_to_tsc(u64 cycles)
> +{
> + u64 tmp, res = tsc_adjust;
> +
> + res += (cycles / tsc_denominator) * tsc_numerator;
> + tmp = (cycles % tsc_denominator) * tsc_numerator;
> + res += tmp / tsc_denominator;
> + return res;

Nice trick!

> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> index 278dd27..2ed3d0c 100644
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -258,4 +258,34 @@ 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)(u64 cycles);

Should the name make it clearer which way it converts? For example,
convert_to_related? We might also want convert_from_related.

--Andy

2015-08-10 08:49:56

by Richard Cochran

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] Added getsynctime64() callback

On Fri, Aug 07, 2015 at 04:01:35PM -0700, Christopher Hall wrote:
> --- a/drivers/net/ethernet/intel/e1000e/defines.h
> +++ b/drivers/net/ethernet/intel/e1000e/defines.h
> @@ -527,6 +527,13 @@
> #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
> + */

Split comment looks bad. Trim this leading space instead. ^^^^^^

> @@ -98,6 +100,91 @@ static int e1000e_phc_adjtime(struct ptp_clock_info *ptp, s64 delta)
> return 0;
> }
>
> +#define HW_WAIT_COUNT (2)
> +#define HW_RETRY_COUNT (2)

A busy wait, plus a retry, ...

> +#define SYNCTIME_RETRY_COUNT (2)

plus another retry!

Seems a bit heavy handed to me. Is the HW really that flakey?

I would expect that a reasonably long polling loop should be
sufficient. If not, then the HW ignores certain requests, and that is
worth a comment.

In any case, I don't understand why you have two nested retry loops.

> +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 i, ret;
> +
> + if (!cpu_has_art)
> + return -EOPNOTSUPP;

Perform this check before registration, setting .getsynctime64
accordingly.

Thanks,
Richard

2015-08-10 08:53:18

by Richard Cochran

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] Add support for driver cross-timestamp to PTP_SYS_OFFSET ioctl

On Fri, Aug 07, 2015 at 04:01:34PM -0700, Christopher Hall wrote:
> 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.

Acked-by: Richard Cochran <[email protected]>

2015-08-13 21:10:40

by Hall, Christopher S

[permalink] [raw]
Subject: RE: [PATCH v2 4/4] Added getsynctime64() callback



> -----Original Message-----
> From: Richard Cochran [mailto:[email protected]]
> Sent: Monday, August 10, 2015 1:50 AM
> To: Hall, Christopher S
> Cc: [email protected]; [email protected]; [email protected]; Kirsher,
> Jeffrey T; Ronciak, John; [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 4/4] Added getsynctime64() callback
>
> On Fri, Aug 07, 2015 at 04:01:35PM -0700, Christopher Hall wrote:
> > --- a/drivers/net/ethernet/intel/e1000e/defines.h
> > +++ b/drivers/net/ethernet/intel/e1000e/defines.h
> > @@ -527,6 +527,13 @@
> > #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
> > + */
>
> Split comment looks bad. Trim this leading space instead. ^^^^^^

OK.

>
> > @@ -98,6 +100,91 @@ static int e1000e_phc_adjtime(struct ptp_clock_info
> *ptp, s64 delta)
> > return 0;
> > }
> >
> > +#define HW_WAIT_COUNT (2)
> > +#define HW_RETRY_COUNT (2)
>
> A busy wait, plus a retry, ...
>
> > +#define SYNCTIME_RETRY_COUNT (2)
>
> plus another retry!
>
> Seems a bit heavy handed to me. Is the HW really that flakey?
>
> I would expect that a reasonably long polling loop should be
> sufficient. If not, then the HW ignores certain requests, and that is
> worth a comment.
>
> In any case, I don't understand why you have two nested retry loops.

The retry in get_synctime() is a left over from the previous patch. It's not necessary,
the current timekeeping code won't fail in a way necessitating a retry. It's removed.

The inner retry loop is due to huge inaccuracies in udelay(). I've done some testing
and it appears udelay(2) actually results in about an 8 microsecond delay. On
Skylake the time for completion of the cross timestamp should be about 2 microseconds.
If we eliminate the inner most loop we either spin for too long or possibly risk not
waiting long enough. Are there any guarantees for udelay()?

As for HW_RETRY_LOOP, I will confirm whether this is necessary. It was in reference
code I was given, but, I agree, it seems odd.

>
> > +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 i, ret;
> > +
> > + if (!cpu_has_art)
> > + return -EOPNOTSUPP;
>
> Perform this check before registration, setting .getsynctime64
> accordingly.

The problem here is that ART initialization doesn't happen until we install TSC as a clocksource. This design is per Thomas' suggestion. That occurs after the driver is loaded (as a module).

In my somewhat limited testing, it's about 400 ms later. The problem is the several seconds of TSC frequency refinement. I, in principle, agree, but we either need to move the ART initialization earlier (probably split it) or defer PTP clock initialization in the driver.

>
> Thanks,
> Richard

2015-08-14 06:32:58

by Richard Cochran

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] Added getsynctime64() callback

On Thu, Aug 13, 2015 at 09:10:36PM +0000, Hall, Christopher S wrote:
> > > + if (!cpu_has_art)
> > > + return -EOPNOTSUPP;
> >
> > Perform this check before registration, setting .getsynctime64
> > accordingly.
>
> The problem here is that ART initialization doesn't happen until we
> install TSC as a clocksource. This design is per Thomas'
> suggestion. That occurs after the driver is loaded (as a module).

So that 'cpu_has_art' actually means 'cpu_has_art_and_has_been_initialized'?

In any case, returning EOPNOTSUPP early on, but OK later seems mean to
me. If the clocks aren't ready yet, the error should be EBUSY so that
user space knows it can try again.

Thanks,
Richard