On 05-02 02:45, Rafael J. Wysocki wrote: > On Wednesday, May 01, 2013 11:55:10 AM H. Peter Anvin wrote: > > On 05/01/2013 11:51 AM, Jonas Heinrich wrote: > > > Well, you could give me instructions on how to debug this (I'll do > > > everything ;)) or I could ship you the Thinkpad T43. I guess this > > > would worth the effort since this bug is somehow critical. > > > > > > Best regards, Jonas > > > > I'll put together a debug patch unless I can trick Rafael into doing > > it first... > > I'm afraid that code has changed quite a bit since I looked at it last time. > [Jarkko Sakkinen seems to have worked on it lately, CCed.] > > Jonas, I wonder what happens if you drop the first hunk of the patch (it just > uses a different register, which shouldn't matter)? Does it still help then?
Hello Rafel, first of all, thank you for helping me out :)
You're right, the patch still solves the suspend bug, after removing the first
hunk of the patch and applying it (see attachement:
suspendfix_first_hunk_dropped.patch).
>
> If so, there are still a few things you can do to it, e.g:
> (1) drop the
>
> - btl $WAKEUP_BEHAVIOR_RESTORE_CR4, %edi
> - jnc 1f
>
Still works :) (used suspendfix_1.patch)
> lines,
> (2) drop the
>
> - btl $WAKEUP_BEHAVIOR_RESTORE_EFER, %edi
> - jnc 1f
>
> lines,
Still works :) (used suspendfix_2.patch)
> (3) drop the
>
> + jecxz 1f
>
Still works :) (used suspendfix_3.patch)
> line,
> (4) drop the
>
> + movl %eax, %ecx
> + orl %edx, %ecx
> + jz 1f
>
At this point, the bug reoccurs (used suspendfix_4.patch)!
But that doesn't mean these lines are the only critical, because the more
minimal patch
@@ -119,6 +119,9 @@
jnc 1f
movl pmode_efer, %eax
movl pmode_efer + 4, %edx
+ movl %eax, %ecx
+ orl %edx, %ecx
+ jz 1f
movl $MSR_EFER, %ecx
wrmsr
1:
with removing this part
- movl pmode_cr4, %eax
- movl %eax, %cr4
+ movl pmode_cr4, %ecx
+ movl %ecx, %cr4
also doesn't fix the issue (see suspendfix_5.patch).
> lines and see what the minimal patch needed for things to work again is.
>
So the most minimal working patch is suspendfix_3.patch.
> Thanks,
> Rafael
Thank you and best regards,
Jonas
>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
--- a/arch/x86/realmode/rm/wakeup_asm.S 2013-04-29 02:36:01.000000000 +0200
+++ b/arch/x86/realmode/rm/wakeup_asm.S 2013-05-01 11:42:07.359999895 +0200
@@ -110,15 +110,15 @@
movl pmode_cr3, %eax
movl %eax, %cr3
- btl $WAKEUP_BEHAVIOR_RESTORE_CR4, %edi
- jnc 1f
- movl pmode_cr4, %eax
- movl %eax, %cr4
+ movl pmode_cr4, %ecx
+ jecxz 1f
+ movl %ecx, %cr4
1:
- btl $WAKEUP_BEHAVIOR_RESTORE_EFER, %edi
- jnc 1f
movl pmode_efer, %eax
movl pmode_efer + 4, %edx
+ movl %eax, %ecx
+ orl %edx, %ecx
+ jz 1f
movl $MSR_EFER, %ecx
wrmsr
1:
--- a/arch/x86/realmode/rm/wakeup_asm.S 2013-05-02 17:20:25.266667541 +0200
+++ b/arch/x86/realmode/rm/wakeup_asm.S 2013-05-02 17:22:51.880000874 +0200
@@ -112,13 +112,15 @@
btl $WAKEUP_BEHAVIOR_RESTORE_CR4, %edi
jnc 1f
- movl pmode_cr4, %eax
- movl %eax, %cr4
+ movl pmode_cr4, %ecx
+ jecxz 1f
+ movl %ecx, %cr4
1:
- btl $WAKEUP_BEHAVIOR_RESTORE_EFER, %edi
- jnc 1f
movl pmode_efer, %eax
movl pmode_efer + 4, %edx
+ movl %eax, %ecx
+ orl %edx, %ecx
+ jz 1f
movl $MSR_EFER, %ecx
wrmsr
1:
--- a/arch/x86/realmode/rm/wakeup_asm.S 2013-05-02 17:20:25.266667541 +0200
+++ b/arch/x86/realmode/rm/wakeup_asm.S 2013-05-02 18:54:49.176667541 +0200
@@ -112,13 +112,17 @@
btl $WAKEUP_BEHAVIOR_RESTORE_CR4, %edi
jnc 1f
- movl pmode_cr4, %eax
- movl %eax, %cr4
+ movl pmode_cr4, %ecx
+ jecxz 1f
+ movl %ecx, %cr4
1:
btl $WAKEUP_BEHAVIOR_RESTORE_EFER, %edi
jnc 1f
movl pmode_efer, %eax
movl pmode_efer + 4, %edx
+ movl %eax, %ecx
+ orl %edx, %ecx
+ jz 1f
movl $MSR_EFER, %ecx
wrmsr
1:
--- a/arch/x86/realmode/rm/wakeup_asm.S.org 2013-05-02 17:20:25.266667541
+0200
+++ b/arch/x86/realmode/rm/wakeup_asm.S 2013-05-02 19:56:58.083334207 +0200
@@ -112,13 +112,16 @@
btl $WAKEUP_BEHAVIOR_RESTORE_CR4, %edi
jnc 1f
- movl pmode_cr4, %eax
- movl %eax, %cr4
+ movl pmode_cr4, %ecx
+ movl %ecx, %cr4
1:
btl $WAKEUP_BEHAVIOR_RESTORE_EFER, %edi
jnc 1f
movl pmode_efer, %eax
movl pmode_efer + 4, %edx
+ movl %eax, %ecx
+ orl %edx, %ecx
+ jz 1f
movl $MSR_EFER, %ecx
wrmsr
1:
--- a/arch/x86/realmode/rm/wakeup_asm.S 2013-05-02 17:20:25.266667541 +0200
+++ b/arch/x86/realmode/rm/wakeup_asm.S 2013-05-02 20:43:40.623334207 +0200
@@ -112,8 +112,8 @@
btl $WAKEUP_BEHAVIOR_RESTORE_CR4, %edi
jnc 1f
- movl pmode_cr4, %eax
- movl %eax, %cr4
+ movl pmode_cr4, %ecx
+ movl %ecx, %cr4
1:
btl $WAKEUP_BEHAVIOR_RESTORE_EFER, %edi
jnc 1f
--- a/arch/x86/realmode/rm/wakeup_asm.S 2013-05-02 17:20:25.266667541 +0200
+++ b/arch/x86/realmode/rm/wakeup_asm.S 2013-05-02 21:44:57.973334207 +0200
@@ -119,6 +119,9 @@
jnc 1f
movl pmode_efer, %eax
movl pmode_efer + 4, %edx
+ movl %eax, %ecx
+ orl %edx, %ecx
+ jz 1f
movl $MSR_EFER, %ecx
wrmsr
1:
pgpYC6YmHaKpo.pgp
Description: PGP signature
