Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1340191

Re: [PATCH v7 6/8] x86: tsc: Always Running Timer (ART) correlated clocksource

From Andy Lutomirski <luto@amacapital.net>
Newsgroups linux.kernel
Subject Re: [PATCH v7 6/8] x86: tsc: Always Running Timer (ART) correlated clocksource
Date 2016-02-23 03:50 +0100
Message-ID <r5bcZ-7aX-1@gated-at.bofh.it> (permalink)
References <r1zdT-7MB-3@gated-at.bofh.it> <r1zdU-7MB-17@gated-at.bofh.it> <r3E9r-38D-7@gated-at.bofh.it> <r5b3k-77q-5@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Mon, Feb 22, 2016 at 6:38 PM, Christopher Hall
<christopher.s.hall@intel.com> wrote:
> On Thu, 18 Feb 2016 13:11:33 -0800, Andy Lutomirski <luto@kernel.org> wrote:
>>>
>>> +#define X86_FEATURE_INVARIANT_TSC (7*32+4) /* Intel Invariant TSC */
>
>
> This is removed. It was basically an alias for NONSTOP_TSC and not needed.
>
>>
>>> +/*
>>> + * 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?
>
>
> I don't deal with the k != 0 case. I assume that IA32 TSC adjust MSR is 0
> because it's almost always a *bad idea* to change it. I've discussed this
> with a few other developers and there is some consensus agreeing. From an
> earlier related thread Peter Zijlstra asserts that TSC adjust "had
> better" be 0.(http://lkml.iu.edu/hypermail/linux/kernel/1507.3/03734.html).

I'm having trouble finding that in the link you sent.

>
> Do we really need to accommodate BIOS's that do this?

There are three interesting cases that I can think of:

1. Crappy BIOS that sets TSC_ADJUST.  As the not-so-proud owner of a
piece of crap motherboard that actively messes with the TSC, I don't
trust BIOS.

2. Hypervisors.  What if we're running as a guest with an ART-using
NIC passed through?

3. Hypothetical improved future kernel that politely uses TSC_ADJUST
to keep the TSC from going backwards across suspend/resume.

--Andy

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v7 0/8] Patchset enabling hardware based cross-timestamps for next gen Intel platforms "Christopher S. Hall" <christopher.s.hall@intel.com> - 2016-02-13 04:40 +0100
  [PATCH v7 8/8] net: e1000e: Adds hardware supported cross timestamp on e1000e nic "Christopher S. Hall" <christopher.s.hall@intel.com> - 2016-02-13 04:40 +0100
    Re: [PATCH v7 8/8] net: e1000e: Adds hardware supported cross  timestamp on e1000e nic Jeff Kirsher <jeffrey.t.kirsher@intel.com> - 2016-02-19 01:50 +0100
  [PATCH v7 4/8] time: Add driver cross timestamp interface for higher precision time synchronization "Christopher S. Hall" <christopher.s.hall@intel.com> - 2016-02-13 04:40 +0100
    Re: [PATCH v7 4/8] time: Add driver cross timestamp interface for  higher precision time synchronization Richard Cochran <richardcochran@gmail.com> - 2016-02-16 09:00 +0100
  [PATCH v7 6/8] x86: tsc: Always Running Timer (ART) correlated clocksource "Christopher S. Hall" <christopher.s.hall@intel.com> - 2016-02-13 04:40 +0100
    Re: [PATCH v7 6/8] x86: tsc: Always Running Timer (ART) correlated  clocksource Andy Lutomirski <luto@kernel.org> - 2016-02-18 22:20 +0100
      Re: [PATCH v7 6/8] x86: tsc: Always Running Timer (ART) correlated  clocksource "Christopher Hall" <christopher.s.hall@intel.com> - 2016-02-23 03:40 +0100
        Re: [PATCH v7 6/8] x86: tsc: Always Running Timer (ART) correlated clocksource Andy Lutomirski <luto@amacapital.net> - 2016-02-23 03:50 +0100
  [PATCH v7 2/8] time: Add timekeeping snapshot code capturing system time and counter "Christopher S. Hall" <christopher.s.hall@intel.com> - 2016-02-13 04:40 +0100
  [PATCH v7 7/8] ptp: Add PTP_SYS_OFFSET_PRECISE for driver crosstimestamping "Christopher S. Hall" <christopher.s.hall@intel.com> - 2016-02-13 04:40 +0100
  [PATCH v7 3/8] time: Remove duplicated code in ktime_get_raw_and_real() "Christopher S. Hall" <christopher.s.hall@intel.com> - 2016-02-13 04:40 +0100
    Re: [PATCH v7 3/8] time: Remove duplicated code in  ktime_get_raw_and_real() Richard Cochran <richardcochran@gmail.com> - 2016-02-16 09:00 +0100
      Re: [PATCH v7 3/8] time: Remove duplicated code in  ktime_get_raw_and_real() "Christopher Hall" <christopher.s.hall@intel.com> - 2016-02-16 19:30 +0100
  [PATCH v7 5/8] time: Add history to cross timestamp interface supporting slower devices "Christopher S. Hall" <christopher.s.hall@intel.com> - 2016-02-13 04:40 +0100
    Re: [PATCH v7 5/8] time: Add history to cross timestamp interface  supporting slower devices Richard Cochran <richardcochran@gmail.com> - 2016-02-18 23:20 +0100
      Re: [PATCH v7 5/8] time: Add history to cross timestamp interface  supporting slower devices John Stultz <john.stultz@linaro.org> - 2016-02-19 18:40 +0100
  Re: [PATCH v7 0/8] Patchset enabling hardware based cross-timestamps  for next gen Intel platforms John Stultz <john.stultz@linaro.org> - 2016-02-18 20:30 +0100
    Re: [PATCH v7 0/8] Patchset enabling hardware based cross-timestamps  for next gen Intel platforms "Christopher Hall" <christopher.s.hall@intel.com> - 2016-02-22 19:40 +0100
      Re: [PATCH v7 0/8] Patchset enabling hardware based cross-timestamps  for next gen Intel platforms John Stultz <john.stultz@linaro.org> - 2016-02-22 19:50 +0100

csiph-web