From mboxrd@z Thu Jan  1 00:00:00 1970
From: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
Subject: Re: [PATCH 2/2] mcheck,
 vmce: Allow vmce_amd_* functions to handle AMD thresolding MSRs
Date: Mon, 27 Jan 2014 16:46:01 -0600
Message-ID: <52E6E1A9.3030903@amd.com>
References: <1205617825-10042-1-git-send-email-aravind.gopalakrishnan@amd.com>
	<1205617825-10042-3-git-send-email-aravind.gopalakrishnan@amd.com>
	<52E6B0DA.8070708@oracle.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <52E6B0DA.8070708@oracle.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: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: jinsong.liu@intel.com, chegger@amazon.de, suravee.suthikulpanit@amd.com, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

On 1/27/2014 1:17 PM, Boris Ostrovsky wrote:
> On 03/15/2008 05:50 PM, Aravind Gopalakrishnan wrote:
>>
>> diff --git a/xen/arch/x86/cpu/mcheck/vmce.c 
>> b/xen/arch/x86/cpu/mcheck/vmce.c
>> index f6c35db..cb4fd12 100644
>> --- a/xen/arch/x86/cpu/mcheck/vmce.c
>> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
>> @@ -107,7 +107,7 @@ static int bank_mce_rdmsr(const struct vcpu *v, 
>> uint32_t msr, uint64_t *val)
>>         *val = 0;
>>   -    switch ( msr & (MSR_IA32_MC0_CTL | 3) )
>> +    switch ( msr & (MSR_IA32_MC0_CTL | (0x3 << 30) | 0x13))
>
> Which MSRs are going to be handled in the non-default cases? 
> MSR0000_040[0:3] and MSRC000_040[0:3]? The first four already have 
> explicit cases and I think it would be more readable if you explicitly 
> created case statements for the latter four and had a simple 
> 'switch(msr)'.
>
In non-default cases, we need to handle MSR0x40[0:7]. MSR0x40[0:3] is 
bank 0 and remaining ones are bank 1.
We can't do a simple switch(msr) since if we do that, MSR0x40[4:7] will 
fall to 'default' (which is incorrect)

> In fact, do MSRC000_040[0:3] even exist?
>
Nope. They don't..

we only allow MCE MSR's here anyway:
ret = mce_bank_msr(cur, msr) ? bank_mce_rdmsr(cur, msr, val) : 0;

'mce_bank_msr' returns 1 only if we see MSR0x40[0:7] or some 'special 
MSR's' which in AMD's case are the three thresholding regs 
MSR_F10_MC4_MISC1 to MSR_F10_MC4_MISC3

> (You may also want to adjust your clock --- your emails are being sent 
> from distant past ;-) )
>
>
Yes, thanks for pointing that out :)
I'll correct that and send V2

-Aravind.