Skip to contentSkip to conversation

x86: Cleanup IDT code

PATCH Superseded Thomas Gleixner 7 participants 69 messages Aug 25, 2017 – Aug 28, 2017

Subsystems (9)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover x86: Cleanup IDT code

The IDT code in x86 is a convoluted mess and contains quite some bogosities.

One of the main offenders is the extra tracing IDT which has been introduced to reduce the impact of tracing on the exception vectors when tracepoints are disabled.

This results in an unholy macro mess which magically creates IDT entries by prepending the exception handler name by trace_.

For most exceptions the tracepoint overhead in the disabled case is two NOP5, which are negligible compared to the heavy work of the exception handler itself. Modern CPUs are pretty good in optimizing NOPs.

The only two handlers where the overhead matters are the pagefault handler and the reschedule IPI, where an active tracepoint introduces more extra work. These two can be handled with separate static keys which reduces the impact to a minimum.

As a result the whole tracing IDT machinery can be removed.

The second part of the series cleans up the handling of IDT/GDT/LDT/TSS descriptors to get rid of macro based initialization via the obscure raw 32bit union fields a and b.

The last part moves the setup of the IDT into a separate file, where all gate related code is moved to. The gate setup is converted to a table based mechanism so the resulting code is smaller and better understandable.

The series applies on top of

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/apic

and is available as a git branch from

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.x86/apic

There is more cleanup to come in the near future, which aims at replacing the braindamaged vector assignment mechanics, which has quite some shortcomings.

Thanks,

tglx

---
 arch/x86/boot/compressed/eboot.c         |    8 
 arch/x86/entry/entry_32.S                |   20 -
 arch/x86/entry/entry_64.S                |   26 --
 arch/x86/entry/vdso/vma.c                |    2 
 arch/x86/include/asm/desc.h              |  246 +++-----------------
 arch/x86/include/asm/desc_defs.h         |  122 +++++-----
 arch/x86/include/asm/entry_arch.h        |   14 -
 arch/x86/include/asm/hw_irq.h            |   20 -
 arch/x86/include/asm/irq.h               |    4 
 arch/x86/include/asm/segment.h           |    4 
 arch/x86/include/asm/trace/exceptions.h  |    8 
 arch/x86/include/asm/trace/irq_vectors.h |   24 +-
 arch/x86/include/asm/traps.h             |   10 
 arch/x86/include/asm/xen/hypercall.h     |    6 
 arch/x86/kernel/Makefile                 |    3 
 arch/x86/kernel/apic/apic.c              |   70 +----
 arch/x86/kernel/apic/vector.c            |    2 
 arch/x86/kernel/cpu/common.c             |    9 
 arch/x86/kernel/cpu/mcheck/mce_amd.c     |   16 -
 arch/x86/kernel/cpu/mcheck/therm_throt.c |   20 -
 arch/x86/kernel/cpu/mcheck/threshold.c   |   16 -
 arch/x86/kernel/cpu/mshyperv.c           |    9 
 arch/x86/kernel/head32.c                 |    4 
 arch/x86/kernel/head64.c                 |    6 
 arch/x86/kernel/head_32.S                |   42 ---
 arch/x86/kernel/irq.c                    |   29 --
 arch/x86/kernel/irq_work.c               |   16 -
 arch/x86/kernel/irqinit.c                |  102 --------
 arch/x86/kernel/kvm.c                    |    4 
 arch/x86/kernel/machine_kexec_32.c       |   14 -
 arch/x86/kernel/reboot.c                 |    4 
 arch/x86/kernel/setup.c                  |    4 
 arch/x86/kernel/setup_percpu.c           |    9 
 arch/x86/kernel/smp.c                    |   81 +-----
 arch/x86/kernel/tls.c                    |    2 
 arch/x86/kernel/tracepoint.c             |   57 +---
 arch/x86/kernel/traps.c                  |  108 ---------
 arch/x86/kvm/vmx.c                       |    2 
 arch/x86/math-emu/fpu_entry.c            |   11 
 arch/x86/math-emu/fpu_system.h           |   48 +++-
 arch/x86/math-emu/get_address.c          |   17 -
 arch/x86/mm/fault.c                      |   47 +--
 arch/x86/xen/enlighten_pv.c              |   14 -
 b/arch/x86/include/asm/trace/common.h    |   16 +
 b/arch/x86/kernel/eisa.c                 |   18 +
 b/arch/x86/kernel/idt.c                  |  365 +++++++++++++++++++++++++++++++
 drivers/xen/events/events_base.c         |    6 
 47 files changed, 718 insertions(+), 967 deletions(-)
#1 lore raw

PATCH 1/41 x86/irq: Remove vector_used_by_percpu_irq()

Last user (lguest) is gone. Remove it.

---
 arch/x86/include/asm/irq.h |    1 -
 arch/x86/kernel/irq.c      |    2 --
 arch/x86/kernel/irqinit.c  |   12 ------------
 3 files changed, 15 deletions(-)
arch/x86/include/asm/irq.h +0 -1
@@ -44,7 +44,6 @@ extern __visible unsigned int do_IRQ(str
 
 /* Interrupt vector management */
 extern DECLARE_BITMAP(used_vectors, NR_VECTORS);
-extern int vector_used_by_percpu_irq(unsigned int vector);
 
 extern void init_ISA_irqs(void);
 
arch/x86/kernel/irq.c +0 -2
@@ -346,8 +346,6 @@ EXPORT_SYMBOL_GPL(kvm_set_posted_intr_wa
 	set_irq_regs(old_regs);
 }
 
-EXPORT_SYMBOL_GPL(vector_used_by_percpu_irq);
-
 #ifdef CONFIG_HOTPLUG_CPU
 
 /* These two declarations are only used in check_irq_vectors_for_cpu_disable()
arch/x86/kernel/irqinit.c +0 -12
@@ -55,18 +55,6 @@ DEFINE_PER_CPU(vector_irq_t, vector_irq)
 	[0 ... NR_VECTORS - 1] = VECTOR_UNUSED,
 };
 
-int vector_used_by_percpu_irq(unsigned int vector)
-{
-	int cpu;
-
-	for_each_online_cpu(cpu) {
-		if (!IS_ERR_OR_NULL(per_cpu(vector_irq, cpu)[vector]))
-			return 1;
-	}
-
-	return 0;
-}
-
 void __init init_ISA_irqs(void)
 {
 	struct irq_chip *chip = legacy_pic->chip;
#2 lore raw

PATCH 2/41 x86/irq: Unexport used_vectors

In reply to Thomas Gleixner

No modular users.

---
 arch/x86/kernel/traps.c |    1 -
 1 file changed, 1 deletion(-)
arch/x86/kernel/traps.c +0 -1
@@ -83,7 +83,6 @@ gate_desc debug_idt_table[NR_VECTORS] __
 gate_desc idt_table[NR_VECTORS] __page_aligned_bss;
 
 DECLARE_BITMAP(used_vectors, NR_VECTORS);
-EXPORT_SYMBOL_GPL(used_vectors);
 
 static inline void cond_local_irq_enable(struct pt_regs *regs)
 {
#3 lore raw

PATCH 3/41 x86/irq: Move ifdeffery to header file

In reply to Thomas Gleixner

Having this in the middle of code is beyond ugly.

---
 arch/x86/include/asm/desc.h |    5 +++++
 arch/x86/kernel/irqinit.c   |    3 ---
 2 files changed, 5 insertions(+), 3 deletions(-)
arch/x86/include/asm/desc.h +5 -0
@@ -482,7 +482,12 @@ static inline void _set_gate(int gate, u
 				0, 0, __KERNEL_CS);			\
 	} while (0)
 
+#ifdef CONFIG_X86_LOCAL_APIC
 extern int first_system_vector;
+#else
+#define first_system_vector		NR_VECTORS
+#endif
+
 /* used_vectors is BITMAP for irq is not managed by percpu vector_irq */
 extern unsigned long used_vectors[];
 
arch/x86/kernel/irqinit.c +0 -3
@@ -169,9 +169,6 @@ void __init native_init_IRQ(void)
 	 * 'special' SMP interrupts)
 	 */
 	i = FIRST_EXTERNAL_VECTOR;
-#ifndef CONFIG_X86_LOCAL_APIC
-#define first_system_vector NR_VECTORS
-#endif
 	for_each_clear_bit_from(i, used_vectors, first_system_vector) {
 		/* IA32_SYSCALL_VECTOR could be used in trap_init already. */
 		set_intr_gate(i, irq_entries_start +
#4 lore raw

Re: [patch 03/41] x86/irq: Move ifdeffery to header file

Quoted 1 line · Thomas Gleixner

Having this in the middle of code is beyond ugly.

Bah, I know why this was in the middle of the code.

I reorder the series so the whole first_system_vector nonsense gets removed early.

Thanks,

tglx

#5 lore raw

PATCH 4/41 x86/boot: Move EISA setup to a proper place

In reply to Thomas Gleixner

EISA has absolutely nothing to do with traps. The EISA bus detection does not need to run in the very early boot. It's good enough to run it before the EISA bus and drivers are initialized.

---
 arch/x86/kernel/Makefile |    1 +
 arch/x86/kernel/eisa.c   |   18 ++++++++++++++++++
 arch/x86/kernel/traps.c  |   13 -------------
 3 files changed, 19 insertions(+), 13 deletions(-)
arch/x86/kernel/Makefile +1 -0
Index: b/arch/x86/kernel/Makefile
===================================================================
@@ -111,6 +111,7 @@ obj-$(CONFIG_PARAVIRT_SPINLOCKS)+= parav
 obj-$(CONFIG_PARAVIRT_CLOCK)	+= pvclock.o
 obj-$(CONFIG_X86_PMEM_LEGACY_DEVICE) += pmem.o
 
+obj-$(CONFIG_EISA)		+= eisa.o
 obj-$(CONFIG_PCSPKR_PLATFORM)	+= pcspeaker.o
 
 obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION) += check.o
arch/x86/kernel/eisa.c new file +18 -0
Index: b/arch/x86/kernel/eisa.c
===================================================================
@@ -0,0 +1,18 @@
+/*
+ * EISA specific code
+ *
+ * This file is licensed under the GPL V2
+ */
+#include <linux/ioport.h>
+#include <linux/eisa.h>
+
+static __init int eisa_bus_probe(void)
+{
+	void __iomem *p = ioremap(0x0FFFD9, 4);
+
+	if (readl(p) == 'E' + ('I'<<8) + ('S'<<16) + ('A'<<24))
+		EISA_bus = 1;
+	iounmap(p, 4);
+	return 0;
+}
+subsys_initcall(eisa_bus_probe);
arch/x86/kernel/traps.c +0 -13
Index: b/arch/x86/kernel/traps.c
===================================================================
@@ -38,11 +38,6 @@
 #include <linux/smp.h>
 #include <linux/io.h>
 
-#ifdef CONFIG_EISA
-#include <linux/ioport.h>
-#include <linux/eisa.h>
-#endif
-
 #if defined(CONFIG_EDAC)
 #include <linux/edac.h>
 #endif
@@ -969,14 +964,6 @@ void __init trap_init(void)
 {
 	int i;
 
-#ifdef CONFIG_EISA
-	void __iomem *p = early_ioremap(0x0FFFD9, 4);
-
-	if (readl(p) == 'E' + ('I'<<8) + ('S'<<16) + ('A'<<24))
-		EISA_bus = 1;
-	early_iounmap(p, 4);
-#endif
-
 	set_intr_gate(X86_TRAP_DE, divide_error);
 	set_intr_gate_ist(X86_TRAP_NMI, &nmi, NMI_STACK);
 	/* int4 can be called from all */
#6 lore raw

Re: [patch 04/41] x86/boot: Move EISA setup to a proper place

arch/x86/kernel/eisa.c line 15

Quoted 7 lines · Thomas Gleixner
arch/x86/kernel/eisa.c new file +7 -0
+static __init int eisa_bus_probe(void)
+{
+	void __iomem *p = ioremap(0x0FFFD9, 4);
+
+	if (readl(p) == 'E' + ('I'<<8) + ('S'<<16) + ('A'<<24))
+		EISA_bus = 1;
+	iounmap(p, 4);

That should obviously be:

     iounmap(p);

But as a side note:

Can't we finally get rid of this EISA cruft alltogether?

Thanks,

tglx

#7 lore raw

PATCH 5/41 x86/tracing: Introduce a static key for exception tracing

In reply to Thomas Gleixner

Switching the IDT just for avoiding tracepoints creates a complete impenetrable macro/inline/ifdef mess.

There is no point in avoiding tracepoints for most of the traps/exceptions. For the more expensive tracepoints, like pagefaults, this can be handled with an explicit static key.

Preparatory patch to remove the tracing idt.

---
 arch/x86/include/asm/trace/common.h      |   15 +++++++++++++++
 arch/x86/include/asm/trace/exceptions.h  |    4 +---
 arch/x86/include/asm/trace/irq_vectors.h |    4 +---
 arch/x86/kernel/tracepoint.c             |    9 ++++++++-
 4 files changed, 25 insertions(+), 7 deletions(-)
arch/x86/include/asm/trace/common.h new file +15 -0
@@ -0,0 +1,15 @@
+#ifndef _ASM_TRACE_COMMON_H
+#define _ASM_TRACE_COMMON_H
+
+extern int trace_irq_vector_regfunc(void);
+extern void trace_irq_vector_unregfunc(void);
+
+#ifdef CONFIG_TRACING
+DECLARE_STATIC_KEY_FALSE(trace_irqvectors_key);
+#define trace_irqvectors_enabled()			\
+	static_branch_unlikely(&trace_irqvectors_key)
+#else
+static inline bool trace_irqvectors_enabled(void) { return false; }
+#endif
+
+#endif
arch/x86/include/asm/trace/exceptions.h +1 -3
@@ -5,9 +5,7 @@
 #define _TRACE_PAGE_FAULT_H
 
 #include <linux/tracepoint.h>
-
-extern int trace_irq_vector_regfunc(void);
-extern void trace_irq_vector_unregfunc(void);
+#include <asm/trace/common.h>
 
 DECLARE_EVENT_CLASS(x86_exceptions,
 
arch/x86/include/asm/trace/irq_vectors.h +1 -3
@@ -5,9 +5,7 @@
 #define _TRACE_IRQ_VECTORS_H
 
 #include <linux/tracepoint.h>
-
-extern int trace_irq_vector_regfunc(void);
-extern void trace_irq_vector_unregfunc(void);
+#include <asm/trace/common.h>
 
 DECLARE_EVENT_CLASS(x86_irq_vector,
 
arch/x86/kernel/tracepoint.c +8 -1
@@ -4,9 +4,11 @@
  * Copyright (C) 2013 Seiji Aguchi <[email protected]>
  *
  */
+#include <linux/jump_label.h>
+#include <linux/atomic.h>
+
 #include <asm/hw_irq.h>
 #include <asm/desc.h>
-#include <linux/atomic.h>
 
 atomic_t trace_idt_ctr = ATOMIC_INIT(0);
 struct desc_ptr trace_idt_descr = { NR_VECTORS * 16 - 1,
@@ -15,6 +17,7 @@ struct desc_ptr trace_idt_descr = { NR_V
 /* No need to be aligned, but done to keep all IDTs defined the same way. */
 gate_desc trace_idt_table[NR_VECTORS] __page_aligned_bss;
 
+DEFINE_STATIC_KEY_FALSE(trace_irqvectors_key);
 static int trace_irq_vector_refcount;
 static DEFINE_MUTEX(irq_vector_mutex);
 
@@ -36,6 +39,8 @@ static void switch_idt(void *arg)
 
 int trace_irq_vector_regfunc(void)
 {
+	static_branch_inc(&trace_irqvectors_key);
+
 	mutex_lock(&irq_vector_mutex);
 	if (!trace_irq_vector_refcount) {
 		set_trace_idt_ctr(1);
@@ -49,6 +54,8 @@ int trace_irq_vector_regfunc(void)
 
 void trace_irq_vector_unregfunc(void)
 {
+	static_branch_dec(&trace_irqvectors_key);
+
 	mutex_lock(&irq_vector_mutex);
 	trace_irq_vector_refcount--;
 	if (!trace_irq_vector_refcount) {
Reviewed-by on v1 5/41
#8 lore raw

Re: [patch 05/41] x86/tracing: Introduce a static key for exception tracing

arch/x86/include/asm/trace/common.h

Quoted 19 lines · On Fri, 25 Aug 2017 12:31:08 +0200 Thomas Gleixner <[email protected]> wrote:

Switching the IDT just for avoiding tracepoints creates a complete impenetrable macro/inline/ifdef mess.

There is no point in avoiding tracepoints for most of the traps/exceptions. For the more expensive tracepoints, like pagefaults, this can be handled with an explicit static key.

Preparatory patch to remove the tracing idt.

---
 arch/x86/include/asm/trace/common.h      |   15 +++++++++++++++
 arch/x86/include/asm/trace/exceptions.h  |    4 +---
 arch/x86/include/asm/trace/irq_vectors.h |    4 +---
 arch/x86/kernel/tracepoint.c             |    9 ++++++++-
 4 files changed, 25 insertions(+), 7 deletions(-)
arch/x86/include/asm/trace/common.h new file +0 -0

-- Steve

#9 lore raw

PATCH 6/41 x86/traps: Simplify pagefault tracing logic

In reply to Thomas Gleixner

Make use of the new irqvector tracing static key and remove the duplicated trace_do_pagefault() implementation.

If irq vector tracing is disabled, then the overhead of this is a single NOP5, which is a reasonable tradeoff to avoid duplicated code and the unholy macro mess.

---
 arch/x86/entry/entry_32.S    |    8 -------
 arch/x86/entry/entry_64.S    |   13 -----------
 arch/x86/include/asm/traps.h |   10 ---------
 arch/x86/kernel/kvm.c        |    2 -
 arch/x86/mm/fault.c          |   47 ++++++++++---------------------------------
 5 files changed, 15 insertions(+), 65 deletions(-)
arch/x86/entry/entry_32.S +0 -8
@@ -891,14 +891,6 @@ BUILD_INTERRUPT3(hyperv_callback_vector,
 
 #endif /* CONFIG_HYPERV */
 
-#ifdef CONFIG_TRACING
-ENTRY(trace_page_fault)
-	ASM_CLAC
-	pushl	$trace_do_page_fault
-	jmp	common_exception
-END(trace_page_fault)
-#endif
-
 ENTRY(page_fault)
 	ASM_CLAC
 	pushl	$do_page_fault
arch/x86/entry/entry_64.S +1 -12
@@ -829,17 +829,6 @@ ENTRY(\sym)
 END(\sym)
 .endm
 
-#ifdef CONFIG_TRACING
-.macro trace_idtentry sym do_sym has_error_code:req
-idtentry trace(\sym) trace(\do_sym) has_error_code=\has_error_code
-idtentry \sym \do_sym has_error_code=\has_error_code
-.endm
-#else
-.macro trace_idtentry sym do_sym has_error_code:req
-idtentry \sym \do_sym has_error_code=\has_error_code
-.endm
-#endif
-
 idtentry divide_error			do_divide_error			has_error_code=0
 idtentry overflow			do_overflow			has_error_code=0
 idtentry bounds				do_bounds			has_error_code=0
@@ -1004,7 +993,7 @@ idtentry xen_stack_segment	do_stack_segm
 #endif
 
 idtentry general_protection	do_general_protection	has_error_code=1
-trace_idtentry page_fault	do_page_fault		has_error_code=1
+idtentry page_fault		do_page_fault		has_error_code=1
 
 #ifdef CONFIG_KVM_GUEST
 idtentry async_page_fault	do_async_page_fault	has_error_code=1
arch/x86/include/asm/traps.h +1 -9
@@ -39,7 +39,6 @@ asmlinkage void machine_check(void);
 asmlinkage void simd_coprocessor_error(void);
 
 #ifdef CONFIG_TRACING
-asmlinkage void trace_page_fault(void);
 #define trace_stack_segment stack_segment
 #define trace_divide_error divide_error
 #define trace_bounds bounds
@@ -54,6 +53,7 @@ asmlinkage void trace_page_fault(void);
 #define trace_alignment_check alignment_check
 #define trace_simd_coprocessor_error simd_coprocessor_error
 #define trace_async_page_fault async_page_fault
+#define trace_page_fault page_fault
 #endif
 
 dotraplinkage void do_divide_error(struct pt_regs *, long);
@@ -74,14 +74,6 @@ asmlinkage struct pt_regs *sync_regs(str
 #endif
 dotraplinkage void do_general_protection(struct pt_regs *, long);
 dotraplinkage void do_page_fault(struct pt_regs *, unsigned long);
-#ifdef CONFIG_TRACING
-dotraplinkage void trace_do_page_fault(struct pt_regs *, unsigned long);
-#else
-static inline void trace_do_page_fault(struct pt_regs *regs, unsigned long error)
-{
-	do_page_fault(regs, error);
-}
-#endif
 dotraplinkage void do_spurious_interrupt_bug(struct pt_regs *, long);
 dotraplinkage void do_coprocessor_error(struct pt_regs *, long);
 dotraplinkage void do_alignment_check(struct pt_regs *, long);
arch/x86/kernel/kvm.c +1 -1
@@ -263,7 +263,7 @@ do_async_page_fault(struct pt_regs *regs
 
 	switch (kvm_read_and_reset_pf_reason()) {
 	default:
-		trace_do_page_fault(regs, error_code);
+		do_page_fault(regs, error_code);
 		break;
 	case KVM_PV_REASON_PAGE_NOT_PRESENT:
 		/* page is swapped out by the host. */
arch/x86/mm/fault.c +12 -35

70 lines not on this page · raw message

maintainer
#10 lore raw

Re: [patch 06/41] x86/traps: Simplify pagefault tracing logic

Quoted 6 lines · Thomas Gleixner

Make use of the new irqvector tracing static key and remove the duplicated trace_do_pagefault() implementation.

If irq vector tracing is disabled, then the overhead of this is a single NOP5, which is a reasonable tradeoff to avoid duplicated code and the unholy macro mess.

This is great. Thanks!

#11 lore raw

Re: [patch 06/41] x86/traps: Simplify pagefault tracing logic

In reply to Thomas Gleixner
Quoted 15 lines · On Fri, 25 Aug 2017 12:31:09 +0200 Thomas Gleixner <[email protected]> wrote:

dotraplinkage void notrace

arch/x86/mm/fault.c +7 -5
-trace_do_page_fault(struct pt_regs *regs, unsigned long error_code)
+do_page_fault(struct pt_regs *regs, unsigned long error_code)
 {
+	unsigned long address = read_cr2(); /* Get the faulting address */
+	enum ctx_state prev_state;
+
 	/*
-	 * The exception_enter and tracepoint processing could
-	 * trigger another page faults (user space callchain
-	 * reading) and destroy the original cr2 value, so read
-	 * the faulting address now.
+	 * We must have this function tagged with __kprobes, notrace and call
+	 * read_cr2() before calling anything else. To avoid calling any kind
+	 * of tracing machinery before we've observed the CR2 value.

arch/x86/mm/fault.c trace_page_fault_entries(unsigned long a · line 1537

Quoted 10 lines
arch/x86/mm/fault.c +4 -3
+	 *
+	 * exception_{enter,exit}() contain all sorts of tracepoints.
 	 */
-	unsigned long address = read_cr2();
-	enum ctx_state prev_state;
+	if (trace_irqvectors_enabled())
+		trace_page_fault_entries(address, regs, error_code);
 
 	prev_state = exception_enter();
-	trace_page_fault_entries(address, regs, error_code);

I believe you need to keep the tracing after the "exception_enter()", as for NO_HZ_FULL, that enables RCU again, and the tracepoint needs RCU enabled.

-- Steve

Quoted 7 lines
arch/x86/mm/fault.c +1 -2
 	__do_page_fault(regs, error_code, address);
 	exception_exit(prev_state);
 }
-NOKPROBE_SYMBOL(trace_do_page_fault);
-#endif /* CONFIG_TRACING */
+NOKPROBE_SYMBOL(do_page_fault);
#12 lore raw

Re: [patch 06/41] x86/traps: Simplify pagefault tracing logic

Quoted 26 lines · Steven Rostedt
Quoted 23 lines · On Fri, 25 Aug 2017 12:31:09 +0200 Thomas Gleixner <[email protected]> wrote:

dotraplinkage void notrace

arch/x86/mm/fault.c +11 -7
-trace_do_page_fault(struct pt_regs *regs, unsigned long error_code)
+do_page_fault(struct pt_regs *regs, unsigned long error_code)
 {
+	unsigned long address = read_cr2(); /* Get the faulting address */
+	enum ctx_state prev_state;
+
 	/*
-	 * The exception_enter and tracepoint processing could
-	 * trigger another page faults (user space callchain
-	 * reading) and destroy the original cr2 value, so read
-	 * the faulting address now.
+	 * We must have this function tagged with __kprobes, notrace and call
+	 * read_cr2() before calling anything else. To avoid calling any kind
+	 * of tracing machinery before we've observed the CR2 value.
+	 *
+	 * exception_{enter,exit}() contain all sorts of tracepoints.
 	 */
-	unsigned long address = read_cr2();
-	enum ctx_state prev_state;
+	if (trace_irqvectors_enabled())
+		trace_page_fault_entries(address, regs, error_code);
Quoted 6 lines

arch/x86/mm/fault.c trace_page_fault_entries(unsigned long a · line 1537

Quoted 2 lines
arch/x86/mm/fault.c +0 -1
 	prev_state = exception_enter();
-	trace_page_fault_entries(address, regs, error_code);

I believe you need to keep the tracing after the "exception_enter()", as for NO_HZ_FULL, that enables RCU again, and the tracepoint needs RCU enabled.

Indeed. Thanks for spotting it.

#13 lore raw

PATCH 7/41 x86/apic: Remove the duplicated tracing version of local_timer_interrupt

In reply to Thomas Gleixner

The two NOP5 are noise in the rest of the work which is done by the timer interrupt and modern CPUs are pretty good in optimizing nops.

Get rid of the interrupt handler duplication and move the tracepoints into the regular handler.

---
 arch/x86/include/asm/hw_irq.h |    2 +-
 arch/x86/kernel/apic/apic.c   |   19 -------------------
 2 files changed, 1 insertion(+), 20 deletions(-)
arch/x86/include/asm/hw_irq.h +1 -1
@@ -48,7 +48,6 @@ extern asmlinkage void call_function_sin
 
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
-extern void trace_apic_timer_interrupt(void);
 extern void trace_x86_platform_ipi(void);
 extern void trace_error_interrupt(void);
 extern void trace_irq_work_interrupt(void);
@@ -59,6 +58,7 @@ extern void trace_threshold_interrupt(vo
 extern void trace_deferred_error_interrupt(void);
 extern void trace_call_function_interrupt(void);
 extern void trace_call_function_single_interrupt(void);
+#define trace_apic_timer_interrupt	apic_timer_interrupt
 #define trace_irq_move_cleanup_interrupt  irq_move_cleanup_interrupt
 #define trace_reboot_interrupt  reboot_interrupt
 #define trace_kvm_posted_intr_ipi kvm_posted_intr_ipi
arch/x86/kernel/apic/apic.c +0 -19
@@ -1040,25 +1040,6 @@ static void local_apic_timer_interrupt(v
 	 * interrupt lock, which is the WrongThing (tm) to do.
 	 */
 	entering_ack_irq();
-	local_apic_timer_interrupt();
-	exiting_irq();
-
-	set_irq_regs(old_regs);
-}
-
-__visible void __irq_entry smp_trace_apic_timer_interrupt(struct pt_regs *regs)
-{
-	struct pt_regs *old_regs = set_irq_regs(regs);
-
-	/*
-	 * NOTE! We'd better ACK the irq immediately,
-	 * because timer handling can be slow.
-	 *
-	 * update_process_times() expects us to have done irq_enter().
-	 * Besides, if we don't timer interrupts ignore the global
-	 * interrupt lock, which is the WrongThing (tm) to do.
-	 */
-	entering_ack_irq();
 	trace_local_timer_entry(LOCAL_TIMER_VECTOR);
 	local_apic_timer_interrupt();
 	trace_local_timer_exit(LOCAL_TIMER_VECTOR);
Reviewed-by on v1 7/41
#14 lore raw

Re: [patch 07/41] x86/apic: Remove the duplicated tracing version of local_timer_interrupt

Quoted 7 lines · On Fri, 25 Aug 2017 12:31:10 +0200 Thomas Gleixner <[email protected]> wrote:

The two NOP5 are noise in the rest of the work which is done by the timer interrupt and modern CPUs are pretty good in optimizing nops.

Get rid of the interrupt handler duplication and move the tracepoints into the regular handler.

-- Steve

#15 lore raw

PATCH 8/41 x86/apic: Use this_cpu_ptr in local_timer_interrupt

In reply to Thomas Gleixner

Accessing the per cpu data via per_cpu(, smp_processor_id()) is pointless. Use this_cpu_ptr() instead.

---
 arch/x86/kernel/apic/apic.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/x86/kernel/apic/apic.c +3 -3
@@ -990,8 +990,7 @@ void setup_secondary_APIC_clock(void)
  */
 static void local_apic_timer_interrupt(void)
 {
-	int cpu = smp_processor_id();
-	struct clock_event_device *evt = &per_cpu(lapic_events, cpu);
+	struct clock_event_device *evt = this_cpu_ptr(&lapic_events);
 
 	/*
 	 * Normally we should not be here till LAPIC has been initialized but
@@ -1005,7 +1004,8 @@ static void local_apic_timer_interrupt(v
 	 * spurious.
 	 */
 	if (!evt->event_handler) {
-		pr_warning("Spurious LAPIC timer interrupt on cpu %d\n", cpu);
+		pr_warning("Spurious LAPIC timer interrupt on cpu %d\n",
+			   smp_processor_id());
 		/* Switch it off */
 		lapic_timer_shutdown(evt);
 		return;
#16 lore raw

PATCH 9/41 x86/irq: Get rid of duplicated trace_x86_platform_ipi() code

In reply to Thomas Gleixner

Two NOP5 are really a good tradeoff vs. the unholy IDT switching mess, which duplicates code all over the place.

---
 arch/x86/include/asm/hw_irq.h |    2 +-
 arch/x86/kernel/irq.c         |   25 +++++--------------------
 2 files changed, 6 insertions(+), 21 deletions(-)
arch/x86/include/asm/hw_irq.h +1 -1
@@ -48,7 +48,6 @@ extern asmlinkage void call_function_sin
 
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
-extern void trace_x86_platform_ipi(void);
 extern void trace_error_interrupt(void);
 extern void trace_irq_work_interrupt(void);
 extern void trace_spurious_interrupt(void);
@@ -58,6 +57,7 @@ extern void trace_threshold_interrupt(vo
 extern void trace_deferred_error_interrupt(void);
 extern void trace_call_function_interrupt(void);
 extern void trace_call_function_single_interrupt(void);
+#define trace_x86_platform_ipi	x86_platform_ipi
 #define trace_apic_timer_interrupt	apic_timer_interrupt
 #define trace_irq_move_cleanup_interrupt  irq_move_cleanup_interrupt
 #define trace_reboot_interrupt  reboot_interrupt
arch/x86/kernel/irq.c +5 -20
@@ -262,20 +262,16 @@ u64 arch_irq_stat(void)
 /*
  * Handler for X86_PLATFORM_IPI_VECTOR.
  */
-void __smp_x86_platform_ipi(void)
-{
-	inc_irq_stat(x86_platform_ipis);
-
-	if (x86_platform_ipi_callback)
-		x86_platform_ipi_callback();
-}
-
 __visible void __irq_entry smp_x86_platform_ipi(struct pt_regs *regs)
 {
 	struct pt_regs *old_regs = set_irq_regs(regs);
 
 	entering_ack_irq();
-	__smp_x86_platform_ipi();
+	trace_x86_platform_ipi_entry(X86_PLATFORM_IPI_VECTOR);
+	inc_irq_stat(x86_platform_ipis);
+	if (x86_platform_ipi_callback)
+		x86_platform_ipi_callback();
+	trace_x86_platform_ipi_exit(X86_PLATFORM_IPI_VECTOR);
 	exiting_irq();
 	set_irq_regs(old_regs);
 }
@@ -334,17 +330,6 @@ EXPORT_SYMBOL_GPL(kvm_set_posted_intr_wa
 }
 #endif
 
-__visible void __irq_entry smp_trace_x86_platform_ipi(struct pt_regs *regs)
-{
-	struct pt_regs *old_regs = set_irq_regs(regs);
-
-	entering_ack_irq();
-	trace_x86_platform_ipi_entry(X86_PLATFORM_IPI_VECTOR);
-	__smp_x86_platform_ipi();
-	trace_x86_platform_ipi_exit(X86_PLATFORM_IPI_VECTOR);
-	exiting_irq();
-	set_irq_regs(old_regs);
-}
 
 #ifdef CONFIG_HOTPLUG_CPU
 
Reviewed-by on v1 9/41
#17 lore raw

Re: [patch 09/41] x86/irq: Get rid of duplicated trace_x86_platform_ipi() code

Quoted 4 lines · On Fri, 25 Aug 2017 12:31:12 +0200 Thomas Gleixner <[email protected]> wrote:

Two NOP5 are really a good tradeoff vs. the unholy IDT switching mess, which duplicates code all over the place.

-- Steve

Quoted 5 lines
---
 arch/x86/include/asm/hw_irq.h |    2 +-
 arch/x86/kernel/irq.c         |   25 +++++--------------------
 2 files changed, 6 insertions(+), 21 deletions(-)
#18 lore raw

PATCH 10/41 x86/apic: Remove the duplicated tracing versions of interrupts

In reply to Thomas Gleixner

The error and the spurious interrupt are really rare events and not at all so performance sensitive that two NOP5s can not be tolerated when tracing is disabled.

Remove the nonsense.

---
 arch/x86/include/asm/hw_irq.h |    4 +--
 arch/x86/kernel/apic/apic.c   |   43 +++++++++---------------------------------
 2 files changed, 12 insertions(+), 35 deletions(-)
arch/x86/include/asm/hw_irq.h +2 -2
@@ -48,15 +48,15 @@ extern asmlinkage void call_function_sin
 
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
-extern void trace_error_interrupt(void);
 extern void trace_irq_work_interrupt(void);
-extern void trace_spurious_interrupt(void);
 extern void trace_thermal_interrupt(void);
 extern void trace_reschedule_interrupt(void);
 extern void trace_threshold_interrupt(void);
 extern void trace_deferred_error_interrupt(void);
 extern void trace_call_function_interrupt(void);
 extern void trace_call_function_single_interrupt(void);
+#define trace_error_interrupt error_interrupt
+#define trace_spurious_interrupt spurious_interrupt
 #define trace_x86_platform_ipi	x86_platform_ipi
 #define trace_apic_timer_interrupt	apic_timer_interrupt
 #define trace_irq_move_cleanup_interrupt  irq_move_cleanup_interrupt
arch/x86/kernel/apic/apic.c +10 -33

78 lines not on this page · raw message

Reviewed-by on v1 10/41
#19 lore raw

Re: [patch 10/41] x86/apic: Remove the duplicated tracing versions of interrupts

Quoted 3 lines · On Fri, 25 Aug 2017 12:31:13 +0200 Thomas Gleixner <[email protected]> wrote:

The error and the spurious interrupt are really rare events and not at all so performance sensitive that two NOP5s can not be tolerated when tracing is disabled.

Just a note. I'm sure if we disassembled it, it may be a little more work done than just two NOPs, as parameter passing to the tracepoints sometimes leak out of the static jump block. It's moot on this patch, but other irqs with fast paths may need to be looked at.

Quoted 9 lines

Remove the nonsense.

---
 arch/x86/include/asm/hw_irq.h |    4 +--
 arch/x86/kernel/apic/apic.c   |   43 +++++++++---------------------------------
 2 files changed, 12 insertions(+), 35 deletions(-)

-- Steve

maintainer
#20 lore raw

Re: [patch 10/41] x86/apic: Remove the duplicated tracing versions of interrupts

Quoted 11 lines · Steven Rostedt
Quoted 3 lines · On Fri, 25 Aug 2017 12:31:13 +0200 Thomas Gleixner <[email protected]> wrote:

The error and the spurious interrupt are really rare events and not at all so performance sensitive that two NOP5s can not be tolerated when tracing is disabled.

Just a note. I'm sure if we disassembled it, it may be a little more work done than just two NOPs, as parameter passing to the tracepoints sometimes leak out of the static jump block. It's moot on this patch, but other irqs with fast paths may need to be looked at.

Is that something we can fix with the trace macros?

They have a general shape of:

#define trace_foo(args...)
	if (static_branch_unlikely(&foo_enabled)) {
		__trace_foo(args...);
	}

Right? And I suppose I see why the compiler would want to sometimes lift stuff out of the branch block, but we'd really like it not to do that. Would putting a barrier() in front of __trace_foo() help?

#21 lore raw

PATCH 11/41 x86/irqwork: Get rid of duplicated tracing interrupt code

In reply to Thomas Gleixner

Two NOP5 are a reasonable tradeoff to avoid duplicated code and the requirement to switch the IDT.

---
 arch/x86/include/asm/hw_irq.h |    2 +-
 arch/x86/kernel/irq_work.c    |   16 ++--------------
 2 files changed, 3 insertions(+), 15 deletions(-)
arch/x86/include/asm/hw_irq.h +1 -1
@@ -48,13 +48,13 @@ extern asmlinkage void call_function_sin
 
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
-extern void trace_irq_work_interrupt(void);
 extern void trace_thermal_interrupt(void);
 extern void trace_reschedule_interrupt(void);
 extern void trace_threshold_interrupt(void);
 extern void trace_deferred_error_interrupt(void);
 extern void trace_call_function_interrupt(void);
 extern void trace_call_function_single_interrupt(void);
+#define trace_irq_work_interrupt irq_work_interrupt
 #define trace_error_interrupt error_interrupt
 #define trace_spurious_interrupt spurious_interrupt
 #define trace_x86_platform_ipi	x86_platform_ipi
arch/x86/kernel/irq_work.c +2 -14
@@ -11,24 +11,12 @@
 #include <asm/trace/irq_vectors.h>
 #include <linux/interrupt.h>
 
-static inline void __smp_irq_work_interrupt(void)
-{
-	inc_irq_stat(apic_irq_work_irqs);
-	irq_work_run();
-}
-
 __visible void __irq_entry smp_irq_work_interrupt(struct pt_regs *regs)
 {
 	ipi_entering_ack_irq();
-	__smp_irq_work_interrupt();
-	exiting_irq();
-}
-
-__visible void __irq_entry smp_trace_irq_work_interrupt(struct pt_regs *regs)
-{
-	ipi_entering_ack_irq();
+	inc_irq_stat(apic_irq_work_irqs);
 	trace_irq_work_entry(IRQ_WORK_VECTOR);
-	__smp_irq_work_interrupt();
+	irq_work_run();
 	trace_irq_work_exit(IRQ_WORK_VECTOR);
 	exiting_irq();
 }
#22 lore raw

Re: [patch 11/41] x86/irqwork: Get rid of duplicated tracing interrupt code

Quoted 15 lines · On Fri, 25 Aug 2017 12:31:14 +0200 Thomas Gleixner <[email protected]> wrote:
arch/x86/kernel/irq_work.c +0 -6
@@ -11,24 +11,12 @@
 #include <asm/trace/irq_vectors.h>
 #include <linux/interrupt.h>
 
-static inline void __smp_irq_work_interrupt(void)
-{
-	inc_irq_stat(apic_irq_work_irqs);
-	irq_work_run();
-}
-
 __visible void __irq_entry smp_irq_work_interrupt(struct pt_regs *regs)
 {
 	ipi_entering_ack_irq();

arch/x86/kernel/irq_work.c line 31

Quoted 10 lines
arch/x86/kernel/irq_work.c +1 -8
-	__smp_irq_work_interrupt();
-	exiting_irq();
-}
-
-__visible void __irq_entry smp_trace_irq_work_interrupt(struct pt_regs *regs)
-{
-	ipi_entering_ack_irq();
+	inc_irq_stat(apic_irq_work_irqs);
 	trace_irq_work_entry(IRQ_WORK_VECTOR);
-	__smp_irq_work_interrupt();

This changes from what we use to have:

Original:

	ipi_entering_ack_irq();
	trace_irq_work_entry(IRQ_WORK_VECTOR);
	inc_irq_stat(apic_irq_work_irqs);
	irq_work_run();

Patched:

	ipi_entering_ack_irq();
	inc_irq_stat(apic_irq_work_irqs);
	trace_irq_work_entry(IRQ_WORK_VECTOR);
	irq_work_run();

Is there a reason for the swap? It's not mentioned in the change log.

Also, tracepoints can issue new irq works. I don't think that would be a problem here, as interrupts are disabled in this path. Just wanted to point it out.

-- Steve

Quoted 5 lines
arch/x86/kernel/irq_work.c +1 -0
+	irq_work_run();
 	trace_irq_work_exit(IRQ_WORK_VECTOR);
 	exiting_irq();
 }
#23 lore raw

PATCH 12/41 x86/mce: Remove duplicated tracing interrupt code

In reply to Thomas Gleixner

Machine checks are not really high frequency events. The extra two NOP5 for the disabled tracepoints are noise vs. the heavy lifting which needs to be done in the MCE handler.

---
 arch/x86/include/asm/hw_irq.h            |    6 +++---
 arch/x86/kernel/cpu/mcheck/mce_amd.c     |   16 ++--------------
 arch/x86/kernel/cpu/mcheck/therm_throt.c |   20 +++-----------------
 arch/x86/kernel/cpu/mcheck/threshold.c   |   16 ++--------------
 4 files changed, 10 insertions(+), 48 deletions(-)
arch/x86/include/asm/hw_irq.h +3 -3
@@ -48,12 +48,12 @@ extern asmlinkage void call_function_sin
 
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
-extern void trace_thermal_interrupt(void);
 extern void trace_reschedule_interrupt(void);
-extern void trace_threshold_interrupt(void);
-extern void trace_deferred_error_interrupt(void);
 extern void trace_call_function_interrupt(void);
 extern void trace_call_function_single_interrupt(void);
+#define trace_thermal_interrupt	thermal_interrupt
+#define trace_threshold_interrupt	threshold_interrupt
+#define trace_deferred_error_interrupt	deferred_error_interrupt
 #define trace_irq_work_interrupt irq_work_interrupt
 #define trace_error_interrupt error_interrupt
 #define trace_spurious_interrupt spurious_interrupt
arch/x86/kernel/cpu/mcheck/mce_amd.c +2 -14
@@ -776,24 +776,12 @@ static void __log_error(unsigned int ban
 	mce_log(&m);
 }
 
-static inline void __smp_deferred_error_interrupt(void)
-{
-	inc_irq_stat(irq_deferred_error_count);
-	deferred_error_int_vector();
-}
-
 asmlinkage __visible void __irq_entry smp_deferred_error_interrupt(void)
 {
 	entering_irq();
-	__smp_deferred_error_interrupt();
-	exiting_ack_irq();
-}
-
-asmlinkage __visible void __irq_entry smp_trace_deferred_error_interrupt(void)
-{
-	entering_irq();
 	trace_deferred_error_apic_entry(DEFERRED_ERROR_VECTOR);
-	__smp_deferred_error_interrupt();
+	inc_irq_stat(irq_deferred_error_count);
+	deferred_error_int_vector();
 	trace_deferred_error_apic_exit(DEFERRED_ERROR_VECTOR);
 	exiting_ack_irq();
 }
arch/x86/kernel/cpu/mcheck/therm_throt.c +3 -17
@@ -390,26 +390,12 @@ static void unexpected_thermal_interrupt
 
 static void (*smp_thermal_vector)(void) = unexpected_thermal_interrupt;
 
-static inline void __smp_thermal_interrupt(void)
-{
-	inc_irq_stat(irq_thermal_count);
-	smp_thermal_vector();
-}
-
-asmlinkage __visible void __irq_entry
-smp_thermal_interrupt(struct pt_regs *regs)
-{
-	entering_irq();
-	__smp_thermal_interrupt();
-	exiting_ack_irq();
-}
-
-asmlinkage __visible void __irq_entry
-smp_trace_thermal_interrupt(struct pt_regs *regs)
+asmlinkage __visible void __irq_entry smp_thermal_interrupt(struct pt_regs *r)
 {
 	entering_irq();
+	inc_irq_stat(irq_thermal_count);
 	trace_thermal_apic_entry(THERMAL_APIC_VECTOR);
-	__smp_thermal_interrupt();
+	smp_thermal_vector();
 	trace_thermal_apic_exit(THERMAL_APIC_VECTOR);
 	exiting_ack_irq();
 }
arch/x86/kernel/cpu/mcheck/threshold.c +2 -14
@@ -17,24 +17,12 @@ static void default_threshold_interrupt(
 
 void (*mce_threshold_vector)(void) = default_threshold_interrupt;
 
-static inline void __smp_threshold_interrupt(void)
-{
-	inc_irq_stat(irq_threshold_count);
-	mce_threshold_vector();
-}
-
 asmlinkage __visible void __irq_entry smp_threshold_interrupt(void)
 {
 	entering_irq();
-	__smp_threshold_interrupt();
-	exiting_ack_irq();
-}
-
-asmlinkage __visible void __irq_entry smp_trace_threshold_interrupt(void)
-{
-	entering_irq();
 	trace_threshold_apic_entry(THRESHOLD_APIC_VECTOR);
-	__smp_threshold_interrupt();
+	inc_irq_stat(irq_threshold_count);
+	mce_threshold_vector();
 	trace_threshold_apic_exit(THRESHOLD_APIC_VECTOR);
 	exiting_ack_irq();
 }
#24 lore raw

Re: [patch 12/41] x86/mce: Remove duplicated tracing interrupt code

Quoted 15 lines · On Fri, 25 Aug 2017 12:31:15 +0200 Thomas Gleixner <[email protected]> wrote:
arch/x86/kernel/cpu/mcheck/mce_amd.c +0 -6
@@ -776,24 +776,12 @@ static void __log_error(unsigned int ban
 	mce_log(&m);
 }
 
-static inline void __smp_deferred_error_interrupt(void)
-{
-	inc_irq_stat(irq_deferred_error_count);
-	deferred_error_int_vector();
-}
-
 asmlinkage __visible void __irq_entry smp_deferred_error_interrupt(void)
 {
 	entering_irq();

arch/x86/kernel/cpu/mcheck/mce_amd.c static void __log_error(unsigned int ban · line 783

Quoted 10 lines
arch/x86/kernel/cpu/mcheck/mce_amd.c +1 -8
-	__smp_deferred_error_interrupt();
-	exiting_ack_irq();
-}
-
-asmlinkage __visible void __irq_entry smp_trace_deferred_error_interrupt(void)
-{
-	entering_irq();
 	trace_deferred_error_apic_entry(DEFERRED_ERROR_VECTOR);
-	__smp_deferred_error_interrupt();
+	inc_irq_stat(irq_deferred_error_count);

Here we do the inc_irq_stat() after the tracepoint (same as the original).

Quoted 21 lines
arch/x86/kernel/cpu/mcheck/mce_amd.c +1 -0
+	deferred_error_int_vector();
 	trace_deferred_error_apic_exit(DEFERRED_ERROR_VECTOR);
 	exiting_ack_irq();
 }
arch/x86/kernel/cpu/mcheck/therm_throt.c +0 -11
@@ -390,26 +390,12 @@ static void unexpected_thermal_interrupt
 
 static void (*smp_thermal_vector)(void) = unexpected_thermal_interrupt;
 
-static inline void __smp_thermal_interrupt(void)
-{
-	inc_irq_stat(irq_thermal_count);
-	smp_thermal_vector();
-}
-
-asmlinkage __visible void __irq_entry
-smp_thermal_interrupt(struct pt_regs *regs)
-{
-	entering_irq();
-	__smp_thermal_interrupt();

arch/x86/kernel/cpu/mcheck/therm_throt.c static void unexpected_thermal_interrupt · line 397

Quoted 10 lines
arch/x86/kernel/cpu/mcheck/therm_throt.c +2 -5
-	exiting_ack_irq();
-}
-
-asmlinkage __visible void __irq_entry
-smp_trace_thermal_interrupt(struct pt_regs *regs)
+asmlinkage __visible void __irq_entry smp_thermal_interrupt(struct pt_regs *r)
 {
 	entering_irq();
+	inc_irq_stat(irq_thermal_count);
 	trace_thermal_apic_entry(THERMAL_APIC_VECTOR);

Here we do the inc_irq_stat() after the tracepoint (swapped from the original). Was that intentional?

Quoted 20 lines
arch/x86/kernel/cpu/mcheck/therm_throt.c +1 -1
-	__smp_thermal_interrupt();
+	smp_thermal_vector();
 	trace_thermal_apic_exit(THERMAL_APIC_VECTOR);
 	exiting_ack_irq();
 }
arch/x86/kernel/cpu/mcheck/threshold.c +0 -6
@@ -17,24 +17,12 @@ static void default_threshold_interrupt(
 
 void (*mce_threshold_vector)(void) = default_threshold_interrupt;
 
-static inline void __smp_threshold_interrupt(void)
-{
-	inc_irq_stat(irq_threshold_count);
-	mce_threshold_vector();
-}
-
 asmlinkage __visible void __irq_entry smp_threshold_interrupt(void)
 {
 	entering_irq();

arch/x86/kernel/cpu/mcheck/threshold.c static void default_threshold_interrupt( · line 24

Quoted 10 lines
arch/x86/kernel/cpu/mcheck/threshold.c +1 -8
-	__smp_threshold_interrupt();
-	exiting_ack_irq();
-}
-
-asmlinkage __visible void __irq_entry smp_trace_threshold_interrupt(void)
-{
-	entering_irq();
 	trace_threshold_apic_entry(THRESHOLD_APIC_VECTOR);
-	__smp_threshold_interrupt();
+	inc_irq_stat(irq_threshold_count);

Back to inc_irq_stat() after the tracepoint.

-- Steve

Quoted 5 lines
arch/x86/kernel/cpu/mcheck/threshold.c +1 -0
+	mce_threshold_vector();
 	trace_threshold_apic_exit(THRESHOLD_APIC_VECTOR);
 	exiting_ack_irq();
 }
#25 lore raw

Re: [patch 12/41] x86/mce: Remove duplicated tracing interrupt code

Quoted 8 lines · Steven Rostedt

arch/x86/kernel/cpu/mcheck/mce_amd.c static void __log_error(unsigned int ban · line 783

Quoted 3 lines · On Fri, 25 Aug 2017 12:31:15 +0200 Thomas Gleixner <[email protected]> wrote:
arch/x86/kernel/cpu/mcheck/mce_amd.c +1 -1
 	trace_deferred_error_apic_entry(DEFERRED_ERROR_VECTOR);
-	__smp_deferred_error_interrupt();
+	inc_irq_stat(irq_deferred_error_count);

Here we do the inc_irq_stat() after the tracepoint (same as the original).

Quoted 6 lines

arch/x86/kernel/cpu/mcheck/therm_throt.c static void unexpected_thermal_interrupt · line 397

Quoted 3 lines
arch/x86/kernel/cpu/mcheck/therm_throt.c +1 -0
 	entering_irq();
+	inc_irq_stat(irq_thermal_count);
 	trace_thermal_apic_entry(THERMAL_APIC_VECTOR);

Here we do the inc_irq_stat() after the tracepoint (swapped from the original). Was that intentional?

No. Not that it matters at all, but I can make it consistent everywhere.

#26 lore raw

PATCH 13/41 x86/smp: Remove pointless duplicated interrupt code

In reply to Thomas Gleixner

Two NOP5 are really a good tradeoff vs. the unholy IDT switching mess, which duplicates code all over the place. The rescheduling interrupt gets optimized in a later step.

---
 arch/x86/include/asm/hw_irq.h |    4 +--
 arch/x86/kernel/smp.c         |   43 ++++++------------------------------------
 2 files changed, 9 insertions(+), 38 deletions(-)
arch/x86/include/asm/hw_irq.h +2 -2
@@ -49,8 +49,8 @@ extern asmlinkage void call_function_sin
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
 extern void trace_reschedule_interrupt(void);
-extern void trace_call_function_interrupt(void);
-extern void trace_call_function_single_interrupt(void);
+#define trace_call_function_interrupt	call_function_interrupt
+#define trace_call_function_single_interrupt	call_function_single_interrupt
 #define trace_thermal_interrupt	thermal_interrupt
 #define trace_threshold_interrupt	threshold_interrupt
 #define trace_deferred_error_interrupt	deferred_error_interrupt
arch/x86/kernel/smp.c +7 -36

64 lines not on this page · raw message

#27 lore raw

Re: [patch 13/41] x86/smp: Remove pointless duplicated interrupt code

arch/x86/kernel/smp.c static inline void __smp_reschedule_inte · line 295

Quoted 4 lines · On Fri, 25 Aug 2017 12:31:16 +0200 Thomas Gleixner <[email protected]> wrote:
arch/x86/kernel/smp.c +0 -4
-static inline void __smp_call_function_interrupt(void)
-{
-	generic_smp_call_function_interrupt();
-	inc_irq_stat(irq_call_count);

Interesting. The origin code calls the generic_smp_call_function_interrupt() and then calls inc_irq_stat(). Not sure it matters, but just stating this because...

Quoted 21 lines

}

__visible void __irq_entry smp_call_function_interrupt(struct pt_regs *regs) {

arch/x86/kernel/smp.c +0 -15
 	ipi_entering_ack_irq();
-	__smp_call_function_interrupt();
-	exiting_irq();
-}
-
-__visible void __irq_entry
-smp_trace_call_function_interrupt(struct pt_regs *regs)
-{
-	ipi_entering_ack_irq();
 	trace_call_function_entry(CALL_FUNCTION_VECTOR);
-	__smp_call_function_interrupt();
-	trace_call_function_exit(CALL_FUNCTION_VECTOR);
-	exiting_irq();
-}
-
-static inline void __smp_call_function_single_interrupt(void)
-{

arch/x86/kernel/smp.c static inline void __smp_reschedule_inte · line 295

Quoted 10 lines
arch/x86/kernel/smp.c +1 -8
-	generic_smp_call_function_single_interrupt();
 	inc_irq_stat(irq_call_count);
-}
-
-__visible void __irq_entry
-smp_call_function_single_interrupt(struct pt_regs *regs)
-{
-	ipi_entering_ack_irq();
-	__smp_call_function_single_interrupt();
+	generic_smp_call_function_interrupt();

here and

arch/x86/kernel/smp.c static inline void __smp_reschedule_inte · line 305

Quoted 13 lines
arch/x86/kernel/smp.c +4 -3
+	trace_call_function_exit(CALL_FUNCTION_VECTOR);
 	exiting_irq();
 }
 
-__visible void __irq_entry
-smp_trace_call_function_single_interrupt(struct pt_regs *regs)
+__visible void __irq_entry smp_call_function_single_interrupt(struct pt_regs *r)
 {
 	ipi_entering_ack_irq();
 	trace_call_function_single_entry(CALL_FUNCTION_SINGLE_VECTOR);
-	__smp_call_function_single_interrupt();
+	inc_irq_stat(irq_call_count);
+	generic_smp_call_function_single_interrupt();

here, you reverse it.

Should at least be mentioned in the change log.

-- Steve

Quoted 4 lines
arch/x86/kernel/smp.c +0 -0
 	trace_call_function_single_exit(CALL_FUNCTION_SINGLE_VECTOR);
 	exiting_irq();
 }
#28 lore raw

PATCH 14/41 x86/smp: Use static key for reschedule interrupt tracing

In reply to Thomas Gleixner

It's worth to avoid the extra irq_enter()/irq_exit() pair in the case that the reschedule interrupt tracepoints are disabled.

Use the static key which indicates that exception tracing is enabled. For now this key is global. It will be optimized in a later step.

---
 arch/x86/include/asm/hw_irq.h |    2 +-
 arch/x86/kernel/smp.c         |   40 ++++++++++++++++------------------------
 2 files changed, 17 insertions(+), 25 deletions(-)
arch/x86/include/asm/hw_irq.h +1 -1
@@ -48,7 +48,7 @@ extern asmlinkage void call_function_sin
 
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
-extern void trace_reschedule_interrupt(void);
+#define trace_reschedule_interrupt	reschedule_interrupt
 #define trace_call_function_interrupt	call_function_interrupt
 #define trace_call_function_single_interrupt	call_function_single_interrupt
 #define trace_thermal_interrupt	thermal_interrupt
arch/x86/kernel/smp.c +16 -24
@@ -254,37 +254,29 @@ static void native_stop_other_cpus(int w
 }
 
 /*
- * Reschedule call back.
+ * Reschedule call back. KVM uses this interrupt to force a cpu out of
+ * guest mode
  */
-static inline void __smp_reschedule_interrupt(void)
-{
-	inc_irq_stat(irq_resched_count);
-	scheduler_ipi();
-}
-
 __visible void __irq_entry smp_reschedule_interrupt(struct pt_regs *regs)
 {
 	ack_APIC_irq();
-	__smp_reschedule_interrupt();
-	/*
-	 * KVM uses this interrupt to force a cpu out of guest mode
-	 */
-}
 
-__visible void __irq_entry smp_trace_reschedule_interrupt(struct pt_regs *regs)
-{
-	/*
-	 * Need to call irq_enter() before calling the trace point.
-	 * __smp_reschedule_interrupt() calls irq_enter/exit() too (in
-	 * scheduler_ipi(). This is OK, since those functions are allowed
-	 * to nest.
-	 */
-	ipi_entering_ack_irq();
-	trace_reschedule_entry(RESCHEDULE_VECTOR);
+	if (trace_irqvectors_enabled()) {
+		/*
+		 * scheduler_ipi() might call irq_enter() as well, but
+		 * nested calls are fine.
+		 */
+		irq_enter();
+		trace_reschedule_entry(RESCHEDULE_VECTOR);
+	}
+
 	inc_irq_stat(irq_resched_count);
 	scheduler_ipi();
-	trace_reschedule_exit(RESCHEDULE_VECTOR);
-	exiting_irq();
+
+	if (trace_irqvectors_enabled()) {
+		trace_reschedule_exit(RESCHEDULE_VECTOR);
+		irq_exit();
+	}
 }
 
 __visible void __irq_entry smp_call_function_interrupt(struct pt_regs *regs)
#29 lore raw

Re: [patch 14/41] x86/smp: Use static key for reschedule interrupt tracing

Quoted 17 lines · On Fri, 25 Aug 2017 12:31:17 +0200 Thomas Gleixner <[email protected]> wrote:
arch/x86/kernel/smp.c +7 -10
-__visible void __irq_entry smp_trace_reschedule_interrupt(struct pt_regs *regs)
-{
-	/*
-	 * Need to call irq_enter() before calling the trace point.
-	 * __smp_reschedule_interrupt() calls irq_enter/exit() too (in
-	 * scheduler_ipi(). This is OK, since those functions are allowed
-	 * to nest.
-	 */
-	ipi_entering_ack_irq();
-	trace_reschedule_entry(RESCHEDULE_VECTOR);
+	if (trace_irqvectors_enabled()) {
+		/*
+		 * scheduler_ipi() might call irq_enter() as well, but
+		 * nested calls are fine.
+		 */
+		irq_enter();
+		trace_reschedule_entry(RESCHEDULE_VECTOR);

arch/x86/kernel/smp.c static void native_stop_other_cpus(int w · line 278

Quoted 10 lines
arch/x86/kernel/smp.c +6 -2
+	}
+
 	inc_irq_stat(irq_resched_count);
 	scheduler_ipi();
-	trace_reschedule_exit(RESCHEDULE_VECTOR);
-	exiting_irq();
+
+	if (trace_irqvectors_enabled()) {
+		trace_reschedule_exit(RESCHEDULE_VECTOR);
+		irq_exit();

This is racy. If the static key gets changed on one and not the other, we lose the irq_enter/irq_exit match.

-- Steve

Quoted 5 lines
arch/x86/kernel/smp.c +1 -0
+	}
 }
 
 __visible void __irq_entry smp_call_function_interrupt(struct pt_regs *regs)
#30 lore raw

Re: [patch 14/41] x86/smp: Use static key for reschedule interrupt tracing

Quoted 26 lines · Steven Rostedt
Quoted 22 lines · On Fri, 25 Aug 2017 12:31:17 +0200 Thomas Gleixner <[email protected]> wrote:
(quoted diff) +9 -11
-__visible void __irq_entry smp_trace_reschedule_interrupt(struct pt_regs *regs)
-{
-	/*
-	 * Need to call irq_enter() before calling the trace point.
-	 * __smp_reschedule_interrupt() calls irq_enter/exit() too (in
-	 * scheduler_ipi(). This is OK, since those functions are allowed
-	 * to nest.
-	 */
-	ipi_entering_ack_irq();
-	trace_reschedule_entry(RESCHEDULE_VECTOR);
+	if (trace_irqvectors_enabled()) {
+		/*
+		 * scheduler_ipi() might call irq_enter() as well, but
+		 * nested calls are fine.
+		 */
+		irq_enter();
+		trace_reschedule_entry(RESCHEDULE_VECTOR);
+	}
+
 	inc_irq_stat(irq_resched_count);
 	scheduler_ipi();
-	trace_reschedule_exit(RESCHEDULE_VECTOR);
Quoted 8 lines

Review

Quoted 5 lines
(quoted diff) +4 -1
-	exiting_irq();
+
+	if (trace_irqvectors_enabled()) {
+		trace_reschedule_exit(RESCHEDULE_VECTOR);
+		irq_exit();

This is racy. If the static key gets changed on one and not the other, we lose the irq_enter/irq_exit match.

Indeed. Good point. Fixed up version below.

Thanks,

tglx
8<---------------------

arch/x86/include/asm/hw_irq.h +1 -1
@@ -48,7 +48,7 @@ extern asmlinkage void call_function_sin
 
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
-extern void trace_reschedule_interrupt(void);
+#define trace_reschedule_interrupt	reschedule_interrupt
 #define trace_call_function_interrupt	call_function_interrupt
 #define trace_call_function_single_interrupt	call_function_single_interrupt
 #define trace_thermal_interrupt	thermal_interrupt
arch/x86/kernel/smp.c +15 -25
@@ -254,37 +254,27 @@ static void native_stop_other_cpus(int w
 }
 
 /*
- * Reschedule call back.
+ * Reschedule call back. KVM uses this interrupt to force a cpu out of
+ * guest mode
  */
-static inline void __smp_reschedule_interrupt(void)
-{
-	inc_irq_stat(irq_resched_count);
-	scheduler_ipi();
-}
-
 __visible void __irq_entry smp_reschedule_interrupt(struct pt_regs *regs)
 {
 	ack_APIC_irq();
-	__smp_reschedule_interrupt();
-	/*
-	 * KVM uses this interrupt to force a cpu out of guest mode
-	 */
-}
-
-__visible void __irq_entry smp_trace_reschedule_interrupt(struct pt_regs *regs)
-{
-	/*
-	 * Need to call irq_enter() before calling the trace point.
-	 * __smp_reschedule_interrupt() calls irq_enter/exit() too (in
-	 * scheduler_ipi(). This is OK, since those functions are allowed
-	 * to nest.
-	 */
-	ipi_entering_ack_irq();
-	trace_reschedule_entry(RESCHEDULE_VECTOR);
 	inc_irq_stat(irq_resched_count);
+
+	if (trace_irqvectors_enabled()) {
+		/*
+		 * scheduler_ipi() might call irq_enter() as well, but
+		 * nested calls are fine.
+		 */
+		irq_enter();
+		trace_reschedule_entry(RESCHEDULE_VECTOR);
+		scheduler_ipi();
+		trace_reschedule_exit(RESCHEDULE_VECTOR);
+		irq_exit();
+		return;
+	}
 	scheduler_ipi();
-	trace_reschedule_exit(RESCHEDULE_VECTOR);
-	exiting_irq();
 }
 
 __visible void __irq_entry smp_call_function_interrupt(struct pt_regs *regs)
Reviewed-by
#31 lore raw

Re: [patch 14/41] x86/smp: Use static key for reschedule interrupt tracing

Quoted 2 lines · On Fri, 25 Aug 2017 22:07:33 +0200 (CEST) Thomas Gleixner <[email protected]>…

Fixed up version below.

-- Steve

Quoted 71 lines

Thanks,

tglx
8<---------------------

arch/x86/include/asm/hw_irq.h +1 -1
@@ -48,7 +48,7 @@ extern asmlinkage void call_function_sin
 
 #ifdef CONFIG_TRACING
 /* Interrupt handlers registered during init_IRQ */
-extern void trace_reschedule_interrupt(void);
+#define trace_reschedule_interrupt	reschedule_interrupt
 #define trace_call_function_interrupt	call_function_interrupt
 #define trace_call_function_single_interrupt	call_function_single_interrupt
 #define trace_thermal_interrupt	thermal_interrupt
arch/x86/kernel/smp.c +15 -25
@@ -254,37 +254,27 @@ static void native_stop_other_cpus(int w
 }
 
 /*
- * Reschedule call back.
+ * Reschedule call back. KVM uses this interrupt to force a cpu out of
+ * guest mode
  */
-static inline void __smp_reschedule_interrupt(void)
-{
-	inc_irq_stat(irq_resched_count);
-	scheduler_ipi();
-}
-
 __visible void __irq_entry smp_reschedule_interrupt(struct pt_regs *regs)
 {
 	ack_APIC_irq();
-	__smp_reschedule_interrupt();
-	/*
-	 * KVM uses this interrupt to force a cpu out of guest mode
-	 */
-}
-
-__visible void __irq_entry smp_trace_reschedule_interrupt(struct pt_regs *regs)
-{
-	/*
-	 * Need to call irq_enter() before calling the trace point.
-	 * __smp_reschedule_interrupt() calls irq_enter/exit() too (in
-	 * scheduler_ipi(). This is OK, since those functions are allowed
-	 * to nest.
-	 */
-	ipi_entering_ack_irq();
-	trace_reschedule_entry(RESCHEDULE_VECTOR);
 	inc_irq_stat(irq_resched_count);
+
+	if (trace_irqvectors_enabled()) {
+		/*
+		 * scheduler_ipi() might call irq_enter() as well, but
+		 * nested calls are fine.
+		 */
+		irq_enter();
+		trace_reschedule_entry(RESCHEDULE_VECTOR);
+		scheduler_ipi();
+		trace_reschedule_exit(RESCHEDULE_VECTOR);
+		irq_exit();
+		return;
+	}
 	scheduler_ipi();
-	trace_reschedule_exit(RESCHEDULE_VECTOR);
-	exiting_irq();
 }
 
 __visible void __irq_entry smp_call_function_interrupt(struct pt_regs *regs)
#32 lore raw

PATCH 15/41 x86/idt: Remove tracing idt completely

In reply to Thomas Gleixner

No more users of the tracing idt. All exception tracepoints have been moved into the regular handlers. Get rid of the mess which shouldn't have been created in the first place.

---
 arch/x86/entry/entry_32.S         |   12 -----
 arch/x86/entry/entry_64.S         |   13 ------
 arch/x86/include/asm/desc.h       |   82 ++------------------------------------
 arch/x86/include/asm/entry_arch.h |   14 ++----
 arch/x86/include/asm/hw_irq.h     |   20 ---------
 arch/x86/include/asm/segment.h    |    3 -
 arch/x86/kernel/tracepoint.c      |   43 -------------------
 arch/x86/kernel/traps.c           |    6 --
 8 files changed, 14 insertions(+), 179 deletions(-)
arch/x86/entry/entry_32.S +2 -10
@@ -673,16 +673,8 @@ ENTRY(name)				\
 	jmp	ret_from_intr;		\
 ENDPROC(name)
 
-
-#ifdef CONFIG_TRACING
-# define TRACE_BUILD_INTERRUPT(name, nr)	BUILD_INTERRUPT3(trace_##name, nr, smp_trace_##name)
-#else
-# define TRACE_BUILD_INTERRUPT(name, nr)
-#endif
-
 #define BUILD_INTERRUPT(name, nr)		\
 	BUILD_INTERRUPT3(name, nr, smp_##name);	\
-	TRACE_BUILD_INTERRUPT(name, nr)
 
 /* The include is where all of the SMP etc. interrupts come from */
 #include <asm/entry_arch.h>
@@ -880,14 +872,14 @@ ENTRY(xen_failsafe_callback)
 ENDPROC(xen_failsafe_callback)
 
 BUILD_INTERRUPT3(xen_hvm_callback_vector, HYPERVISOR_CALLBACK_VECTOR,
-		xen_evtchn_do_upcall)
+		 xen_evtchn_do_upcall)
 
 #endif /* CONFIG_XEN */
 
 #if IS_ENABLED(CONFIG_HYPERV)
 
 BUILD_INTERRUPT3(hyperv_callback_vector, HYPERVISOR_CALLBACK_VECTOR,
-	hyperv_vector_handler)
+		 hyperv_vector_handler)
 
 #endif /* CONFIG_HYPERV */
 
arch/x86/entry/entry_64.S +0 -13
@@ -748,18 +748,6 @@ ENTRY(\sym)
 END(\sym)
 .endm
 
-#ifdef CONFIG_TRACING
-#define trace(sym) trace_##sym
-#define smp_trace(sym) smp_trace_##sym
-
-.macro trace_apicinterrupt num sym
-apicinterrupt3 \num trace(\sym) smp_trace(\sym)
-.endm
-#else
-.macro trace_apicinterrupt num sym do_sym
-.endm
-#endif
-
 /* Make sure APIC interrupt handlers end up in the irqentry section: */
 #if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN)
 # define PUSH_SECTION_IRQENTRY	.pushsection .irqentry.text, "ax"
@@ -772,7 +760,6 @@ apicinterrupt3 \num trace(\sym) smp_trac
 .macro apicinterrupt num sym do_sym
 PUSH_SECTION_IRQENTRY
 apicinterrupt3 \num \sym \do_sym
-trace_apicinterrupt \num \sym
 POP_SECTION_IRQENTRY
 .endm
 
arch/x86/include/asm/desc.h +6 -76

108 lines not on this page · raw message

arch/x86/include/asm/entry_arch.h +5 -9
@@ -13,20 +13,16 @@
 BUILD_INTERRUPT(reschedule_interrupt,RESCHEDULE_VECTOR)
 BUILD_INTERRUPT(call_function_interrupt,CALL_FUNCTION_VECTOR)
 BUILD_INTERRUPT(call_function_single_interrupt,CALL_FUNCTION_SINGLE_VECTOR)
-BUILD_INTERRUPT3(irq_move_cleanup_interrupt, IRQ_MOVE_CLEANUP_VECTOR,
-		 smp_irq_move_cleanup_interrupt)
-BUILD_INTERRUPT3(reboot_interrupt, REBOOT_VECTOR, smp_reboot_interrupt)
+BUILD_INTERRUPT(irq_move_cleanup_interrupt, IRQ_MOVE_CLEANUP_VECTOR)
+BUILD_INTERRUPT(reboot_interrupt, REBOOT_VECTOR)
 #endif
 
 BUILD_INTERRUPT(x86_platform_ipi, X86_PLATFORM_IPI_VECTOR)
 
 #ifdef CONFIG_HAVE_KVM
-BUILD_INTERRUPT3(kvm_posted_intr_ipi, POSTED_INTR_VECTOR,
-		 smp_kvm_posted_intr_ipi)
-BUILD_INTERRUPT3(kvm_posted_intr_wakeup_ipi, POSTED_INTR_WAKEUP_VECTOR,
-		 smp_kvm_posted_intr_wakeup_ipi)
-BUILD_INTERRUPT3(kvm_posted_intr_nested_ipi, POSTED_INTR_NESTED_VECTOR,
-		 smp_kvm_posted_intr_nested_ipi)
+BUILD_INTERRUPT(kvm_posted_intr_ipi, POSTED_INTR_VECTOR)
+BUILD_INTERRUPT(kvm_posted_intr_wakeup_ipi, POSTED_INTR_WAKEUP_VECTOR)
+BUILD_INTERRUPT(kvm_posted_intr_nested_ipi, POSTED_INTR_NESTED_VECTOR)
 #endif
 
 /*
arch/x86/include/asm/hw_irq.h +0 -20
@@ -46,26 +46,6 @@ extern asmlinkage void deferred_error_in
 extern asmlinkage void call_function_interrupt(void);
 extern asmlinkage void call_function_single_interrupt(void);
 
-#ifdef CONFIG_TRACING
-/* Interrupt handlers registered during init_IRQ */
-#define trace_reschedule_interrupt	reschedule_interrupt
-#define trace_call_function_interrupt	call_function_interrupt
-#define trace_call_function_single_interrupt	call_function_single_interrupt
-#define trace_thermal_interrupt	thermal_interrupt
-#define trace_threshold_interrupt	threshold_interrupt
-#define trace_deferred_error_interrupt	deferred_error_interrupt
-#define trace_irq_work_interrupt irq_work_interrupt
-#define trace_error_interrupt error_interrupt
-#define trace_spurious_interrupt spurious_interrupt
-#define trace_x86_platform_ipi	x86_platform_ipi
-#define trace_apic_timer_interrupt	apic_timer_interrupt
-#define trace_irq_move_cleanup_interrupt  irq_move_cleanup_interrupt
-#define trace_reboot_interrupt  reboot_interrupt
-#define trace_kvm_posted_intr_ipi kvm_posted_intr_ipi
-#define trace_kvm_posted_intr_wakeup_ipi kvm_posted_intr_wakeup_ipi
-#define trace_kvm_posted_intr_nested_ipi kvm_posted_intr_nested_ipi
-#endif /* CONFIG_TRACING */
-
 #ifdef	CONFIG_X86_LOCAL_APIC
 struct irq_data;
 struct pci_dev;
arch/x86/include/asm/segment.h +0 -3
@@ -238,9 +238,6 @@
 #ifndef __ASSEMBLY__
 
 extern const char early_idt_handler_array[NUM_EXCEPTION_VECTORS][EARLY_IDT_HANDLER_SIZE];
-#ifdef CONFIG_TRACING
-# define trace_early_idt_handler_array early_idt_handler_array
-#endif
 
 /*
  * Load a segment. Fall back on loading the zero segment if something goes
arch/x86/kernel/tracepoint.c +0 -43

58 lines not on this page · raw message

arch/x86/kernel/traps.c +1 -5
@@ -939,12 +939,8 @@ void __init early_trap_init(void)
 	 * stack.  Using the original stack works well enough at this
 	 * early stage. DEBUG_STACK will be equipped after cpu_init() in
 	 * trap_init().
-	 *
-	 * We don't need to set trace_idt_table like set_intr_gate(),
-	 * since we don't have trace_debug and it will be reset to
-	 * 'debug' in trap_init() by set_intr_gate_ist().
 	 */
-	set_intr_gate_notrace(X86_TRAP_DB, debug);
+	set_intr_gate(X86_TRAP_DB, debug);
 	/* int3 can be called from all */
 	set_system_intr_gate(X86_TRAP_BP, &int3);
 #ifdef CONFIG_X86_32
#33 lore raw

Re: [patch 15/41] x86/idt: Remove tracing idt completely

Quoted 32 lines · On Fri, 25 Aug 2017 12:31:18 +0200 Thomas Gleixner <[email protected]> wrote:

No more users of the tracing idt. All exception tracepoints have been moved into the regular handlers. Get rid of the mess which shouldn't have been created in the first place.

---
 arch/x86/entry/entry_32.S         |   12 -----
 arch/x86/entry/entry_64.S         |   13 ------
 arch/x86/include/asm/desc.h       |   82 ++------------------------------------
 arch/x86/include/asm/entry_arch.h |   14 ++----
 arch/x86/include/asm/hw_irq.h     |   20 ---------
 arch/x86/include/asm/segment.h    |    3 -
 arch/x86/kernel/tracepoint.c      |   43 -------------------
 arch/x86/kernel/traps.c           |    6 --
 8 files changed, 14 insertions(+), 179 deletions(-)
arch/x86/include/asm/entry_arch.h +2 -3
@@ -13,20 +13,16 @@
 BUILD_INTERRUPT(reschedule_interrupt,RESCHEDULE_VECTOR)
 BUILD_INTERRUPT(call_function_interrupt,CALL_FUNCTION_VECTOR)
 BUILD_INTERRUPT(call_function_single_interrupt,CALL_FUNCTION_SINGLE_VECTOR)
-BUILD_INTERRUPT3(irq_move_cleanup_interrupt, IRQ_MOVE_CLEANUP_VECTOR,
-		 smp_irq_move_cleanup_interrupt)
-BUILD_INTERRUPT3(reboot_interrupt, REBOOT_VECTOR, smp_reboot_interrupt)
+BUILD_INTERRUPT(irq_move_cleanup_interrupt, IRQ_MOVE_CLEANUP_VECTOR)
+BUILD_INTERRUPT(reboot_interrupt, REBOOT_VECTOR)
 #endif
 
 BUILD_INTERRUPT(x86_platform_ipi, X86_PLATFORM_IPI_VECTOR)
 
 #ifdef CONFIG_HAVE_KVM

arch/x86/include/asm/entry_arch.h line 26

Quoted 10 lines
arch/x86/include/asm/entry_arch.h +3 -6
-BUILD_INTERRUPT3(kvm_posted_intr_ipi, POSTED_INTR_VECTOR,
-		 smp_kvm_posted_intr_ipi)
-BUILD_INTERRUPT3(kvm_posted_intr_wakeup_ipi, POSTED_INTR_WAKEUP_VECTOR,
-		 smp_kvm_posted_intr_wakeup_ipi)
-BUILD_INTERRUPT3(kvm_posted_intr_nested_ipi, POSTED_INTR_NESTED_VECTOR,
-		 smp_kvm_posted_intr_nested_ipi)
+BUILD_INTERRUPT(kvm_posted_intr_ipi, POSTED_INTR_VECTOR)
+BUILD_INTERRUPT(kvm_posted_intr_wakeup_ipi, POSTED_INTR_WAKEUP_VECTOR)
+BUILD_INTERRUPT(kvm_posted_intr_nested_ipi, POSTED_INTR_NESTED_VECTOR)
 #endif

This looks like a different change than what is specified in the change log. It looks like a clean up. Shouldn't it be a separate patch?

-- Steve

#34 lore raw

Re: [patch 15/41] x86/idt: Remove tracing idt completely

Quoted 34 lines · Steven Rostedt
Quoted 31 lines · On Fri, 25 Aug 2017 12:31:18 +0200 Thomas Gleixner <[email protected]> wrote:

No more users of the tracing idt. All exception tracepoints have been moved into the regular handlers. Get rid of the mess which shouldn't have been created in the first place.

---
 arch/x86/entry/entry_32.S         |   12 -----
 arch/x86/entry/entry_64.S         |   13 ------
 arch/x86/include/asm/desc.h       |   82 ++------------------------------------
 arch/x86/include/asm/entry_arch.h |   14 ++----
 arch/x86/include/asm/hw_irq.h     |   20 ---------
 arch/x86/include/asm/segment.h    |    3 -
 arch/x86/kernel/tracepoint.c      |   43 -------------------
 arch/x86/kernel/traps.c           |    6 --
 8 files changed, 14 insertions(+), 179 deletions(-)
arch/x86/include/asm/entry_arch.h +2 -3
@@ -13,20 +13,16 @@
 BUILD_INTERRUPT(reschedule_interrupt,RESCHEDULE_VECTOR)
 BUILD_INTERRUPT(call_function_interrupt,CALL_FUNCTION_VECTOR)
 BUILD_INTERRUPT(call_function_single_interrupt,CALL_FUNCTION_SINGLE_VECTOR)
-BUILD_INTERRUPT3(irq_move_cleanup_interrupt, IRQ_MOVE_CLEANUP_VECTOR,
-		 smp_irq_move_cleanup_interrupt)
-BUILD_INTERRUPT3(reboot_interrupt, REBOOT_VECTOR, smp_reboot_interrupt)
+BUILD_INTERRUPT(irq_move_cleanup_interrupt, IRQ_MOVE_CLEANUP_VECTOR)
+BUILD_INTERRUPT(reboot_interrupt, REBOOT_VECTOR)
 #endif
 
 BUILD_INTERRUPT(x86_platform_ipi, X86_PLATFORM_IPI_VECTOR)
 
Quoted 15 lines

arch/x86/include/asm/entry_arch.h line 26

Quoted 11 lines
arch/x86/include/asm/entry_arch.h +3 -6
 #ifdef CONFIG_HAVE_KVM
-BUILD_INTERRUPT3(kvm_posted_intr_ipi, POSTED_INTR_VECTOR,
-		 smp_kvm_posted_intr_ipi)
-BUILD_INTERRUPT3(kvm_posted_intr_wakeup_ipi, POSTED_INTR_WAKEUP_VECTOR,
-		 smp_kvm_posted_intr_wakeup_ipi)
-BUILD_INTERRUPT3(kvm_posted_intr_nested_ipi, POSTED_INTR_NESTED_VECTOR,
-		 smp_kvm_posted_intr_nested_ipi)
+BUILD_INTERRUPT(kvm_posted_intr_ipi, POSTED_INTR_VECTOR)
+BUILD_INTERRUPT(kvm_posted_intr_wakeup_ipi, POSTED_INTR_WAKEUP_VECTOR)
+BUILD_INTERRUPT(kvm_posted_intr_nested_ipi, POSTED_INTR_NESTED_VECTOR)
 #endif

This looks like a different change than what is specified in the change log. It looks like a clean up. Shouldn't it be a separate patch?

Let me split it up.

#35 lore raw

Re: [patch 15/41] x86/idt: Remove tracing idt completely

Quoted 51 lines · Thomas Gleixner
Quoted 34 lines · Steven Rostedt
Quoted 31 lines · On Fri, 25 Aug 2017 12:31:18 +0200 Thomas Gleixner <[email protected]> wrote:

No more users of the tracing idt. All exception tracepoints have been moved into the regular handlers. Get rid of the mess which shouldn't have been created in the first place.

---
 arch/x86/entry/entry_32.S         |   12 -----
 arch/x86/entry/entry_64.S         |   13 ------
 arch/x86/include/asm/desc.h       |   82 ++------------------------------------
 arch/x86/include/asm/entry_arch.h |   14 ++----
 arch/x86/include/asm/hw_irq.h     |   20 ---------
 arch/x86/include/asm/segment.h    |    3 -
 arch/x86/kernel/tracepoint.c      |   43 -------------------
 arch/x86/kernel/traps.c           |    6 --
 8 files changed, 14 insertions(+), 179 deletions(-)
arch/x86/include/asm/entry_arch.h +2 -3
@@ -13,20 +13,16 @@
 BUILD_INTERRUPT(reschedule_interrupt,RESCHEDULE_VECTOR)
 BUILD_INTERRUPT(call_function_interrupt,CALL_FUNCTION_VECTOR)
 BUILD_INTERRUPT(call_function_single_interrupt,CALL_FUNCTION_SINGLE_VECTOR)
-BUILD_INTERRUPT3(irq_move_cleanup_interrupt, IRQ_MOVE_CLEANUP_VECTOR,
-		 smp_irq_move_cleanup_interrupt)
-BUILD_INTERRUPT3(reboot_interrupt, REBOOT_VECTOR, smp_reboot_interrupt)
+BUILD_INTERRUPT(irq_move_cleanup_interrupt, IRQ_MOVE_CLEANUP_VECTOR)
+BUILD_INTERRUPT(reboot_interrupt, REBOOT_VECTOR)
 #endif
 
 BUILD_INTERRUPT(x86_platform_ipi, X86_PLATFORM_IPI_VECTOR)
 
Quoted 15 lines

arch/x86/include/asm/entry_arch.h line 26

Quoted 11 lines
arch/x86/include/asm/entry_arch.h +3 -6
 #ifdef CONFIG_HAVE_KVM
-BUILD_INTERRUPT3(kvm_posted_intr_ipi, POSTED_INTR_VECTOR,
-		 smp_kvm_posted_intr_ipi)
-BUILD_INTERRUPT3(kvm_posted_intr_wakeup_ipi, POSTED_INTR_WAKEUP_VECTOR,
-		 smp_kvm_posted_intr_wakeup_ipi)
-BUILD_INTERRUPT3(kvm_posted_intr_nested_ipi, POSTED_INTR_NESTED_VECTOR,
-		 smp_kvm_posted_intr_nested_ipi)
+BUILD_INTERRUPT(kvm_posted_intr_ipi, POSTED_INTR_VECTOR)
+BUILD_INTERRUPT(kvm_posted_intr_wakeup_ipi, POSTED_INTR_WAKEUP_VECTOR)
+BUILD_INTERRUPT(kvm_posted_intr_nested_ipi, POSTED_INTR_NESTED_VECTOR)
 #endif

This looks like a different change than what is specified in the change log. It looks like a clean up. Shouldn't it be a separate patch?

Quoted 1 line

Let me split it up.

OTOH, the BUILD_INTERRUPT3 users were explicit to avoid the TRACE_BUILD_INTERRUPT() extra which was hidden behind BUILD_INTERRUPT. So yes, it's a cleanup, but related.

Thanks,

tglx

#36 lore raw

PATCH 16/41 x86/tracing: Disentangle pagefault and resched IPI tracing key

In reply to Thomas Gleixner

The pagefault and the resched IPI handler are the only ones where it is worth to optimize the code further in case tracepoints are disabled. But it makes no sense to have a single static key for both.

Seperate the static keys so the facilities are handled seperately.

---
 arch/x86/include/asm/trace/common.h      |   15 ++++++++-------
 arch/x86/include/asm/trace/exceptions.h  |    6 ++++--
 arch/x86/include/asm/trace/irq_vectors.h |   22 ++++++++++++++++------
 arch/x86/kernel/smp.c                    |    4 ++--
 arch/x86/kernel/tracepoint.c             |   27 ++++++++++++++++++++++-----
 arch/x86/mm/fault.c                      |    2 +-
 6 files changed, 53 insertions(+), 23 deletions(-)
arch/x86/include/asm/trace/common.h +8 -7
Index: b/arch/x86/include/asm/trace/common.h
===================================================================
@@ -1,15 +1,16 @@
 #ifndef _ASM_TRACE_COMMON_H
 #define _ASM_TRACE_COMMON_H
 
-extern int trace_irq_vector_regfunc(void);
-extern void trace_irq_vector_unregfunc(void);
-
 #ifdef CONFIG_TRACING
-DECLARE_STATIC_KEY_FALSE(trace_irqvectors_key);
-#define trace_irqvectors_enabled()			\
-	static_branch_unlikely(&trace_irqvectors_key)
+DECLARE_STATIC_KEY_FALSE(trace_pagefault_key);
+#define trace_pagefault_enabled()			\
+	static_branch_unlikely(&trace_pagefault_key)
+DECLARE_STATIC_KEY_FALSE(trace_resched_ipi_key);
+#define trace_resched_ipi_enabled()			\
+	static_branch_unlikely(&trace_resched_ipi_key)
 #else
-static inline bool trace_irqvectors_enabled(void) { return false; }
+static inline bool trace_pagefault_enabled(void) { return false; }
+static inline bool trace_resched_ipi_enabled(void) { return false; }
 #endif
 
 #endif
arch/x86/include/asm/trace/exceptions.h +4 -2
Index: b/arch/x86/include/asm/trace/exceptions.h
===================================================================
@@ -7,6 +7,9 @@
 #include <linux/tracepoint.h>
 #include <asm/trace/common.h>
 
+extern int trace_pagefault_reg(void);
+extern void trace_pagefault_unreg(void);
+
 DECLARE_EVENT_CLASS(x86_exceptions,
 
 	TP_PROTO(unsigned long address, struct pt_regs *regs,
@@ -35,8 +38,7 @@ DEFINE_EVENT_FN(x86_exceptions, name,
 	TP_PROTO(unsigned long address,	struct pt_regs *regs,	\
 		 unsigned long error_code),			\
 	TP_ARGS(address, regs, error_code),			\
-	trace_irq_vector_regfunc,				\
-	trace_irq_vector_unregfunc);
+	trace_pagefault_reg, trace_pagefault_unreg);
 
 DEFINE_PAGE_FAULT_EVENT(page_fault_user);
 DEFINE_PAGE_FAULT_EVENT(page_fault_kernel);
arch/x86/include/asm/trace/irq_vectors.h +16 -6
Index: b/arch/x86/include/asm/trace/irq_vectors.h
===================================================================
@@ -7,6 +7,9 @@
 #include <linux/tracepoint.h>
 #include <asm/trace/common.h>
 
+extern int trace_resched_ipi_reg(void);
+extern void trace_resched_ipi_unreg(void);
+
 DECLARE_EVENT_CLASS(x86_irq_vector,
 
 	TP_PROTO(int vector),
@@ -26,15 +29,22 @@ DECLARE_EVENT_CLASS(x86_irq_vector,
 #define DEFINE_IRQ_VECTOR_EVENT(name)		\
 DEFINE_EVENT_FN(x86_irq_vector, name##_entry,	\
 	TP_PROTO(int vector),			\
+	TP_ARGS(vector), NULL, NULL);		\
+DEFINE_EVENT_FN(x86_irq_vector, name##_exit,	\
+	TP_PROTO(int vector),			\
+	TP_ARGS(vector), NULL, NULL);
+
+#define DEFINE_RESCHED_IPI_EVENT(name)		\
+DEFINE_EVENT_FN(x86_irq_vector, name##_entry,	\
+	TP_PROTO(int vector),			\
 	TP_ARGS(vector),			\
-	trace_irq_vector_regfunc,		\
-	trace_irq_vector_unregfunc);		\
+	trace_resched_ipi_reg,			\
+	trace_resched_ipi_unreg);		\
 DEFINE_EVENT_FN(x86_irq_vector, name##_exit,	\
 	TP_PROTO(int vector),			\
 	TP_ARGS(vector),			\
-	trace_irq_vector_regfunc,		\
-	trace_irq_vector_unregfunc);
-
+	trace_resched_ipi_reg,			\
+	trace_resched_ipi_unreg);
 
 /*
  * local_timer - called when entering/exiting a local timer interrupt
@@ -45,7 +55,7 @@ DEFINE_IRQ_VECTOR_EVENT(local_timer);
 /*
  * reschedule - called when entering/exiting a reschedule vector handler
  */
-DEFINE_IRQ_VECTOR_EVENT(reschedule);
+DEFINE_RESCHED_IPI_EVENT(reschedule);
 
 /*
  * spurious_apic - called when entering/exiting a spurious apic vector handler
arch/x86/kernel/smp.c +2 -2
Index: b/arch/x86/kernel/smp.c
===================================================================
@@ -261,7 +261,7 @@ static void native_stop_other_cpus(int w
 {
 	ack_APIC_irq();
 
-	if (trace_irqvectors_enabled()) {
+	if (trace_resched_ipi_enabled()) {
 		/*
 		 * scheduler_ipi() might call irq_enter() as well, but
 		 * nested calls are fine.
@@ -273,7 +273,7 @@ static void native_stop_other_cpus(int w
 	inc_irq_stat(irq_resched_count);
 	scheduler_ipi();
 
-	if (trace_irqvectors_enabled()) {
+	if (trace_resched_ipi_enabled()) {
 		trace_reschedule_exit(RESCHEDULE_VECTOR);
 		irq_exit();
 	}
arch/x86/kernel/tracepoint.c +22 -5
Index: b/arch/x86/kernel/tracepoint.c
===================================================================
@@ -10,15 +10,32 @@
 #include <asm/hw_irq.h>
 #include <asm/desc.h>
 
-DEFINE_STATIC_KEY_FALSE(trace_irqvectors_key);
+DEFINE_STATIC_KEY_FALSE(trace_pagefault_key);
 
-int trace_irq_vector_regfunc(void)
+int trace_pagefault_reg(void)
 {
-	static_branch_inc(&trace_irqvectors_key);
+	static_branch_inc(&trace_pagefault_key);
 	return 0;
 }
 
-void trace_irq_vector_unregfunc(void)
+void trace_pagefault_unreg(void)
 {
-	static_branch_dec(&trace_irqvectors_key);
+	static_branch_dec(&trace_pagefault_key);
 }
+
+#ifdef CONFIG_SMP
+
+DEFINE_STATIC_KEY_FALSE(trace_resched_ipi_key);
+
+int trace_resched_ipi_reg(void)
+{
+	static_branch_inc(&trace_resched_ipi_key);
+	return 0;
+}
+
+void trace_resched_ipi_unreg(void)
+{
+	static_branch_dec(&trace_resched_ipi_key);
+}
+
+#endif
arch/x86/mm/fault.c +1 -1
Index: b/arch/x86/mm/fault.c
===================================================================
@@ -1509,7 +1509,7 @@ do_page_fault(struct pt_regs *regs, unsi
 	 *
 	 * exception_{enter,exit}() contain all sorts of tracepoints.
 	 */
-	if (trace_irqvectors_enabled())
+	if (trace_pagefault_enabled())
 		trace_page_fault_entries(address, regs, error_code);
 
 	prev_state = exception_enter();
#37 lore raw

PATCH 17/41 x86/idt: Unify gate_struct handling for 32/64bit

In reply to Thomas Gleixner

The first 32bits of gate struct are the same for 32 and 64 bit. The 32bit version uses desc_struct and no designated data structure, so we need different accessors for 32 and 64 bit. Aside of that the macros which are necessary to build the 32bit gate descriptor are horrible to read.

Unify the gate structs and switch all code fiddling with it over.

---
 arch/x86/boot/compressed/eboot.c |    8 ++---
 arch/x86/include/asm/desc.h      |   45 +++++++++++++-----------------
 arch/x86/include/asm/desc_defs.h |   57 +++++++++++++++++++++++++--------------
 arch/x86/kvm/vmx.c               |    2 -
 arch/x86/xen/enlighten_pv.c      |   12 ++++----
 5 files changed, 67 insertions(+), 57 deletions(-)
arch/x86/boot/compressed/eboot.c +4 -4
@@ -1058,7 +1058,7 @@ struct boot_params *efi_main(struct efi_
 		desc->s = DESC_TYPE_CODE_DATA;
 		desc->dpl = 0;
 		desc->p = 1;
-		desc->limit = 0xf;
+		desc->limit1 = 0xf;
 		desc->avl = 0;
 		desc->l = 0;
 		desc->d = SEG_OP_SIZE_32BIT;
@@ -1078,7 +1078,7 @@ struct boot_params *efi_main(struct efi_
 	desc->s = DESC_TYPE_CODE_DATA;
 	desc->dpl = 0;
 	desc->p = 1;
-	desc->limit = 0xf;
+	desc->limit1 = 0xf;
 	desc->avl = 0;
 	if (IS_ENABLED(CONFIG_X86_64)) {
 		desc->l = 1;
@@ -1099,7 +1099,7 @@ struct boot_params *efi_main(struct efi_
 	desc->s = DESC_TYPE_CODE_DATA;
 	desc->dpl = 0;
 	desc->p = 1;
-	desc->limit = 0xf;
+	desc->limit1 = 0xf;
 	desc->avl = 0;
 	desc->l = 0;
 	desc->d = SEG_OP_SIZE_32BIT;
@@ -1116,7 +1116,7 @@ struct boot_params *efi_main(struct efi_
 		desc->s = 0;
 		desc->dpl = 0;
 		desc->p = 1;
-		desc->limit = 0x0;
+		desc->limit1 = 0x0;
 		desc->avl = 0;
 		desc->l = 0;
 		desc->d = 0;
arch/x86/include/asm/desc.h +19 -26

72 lines not on this page · raw message

arch/x86/include/asm/desc_defs.h +37 -20

77 lines not on this page · raw message

arch/x86/kvm/vmx.c +1 -1
@@ -8786,7 +8786,7 @@ static void vmx_handle_external_intr(str
 
 		vector =  exit_intr_info & INTR_INFO_VECTOR_MASK;
 		desc = (gate_desc *)vmx->host_idt_base + vector;
-		entry = gate_offset(*desc);
+		entry = gate_offset(desc);
 		asm volatile(
 #ifdef CONFIG_X86_64
 			"mov %%" _ASM_SP ", %[sp]\n\t"
arch/x86/xen/enlighten_pv.c +6 -6
@@ -584,12 +584,12 @@ static int cvt_gate_to_trap(int vector,
 {
 	unsigned long addr;
 
-	if (val->type != GATE_TRAP && val->type != GATE_INTERRUPT)
+	if (val->bits.type != GATE_TRAP && val->bits.type != GATE_INTERRUPT)
 		return 0;
 
 	info->vector = vector;
 
-	addr = gate_offset(*val);
+	addr = gate_offset(val);
 #ifdef CONFIG_X86_64
 	/*
 	 * Look for known traps using IST, and substitute them
@@ -622,16 +622,16 @@ static int cvt_gate_to_trap(int vector,
 		;
 	else {
 		/* Some other trap using IST? */
-		if (WARN_ON(val->ist != 0))
+		if (WARN_ON(val->bits.ist != 0))
 			return 0;
 	}
 #endif	/* CONFIG_X86_64 */
 	info->address = addr;
 
-	info->cs = gate_segment(*val);
-	info->flags = val->dpl;
+	info->cs = gate_segment(val);
+	info->flags = val->bits.dpl;
 	/* interrupt gates clear IF */
-	if (val->type == GATE_INTERRUPT)
+	if (val->bits.type == GATE_INTERRUPT)
 		info->flags |= 1 << 2;
 
 	return 1;
#38 lore raw

PATCH 18/41 x86/percpu: Use static initializer for GDT entry

In reply to Thomas Gleixner

The IDT cleanup is about to remove pack_descriptor(). The GDT setup for the percpu storage can be achieved with the static initializer as well. Replace it.

---
 arch/x86/kernel/setup_percpu.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
arch/x86/kernel/setup_percpu.c +3 -6
@@ -155,13 +155,10 @@ static void __init pcpup_populate_pte(un
 static inline void setup_percpu_segment(int cpu)
 {
 #ifdef CONFIG_X86_32
-	struct desc_struct gdt;
+	struct desc_struct d = GDT_ENTRY_INIT(0x8092, per_cpu_offset(cpu),
+					      0xFFFFF);
 
-	pack_descriptor(&gdt, per_cpu_offset(cpu), 0xFFFFF,
-			0x2 | DESCTYPE_S, 0x8);
-	gdt.s = 1;
-	write_gdt_entry(get_cpu_gdt_rw(cpu),
-			GDT_ENTRY_PERCPU, &gdt, DESCTYPE_S);
+	write_gdt_entry(get_cpu_gdt_rw(cpu), GDT_ENTRY_PERCPU, &d, DESCTYPE_S);
 #endif
 }
 
#39 lore raw

PATCH 19/41 x86/fpu: Use bitfield accessors for desc_struct

In reply to Thomas Gleixner

desc_struct is a union of u32 fields and bitfields. The access to the u32 fields is done with magic macros.

Convert it to use the bitfields and replace the macro magic with parseable inline functions.

---
 arch/x86/math-emu/fpu_entry.c   |   11 ++++-----
 arch/x86/math-emu/fpu_system.h  |   48 ++++++++++++++++++++++++++++++----------
 arch/x86/math-emu/get_address.c |   17 +++++++-------
 3 files changed, 51 insertions(+), 25 deletions(-)
arch/x86/math-emu/fpu_entry.c +5 -6
Index: b/arch/x86/math-emu/fpu_entry.c
===================================================================
@@ -147,7 +147,7 @@ void math_emulate(struct math_emu_info *
 		}
 
 		code_descriptor = FPU_get_ldt_descriptor(FPU_CS);
-		if (SEG_D_SIZE(code_descriptor)) {
+		if (code_descriptor.d) {
 			/* The above test may be wrong, the book is not clear */
 			/* Segmented 32 bit protected mode */
 			addr_modes.default_mode = SEG32;
@@ -155,11 +155,10 @@ void math_emulate(struct math_emu_info *
 			/* 16 bit protected mode */
 			addr_modes.default_mode = PM16;
 		}
-		FPU_EIP += code_base = SEG_BASE_ADDR(code_descriptor);
-		code_limit = code_base
-		    + (SEG_LIMIT(code_descriptor) +
-		       1) * SEG_GRANULARITY(code_descriptor)
-		    - 1;
+		FPU_EIP += code_base = seg_get_base(&code_descriptor);
+		code_limit = seg_get_limit(&code_descriptor) + 1;
+		code_limit *= seg_get_granularity(&code_descriptor);
+		code_limit += code_base - 1;
 		if (code_limit < code_base)
 			code_limit = 0xffffffff;
 	}
arch/x86/math-emu/fpu_system.h +37 -11

54 lines not on this page · raw message

arch/x86/math-emu/get_address.c +9 -8
Index: b/arch/x86/math-emu/get_address.c
===================================================================
@@ -159,17 +159,18 @@ static long pm_address(u_char FPU_modrm,
 	}
 
 	descriptor = FPU_get_ldt_descriptor(addr->selector);
-	base_address = SEG_BASE_ADDR(descriptor);
+	base_address = seg_get_base(&descriptor);
 	address = base_address + offset;
-	limit = base_address
-	    + (SEG_LIMIT(descriptor) + 1) * SEG_GRANULARITY(descriptor) - 1;
+	limit = seg_get_limit(&descriptor) + 1;
+	limit *= seg_get_granularity(&descriptor);
+	limit += base_address - 1;
 	if (limit < base_address)
 		limit = 0xffffffff;
 
-	if (SEG_EXPAND_DOWN(descriptor)) {
-		if (SEG_G_BIT(descriptor))
+	if (seg_expands_down(&descriptor)) {
+		if (descriptor.g) {
 			seg_top = 0xffffffff;
-		else {
+		} else {
 			seg_top = base_address + (1 << 20);
 			if (seg_top < base_address)
 				seg_top = 0xffffffff;
@@ -182,8 +183,8 @@ static long pm_address(u_char FPU_modrm,
 		    (address > limit) || (address < base_address) ? 0 :
 		    ((limit - address) >= 254 ? 255 : limit - address + 1);
 	}
-	if (SEG_EXECUTE_ONLY(descriptor) ||
-	    (!SEG_WRITE_PERM(descriptor) && (FPU_modrm & FPU_WRITE_BIT))) {
+	if (seg_execute_only(&descriptor) ||
+	    (!seg_writable(&descriptor) && (FPU_modrm & FPU_WRITE_BIT))) {
 		access_limit = 0;
 	}
 	return address;
#40 lore raw

PATCH 20/41 x86: Replace access to desc_struct:a/b fields

In reply to Thomas Gleixner

The union inside of desc_struct which allows access to the raw u32 parts of the descriptors. This raw access part is about to go away.

Replace the few code parts which access those fields.

Cc Boris Ostrovsky <[email protected]>
Cc Juergen Gross <[email protected]>
---
 arch/x86/include/asm/xen/hypercall.h |    6 ++++--
 arch/x86/kernel/tls.c                |    2 +-
 arch/x86/xen/enlighten_pv.c          |    2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)
arch/x86/include/asm/xen/hypercall.h +4 -2
Index: b/arch/x86/include/asm/xen/hypercall.h
===================================================================
@@ -552,6 +552,8 @@ static inline void
 MULTI_update_descriptor(struct multicall_entry *mcl, u64 maddr,
 			struct desc_struct desc)
 {
+	u32 *p = (u32 *) &desc;
+
 	mcl->op = __HYPERVISOR_update_descriptor;
 	if (sizeof(maddr) == sizeof(long)) {
 		mcl->args[0] = maddr;
@@ -559,8 +561,8 @@ MULTI_update_descriptor(struct multicall
 	} else {
 		mcl->args[0] = maddr;
 		mcl->args[1] = maddr >> 32;
-		mcl->args[2] = desc.a;
-		mcl->args[3] = desc.b;
+		mcl->args[2] = *p++;
+		mcl->args[3] = *p;
 	}
 
 	trace_xen_mc_entry(mcl, sizeof(maddr) == sizeof(long) ? 2 : 4);
arch/x86/kernel/tls.c +1 -1
Index: b/arch/x86/kernel/tls.c
===================================================================
@@ -93,7 +93,7 @@ static void set_tls_desc(struct task_str
 
 	while (n-- > 0) {
 		if (LDT_empty(info) || LDT_zero(info)) {
-			desc->a = desc->b = 0;
+			memset(desc, 0, sizeof(*desc));
 		} else {
 			fill_ldt(desc, info);
 
arch/x86/xen/enlighten_pv.c +1 -1
Index: b/arch/x86/xen/enlighten_pv.c
===================================================================
@@ -494,7 +494,7 @@ static void __init xen_load_gdt_boot(con
 static inline bool desc_equal(const struct desc_struct *d1,
 			      const struct desc_struct *d2)
 {
-	return d1->a == d2->a && d1->b == d2->b;
+	return memcmp(d1, d2, sizeof(*d1));
 }
 
 static void load_TLS_descriptor(struct thread_struct *t,
maintainer Reviewed-by on v1 20/41
#41 lore raw

Re: [patch 20/41] x86: Replace access to desc_struct:a/b fields

Quoted 8 lines · Thomas Gleixner

The union inside of desc_struct which allows access to the raw u32 parts of the descriptors. This raw access part is about to go away.

Replace the few code parts which access those fields.

Cc Boris Ostrovsky <[email protected]>
Cc Juergen Gross <[email protected]>

Juergen

#42 lore raw

Re: [patch 20/41] x86: Replace access to desc_struct:a/b fields

In reply to Thomas Gleixner

arch/x86/xen/enlighten_pv.c static void __init xen_load_gdt_boot(con · line 498

Quoted 13 lines

Index: b/arch/x86/xen/enlighten_pv.c
===================================================================

arch/x86/xen/enlighten_pv.c +1 -1
@@ -494,7 +494,7 @@ static void __init xen_load_gdt_boot(con
 static inline bool desc_equal(const struct desc_struct *d1,
 			      const struct desc_struct *d2)
 {
-	return d1->a == d2->a && d1->b == d2->b;
+	return memcmp(d1, d2, sizeof(*d1));
 }

Shouldn't this be !memcmp() ?

-boris

#43 lore raw

Re: [patch 20/41] x86: Replace access to desc_struct:a/b fields

Quoted 15 lines · Boris Ostrovsky

arch/x86/xen/enlighten_pv.c static void __init xen_load_gdt_boot(con · line 498

Quoted 13 lines

Index: b/arch/x86/xen/enlighten_pv.c
===================================================================

arch/x86/xen/enlighten_pv.c +1 -1
@@ -494,7 +494,7 @@ static void __init xen_load_gdt_boot(con
 static inline bool desc_equal(const struct desc_struct *d1,
 			      const struct desc_struct *d2)
 {
-	return d1->a == d2->a && d1->b == d2->b;
+	return memcmp(d1, d2, sizeof(*d1));
 }

Shouldn't this be !memcmp() ?

Bah yes.

#44 lore raw

Re: [patch 20/41] x86: Replace access to desc_struct:a/b fields

Quoted 18 lines · On Fri, 25 Aug 2017 16:02:02 +0200 (CEST) Thomas Gleixner <[email protected]>…
Quoted 15 lines · Boris Ostrovsky

arch/x86/xen/enlighten_pv.c static void __init xen_load_gdt_boot(con · line 498

Quoted 13 lines

Index: b/arch/x86/xen/enlighten_pv.c
===================================================================

arch/x86/xen/enlighten_pv.c +1 -1
@@ -494,7 +494,7 @@ static void __init xen_load_gdt_boot(con
 static inline bool desc_equal(const struct desc_struct *d1,
 			      const struct desc_struct *d2)
 {
-	return d1->a == d2->a && d1->b == d2->b;
+	return memcmp(d1, d2, sizeof(*d1));
 }  

Shouldn't this be !memcmp() ?

Quoted 1 line

Bah yes.

This is why I'm one of those that like to add the "== 0" to it. Because I always get this wrong :-p

-- Steve

#45 lore raw

PATCH 21/41 x86/gdt: Use bitfields for initialization

In reply to Thomas Gleixner

The GDT entry related code uses partially bitfields and macros which initialize the two 16 bit parts of the entry by magic shift and mask operations.

Clean it up and use the bitfields to initialize and access entries.

---
 arch/x86/entry/vdso/vma.c        |    2 -
 arch/x86/include/asm/desc.h      |   26 ++++++++++++++---------
 arch/x86/include/asm/desc_defs.h |   44 +++++++++++++++++----------------------
 arch/x86/math-emu/fpu_system.h   |    2 -
 4 files changed, 38 insertions(+), 36 deletions(-)
arch/x86/entry/vdso/vma.c +1 -1
Index: b/arch/x86/entry/vdso/vma.c
===================================================================
@@ -351,7 +351,7 @@ static void vgetcpu_cpu_init(void *arg)
 	 * and 8 bits for the node)
 	 */
 	d.limit0 = cpu | ((node & 0xf) << 12);
-	d.limit = node >> 4;
+	d.limit1 = node >> 4;
 	d.type = 5;		/* RO data, expand down, accessed */
 	d.dpl = 3;		/* Visible to user code */
 	d.s = 1;		/* Not a system segment */
arch/x86/include/asm/desc.h +16 -10
Index: b/arch/x86/include/asm/desc.h
===================================================================
@@ -22,7 +22,7 @@ static inline void fill_ldt(struct desc_
 	desc->s			= 1;
 	desc->dpl		= 0x3;
 	desc->p			= info->seg_not_present ^ 1;
-	desc->limit		= (info->limit & 0xf0000) >> 16;
+	desc->limit1		= (info->limit & 0xf0000) >> 16;
 	desc->avl		= info->useable;
 	desc->d			= info->seg_32bit;
 	desc->g			= info->limit_in_pages;
@@ -169,14 +169,20 @@ static inline void pack_descriptor(struc
 				   unsigned long limit, unsigned char type,
 				   unsigned char flags)
 {
-	desc->a = ((base & 0xffff) << 16) | (limit & 0xffff);
-	desc->b = (base & 0xff000000) | ((base & 0xff0000) >> 16) |
-		(limit & 0x000f0000) | ((type & 0xff) << 8) |
-		((flags & 0xf) << 20);
-	desc->p = 1;
+	desc->limit0		= (u16) limit;
+	desc->base0		= (u16) base;
+	desc->base1		= (base >> 16) & 0xFF;
+	desc->type		= type & 0x0F;
+	desc->s			= 0;
+	desc->dpl		= 0;
+	desc->p			= 1;
+	desc->limit1		= (limit >> 16) & 0xF;
+	desc->avl		= (flags >> 0) & 0x01;
+	desc->l			= (flags >> 1) & 0x01;
+	desc->d			= (flags >> 2) & 0x01;
+	desc->g			= (flags >> 3) & 0x01;
 }
 
-
 static inline void set_tssldt_descriptor(void *d, unsigned long addr,
 					 unsigned type, unsigned size)
 {
@@ -194,7 +200,7 @@ static inline void set_tssldt_descriptor
 	desc->base2		= (addr >> 24) & 0xFF;
 	desc->base3		= (u32) (addr >> 32);
 #else
-	pack_descriptor((struct desc_struct *)d, addr, size, 0x80 | type, 0);
+	pack_descriptor((struct desc_struct *)d, addr, size, type, 0);
 #endif
 }
 
@@ -394,13 +400,13 @@ static inline void set_desc_base(struct
 
 static inline unsigned long get_desc_limit(const struct desc_struct *desc)
 {
-	return desc->limit0 | (desc->limit << 16);
+	return desc->limit0 | (desc->limit1 << 16);
 }
 
 static inline void set_desc_limit(struct desc_struct *desc, unsigned long limit)
 {
 	desc->limit0 = limit & 0xffff;
-	desc->limit = (limit >> 16) & 0xf;
+	desc->limit1 = (limit >> 16) & 0xf;
 }
 
 #ifdef CONFIG_X86_64
arch/x86/include/asm/desc_defs.h +20 -24

54 lines not on this page · raw message

arch/x86/math-emu/fpu_system.h +1 -1
Index: b/arch/x86/math-emu/fpu_system.h
===================================================================
@@ -49,7 +49,7 @@ static inline unsigned long seg_get_base
 
 static inline unsigned long seg_get_limit(struct desc_struct *d)
 {
-	return ((unsigned long)d->limit << 16) | d->limit0;
+	return ((unsigned long)d->limit1 << 16) | d->limit0;
 }
 
 static inline unsigned long seg_get_granularity(struct desc_struct *d)
#46 lore raw

PATCH 22/41 x86/ldttss: Cleanup 32bit descriptors

In reply to Thomas Gleixner

Like the IDT descriptors the LDT/TSS descriptors are pointlessly different on 32 and 64 bit.

Unify them and get rid of the duplicated code.

---
 arch/x86/include/asm/desc.h      |   26 +++-----------------------
 arch/x86/include/asm/desc_defs.h |   27 ++++++++++++---------------
 2 files changed, 15 insertions(+), 38 deletions(-)
arch/x86/include/asm/desc.h +3 -23
@@ -165,42 +165,22 @@ native_write_gdt_entry(struct desc_struc
 	memcpy(&gdt[entry], desc, size);
 }
 
-static inline void pack_descriptor(struct desc_struct *desc, unsigned long base,
-				   unsigned long limit, unsigned char type,
-				   unsigned char flags)
-{
-	desc->limit0		= (u16) limit;
-	desc->base0		= (u16) base;
-	desc->base1		= (base >> 16) & 0xFF;
-	desc->type		= type & 0x0F;
-	desc->s			= 0;
-	desc->dpl		= 0;
-	desc->p			= 1;
-	desc->limit1		= (limit >> 16) & 0xF;
-	desc->avl		= (flags >> 0) & 0x01;
-	desc->l			= (flags >> 1) & 0x01;
-	desc->d			= (flags >> 2) & 0x01;
-	desc->g			= (flags >> 3) & 0x01;
-}
-
 static inline void set_tssldt_descriptor(void *d, unsigned long addr,
 					 unsigned type, unsigned size)
 {
-#ifdef CONFIG_X86_64
-	struct ldttss_desc64 *desc = d;
+	struct ldttss_desc *desc = d;
 
 	memset(desc, 0, sizeof(*desc));
 
-	desc->limit0		= size & 0xFFFF;
+	desc->limit0		= (u16) size;
 	desc->base0		= (u16) addr;
 	desc->base1		= (addr >> 16) & 0xFF;
 	desc->type		= type;
 	desc->p			= 1;
 	desc->limit1		= (size >> 16) & 0xF;
 	desc->base2		= (addr >> 24) & 0xFF;
+#ifdef CONFIG_X86_64
 	desc->base3		= (u32) (addr >> 32);
-#else
-	pack_descriptor((struct desc_struct *)d, addr, size, type, 0);
 #endif
 }
 
arch/x86/include/asm/desc_defs.h +12 -15
@@ -49,24 +49,21 @@ enum {
 	DESCTYPE_S = 0x10,	/* !system */
 };
 
-/* LDT or TSS descriptor in the GDT. 16 bytes. */
-struct ldttss_desc64 {
-	u16 limit0;
-	u16 base0;
-	unsigned base1 : 8, type : 5, dpl : 2, p : 1;
-	unsigned limit1 : 4, zero0 : 3, g : 1, base2 : 8;
-	u32 base3;
-	u32 zero1;
-} __attribute__((packed));
-
+/* LDT or TSS descriptor in the GDT. */
+struct ldttss_desc {
+	u16	limit0;
+	u16	base0;
 
+	u16	base1 : 8, type : 5, dpl : 2, p : 1;
+	u16	limit1 : 4, zero0 : 3, g : 1, base2 : 8;
 #ifdef CONFIG_X86_64
-typedef struct ldttss_desc64 ldt_desc;
-typedef struct ldttss_desc64 tss_desc;
-#else
-typedef struct desc_struct ldt_desc;
-typedef struct desc_struct tss_desc;
+	u32	base3;
+	u32	zero1;
 #endif
+} __attribute__((packed));
+
+typedef struct ldttss_desc ldt_desc;
+typedef struct ldttss_desc tss_desc;
 
 struct idt_bits {
 	u16		ist	: 3,
#47 lore raw

PATCH 23/41 x86/idt: Create file for IDT related code

In reply to Thomas Gleixner

IDT related code lives in different places. Create a new source file to hold it.

Move the idt_tables and descriptors to it for a start. Follow up patches will gradually move more code over.

---
 arch/x86/kernel/Makefile     |    2 +-
 arch/x86/kernel/cpu/common.c |    9 ---------
 arch/x86/kernel/idt.c        |   26 ++++++++++++++++++++++++++
 arch/x86/kernel/traps.c      |    6 ------
 4 files changed, 27 insertions(+), 16 deletions(-)
arch/x86/kernel/Makefile +1 -1
@@ -42,7 +42,7 @@ CFLAGS_irq.o := -I$(src)/../include/asm/
 
 obj-y			:= process_$(BITS).o signal.o
 obj-$(CONFIG_COMPAT)	+= signal_compat.o
-obj-y			+= traps.o irq.o irq_$(BITS).o dumpstack_$(BITS).o
+obj-y			+= traps.o idt.o irq.o irq_$(BITS).o dumpstack_$(BITS).o
 obj-y			+= time.o ioport.o dumpstack.o nmi.o
 obj-$(CONFIG_MODIFY_LDT_SYSCALL)	+= ldt.o
 obj-y			+= setup.o x86_init.o i8259.o irqinit.o jump_label.o
arch/x86/kernel/cpu/common.c +0 -9
@@ -1289,15 +1289,6 @@ static __init int setup_disablecpuid(cha
 __setup("clearcpuid=", setup_disablecpuid);
 
 #ifdef CONFIG_X86_64
-struct desc_ptr idt_descr __ro_after_init = {
-	.size = NR_VECTORS * 16 - 1,
-	.address = (unsigned long) idt_table,
-};
-const struct desc_ptr debug_idt_descr = {
-	.size = NR_VECTORS * 16 - 1,
-	.address = (unsigned long) debug_idt_table,
-};
-
 DEFINE_PER_CPU_FIRST(union irq_stack_union,
 		     irq_stack_union) __aligned(PAGE_SIZE) __visible;
 
arch/x86/kernel/idt.c new file +26 -0
@@ -0,0 +1,26 @@
+/*
+ * Interrupt descriptor table related code
+ *
+ * This file is licensed under the GPL V2
+ */
+#include <linux/interrupt.h>
+
+#include <asm/desc.h>
+
+/* Must be page-aligned because the real IDT is used in a fixmap. */
+gate_desc idt_table[IDT_ENTRIES] __page_aligned_bss;
+
+#ifdef CONFIG_X86_64
+/* No need to be aligned, but done to keep all IDTs defined the same way. */
+gate_desc debug_idt_table[IDT_ENTRIES] __page_aligned_bss;
+
+struct desc_ptr idt_descr __ro_after_init = {
+	.size		= IDT_ENTRIES * 16 - 1,
+	.address	= (unsigned long) idt_table,
+};
+
+const struct desc_ptr debug_idt_descr = {
+	.size		= IDT_ENTRIES * 16 - 1,
+	.address	= (unsigned long) debug_idt_table,
+};
+#endif
arch/x86/kernel/traps.c +0 -6
@@ -65,18 +65,12 @@
 #include <asm/x86_init.h>
 #include <asm/pgalloc.h>
 #include <asm/proto.h>
-
-/* No need to be aligned, but done to keep all IDTs defined the same way. */
-gate_desc debug_idt_table[NR_VECTORS] __page_aligned_bss;
 #else
 #include <asm/processor-flags.h>
 #include <asm/setup.h>
 #include <asm/proto.h>
 #endif
 
-/* Must be page-aligned because the real IDT is used in a fixmap. */
-gate_desc idt_table[NR_VECTORS] __page_aligned_bss;
-
 DECLARE_BITMAP(used_vectors, NR_VECTORS);
 
 static inline void cond_local_irq_enable(struct pt_regs *regs)
#48 lore raw

PATCH 24/41 x86/idt: Move 32bit idt_descr to C code

In reply to Thomas Gleixner

32bit has the idt_descr sitting in the low level assembly entry code. There is no reason for that. Move it into the C file and use the 64bit version of it.

---
 arch/x86/kernel/head_32.S |    6 ------
 arch/x86/kernel/idt.c     |   10 +++++-----
 2 files changed, 5 insertions(+), 11 deletions(-)
arch/x86/kernel/head_32.S +0 -6
@@ -626,7 +626,6 @@ ENTRY(initial_stack)
 
 	.data
 .globl boot_gdt_descr
-.globl idt_descr
 
 	ALIGN
 # early boot GDT descriptor (must use 1:1 address mapping)
@@ -635,11 +634,6 @@ ENTRY(initial_stack)
 	.word __BOOT_DS+7
 	.long boot_gdt - __PAGE_OFFSET
 
-	.word 0				# 32-bit align idt_desc.address
-idt_descr:
-	.word IDT_ENTRIES*8-1		# idt contains 256 entries
-	.long idt_table
-
 # boot GDT descriptor (later on used by CPU#0):
 	.word 0				# 32 bit align gdt_desc.address
 ENTRY(early_gdt_descr)
arch/x86/kernel/idt.c +5 -5
@@ -10,15 +10,15 @@
 /* Must be page-aligned because the real IDT is used in a fixmap. */
 gate_desc idt_table[IDT_ENTRIES] __page_aligned_bss;
 
-#ifdef CONFIG_X86_64
-/* No need to be aligned, but done to keep all IDTs defined the same way. */
-gate_desc debug_idt_table[IDT_ENTRIES] __page_aligned_bss;
-
 struct desc_ptr idt_descr __ro_after_init = {
-	.size		= IDT_ENTRIES * 16 - 1,
+	.size		= (IDT_ENTRIES * 2 * sizeof(unsigned long)) - 1,
 	.address	= (unsigned long) idt_table,
 };
 
+#ifdef CONFIG_X86_64
+/* No need to be aligned, but done to keep all IDTs defined the same way. */
+gate_desc debug_idt_table[IDT_ENTRIES] __page_aligned_bss;
+
 const struct desc_ptr debug_idt_descr = {
 	.size		= IDT_ENTRIES * 16 - 1,
 	.address	= (unsigned long) debug_idt_table,
#49 lore raw

PATCH 25/41 x86/idt: Remove unused set_trap_gate()

In reply to Thomas Gleixner

This inline is not used at all.

---
 arch/x86/include/asm/desc.h |   12 ------------
 1 file changed, 12 deletions(-)
arch/x86/include/asm/desc.h +0 -12
@@ -455,18 +455,6 @@ static inline void set_system_intr_gate(
 	_set_gate(n, GATE_INTERRUPT, addr, 0x3, 0, __KERNEL_CS);
 }
 
-static inline void set_system_trap_gate(unsigned int n, void *addr)
-{
-	BUG_ON((unsigned)n > 0xFF);
-	_set_gate(n, GATE_TRAP, addr, 0x3, 0, __KERNEL_CS);
-}
-
-static inline void set_trap_gate(unsigned int n, void *addr)
-{
-	BUG_ON((unsigned)n > 0xFF);
-	_set_gate(n, GATE_TRAP, addr, 0, 0, __KERNEL_CS);
-}
-
 static inline void set_task_gate(unsigned int n, unsigned int gdt_entry)
 {
 	BUG_ON((unsigned)n > 0xFF);
#50 lore raw

PATCH 26/41 x86/idt: Consolidate IDT invalidation

In reply to Thomas Gleixner

kexec and reboot have both code to invalidate IDT. Create a common function and use it.

---
 arch/x86/include/asm/desc.h        |    3 +++
 arch/x86/kernel/idt.c              |   11 +++++++++++
 arch/x86/kernel/machine_kexec_32.c |   14 +-------------
 arch/x86/kernel/reboot.c           |    4 +---
 4 files changed, 16 insertions(+), 16 deletions(-)
arch/x86/include/asm/desc.h +3 -0
@@ -512,4 +512,7 @@ static inline void load_current_idt(void
 	else
 		load_idt((const struct desc_ptr *)&idt_descr);
 }
+
+extern void idt_invalidate(void *addr);
+
 #endif /* _ASM_X86_DESC_H */
arch/x86/kernel/idt.c +11 -0
@@ -24,3 +24,14 @@ const struct desc_ptr debug_idt_descr =
 	.address	= (unsigned long) debug_idt_table,
 };
 #endif
+
+/**
+ * idt_invalidate - Invalidate interrupt descriptor table
+ * @addr:	The virtual address of the 'invalid' IDT
+ */
+void idt_invalidate(void *addr)
+{
+	struct desc_ptr idt = { .address = (unsigned long) addr, .size = 0 };
+
+	load_idt(&idt);
+}
arch/x86/kernel/machine_kexec_32.c +1 -13
@@ -26,18 +26,6 @@
 #include <asm/set_memory.h>
 #include <asm/debugreg.h>
 
-static void set_idt(void *newidt, __u16 limit)
-{
-	struct desc_ptr curidt;
-
-	/* ia32 supports unaliged loads & stores */
-	curidt.size    = limit;
-	curidt.address = (unsigned long)newidt;
-
-	load_idt(&curidt);
-}
-
-
 static void set_gdt(void *newgdt, __u16 limit)
 {
 	struct desc_ptr curgdt;
@@ -245,7 +233,7 @@ void machine_kexec(struct kimage *image)
 	 * If you want to load them you must set up your own idt & gdt.
 	 */
 	set_gdt(phys_to_virt(0), 0);
-	set_idt(phys_to_virt(0), 0);
+	idt_invalidate(phys_to_virt(0));
 
 	/* now call it */
 	image->start = relocate_kernel_ptr((unsigned long)image->head,
arch/x86/kernel/reboot.c +1 -3
@@ -38,8 +38,6 @@
 void (*pm_power_off)(void);
 EXPORT_SYMBOL(pm_power_off);
 
-static const struct desc_ptr no_idt = {};
-
 /*
  * This is set if we need to go through the 'emergency' path.
  * When machine_emergency_restart() is called, we may be on
@@ -638,7 +636,7 @@ static void native_machine_emergency_res
 			break;
 
 		case BOOT_TRIPLE:
-			load_idt(&no_idt);
+			idt_invalidate(NULL);
 			__asm__ __volatile__("int3");
 
 			/* We're probably dead after this, but... */
#51 lore raw

PATCH 27/41 x86/idt: Move early IDT handler setup to IDT code

In reply to Thomas Gleixner

The early IDT handler setup is done in C entry code for 64 bit and in ASM entry code for 32 bit. Move the 64bit variant to the IDT code so it can be shared with 32bit in the next step.

---
 arch/x86/include/asm/desc.h |    9 +++++++++
 arch/x86/kernel/head64.c    |    6 +-----
 arch/x86/kernel/idt.c       |   12 ++++++++++++
 3 files changed, 22 insertions(+), 5 deletions(-)
arch/x86/include/asm/desc.h +9 -0
@@ -511,6 +511,15 @@ static inline void load_current_idt(void
 		load_idt((const struct desc_ptr *)&idt_descr);
 }
 
+extern void idt_setup_early_handler(void);
+extern void idt_setup_early_traps(void);
+
+#ifdef CONFIG_X86_64
+extern void idt_setup_early_pf(void);
+#else
+static inline void idt_setup_early_pf(void) { }
+#endif
+
 extern void idt_invalidate(void *addr);
 
 #endif /* _ASM_X86_DESC_H */
arch/x86/kernel/head64.c +1 -5
@@ -311,8 +311,6 @@ static void __init copy_bootdata(char *r
 
 asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
 {
-	int i;
-
 	/*
 	 * Build-time sanity checks on the kernel image and module
 	 * area mappings. (these are purely build-time and produce no code)
@@ -345,9 +343,7 @@ asmlinkage __visible void __init x86_64_
 
 	kasan_early_init();
 
-	for (i = 0; i < NUM_EXCEPTION_VECTORS; i++)
-		set_intr_gate(i, early_idt_handler_array[i]);
-	load_idt((const struct desc_ptr *)&idt_descr);
+	idt_setup_early_handler();
 
 	copy_bootdata(__va(real_mode_data));
 
arch/x86/kernel/idt.c +12 -0
@@ -26,6 +26,18 @@ const struct desc_ptr debug_idt_descr =
 #endif
 
 /**
+ * idt_setup_early_handler - Initializes the idt table with early handlers
+ */
+void __init idt_setup_early_handler(void)
+{
+	int i;
+
+	for (i = 0; i < NUM_EXCEPTION_VECTORS; i++)
+		set_intr_gate(i, early_idt_handler_array[i]);
+	load_idt((const struct desc_ptr *)&idt_descr);
+}
+
+/**
  * idt_invalidate - Invalidate interrupt descriptor table
  * @addr:	The virtual address of the 'invalid' IDT
  */
#52 lore raw

PATCH 28/41 x86/idt: Move early IDT setup out of 32bit asm

In reply to Thomas Gleixner

The early IDT setup can be done in C code like it's done on 64 bit. Reuse the 64 bit version.

---
 arch/x86/include/asm/segment.h |    1 +
 arch/x86/kernel/head32.c       |    4 ++++
 arch/x86/kernel/head_32.S      |   36 ++----------------------------------
 arch/x86/kernel/idt.c          |    4 ++++
 4 files changed, 11 insertions(+), 34 deletions(-)
arch/x86/include/asm/segment.h +1 -0
Index: b/arch/x86/include/asm/segment.h
===================================================================
@@ -238,6 +238,7 @@
 #ifndef __ASSEMBLY__
 
 extern const char early_idt_handler_array[NUM_EXCEPTION_VECTORS][EARLY_IDT_HANDLER_SIZE];
+extern void early_ignore_irq(void);
 
 /*
  * Load a segment. Fall back on loading the zero segment if something goes
arch/x86/kernel/head32.c +4 -0
Index: b/arch/x86/kernel/head32.c
===================================================================
@@ -10,6 +10,7 @@
 #include <linux/mm.h>
 #include <linux/memblock.h>
 
+#include <asm/desc.h>
 #include <asm/setup.h>
 #include <asm/sections.h>
 #include <asm/e820/api.h>
@@ -30,6 +31,9 @@ static void __init i386_default_early_se
 asmlinkage __visible void __init i386_start_kernel(void)
 {
 	cr4_init_shadow();
+
+	idt_setup_early_handler();
+
 	sanitize_boot_params(&boot_params);
 
 	x86_early_init_platform_quirks();
arch/x86/kernel/head_32.S +2 -34
Index: b/arch/x86/kernel/head_32.S
===================================================================
@@ -345,7 +345,6 @@ ENTRY(startup_32_smp)
 	movl %eax,%cr0
 
 	lgdt early_gdt_descr
-	lidt idt_descr
 	ljmp $(__KERNEL_CS),$1f
 1:	movl $(__KERNEL_DS),%eax	# reload all the segment registers
 	movl %eax,%ss			# after changing gdt.
@@ -378,37 +377,6 @@ ENDPROC(startup_32_smp)
  */
 __INIT
 setup_once:
-	/*
-	 * Set up a idt with 256 interrupt gates that push zero if there
-	 * is no error code and then jump to early_idt_handler_common.
-	 * It doesn't actually load the idt - that needs to be done on
-	 * each CPU. Interrupts are enabled elsewhere, when we can be
-	 * relatively sure everything is ok.
-	 */
-
-	movl $idt_table,%edi
-	movl $early_idt_handler_array,%eax
-	movl $NUM_EXCEPTION_VECTORS,%ecx
-1:
-	movl %eax,(%edi)
-	movl %eax,4(%edi)
-	/* interrupt gate, dpl=0, present */
-	movl $(0x8E000000 + __KERNEL_CS),2(%edi)
-	addl $EARLY_IDT_HANDLER_SIZE,%eax
-	addl $8,%edi
-	loop 1b
-
-	movl $256 - NUM_EXCEPTION_VECTORS,%ecx
-	movl $ignore_int,%edx
-	movl $(__KERNEL_CS << 16),%eax
-	movw %dx,%ax		/* selector = 0x0010 = cs */
-	movw $0x8E00,%dx	/* interrupt gate - dpl=0, present */
-2:
-	movl %eax,(%edi)
-	movl %edx,4(%edi)
-	addl $8,%edi
-	loop 2b
-
 #ifdef CONFIG_CC_STACKPROTECTOR
 	/*
 	 * Configure the stack canary. The linker can't handle this by
@@ -498,7 +466,7 @@ ENDPROC(early_idt_handler_common)
 
 /* This is the default interrupt "handler" :-) */
 	ALIGN
-ignore_int:
+ENTRY(early_ignore_irq)
 	cld
 #ifdef CONFIG_PRINTK
 	pushl %eax
@@ -533,7 +501,7 @@ ENDPROC(early_idt_handler_common)
 hlt_loop:
 	hlt
 	jmp hlt_loop
-ENDPROC(ignore_int)
+ENDPROC(early_ignore_irq)
 __INITDATA
 	.align 4
 GLOBAL(early_recursion_flag)
arch/x86/kernel/idt.c +4 -0
Index: b/arch/x86/kernel/idt.c
===================================================================
@@ -34,6 +34,10 @@ void __init idt_setup_early_handler(void
 
 	for (i = 0; i < NUM_EXCEPTION_VECTORS; i++)
 		set_intr_gate(i, early_idt_handler_array[i]);
+#ifdef CONFIG_X86_32
+	for ( ; i < NR_VECTORS; i++)
+		set_intr_gate(i, early_ignore_irq);
+#endif
 	load_idt((const struct desc_ptr *)&idt_descr);
 }
 
#53 lore raw

PATCH 29/41 x86/idt: Prepare for table based init

In reply to Thomas Gleixner

The IDT setup code is handled in several places. All of them use variants of set_intr_gate() inlines. This can be done with a table based initialization, which allows to reduce the inline zoo and puts all IDT related code and information into a single place.

Add the infrastructure.

---
 arch/x86/kernel/idt.c |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)
arch/x86/kernel/idt.c +67 -0
Index: b/arch/x86/kernel/idt.c
===================================================================
@@ -5,8 +5,49 @@
  */
 #include <linux/interrupt.h>
 
+#include <asm/traps.h>
+#include <asm/proto.h>
 #include <asm/desc.h>
 
+struct idt_data {
+	unsigned int	vector;
+	unsigned int	segment;
+	struct idt_bits	bits;
+	const void	*addr;
+};
+
+#define DPL0		0x0
+#define DPL3		0x3
+
+#define DEFAULT_STACK	0
+
+#define G(_vector, _addr, _ist, _type, _dpl, _segment)	\
+	{						\
+		.vector		= _vector,		\
+		.bits.ist	= _ist,			\
+		.bits.type	= _type,		\
+		.bits.dpl	= _dpl,			\
+		.bits.p		= 1,			\
+		.addr		= _addr,		\
+		.segment	= _segment,		\
+	}
+
+/* Interrupt gate */
+#define INTG(_vector, _addr)				\
+	G(_vector, _addr, DEFAULT_STACK, GATE_INTERRUPT, DPL0, __KERNEL_CS)
+
+/* System interrupt gate */
+#define SYSG(_vector, _addr)				\
+	G(_vector, _addr, DEFAULT_STACK, GATE_INTERRUPT, DPL3, __KERNEL_CS)
+
+/* Interrupt gate with interrupt stack */
+#define ISTG(_vector, _addr, _ist)			\
+	G(_vector, _addr, _ist, GATE_INTERRUPT, DPL0, __KERNEL_CS)
+
+/* Task gate */
+#define TSKG(_vector, _gdt)				\
+	G(_vector, NULL, DEFAULT_STACK, GATE_TASK, DPL0, _gdt << 3)
+
 /* Must be page-aligned because the real IDT is used in a fixmap. */
 gate_desc idt_table[IDT_ENTRIES] __page_aligned_bss;
 
@@ -25,6 +66,32 @@ const struct desc_ptr debug_idt_descr =
 };
 #endif
 
+static inline void idt_init_desc(gate_desc *gate, const struct idt_data *d)
+{
+	unsigned long addr = (unsigned long) d->addr;
+
+	gate->offset_low	= (u16) addr;
+	gate->segment		= (u16) d->segment;
+	gate->bits		= d->bits;
+	gate->offset_middle	= (u16) (addr >> 16);
+#ifdef CONFIG_X86_64
+	gate->offset_high	= (u32) (addr >> 32);
+	gate->reserved		= 0;
+#endif
+}
+
+static __init void
+idt_setup_from_table(gate_desc *idt, const struct idt_data *t, int size)
+{
+	gate_desc desc;
+
+	for (; size > 0; t++, size--) {
+		idt_init_desc(&desc, t);
+		set_bit(t->vector, used_vectors);
+		write_idt_entry(idt, t->vector, &desc);
+	}
+}
+
 /**
  * idt_setup_early_handler - Initializes the idt table with early handlers
  */
#54 lore raw

PATCH 30/41 x86/idt: Switch early trap init to IDT tables

In reply to Thomas Gleixner

Add the initialization table for the early trap setup and replace the early trap init code.

---
 arch/x86/kernel/idt.c   |   52 ++++++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/setup.c |    4 +--
 arch/x86/kernel/traps.c |   27 ------------------------
 3 files changed, 54 insertions(+), 29 deletions(-)
arch/x86/kernel/idt.c +52 -0
Index: b/arch/x86/kernel/idt.c
===================================================================
@@ -48,6 +48,28 @@ struct idt_data {
 #define TSKG(_vector, _gdt)				\
 	G(_vector, NULL, DEFAULT_STACK, GATE_TASK, DPL0, _gdt << 3)
 
+/*
+ * Early traps running on the DEFAULT_STACK because the other interrupt
+ * stacks work only after cpu_init().
+ */
+static const __initdata struct idt_data early_idts[] = {
+	INTG(X86_TRAP_DB,		debug),
+	SYSG(X86_TRAP_BP,		int3),
+#ifdef CONFIG_X86_32
+	INTG(X86_TRAP_PF,		page_fault),
+#endif
+};
+
+#ifdef CONFIG_X86_64
+/*
+ * Early traps running on the DEFAULT_STACK because the other interrupt
+ * stacks work only after cpu_init().
+ */
+static const __initdata struct idt_data early_pf_idts[] = {
+	INTG(X86_TRAP_PF,		page_fault),
+};
+#endif
+
 /* Must be page-aligned because the real IDT is used in a fixmap. */
 gate_desc idt_table[IDT_ENTRIES] __page_aligned_bss;
 
@@ -93,6 +115,36 @@ idt_setup_from_table(gate_desc *idt, con
 }
 
 /**
+ * idt_setup_early_traps - Initialize the idt table with early traps
+ *
+ * On X8664 these traps do not use interrupt stacks as they can't work
+ * before cpu_init() is invoked and sets up TSS. The IST variants are
+ * installed after that.
+ */
+void __init idt_setup_early_traps(void)
+{
+	idt_setup_from_table(idt_table, early_idts, ARRAY_SIZE(early_idts));
+}
+
+#ifdef CONFIG_X86_64
+/**
+ * idt_setup_early_pf - Initialize the idt table with early pagefault handler
+ *
+ * On X8664 this does not use interrupt stacks as they can't work before
+ * cpu_init() is invoked and sets up TSS. The IST variant is installed
+ * after that.
+ *
+ * FIXME: Why is 32bit and 64bit installing the PF handler at different
+ * places in the early setup code?
+ */
+void __init idt_setup_early_pf(void)
+{
+	idt_setup_from_table(idt_table, early_pf_idts,
+			     ARRAY_SIZE(early_pf_idts));
+}
+#endif
+
+/**
  * idt_setup_early_handler - Initializes the idt table with early handlers
  */
 void __init idt_setup_early_handler(void)
arch/x86/kernel/setup.c +2 -2
Index: b/arch/x86/kernel/setup.c
===================================================================
@@ -891,7 +891,7 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	olpc_ofw_detect();
 
-	early_trap_init();
+	idt_setup_early_traps();
 	early_cpu_init();
 	early_ioremap_init();
 
@@ -1162,7 +1162,7 @@ void __init setup_arch(char **cmdline_p)
 
 	init_mem_mapping();
 
-	early_trap_pf_init();
+	idt_setup_early_pf();
 
 	/*
 	 * Update mmu_cr4_features (and, indirectly, trampoline_cr4_features)
arch/x86/kernel/traps.c +0 -27
Index: b/arch/x86/kernel/traps.c
===================================================================
@@ -923,33 +923,6 @@ dotraplinkage void do_iret_error(struct
 }
 #endif
 
-/* Set of traps needed for early debugging. */
-void __init early_trap_init(void)
-{
-	/*
-	 * Don't use IST to set DEBUG_STACK as it doesn't work until TSS
-	 * is ready in cpu_init() <-- trap_init(). Before trap_init(),
-	 * CPU runs at ring 0 so it is impossible to hit an invalid
-	 * stack.  Using the original stack works well enough at this
-	 * early stage. DEBUG_STACK will be equipped after cpu_init() in
-	 * trap_init().
-	 */
-	set_intr_gate(X86_TRAP_DB, debug);
-	/* int3 can be called from all */
-	set_system_intr_gate(X86_TRAP_BP, &int3);
-#ifdef CONFIG_X86_32
-	set_intr_gate(X86_TRAP_PF, page_fault);
-#endif
-	load_idt(&idt_descr);
-}
-
-void __init early_trap_pf_init(void)
-{
-#ifdef CONFIG_X86_64
-	set_intr_gate(X86_TRAP_PF, page_fault);
-#endif
-}
-
 void __init trap_init(void)
 {
 	int i;
maintainer
#55 lore raw

Re: [patch 30/41] x86/idt: Switch early trap init to IDT tables

Quoted 2 lines · Thomas Gleixner

Add the initialization table for the early trap setup and replace the early trap init code.

Nice.

#56 lore raw

PATCH 31/41 x86/idt: Move debug stack init to table based

In reply to Thomas Gleixner

Add the debug_idt init table and make use of it.

---
 arch/x86/include/asm/desc.h |    2 ++
 arch/x86/kernel/idt.c       |   23 +++++++++++++++++++++++
 arch/x86/kernel/traps.c     |    6 +-----
 3 files changed, 26 insertions(+), 5 deletions(-)
arch/x86/include/asm/desc.h +2 -0
@@ -516,8 +516,10 @@ extern void idt_setup_early_traps(void);
 
 #ifdef CONFIG_X86_64
 extern void idt_setup_early_pf(void);
+extern void idt_setup_debugidt_traps(void);
 #else
 static inline void idt_setup_early_pf(void) { }
+static inline void idt_setup_debugidt_traps(void) { }
 #endif
 
 extern void idt_invalidate(void *addr);
arch/x86/kernel/idt.c +23 -0
@@ -68,6 +68,15 @@ static const __initdata struct idt_data
 static const __initdata struct idt_data early_pf_idts[] = {
 	INTG(X86_TRAP_PF,		page_fault),
 };
+
+/*
+ * Override for the debug_idt. Same as the default, but with interrupt
+ * stack set to DEFAULT_STACK (0). Required for NMI trap handling.
+ */
+static const __initdata struct idt_data dbg_idts[] = {
+	INTG(X86_TRAP_DB,	debug),
+	INTG(X86_TRAP_BP,	int3),
+};
 #endif
 
 /* Must be page-aligned because the real IDT is used in a fixmap. */
@@ -82,6 +91,10 @@ struct desc_ptr idt_descr __ro_after_ini
 /* No need to be aligned, but done to keep all IDTs defined the same way. */
 gate_desc debug_idt_table[IDT_ENTRIES] __page_aligned_bss;
 
+/*
+ * Override for the debug_idt. Same as the default, but with interrupt
+ * stack set to DEFAULT_STACK (0). Required for NMI trap handling.
+ */
 const struct desc_ptr debug_idt_descr = {
 	.size		= IDT_ENTRIES * 16 - 1,
 	.address	= (unsigned long) debug_idt_table,
@@ -142,6 +155,16 @@ void __init idt_setup_early_pf(void)
 	idt_setup_from_table(idt_table, early_pf_idts,
 			     ARRAY_SIZE(early_pf_idts));
 }
+
+/**
+ * idt_setup_debugidt_traps - Initialize the debug idt table with debug traps
+ */
+void __init idt_setup_debugidt_traps(void)
+{
+	memcpy(&debug_idt_table, &idt_table, IDT_ENTRIES * 16);
+
+	idt_setup_from_table(debug_idt_table, dbg_idts, ARRAY_SIZE(dbg_idts));
+}
 #endif
 
 /**
arch/x86/kernel/traps.c +1 -5
@@ -990,9 +990,5 @@ void __init trap_init(void)
 
 	x86_init.irqs.trap_init();
 
-#ifdef CONFIG_X86_64
-	memcpy(&debug_idt_table, &idt_table, IDT_ENTRIES * 16);
-	set_nmi_gate(X86_TRAP_DB, &debug);
-	set_nmi_gate(X86_TRAP_BP, &int3);
-#endif
+	idt_setup_debugidt_traps();
 }
#57 lore raw

PATCH 32/41 x86/idt: Move ist stack based traps to table init

In reply to Thomas Gleixner

Initialize the IST based traps via a table

---
 arch/x86/include/asm/desc.h |    2 ++
 arch/x86/kernel/idt.c       |   22 ++++++++++++++++++++++
 arch/x86/kernel/traps.c     |    9 +--------
 3 files changed, 25 insertions(+), 8 deletions(-)
arch/x86/include/asm/desc.h +2 -0
@@ -516,9 +516,11 @@ extern void idt_setup_early_traps(void);
 
 #ifdef CONFIG_X86_64
 extern void idt_setup_early_pf(void);
+extern void idt_setup_ist_traps(void);
 extern void idt_setup_debugidt_traps(void);
 #else
 static inline void idt_setup_early_pf(void) { }
+static inline void idt_setup_ist_traps(void) { }
 static inline void idt_setup_debugidt_traps(void) { }
 #endif
 
arch/x86/kernel/idt.c +22 -0
@@ -92,6 +92,20 @@ struct desc_ptr idt_descr __ro_after_ini
 gate_desc debug_idt_table[IDT_ENTRIES] __page_aligned_bss;
 
 /*
+ * The exceptions which use Interrupt stacks. They are setup after
+ * cpu_init() when the TSS has been initialized.
+ */
+static const __initdata struct idt_data ist_idts[] = {
+	ISTG(X86_TRAP_DB,	debug,		DEBUG_STACK),
+	ISTG(X86_TRAP_NMI,	nmi,		NMI_STACK),
+	ISTG(X86_TRAP_BP,	int3,		DEBUG_STACK),
+	ISTG(X86_TRAP_DF,	double_fault,	DOUBLEFAULT_STACK),
+#ifdef CONFIG_X86_MCE
+	ISTG(X86_TRAP_MC,	&machine_check,	MCE_STACK),
+#endif
+};
+
+/*
  * Override for the debug_idt. Same as the default, but with interrupt
  * stack set to DEFAULT_STACK (0). Required for NMI trap handling.
  */
@@ -157,6 +171,14 @@ void __init idt_setup_early_pf(void)
 }
 
 /**
+ * idt_setup_ist_traps - Initialize the idt table with traps using IST
+ */
+void __init idt_setup_ist_traps(void)
+{
+	idt_setup_from_table(idt_table, ist_idts, ARRAY_SIZE(ist_idts));
+}
+
+/**
  * idt_setup_debugidt_traps - Initialize the debug idt table with debug traps
  */
 void __init idt_setup_debugidt_traps(void)
arch/x86/kernel/traps.c +1 -8
@@ -979,14 +979,7 @@ void __init trap_init(void)
 	 */
 	cpu_init();
 
-	/*
-	 * X86_TRAP_DB and X86_TRAP_BP have been set
-	 * in early_trap_init(). However, ITS works only after
-	 * cpu_init() loads TSS. See comments in early_trap_init().
-	 */
-	set_intr_gate_ist(X86_TRAP_DB, &debug, DEBUG_STACK);
-	/* int3 can be called from all */
-	set_system_intr_gate_ist(X86_TRAP_BP, &int3, DEBUG_STACK);
+	idt_setup_ist_traps();
 
 	x86_init.irqs.trap_init();
 
#58 lore raw

PATCH 33/41 x86/idt: Move regular trap init to tables

In reply to Thomas Gleixner

Initialize the regular traps with a table.

---
 arch/x86/include/asm/desc.h |    1 
 arch/x86/kernel/idt.c       |   51 ++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/traps.c     |   42 +-----------------------------------
 3 files changed, 54 insertions(+), 40 deletions(-)
arch/x86/include/asm/desc.h +1 -0
@@ -513,6 +513,7 @@ static inline void load_current_idt(void
 
 extern void idt_setup_early_handler(void);
 extern void idt_setup_early_traps(void);
+extern void idt_setup_traps(void);
 
 #ifdef CONFIG_X86_64
 extern void idt_setup_early_pf(void);
arch/x86/kernel/idt.c +51 -0

63 lines not on this page · raw message

arch/x86/kernel/traps.c +2 -40

48 lines not on this page · raw message

#59 lore raw

PATCH 34/41 x86/idt: Move APIC gate initialization to tables

In reply to Thomas Gleixner

Replace the APIC/SMP vector gate initialization with the table based mechanism.

---
 arch/x86/include/asm/desc.h |    1 
 arch/x86/kernel/idt.c       |   48 ++++++++++++++++++++++++++++++
 arch/x86/kernel/irqinit.c   |   69 --------------------------------------------
 3 files changed, 50 insertions(+), 68 deletions(-)
arch/x86/include/asm/desc.h +1 -0
@@ -514,6 +514,7 @@ static inline void load_current_idt(void
 extern void idt_setup_early_handler(void);
 extern void idt_setup_early_traps(void);
 extern void idt_setup_traps(void);
+extern void idt_setup_apic_and_irq_gates(void);
 
 #ifdef CONFIG_X86_64
 extern void idt_setup_early_pf(void);
arch/x86/kernel/idt.c +48 -0

60 lines not on this page · raw message

arch/x86/kernel/irqinit.c +1 -68

81 lines not on this page · raw message

#60 lore raw

PATCH 35/41 x86/idt: Move interrupt gate initialization to IDT code

In reply to Thomas Gleixner

Move the gate intialization from interrupt init to the IDT code so all IDT related operations are at a single place.

---
 arch/x86/kernel/idt.c     |   18 ++++++++++++++++++
 arch/x86/kernel/irqinit.c |   18 ------------------
 2 files changed, 18 insertions(+), 18 deletions(-)
arch/x86/kernel/idt.c +18 -0
@@ -285,7 +285,25 @@ void __init idt_setup_debugidt_traps(voi
  */
 void __init idt_setup_apic_and_irq_gates(void)
 {
+	int i = FIRST_EXTERNAL_VECTOR;
+	void *entry;
+
 	idt_setup_from_table(idt_table, apic_idts, ARRAY_SIZE(apic_idts));
+
+	for_each_clear_bit_from(i, used_vectors, first_system_vector) {
+		entry = irq_entries_start + 8 * (i - FIRST_EXTERNAL_VECTOR);
+		set_intr_gate(i, entry);
+	}
+
+	for_each_clear_bit_from(i, used_vectors, NR_VECTORS) {
+#ifdef CONFIG_X86_LOCAL_APIC
+		set_bit(i, used_vectors);
+		set_intr_gate(i, spurious_interrupt);
+#else
+		entry = irq_entries_start + 8 * (i - FIRST_EXTERNAL_VECTOR);
+		set_intr_gate(i, entry);
+#endif
+	}
 }
 
 /**
arch/x86/kernel/irqinit.c +0 -18
@@ -89,29 +89,11 @@ void __init init_IRQ(void)
 
 void __init native_init_IRQ(void)
 {
-	int i;
-
 	/* Execute any quirks before the call gates are initialised: */
 	x86_init.irqs.pre_vector_init();
 
 	idt_setup_apic_and_irq_gates();
 
-	/*
-	 * Cover the whole vector space, no vector can escape
-	 * us. (some of these will be overridden and become
-	 * 'special' SMP interrupts)
-	 */
-	i = FIRST_EXTERNAL_VECTOR;
-	for_each_clear_bit_from(i, used_vectors, first_system_vector) {
-		/* IA32_SYSCALL_VECTOR could be used in trap_init already. */
-		set_intr_gate(i, irq_entries_start +
-				8 * (i - FIRST_EXTERNAL_VECTOR));
-	}
-#ifdef CONFIG_X86_LOCAL_APIC
-	for_each_clear_bit_from(i, used_vectors, NR_VECTORS)
-		set_intr_gate(i, spurious_interrupt);
-#endif
-
 	if (!acpi_ioapic && !of_ioapic && nr_legacy_irqs())
 		setup_irq(2, &irq2);
 
#61 lore raw

PATCH 36/41 x86/idt: Remove unused functions/inlines

In reply to Thomas Gleixner

The IDT related inlines are not longer used. Remove them.

---
 arch/x86/include/asm/desc.h |   36 ------------------------------------
 1 file changed, 36 deletions(-)
arch/x86/include/asm/desc.h +0 -36
@@ -389,16 +389,6 @@ static inline void set_desc_limit(struct
 	desc->limit1 = (limit >> 16) & 0xf;
 }
 
-#ifdef CONFIG_X86_64
-static inline void set_nmi_gate(int gate, void *addr)
-{
-	gate_desc s;
-
-	pack_gate(&s, GATE_INTERRUPT, (unsigned long)addr, 0, 0, __KERNEL_CS);
-	write_idt_entry(debug_idt_table, gate, &s);
-}
-#endif
-
 static inline void _set_gate(int gate, unsigned type, const void *addr,
 			     unsigned dpl, unsigned ist, unsigned seg)
 {
@@ -444,32 +434,6 @@ static inline void alloc_system_vector(i
 		set_intr_gate(n, addr);				\
 	} while (0)
 
-/*
- * This routine sets up an interrupt gate at directory privilege level 3.
- */
-static inline void set_system_intr_gate(unsigned int n, void *addr)
-{
-	BUG_ON((unsigned)n > 0xFF);
-	_set_gate(n, GATE_INTERRUPT, addr, 0x3, 0, __KERNEL_CS);
-}
-
-static inline void set_task_gate(unsigned int n, unsigned int gdt_entry)
-{
-	BUG_ON((unsigned)n > 0xFF);
-	_set_gate(n, GATE_TASK, (void *)0, 0, 0, (gdt_entry<<3));
-}
-
-static inline void set_intr_gate_ist(int n, void *addr, unsigned ist)
-{
-	BUG_ON((unsigned)n > 0xFF);
-	_set_gate(n, GATE_INTERRUPT, addr, 0, ist, __KERNEL_CS);
-}
-
-static inline void set_system_intr_gate_ist(int n, void *addr, unsigned ist)
-{
-	BUG_ON((unsigned)n > 0xFF);
-	_set_gate(n, GATE_INTERRUPT, addr, 0x3, ist, __KERNEL_CS);
-}
 
 #ifdef CONFIG_X86_64
 DECLARE_PER_CPU(u32, debug_idt_ctr);
#62 lore raw

PATCH 37/41 x86/irq: Get rid of the first_system_vector bogisity

In reply to Thomas Gleixner

This variable is beyond pointless. Nothing allocates a vector via alloc_gate() below FIRST_SYSTEM_VECTOR. So nothing can change first_system_vector.

If there is a need for a gate below FIRST_SYSTEM_VECTOR then it can be added to the vector defines and FIRST_SYSTEM_VECTOR adjusted accordingly.

---
 arch/x86/include/asm/desc.h   |   11 ++---------
 arch/x86/kernel/apic/apic.c   |    2 --
 arch/x86/kernel/apic/vector.c |    2 +-
 arch/x86/kernel/idt.c         |    2 +-
 arch/x86/kernel/irq.c         |    2 +-
 5 files changed, 5 insertions(+), 14 deletions(-)
arch/x86/include/asm/desc.h +2 -9
@@ -408,21 +408,14 @@ static inline void set_intr_gate(unsigne
 	_set_gate(n, GATE_INTERRUPT, addr, 0, 0, __KERNEL_CS);
 }
 
-#ifdef CONFIG_X86_LOCAL_APIC
-extern int first_system_vector;
-#else
-#define first_system_vector		NR_VECTORS
-#endif
-
 /* used_vectors is BITMAP for irq is not managed by percpu vector_irq */
 extern unsigned long used_vectors[];
 
-static inline void alloc_system_vector(int vector)
+static inline void alloc_system_vector(unsigned int vector)
 {
+	BUG_ON(vector < FIRST_SYSTEM_VECTOR);
 	if (!test_bit(vector, used_vectors)) {
 		set_bit(vector, used_vectors);
-		if (first_system_vector > vector)
-			first_system_vector = vector;
 	} else {
 		BUG();
 	}
arch/x86/kernel/apic/apic.c +0 -2
@@ -177,8 +177,6 @@ static int disable_apic_timer __initdata
 int local_apic_timer_c2_ok;
 EXPORT_SYMBOL_GPL(local_apic_timer_c2_ok);
 
-int first_system_vector = FIRST_SYSTEM_VECTOR;
-
 /*
  * Debug level, exported for io_apic.c
  */
arch/x86/kernel/apic/vector.c +1 -1
@@ -166,7 +166,7 @@ static int __assign_irq_vector(int irq,
 		offset = current_offset;
 next:
 		vector += 16;
-		if (vector >= first_system_vector) {
+		if (vector >= FIRST_SYSTEM_VECTOR) {
 			offset = (offset + 1) % 16;
 			vector = FIRST_EXTERNAL_VECTOR + offset;
 		}
arch/x86/kernel/idt.c +1 -1
@@ -290,7 +290,7 @@ void __init idt_setup_apic_and_irq_gates
 
 	idt_setup_from_table(idt_table, apic_idts, ARRAY_SIZE(apic_idts));
 
-	for_each_clear_bit_from(i, used_vectors, first_system_vector) {
+	for_each_clear_bit_from(i, used_vectors, FIRST_SYSTEM_VECTOR) {
 		entry = irq_entries_start + 8 * (i - FIRST_EXTERNAL_VECTOR);
 		set_intr_gate(i, entry);
 	}
arch/x86/kernel/irq.c +1 -1
@@ -414,7 +414,7 @@ int check_irq_vectors_for_cpu_disable(vo
 		 * this w/o holding vector_lock.
 		 */
 		for (vector = FIRST_EXTERNAL_VECTOR;
-		     vector < first_system_vector; vector++) {
+		     vector < FIRST_SYSTEM_VECTOR; vector++) {
 			if (!test_bit(vector, used_vectors) &&
 			    IS_ERR_OR_NULL(per_cpu(vector_irq, cpu)[vector])) {
 				if (++count == this_count)
#63 lore raw

PATCH 38/41 x86/idt: Deinline setup functions

In reply to Thomas Gleixner
---
 arch/x86/include/asm/desc.h |   37 ++-----------------------------------
 arch/x86/kernel/idt.c       |   42 +++++++++++++++++++++++++++++++++---------
 2 files changed, 35 insertions(+), 44 deletions(-)
arch/x86/include/asm/desc.h +2 -35
@@ -389,45 +389,12 @@ static inline void set_desc_limit(struct
 	desc->limit1 = (limit >> 16) & 0xf;
 }
 
-static inline void _set_gate(int gate, unsigned type, const void *addr,
-			     unsigned dpl, unsigned ist, unsigned seg)
-{
-	gate_desc s;
-
-	pack_gate(&s, type, (unsigned long)addr, dpl, ist, seg);
-	/*
-	 * does not need to be atomic because it is only done once at
-	 * setup time
-	 */
-	write_idt_entry(idt_table, gate, &s);
-}
-
-static inline void set_intr_gate(unsigned int n, const void *addr)
-{
-	BUG_ON((unsigned)n > 0xFF);
-	_set_gate(n, GATE_INTERRUPT, addr, 0, 0, __KERNEL_CS);
-}
+void set_intr_gate(unsigned int n, const void *addr);
+void alloc_intr_gate(unsigned int n, const void *addr);
 
 /* used_vectors is BITMAP for irq is not managed by percpu vector_irq */
 extern unsigned long used_vectors[];
 
-static inline void alloc_system_vector(unsigned int vector)
-{
-	BUG_ON(vector < FIRST_SYSTEM_VECTOR);
-	if (!test_bit(vector, used_vectors)) {
-		set_bit(vector, used_vectors);
-	} else {
-		BUG();
-	}
-}
-
-#define alloc_intr_gate(n, addr)				\
-	do {							\
-		alloc_system_vector(n);				\
-		set_intr_gate(n, addr);				\
-	} while (0)
-
-
 #ifdef CONFIG_X86_64
 DECLARE_PER_CPU(u32, debug_idt_ctr);
 static inline bool is_debug_idt_enabled(void)
arch/x86/kernel/idt.c +33 -9

93 lines not on this page · raw message

#64 lore raw

PATCH 39/41 x86/idt: Simplify alloc_intr_gate

In reply to Thomas Gleixner

The only users of alloc_intr_gate() are hypervisors, which both check the used_vectors bitmap whether they have allocated the gate already. Move that check into alloc_intr_gate() and simplify the users.

Cc "K. Y. Srinivasan" <[email protected]>
Cc Stephen Hemminger <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc Juergen Gross <[email protected]>
---
 arch/x86/kernel/cpu/mshyperv.c   |    9 ++-------
 arch/x86/kernel/idt.c            |    6 +++---
 drivers/xen/events/events_base.c |    6 ++----
 3 files changed, 7 insertions(+), 14 deletions(-)
arch/x86/kernel/cpu/mshyperv.c +2 -7
@@ -59,13 +59,8 @@ void hyperv_vector_handler(struct pt_reg
 void hv_setup_vmbus_irq(void (*handler)(void))
 {
 	vmbus_handler = handler;
-	/*
-	 * Setup the IDT for hypervisor callback. Prevent reallocation
-	 * at module reload.
-	 */
-	if (!test_bit(HYPERVISOR_CALLBACK_VECTOR, used_vectors))
-		alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR,
-				hyperv_callback_vector);
+	/* Setup the IDT for hypervisor callback */
+	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, hyperv_callback_vector);
 }
 
 void hv_remove_vmbus_irq(void)
arch/x86/kernel/idt.c +3 -3
@@ -352,7 +352,7 @@ void set_intr_gate(unsigned int n, const
 
 void alloc_intr_gate(unsigned int n, const void *addr)
 {
-	BUG_ON(test_bit(n, used_vectors) || n < FIRST_SYSTEM_VECTOR);
-	set_bit(n, used_vectors);
-	set_intr_gate(n, addr);
+	BUG_ON(n < FIRST_SYSTEM_VECTOR);
+	if (!test_and_set_bit(n, used_vectors))
+		set_intr_gate(n, addr);
 }
drivers/xen/events/events_base.c +2 -4
@@ -1653,10 +1653,8 @@ void xen_callback_vector(void)
 			return;
 		}
 		pr_info("Xen HVM callback vector for event delivery is enabled\n");
-		/* in the restore case the vector has already been allocated */
-		if (!test_bit(HYPERVISOR_CALLBACK_VECTOR, used_vectors))
-			alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR,
-					xen_hvm_callback_vector);
+		alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR,
+				xen_hvm_callback_vector);
 	}
 }
 #else
maintainer Reviewed-by on v1 39/41
#65 lore raw

Re: [patch 39/41] x86/idt: Simplify alloc_intr_gate

Quoted 9 lines · Thomas Gleixner

The only users of alloc_intr_gate() are hypervisors, which both check the used_vectors bitmap whether they have allocated the gate already. Move that check into alloc_intr_gate() and simplify the users.

Cc "K. Y. Srinivasan" <[email protected]>
Cc Stephen Hemminger <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc Juergen Gross <[email protected]>

Juergen

#66 lore raw

PATCH 40/41 x86/idt: Hide set_intr_gate()

In reply to Thomas Gleixner

set_intr_gate() is an internal function of the IDT code. The only user left is the KVM code which replaces the pagefault handler eventually.

Provide an explicit update_intr_gate() function and make set_intr_gate() static. While at it replace the magic number 14 in the KVM code with the proper trap define.

---
 arch/x86/include/asm/desc.h |    2 +-
 arch/x86/kernel/idt.c       |   33 ++++++++++++++++++++-------------
 arch/x86/kernel/kvm.c       |    2 +-
 3 files changed, 22 insertions(+), 15 deletions(-)
arch/x86/include/asm/desc.h +1 -1
@@ -389,7 +389,7 @@ static inline void set_desc_limit(struct
 	desc->limit1 = (limit >> 16) & 0xf;
 }
 
-void set_intr_gate(unsigned int n, const void *addr);
+void update_intr_gate(unsigned int n, const void *addr);
 void alloc_intr_gate(unsigned int n, const void *addr);
 
 /* used_vectors is BITMAP for irq is not managed by percpu vector_irq */
arch/x86/kernel/idt.c +20 -13
@@ -225,6 +225,22 @@ idt_setup_from_table(gate_desc *idt, con
 	}
 }
 
+static void set_intr_gate(unsigned int n, const void *addr)
+{
+	struct idt_data data;
+
+	BUG_ON(n > 0xFF);
+
+	memset(&data, 0, sizeof(data));
+	data.vector	= n;
+	data.addr	= addr;
+	data.segment	= __KERNEL_CS;
+	data.bits.type	= GATE_INTERRUPT;
+	data.bits.p	= 1;
+
+	idt_setup_from_table(idt_table, &data, 1, false);
+}
+
 /**
  * idt_setup_early_traps - Initialize the idt table with early traps
  *
@@ -334,20 +350,11 @@ void idt_invalidate(void *addr)
 	load_idt(&idt);
 }
 
-void set_intr_gate(unsigned int n, const void *addr)
+void __init update_intr_gate(unsigned int n, const void *addr)
 {
-	struct idt_data data;
-
-	BUG_ON(n > 0xFF);
-
-	memset(&data, 0, sizeof(data));
-	data.vector	= n;
-	data.addr	= addr;
-	data.segment	= __KERNEL_CS;
-	data.bits.type	= GATE_INTERRUPT;
-	data.bits.p	= 1;
-
-	idt_setup_from_table(idt_table, &data, 1, false);
+	if (WARN_ON_ONCE(test_bit(n, used_vectors)))
+		return;
+	set_intr_gate(n, addr);
 }
 
 void alloc_intr_gate(unsigned int n, const void *addr)
arch/x86/kernel/kvm.c +1 -1
@@ -455,7 +455,7 @@ static int kvm_cpu_down_prepare(unsigned
 
 static void __init kvm_apf_trap_init(void)
 {
-	set_intr_gate(14, async_page_fault);
+	update_intr_gate(X86_TRAP_PF, async_page_fault);
 }
 
 void __init kvm_guest_init(void)
maintainer Acked-by on v1 40/41
#67 lore raw

Re: [patch 40/41] x86/idt: Hide set_intr_gate()

Quoted 9 lines · Thomas Gleixner

set_intr_gate() is an internal function of the IDT code. The only user left is the KVM code which replaces the pagefault handler eventually.

Provide an explicit update_intr_gate() function and make set_intr_gate() static. While at it replace the magic number 14 in the KVM code with the proper trap define.

Thanks,

Paolo

#68 lore raw

PATCH 41/41 x86/irq: Remove duplicated used_vectors definition

In reply to Thomas Gleixner
---
 arch/x86/include/asm/irq.h |    3 ---
 1 file changed, 3 deletions(-)
arch/x86/include/asm/irq.h +0 -3
@@ -42,9 +42,6 @@ extern bool handle_irq(struct irq_desc *
 
 extern __visible unsigned int do_IRQ(struct pt_regs *regs);
 
-/* Interrupt vector management */
-extern DECLARE_BITMAP(used_vectors, NR_VECTORS);
-
 extern void init_ISA_irqs(void);
 
 #ifdef CONFIG_X86_LOCAL_APIC