Re: [PATCH v2 18/30] x86/cpu: Rework AMD masking MSR setup

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 17/02/16 07:40, Jan Beulich wrote:
>
>> +	if ((rdmsr_amd_safe(msr, &lo, &hi) == 0) &&
>> +	    (wrmsr_amd_safe(msr, lo, hi) == 0))
>> +		levelling_caps |= caps;
>> +
>> +	*msr_val = ((uint64_t)hi << 32) | lo;
>> +}
> Why can't this function, currently returning void, simply return the
> value read?

Hmm - it can.  This current layout is an artefact of several changes in
design.

>> +static void __init noinline amd_init_levelling(void)
>>  {
>> -	static unsigned int feat_ecx, feat_edx;
>> -	static unsigned int extfeat_ecx, extfeat_edx;
>> -	static unsigned int l7s0_eax, l7s0_ebx;
>> -	static unsigned int thermal_ecx;
>> -	static bool_t skip_feat, skip_extfeat;
>> -	static bool_t skip_l7s0_eax_ebx, skip_thermal_ecx;
>> -	static enum { not_parsed, no_mask, set_mask } status;
>> -	unsigned int eax, ebx, ecx, edx;
>> -
>> -	if (status == no_mask)
>> -		return;
>> +	const struct cpuidmask *m = NULL;
>>  
>> -	if (status == set_mask)
>> -		goto setmask;
>> +	probe_masking_msrs();
>>  
>> -	ASSERT((status == not_parsed) && (c == &boot_cpu_data));
>> -	status = no_mask;
>> +	if (*opt_famrev != '\0') {
>> +		m = get_cpuidmask(opt_famrev);
>>  
>> -	/* Fam11 doesn't support masking at all. */
>> -	if (c->x86 == 0x11)
>> -		return;
>> +		if (!m)
>> +			printk("Invalid processor string: %s\n", opt_famrev);
>> +	}
>>  
>> -	if (~(opt_cpuid_mask_ecx & opt_cpuid_mask_edx &
>> -	      opt_cpuid_mask_ext_ecx & opt_cpuid_mask_ext_edx &
>> -	      opt_cpuid_mask_l7s0_eax & opt_cpuid_mask_l7s0_ebx &
>> -	      opt_cpuid_mask_thermal_ecx)) {
>> -		feat_ecx = opt_cpuid_mask_ecx;
>> -		feat_edx = opt_cpuid_mask_edx;
>> -		extfeat_ecx = opt_cpuid_mask_ext_ecx;
>> -		extfeat_edx = opt_cpuid_mask_ext_edx;
>> -		l7s0_eax = opt_cpuid_mask_l7s0_eax;
>> -		l7s0_ebx = opt_cpuid_mask_l7s0_ebx;
>> -		thermal_ecx = opt_cpuid_mask_thermal_ecx;
>> -	} else if (*opt_famrev == '\0') {
>> -		return;
>> -	} else {
>> -		const struct cpuidmask *m = get_cpuidmask(opt_famrev);
>> +	if ((levelling_caps & LCAP_1cd) == LCAP_1cd) {
>> +		uint32_t ecx, edx, tmp;
>>  
>> -		if (!m) {
>> -			printk("Invalid processor string: %s\n", opt_famrev);
>> -			printk("CPUID will not be masked\n");
>> -			return;
>> +		cpuid(0x00000001, &tmp, &tmp, &ecx, &edx);
> Didn't you collect raw CPUID output already?

This is now c_early_init(), which is ahead of populating c->x86_capability

~Andrew