linux-kernel - Re: [PATCH] x86/MCE/AMD: Clear DFR errors found in THR handler

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: <Y1pvkIJ/Uipxolqy@zn.tnic>
Date:   Thu, 27 Oct 2022 13:46:24 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Yazen Ghannam <yazen.ghannam@....com>
Cc:     linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org,
        tony.luck@...el.com, x86@...nel.org,
        Smita.KoralahalliChannabasappa@....com
Subject: Re: [PATCH] x86/MCE/AMD: Clear DFR errors found in THR handler

On Tue, Jun 21, 2022 at 03:59:43PM +0000, Yazen Ghannam wrote:
> AMD's MCA Thresholding feature counts errors of all severites not just
> correctable errors. If a deferred error causes the threshold limit to be
> reached (it was the error that caused the overflow), then both a
> deferred error interrupt and a thresholding interrupt will be triggered.
> 
> The order of the interrupts is not guaranteed. If the threshold
> interrupt handler is executed first, then it will clear MCA_STATUS for
> the error. It will not check or clear MCA_DESTAT which also holds a copy
> of the deferred error. When the deferred error interrupt handler runs it
> will not find an error in MCA_STATUS, but it will find the error in
> MCA_DESTAT. This will cause two errors to be logged.
> 
> Check for deferred errors when handling a threshold interrupt. If a bank
> contains a deferred error, then clear the bank's MCA_DESTAT register.
> 
> Define a new helper function to do the deferred error check and clearing
> of MCA_DESTAT.
> 
> Fixes: 37d43acfd79f ("x86/mce/AMD: Redo error logging from APIC LVT interrupt handlers")
> Signed-off-by: Yazen Ghannam <yazen.ghannam@....com>
> Cc: stable@...r.kernel.org
> ---
>  arch/x86/kernel/cpu/mce/amd.c | 37 +++++++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
> index 1c87501e0fa3..ab1145cf8328 100644
> --- a/arch/x86/kernel/cpu/mce/amd.c
> +++ b/arch/x86/kernel/cpu/mce/amd.c
> @@ -788,6 +788,28 @@ _log_error_bank(unsigned int bank, u32 msr_stat, u32 msr_addr, u64 misc)
>  	return status & MCI_STATUS_DEFERRED;
>  }
>  
> +static bool _log_error_deferred(unsigned int bank, u32 misc)
> +{
> +	bool defrd;
> +
> +	defrd = _log_error_bank(bank, mca_msr_reg(bank, MCA_STATUS),
> +				mca_msr_reg(bank, MCA_ADDR), misc);
> +
> +	if (!defrd)
> +		return false;

I've zapped that defrd variable:

diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
index ab1145cf8328..6ae7edea3270 100644
--- a/arch/x86/kernel/cpu/mce/amd.c
+++ b/arch/x86/kernel/cpu/mce/amd.c
@@ -790,12 +790,8 @@ _log_error_bank(unsigned int bank, u32 msr_stat, u32 msr_addr, u64 misc)
 
 static bool _log_error_deferred(unsigned int bank, u32 misc)
 {
-	bool defrd;
-
-	defrd = _log_error_bank(bank, mca_msr_reg(bank, MCA_STATUS),
-				mca_msr_reg(bank, MCA_ADDR), misc);
-
-	if (!defrd)
+	if (!_log_error_bank(bank, mca_msr_reg(bank, MCA_STATUS),
+			     mca_msr_reg(bank, MCA_ADDR), misc))
 		return false;
 
 	/*

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette