From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753494AbbKWIB1 (ORCPT ); Mon, 23 Nov 2015 03:01:27 -0500 Received: from mail-wm0-f52.google.com ([74.125.82.52]:34659 "EHLO mail-wm0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753307AbbKWIBY (ORCPT ); Mon, 23 Nov 2015 03:01:24 -0500 Date: Mon, 23 Nov 2015 09:01:20 +0100 From: Ingo Molnar To: Juergen Gross Cc: Linux Kernel Mailing List , the arch/x86 maintainers , Thomas Gleixner , "H. Peter Anvin" , Borislav Petkov Subject: Re: lapic_suspend/lapic_resume wrong? Message-ID: <20151123080120.GA20696@gmail.com> References: <5652C472.1010201@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5652C472.1010201@suse.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 * Juergen Gross wrote: > Hi, > > while trying to find the reason for a hanging kernel during resume > handling I found a strange inconsistency in arch/x86/kernel/apic/apic.c > regarding usage of config options. > > Attached patch addresses this, no test done as I'm not sure whether > this is a correct approach. Can you have a look at it, please? > > > Juergen > > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c > index 2f69e3b..bc06c9d 100644 > --- a/arch/x86/kernel/apic/apic.c > +++ b/arch/x86/kernel/apic/apic.c > @@ -2270,6 +2270,7 @@ static struct { > unsigned int apic_tmict; > unsigned int apic_tdcr; > unsigned int apic_thmr; > + unsigned int apic_cmci; > } apic_pm_state; > > static int lapic_suspend(void) > @@ -2299,6 +2300,10 @@ static int lapic_suspend(void) > if (maxlvt >= 5) > apic_pm_state.apic_thmr = apic_read(APIC_LVTTHMR); > #endif > +#ifdef CONFIG_X86_MCE_INTEL > + if (maxlvt >= 6) > + apic_pm_state.apic_cmci = apic_read(APIC_LVTCMCI); > +#endif > > local_irq_save(flags); > disable_local_APIC(); > @@ -2355,10 +2360,14 @@ static void lapic_resume(void) > apic_write(APIC_SPIV, apic_pm_state.apic_spiv); > apic_write(APIC_LVT0, apic_pm_state.apic_lvt0); > apic_write(APIC_LVT1, apic_pm_state.apic_lvt1); > -#if defined(CONFIG_X86_MCE_INTEL) > +#if defined(CONFIG_X86_THERMAL_VECTOR) > if (maxlvt >= 5) > apic_write(APIC_LVTTHMR, apic_pm_state.apic_thmr); > #endif > +#if defined(CONFIG_X86_MCE_INTEL) > + if (maxlvt >= 6) > + apic_write(APIC_LVTCMCI, apic_pm_state.apic_cmci); > +#endif > if (maxlvt >= 4) > apic_write(APIC_LVTPC, apic_pm_state.apic_lvtpc); > apic_write(APIC_LVTT, apic_pm_state.apic_lvtt); the x86 bit looks absolutely sensible to me. Have you checked whether we indeed lose this value over S/R, or is this mostly working fine by accident, due to us executing the CMCI vector initialization via: mce_syscore_resume()->__mcheck_cpu_init_vendor()->mce_intel_feature_init()->intel_init_cmci() on every resume event? The Xen fix is unrelated, just put into the same patch, right? Thanks, Ingo