diff mbox series

[v2,13/16] x86/mce: Unify AMD DFR handler with MCA Polling

Message ID 20250213-wip-mca-updates-v2-13-3636547fe05f@amd.com (mailing list archive)
State New
Headers show
Series AMD MCA interrupts rework | expand

Commit Message

Yazen Ghannam Feb. 13, 2025, 4:46 p.m. UTC
AMD systems optionally support a deferred error interrupt. The interrupt
should be used as another signal to trigger MCA polling. This is similar
to how other MCA interrupts are handled.

Deferred errors do not require any special handling related to the
interrupt, e.g. resetting or rearming the interrupt, etc.

However, Scalable MCA systems include a pair of registers, MCA_DESTAT
and MCA_DEADDR, that should be checked for valid errors. This check
should be done whenever MCA registers are polled. Currently, the
deferred error interrupt does this check, but the MCA polling function
does not.

Call the MCA polling function when handling the deferred error
interrupt. This keeps all "polling" cases in a common function.

Call the polling function only for banks that have the deferred error
interrupt enabled.

Add an SMCA status check helper. This will do the same status check and
register clearing that the interrupt handler has done. And it extends
the common polling flow to find AMD deferred errors.

Remove old code whose functionality is already covered in the common MCA
code.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
---

Notes:
    Link:
    https://lore.kernel.org/r/20240523155641.2805411-8-yazen.ghannam@amd.com
    
    v1->v2:
    * Keep code comment.
    * Log directly from helper function rather than pass values.

 arch/x86/kernel/cpu/mce/amd.c  | 103 ++---------------------------------------
 arch/x86/kernel/cpu/mce/core.c |  58 +++++++++++++++++++++++
 2 files changed, 63 insertions(+), 98 deletions(-)

Comments

Zhuo, Qiuxu Feb. 18, 2025, 7:37 a.m. UTC | #1
> From: Yazen Ghannam <yazen.ghannam@amd.com>
> [...]
> +static bool smca_should_log_poll_error(enum mcp_flags flags, struct
> +mce_hw_err *err) {
> +	struct mce *m = &err->m;
> +
> +	/*
> +	 * If this is a deferred error found in MCA_STATUS, then clear
> +	 * the redundant data from the MCA_DESTAT register.
> +	 */
> +	if (m->status & MCI_STATUS_VAL) {
> +		if (m->status & MCI_STATUS_DEFERRED)
> +			mce_wrmsrl(MSR_AMD64_SMCA_MCx_DESTAT(m-
> >bank), 0);
> +
> +		return true;
> +	}
> +
> +	/*
> +	 * If the MCA_DESTAT register has valid data, then use
> +	 * it as the status register.
> +	 */
> +	m->status = mce_rdmsrl(MSR_AMD64_SMCA_MCx_DESTAT(m-
> >bank));
> +
> +	if (!(m->status & MCI_STATUS_VAL))
> +		return false;
> +
> +	/*
> +	 * Gather all relevant data now and log the record before clearing
> +	 * the deferred status register. This avoids needing to go back to
> +	 * the polling function for these actions.
> +	 */
> +	mce_read_aux(err, m->bank);
> +
> +	if (m->status & MCI_STATUS_ADDRV)
> +		m->addr =
> mce_rdmsrl(MSR_AMD64_SMCA_MCx_DEADDR(m->bank));
> +
> +	smca_extract_err_addr(m);
> +	m->severity = mce_severity(m, NULL, NULL, false);
> +

Is the following check in machine_check_poll() needed before 
queuing/logging AMD's deferred error?

       if (mca_cfg.dont_log_ce && !mce_usable_address(m))
             //Just clear MCA_STATUS, but not queue/log errors.

> +	if (flags & MCP_QUEUE_LOG)
> +		mce_gen_pool_add(err);
> +	else
> +		mce_log(err);
> +
> +	mce_wrmsrl(MSR_AMD64_SMCA_MCx_DESTAT(m->bank), 0);
> +	return false;
> +}
[...]
Yazen Ghannam Feb. 19, 2025, 4:09 p.m. UTC | #2
On Tue, Feb 18, 2025 at 07:37:18AM +0000, Zhuo, Qiuxu wrote:
> > From: Yazen Ghannam <yazen.ghannam@amd.com>
> > [...]
> > +static bool smca_should_log_poll_error(enum mcp_flags flags, struct
> > +mce_hw_err *err) {
> > +	struct mce *m = &err->m;
> > +
> > +	/*
> > +	 * If this is a deferred error found in MCA_STATUS, then clear
> > +	 * the redundant data from the MCA_DESTAT register.
> > +	 */
> > +	if (m->status & MCI_STATUS_VAL) {
> > +		if (m->status & MCI_STATUS_DEFERRED)
> > +			mce_wrmsrl(MSR_AMD64_SMCA_MCx_DESTAT(m-
> > >bank), 0);
> > +
> > +		return true;
> > +	}
> > +
> > +	/*
> > +	 * If the MCA_DESTAT register has valid data, then use
> > +	 * it as the status register.
> > +	 */
> > +	m->status = mce_rdmsrl(MSR_AMD64_SMCA_MCx_DESTAT(m-
> > >bank));
> > +
> > +	if (!(m->status & MCI_STATUS_VAL))
> > +		return false;
> > +
> > +	/*
> > +	 * Gather all relevant data now and log the record before clearing
> > +	 * the deferred status register. This avoids needing to go back to
> > +	 * the polling function for these actions.
> > +	 */
> > +	mce_read_aux(err, m->bank);
> > +
> > +	if (m->status & MCI_STATUS_ADDRV)
> > +		m->addr =
> > mce_rdmsrl(MSR_AMD64_SMCA_MCx_DEADDR(m->bank));
> > +
> > +	smca_extract_err_addr(m);
> > +	m->severity = mce_severity(m, NULL, NULL, false);
> > +
> 
> Is the following check in machine_check_poll() needed before 
> queuing/logging AMD's deferred error?
> 
>        if (mca_cfg.dont_log_ce && !mce_usable_address(m))
>              //Just clear MCA_STATUS, but not queue/log errors.
> 

Good question. Deferred errors are uncorrectable errors that don't need
immediate action. They are not correctable errors, so the 'dont_log_ce'
flag shouldn't apply.

Thanks,
Yazen
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
index 5e491dbdeecc..5f547f5b3de0 100644
--- a/arch/x86/kernel/cpu/mce/amd.c
+++ b/arch/x86/kernel/cpu/mce/amd.c
@@ -56,6 +56,7 @@  static bool thresholding_irq_en;
 
 struct mce_amd_cpu_data {
 	mce_banks_t     thr_intr_banks;
+	mce_banks_t     dfr_intr_banks;
 };
 
 static DEFINE_PER_CPU_READ_MOSTLY(struct mce_amd_cpu_data, mce_amd_data);
@@ -300,8 +301,10 @@  static void smca_configure(unsigned int bank, unsigned int cpu)
 		 * APIC based interrupt. First, check that no interrupt has been
 		 * set.
 		 */
-		if ((low & BIT(5)) && !((high >> 5) & 0x3))
+		if ((low & BIT(5)) && !((high >> 5) & 0x3)) {
+			__set_bit(bank, this_cpu_ptr(&mce_amd_data)->dfr_intr_banks);
 			high |= BIT(5);
+		}
 
 		this_cpu_ptr(mce_banks_array)[bank].lsb_in_status = !!(low & BIT(8));
 
@@ -794,37 +797,6 @@  bool amd_mce_usable_address(struct mce *m)
 	return false;
 }
 
-static void __log_error(unsigned int bank, u64 status, u64 addr, u64 misc)
-{
-	struct mce_hw_err err;
-	struct mce *m = &err.m;
-
-	mce_prep_record(&err);
-
-	m->status = status;
-	m->misc   = misc;
-	m->bank   = bank;
-	m->tsc	 = rdtsc();
-
-	if (m->status & MCI_STATUS_ADDRV) {
-		m->addr = addr;
-
-		smca_extract_err_addr(m);
-	}
-
-	if (mce_flags.smca) {
-		rdmsrl(MSR_AMD64_SMCA_MCx_IPID(bank), m->ipid);
-
-		if (m->status & MCI_STATUS_SYNDV) {
-			rdmsrl(MSR_AMD64_SMCA_MCx_SYND(bank), m->synd);
-			rdmsrl(MSR_AMD64_SMCA_MCx_SYND1(bank), err.vendor.amd.synd1);
-			rdmsrl(MSR_AMD64_SMCA_MCx_SYND2(bank), err.vendor.amd.synd2);
-		}
-	}
-
-	mce_log(&err);
-}
-
 DEFINE_IDTENTRY_SYSVEC(sysvec_deferred_error)
 {
 	trace_deferred_error_apic_entry(DEFERRED_ERROR_VECTOR);
@@ -834,75 +806,10 @@  DEFINE_IDTENTRY_SYSVEC(sysvec_deferred_error)
 	apic_eoi();
 }
 
-/*
- * Returns true if the logged error is deferred. False, otherwise.
- */
-static inline bool
-_log_error_bank(unsigned int bank, u32 msr_stat, u32 msr_addr, u64 misc)
-{
-	u64 status, addr = 0;
-
-	rdmsrl(msr_stat, status);
-	if (!(status & MCI_STATUS_VAL))
-		return false;
-
-	if (status & MCI_STATUS_ADDRV)
-		rdmsrl(msr_addr, addr);
-
-	__log_error(bank, status, addr, misc);
-
-	wrmsrl(msr_stat, 0);
-
-	return status & MCI_STATUS_DEFERRED;
-}
-
-static bool _log_error_deferred(unsigned int bank, u32 misc)
-{
-	if (!_log_error_bank(bank, mca_msr_reg(bank, MCA_STATUS),
-			     mca_msr_reg(bank, MCA_ADDR), misc))
-		return false;
-
-	/*
-	 * Non-SMCA systems don't have MCA_DESTAT/MCA_DEADDR registers.
-	 * Return true here to avoid accessing these registers.
-	 */
-	if (!mce_flags.smca)
-		return true;
-
-	/* Clear MCA_DESTAT if the deferred error was logged from MCA_STATUS. */
-	wrmsrl(MSR_AMD64_SMCA_MCx_DESTAT(bank), 0);
-	return true;
-}
-
-/*
- * We have three scenarios for checking for Deferred errors:
- *
- * 1) Non-SMCA systems check MCA_STATUS and log error if found.
- * 2) SMCA systems check MCA_STATUS. If error is found then log it and also
- *    clear MCA_DESTAT.
- * 3) SMCA systems check MCA_DESTAT, if error was not found in MCA_STATUS, and
- *    log it.
- */
-static void log_error_deferred(unsigned int bank)
-{
-	if (_log_error_deferred(bank, 0))
-		return;
-
-	/*
-	 * Only deferred errors are logged in MCA_DE{STAT,ADDR} so just check
-	 * for a valid error.
-	 */
-	_log_error_bank(bank, MSR_AMD64_SMCA_MCx_DESTAT(bank),
-			      MSR_AMD64_SMCA_MCx_DEADDR(bank), 0);
-}
-
 /* APIC interrupt handler for deferred errors */
 static void amd_deferred_error_interrupt(void)
 {
-	unsigned int bank;
-
-	for (bank = 0; bank < this_cpu_read(mce_num_banks); ++bank)
-		log_error_deferred(bank);
+	machine_check_poll(MCP_TIMESTAMP, &this_cpu_ptr(&mce_amd_data)->dfr_intr_banks);
 }
 
 static void reset_block(struct threshold_block *block)
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index b26eb576e413..9502e8d3fee7 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -692,6 +692,61 @@  static noinstr void mce_read_aux(struct mce_hw_err *err, int i)
 
 DEFINE_PER_CPU(unsigned, mce_poll_count);
 
+/*
+ * We have three scenarios for checking for Deferred errors:
+ *
+ * 1) Non-SMCA systems check MCA_STATUS and log error if found.
+ * 2) SMCA systems check MCA_STATUS. If error is found then log it and also
+ *    clear MCA_DESTAT.
+ * 3) SMCA systems check MCA_DESTAT, if error was not found in MCA_STATUS, and
+ *    log it.
+ */
+static bool smca_should_log_poll_error(enum mcp_flags flags, struct mce_hw_err *err)
+{
+	struct mce *m = &err->m;
+
+	/*
+	 * If this is a deferred error found in MCA_STATUS, then clear
+	 * the redundant data from the MCA_DESTAT register.
+	 */
+	if (m->status & MCI_STATUS_VAL) {
+		if (m->status & MCI_STATUS_DEFERRED)
+			mce_wrmsrl(MSR_AMD64_SMCA_MCx_DESTAT(m->bank), 0);
+
+		return true;
+	}
+
+	/*
+	 * If the MCA_DESTAT register has valid data, then use
+	 * it as the status register.
+	 */
+	m->status = mce_rdmsrl(MSR_AMD64_SMCA_MCx_DESTAT(m->bank));
+
+	if (!(m->status & MCI_STATUS_VAL))
+		return false;
+
+	/*
+	 * Gather all relevant data now and log the record before clearing
+	 * the deferred status register. This avoids needing to go back to
+	 * the polling function for these actions.
+	 */
+	mce_read_aux(err, m->bank);
+
+	if (m->status & MCI_STATUS_ADDRV)
+		m->addr = mce_rdmsrl(MSR_AMD64_SMCA_MCx_DEADDR(m->bank));
+
+	smca_extract_err_addr(m);
+	m->severity = mce_severity(m, NULL, NULL, false);
+
+	if (flags & MCP_QUEUE_LOG)
+		mce_gen_pool_add(err);
+	else
+		mce_log(err);
+
+	mce_wrmsrl(MSR_AMD64_SMCA_MCx_DESTAT(m->bank), 0);
+	return false;
+}
+
 /*
  * Newer Intel systems that support software error
  * recovery need to make additional checks. Other
@@ -718,6 +773,9 @@  static bool should_log_poll_error(enum mcp_flags flags, struct mce_hw_err *err)
 {
 	struct mce *m = &err->m;
 
+	if (mce_flags.smca)
+		return smca_should_log_poll_error(flags, err);
+
 	/* If this entry is not valid, ignore it. */
 	if (!(m->status & MCI_STATUS_VAL))
 		return false;