On 08.02.2022 09:54, Roger Pau Monné wrote:
> On Fri, Feb 04, 2022 at 02:56:43PM +0100, Jan Beulich wrote:
>> Models 0F and 17 don't have PLATFORM_INFO documented. While it exists on
>> at least model 0F, the information there doesn't match the scheme used
>> on newer models (I'm observing a range of 700 ... 600 MHz reported on a
>> Xeon E5345).
>
> Maybe it would be best to limit ourselves to the models that have the
> MSR documented in the SDM?
Well, yes, that's what I wasn't sure about: The information is used only
for logging, so it's not the end of the world if we display something
strange. We'd want to address such anomalies (like the one I did observe
here) of course. But I wonder whether being entirely silent is really
better.
>> --- a/xen/arch/x86/cpu/intel.c
>> +++ b/xen/arch/x86/cpu/intel.c
>> @@ -435,6 +435,26 @@ static void intel_log_freq(const struct
>> if ( c->x86 == 6 )
>> switch ( c->x86_model )
>> {
>> + static const unsigned short core_factors[] =
>> + { 26667, 13333, 20000, 16667, 33333, 10000, 40000 };
>> +
>> + case 0x0e: /* Core */
>> + case 0x0f: case 0x16: case 0x17: case 0x1d: /* Core2 */
>> + /*
>> + * PLATFORM_INFO, while not documented for these, appears to
>> + * exist in at least some cases, but what it holds doesn't
>> + * match the scheme used by newer CPUs. At a guess, the min
>> + * and max fields look to be reversed, while the scaling
>> + * factor is encoded in FSB_FREQ.
>> + */
>> + if ( min_ratio > max_ratio )
>> + SWAP(min_ratio, max_ratio);
>> + if ( rdmsr_safe(MSR_FSB_FREQ, msrval) ||
>> + (msrval &= 7) >= ARRAY_SIZE(core_factors) )
>> + return;
>> + factor = core_factors[msrval];
>> + break;
>> +
>> case 0x1a: case 0x1e: case 0x1f: case 0x2e: /* Nehalem */
>> case 0x25: case 0x2c: case 0x2f: /* Westmere */
>> factor = 13333;
>
> Seeing that the MSR is present on non documented models and has
> unknown behavior we might want to further sanity check that min < max
> before printing anything?
But I'm already swapping the two in the opposite case?
Jan
