From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v2 2/3] x86/AMD: support further feature
 masking MSRs
Date: Mon, 7 Apr 2014 13:09:08 +0100
Message-ID: <53429564.70102@citrix.com>
References: <53428DCC020000780000607D@nat28.tlf.novell.com>
	<53428F4402000078000060BB@nat28.tlf.novell.com>
	<53427C99.8010905@citrix.com>
	<5342ADEE0200007800006219@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta4.messagelabs.com ([85.158.143.247])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <Andrew.Cooper3@citrix.com>) id 1WX8MT-0006OR-8x
	for xen-devel@lists.xenproject.org; Mon, 07 Apr 2014 12:09:13 +0000
In-Reply-To: <5342ADEE0200007800006219@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>, Keir Fraser <keir@xen.org>, Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>, suravee.suthikulpanit@amd.com
List-Id: xen-devel@lists.xenproject.org

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