Re: [PATCH 3/3] x86/tsc_msr: Make MSR derived TSC frequency more accurate

[Hans de Goede]

Hi,

On 30-01-2020 14:43, Peter Zijlstra wrote:
> On Thu, Jan 30, 2020 at 12:52:55PM +0100, Hans de Goede wrote:
>
> > This does not matter though, we can model the chain of PLLs as a single
> > PLL with a quotient equal to the quotients of all PLLs in the chain
> > multiplied.
> >
> > So we can create a simplified model of the CPU clock setup using a
> > reference clock of 100 MHz plus a quotient which gets us as close to the
> > frequency from the DSM as possible.
>
> s/DSM/SDM/ ?

Yes.

> > For the 83.3 MHz example from above this would give us 100 MHz * 5 / 6 =
> > 83 and 1/3 MHz, which matches exactly what has been measured on actual hw.
> >
> > This commit makes the tsc_msr.c code use a simplified PLL model with a
> > reference clock of 100 MHz for all Bay and Cherry Trail models.
>
>
> > + * Bay Trail SDM MSR_FSB_FREQ frequencies simplified PLL model:
> > + *  000:   100 *  5 /  6  =  83.3333 MHz
> > + *  001:   100 *  1 /  1  = 100.0000 MHz
> > + *  010:   100 *  4 /  3  = 133.3333 MHz
> > + *  011:   100 *  7 /  6  = 116.6667 MHz
> > + *  100:   100 *  4 /  5  =  80.0000 MHz
>
> > + * Cherry Trail SDM MSR_FSB_FREQ frequencies simplified PLL model:
> > + * 0000:   100 *  5 /  6  =  83.3333 MHz
> > + * 0001:   100 *  1 /  1  = 100.0000 MHz
> > + * 0010:   100 *  4 /  3  = 133.3333 MHz
> > + * 0011:   100 *  7 /  6  = 116.6667 MHz
> > + * 0100:   100 *  4 /  5  =  80.0000 MHz
> > + * 0101:   100 * 14 / 15  =  93.3333 MHz
> > + * 0110:   100 *  9 / 10  =  90.0000 MHz
> > + * 0111:   100 *  8 /  9  =  88.8889 MHz
> > + * 1000:   100 *  7 /  8  =  87.5000 MHz
>
> > + * Merriefield (BYT MID) SDM MSR_FSB_FREQ frequencies simplified PLL model:
> > + * 0001:   100 *  1 /  1  = 100.0000 MHz
> > + * 0010:   100 *  4 /  3  = 133.3333 MHz
>
> > + * Moorefield (CHT MID) SDM MSR_FSB_FREQ frequencies simplified PLL model:
> > + * 0000:   100 *  5 /  6  =  83.3333 MHz
> > + * 0001:   100 *  1 /  1  = 100.0000 MHz
> > + * 0010:   100 *  4 /  3  = 133.3333 MHz
> > + * 0011:   100 *  1 /  1  = 100.0000 MHz
>
> Unless I'm going cross-eyed, that's 4 times the exact same table.

Correct, except that the not listed values on the none Cherry Trail
table are undefined in the SDM, so we should probably deny them
(or as the old code was doing simply return 0).

And at least the Moorefield (CHT MID) table is different for 0011, that
is again 100 MHz like 0001 instead of 116.6667 as it is for BYT and CHT.

Note that the Merriefield (BYT MID) and Moorefield (CHT MID) values are
based on the old code I've not seen those values in the current latest
version of the SDM.

> Do we want to use the Cherry Trail table (for being the most complete)
> for all of them?

The old code had per model tables, and at least for BYT the SDM specifies
that only the lowest 3 bits of the MSR_FSB_FREQ register should be used,
the rest is marked as reserved (with no defined/default value given for them)
and on BYT only 000 - 100 are defined as being used, note 101 - 111 are
not marked as reserved, they are simply not documented for BYT.
_4_ bits should be used and there are frequency values defined for 0000 - 1000
again 1001 - 1111 are not marked as reserved, they are simply undocumented.

And for both MID variants I have no docs.

Regards,

Hans