Re: [PATCH v2 3/3] x86/mce/amd: Reset deferred-register state between polling banks

[Yazen Ghannam]

On Thu, Sep 03, 2026 at 10:39:10PM +0000, OptoCloud wrote:
> Thanks for the review, here's v2.
> 
> machine_check_poll() reuses a single struct mce_hw_err while
> iterating over the MCA banks. smca_should_log_poll_error() sets
> MCE_CHECK_DFR_REGS in m->kflags when an error was taken from
> MCA_DESTAT rather than MCA_STATUS. Nothing clears the bit again for
> the rest of the poll, including on iterations that bail out early,
> so every later bank in the same pass inherits it.
> 
> A stale MCE_CHECK_DFR_REGS has two effects on a later bank:
> 
>   - mce_read_aux() reads MCx_DEADDR instead of MCA_ADDR, so the
>     address reported for that bank is wrong.
> 
>   - amd_clear_bank() returns before writing 0 to MCA_STATUS, so the
>     bank is logged again on the next poll.
> 
> Reset m->kflags at the start of each iteration, alongside the other
> bank-local resets, rather than clearing MCE_CHECK_DFR_REGS alone.
> The only other place kflags is set to zero is once, before the loop
> starts (mce_prep_record()'s initial memset), so there's no per-loop
> state a full reset would wipe out. It also covers any future kflags
> bit that ends up set within the loop the same way MCE_CHECK_DFR_REGS
> does today.
> 
> Found by code inspection; not reproduced on hardware.
> 
> Fixes: 7cb735d7c0cb ("x86/mce: Unify AMD DFR handler with MCA Polling")
> Signed-off-by: Eirik Bøe <git@xxxxxxxxxxxx>
> ---
> Changes since v1:
> - Dropped Cc: stable (Yazen)
> - Reset the whole m->kflags field instead of just MCE_CHECK_DFR_REGS
>   (Yazen)
> 
>  arch/x86/kernel/cpu/mce/core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
> index 6fa15e19988e..16ac2418fd51 100644
> --- a/arch/x86/kernel/cpu/mce/core.c
> +++ b/arch/x86/kernel/cpu/mce/core.c
> @@ -823,6 +823,7 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
>  			continue;
> 
>  		mce_clear_hw_err_fields(&err);
> +		m->kflags = 0;
>  		m->bank = i;
> 

Why not include this in the helper function? In fact, this patch could
be merged with the other.

Thanks,
Yazen