Messages in this thread | | | Date | Sun, 3 May 2015 11:22:12 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH 2/4] x86/mce/amd: Introduce deferred error interrupt handler |
| |
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. :-)
> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> > --- > arch/x86/include/asm/entry_arch.h | 3 + > arch/x86/include/asm/hardirq.h | 3 + > arch/x86/include/asm/hw_irq.h | 2 + > arch/x86/include/asm/irq_vectors.h | 1 + > arch/x86/include/asm/mce.h | 3 + > arch/x86/include/asm/trace/irq_vectors.h | 6 ++ > arch/x86/include/asm/traps.h | 3 +- > arch/x86/kernel/cpu/mcheck/mce_amd.c | 101 +++++++++++++++++++++++++++++++ > arch/x86/kernel/entry_64.S | 5 ++ > arch/x86/kernel/irq.c | 6 ++ > arch/x86/kernel/irqinit.c | 4 ++ > arch/x86/kernel/traps.c | 4 ++ > 12 files changed, 140 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/entry_arch.h b/arch/x86/include/asm/entry_arch.h > index dc5fa66..f7b957b 100644 > --- a/arch/x86/include/asm/entry_arch.h > +++ b/arch/x86/include/asm/entry_arch.h > @@ -50,4 +50,7 @@ BUILD_INTERRUPT(thermal_interrupt,THERMAL_APIC_VECTOR) > BUILD_INTERRUPT(threshold_interrupt,THRESHOLD_APIC_VECTOR) > #endif > > +#ifdef CONFIG_X86_MCE_AMD > +BUILD_INTERRUPT(deferred_interrupt, DEFERRED_APIC_VECTOR)
All the other names are written out so you can simply do
BUILD_INTERRUPT(deferred_error_interrupt, DEFERRED_ERROR_VECTOR)
> +#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;
> +#endif > #if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN) > unsigned int irq_hv_callback_count; > #endif > diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h > index e9571dd..7cf2670 100644 > --- a/arch/x86/include/asm/hw_irq.h > +++ b/arch/x86/include/asm/hw_irq.h
...
> +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...
> + def_new = DEF_LVT_OFF; > + low = (low & ~MASK_DEF_LVTOFF) | (DEF_LVT_OFF << 4); > + } > + > + def_offset = setup_APIC_deferred_error(def_offset, def_new); > + > + if ((def_offset == def_new) && > + (def_int_vector != amd_deferred_error_interrupt)) > + def_int_vector = amd_deferred_error_interrupt; > + > + low = (low & ~MASK_DEF_INT_TYPE) | DEF_INT_TYPE_APIC; > + wrmsr(MSR_CU_DEF_ERR, low, high); > +}
...
> +/* 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.
-- Regards/Gruss, Boris.
ECO tip #101: Trim your mails when you reply. --
| |