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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/04/14 12:53, Jan Beulich wrote:
>>>> On 07.04.14 at 12:23, <andrew.cooper3@xxxxxxxxxx> 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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel