linux-kernel - Re: [PATCH] i386: do a global tlb flush in S4 resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201003052155.26292.rjw@sisk.pl>
Date:	Fri, 5 Mar 2010 21:55:26 +0100
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Shaohua Li <shaohua.li@...el.com>
Cc:	"H. Peter Anvin" <hpa@...or.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"mingo@...e.hu" <mingo@...e.hu>,
	"colin.king@...onical.com" <colin.king@...onical.com>
Subject: Re: [PATCH] i386: do a global tlb flush in S4 resume

On Friday 05 March 2010, Shaohua Li wrote:
> On Fri, Mar 05, 2010 at 03:49:46AM +0800, Rafael J. Wysocki wrote:
> > On Thursday 04 March 2010, Shaohua Li wrote:
...
> > 
> > In that case please also remove the turning GPE off down the road.
> 
>     i386: do a global tlb flush in S4 resume
> 
>     Colin reported a strange oops in S4 resume code path (see below). The test
>     system has i5/i7 CPU. The kernel doesn't open PAE, so 4M page table is used.
>     The oops always happen a virtual address 0xc03ff000, which is mapped to the
>     last 4k of first 4M memory. Doing a global tlb flush fixes the issue.
> 
>     EIP: 0060:[<c0493a01>] EFLAGS: 00010086 CPU: 0
>     EIP is at copy_loop+0xe/0x15
>     EAX: 36aeb000 EBX: 00000000 ECX: 00000400 EDX: f55ad46c
>     ESI: 0f800000 EDI: c03ff000 EBP: f67fbec4 ESP: f67fbea8
>      DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
>     ...
>     ...
>     CR2: 00000000c03ff000
> 
>     Tested-by: Colin Ian King <colin.king@...onical.com>
>     Signed-off-by: Shaohua Li <shaohua.li@...el.com>
> 
> diff --git a/arch/x86/power/hibernate_asm_32.S b/arch/x86/power/hibernate_asm_32.S
> index b641388..ad47dae 100644
> --- a/arch/x86/power/hibernate_asm_32.S
> +++ b/arch/x86/power/hibernate_asm_32.S
> @@ -27,10 +27,17 @@ ENTRY(swsusp_arch_suspend)
>  	ret
>  
>  ENTRY(restore_image)
> +	movl	mmu_cr4_features, %ecx
>  	movl	resume_pg_dir, %eax
>  	subl	$__PAGE_OFFSET, %eax
>  	movl	%eax, %cr3
>  
> +	jecxz	1f	# cr4 Pentium and higher, skip if zero
> +	andl	$~(X86_CR4_PGE), %ecx
> +	movl	%ecx, %cr4;  # turn off PGE
> +	movl	%cr3, %eax;  # flush TLB
> +	movl	%eax, %cr3
> +1:
>  	movl	restore_pblist, %edx
>  	.p2align 4,,7
>  
> @@ -54,16 +61,8 @@ done:
>  	movl	$swapper_pg_dir, %eax
>  	subl	$__PAGE_OFFSET, %eax
>  	movl	%eax, %cr3
> -	/* Flush TLB, including "global" things (vmalloc) */
>  	movl	mmu_cr4_features, %ecx
>  	jecxz	1f	# cr4 Pentium and higher, skip if zero
> -	movl	%ecx, %edx
> -	andl	$~(X86_CR4_PGE), %edx
> -	movl	%edx, %cr4;  # turn off PGE
> -1:
> -	movl	%cr3, %eax;  # flush TLB
> -	movl	%eax, %cr3

Now that's too much removed.  We actually _want_ to do the TLB flush here,
but not unset PGE, because it's been unset already.

So, the above two lines should not be removed.

Also, I'd remove the first jecxz and keep the second one.

> -	jecxz	1f	# cr4 Pentium and higher, skip if zero
>  	movl	%ecx, %cr4;  # turn PGE back on
>  1:

Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/