Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934831AbeAHRL1 (ORCPT + 1 other); Mon, 8 Jan 2018 12:11:27 -0500 Received: from mail-wr0-f194.google.com ([209.85.128.194]:37482 "EHLO mail-wr0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932188AbeAHRL0 (ORCPT ); Mon, 8 Jan 2018 12:11:26 -0500 X-Google-Smtp-Source: ACJfBovPqLGymazCihp4RQa1ZBUqoLOtmZDsbWrpyNPHr3x1n6ADlUQlLcya6pBbj0EUV/BbkHozMw== Date: Mon, 8 Jan 2018 18:11:22 +0100 From: Ingo Molnar To: Willy Tarreau Cc: linux-kernel@vger.kernel.org, x86@kernel.org, tglx@linutronix.de, gnomes@lxorguk.ukuu.org.uk, torvalds@linux-foundation.org, Andy Lutomirski , Borislav Petkov , Peter Zijlstra , Josh Poimboeuf , Dave Hansen Subject: Re: [PATCH RFC 4/4] x86/entry/pti: don't switch PGD on tasks holding flag TIF_NOPTI Message-ID: <20180108171122.iktxeqioidlz33eq@gmail.com> References: <1515427939-10999-1-git-send-email-w@1wt.eu> <1515427939-10999-5-git-send-email-w@1wt.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1515427939-10999-5-git-send-email-w@1wt.eu> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: * Willy Tarreau wrote: > If a task has the TIF_NOPTI flag set, it doesn't want to experience > page table isolation. In this case, returns from kernel to user will > not switch the CR3, leaving it to the kernel one which already maps > both user and kernel pages. Upon entry in the kernel, we can't check > this flag so we simply check if CR3 was pointing to the kernel's PGD, > indicating an earlier absence of switch, and in this case we don't > change it. > > Thanks to these changes, haproxy running under KVM went back from > 12400 conn/s to 21000 once loaded after calling prctl(). > > Signed-off-by: Willy Tarreau > --- > arch/x86/entry/calling.h | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) Just a few nits: > > diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h > index 45a63e0..054b8b7 100644 > --- a/arch/x86/entry/calling.h > +++ b/arch/x86/entry/calling.h > @@ -1,5 +1,6 @@ > /* SPDX-License-Identifier: GPL-2.0 */ > #include > +#include > #include > #include > #include > @@ -214,6 +215,11 @@ > .macro SWITCH_TO_KERNEL_CR3 scratch_reg:req > ALTERNATIVE "jmp .Lend_\@", "", X86_FEATURE_PTI > mov %cr3, \scratch_reg > + > + /* if we're already on the kernel PGD, we don't switch */ > + testq $(PTI_SWITCH_PGTABLES_MASK), \scratch_reg > + jz .Lend_\@ > + > ADJUST_KERNEL_CR3 \scratch_reg > mov \scratch_reg, %cr3 > .Lend_\@: > @@ -224,6 +230,12 @@ > > .macro SWITCH_TO_USER_CR3_NOSTACK scratch_reg:req scratch_reg2:req > ALTERNATIVE "jmp .Lend_\@", "", X86_FEATURE_PTI > + > + /* "NOPTI" taskflag avoids the switch */ > + movq PER_CPU_VAR(current_task), \scratch_reg > + btq $TIF_NOPTI, TASK_TI_flags(\scratch_reg) > + jc .Lend_\@ s/"NOPTI" taskflag /The "NOPTI" task flag > + > mov %cr3, \scratch_reg > > ALTERNATIVE "jmp .Lwrcr3_\@", "", X86_FEATURE_PCID > @@ -262,6 +274,13 @@ > ALTERNATIVE "jmp .Ldone_\@", "", X86_FEATURE_PTI > movq %cr3, \scratch_reg > movq \scratch_reg, \save_reg > + > + /* if we're already on the kernel PGD, we don't switch, > + * we just save the current cr3. > + */ > + testq $(PTI_SWITCH_PGTABLES_MASK), \scratch_reg > + jz .Ldone_\@ Proper kernel comment style please. Also: s/cr3/CR3 > + > /* > * Is the "switch mask" all zero? That means that both of > * these are zero: > @@ -284,6 +303,10 @@ > .macro RESTORE_CR3 scratch_reg:req save_reg:req > ALTERNATIVE "jmp .Lend_\@", "", X86_FEATURE_PTI > > + /* if we saved a kernel context, we didn't switch so we don't switch */ > + testq $(PTI_SWITCH_PGTABLES_MASK), \save_reg > + jz .Lend_\@ Maybe clarify this a bit? How about: /* * If we saved a kernel context on entry, we didn't switch the CR3, * so we don't need to restore it on the way out either: */ or so? Thanks, Ingo