On Wed, Aug 10, 2016 at 6:35 PM, Rafael J. Wysocki <raf...@kernel.org> wrote:
> On Thu, Aug 11, 2016 at 3:17 AM, Thomas Garnier <thgar...@google.com> wrote:
>> On Wed, Aug 10, 2016 at 5:35 PM, Rafael J. Wysocki <raf...@kernel.org> wrote:
>>> On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina <ji...@kernel.org> wrote:
>>>> On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
>>>>
>>>>> 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!
>>
>>>
>>> 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=fb754f958f8e46202c1efd7f66d5b3db1208117d
>
> Well, 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
