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

[OptoCloud]

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;

 		barrier();
--
Cheers,
Eirik Bøe