All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH v7 6/8] x86: tsc: Always Running Timer (ART) correlated clocksource
Date: Thu, 18 Feb 2016 13:11:33 -0800	[thread overview]
Message-ID: <56C63385.9050703@kernel.org> (raw)
In-Reply-To: <1455308729-6280-7-git-send-email-christopher.s.hall@intel.com>

On 02/12/2016 12:25 PM, Christopher S. Hall wrote:
> On modern Intel systems TSC is derived from the new Always Running Timer
> (ART). ART can be captured simultaneous to the capture of
> audio and network device clocks, allowing a correlation between timebases
> to be constructed. Upon capture, the driver converts the captured ART
> value to the appropriate system clock using the correlated clocksource
> mechanism.
>
> On systems that support ART a new CPUID leaf (0x15) returns parameters
> ?m? and ?n? such that:
>
> TSC_value = (ART_value * m) / n + k [n >= 2]
>
> [k is an offset that can adjusted by a privileged agent. The
> IA32_TSC_ADJUST MSR is an example of an interface to adjust k.
> See 17.14.4 of the Intel SDM for more details]
>
> Signed-off-by: Christopher S. Hall <christopher.s.hall@intel.com>
> [jstultz: Tweaked to fix build issue, also reworked math for
> 64bit division on 32bit systems]
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>   arch/x86/include/asm/cpufeature.h |  3 ++-
>   arch/x86/include/asm/tsc.h        |  2 ++
>   arch/x86/kernel/cpu/scattered.c   |  1 +
>   arch/x86/kernel/tsc.c             | 50 +++++++++++++++++++++++++++++++++++++++
>   4 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 7ad8c94..111b892 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 */
> @@ -188,6 +188,7 @@
>
>   #define X86_FEATURE_CPB		( 7*32+ 2) /* AMD Core Performance Boost */
>   #define X86_FEATURE_EPB		( 7*32+ 3) /* IA32_ENERGY_PERF_BIAS support */
> +#define X86_FEATURE_INVARIANT_TSC (7*32+4) /* Intel Invariant TSC */

How is this related to the rest of the patch?

> +/*
> + * Convert ART to TSC given numerator/denominator found in detect_art()
> + */
> +struct system_counterval_t convert_art_to_tsc(cycle_t art)
> +{
> +	u64 tmp, res, rem;
> +
> +	rem = do_div(art, art_to_tsc_denominator);
> +
> +	res = art * art_to_tsc_numerator;
> +	tmp = rem * art_to_tsc_numerator;
> +
> +	do_div(tmp, art_to_tsc_denominator);
> +	res += tmp;
> +
> +	return (struct system_counterval_t) {.cs = art_related_clocksource,
> +			.cycles = res};

The SDM and the patch description both mention an offset "k".  Shouldn't 
this code at least have a comment about how it deals with the k != 0 case?

--Andy

WARNING: multiple messages have this Message-ID (diff)
From: Andy Lutomirski <luto@kernel.org>
To: "Christopher S. Hall" <christopher.s.hall@intel.com>,
	tglx@linutronix.de, richardcochran@gmail.com, mingo@redhat.com,
	john.stultz@linaro.org, hpa@zytor.com,
	jeffrey.t.kirsher@intel.com
Cc: x86@kernel.org, linux-kernel@vger.kernel.org,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	kevin.b.stanton@intel.com, kevin.j.clarke@intel.com
Subject: Re: [PATCH v7 6/8] x86: tsc: Always Running Timer (ART) correlated clocksource
Date: Thu, 18 Feb 2016 13:11:33 -0800	[thread overview]
Message-ID: <56C63385.9050703@kernel.org> (raw)
In-Reply-To: <1455308729-6280-7-git-send-email-christopher.s.hall@intel.com>

On 02/12/2016 12:25 PM, Christopher S. Hall wrote:
> On modern Intel systems TSC is derived from the new Always Running Timer
> (ART). ART can be captured simultaneous to the capture of
> audio and network device clocks, allowing a correlation between timebases
> to be constructed. Upon capture, the driver converts the captured ART
> value to the appropriate system clock using the correlated clocksource
> mechanism.
>
> On systems that support ART a new CPUID leaf (0x15) returns parameters
> “m” and “n” such that:
>
> TSC_value = (ART_value * m) / n + k [n >= 2]
>
> [k is an offset that can adjusted by a privileged agent. The
> IA32_TSC_ADJUST MSR is an example of an interface to adjust k.
> See 17.14.4 of the Intel SDM for more details]
>
> Signed-off-by: Christopher S. Hall <christopher.s.hall@intel.com>
> [jstultz: Tweaked to fix build issue, also reworked math for
> 64bit division on 32bit systems]
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>   arch/x86/include/asm/cpufeature.h |  3 ++-
>   arch/x86/include/asm/tsc.h        |  2 ++
>   arch/x86/kernel/cpu/scattered.c   |  1 +
>   arch/x86/kernel/tsc.c             | 50 +++++++++++++++++++++++++++++++++++++++
>   4 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 7ad8c94..111b892 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 */
> @@ -188,6 +188,7 @@
>
>   #define X86_FEATURE_CPB		( 7*32+ 2) /* AMD Core Performance Boost */
>   #define X86_FEATURE_EPB		( 7*32+ 3) /* IA32_ENERGY_PERF_BIAS support */
> +#define X86_FEATURE_INVARIANT_TSC (7*32+4) /* Intel Invariant TSC */

How is this related to the rest of the patch?

> +/*
> + * Convert ART to TSC given numerator/denominator found in detect_art()
> + */
> +struct system_counterval_t convert_art_to_tsc(cycle_t art)
> +{
> +	u64 tmp, res, rem;
> +
> +	rem = do_div(art, art_to_tsc_denominator);
> +
> +	res = art * art_to_tsc_numerator;
> +	tmp = rem * art_to_tsc_numerator;
> +
> +	do_div(tmp, art_to_tsc_denominator);
> +	res += tmp;
> +
> +	return (struct system_counterval_t) {.cs = art_related_clocksource,
> +			.cycles = res};

The SDM and the patch description both mention an offset "k".  Shouldn't 
this code at least have a comment about how it deals with the k != 0 case?

--Andy

  reply	other threads:[~2016-02-18 21:11 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-12 20:25 [Intel-wired-lan] [PATCH v7 0/8] Patchset enabling hardware based cross-timestamps for next gen Intel platforms Christopher S. Hall
2016-02-12 20:25 ` Christopher S. Hall
2016-02-12 20:25 ` [Intel-wired-lan] [PATCH v7 1/8] time: Add cycles to nanoseconds translation Christopher S. Hall
2016-02-12 20:25   ` Christopher S. Hall
2016-02-12 20:25 ` [Intel-wired-lan] [PATCH v7 2/8] time: Add timekeeping snapshot code capturing system time and counter Christopher S. Hall
2016-02-12 20:25   ` Christopher S. Hall
2016-02-12 20:25 ` [Intel-wired-lan] [PATCH v7 3/8] time: Remove duplicated code in ktime_get_raw_and_real() Christopher S. Hall
2016-02-12 20:25   ` Christopher S. Hall
2016-02-16  7:52   ` [Intel-wired-lan] " Richard Cochran
2016-02-16  7:52     ` Richard Cochran
2016-02-16 18:23     ` [Intel-wired-lan] " Christopher Hall
2016-02-16 18:23       ` Christopher Hall
2016-02-12 20:25 ` [Intel-wired-lan] [PATCH v7 4/8] time: Add driver cross timestamp interface for higher precision time synchronization Christopher S. Hall
2016-02-12 20:25   ` Christopher S. Hall
2016-02-16  7:56   ` [Intel-wired-lan] " Richard Cochran
2016-02-16  7:56     ` Richard Cochran
2016-02-12 20:25 ` [Intel-wired-lan] [PATCH v7 5/8] time: Add history to cross timestamp interface supporting slower devices Christopher S. Hall
2016-02-12 20:25   ` Christopher S. Hall
2016-02-18 22:17   ` [Intel-wired-lan] " Richard Cochran
2016-02-18 22:17     ` Richard Cochran
2016-02-19 17:33     ` [Intel-wired-lan] " John Stultz
2016-02-19 17:33       ` John Stultz
2016-02-12 20:25 ` [Intel-wired-lan] [PATCH v7 6/8] x86: tsc: Always Running Timer (ART) correlated clocksource Christopher S. Hall
2016-02-12 20:25   ` Christopher S. Hall
2016-02-18 21:11   ` Andy Lutomirski [this message]
2016-02-18 21:11     ` Andy Lutomirski
2016-02-23  2:38     ` [Intel-wired-lan] " Christopher Hall
2016-02-23  2:38       ` Christopher Hall
2016-02-23  2:49       ` [Intel-wired-lan] " Andy Lutomirski
2016-02-23  2:49         ` Andy Lutomirski
2016-02-12 20:25 ` [Intel-wired-lan] [PATCH v7 7/8] ptp: Add PTP_SYS_OFFSET_PRECISE for driver crosstimestamping Christopher S. Hall
2016-02-12 20:25   ` Christopher S. Hall
2016-02-12 20:25 ` [Intel-wired-lan] [PATCH v7 8/8] net: e1000e: Adds hardware supported cross timestamp on e1000e nic Christopher S. Hall
2016-02-12 20:25   ` Christopher S. Hall
2016-02-19  0:43   ` [Intel-wired-lan] " Jeff Kirsher
2016-02-19  0:43     ` Jeff Kirsher
2016-02-18 19:26 ` [Intel-wired-lan] [PATCH v7 0/8] Patchset enabling hardware based cross-timestamps for next gen Intel platforms John Stultz
2016-02-18 19:26   ` John Stultz
2016-02-22 18:33   ` [Intel-wired-lan] " Christopher Hall
2016-02-22 18:33     ` Christopher Hall
2016-02-22 18:49     ` [Intel-wired-lan] " John Stultz
2016-02-22 18:49       ` John Stultz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56C63385.9050703@kernel.org \
    --to=luto@kernel.org \
    --cc=intel-wired-lan@osuosl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.