linux-kernel - Re: [PATCH v6 12/15] x86/mce/amd: Remove redundant reset

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: <e85965a9-3aa5-4368-95bc-c46ab18a25fa@suse.com>
Date: Thu, 11 Sep 2025 17:42:39 +0300
From: Nikolay Borisov <nik.borisov@...e.com>
To: Yazen Ghannam <yazen.ghannam@....com>, x86@...nel.org,
 Tony Luck <tony.luck@...el.com>, "Rafael J. Wysocki" <rafael@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-edac@...r.kernel.org,
 Smita.KoralahalliChannabasappa@....com, Qiuxu Zhuo <qiuxu.zhuo@...el.com>,
 linux-acpi@...r.kernel.org
Subject: Re: [PATCH v6 12/15] x86/mce/amd: Remove redundant reset_block()



On 8.09.25 г. 18:40 ч., Yazen Ghannam wrote:
> Many of the checks in reset_block() are done again in the block reset
> function. So drop the redundant checks.
> 
> Signed-off-by: Yazen Ghannam <yazen.ghannam@....com>


> ---
> 
> Notes:
>      Link:
>      https://lore.kernel.org/r/20250825-wip-mca-updates-v5-17-865768a2eef8@amd.com
>      
>      v5->v6:
>      * No change.
>      
>      v4->v5:
>      * No change.
>      
>      v3->v4:
>      * New in v4.
> 
>   arch/x86/kernel/cpu/mce/amd.c | 28 +++++++---------------------
>   1 file changed, 7 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
> index 34268940c88a..9ca4079ff342 100644
> --- a/arch/x86/kernel/cpu/mce/amd.c
> +++ b/arch/x86/kernel/cpu/mce/amd.c
> @@ -812,29 +812,11 @@ static void amd_deferred_error_interrupt(void)
>   	machine_check_poll(MCP_TIMESTAMP, &this_cpu_ptr(&mce_amd_data)->dfr_intr_banks);
>   }
>   
> -static void reset_block(struct threshold_block *block)
> -{
> -	struct thresh_restart tr;
> -	u32 low = 0, high = 0;
> -
> -	if (!block)
> -		return;
> -
> -	if (rdmsr_safe(block->address, &low, &high))
> -		return;


This is being replaced by rdmsr, I guess it's safe because the fact we 
are processing a block which has been on the bank list means it's 
unlikely the rdmsr will fault.


> -
> -	if (!(high & MASK_OVERFLOW_HI))
> -		return;

nit: However, now, if mask overflow is not set a write to the msr will 
be performed, with the effect that IntType is going to be cleared (hi &= 
~MASK_INT_TYPE_HI; in threshold_restart_block), and MASK_COUNT_EN_HI 
will be set, that's different than the existing code, albeit it might be 
ok.
> -
> -	memset(&tr, 0, sizeof(tr));
> -	tr.b = block;
> -	threshold_restart_block(&tr);
> -}
> -
>   static void amd_reset_thr_limit(unsigned int bank)
>   {
>   	struct threshold_bank **bp = this_cpu_read(threshold_banks);
>   	struct threshold_block *block, *tmp;
> +	struct thresh_restart tr;
>   
>   	/*
>   	 * Validate that the threshold bank has been initialized already. The
> @@ -844,8 +826,12 @@ static void amd_reset_thr_limit(unsigned int bank)
>   	if (!bp || !bp[bank])
>   		return;
>   
> -	list_for_each_entry_safe(block, tmp, &bp[bank]->miscj, miscj)
> -		reset_block(block);
> +	memset(&tr, 0, sizeof(tr));
> +
> +	list_for_each_entry_safe(block, tmp, &bp[bank]->miscj, miscj) {
> +		tr.b = block;
> +		threshold_restart_block(&tr);
> +	}
>   }
>   
>   /*
>