public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
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: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-12 20:25 [PATCH v7 0/8] Patchset enabling hardware based cross-timestamps for next gen Intel platforms Christopher S. Hall
2016-02-12 20:25 ` [PATCH v7 1/8] time: Add cycles to nanoseconds translation Christopher S. Hall
2016-02-12 20:25 ` [PATCH v7 2/8] time: Add timekeeping snapshot code capturing system time and counter Christopher S. Hall
2016-02-12 20:25 ` [PATCH v7 3/8] time: Remove duplicated code in ktime_get_raw_and_real() Christopher S. Hall
2016-02-16  7:52   ` Richard Cochran
2016-02-16 18:23     ` Christopher Hall
2016-02-12 20:25 ` [PATCH v7 4/8] time: Add driver cross timestamp interface for higher precision time synchronization Christopher S. Hall
2016-02-16  7:56   ` Richard Cochran
2016-02-12 20:25 ` [PATCH v7 5/8] time: Add history to cross timestamp interface supporting slower devices Christopher S. Hall
2016-02-18 22:17   ` Richard Cochran
2016-02-19 17:33     ` John Stultz
2016-02-12 20:25 ` [PATCH v7 6/8] x86: tsc: Always Running Timer (ART) correlated clocksource Christopher S. Hall
2016-02-18 21:11   ` Andy Lutomirski [this message]
2016-02-23  2:38     ` Christopher Hall
2016-02-23  2:49       ` Andy Lutomirski
2016-02-12 20:25 ` [PATCH v7 7/8] ptp: Add PTP_SYS_OFFSET_PRECISE for driver crosstimestamping Christopher S. Hall
2016-02-12 20:25 ` [PATCH v7 8/8] net: e1000e: Adds hardware supported cross timestamp on e1000e nic Christopher S. Hall
2016-02-19  0:43   ` Jeff Kirsher
2016-02-18 19:26 ` [PATCH v7 0/8] Patchset enabling hardware based cross-timestamps for next gen Intel platforms John Stultz
2016-02-22 18:33   ` Christopher Hall
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$(echo .)org \
    --cc=christopher.s.hall@intel$(echo .)com \
    --cc=hpa@zytor$(echo .)com \
    --cc=intel-wired-lan@lists$(echo .)osuosl.org \
    --cc=jeffrey.t.kirsher@intel$(echo .)com \
    --cc=john.stultz@linaro$(echo .)org \
    --cc=kevin.b.stanton@intel$(echo .)com \
    --cc=kevin.j.clarke@intel$(echo .)com \
    --cc=linux-kernel@vger$(echo .)kernel.org \
    --cc=mingo@redhat$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=richardcochran@gmail$(echo .)com \
    --cc=tglx@linutronix$(echo .)de \
    --cc=x86@kernel$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox