LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>,
	Rik van Riel <riel@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Brian Gerst <brgerst@gmail.com>,
	Denys Vlasenko <vda.linux@googlemail.com>,
	Kees Cook <keescook@chromium.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Oleg Nesterov <oleg@redhat.com>,
	Andrew Lutomirski <luto@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-tip-commits@vger.kernel.org" 
	<linux-tip-commits@vger.kernel.org>
Subject: Re: [tip:x86/asm] x86/asm/entry/64: Migrate error and IRQ exit work to C and remove old assembly code
Date: Tue, 18 Aug 2015 15:40:20 -0700
Message-ID: <CALCETrVD1EyH075Lwja4rCLg9OXSB7ZgQtP_5ZwFypf3b9q9sQ@mail.gmail.com> (raw)
In-Reply-To: <20150818223409.GB12858@lerouge>

On Tue, Aug 18, 2015 at 3:34 PM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Wed, Aug 12, 2015 at 07:59:44AM -0700, Andy Lutomirski wrote:
>> On Wed, Aug 12, 2015 at 6:32 AM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
>> > Right, and doing it the way we did previously was safe wrt. that.
>> >
>> > Can't we have exceptions slow path just like the way we do it in syscalls?
>> >
>> > Then the exception slow path would just do:
>> >
>> >     if TIF_NOHZ
>> >        ctx = exception_enter()
>> >     exception_handler()
>> >     if TIF_NOHZ
>> >        exception_exit(ctx)
>>
>> What's the purpose of TIF_NOHZ right now?  For syscalls, it makes
>> sense, but is there any case in which TIF_NOHZ is set on one CPU but
>> not on another CPU?  It might make sense to get the performance back
>> using static keys instead of TIF_NOHZ.
>
> Sure if we can manage to do that. The nice thing about TIF flags is that
> they are a single check that is always there.
>

True, although my patch loses that benefit for the fast compat entries
due to the syscall arg fault stuff (what a mess!).

>>
>> If we switched back to exception_enter, we'd have to remember the
>> previous state, and, with a single exception right now, I think that's
>> unnecessary.
>>
>> I think there are only three states we can be in at exception entry:
>> user (and user_mode(regs)), kernel (and kernel_mode(regs)), or
>> NMI-like.
>
> But we can have user && (!user_mode(regs)) if exception happens on exception
> entry code.

I sure hope not, unless it nests inside an NMI-like thing.  It's
conceivable that this might happen due to perf NMIs causing a failed
MSR read or similar.  We might need to relax the assertions to check
that we're either in kernel or NMI context.  If so, that's
straightforward.  Meanwhile no one has reported this happening.

>
>> In the user case, the new code is correct.  In the kernel
>> case, the new code is also correct.  In the NMI case (if we're nested
>> in an NMI or similar entry)) then it is and was the responsibility of
>> the NMI-like entry to call rcu_nmi_enter(), and things that nest
>> inside that shouldn't touch context tracking (with the possible
>> exception of calling rcu_nmi_enter() again).
>>
>> In current -tip, there's a slight hole in this due to syscalls, and I'll fix it.
>
> There must be a check for context tracking enabled anyway. So why can't
> we just just do in exception entry code:
>
>        if (exception_slow_path()) {
>            exception_enter()
>            exception_handler()
>            exception_exit()
>        } else {
>            normal stuff
>        }
>
> Especially if we can manage to implement static keys in ASM, this will sum up to
> a single one.

There isn't really an exception slow path.  There's already a branch
for user vs kernel (in the CPL sense), and with my patches, there's no
additional branch for previous context tracking state.

>
>> >> The latter is annoying, but the entry code needs to deal with it
>> >> anyway.  For example, any exception early in NMI is currently really
>> >> bad.  Non-IST exceptions very early in SYSCALL are fatal.
>> >> Non-paranoid exceptions outside swapgs are fatal.  Etc.
>> >
>> > Sure but that doesn't mean I'm happy with introducing new fragile path
>> > like those. Especially as we have a way to fix without more overhead.
>>
>> I think my approach can work with even less overhead: there are fewer
>> branches due to checking the previous state.
>>
>> >> > Also as long as there is at least one instruction between entry to the kernel
>> >> > and context tracking noting it, there is a risk for an exception. Hence entry
>> >> > code will never be atomic enough to avoid this kind of bugs.
>> >>
>> >> By that argument, we're doomed.  Non-IST exceptions outside swapgs are fatal.
>> >
>> > Does that concern only error_entry() exceptions?
>>
>> Yes, but the set of paranoid_entry exceptions is shrinking.  In -tip, there are:
>>
>> NMI: NMI is special and will call rcu_nmi_enter().  Nothing's changing here.
>>
>> MCE: Once upon a time, MCE was simply buggy.  As of 4.0 (IIRC) MCE
>> from kernel mode calls rcu_nmi_enter().
>>
>> BP: This is going away, I think.  #BP should stop being special by 4.4.
>>
>> DB: That's the only weird case.  Patches to prevent instruction
>> breakpoints in entry code are already in -tip.  The only thing left is
>> kernel watchpoints, and we need to do something about that.
>
> So now we can't set a breakpoint on syscall entry anymore?
>
> I'm still nervous with all that.

We haven't done anything that would make breakpoints on syscall entry
less safe than they were, but we now disallow the breakpoints.  In the
future, we might take advantage of that change.

-- 
Andy Lutomirski
AMA Capital Management, LLC

  reply index

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-03 19:44 [PATCH v5 00/17] x86: Rewrite exit-to-userspace code Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 01/17] selftests/x86: Add a test for 32-bit fast syscall arg faults Andy Lutomirski
2015-07-07 10:49   ` [tip:x86/asm] x86/entry, " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 02/17] x86/entry/64/compat: Fix bad fast syscall arg failure path Andy Lutomirski
2015-07-07 10:49   ` [tip:x86/asm] " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 03/17] uml: Fix do_signal() prototype Andy Lutomirski
2015-07-07 10:49   ` [tip:x86/asm] um: " tip-bot for Ingo Molnar
2015-07-03 19:44 ` [PATCH v5 04/17] context_tracking: Add ct_state and CT_WARN_ON Andy Lutomirski
2015-07-07 10:50   ` [tip:x86/asm] context_tracking: Add ct_state() and CT_WARN_ON() tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 05/17] notifiers: Assert that RCU is watching in notify_die Andy Lutomirski
2015-07-07 10:50   ` [tip:x86/asm] notifiers, RCU: Assert that RCU is watching in notify_die() tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 06/17] x86: Move C entry and exit code to arch/x86/entry/common.c Andy Lutomirski
2015-07-07 10:50   ` [tip:x86/asm] x86/entry: Move C entry and exit code to arch/x86/ entry/common.c tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 07/17] x86/traps: Assert that we're in CONTEXT_KERNEL in exception entries Andy Lutomirski
2015-07-07 10:51   ` [tip:x86/asm] x86/traps, context_tracking: Assert that we' re " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 08/17] x86/entry: Add enter_from_user_mode and use it in syscalls Andy Lutomirski
2015-07-07 10:51   ` [tip:x86/asm] x86/entry: Add enter_from_user_mode() " tip-bot for Andy Lutomirski
2015-07-14 23:00     ` Frederic Weisbecker
2015-07-14 23:04       ` Andy Lutomirski
2015-07-14 23:28         ` Frederic Weisbecker
2015-12-21 20:50   ` [PATCH v5 08/17] x86/entry: Add enter_from_user_mode " Sasha Levin
2015-12-21 22:44     ` Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 09/17] x86/entry: Add new, comprehensible entry and exit hooks Andy Lutomirski
2015-07-07 10:51   ` [tip:x86/asm] x86/entry: Add new, comprehensible entry and exit handlers written in C tip-bot for Andy Lutomirski
2015-07-14 23:07     ` Frederic Weisbecker
2015-07-15 19:56       ` Linus Torvalds
2015-07-15 20:46         ` Andy Lutomirski
2015-07-15 21:25           ` [PATCH] x86/entry: Fix _TIF_USER_RETURN_NOTIFY check in prepare_exit_to_usermode Andy Lutomirski
2015-07-18  3:25             ` [tip:x86/asm] " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 10/17] x86/entry/64: Really create an error-entry-from-usermode code path Andy Lutomirski
2015-07-07 10:52   ` [tip:x86/asm] " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 11/17] x86/entry/64: Migrate 64-bit and compat syscalls to new exit hooks Andy Lutomirski
2015-07-07 10:52   ` [tip:x86/asm] x86/entry/64: Migrate 64-bit and compat syscalls to the new exit handlers and remove old assembly code tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 12/17] x86/asm/entry/64: Save all regs on interrupt entry Andy Lutomirski
2015-07-07 10:52   ` [tip:x86/asm] " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 13/17] x86/asm/entry/64: Simplify irq stack pt_regs handling Andy Lutomirski
2015-07-07 10:53   ` [tip:x86/asm] x86/asm/entry/64: Simplify IRQ " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 14/17] x86/asm/entry/64: Migrate error and interrupt exit work to C Andy Lutomirski
2015-07-07 10:53   ` [tip:x86/asm] x86/asm/entry/64: Migrate error and IRQ exit work to C and remove old assembly code tip-bot for Andy Lutomirski
2015-08-11 22:18     ` Frederic Weisbecker
2015-08-11 22:25       ` Andy Lutomirski
2015-08-11 22:49         ` Frederic Weisbecker
2015-08-11 22:59           ` Andy Lutomirski
2015-08-12  1:02             ` Paul E. McKenney
2015-08-12 13:13             ` Frederic Weisbecker
2015-08-11 22:38     ` Frederic Weisbecker
2015-08-11 22:51       ` Andy Lutomirski
2015-08-11 23:22         ` Frederic Weisbecker
2015-08-11 23:33           ` Andy Lutomirski
2015-08-12 13:32             ` Frederic Weisbecker
2015-08-12 14:59               ` Andy Lutomirski
2015-08-18 22:34                 ` Frederic Weisbecker
2015-08-18 22:40                   ` Andy Lutomirski [this message]
2015-08-19 17:18                     ` Frederic Weisbecker
2015-08-19 18:02                       ` Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 15/17] x86/entry: Remove exception_enter from most trap handlers Andy Lutomirski
2015-07-07 10:53   ` [tip:x86/asm] x86/entry: Remove exception_enter() " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 16/17] x86/entry: Remove SCHEDULE_USER and asm/context-tracking.h Andy Lutomirski
2015-07-07 10:54   ` [tip:x86/asm] x86/entry: Remove SCHEDULE_USER and asm/ context-tracking.h tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 17/17] x86/irq: Document how IRQ context tracking works and add an assertion Andy Lutomirski
2015-07-07 10:54   ` [tip:x86/asm] x86/irq, context_tracking: Document how IRQ context tracking works and add an RCU assertion tip-bot for Andy Lutomirski
2015-07-14 23:26     ` Frederic Weisbecker
2015-07-14 23:33       ` Andy Lutomirski
2015-07-18 13:23         ` Frederic Weisbecker
2015-07-18 14:10           ` Paul E. McKenney
2015-07-07 11:12 ` [PATCH v5 00/17] x86: Rewrite exit-to-userspace code Ingo Molnar
2015-07-07 16:03   ` Andy Lutomirski
2015-07-07 17:55     ` [PATCH] x86/entry/64: Fix warning on compat syscalls with CONFIG_AUDITSYSCALL=n Andy Lutomirski
2015-07-08  9:57       ` [tip:x86/asm] x86/entry/64: Fix IRQ state confusion and related warning on compat syscalls with CONFIG_AUDITSYSCALL =n tip-bot for Andy Lutomirski
2015-07-08 19:12       ` tip-bot for Andy Lutomirski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CALCETrVD1EyH075Lwja4rCLg9OXSB7ZgQtP_5ZwFypf3b9q9sQ@mail.gmail.com \
    --to=luto@amacapital.net \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=dvlasenk@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=hpa@zytor.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=vda.linux@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git