Re: [RFC][PATCH -mm 1/4] Hibernation: Arbitrary boot kernel support on x86_64

[Pavel Machek <pavel@ucw.cz>]

Hi!

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Make it possible to restore a hibernation image on x86_64 with the help of a
> kernel different from the one in the image.

Looks mostly ok to me.

Should this be split in half (generic support, x86-64 support) and be
done last in the series, so that suspend still works when bisecting?

>  done:
> +	/* jump to the restore_registers address from the image header */
> +	jmpq	*%rax

So this is where the change from kernel text 1 to kernel text 2
happens, right? I see you are using %r10 for something here, perhaps
that should be commented?

> +.balign PAGE_SIZE
> +ENTRY(restore_registers)
> +	/* we are in the image kernel's text now */
> +	testq	%r10, %r10
> +	jz	1f
> +	/* copy the skipped page */
> +	movq	%r10, %rsi
> +	movq	%r9, %rdi
> +	movq	$(PAGE_SIZE >> 3), %rcx
> +	rep
> +	movsq

> @@ -84,10 +127,7 @@ done:
>  	movq	%rcx, %cr3
>  	movq	%rax, %cr4;  # turn PGE back on
>  
> -	movl	$24, %eax
> -	movl	%eax, %ds
> -
> -	/* We don't restore %rax, it must be 0 anyway */
> +	/* restore GPRs (we don't restore %rax, it must be 0 anyway) */
>  	movq	$saved_context, %rax
>  	movq	pt_regs_rsp(%rax), %rsp
>  	movq	pt_regs_rbp(%rax), %rbp

Hmm, in the old code, we knew we don't have to restore %ds, because it
is constant for one kernel. Now, we rely on %ds being constant accross
kernels. Not nice, and should be at least documented.

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