linux-kernel - Re: [PATCH] x86/mce: Pay no attention to 'F' bit in MCACOD when parsing 'UC' errors.

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20130724155510.GA29756@naverao1-tp.watson.ibm.com>
Date:	Wed, 24 Jul 2013 21:25:10 +0530
From:	"Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
To:	"Luck, Tony" <tony.luck@...el.com>
Cc:	linux-kernel@...r.kernel.org, Borislav Petkov <bp@...e.de>,
	Chen Gong <gong.chen@...ux.intel.com>
Subject: Re: [PATCH] x86/mce: Pay no attention to 'F' bit in MCACOD when
 parsing 'UC' errors.

On 2013/07/23 01:34PM, Tony Luck wrote:
> The 0x1000 bit of the MCACOD field of machine check MCi_STATUS
> registers is only defined for corrected errors (where it means
> that hardware may be filtering errors see SDM section 15.9.2.1).
> 
> For uncorrected errors it may, or may not be set - so we should mask
> it out when checking for the architecturaly defined recoverable
> error signatures (see SDM 15.9.3.1 and 15.9.3.2)
> 
> While fixing this - I also noticed a bug introduced by
>   commit 33d7885b594e169256daef652e8d3527b2298e75
>   x86/mce: Update MCE severity condition check
> where we were including MCACOD bits in the check for the
> unaffected thread(s) during a machine check.

Good catch!

> 
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
>  arch/x86/include/asm/mce.h                | 3 ++-
>  arch/x86/kernel/cpu/mcheck/mce-severity.c | 8 ++++----
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
> index fa5f71e..a528f28 100644
> --- a/arch/x86/include/asm/mce.h
> +++ b/arch/x86/include/asm/mce.h
> @@ -33,10 +33,11 @@
>  #define MCI_STATUS_S	 (1ULL<<56)  /* Signaled machine check */
>  #define MCI_STATUS_AR	 (1ULL<<55)  /* Action required */
>  #define MCACOD		  0xffff     /* MCA Error Code */
> +#define MCACOD_UC	  0xefff     /* MCA Error Code - for UC errors */

How about just changing MCACOD to 0xefff? I don't think we ever care
about the 'F' bit, so we could simplify this by just changing MCACOD.

Regards,
Naveen

>  
>  /* Architecturally defined codes from SDM Vol. 3B Chapter 15 */
>  #define MCACOD_SCRUB	0x00C0	/* 0xC0-0xCF Memory Scrubbing */
> -#define MCACOD_SCRUBMSK	0xfff0
> +#define MCACOD_SCRUBMSK	0xeff0
>  #define MCACOD_L3WB	0x017A	/* L3 Explicit Writeback */
>  #define MCACOD_DATA	0x0134	/* Data Load */
>  #define MCACOD_INSTR	0x0150	/* Instruction Fetch */
> diff --git a/arch/x86/kernel/cpu/mcheck/mce-severity.c b/arch/x86/kernel/cpu/mcheck/mce-severity.c
> index e2703520..7f6ab4e 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce-severity.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce-severity.c
> @@ -111,17 +111,17 @@ static struct severity {
>  #ifdef	CONFIG_MEMORY_FAILURE
>  	MCESEV(
>  		KEEP, "Action required but unaffected thread is continuable",
> -		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR|MCACOD, MCI_UC_SAR|MCI_ADDR),
> +		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR, MCI_UC_SAR|MCI_ADDR),
>  		MCGMASK(MCG_STATUS_RIPV, MCG_STATUS_RIPV)
>  		),
>  	MCESEV(
>  		AR, "Action required: data load error in a user process",
> -		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR|MCACOD, MCI_UC_SAR|MCI_ADDR|MCACOD_DATA),
> +		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR|MCACOD_UC, MCI_UC_SAR|MCI_ADDR|MCACOD_DATA),
>  		USER
>  		),
>  	MCESEV(
>  		AR, "Action required: instruction fetch error in a user process",
> -		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR|MCACOD, MCI_UC_SAR|MCI_ADDR|MCACOD_INSTR),
> +		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR|MCACOD_UC, MCI_UC_SAR|MCI_ADDR|MCACOD_INSTR),
>  		USER
>  		),
>  #endif
> @@ -137,7 +137,7 @@ static struct severity {
>  		),
>  	MCESEV(
>  		AO, "Action optional: last level cache writeback error",
> -		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCACOD, MCI_UC_S|MCACOD_L3WB)
> +		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCACOD_UC, MCI_UC_S|MCACOD_L3WB)
>  		),
>  	MCESEV(
>  		SOME, "Action optional: unknown MCACOD",
> -- 
> 1.8.1.4
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/