linux-kernel - Re: [PATCH 5/5] x86/mcheck/AMD: Set MCAX Enable bit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56982706.502@amd.com>
Date:	Thu, 14 Jan 2016 16:53:58 -0600
From:	Aravind Gopalakrishnan <aravind.gopalakrishnan@....com>
To:	Borislav Petkov <bp@...en8.de>
CC:	<tony.luck@...el.com>, <tglx@...utronix.de>, <mingo@...hat.com>,
	<hpa@...or.com>, <x86@...nel.org>, <linux-edac@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 5/5] x86/mcheck/AMD: Set MCAX Enable bit

On 1/14/2016 4:46 PM, Borislav Petkov wrote:
> On Thu, Jan 14, 2016 at 04:05:40PM -0600, Aravind Gopalakrishnan wrote:
>>   
>> +/* SMCA defined MSR register set for AMD64 */
>> +#define MSR_AMD64_SMCA_MC0_CTL		0xc0002000
>> +#define MSR_AMD64_SMCA_MC0_STATUS	0xc0002001
>> +#define MSR_AMD64_SMCA_MC0_ADDR		0xc0002002
>> +#define MSR_AMD64_SMCA_MC0_MISC0	0xc0002003
>> +#define MSR_AMD64_SMCA_MC0_CONFIG	0xc0002004
>>
>> +
>> +#define MSR_AMD64_SMCA_MCx_CTL(x)	(MSR_AMD64_SMCA_MC0_CTL + 0x10*(x))
>> +#define MSR_AMD64_SMCA_MCx_STATUS(x)	(MSR_AMD64_SMCA_MC0_STATUS + 0x10*(x))
>> +#define MSR_AMD64_SMCA_MCx_ADDR(x)	(MSR_AMD64_SMCA_MC0_ADDR + 0x10*(x))
>> +#define MSR_AMD64_SMCA_MCx_MISC(x)	(MSR_AMD64_SMCA_MC0_MISC0 + 0x10*(x))
>> +#define MSR_AMD64_SMCA_MCx_CONFIG(x)	(MSR_AMD64_SMCA_MC0_CONFIG + 0x10*(x))
>>
> Please add MSRs only with the respective patch that uses them.
>
> AFAICT, you need to add only MSR_AMD64_SMCA_MCx_CONFIG() here.

Ok, Will fix this.

>>   
>>   /* SMCA settings */
>>   #define SMCA_THR_LVT_OFF	0xF000
>> +#define SMCA_MCAX_EN_OFF	0x1
> SMCA *and* MCAX.
>
> SMCA_EN_OFF is not enough?

Well McaX is name of the field in the MSR. I retained the "SMCA" prefix 
as these are all still part of the ScalableMCA changes.
I would prefer if "MCAX" is retained as it is indicative of which bit we 
are touching. So how about just MCAX_EN_OFF ?

>>   
>>
>> +
>> +				smca_high = (smca_high & ~SMCA_MCAX_EN_OFF) | 0x1;
> So this can simply be:
>
> 				smca_high |= SMCA_MCAX_EN_OFF;
>
> ?

Yes. Will fix this.

Thanks,
-Aravind.