[PATCH 0/7] traps: x86: irqtrace cleanup and some traps.c unification

[PATCH 0/7] traps: x86: irqtrace cleanup and some traps.c unification

[Alexander van Heukelum]

Aparantly the conversion of exceptions from traps to interrupts
and the changes to the irq-tracer on i386 have not given any
trouble, yet? (tip/x86/traps)

This is a small set of patches removing trace_hardirqs_fixup
completely from the code. TRACE_IRQS_OFF is added to the assembly
code instead, where needed.

7/7 is a small unification step between traps_32.c and traps_64.c

Patches were generated against tip/x86/traps.

Greetings,
    Alexander

arch/x86/kernel/entry_64.S |    6 ++++-
arch/x86/kernel/traps_32.c |   55 ++++++++++++-------------------------------
arch/x86/kernel/traps_64.c |   11 ++------
arch/x86/mm/fault.c        |    5 ----
include/asm-x86/irqflags.h |   21 ----------------
5 files changed, 24 insertions(+), 74 deletions(-)
      

[PATCH 1/7] traps: x86_64: add TRACE_IRQS_OFF in error_entry

[Alexander van Heukelum]

Add TRACE_IRQS_OFF just before entering the C code.

All exceptions are taken via interrupt gates. If irq tracing is
enabled, it should be notified as soon as possible. Interrupts
are only (conditionally) re-enabled in C code.

Signed-off-by: Alexander van Heukelum <fastmail.fm>
---
 arch/x86/kernel/entry_64.S |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 89434d4..78fa552 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -1058,7 +1058,8 @@ KPROBE_ENTRY(error_entry)
 	je  error_kernelspace
 error_swapgs:	
 	SWAPGS
-error_sti:	
+error_sti:
+	TRACE_IRQS_OFF
 	movq %rdi,RDI(%rsp) 	
 	CFI_REL_OFFSET	rdi,RDI
 	movq %rsp,%rdi
-- 
1.5.4.3

[PATCH 2/7] traps: x86_64: add TRACE_IRQS_OFF in paranoidentry macro

[Alexander van Heukelum]

Add TRACE_IRQS_OFF just before entering the C code.

All exceptions are taken via interrupt gates. If irq tracing is
enabled, it should be notified as soon as possible. Interrupts
are only (conditionally) re-enabled in C code.

Signed-off-by: Alexander van Heukelum <fastmail.fm>
---
 arch/x86/kernel/entry_64.S |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 78fa552..14ea704 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -932,6 +932,9 @@ END(spurious_interrupt)
 	.if \ist
 	movq	%gs:pda_data_offset, %rbp
 	.endif
+	.if \irqtrace
+	TRACE_IRQS_OFF
+	.endif
 	movq %rsp,%rdi
 	movq ORIG_RAX(%rsp),%rsi
 	movq $-1,ORIG_RAX(%rsp)
-- 
1.5.4.3

[PATCH 3/7] traps: x86_64: remove trace_hardirqs_fixup from DO_ERROR_INFO macro

[Alexander van Heukelum]

All exceptions are taken via interrupt gates. TRACE_IRQS_OFF
is called just before entering the C code, so the irq state
is known to the irq tracer at this point. No need to call
trace_hardirqs_fixup.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/kernel/traps_64.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/traps_64.c b/arch/x86/kernel/traps_64.c
index 513caac..04b163e 100644
--- a/arch/x86/kernel/traps_64.c
+++ b/arch/x86/kernel/traps_64.c
@@ -655,7 +655,6 @@ asmlinkage void do_##name(struct pt_regs * regs, long error_code)	\
 	info.si_errno = 0;						\
 	info.si_code = sicode;						\
 	info.si_addr = (void __user *)siaddr;				\
-	trace_hardirqs_fixup();						\
 	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr)	\
 							== NOTIFY_STOP)	\
 		return;							\
-- 
1.5.4.3

[PATCH 4/7] traps: x86_64: remove trace_hardirqs_fixup from int3 handler

[Alexander van Heukelum]

All exceptions are taken via interrupt gates. TRACE_IRQS_OFF
is called just before entering the C code, so the irq state
is known to the irq tracer at this point. No need to call
trace_hardirqs_fixup.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/kernel/traps_64.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/traps_64.c b/arch/x86/kernel/traps_64.c
index 04b163e..2ff759e 100644
--- a/arch/x86/kernel/traps_64.c
+++ b/arch/x86/kernel/traps_64.c
@@ -857,8 +857,6 @@ void restart_nmi(void)
 /* runs on IST stack. */
 asmlinkage void __kprobes do_int3(struct pt_regs *regs, long error_code)
 {
-	trace_hardirqs_fixup();
-
 	if (notify_die(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
 			== NOTIFY_STOP)
 		return;
-- 
1.5.4.3

[PATCH 5/7] traps: x86_64: remove trace_hardirqs_fixup from debug handler

[Alexander van Heukelum]

All exceptions are taken via interrupt gates. TRACE_IRQS_OFF
is called just before entering the C code, so the irq state
is known to the irq tracer at this point. No need to call
trace_hardirqs_fixup.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/kernel/traps_64.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/traps_64.c b/arch/x86/kernel/traps_64.c
index 2ff759e..c5fb747 100644
--- a/arch/x86/kernel/traps_64.c
+++ b/arch/x86/kernel/traps_64.c
@@ -895,8 +895,6 @@ asmlinkage void __kprobes do_debug(struct pt_regs * regs,
 	unsigned long condition;
 	siginfo_t info;
 
-	trace_hardirqs_fixup();
-
 	get_debugreg(condition, 6);
 
 	/*
-- 
1.5.4.3

[PATCH 6/7] traps: x86: remove trace_hardirqs_fixup from pagefault handler

[Alexander van Heukelum]

The last use of trace_hardirqs_fixup is unnecessary, because the
trap is taken with interrupt off on i386 as well as x86_64, and
the irq-tracer is notified of this from the assembly code.

trace_hardirqs_fixup and trace_hardirqs_fixup_flags are removed
from include/asm-x86/irqflags.h as they are no longer used.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/mm/fault.c        |    5 -----
 include/asm-x86/irqflags.h |   21 ---------------------
 2 files changed, 0 insertions(+), 26 deletions(-)

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 455f3fe..c5bafc9 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -593,11 +593,6 @@ void __kprobes do_page_fault(struct pt_regs *regs, unsigned long error_code)
 	unsigned long flags;
 #endif
 
-	/*
-	 * We can fault from pretty much anywhere, with unknown IRQ state.
-	 */
-	trace_hardirqs_fixup();
-
 	tsk = current;
 	mm = tsk->mm;
 	prefetchw(&mm->mmap_sem);
diff --git a/include/asm-x86/irqflags.h b/include/asm-x86/irqflags.h
index 424acb4..2bdab21 100644
--- a/include/asm-x86/irqflags.h
+++ b/include/asm-x86/irqflags.h
@@ -166,27 +166,6 @@ static inline int raw_irqs_disabled(void)
 	return raw_irqs_disabled_flags(flags);
 }
 
-/*
- * makes the traced hardirq state match with the machine state
- *
- * should be a rarely used function, only in places where its
- * otherwise impossible to know the irq state, like in traps.
- */
-static inline void trace_hardirqs_fixup_flags(unsigned long flags)
-{
-	if (raw_irqs_disabled_flags(flags))
-		trace_hardirqs_off();
-	else
-		trace_hardirqs_on();
-}
-
-static inline void trace_hardirqs_fixup(void)
-{
-	unsigned long flags = __raw_local_save_flags();
-
-	trace_hardirqs_fixup_flags(flags);
-}
-
 #else
 
 #ifdef CONFIG_X86_64
-- 
1.5.4.3

[PATCH 7/7] traps: i386: make do_trap more like x86_64

[Alexander van Heukelum]

This patch hardcodes which traps should be forwarded to
handle_vm86_trap in do_trap. This allows to remove the
vm86 parameter from the i386-version of do_trap, which
makes the DO_VM86_ERROR and DO_VM86_ERROR_INFO macros
unnecessary.

x86_64 part is whitespace only.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/kernel/traps_32.c |   55 ++++++++++++-------------------------------
 arch/x86/kernel/traps_64.c |    6 ++--
 2 files changed, 19 insertions(+), 42 deletions(-)

diff --git a/arch/x86/kernel/traps_32.c b/arch/x86/kernel/traps_32.c
index 041a25e..a603554 100644
--- a/arch/x86/kernel/traps_32.c
+++ b/arch/x86/kernel/traps_32.c
@@ -482,13 +482,17 @@ die_if_kernel(const char *str, struct pt_regs *regs, long err)
 }
 
 static void __kprobes
-do_trap(int trapnr, int signr, char *str, int vm86, struct pt_regs *regs,
+do_trap(int trapnr, int signr, char *str, struct pt_regs *regs,
 	long error_code, siginfo_t *info)
 {
 	struct task_struct *tsk = current;
 
 	if (regs->flags & X86_VM_MASK) {
-		if (vm86)
+		/*
+		 * traps 0, 1, 3, 4, and 5 should be forwarded to vm86.
+		 * On nmi (interrupt 2), do_trap should not be called.
+		 */
+		if (trapnr < 6)
 			goto vm86_trap;
 		goto trap_signal;
 	}
@@ -537,37 +541,10 @@ void do_##name(struct pt_regs *regs, long error_code)			\
 							== NOTIFY_STOP)	\
 		return;							\
 	conditional_sti(regs);						\
-	do_trap(trapnr, signr, str, 0, regs, error_code, NULL);		\
-}
-
-#define DO_ERROR_INFO(trapnr, signr, str, name, sicode, siaddr, irq)	\
-void do_##name(struct pt_regs *regs, long error_code)			\
-{									\
-	siginfo_t info;							\
-	if (irq)							\
-		local_irq_enable();					\
-	info.si_signo = signr;						\
-	info.si_errno = 0;						\
-	info.si_code = sicode;						\
-	info.si_addr = (void __user *)siaddr;				\
-	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr)	\
-							== NOTIFY_STOP)	\
-		return;							\
-	conditional_sti(regs);						\
-	do_trap(trapnr, signr, str, 0, regs, error_code, &info);	\
-}
-
-#define DO_VM86_ERROR(trapnr, signr, str, name)				\
-void do_##name(struct pt_regs *regs, long error_code)			\
-{									\
-	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr)	\
-							== NOTIFY_STOP)	\
-		return;							\
-	conditional_sti(regs);						\
-	do_trap(trapnr, signr, str, 1, regs, error_code, NULL);		\
+	do_trap(trapnr, signr, str, regs, error_code, NULL);		\
 }
 
-#define DO_VM86_ERROR_INFO(trapnr, signr, str, name, sicode, siaddr)	\
+#define DO_ERROR_INFO(trapnr, signr, str, name, sicode, siaddr)		\
 void do_##name(struct pt_regs *regs, long error_code)			\
 {									\
 	siginfo_t info;							\
@@ -579,18 +556,18 @@ void do_##name(struct pt_regs *regs, long error_code)			\
 							== NOTIFY_STOP)	\
 		return;							\
 	conditional_sti(regs);						\
-	do_trap(trapnr, signr, str, 1, regs, error_code, &info);	\
+	do_trap(trapnr, signr, str, regs, error_code, &info);		\
 }
 
-DO_VM86_ERROR_INFO(0, SIGFPE, "divide error", divide_error, FPE_INTDIV, regs->ip)
-DO_VM86_ERROR(4, SIGSEGV, "overflow", overflow)
-DO_VM86_ERROR(5, SIGSEGV, "bounds", bounds)
-DO_ERROR_INFO(6, SIGILL, "invalid opcode", invalid_op, ILL_ILLOPN, regs->ip, 0)
+DO_ERROR_INFO(0, SIGFPE, "divide error", divide_error, FPE_INTDIV, regs->ip)
+DO_ERROR(4, SIGSEGV, "overflow", overflow)
+DO_ERROR(5, SIGSEGV, "bounds", bounds)
+DO_ERROR_INFO(6, SIGILL, "invalid opcode", invalid_op, ILL_ILLOPN, regs->ip)
 DO_ERROR(9, SIGFPE, "coprocessor segment overrun", coprocessor_segment_overrun)
 DO_ERROR(10, SIGSEGV, "invalid TSS", invalid_TSS)
 DO_ERROR(11, SIGBUS, "segment not present", segment_not_present)
 DO_ERROR(12, SIGBUS, "stack segment", stack_segment)
-DO_ERROR_INFO(17, SIGBUS, "alignment check", alignment_check, BUS_ADRALN, 0, 0)
+DO_ERROR_INFO(17, SIGBUS, "alignment check", alignment_check, BUS_ADRALN, 0)
 
 void __kprobes
 do_general_protection(struct pt_regs *regs, long error_code)
@@ -868,7 +845,7 @@ void __kprobes do_int3(struct pt_regs *regs, long error_code)
 		return;
 #endif
 
-	do_trap(3, SIGTRAP, "int3", 1, regs, error_code, NULL);
+	do_trap(3, SIGTRAP, "int3", regs, error_code, NULL);
 }
 
 /*
@@ -1212,7 +1189,7 @@ void do_iret_error(struct pt_regs *regs, long error_code)
 	if (notify_die(DIE_TRAP, "iret exception",
 			regs, error_code, 32, SIGILL) == NOTIFY_STOP)
 		return;
-	do_trap(32, SIGILL, "iret exception", 0, regs, error_code, &info);
+	do_trap(32, SIGILL, "iret exception", regs, error_code, &info);
 }
 
 void __init trap_init(void)
diff --git a/arch/x86/kernel/traps_64.c b/arch/x86/kernel/traps_64.c
index c5fb747..40da132 100644
--- a/arch/x86/kernel/traps_64.c
+++ b/arch/x86/kernel/traps_64.c
@@ -637,8 +637,8 @@ kernel_trap:
 	return;
 }
 
-#define DO_ERROR(trapnr, signr, str, name) \
-asmlinkage void do_##name(struct pt_regs * regs, long error_code)	\
+#define DO_ERROR(trapnr, signr, str, name)				\
+asmlinkage void do_##name(struct pt_regs *regs, long error_code)	\
 {									\
 	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr)	\
 							== NOTIFY_STOP)	\
@@ -648,7 +648,7 @@ asmlinkage void do_##name(struct pt_regs * regs, long error_code)	\
 }
 
 #define DO_ERROR_INFO(trapnr, signr, str, name, sicode, siaddr)		\
-asmlinkage void do_##name(struct pt_regs * regs, long error_code)	\
+asmlinkage void do_##name(struct pt_regs *regs, long error_code)	\
 {									\
 	siginfo_t info;							\
 	info.si_signo = signr;						\
-- 
1.5.4.3

Re: [PATCH 1/7] traps: x86_64: add TRACE_IRQS_OFF in error_entry

[Alexander van Heukelum]

On Fri, 26 Sep 2008 14:03:02 +0200, "Alexander van Heukelum"
<heukelum@fastmail.fm> said:
> Add TRACE_IRQS_OFF just before entering the C code.
> 
> All exceptions are taken via interrupt gates. If irq tracing is
> enabled, it should be notified as soon as possible. Interrupts
> are only (conditionally) re-enabled in C code.
> 
> Signed-off-by: Alexander van Heukelum <fastmail.fm>

That should be: <heukelum@fastmail.fm> ---^^^
Same for patch 2/7. How did that happen?

> ---
>  arch/x86/kernel/entry_64.S |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index 89434d4..78fa552 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -1058,7 +1058,8 @@ KPROBE_ENTRY(error_entry)
>  	je  error_kernelspace
>  error_swapgs:	
>  	SWAPGS
> -error_sti:	
> +error_sti:
> +	TRACE_IRQS_OFF
>  	movq %rdi,RDI(%rsp) 	
>  	CFI_REL_OFFSET	rdi,RDI
>  	movq %rsp,%rdi
> -- 
> 1.5.4.3
> 
-- 
  Alexander van Heukelum
  heukelum@fastmail.fm

-- 
http://www.fastmail.fm - Or how I learned to stop worrying and
                          love email again

Re: [PATCH 0/7] traps: x86: irqtrace cleanup and some traps.c unification

[Ingo Molnar]

* Alexander van Heukelum <heukelum@fastmail.fm> wrote:

> Aparantly the conversion of exceptions from traps to interrupts and 
> the changes to the irq-tracer on i386 have not given any trouble, yet? 
> (tip/x86/traps)

Correct.

Generally you'll hear from the -tip maintainers if there's any trouble 
with a topic. If you see it integrated into tip/master and if you see no 
comments then it means that they work just fine.

> This is a small set of patches removing trace_hardirqs_fixup 
> completely from the code. TRACE_IRQS_OFF is added to the assembly code 
> instead, where needed.
>
> 7/7 is a small unification step between traps_32.c and traps_64.c

cool! Any plans on finishing the unification? ;-)

> Patches were generated against tip/x86/traps.

applied to tip/x86/traps, thanks.

	Ingo