Re: [PATCH] xen: fix s3 resume on AMD CPUs

[Christoph Egger <Christoph.Egger@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 951 bytes --]

On Friday 05 June 2009 10:21:26 Keir Fraser wrote:
> On 05/06/2009 09:00, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > attached patch fixes S3 resume on AMD CPUs.
> >
> > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
>
> Please make a minimal patch and explain each individual change. This one
> has bizarre additional alignment, nops, double short jumps,
> longjmp->shortjmp but keeps a comment which is then incorrect. Why are they
> needed? Is the need documented? I don't see anything similar in Linux
> 2.6.27 wakeup routines (2.6.27 is what I happen to have to hand).
>
> I won't take your random permutations on this file of all files especially,
> since it is a pain in the arse to debug when it gets broken.

Attached patch adds a few comments, turns a long jump into a short jump
to avoid a #GP.
Reload %cs via lretq right after stack pointer has been restored.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

[-- Attachment #2: xen_s3.diff --]
[-- Type: text/x-diff, Size: 1550 bytes --]

diff -r 112680f620bf xen/arch/x86/acpi/wakeup_prot.S
--- a/xen/arch/x86/acpi/wakeup_prot.S	Mon Jun 08 18:23:57 2009 +0100
+++ b/xen/arch/x86/acpi/wakeup_prot.S	Wed Jun 10 15:15:16 2009 +0200
@@ -142,6 +142,12 @@ __ret_point:
         LOAD_GREG(sp)
 
 #if defined(__x86_64__)
+	/* Reload code selector */
+	pushq	$(__HYPERVISOR_CS64)
+	leaq	1f(%rip),%rax
+	pushq	%rax
+	lretq
+1:
 
         mov     REF(saved_cr8), %rax
         mov     %rax, %cr8
diff -r 112680f620bf xen/arch/x86/boot/wakeup.S
--- a/xen/arch/x86/boot/wakeup.S	Mon Jun 08 18:23:57 2009 +0100
+++ b/xen/arch/x86/boot/wakeup.S	Wed Jun 10 15:15:16 2009 +0200
@@ -110,6 +110,7 @@ video_flags:    .long 0
         # Add offset for any reference to xen specific symbols
 
 wakeup_32:
+        /* Set up segment registers and initial stack for protected mode */
         mov     $BOOT_DS, %eax
         mov     %eax, %ds
         mov     %eax, %ss
@@ -152,8 +153,10 @@ 1:      wrmsr
 
         wbinvd
 
+        /* Enable paging */
         mov     $0x80050033,%eax /* hi-to-lo: PG,AM,WP,NE,ET,MP,PE */
         mov     %eax,%cr0
+        /* Flush prefetch queue */
         jmp     1f
 1:
 
@@ -174,8 +177,11 @@ wakeup_64:
         mov     $(__HYPERVISOR_DS64), %eax
         mov     %eax, %ds
 
-        # long jump to return point, with cs reload
-        rex64 ljmp    *ret_point(%rip)
+        /* Continue with wakeup in the high-level wakeup code.
+         * Reload cs there.
+         */
+        movq    ret_point(%rip), %rbx
+        jmp     *%rbx
 
         .align 8
 ret_point:

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel