linux-kernel - Re: [PATCH 02/10] x86/entry/compat: In SYSENTER, sink AC clearing below the existing FLAGS test

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: <20160229203941.GG3724@pd.tnic>
Date:	Mon, 29 Feb 2016 21:39:41 +0100
From:	Borislav Petkov <bp@...en8.de>
To:	Andy Lutomirski <luto@...nel.org>
Cc:	x86@...nel.org, linux-kernel@...r.kernel.org,
	Oleg Nesterov <oleg@...hat.com>,
	Andrew Cooper <andrew.cooper3@...rix.com>,
	Brian Gerst <brgerst@...il.com>
Subject: Re: [PATCH 02/10] x86/entry/compat: In SYSENTER, sink AC clearing
 below the existing FLAGS test

On Sun, Feb 28, 2016 at 09:28:47PM -0800, Andy Lutomirski wrote:
> CLAC is slow, and the SYSENTER code already has an unlikely path
> that runs if unusual flags are set.  Drop the CLAC and instead rely
> on the unlikely path to clear AC.
> 
> This seems to save ~24 cycles on my Skylake laptop.  (Hey, Intel,
> make this faster please!)
> 
> Signed-off-by: Andy Lutomirski <luto@...nel.org>
> ---
>  arch/x86/entry/entry_64_compat.S | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
> index 89bcb4979e7a..7c8e72da7654 100644
> --- a/arch/x86/entry/entry_64_compat.S
> +++ b/arch/x86/entry/entry_64_compat.S
> @@ -66,8 +66,6 @@ ENTRY(entry_SYSENTER_compat)
>  	 */
>  	pushfq				/* pt_regs->flags (except IF = 0) */
>  	orl	$X86_EFLAGS_IF, (%rsp)	/* Fix saved flags */
> -	ASM_CLAC			/* Clear AC after saving FLAGS */
> -
>  	pushq	$__USER32_CS		/* pt_regs->cs */
>  	xorq    %r8,%r8
>  	pushq	%r8			/* pt_regs->ip = 0 (placeholder) */
> @@ -90,9 +88,9 @@ ENTRY(entry_SYSENTER_compat)
>  	cld
>  
>  	/*
> -	 * Sysenter doesn't filter flags, so we need to clear NT
> +	 * Sysenter doesn't filter flags, so we need to clear NT and AC
>  	 * ourselves.  To save a few cycles, we can check whether
> -	 * NT was set instead of doing an unconditional popfq.
> +	 * either was set instead of doing an unconditional popfq.
>  	 * This needs to happen before enabling interrupts so that
>  	 * we don't get preempted with NT set.
>  	 *
> @@ -102,7 +100,7 @@ ENTRY(entry_SYSENTER_compat)
>  	 * we're keeping that code behind a branch which will predict as
>  	 * not-taken and therefore its instructions won't be fetched.
>  	 */
> -	testl	$X86_EFLAGS_NT, EFLAGS(%rsp)
> +	testl	$X86_EFLAGS_NT|X86_EFLAGS_AC, EFLAGS(%rsp)
>  	jnz	.Lsysenter_fix_flags
>  .Lsysenter_flags_fixed:

Do I see it correctly that with this change, that .Lsysenter_fix_flags:
is going to be visited each time on SMAP machines and then we can get
rid of it? The reason for it was not to pollute I$ as the comment says
but that happening now anyway...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.