linux-kernel - Re: [PATCH 2/4] x86/mce/AMD: Fix logic to obtain block address

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: <20160223123907.GD3673@pd.tnic>
Date:	Tue, 23 Feb 2016 13:39:07 +0100
From:	Borislav Petkov <bp@...en8.de>
To:	Aravind Gopalakrishnan <Aravind.Gopalakrishnan@....com>
Cc:	tony.luck@...el.com, hpa@...or.com, mingo@...hat.com,
	tglx@...utronix.de, dougthompson@...ssion.com,
	mchehab@....samsung.com, x86@...nel.org,
	linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org,
	ashok.raj@...el.com, gong.chen@...ux.intel.com,
	len.brown@...el.com, peterz@...radead.org, ak@...ux.intel.com,
	alexander.shishkin@...ux.intel.com
Subject: Re: [PATCH 2/4] x86/mce/AMD: Fix logic to obtain block address

On Tue, Feb 16, 2016 at 03:45:09PM -0600, Aravind Gopalakrishnan wrote:
> In upcoming processors, the BLKPTR field is no longer used
> to indicate the MSR number of the additional register.
> Insted, it simply indicates the prescence of additional MSRs.
> 
> Fixing the logic here to gather MSR address from
> MSR_AMD64_SMCA_MCx_MISC() for newer processors
> and we fall back to existing logic for older processors.
> 
> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@....com>
> ---
>  arch/x86/include/asm/msr-index.h     |  4 ++
>  arch/x86/kernel/cpu/mcheck/mce_amd.c | 94 +++++++++++++++++++++++++-----------
>  2 files changed, 69 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 93bccbc..ca49e928e 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -265,10 +265,14 @@
>  #define MSR_IA32_MCx_CTL2(x)		(MSR_IA32_MC0_CTL2 + (x))
>  
>  /* 'SMCA': AMD64 Scalable MCA */
> +#define MSR_AMD64_SMCA_MC0_MISC0	0xc0002003
>  #define MSR_AMD64_SMCA_MC0_CONFIG	0xc0002004
>  #define MSR_AMD64_SMCA_MC0_IPID		0xc0002005
> +#define MSR_AMD64_SMCA_MC0_MISC1	0xc000200a
> +#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))
>  #define MSR_AMD64_SMCA_MCx_IPID(x)	(MSR_AMD64_SMCA_MC0_IPID + 0x10*(x))
> +#define MSR_AMD64_SMCA_MCx_MISCy(x, y)	((MSR_AMD64_SMCA_MC0_MISC1 + y) + (0x10*(x)))

Are those MSRs going to be used in multiple files? If not, they should
all go to mce.h.

>  #define MSR_P6_PERFCTR0			0x000000c1
>  #define MSR_P6_PERFCTR1			0x000000c2
> diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> index 8169103..4bdc836 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> @@ -286,6 +286,58 @@ static void deferred_error_interrupt_enable(struct cpuinfo_x86 *c)
>  	wrmsr(MSR_CU_DEF_ERR, low, high);
>  }
>  
> +static u32 get_block_address(u32 current_addr,
> +			     u32 low,
> +			     u32 high,
> +			     unsigned int bank,
> +			     unsigned int block)

Use arg formatting like the rest of functions in the file please.

> +{
> +	u32 addr = 0, offset = 0;
> +
> +	if (mce_flags.smca) {
> +		if (!block) {
> +			addr = MSR_AMD64_SMCA_MCx_MISC(bank);
> +		} else {
> +			/*
> +			 * For SMCA enabled processors, BLKPTR field
> +			 * of the first MISC register (MCx_MISC0) indicates
> +			 * presence of additional MISC register set (MISC1-4)
> +			 */
> +			u32 smca_low, smca_high;

s/smca_//

> +
> +			if (rdmsr_safe(MSR_AMD64_SMCA_MCx_CONFIG(bank),
> +				       &smca_low, &smca_high) ||
> +			    !(smca_low & MCI_CONFIG_MCAX))
> +				goto nextaddr_out;
> +
> +			if (!rdmsr_safe(MSR_AMD64_SMCA_MCx_MISC(bank),
> +					&smca_low, &smca_high) &&
> +			    (smca_low & MASK_BLKPTR_LO))
> +				addr = MSR_AMD64_SMCA_MCx_MISCy(bank,
> +								block - 1);

unnecessary line break.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.