Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751666AbbECJWd (ORCPT ); Sun, 3 May 2015 05:22:33 -0400 Received: from mail.skyhub.de ([78.46.96.112]:57763 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751016AbbECJWX (ORCPT ); Sun, 3 May 2015 05:22:23 -0400 Date: Sun, 3 May 2015 11:22:12 +0200 From: Borislav Petkov To: Aravind Gopalakrishnan Cc: tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com, tony.luck@intel.com, jiang.liu@linux.intel.com, yinghai@kernel.org, x86@kernel.org, dvlasenk@redhat.com, JBeulich@suse.com, slaoub@gmail.com, luto@amacapital.net, dave.hansen@linux.intel.com, oleg@redhat.com, rostedt@goodmis.org, rusty@rustcorp.com.au, prarit@redhat.com, linux@rasmusvillemoes.dk, jroedel@suse.de, andriy.shevchenko@linux.intel.com, macro@linux-mips.org, wangnan0@huawei.com, linux-kernel@vger.kernel.org, linux-edac@vger.kernel.org Subject: Re: [PATCH 2/4] x86/mce/amd: Introduce deferred error interrupt handler Message-ID: <20150503092212.GC18048@pd.tnic> References: <1430405365-4473-1-git-send-email-Aravind.Gopalakrishnan@amd.com> <1430405365-4473-3-git-send-email-Aravind.Gopalakrishnan@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1430405365-4473-3-git-send-email-Aravind.Gopalakrishnan@amd.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5049 Lines: 153 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 > --- > 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. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/