Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly
From: Thomas Garnier <hidden>
Date: 2016-08-11 18:47:32
Also in:
lkml
Subsystem:
hibernation (aka software suspend, aka swsusp), suspend to ram, the rest · Maintainers:
"Rafael J. Wysocki", Linus Torvalds
On Wed, Aug 10, 2016 at 6:35 PM, Rafael J. Wysocki [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On Thu, Aug 11, 2016 at 3:17 AM, Thomas Garnier [off-list ref] wrote:quoted
On Wed, Aug 10, 2016 at 5:35 PM, Rafael J. Wysocki [off-list ref] wrote:quoted
On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina [off-list ref] wrote:quoted
On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:quoted
So I used your .config to generate one for my test machine and with that I can reproduce.Was that the config I've sent, or did Boris provide one as well? Which one are you able to reproduce with please?It's the Boris' one. Moreover, I have found the options that make the difference: unsetting CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied. Unbelievable, but that's what I'm seeing.Nice find!quoted
Now, that leads to a few questions: - How does lockdep change the picture so it matters for hibernation? - Why is hibernation the only piece that's affected? - Why is RANDOMIZE_MEMORY necessary to make this breakage show up? Thomas, any ideas?No idea so far. I will investigate though. We had an unrelated issue with CONFIG_DEBUG_PAGEALLOC on early boot. I don't think it was related because it was on early boot and with certain e820 memory layout (and PUD randomization that I disabled on the previous patch test). The fix is on tip: http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fb754f958f8e46202c1efd7f66d5b3db1208117dWell, I don't think this is related. In the meantime, I went back to my original .config and verified that setting CONFIG_DEBUG_LOCK_ALLOC in it caused hibernation to fail (with CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied), so this really matters somehow. Besides, now that I have a reproducer, I can check various other things and for example this change (sorry for broken whitespace): Index: linux-pm/arch/x86/mm/kaslr.c ===================================================================--- linux-pm.orig/arch/x86/mm/kaslr.c +++ linux-pm/arch/x86/mm/kaslr.c@@ -122,7 +122,7 @@ void __init kernel_randomize_memory(void prandom_bytes_state(&rand_state, &rand, sizeof(rand)); entropy = (rand % (entropy + 1)) & PUD_MASK; vaddr += entropy; - *kaslr_regions[i].base = vaddr; + *kaslr_regions[i].base += PUD_SIZE;
I think it works because the address is fixed now (just PUD aligned).
/*
* Jump the region and add a minimum padding based on
makes hibernation work for me again in the above configuration. To
me, this means that the $subject patch works as expected and the
problem really is related to the vaddr value being too big.
I managed to debug the restoration and found that a first access
violation happens here:
(gdb) x/20i 0xffffffffb20a46de
0xffffffffb20a46de <trace_suspend_resume+14>:
mov eax,DWORD PTR gs:[rip+0x4df65a4b] # 0xa130 <cpu_number>
Handled by do_async_page_fault which will fault as well on this instruction:
=> 0xffffffffb2047ca1 <do_async_page_fault+1>:
mov eax,DWORD PTR gs:[rip+0x4dfc4e58] # 0xcb00 <apf_reason+64>
So there is a problem with the gs register not being restored
correctly at this stage.
In create_image, there is tracing (trace_suspend_resume) before and
after the suspend. Except at this stage, gs was not yet restored. It
uses the old gs leading to the double fault.
I tested this fix to be correct:
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index a881c6a..33c79b6 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c@@ -300,12 +300,12 @@ static int create_image(int platform_mode) save_processor_state(); trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, true); error = swsusp_arch_suspend(); + /* Restore control flow magically appears here */ + restore_processor_state(); trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, false); if (error) printk(KERN_ERR "PM: Error %d creating hibernation image\n", error); - /* Restore control flow magically appears here */ - restore_processor_state(); if (!in_suspend) events_check_enabled = false;
Let me know if it works for you. Note that I don't know why this issue popup with the different config.
Thanks, Rafael