Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752379AbdLLSGI (ORCPT ); Tue, 12 Dec 2017 13:06:08 -0500 Received: from mail.kernel.org ([198.145.29.99]:42208 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751554AbdLLSGH (ORCPT ); Tue, 12 Dec 2017 13:06:07 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0AC2520837 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=luto@kernel.org X-Google-Smtp-Source: ACJfBouiKABjfcRzwJMF71+tATYbSdiX4RenBQIfxV4bt9whM5LfkLlqTZs5LFFkYEGDypxe6841CpGunaQBFnw++Rs= MIME-Version: 1.0 In-Reply-To: References: <2809506.pL8kVbvXcY@aspire.rjw.lan> <1578405.51lzoSX1jh@aspire.rjw.lan> <20171209103325.GA13867@amd> <20171209220110.GA11496@amd> <20171210162305.GA10159@amd> <20171210185638.GA10363@amd> <20171210204350.GA25013@amd> From: Andy Lutomirski Date: Tue, 12 Dec 2017 10:05:45 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: Linux 4.15-rc2: Regression in resume from ACPI S3 To: Linus Torvalds Cc: Pavel Machek , Zhang Rui , Andrew Lutomirski , Thomas Gleixner , Jarkko Nikula , "Rafael J. Wysocki" , Linux Kernel Mailing List , "the arch/x86 maintainers" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2050 Lines: 53 On Tue, Dec 12, 2017 at 9:27 AM, Linus Torvalds wrote: > On Sun, Dec 10, 2017 at 1:28 PM, Linus Torvalds > wrote: >> >> That said, this *all* smells wrong. Why is there a special >> fix_processor_context() function at all with different 32-bit and >> 64-bit behavior? This code is all written to be maximally confusing. > > Hmm. Looking a bit more at this, I think it should be solved by: > > - load the original read-write GDT early, along with the IDT. > > We have already saved it off in __save_processor_state(), and it may > have already gotten loaded very early in at least some of the paths, > but it definitely hasn't gotten reloaded in all the paths (think > "suspend/resume testing" etc). > > - add the LDT descriptor to the save area too, exactly like we > already have IDT/GDT. > > Then, we can do "load_ldt()" early (along with IDT and GDT). > > - now we can just load all the segments early, and get the percpu and > stack canary stuff without any special cases > > - do NOT use "load_gs_index()", which does that swapgs dance (twice!) > and plays with interrupt state. Just load the segment register, and > then do the wrmsrl() of the {FS,GS,KERNEL_GS}_BASE values. There is no > need for the swapgs dance. Using what helper? On x86_64, it can fault, and IIRC we explicitly don't allow loadsegment(gs, ...). > > - now that we have a fully working system, then the > "fix_processor_context()" code can do the "fancy" stuff like setting > up the RO fixmap GDT and re-initializing the TLB state. Those want > percpu stuff. > > In other words, why not try to organize this so that we do a simple > and fairly mindless restore of the low-level state first? Avoid all > the "system is halfway up" crud. > > Would that work for people? Andy? Other than the above, more or less. But we should really do all the user segments last. They're not at all needed for normal kernel execution, so I think they should be the very last part. I'll try to get to this today.