On Mon, Feb 22, 2016 at 6:38 PM, Christopher Hall
<christopher.s.h...@intel.com> wrote:
> On Thu, 18 Feb 2016 13:11:33 -0800, Andy Lutomirski <l...@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
