Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752566AbbFMHBD (ORCPT ); Sat, 13 Jun 2015 03:01:03 -0400 Received: from mail-wi0-f182.google.com ([209.85.212.182]:35078 "EHLO mail-wi0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751406AbbFMHAz (ORCPT ); Sat, 13 Jun 2015 03:00:55 -0400 Date: Sat, 13 Jun 2015 09:00:46 +0200 From: Ingo Molnar To: Srinivas Pandruvada Cc: Andy Lutomirski , Brian Gerst , "H. Peter Anvin" , Ingo Molnar , Thomas Gleixner , Pavel Machek , "Rafael J. Wysocki" , X86 ML , "linux-pm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Denys Vlasenko , Borislav Petkov , Linus Torvalds Subject: Re: [PATCH] x86: General protection fault after STR (32 bit systems only) Message-ID: <20150613070046.GA26502@gmail.com> References: <1434066338-6619-1-git-send-email-srinivas.pandruvada@linux.intel.com> <20150612060747.GA25024@gmail.com> <20150612075013.GA8759@gmail.com> <20150612083625.GA22760@gmail.com> <1434133883.2353.25.camel@spandruv-DESK3.jf.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1434133883.2353.25.camel@spandruv-DESK3.jf.intel.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: 3574 Lines: 87 * Srinivas Pandruvada wrote: > On Fri, 2015-06-12 at 11:11 -0700, Andy Lutomirski wrote: > > On Fri, Jun 12, 2015 at 8:48 AM, Brian Gerst wrote: > > > On Fri, Jun 12, 2015 at 4:36 AM, Ingo Molnar wrote: > > >> > > >> * H. Peter Anvin wrote: > > >> > > >>> %es is used implicitly by string instructions. > > >> > > >> Ok, so we are probably better off reloading ES as well early, right > > >> when we return from the firmware, just in case something does > > >> a copy before we hit the ES restore in restore_processor_state(), > > >> which is a generic C function? > > >> > > >> Something like the patch below? > > >> > > >> I also added FS/GS/SS reloading to make it complete. If this (or a variant > > >> thereof, it's still totally untested) works then we can remove the segment > > >> save/restore layer in __save/restore_processor_state(). > > >> > > >> Thanks, > > >> > > >> Ingo > > >> > > >> ===========> > > >> arch/x86/kernel/acpi/wakeup_32.S | 13 +++++++++++++ > > >> 1 file changed, 13 insertions(+) > > >> > > >> diff --git a/arch/x86/kernel/acpi/wakeup_32.S b/arch/x86/kernel/acpi/wakeup_32.S > > >> index 665c6b7d2ea9..1376a7fc21b7 100644 > > >> --- a/arch/x86/kernel/acpi/wakeup_32.S > > >> +++ b/arch/x86/kernel/acpi/wakeup_32.S > > >> @@ -61,6 +61,19 @@ ENTRY(wakeup_pmode_return) > > >> > > >> > > >> restore_registers: > > >> + /* > > >> + * In case the BIOS corrupted our segment descriptors, > > >> + * reload them to clear out any shadow descriptor > > >> + * state: > > >> + */ > > >> + movl $__USER_DS, %eax > > >> + movl %eax, %ds > > >> + movl %eax, %es > > >> + movl %eax, %fs > > >> + movl %eax, %gs > > >> + movl $__KERNEL_DS, %eax > > >> + movl %eax, %ss > > >> + > > >> movl saved_context_ebp, %ebp > > >> movl saved_context_ebx, %ebx > > >> movl saved_context_esi, %esi > > > > > > If you follow the convoluted flow of the calls in this file, > > > wakeup_pmode_return is the first thing called from the trampoline on > > > resume, and that loads the data segments with __KERNEL_DS. The better > > > fix would be to set ds/es to __USER_DS there. Also since we are in > > > the kernel, fs is fixed at __KERNEL_PERCPU, and gs is either > > > __KERNEL_STACK_CANARY or user's gs. > > > > So why is this issue rare? Is it just that the first entry to > > userspace after resume is only very rarely via SYSEXIT? Or is there > > some other code path that usually, but not quite always, fixes up the > > segment registers? > > We spent a quite a bit of time debugging this. Any small debug code or printk > causes problem to disappear. We noticed that we can reproduce this more > frequently when the we run just one user space application to just do > suspend/resume test, which supports yours theory. So what happens if you add a couple of CPUID (or NOP) calls? If it's timing related, not segment register related, then that might 'fix' the bug too. Which would suggest that it might be some hardware problem, or a race with something (which could not really be on the Linux side, as we are the only thread of execution at this point)? Thanks, Ingo -- 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/