Re: [PATCH v4 05/27] x86: Replace ist_enter() with nmi_enter()

Peter Zijlstra
2020-02-24T10:44:16Z

On Fri, Feb 21, 2020 at 09:22:46PM +0100, Peter Zijlstra wrote:
> On Fri, Feb 21, 2020 at 11:05:36AM -0800, Andy Lutomirski wrote:
>
> > > - /*
> > > - * Use ist_enter despite the fact that we don't use an IST stack.
> > > - * We can be called from a kprobe in non-CONTEXT_KERNEL kernel
> > > - * mode or even during context tracking state changes.
> > > - *
> > > - * This means that we can't schedule. That's okay.
> > > - */
> > > - ist_enter(regs);
> > > + nmi_enter();
> >
> > I agree with the change, but some commentary might be nice. Maybe
> > copy from here:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/idtentry&id=061eaa900b4f63601ab6381ab431fcef8dfd84be
>
> Fair enough; I'll add something to #DB and #BP for that.

do_int3() is now like:

@@ -529,19 +497,18 @@ dotraplinkage void do_general_protection
}
NOKPROBE_SYMBOL(do_general_protection);

-dotraplinkage void notrace do_int3(struct pt_regs *regs, long error_code)
+dotraplinkage notrace void do_int3(struct pt_regs *regs, long error_code)
{
if (poke_int3_handler(regs))
return;

/*
- * Use ist_enter despite the fact that we don't use an IST stack.
- * We can be called from a kprobe in non-CONTEXT_KERNEL kernel
- * mode or even during context tracking state changes.
- *
- * This means that we can't schedule. That's okay.
+ * Unlike any other non-IST entry, we can be called from pretty much
+ * any location in the kernel through kprobes -- text_poke() will most
+ * likely be handled by poke_int3_handler() above. This means this
+ * handler is effectively NMI-like.
*/
- ist_enter(regs);
+ nmi_enter();
RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
#ifdef CONFIG_KGDB_LOW_LEVEL_TRAP
if (kgdb_ll_trap(DIE_INT3, "int3", regs, error_code, X86_TRAP_BP,
@@ -563,7 +530,7 @@ dotraplinkage void notrace do_int3(struc
cond_local_irq_disable(regs);

exit:
- ist_exit(regs);
+ nmi_exit();
}
NOKPROBE_SYMBOL(do_int3);

And I'm going to have to do another (few) patches for do_debug(),
because that comment it has it patently false and I remember we had a
bunch of crud for that. Let me dig those out.