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

Hi,

On 2/8/20 1:05 AM, Thomas Gleixner wrote:

Hans,

Hans de Goede hdegoede@redhat.com writes:

...

@@ -120,11 +180,23 @@ unsigned long cpu_khz_from_msr(void) rdmsr(MSR_FSB_FREQ, lo, hi); index = lo & freq_desc->mask;

• /* Map CPU reference clock freq ID(0-7) to CPU reference clock freq(KHz) */
• freq = freq_desc->freqs[index];
• /* TSC frequency = maximum resolved freq * maximum resolved bus ratio */
• res = freq * ratio;

• /*

• * Note this also catches cases where the index points to an unpopulated
  

• * part of muldiv, in that case the else will set freq and res to 0.
  

• */
  

• if (freq_desc->muldiv[index].divider) {

• freq = DIV_ROUND_CLOSEST(TSC_REFERENCE_KHZ *
  

• 			   freq_desc->muldiv[index].multiplier,
  

• 			 freq_desc->muldiv[index].divider);
  

• /* Multiply by ratio before the divide for better accuracy */
  

• res = DIV_ROUND_CLOSEST(TSC_REFERENCE_KHZ *
  

• 			   freq_desc->muldiv[index].multiplier *
  

• 			   ratio,
  

• 			freq_desc->muldiv[index].divider);
  

What about:

     struct muldiv *md = &freq_desc->muldiv[index];

     if (md->divider) {
tscref = TSC_REFERENCE_KHZ * md->multiplier;
     	freq = DIV_ROUND_CLOSEST(tscref, md->divider);
/*
              * Multiplying by ratio before the division has better
              * accuracy than just calculating freq * ratio
              */
             res = DIV_ROUND_CLOSEST(tscref * ratio, md->divider);

Hmm?

That indeed looks nicer, I've prepared (and tested) a v4 with the suggested change, I'll send out v4 right after this email.

Regards,

Hans