On Wed, Oct 23, 2019 at 5:31 AM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>
> Interrupt state tracing can be safely done in C code. The few stack
> operations in assembly do not need to be covered.
>
> Remove the now pointless indirection via .Lsyscall_32_done and jump to
> swapgs_restore_regs_and_return_to_usermode directly.
This doesn't look right.
> #define SYSCALL_EXIT_WORK_FLAGS \
> @@ -279,6 +282,9 @@ static void syscall_slow_exit_work(struc
> {
> struct thread_info *ti;
>
> + /* User to kernel transition disabled interrupts. */
> + trace_hardirqs_off();
> +
So you just traced IRQs off, but...
> enter_from_user_mode();
> local_irq_enable();
Now they're on and traced on again?
I also don't see how your patch handles the fastpath case.
How about the attached patch instead?
commit e154af0d7ff2d605f155f5aca059e3e835e426d4
Author: Andy Lutomirski <luto@xxxxxxxxxx>
Date: Fri Aug 2 10:30:44 2019 -0700
x86/entry: Move exit-to-usermode irqflag tracing to prepare_exit_to_usermode()
prepare_exit_to_usermode() can easily handle irqflag tracing. Move
the logic there and remove it from the entry asm.
Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index cc49380ef8ab..f4ce0cf2fb74 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -217,6 +217,13 @@ __visible inline void prepare_exit_to_usermode(struct pt_regs *regs)
user_enter_irqoff();
+ /*
+ * The actual return to usermode will almost certainly turn IRQs on.
+ * Trace it here to simplify the asm code.
+ */
+ if (likely(regs->flags & X86_EFLAGS_IF))
+ trace_hardirqs_on();
+
mds_user_clear_cpu_buffers();
}
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index f83ca5aa8b77..c703c29bebb1 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -1062,7 +1062,6 @@ ENTRY(entry_INT80_32)
STACKLEAK_ERASE
restore_all:
- TRACE_IRQS_IRET
SWITCH_TO_ENTRY_STACK
.Lrestore_all_notrace:
CHECK_AND_APPLY_ESPFIX
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 3abae80d4902..056419a0e76f 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -172,8 +172,6 @@ GLOBAL(entry_SYSCALL_64_after_hwframe)
movq %rsp, %rsi
call do_syscall_64 /* returns with IRQs disabled */
- TRACE_IRQS_IRETQ /* we're about to change IF */
-
/*
* Try to use SYSRET instead of IRET if we're returning to
* a completely clean 64-bit userspace context. If we're not,
@@ -617,7 +615,6 @@ ret_from_intr:
.Lretint_user:
mov %rsp,%rdi
call prepare_exit_to_usermode
- TRACE_IRQS_IRETQ
GLOBAL(swapgs_restore_regs_and_return_to_usermode)
#ifdef CONFIG_DEBUG_ENTRY
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 435df637f392..3502f38bde01 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -254,7 +254,6 @@ sysret32_from_system_call:
* stack. So let's erase the thread stack right now.
*/
STACKLEAK_ERASE
- TRACE_IRQS_ON /* User mode traces as IRQs on. */
movq RBX(%rsp), %rbx /* pt_regs->rbx */
movq RBP(%rsp), %rbp /* pt_regs->rbp */
movq EFLAGS(%rsp), %r11 /* pt_regs->flags (in r11) */
@@ -396,6 +395,5 @@ ENTRY(entry_INT80_compat)
.Lsyscall_32_done:
/* Go back to user mode. */
- TRACE_IRQS_ON
jmp swapgs_restore_regs_and_return_to_usermode
END(entry_INT80_compat)
