Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754578AbbFLHu0 (ORCPT ); Fri, 12 Jun 2015 03:50:26 -0400 Received: from mail-wi0-f176.google.com ([209.85.212.176]:37004 "EHLO mail-wi0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754328AbbFLHuT (ORCPT ); Fri, 12 Jun 2015 03:50:19 -0400 Date: Fri, 12 Jun 2015 09:50:13 +0200 From: Ingo Molnar To: Andy Lutomirski Cc: Srinivas Pandruvada , Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , Pavel Machek , "Rafael J. Wysocki" , X86 ML , "linux-pm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Denys Vlasenko , Borislav Petkov , Brian Gerst , Linus Torvalds Subject: Re: [PATCH] x86: General protection fault after STR (32 bit systems only) Message-ID: <20150612075013.GA8759@gmail.com> References: <1434066338-6619-1-git-send-email-srinivas.pandruvada@linux.intel.com> <20150612060747.GA25024@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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: 1941 Lines: 59 * Andy Lutomirski wrote: > > --- a/arch/x86/kernel/acpi/wakeup_32.S > > +++ b/arch/x86/kernel/acpi/wakeup_32.S > > @@ -81,6 +81,10 @@ ENTRY(do_suspend_lowlevel) > > jmp ret_point > > .p2align 4,,7 > > ret_point: > > + /* In case the BIOS corrupted DS, make the kernel context minimally functional: */ > > + movl $__KERNEL_DS, %eax > > + movl %eax, %ds > > + > > On further thought, I think you want movl $__USER_DS, %eax. The > 32-bit kernel is a strange beast. Also, you should probably fix up > %es as well. So restore_processor_state() already restores ES. The idea here was to reload DS early on, because the kernel implicitly uses it for data access so we need it to be good to be able to continue executing any generic kernel code. We don't use %es: prefixed assembly AFAICS, what are the implicit users of ES? Also, to further confuse things, we also have: ENTRY(wakeup_pmode_return) wakeup_pmode_return: movw $__KERNEL_DS, %ax movw %ax, %ss movw %ax, %ds movw %ax, %es movw %ax, %fs movw %ax, %gs # reload the gdt, as we need the full 32 bit address lidt saved_idt lldt saved_ldt ljmp $(__KERNEL_CS), $1f 1: movl %cr3, %eax movl %eax, %cr3 wbinvd which seems to be another layer of restoration - but it possibly does not trigger in the S2RAM case here. Oh, funny the 'reload the gdt' comment: do you see an LGDT there? It reloads all segment selectors, the IDT, LDT and CR3, but does not seem to reload the GDT - the only thing the comment describes. 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/