Re: Yet another S3 issue in Xen 4.14

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.10.2020 14:00, Marek Marczykowski-Górecki wrote:
> On Thu, Oct 01, 2020 at 03:12:47AM +0200, Marek Marczykowski-Górecki wrote:
>> After patching the previous issue ("x86/S3: Fix Shadow Stack resume
>> path") I still encounter issues resume from S3.
>> Since I had it working on Xen 4.13 on this particular hardware (Thinkpad
>> P52), I bisected it and got this:
>>
>> commit 4304ff420e51b973ec9eb9dafd64a917dd9c0fb1
>> Author: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Date:   Wed Dec 11 20:59:19 2019 +0000
>>
>>     x86/S3: Drop {save,restore}_rest_processor_state() completely
>>     
>>     There is no need to save/restore FS/GS/XCR0 state.  It will be handled
>>     suitably on the context switch away from the idle.
>>     
>>     The CR4 restoration in restore_rest_processor_state() was actually 
>> fighting
>>     later code in enter_state() which tried to keep CR4.MCE clear until 
>> everything
>>     was set up.  Delete the intermediate restoration, and defer final 
>> restoration
>>     until after MCE is reconfigured.
>>     
>>     Restoring PAT can be done earlier, and ideally before paging is enabled. 
>>  By
>>     moving it into the trampoline during the setup for 64bit, the call can be
>>     dropped from cpu_init().  The EFI path boot path doesn't disable paging, 
>> so
>>     make the adjustment when switching onto Xen's pagetables.
>>     
>>     The only remaing piece of restoration is load_system_tables(), so 
>> suspend.c
>>     can be deleted in its entirety.
>>     
>>     Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>     Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> Parent of this commit suspends and resumes just fine. With this commit
>> applied, it (I think) it panics, at least I get reboot after 5s. Sadly, I
>> don't have serial console there.
> 
> Reading the patch and discussion about it, I think the important details
> is this uses EFI boot.
> That may also explain why I haven't see it while bisecting the other
> S3 issue, where I use legacy boot path.

Hmm, interesting. I didn't think though that the code paths altered
by the commit above would be in any way dependent upon the way Xen
was booted. But if that's the observation, then perhaps there's
something I'm unaware of.

Jan