On 5/3/2015 4:22 AM, Borislav Petkov wrote:
On Thu, Apr 30, 2015 at 09:49:23AM -0500, Aravind Gopalakrishnan wrote:
Changes introduced in the patch-
- Assign vector number 0xf4 for Deferred errors
- Declare deferred_interrupt, allocate gate and bind it
to DEFERRED_APIC_VECTOR.
- Declare smp_deferred_interrupt to be used as the
entry point for the interrupt in mce_amd.c
- Define trace_deferred_interrupt for tracing
- Enable deferred error interrupt selectively upon detection
of 'succor' bitfield
- Setup amd_deferred_error_interrupt() to handle the interrupt
and assign it to def_int_vector if feature is present in HW.
Else, let default handler deal with it.
- Provide Deferred error interrupt stats on
/proc/interrupts by incrementing irq_deferred_count
This commit message should explain the feature in more high-level way,
what is it good for and so on, not what you're adding.
That I can see. :-)
Okay, I'll include a short description of deferred errors here for V2.
+#endif
#endif
diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h
index 0f5fb6b..448451c 100644
--- a/arch/x86/include/asm/hardirq.h
+++ b/arch/x86/include/asm/hardirq.h
@@ -33,6 +33,9 @@ typedef struct {
#ifdef CONFIG_X86_MCE_THRESHOLD
unsigned int irq_threshold_count;
#endif
+#ifdef CONFIG_X86_MCE_AMD
+ unsigned int irq_deferred_count;
Right
unsigned int irq_deferred_error_count;
Ack.
+static void deferred_error_interrupt_enable(struct cpuinfo_x86 *c)
+{
+ u32 low = 0, high = 0;
+ int def_offset = -1, def_new;
+
+ if (rdmsr_safe(MSR_CU_DEF_ERR, &low, &high))
+ return;
+
+ def_new = (low & MASK_DEF_LVTOFF) >> 4;
+ if (c->x86 == 0x15 && c->x86_model == 0x60 &&
+ !(low & MASK_DEF_LVTOFF)) {
What's the family check for? for BIOSes which don't set the LVT offset
to 2, as they should?
If so, we probably should say
pr_err(FW_BUG "Your BIOS is not setting up LVT offset 0x2 for deferred error
IRQs correctly.\n");
or similar...
Yeah. I meant to provide a comment at least for this.
Forgot to do that.
I'll print out a error message as you suggested (considering we do this
in other places like threshold setup or IBS setup..)
+/* Apic interrupt handler for deferred errors */
+static void amd_deferred_error_interrupt(void)
+{
+ u64 status;
+ unsigned int bank;
+ struct mce m;
+
+ for (bank = 0; bank < mca_cfg.banks; ++bank) {
+ rdmsrl(MSR_IA32_MCx_STATUS(bank), status);
+
+ if (!(status & MCI_STATUS_VAL) ||
+ !(status & MCI_STATUS_DEFERRED))
+ continue;
+
+ mce_setup(&m);
+ m.bank = bank;
+ m.status = status;
+ mce_log(&m);
+ wrmsrl(MSR_IA32_MCx_STATUS(bank), 0);
+ break;
+ }
That's very similar to what we do in the end of
amd_threshold_interrupt(). You could add a generic __log_error() static
helper in a pre-patch and then call it here.
Right. I think a __log_error() is a good idea.
Except, in amd_threshold_interrupt(), we have-
m.misc = ((u64)high << 32) | low;
which, is actually useless as we don't use m.misc anywhere in
amd_decode_mce() or anywhere else in the decoding pipeline AFAICT.
We only print out if 'misc' is valid and we only need status bits for that-
((m->status & MCI_STATUS_MISCV) ? "MiscV" : "-"),
But, more importantly, we don't setup 'm.addr' here (in
amd_threshold_interrupt() or in amd_deferred_error_interrupt())
Which means anytime we pass an error to be decoded from the interrupt
handlers, we don't get any info about the error address.
So, we can do one of these-
1. Remove m.misc setup in amd_threshold_interrupt() and
rdmsrl(MSR_IA32_MCx_ADDR(bank), m.addr) before we call mce_log()
2. Since we have mce_read_aux() that reads misc and addr registers, we
can move the mce_[rd|wr]msrl wrappers and mce_read_aux() into mce.h and
use it here in mce_amd.c
Thoughts?
Thanks,
-Aravind.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
