From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752918AbbGNX2h (ORCPT ); Tue, 14 Jul 2015 19:28:37 -0400 Received: from mail-wi0-f177.google.com ([209.85.212.177]:37932 "EHLO mail-wi0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751818AbbGNX2f (ORCPT ); Tue, 14 Jul 2015 19:28:35 -0400 Date: Wed, 15 Jul 2015 01:28:31 +0200 From: Frederic Weisbecker To: Andy Lutomirski Cc: Brian Gerst , "linux-kernel@vger.kernel.org" , Peter Zijlstra , Ingo Molnar , Denys Vlasenko , Andrew Lutomirski , "H. Peter Anvin" , Denys Vlasenko , Rik van Riel , Linus Torvalds , Borislav Petkov , Thomas Gleixner , Oleg Nesterov , Kees Cook , "linux-tip-commits@vger.kernel.org" Subject: Re: [tip:x86/asm] x86/entry: Add enter_from_user_mode() and use it in syscalls Message-ID: <20150714232831.GF29441@lerouge> References: <853b42420066ec3fb856779cdc223a6dcb5d355b.1435952415.git.luto@kernel.org> <20150714230057.GC29441@lerouge> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jul 14, 2015 at 04:04:47PM -0700, Andy Lutomirski wrote: > On Tue, Jul 14, 2015 at 4:00 PM, Frederic Weisbecker wrote: > > On Tue, Jul 07, 2015 at 03:51:29AM -0700, tip-bot for Andy Lutomirski wrote: > >> Commit-ID: feed36cde0a10adb957445a37e48f957f30b2273 > >> Gitweb: http://git.kernel.org/tip/feed36cde0a10adb957445a37e48f957f30b2273 > >> Author: Andy Lutomirski > >> AuthorDate: Fri, 3 Jul 2015 12:44:25 -0700 > >> Committer: Ingo Molnar > >> CommitDate: Tue, 7 Jul 2015 10:59:06 +0200 > >> > >> x86/entry: Add enter_from_user_mode() and use it in syscalls > >> > >> Changing the x86 context tracking hooks is dangerous because > >> there are no good checks that we track our context correctly. > >> Add a helper to check that we're actually in CONTEXT_USER when > >> we enter from user mode and wire it up for syscall entries. > >> > >> Subsequent patches will wire this up for all non-NMI entries as > >> well. NMIs are their own special beast and cannot currently > >> switch overall context tracking state. Instead, they have their > >> own special RCU hooks. > >> > >> This is a tiny speedup if !CONFIG_CONTEXT_TRACKING (removes a > >> branch) and a tiny slowdown if CONFIG_CONTEXT_TRACING (adds a > >> layer of indirection). Eventually, we should fix up the core > >> context tracking code to supply a function that does what we > >> want (and can be much simpler than user_exit), which will enable > >> us to get rid of the extra call. > >> > >> Signed-off-by: Andy Lutomirski > >> Cc: Andy Lutomirski > >> Cc: Borislav Petkov > >> Cc: Brian Gerst > >> Cc: Denys Vlasenko > >> Cc: Denys Vlasenko > >> Cc: Frederic Weisbecker > >> Cc: H. Peter Anvin > >> Cc: Kees Cook > >> Cc: Linus Torvalds > >> Cc: Oleg Nesterov > >> Cc: Peter Zijlstra > >> Cc: Rik van Riel > >> Cc: Thomas Gleixner > >> Cc: paulmck@linux.vnet.ibm.com > >> Link: http://lkml.kernel.org/r/853b42420066ec3fb856779cdc223a6dcb5d355b.1435952415.git.luto@kernel.org > >> Signed-off-by: Ingo Molnar > >> --- > >> arch/x86/entry/common.c | 13 ++++++++++++- > >> 1 file changed, 12 insertions(+), 1 deletion(-) > >> > >> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c > >> index 917d0c3..9a327ee 100644 > >> --- a/arch/x86/entry/common.c > >> +++ b/arch/x86/entry/common.c > >> @@ -28,6 +28,15 @@ > >> #define CREATE_TRACE_POINTS > >> #include > >> > >> +#ifdef CONFIG_CONTEXT_TRACKING > >> +/* Called on entry from user mode with IRQs off. */ > >> +__visible void enter_from_user_mode(void) > >> +{ > >> + CT_WARN_ON(ct_state() != CONTEXT_USER); > >> + user_exit(); > >> +} > >> +#endif > >> + > >> static void do_audit_syscall_entry(struct pt_regs *regs, u32 arch) > >> { > >> #ifdef CONFIG_X86_64 > >> @@ -65,14 +74,16 @@ unsigned long syscall_trace_enter_phase1(struct pt_regs *regs, u32 arch) > >> work = ACCESS_ONCE(current_thread_info()->flags) & > >> _TIF_WORK_SYSCALL_ENTRY; > >> > >> +#ifdef CONFIG_CONTEXT_TRACKING > >> /* > >> * If TIF_NOHZ is set, we are required to call user_exit() before > >> * doing anything that could touch RCU. > >> */ > >> if (work & _TIF_NOHZ) { > >> - user_exit(); > >> + enter_from_user_mode(); > >> work &= ~_TIF_NOHZ; > > > > We should move the sanity check to user_exit/enter() and use user_exit/enter() > > only when we actually enter/exit user. > > I agree, but I don't know what other arches to. Right, I'll need to check that carefully, once I fully understand your patchset. > > > Here it's the case but syscall_trace_leave() > > and do_notify_resume() are special case that should probably use exception_enter/exit() > > unless your patchset have changed things such that there is only one call to user_exit() > > once we completed everything before resuming userspace. I need to review the rest of > > the patchset to discover that :-) > > syscall_trace_leave and do_notify_resume may be so screwed up that > your suggestion wouldn't even work. However, the next set of patches > (out for review but currently stalled pending Brian Gerst's vm86 work) > remove those functions entirely. Ok so I'm probably confused. I need to check the resulting code. > > --Andy