[PATCH] Hibernation: Document __save_processor_state() on x86-64

[PATCH] Hibernation: Document __save_processor_state() on x86-64

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Document the fact that __save_processor_state() has to save all CPU
registers referred to by the kernel in case a different kernel is
used to load and restore a hibernation image containing it. 

Sigend-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/x86/kernel/suspend_64.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Index: linux-2.6/arch/x86/kernel/suspend_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/suspend_64.c
+++ linux-2.6/arch/x86/kernel/suspend_64.c
@@ -19,6 +19,21 @@ extern const void __nosave_begin, __nosa
 
 struct saved_context saved_context;
 
+/**
+ *	__save_processor_state - save CPU registers before creating a
+ *		hibernation image and before restoring the memory state from it
+ *	@ctxt - structure to store the registers contents in
+ *
+ *	NOTE: If there is a CPU register the modification of which by the
+ *	boot kernel (ie. the kernel used for loading the hibernation image)
+ *	might affect the operations of the restored target kernel (ie. the one
+ *	saved in the hibernation image), then its contents must be saved by this
+ *	function.  In other words, if kernel A is hibernated and different
+ *	kernel B is used for loading the hibernation image into memory, the
+ *	kernel A's __save_processor_state() function must save all registers
+ *	needed by kernel A, so that it can operate correctly after the resume
+ *	regardless of what kernel B does in the meantime.
+ */
 void __save_processor_state(struct saved_context *ctxt)
 {
 	kernel_fpu_begin();
@@ -69,6 +84,11 @@ static void do_fpu_end(void)
 	kernel_fpu_end();
 }
 
+/**
+ *	__restore_processor_state - restore the contents of CPU registers saved
+ *		by __save_processor_state()
+ *	@ctxt - structure to load the registers contents from
+ */
 void __restore_processor_state(struct saved_context *ctxt)
 {
 	/*

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Pavel Machek]

Hi!

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Document the fact that __save_processor_state() has to save all CPU
> registers referred to by the kernel in case a different kernel is
> used to load and restore a hibernation image containing it. 


> Sigend-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  arch/x86/kernel/suspend_64.c |   20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> Index: linux-2.6/arch/x86/kernel/suspend_64.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/suspend_64.c
> +++ linux-2.6/arch/x86/kernel/suspend_64.c
> @@ -19,6 +19,21 @@ extern const void __nosave_begin, __nosa
>  
>  struct saved_context saved_context;
>  
> +/**
> + *	__save_processor_state - save CPU registers before creating a
> + *		hibernation image and before restoring the memory state from it
> + *	@ctxt - structure to store the registers contents in
> + *
> + *	NOTE: If there is a CPU register the modification of which by the
> + *	boot kernel (ie. the kernel used for loading the hibernation image)
> + *	might affect the operations of the restored target kernel (ie. the one
> + *	saved in the hibernation image), then its contents must be saved by this
> + *	function.  In other words, if kernel A is hibernated and different
> + *	kernel B is used for loading the hibernation image into memory, the
> + *	kernel A's __save_processor_state() function must save all registers
> + *	needed by kernel A, so that it can operate correctly after the resume
> + *	regardless of what kernel B does in the meantime.
> + */

Maybe this warning should be appended to struct saved_context
definition? Reordering its fields (etc) would be bad news, too, and
documentation near data structures is easier to find...

Thanks,
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Rafael J. Wysocki]

On Sunday, 30 of December 2007, Pavel Machek wrote:
> Hi!
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Document the fact that __save_processor_state() has to save all CPU
> > registers referred to by the kernel in case a different kernel is
> > used to load and restore a hibernation image containing it. 
> 
> 
> > Sigend-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  arch/x86/kernel/suspend_64.c |   20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > Index: linux-2.6/arch/x86/kernel/suspend_64.c
> > ===================================================================
> > --- linux-2.6.orig/arch/x86/kernel/suspend_64.c
> > +++ linux-2.6/arch/x86/kernel/suspend_64.c
> > @@ -19,6 +19,21 @@ extern const void __nosave_begin, __nosa
> >  
> >  struct saved_context saved_context;
> >  
> > +/**
> > + *	__save_processor_state - save CPU registers before creating a
> > + *		hibernation image and before restoring the memory state from it
> > + *	@ctxt - structure to store the registers contents in
> > + *
> > + *	NOTE: If there is a CPU register the modification of which by the
> > + *	boot kernel (ie. the kernel used for loading the hibernation image)
> > + *	might affect the operations of the restored target kernel (ie. the one
> > + *	saved in the hibernation image), then its contents must be saved by this
> > + *	function.  In other words, if kernel A is hibernated and different
> > + *	kernel B is used for loading the hibernation image into memory, the
> > + *	kernel A's __save_processor_state() function must save all registers
> > + *	needed by kernel A, so that it can operate correctly after the resume
> > + *	regardless of what kernel B does in the meantime.
> > + */
> 
> Maybe this warning should be appended to struct saved_context
> definition? Reordering its fields (etc) would be bad news, too,

Hmm, I think they can be reordered without any problem.  It's always the same
kernel using them, although at different times.

> and documentation near data structures is easier to find...

Well, I'll add a coment next to the definition of struct saved_context to
explain what it's for, but IMO the behavior of __save_processor_state() is what
_really_ matters (ie. it doesn't matter how and where exactly it saves the
registers as long as __restore_processor_state() can restore their "old"
values).

Greetings,
Rafael

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Pavel Machek]

On Sun 2007-12-30 14:30:07, Rafael J. Wysocki wrote:
> On Sunday, 30 of December 2007, Pavel Machek wrote:
> > Hi!
> > 
> > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > 
> > > Document the fact that __save_processor_state() has to save all CPU
> > > registers referred to by the kernel in case a different kernel is
> > > used to load and restore a hibernation image containing it. 
> > 
> > 
> > > Sigend-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > > ---
> > >  arch/x86/kernel/suspend_64.c |   20 ++++++++++++++++++++
> > >  1 file changed, 20 insertions(+)
> > > 
> > > Index: linux-2.6/arch/x86/kernel/suspend_64.c
> > > ===================================================================
> > > --- linux-2.6.orig/arch/x86/kernel/suspend_64.c
> > > +++ linux-2.6/arch/x86/kernel/suspend_64.c
> > > @@ -19,6 +19,21 @@ extern const void __nosave_begin, __nosa
> > >  
> > >  struct saved_context saved_context;
> > >  
> > > +/**
> > > + *	__save_processor_state - save CPU registers before creating a
> > > + *		hibernation image and before restoring the memory state from it
> > > + *	@ctxt - structure to store the registers contents in
> > > + *
> > > + *	NOTE: If there is a CPU register the modification of which by the
> > > + *	boot kernel (ie. the kernel used for loading the hibernation image)
> > > + *	might affect the operations of the restored target kernel (ie. the one
> > > + *	saved in the hibernation image), then its contents must be saved by this
> > > + *	function.  In other words, if kernel A is hibernated and different
> > > + *	kernel B is used for loading the hibernation image into memory, the
> > > + *	kernel A's __save_processor_state() function must save all registers
> > > + *	needed by kernel A, so that it can operate correctly after the resume
> > > + *	regardless of what kernel B does in the meantime.
> > > + */
> > 
> > Maybe this warning should be appended to struct saved_context
> > definition? Reordering its fields (etc) would be bad news, too,
> 
> Hmm, I think they can be reordered without any problem.  It's always the same
> kernel using them, although at different times.

Aha... ok, then. I misunderstood how it works.

> > and documentation near data structures is easier to find...
> 
> Well, I'll add a coment next to the definition of struct saved_context to
> explain what it's for, but IMO the behavior of __save_processor_state() is what
> _really_ matters (ie. it doesn't matter how and where exactly it saves the
> registers as long as __restore_processor_state() can restore their "old"
> values).

Yep, right.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Ingo Molnar]

* Rafael J. Wysocki <rjw@sisk.pl> wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Document the fact that __save_processor_state() has to save all CPU 
> registers referred to by the kernel in case a different kernel is used 
> to load and restore a hibernation image containing it.

ok - so i guess i should drop this from x86.git:

 @@ -53,7 +53,6 @@ void __save_processor_state(struct saved
         ctxt->cr2 = read_cr2();
         ctxt->cr3 = read_cr3();
         ctxt->cr4 = read_cr4();
 -       ctxt->cr8 = read_cr8();
  }

right? But i'm wondering - are we really ever resuming to a different 
kernel version, for this to be an issue?

	Ingo

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Ingo Molnar]

* Rafael J. Wysocki <rjw@sisk.pl> wrote:

> > But i'm wondering - are we really ever resuming to a different 
> > kernel version, for this to be an issue?
> 
> The boot kernel may be different from the kernel within the image, if 
> that's what you're asking for.

how different can it be, for resume to work? I mean, we'll have deeply 
kernel version dependent variables in RAM. Am i missing something 
obvious?

	Ingo

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Ingo Molnar]

* Rafael J. Wysocki <rjw@sisk.pl> wrote:

> > how different can it be, for resume to work? I mean, we'll have 
> > deeply kernel version dependent variables in RAM. Am i missing 
> > something obvious?
> 
> On x86-64 it can be almost totally different (by restoring a 
> hibernation image we replace the entire contents of RAM with almost no 
> constraints).
> 
> [Well, using a relocatable kernel for restoring an image with 
> nonrelocatable one or vice versa is rather not the best idea, but 
> everything else should work in theory.]
> 
> On i386 the boot kernel is still required to be the same as the one in 
> the image.

what's exactly in the hibernation image? Dirty data i suppose - but what 
about kernel-internal pages. What if we go from SLAB to SLUB? What if 
the size of a structure changes? Etc.

	Ingo

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Ingo Molnar]

* Rafael J. Wysocki <rjw@sisk.pl> wrote:

> > what's exactly in the hibernation image? Dirty data i suppose
> 
> No, everything, including the kernel code, page tables etc. :-)
> 
> > - but what about kernel-internal pages. What if we go from SLAB to 
> > SLUB? What if the size of a structure changes? Etc.
> 
> We can go from SLAB to SLUB just fine, it doesn't matter.  The only 
> thing that matters is we have to jump to the right address at the end 
> of core_restore_code() (defined in arch/x86/kernel/suspend_asm_64.S).

ok, just to make sure we are talking about the same thing. Do you mean 
we can restore an image saved by v2.6.12 into v2.6.24? I.e. a 2.6.24 
kernel will be able to run a 2.6.12 kernel's hibernation image, with all 
the kernel internal data from v2.6.12, etc? No way can that work.

	Ingo

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Rafael J. Wysocki]

On Sunday, 30 of December 2007, Ingo Molnar wrote:
> 
> * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> 
> > > what's exactly in the hibernation image? Dirty data i suppose
> > 
> > No, everything, including the kernel code, page tables etc. :-)
> > 
> > > - but what about kernel-internal pages. What if we go from SLAB to 
> > > SLUB? What if the size of a structure changes? Etc.
> > 
> > We can go from SLAB to SLUB just fine, it doesn't matter.  The only 
> > thing that matters is we have to jump to the right address at the end 
> > of core_restore_code() (defined in arch/x86/kernel/suspend_asm_64.S).
> 
> ok, just to make sure we are talking about the same thing. Do you mean 
> we can restore an image saved by v2.6.12 into v2.6.24? I.e. a 2.6.24 
> kernel will be able to run a 2.6.12 kernel's hibernation image, with all 
> the kernel internal data from v2.6.12, etc? No way can that work.

Well, not exactly.  The support for different boot and image kernels has
only been merged recently, but we can use the current git to restore
2.6.24-rc6-mm1, for example.

The trick is to pass a little additional information in the image header
that can be used by the boot kernel to locate the entry point to the image
kernel and the image kernel's page tables.

Rafael

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Torsten Kaiser]

On Dec 30, 2007 10:57 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Rafael J. Wysocki <rjw@sisk.pl> wrote:
>
> > > what's exactly in the hibernation image? Dirty data i suppose
> >
> > No, everything, including the kernel code, page tables etc. :-)
> >
> > > - but what about kernel-internal pages. What if we go from SLAB to
> > > SLUB? What if the size of a structure changes? Etc.
> >
> > We can go from SLAB to SLUB just fine, it doesn't matter.  The only
> > thing that matters is we have to jump to the right address at the end
> > of core_restore_code() (defined in arch/x86/kernel/suspend_asm_64.S).
>
> ok, just to make sure we are talking about the same thing. Do you mean
> we can restore an image saved by v2.6.12 into v2.6.24? I.e. a 2.6.24
> kernel will be able to run a 2.6.12 kernel's hibernation image, with all
> the kernel internal data from v2.6.12, etc? No way can that work.

I suspect here is a slight misunderstanding: It will never be possible
to switch a kernel with a hibernation/resume cycle.
The new thing is, that you can use a 2.6.24-SLUB-kernel to load a
2.6.12-SLAB-kernel(*) and then have the *2.6.12* kernel continue to
run.

The jump that Rafael talks about is a complete switchover from the
kernel doing the resume to the old kernel (and all of its data
structures) from the hibernation image. The now again running old
kernel will discard any internal data from the resuming kernel and so
it is not important if there was a SLAB vs SLUB difference.

In some way you could see the kernel loading the image as a gigantic
bootloader and just as is does not matter if you use lilo or grub it
should no longer matter what bootloader/kernel was used for resuming,
now that a ABI for the resume has been defined.

Torsten

(*): 2.6.12 might not work, as Rafael said the feature supporting
different kernel versions was only merge recently.

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Ingo Molnar]

* Rafael J. Wysocki <rjw@sisk.pl> wrote:

> > ok, just to make sure we are talking about the same thing. Do you 
> > mean we can restore an image saved by v2.6.12 into v2.6.24? I.e. a 
> > 2.6.24 kernel will be able to run a 2.6.12 kernel's hibernation 
> > image, with all the kernel internal data from v2.6.12, etc? No way 
> > can that work.
> 
> Well, not exactly.  The support for different boot and image kernels 
> has only been merged recently, but we can use the current git to 
> restore 2.6.24-rc6-mm1, for example.
> 
> The trick is to pass a little additional information in the image 
> header that can be used by the boot kernel to locate the entry point 
> to the image kernel and the image kernel's page tables.

ok - i thought you meant that there's a general capability to resume 
across kernel versions. (which would be close to impossible without some 
major surgery.)

btw., in what way is this different from kexec?

	Ingo

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Rafael J. Wysocki]

On Monday, 31 of December 2007, Ingo Molnar wrote:
> 
> * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> 
> > > ok, just to make sure we are talking about the same thing. Do you 
> > > mean we can restore an image saved by v2.6.12 into v2.6.24? I.e. a 
> > > 2.6.24 kernel will be able to run a 2.6.12 kernel's hibernation 
> > > image, with all the kernel internal data from v2.6.12, etc? No way 
> > > can that work.
> > 
> > Well, not exactly.  The support for different boot and image kernels 
> > has only been merged recently, but we can use the current git to 
> > restore 2.6.24-rc6-mm1, for example.
> > 
> > The trick is to pass a little additional information in the image 
> > header that can be used by the boot kernel to locate the entry point 
> > to the image kernel and the image kernel's page tables.
> 
> ok - i thought you meant that there's a general capability to resume 
> across kernel versions. (which would be close to impossible without some 
> major surgery.)

Well, there will be one. :-)  For example, one should be able to use a 2.6.25+
boot kernel to load the image containing 2.6.24-rc6 and restore the memory
state from it.

> btw., in what way is this different from kexec?

Not that much different indeed.  The hibernation code is more focused on
restoring the pre-hibernation state of the system rather than anything else,
plus on ACPI systems we try to handle the platform in accordance with the
specification (to some extent - you'd have to use a non-ACPI boot kernel to
follow the specification literally, which is possible but not straightforward).

Rafael

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Rafael J. Wysocki]

On Sunday, 30 of December 2007, Ingo Molnar wrote:
> 
> * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> 
> > > how different can it be, for resume to work? I mean, we'll have 
> > > deeply kernel version dependent variables in RAM. Am i missing 
> > > something obvious?
> > 
> > On x86-64 it can be almost totally different (by restoring a 
> > hibernation image we replace the entire contents of RAM with almost no 
> > constraints).
> > 
> > [Well, using a relocatable kernel for restoring an image with 
> > nonrelocatable one or vice versa is rather not the best idea, but 
> > everything else should work in theory.]
> > 
> > On i386 the boot kernel is still required to be the same as the one in 
> > the image.
> 
> what's exactly in the hibernation image? Dirty data i suppose

No, everything, including the kernel code, page tables etc. :-)

> - but what about kernel-internal pages. What if we go from SLAB to SLUB?
> What if the size of a structure changes? Etc.

We can go from SLAB to SLUB just fine, it doesn't matter.  The only thing
that matters is we have to jump to the right address at the end of
core_restore_code() (defined in arch/x86/kernel/suspend_asm_64.S).

Rafael

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Rafael J. Wysocki]

On Sunday, 30 of December 2007, Ingo Molnar wrote:
> 
> * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> 
> > > But i'm wondering - are we really ever resuming to a different 
> > > kernel version, for this to be an issue?
> > 
> > The boot kernel may be different from the kernel within the image, if 
> > that's what you're asking for.
> 
> how different can it be, for resume to work? I mean, we'll have deeply 
> kernel version dependent variables in RAM. Am i missing something 
> obvious?

On x86-64 it can be almost totally different (by restoring a hibernation image
we replace the entire contents of RAM with almost no constraints).

[Well, using a relocatable kernel for restoring an image with nonrelocatable one
or vice versa is rather not the best idea, but everything else should work in
theory.]

On i386 the boot kernel is still required to be the same as the one in the
image.

Greetings,
Rafael

Re: [PATCH] Hibernation: Document __save_processor_state() on x86-64

[Rafael J. Wysocki]

On Sunday, 30 of December 2007, Ingo Molnar wrote:
> 
> * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Document the fact that __save_processor_state() has to save all CPU 
> > registers referred to by the kernel in case a different kernel is used 
> > to load and restore a hibernation image containing it.
> 
> ok - so i guess i should drop this from x86.git:
> 
>  @@ -53,7 +53,6 @@ void __save_processor_state(struct saved
>          ctxt->cr2 = read_cr2();
>          ctxt->cr3 = read_cr3();
>          ctxt->cr4 = read_cr4();
>  -       ctxt->cr8 = read_cr8();
>   }
> 
> right?

Well, if cr8 is used for anything that matters, then yes.

> But i'm wondering - are we really ever resuming to a different  
> kernel version, for this to be an issue?

The boot kernel may be different from the kernel within the image, if that's
what you're asking for.

Rafael