Quoting Lorenzo Pieralisi (2014-02-28 14:49:33)
On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:
[...]
+/*
- The framework loads the hibernation image into a linked list anchored
- at restore_pblist, for swsusp_arch_resume() to copy back to the proper
- destinations.
- To make this work if resume is triggered from initramfs, the
- pagetables need to be switched to allow writes to kernel mem.
- */
Comment above needs updating. We are switching page tables to a set of page tables that are certain to live at the same location in the older kernel, that's the only reason, as we discussed. soft_restart will make sure (again) to switch to 1:1 page tables so that we can call cpu_resume with the MMU off.
How does this look?
The framework loads as much of the hibernation image to final physical pages as possible. Any pages that were in use, will need to be restored prior to the soft_restart. The pages to restore are maintained in the list anchored at restore_pblist. At this point, we can swap the pages to their final location. We must switch the mapping to 1:1 to ensure that when we overwrite the page table physical pages we're using a known physical location (idmap_pgd) with known contents.
It is ok, a tad too verbose. All I care about is a comment describing what's really needed, the existing one was confusing and wrong.
Maybe more like:
Restore physical pages that were in use while loading hibernation image. Use idmap_pgd so our page tables use the same physical address as the hibernation image. Will be overwriten with the same contents.
+/*
- Resume from the hibernation image.
- Due to the kernel heap / data restore, stack contents change underneath
- and that would make function calls impossible; switch to a temporary
- stack within the nosave region to avoid that problem.
- */
+int swsusp_arch_resume(void) +{
extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
call_with_stack(arch_restore_image, 0,
resume_stack + sizeof(resume_stack));
This does not guarantee your stack is 8-byte aligned, that's not AAPCS compliant and might buy you trouble.
Either you align the stack or you align the pointer you are passing.
Please have a look at kernel/process.c
I've added this for now, do you see any issues?
-static u8 resume_stack[PAGE_SIZE/2] __nosavedata; +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
resume_stack + sizeof(resume_stack));
resume_stack + ARRAY_SIZE(resume_stack));
I do not see why the stack depends on the PAGE_SIZE. I would be surprised if you need more than a few bytes (given that soft_restart switches stack again...), go through it with a debugger, it is easy to check the stack usage and allow for some extra buffer (but half a page is not needed).
I assuming this is becase the no-save region is one page anyway (we skip restoring the no-save region physical page). So maybe 1/2 is a way to leave some room for whatever else may need to be here, but in any case the 4k is used for nosave. I think you're right that it can be much less.
I'll check into this...
Thanks!
Sebastian