Re: [PATCH v2 2/3] x86/AMD: support further feature masking MSRs

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

On 07/04/14 12:53, Jan Beulich wrote:
>>>> On 07.04.14 at 12:23, <andrew.cooper3@citrix.com> wrote:
>> On 07/04/14 10:43, Jan Beulich wrote:
>>>  	/* AMD processors prior to family 10h required a 32-bit password */
>>>  	if (c->x86 >= 0x10) {
>>>  		wrmsr(MSR_K8_FEATURE_MASK, feat_edx, feat_ecx);
>>>  		wrmsr(MSR_K8_EXT_FEATURE_MASK, extfeat_edx, extfeat_ecx);
>>> +		if (!skip_l7s0_eax_ebx)
>>> +			wrmsr(MSR_AMD_L7S0_FEATURE_MASK, l7s0_ebx, l7s0_eax);
>>> +		if (!skip_thermal_ecx) {
>>> +			rdmsr(MSR_AMD_THRM_FEATURE_MASK, eax, edx);
>>> +			wrmsr(MSR_AMD_THRM_FEATURE_MASK, thermal_ecx, edx);
>>> +		}
>>>  	} else {
>>>  		wrmsr_amd(MSR_K8_FEATURE_MASK, feat_edx, feat_ecx);
>>>  		wrmsr_amd(MSR_K8_EXT_FEATURE_MASK, extfeat_edx, extfeat_ecx);
>> While editing this, can we remove this crazy split between wrmsr and
>> wrmsr_amd ?  It is safe to use wrmsr_amd in all cases where wrmsr is needed.
> I'm against this - the way it is now makes it very explicit where the
> extra input is required.
>
> Jan
>

It bloats the function, which would be less bad if it were an __init
function.

A comment can perfectly easily say /* Password required for fam 10h and
older */

~Andrew