Re: [PATCH v6 12/15] x86/mce/amd: Remove redundant reset_block()

[Nikolay Borisov <nik.borisov@suse.com>]

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@amd.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);
> +	}
>   }
>   
>   /*
>