MIME-Version: 1.0
In-Reply-To: <CA+55aFwK96Eryb2PMoeaLxPSu4srGQakh=jO646Wqv9advLp0A@mail.gmail.com>
References: <2809506.pL8kVbvXcY@aspire.rjw.lan> <1578405.51lzoSX1jh@aspire.rjw.lan>
 <20171209103325.GA13867@amd> <CA+55aFw8tuoJ2gcXx3K2sKFf2Y9hXX4naMVQNqGOUivnjwhjkg@mail.gmail.com>
 <20171209220110.GA11496@amd> <CA+55aFySAdiBZhZ0PSDjH5PuvPPcMsBRXbxCkObfm1eY7gHDbQ@mail.gmail.com>
 <20171210162305.GA10159@amd> <CA+55aFxc9e=CuwrAzhX7BgV4Or+unTQDXW1Xk-gXOZPiAtN3Bw@mail.gmail.com>
 <20171210185638.GA10363@amd> <CA+55aFy2-vNAXSnSBoiSfvhijKXzYVmpYr1VCOb87+tgrw0B1Q@mail.gmail.com>
 <20171210204350.GA25013@amd> <CA+55aFwK96Eryb2PMoeaLxPSu4srGQakh=jO646Wqv9advLp0A@mail.gmail.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Tue, 12 Dec 2017 09:27:13 -0800
Message-ID: <CA+55aFwMCHd7ppU2aWafXoHdTme=g=HJ1UMmNp5WuQ9pQ7i7Cg@mail.gmail.com>
Subject: Re: Linux 4.15-rc2: Regression in resume from ACPI S3
To: Pavel Machek <pavel@ucw.cz>
Cc: Zhang Rui <rui.zhang@intel.com>, Andrew Lutomirski <luto@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Jarkko Nikula <jarkko.nikula@linux.intel.com>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "the arch/x86 maintainers" <x86@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1593
Lines: 41

On Sun, Dec 10, 2017 at 1:28 PM, Linus Torvalds
<torvalds@linux-foundation.org> 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.

 - 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?

               Linus