All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch 0/5] x86/idt: Cleanups and consolidation
@ 2020-05-28 14:53 Thomas Gleixner
  2020-05-28 14:53 ` [patch 1/5] x86/idt: Mark init only functions __init Thomas Gleixner
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Thomas Gleixner @ 2020-05-28 14:53 UTC (permalink / raw)
  To: LKML; +Cc: x86

IDT functionality is spread out to places which results in way too many
globals.

This series cleans that up, fixes comments and a few other oddities which I
noticed while working on that code.

Thanks,

	tglx

---
 include/asm/desc.h  |   47 --------------------
 kernel/cpu/common.c |   17 -------
 kernel/idt.c        |  120 +++++++++++++++++++++++++++++++++++++---------------
 kernel/traps.c      |   11 ----
 4 files changed, 88 insertions(+), 107 deletions(-)



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [patch 1/5] x86/idt: Mark init only functions __init
  2020-05-28 14:53 [patch 0/5] x86/idt: Cleanups and consolidation Thomas Gleixner
@ 2020-05-28 14:53 ` Thomas Gleixner
  2020-05-30  9:57   ` [tip: x86/entry] " tip-bot2 for Thomas Gleixner
  2020-05-28 14:53 ` [patch 2/5] x86/idt: Add comments about early #PF handling Thomas Gleixner
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Thomas Gleixner @ 2020-05-28 14:53 UTC (permalink / raw)
  To: LKML; +Cc: x86

Since 8175cfbbbfcb ("x86/idt: Remove update_intr_gate()") set_intr_gate()
and idt_setup_from_table() are only called from __init functions. Mark them
as well.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/idt.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -217,7 +217,7 @@ static inline void idt_init_desc(gate_de
 #endif
 }
 
-static void
+static __init void
 idt_setup_from_table(gate_desc *idt, const struct idt_data *t, int size, bool sys)
 {
 	gate_desc desc;
@@ -230,7 +230,7 @@ idt_setup_from_table(gate_desc *idt, con
 	}
 }
 
-static void set_intr_gate(unsigned int n, const void *addr)
+static __init void set_intr_gate(unsigned int n, const void *addr)
 {
 	struct idt_data data;
 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [patch 2/5] x86/idt: Add comments about early #PF handling
  2020-05-28 14:53 [patch 0/5] x86/idt: Cleanups and consolidation Thomas Gleixner
  2020-05-28 14:53 ` [patch 1/5] x86/idt: Mark init only functions __init Thomas Gleixner
@ 2020-05-28 14:53 ` Thomas Gleixner
  2020-05-28 16:14   ` Peter Zijlstra
  2020-05-30  9:57   ` [tip: x86/entry] " tip-bot2 for Thomas Gleixner
  2020-05-28 14:53 ` [patch 3/5] x86/idt: Use proper constants for table sizes Thomas Gleixner
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Thomas Gleixner @ 2020-05-28 14:53 UTC (permalink / raw)
  To: LKML; +Cc: x86

The difference between 32 and 64 bit vs. early #PF handling is not
documented. Replace the FIXME at idt_setup_early_pf() with proper comments.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/idt.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -61,7 +61,11 @@ static bool idt_setup_done __initdata;
 static const __initconst struct idt_data early_idts[] = {
 	INTG(X86_TRAP_DB,		asm_exc_debug),
 	SYSG(X86_TRAP_BP,		asm_exc_int3),
+
 #ifdef CONFIG_X86_32
+	/*
+	 * Not possible on 64-bit. See idt_setup_early_pf() for details.
+	 */
 	INTG(X86_TRAP_PF,		asm_exc_page_fault),
 #endif
 };
@@ -276,8 +280,10 @@ void __init idt_setup_traps(void)
  * 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?
+ * Note, that X86_64 cannot install the real #PF handler in
+ * idt_setup_early_traps() because the memory intialization needs the #PF
+ * handler from the early_idt_handler_array to initialize the early page
+ * tables.
  */
 void __init idt_setup_early_pf(void)
 {


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [patch 3/5] x86/idt: Use proper constants for table sizes
  2020-05-28 14:53 [patch 0/5] x86/idt: Cleanups and consolidation Thomas Gleixner
  2020-05-28 14:53 ` [patch 1/5] x86/idt: Mark init only functions __init Thomas Gleixner
  2020-05-28 14:53 ` [patch 2/5] x86/idt: Add comments about early #PF handling Thomas Gleixner
@ 2020-05-28 14:53 ` Thomas Gleixner
  2020-05-30  9:57   ` [tip: x86/entry] x86/idt: Use proper constants for table size tip-bot2 for Thomas Gleixner
  2020-05-28 14:53 ` [patch 4/5] x86/idt: Cleanup trap_init() Thomas Gleixner
  2020-05-28 14:53 ` [patch 5/5] x86/idt: Consolidate idt functionality Thomas Gleixner
  4 siblings, 1 reply; 16+ messages in thread
From: Thomas Gleixner @ 2020-05-28 14:53 UTC (permalink / raw)
  To: LKML; +Cc: x86

Use the actual struct size to calculate the IDT table sizes instead of two
different hardcoded values.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/idt.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -51,6 +51,8 @@ struct idt_data {
 #define TSKG(_vector, _gdt)				\
 	G(_vector, NULL, DEFAULT_STACK, GATE_TASK, DPL0, _gdt << 3)
 
+#define IDT_TABLE_SIZE		(IDT_ENTRIES * sizeof(gate_desc))
+#define IDT_DEBUG_TABLE_SIZE	(16 * sizeof(gate_desc))
 
 static bool idt_setup_done __initdata;
 
@@ -176,7 +178,7 @@ static const __initconst struct idt_data
 gate_desc idt_table[IDT_ENTRIES] __page_aligned_bss;
 
 struct desc_ptr idt_descr __ro_after_init = {
-	.size		= (IDT_ENTRIES * 2 * sizeof(unsigned long)) - 1,
+	.size		= IDT_TABLE_SIZE - 1,
 	.address	= (unsigned long) idt_table,
 };
 
@@ -202,7 +204,7 @@ static const __initconst struct idt_data
  * stack set to DEFAULT_STACK (0). Required for NMI trap handling.
  */
 const struct desc_ptr debug_idt_descr = {
-	.size		= IDT_ENTRIES * 16 - 1,
+	.size		= IDT_DEBUG_TABLE_SIZE - 1,
 	.address	= (unsigned long) debug_idt_table,
 };
 #endif
@@ -304,9 +306,10 @@ void __init idt_setup_ist_traps(void)
  */
 void __init idt_setup_debugidt_traps(void)
 {
-	memcpy(&debug_idt_table, &idt_table, IDT_ENTRIES * 16);
+	memcpy(&debug_idt_table, &idt_table, IDT_DEBUG_TABLE_SIZE);
 
-	idt_setup_from_table(debug_idt_table, dbg_idts, ARRAY_SIZE(dbg_idts), false);
+	idt_setup_from_table(debug_idt_table, dbg_idts, ARRAY_SIZE(dbg_idts),
+			     false);
 }
 #endif
 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [patch 4/5] x86/idt: Cleanup trap_init()
  2020-05-28 14:53 [patch 0/5] x86/idt: Cleanups and consolidation Thomas Gleixner
                   ` (2 preceding siblings ...)
  2020-05-28 14:53 ` [patch 3/5] x86/idt: Use proper constants for table sizes Thomas Gleixner
@ 2020-05-28 14:53 ` Thomas Gleixner
  2020-05-30  9:57   ` [tip: x86/entry] " tip-bot2 for Thomas Gleixner
  2020-05-28 14:53 ` [patch 5/5] x86/idt: Consolidate idt functionality Thomas Gleixner
  4 siblings, 1 reply; 16+ messages in thread
From: Thomas Gleixner @ 2020-05-28 14:53 UTC (permalink / raw)
  To: LKML; +Cc: x86

No point in having all the IDT cruft in trap_init(). Move it into the IDT
code, get rid of the extra global function and fixup the comments.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/desc.h |    2 --
 arch/x86/kernel/idt.c       |   26 +++++++++++++++++++-------
 arch/x86/kernel/traps.c     |   11 -----------
 3 files changed, 19 insertions(+), 20 deletions(-)

--- a/arch/x86/include/asm/desc.h
+++ b/arch/x86/include/asm/desc.h
@@ -438,11 +438,9 @@ extern void idt_setup_apic_and_irq_gates
 #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
 
 extern void idt_invalidate(void *addr);
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -4,6 +4,7 @@
  */
 #include <linux/interrupt.h>
 
+#include <asm/cpu_entry_area.h>
 #include <asm/traps.h>
 #include <asm/proto.h>
 #include <asm/desc.h>
@@ -299,20 +300,27 @@ void __init idt_setup_early_pf(void)
 void __init idt_setup_ist_traps(void)
 {
 	idt_setup_from_table(idt_table, ist_idts, ARRAY_SIZE(ist_idts), true);
-}
 
-/**
- * idt_setup_debugidt_traps - Initialize the debug idt table with debug traps
- */
-void __init idt_setup_debugidt_traps(void)
-{
+	/* All traps set up: Initialize the debug IDT table. */
 	memcpy(&debug_idt_table, &idt_table, IDT_DEBUG_TABLE_SIZE);
-
 	idt_setup_from_table(debug_idt_table, dbg_idts, ARRAY_SIZE(dbg_idts),
 			     false);
 }
 #endif
 
+static void idt_map_in_cea(void)
+{
+	/*
+	 * Set the IDT descriptor to a fixed read-only location in the cpu
+	 * entry area, so that the "sidt" instruction will not leak the
+	 * location of the kernel, and to defend the IDT against arbitrary
+	 * memory write vulnerabilities.
+	 */
+	cea_set_pte(CPU_ENTRY_AREA_RO_IDT_VADDR, __pa_symbol(idt_table),
+		    PAGE_KERNEL_RO);
+	idt_descr.address = CPU_ENTRY_AREA_RO_IDT;
+}
+
 /**
  * idt_setup_apic_and_irq_gates - Setup APIC/SMP and normal interrupt gates
  */
@@ -339,6 +347,10 @@ void __init idt_setup_apic_and_irq_gates
 		set_intr_gate(i, entry);
 	}
 #endif
+	/* Map IDT into CPU entry area and reload it. */
+	idt_map_in_cea();
+	load_current_idt();
+
 	idt_setup_done = true;
 }
 
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -1077,20 +1077,9 @@ void __init trap_init(void)
 	idt_setup_traps();
 
 	/*
-	 * Set the IDT descriptor to a fixed read-only location, so that the
-	 * "sidt" instruction will not leak the location of the kernel, and
-	 * to defend the IDT against arbitrary memory write vulnerabilities.
-	 * It will be reloaded in cpu_init() */
-	cea_set_pte(CPU_ENTRY_AREA_RO_IDT_VADDR, __pa_symbol(idt_table),
-		    PAGE_KERNEL_RO);
-	idt_descr.address = CPU_ENTRY_AREA_RO_IDT;
-
-	/*
 	 * Should be a barrier for any external CPU state:
 	 */
 	cpu_init();
 
 	idt_setup_ist_traps();
-
-	idt_setup_debugidt_traps();
 }


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [patch 5/5] x86/idt: Consolidate idt functionality
  2020-05-28 14:53 [patch 0/5] x86/idt: Cleanups and consolidation Thomas Gleixner
                   ` (3 preceding siblings ...)
  2020-05-28 14:53 ` [patch 4/5] x86/idt: Cleanup trap_init() Thomas Gleixner
@ 2020-05-28 14:53 ` Thomas Gleixner
  2020-05-28 16:16   ` Peter Zijlstra
                     ` (3 more replies)
  4 siblings, 4 replies; 16+ messages in thread
From: Thomas Gleixner @ 2020-05-28 14:53 UTC (permalink / raw)
  To: LKML; +Cc: x86

 - Move load_current_idt() out of line and replace the hideous comment
   with a lockdep assert.

 - Move debug IDT cruft into the IDT code and simplify it.  This makes
   debug_idt_ctr static and removes one function call.

 - Move the idt descriptor table definition to spare the extra #ifdef
   sections and make it static.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/desc.h  |   45 -------------------------
 arch/x86/kernel/cpu/common.c |   17 ---------
 arch/x86/kernel/idt.c        |   75 ++++++++++++++++++++++++++++++-------------
 3 files changed, 55 insertions(+), 82 deletions(-)

--- a/arch/x86/include/asm/desc.h
+++ b/arch/x86/include/asm/desc.h
@@ -40,11 +40,6 @@ static inline void fill_ldt(struct desc_
 	desc->l			= 0;
 }
 
-extern struct desc_ptr idt_descr;
-extern gate_desc idt_table[];
-extern const struct desc_ptr debug_idt_descr;
-extern gate_desc debug_idt_table[];
-
 struct gdt_page {
 	struct desc_struct gdt[GDT_ENTRIES];
 } __attribute__((aligned(PAGE_SIZE)));
@@ -390,45 +385,7 @@ void alloc_intr_gate(unsigned int n, con
 
 extern unsigned long system_vectors[];
 
-#ifdef CONFIG_X86_64
-DECLARE_PER_CPU(u32, debug_idt_ctr);
-static __always_inline bool is_debug_idt_enabled(void)
-{
-	if (this_cpu_read(debug_idt_ctr))
-		return true;
-
-	return false;
-}
-
-static __always_inline void load_debug_idt(void)
-{
-	load_idt((const struct desc_ptr *)&debug_idt_descr);
-}
-#else
-static inline bool is_debug_idt_enabled(void)
-{
-	return false;
-}
-
-static inline void load_debug_idt(void)
-{
-}
-#endif
-
-/*
- * The load_current_idt() must be called with interrupts disabled
- * to avoid races. That way the IDT will always be set back to the expected
- * descriptor. It's also called when a CPU is being initialized, and
- * that doesn't need to disable interrupts, as nothing should be
- * bothering the CPU then.
- */
-static __always_inline void load_current_idt(void)
-{
-	if (is_debug_idt_enabled())
-		load_debug_idt();
-	else
-		load_idt((const struct desc_ptr *)&idt_descr);
-}
+void load_current_idt(void);
 
 extern void idt_setup_early_handler(void);
 extern void idt_setup_early_traps(void);
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1689,23 +1689,6 @@ void syscall_init(void)
 	       X86_EFLAGS_IOPL|X86_EFLAGS_AC|X86_EFLAGS_NT);
 }
 
-DEFINE_PER_CPU(int, debug_stack_usage);
-DEFINE_PER_CPU(u32, debug_idt_ctr);
-
-noinstr void debug_stack_set_zero(void)
-{
-	this_cpu_inc(debug_idt_ctr);
-	load_current_idt();
-}
-
-noinstr void debug_stack_reset(void)
-{
-	if (WARN_ON(!this_cpu_read(debug_idt_ctr)))
-		return;
-	if (this_cpu_dec_return(debug_idt_ctr) == 0)
-		load_current_idt();
-}
-
 #else	/* CONFIG_X86_64 */
 
 DEFINE_PER_CPU(struct task_struct *, current_task) = &init_task;
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -157,6 +157,14 @@ static const __initconst struct idt_data
 #endif
 };
 
+/* Must be page-aligned because the real IDT is used in the cpu entry area */
+static gate_desc idt_table[IDT_ENTRIES] __page_aligned_bss;
+
+static struct desc_ptr idt_descr __ro_after_init = {
+	.size		= IDT_TABLE_SIZE - 1,
+	.address	= (unsigned long) idt_table,
+};
+
 #ifdef CONFIG_X86_64
 /*
  * Early traps running on the DEFAULT_STACK because the other interrupt
@@ -173,20 +181,19 @@ static const __initconst struct idt_data
 static const __initconst struct idt_data dbg_idts[] = {
 	INTG(X86_TRAP_DB,		asm_exc_debug),
 };
-#endif
 
-/* Must be page-aligned because the real IDT is used in a fixmap. */
-gate_desc idt_table[IDT_ENTRIES] __page_aligned_bss;
+/* No need to be aligned, but done to keep all IDTs defined the same way. */
+static gate_desc debug_idt_table[IDT_ENTRIES] __page_aligned_bss;
 
-struct desc_ptr idt_descr __ro_after_init = {
-	.size		= IDT_TABLE_SIZE - 1,
-	.address	= (unsigned long) idt_table,
+/*
+ * 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 struct desc_ptr debug_idt_descr = {
+	.size		= IDT_DEBUG_TABLE_SIZE - 1,
+	.address	= (unsigned long) debug_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;
-
 /*
  * The exceptions which use Interrupt stacks. They are setup after
  * cpu_init() when the TSS has been initialized.
@@ -200,15 +207,41 @@ static const __initconst struct idt_data
 #endif
 };
 
-/*
- * 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_DEBUG_TABLE_SIZE - 1,
-	.address	= (unsigned long) debug_idt_table,
-};
-#endif
+DEFINE_PER_CPU(int, debug_stack_usage);
+static DEFINE_PER_CPU(u32, debug_idt_ctr);
+
+noinstr void load_current_idt(void)
+{
+	lockdep_assert_irqs_disabled();
+
+	if (this_cpu_read(debug_idt_ctr))
+		load_idt(&debug_idt_descr);
+	else
+		load_idt(&idt_descr);
+}
+
+noinstr void debug_stack_set_zero(void)
+{
+	this_cpu_inc(debug_idt_ctr);
+	load_idt(&debug_idt_descr);
+}
+
+noinstr void debug_stack_reset(void)
+{
+	if (WARN_ON(!this_cpu_read(debug_idt_ctr)))
+		return;
+	if (this_cpu_dec_return(debug_idt_ctr) == 0)
+		load_idt(&idt_descr);
+}
+
+#else /* X86_64 */
+
+noinstr void load_current_idt(void)
+{
+	load_idt(&idt_descr);
+}
+
+#endif /* !X86_64 */
 
 static inline void idt_init_desc(gate_desc *gate, const struct idt_data *d)
 {
@@ -264,7 +297,7 @@ void __init idt_setup_early_traps(void)
 {
 	idt_setup_from_table(idt_table, early_idts, ARRAY_SIZE(early_idts),
 			     true);
-	load_idt(&idt_descr);
+	load_current_idt();
 }
 
 /**
@@ -374,7 +407,7 @@ void __init idt_setup_early_handler(void
 	for ( ; i < NR_VECTORS; i++)
 		set_intr_gate(i, early_ignore_irq);
 #endif
-	load_idt(&idt_descr);
+	load_current_idt();
 }
 
 /**


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [patch 2/5] x86/idt: Add comments about early #PF handling
  2020-05-28 14:53 ` [patch 2/5] x86/idt: Add comments about early #PF handling Thomas Gleixner
@ 2020-05-28 16:14   ` Peter Zijlstra
  2020-05-28 18:44     ` Thomas Gleixner
  2020-05-30  9:57   ` [tip: x86/entry] " tip-bot2 for Thomas Gleixner
  1 sibling, 1 reply; 16+ messages in thread
From: Peter Zijlstra @ 2020-05-28 16:14 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: LKML, x86

On Thu, May 28, 2020 at 04:53:17PM +0200, Thomas Gleixner wrote:
> The difference between 32 and 64 bit vs. early #PF handling is not
> documented. Replace the FIXME at idt_setup_early_pf() with proper comments.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/kernel/idt.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> --- a/arch/x86/kernel/idt.c
> +++ b/arch/x86/kernel/idt.c
> @@ -61,7 +61,11 @@ static bool idt_setup_done __initdata;
>  static const __initconst struct idt_data early_idts[] = {
>  	INTG(X86_TRAP_DB,		asm_exc_debug),
>  	SYSG(X86_TRAP_BP,		asm_exc_int3),
> +
>  #ifdef CONFIG_X86_32
> +	/*
> +	 * Not possible on 64-bit. See idt_setup_early_pf() for details.
> +	 */
>  	INTG(X86_TRAP_PF,		asm_exc_page_fault),
>  #endif
>  };
> @@ -276,8 +280,10 @@ void __init idt_setup_traps(void)
>   * 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?
> + * Note, that X86_64 cannot install the real #PF handler in
> + * idt_setup_early_traps() because the memory intialization needs the #PF
> + * handler from the early_idt_handler_array to initialize the early page
> + * tables.

+ "See early_make_pgtables()" ?

I had to frob around in head_64.S to find wth that early handler
actually did.

>   */
>  void __init idt_setup_early_pf(void)
>  {
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [patch 5/5] x86/idt: Consolidate idt functionality
  2020-05-28 14:53 ` [patch 5/5] x86/idt: Consolidate idt functionality Thomas Gleixner
@ 2020-05-28 16:16   ` Peter Zijlstra
  2020-05-29  2:58   ` kbuild test robot
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: Peter Zijlstra @ 2020-05-28 16:16 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: LKML, x86

On Thu, May 28, 2020 at 04:53:20PM +0200, Thomas Gleixner wrote:

> -#ifdef CONFIG_X86_64
> -DECLARE_PER_CPU(u32, debug_idt_ctr);
> -static __always_inline bool is_debug_idt_enabled(void)
> -{
> -	if (this_cpu_read(debug_idt_ctr))
> -		return true;
> -
> -	return false;
> -}
> -
> -static __always_inline void load_debug_idt(void)
> -{
> -	load_idt((const struct desc_ptr *)&debug_idt_descr);
> -}
> -#else
> -static inline bool is_debug_idt_enabled(void)
> -{
> -	return false;
> -}
> -
> -static inline void load_debug_idt(void)
> -{
> -}
> -#endif

I have a patch that does this too; except it doesn't put it back. I like
that one better :-)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [patch 2/5] x86/idt: Add comments about early #PF handling
  2020-05-28 16:14   ` Peter Zijlstra
@ 2020-05-28 18:44     ` Thomas Gleixner
  0 siblings, 0 replies; 16+ messages in thread
From: Thomas Gleixner @ 2020-05-28 18:44 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: LKML, x86

Peter Zijlstra <peterz@infradead.org> writes:
> On Thu, May 28, 2020 at 04:53:17PM +0200, Thomas Gleixner wrote:
>> The difference between 32 and 64 bit vs. early #PF handling is not
>> documented. Replace the FIXME at idt_setup_early_pf() with proper comments.
>> + * Note, that X86_64 cannot install the real #PF handler in
>> + * idt_setup_early_traps() because the memory intialization needs the #PF
>> + * handler from the early_idt_handler_array to initialize the early page
>> + * tables.
>
> + "See early_make_pgtables()" ?
>
> I had to frob around in head_64.S to find wth that early handler
> actually did.

True.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [patch 5/5] x86/idt: Consolidate idt functionality
  2020-05-28 14:53 ` [patch 5/5] x86/idt: Consolidate idt functionality Thomas Gleixner
  2020-05-28 16:16   ` Peter Zijlstra
@ 2020-05-29  2:58   ` kbuild test robot
  2020-05-30  9:57   ` [tip: x86/entry] " tip-bot2 for Thomas Gleixner
  2020-06-01 12:55   ` tip-bot2 for Thomas Gleixner
  3 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2020-05-29  2:58 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3589 bytes --]

Hi Thomas,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/auto-latest]
[cannot apply to linus/master tip/x86/core linux/master v5.7-rc7 next-20200528]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Thomas-Gleixner/x86-idt-Cleanups-and-consolidation/20200528-230255
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 4e052965f46b6fd9641d79bf10208eac2631475f
config: i386-randconfig-m021-20200528 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

arch/x86/mm/fault.c: In function 'is_f00f_bug':
>> arch/x86/mm/fault.c:567:19: error: 'idt_descr' undeclared (first use in this function); did you mean 'ldt_desc'?
567 |   nr = (address - idt_descr.address) >> 3;
|                   ^~~~~~~~~
|                   ldt_desc
arch/x86/mm/fault.c:567:19: note: each undeclared identifier is reported only once for each function it appears in

vim +567 arch/x86/mm/fault.c

35f3266ffbee7f arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  557  
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  558  static int is_f00f_bug(struct pt_regs *regs, unsigned long address)
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  559  {
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  560  #ifdef CONFIG_X86_F00F_BUG
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  561  	unsigned long nr;
2d4a71676f4d89 arch/x86/mm/fault.c    Ingo Molnar     2009-02-20  562  
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  563  	/*
2d4a71676f4d89 arch/x86/mm/fault.c    Ingo Molnar     2009-02-20  564  	 * Pentium F0 0F C7 C8 bug workaround:
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  565  	 */
e2604b49e8a882 arch/x86/mm/fault.c    Borislav Petkov 2013-03-20  566  	if (boot_cpu_has_bug(X86_BUG_F00F)) {
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30 @567  		nr = (address - idt_descr.address) >> 3;
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  568  
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  569  		if (nr == 6) {
a99471a459d4c8 arch/x86/mm/fault.c    Thomas Gleixner 2020-02-25  570  			handle_invalid_op(regs);
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  571  			return 1;
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  572  		}
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  573  	}
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  574  #endif
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  575  	return 0;
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  576  }
29caf2f98cdb26 arch/x86/mm/fault_64.c Harvey Harrison 2008-01-30  577  

:::::: The code at line 567 was first introduced by commit
:::::: 29caf2f98cdb266dffb50dfd412f951e8d46f719 x86: add is_f00f_bug helper to fault_32|64.c

:::::: TO: Harvey Harrison <harvey.harrison@gmail.com>
:::::: CC: Ingo Molnar <mingo@elte.hu>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36079 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [tip: x86/entry] x86/idt: Consolidate idt functionality
  2020-05-28 14:53 ` [patch 5/5] x86/idt: Consolidate idt functionality Thomas Gleixner
  2020-05-28 16:16   ` Peter Zijlstra
  2020-05-29  2:58   ` kbuild test robot
@ 2020-05-30  9:57   ` tip-bot2 for Thomas Gleixner
  2020-06-01 12:55   ` tip-bot2 for Thomas Gleixner
  3 siblings, 0 replies; 16+ messages in thread
From: tip-bot2 for Thomas Gleixner @ 2020-05-30  9:57 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Thomas Gleixner, x86, LKML

The following commit has been merged into the x86/entry branch of tip:

Commit-ID:     5980d208e5ef28455e9e8b08f6250b443a2f0893
Gitweb:        https://git.kernel.org/tip/5980d208e5ef28455e9e8b08f6250b443a2f0893
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Thu, 28 May 2020 16:53:20 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Sat, 30 May 2020 11:50:13 +02:00

x86/idt: Consolidate idt functionality

 - Move load_current_idt() out of line and replace the hideous comment with
   a lockdep assert. This allows to make idt_table and idt_descr static.

 - Mark idt_table read only after the IDT initialization is complete.

 - Shuffle code around to consolidate the #ifdef sections into one.

 - Adapt the F00F bug code.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20200528145523.084915381@linutronix.de

---
 arch/x86/include/asm/desc.h | 17 +---------
 arch/x86/kernel/idt.c       | 63 +++++++++++++++++++++---------------
 arch/x86/mm/fault.c         | 16 ++-------
 3 files changed, 44 insertions(+), 52 deletions(-)

diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
index 07632f3..1ced11d 100644
--- a/arch/x86/include/asm/desc.h
+++ b/arch/x86/include/asm/desc.h
@@ -40,9 +40,6 @@ static inline void fill_ldt(struct desc_struct *desc, const struct user_desc *in
 	desc->l			= 0;
 }
 
-extern struct desc_ptr idt_descr;
-extern gate_desc idt_table[];
-
 struct gdt_page {
 	struct desc_struct gdt[GDT_ENTRIES];
 } __attribute__((aligned(PAGE_SIZE)));
@@ -388,22 +385,12 @@ void alloc_intr_gate(unsigned int n, const void *addr);
 
 extern unsigned long system_vectors[];
 
-/*
- * The load_current_idt() must be called with interrupts disabled
- * to avoid races. That way the IDT will always be set back to the expected
- * descriptor. It's also called when a CPU is being initialized, and
- * that doesn't need to disable interrupts, as nothing should be
- * bothering the CPU then.
- */
-static __always_inline void load_current_idt(void)
-{
-	load_idt((const struct desc_ptr *)&idt_descr);
-}
-
+extern 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);
+extern bool idt_is_f00f_address(unsigned long address);
 
 #ifdef CONFIG_X86_64
 extern void idt_setup_early_pf(void);
diff --git a/arch/x86/kernel/idt.c b/arch/x86/kernel/idt.c
index 902cdd0..85e3e22 100644
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -5,6 +5,7 @@
 #include <linux/interrupt.h>
 
 #include <asm/cpu_entry_area.h>
+#include <asm/set_memory.h>
 #include <asm/traps.h>
 #include <asm/proto.h>
 #include <asm/desc.h>
@@ -156,37 +157,25 @@ static const __initconst struct idt_data apic_idts[] = {
 #endif
 };
 
-#ifdef CONFIG_X86_64
-/*
- * Early traps running on the DEFAULT_STACK because the other interrupt
- * stacks work only after cpu_init().
- */
-static const __initconst struct idt_data early_pf_idts[] = {
-	INTG(X86_TRAP_PF,		asm_exc_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;
+/* Must be page-aligned because the real IDT is used in the cpu entry area */
+static gate_desc idt_table[IDT_ENTRIES] __page_aligned_bss;
 
 struct desc_ptr idt_descr __ro_after_init = {
 	.size		= IDT_TABLE_SIZE - 1,
 	.address	= (unsigned long) idt_table,
 };
 
-#ifdef CONFIG_X86_64
-/*
- * The exceptions which use Interrupt stacks. They are setup after
- * cpu_init() when the TSS has been initialized.
- */
-static const __initconst struct idt_data ist_idts[] = {
-	ISTG(X86_TRAP_DB,	asm_exc_debug,		IST_INDEX_DB),
-	ISTG(X86_TRAP_NMI,	asm_exc_nmi,		IST_INDEX_NMI),
-	ISTG(X86_TRAP_DF,	asm_exc_double_fault,	IST_INDEX_DF),
-#ifdef CONFIG_X86_MCE
-	ISTG(X86_TRAP_MC,	asm_exc_machine_check,	IST_INDEX_MCE),
-#endif
-};
+void load_current_idt(void)
+{
+	lockdep_assert_irqs_disabled();
+	load_idt(&idt_descr);
+}
+
+#ifdef CONFIG_X86_F00F_BUG
+bool idt_is_f00f_address(unsigned long address)
+{
+	return (address - idt_descr.address) >> 3) == 6;
+}
 #endif
 
 static inline void idt_init_desc(gate_desc *gate, const struct idt_data *d)
@@ -255,6 +244,27 @@ void __init idt_setup_traps(void)
 }
 
 #ifdef CONFIG_X86_64
+/*
+ * Early traps running on the DEFAULT_STACK because the other interrupt
+ * stacks work only after cpu_init().
+ */
+static const __initconst struct idt_data early_pf_idts[] = {
+	INTG(X86_TRAP_PF,		asm_exc_page_fault),
+};
+
+/*
+ * The exceptions which use Interrupt stacks. They are setup after
+ * cpu_init() when the TSS has been initialized.
+ */
+static const __initconst struct idt_data ist_idts[] = {
+	ISTG(X86_TRAP_DB,	asm_exc_debug,		IST_INDEX_DB),
+	ISTG(X86_TRAP_NMI,	asm_exc_nmi,		IST_INDEX_NMI),
+	ISTG(X86_TRAP_DF,	asm_exc_double_fault,	IST_INDEX_DF),
+#ifdef CONFIG_X86_MCE
+	ISTG(X86_TRAP_MC,	asm_exc_machine_check,	IST_INDEX_MCE),
+#endif
+};
+
 /**
  * idt_setup_early_pf - Initialize the idt table with early pagefault handler
  *
@@ -325,6 +335,9 @@ void __init idt_setup_apic_and_irq_gates(void)
 	idt_map_in_cea();
 	load_idt(&idt_descr);
 
+	/* Make the IDT table read only */
+	set_memory_ro((unsigned long)&idt_table, 1);
+
 	idt_setup_done = true;
 }
 
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 9c57fb8..e4625f4 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -555,21 +555,13 @@ static int is_errata100(struct pt_regs *regs, unsigned long address)
 	return 0;
 }
 
+/* Pentium F0 0F C7 C8 bug workaround: */
 static int is_f00f_bug(struct pt_regs *regs, unsigned long address)
 {
 #ifdef CONFIG_X86_F00F_BUG
-	unsigned long nr;
-
-	/*
-	 * Pentium F0 0F C7 C8 bug workaround:
-	 */
-	if (boot_cpu_has_bug(X86_BUG_F00F)) {
-		nr = (address - idt_descr.address) >> 3;
-
-		if (nr == 6) {
-			handle_invalid_op(regs);
-			return 1;
-		}
+	if (boot_cpu_has_bug(X86_BUG_F00F) && idt_is_f00f_address(address)) {
+		handle_invalid_op(regs);
+		return 1;
 	}
 #endif
 	return 0;

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [tip: x86/entry] x86/idt: Cleanup trap_init()
  2020-05-28 14:53 ` [patch 4/5] x86/idt: Cleanup trap_init() Thomas Gleixner
@ 2020-05-30  9:57   ` tip-bot2 for Thomas Gleixner
  0 siblings, 0 replies; 16+ messages in thread
From: tip-bot2 for Thomas Gleixner @ 2020-05-30  9:57 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Thomas Gleixner, x86, LKML

The following commit has been merged into the x86/entry branch of tip:

Commit-ID:     88dbb6cfb9be0eaaa95c15a4b6d7f49044a2e1b7
Gitweb:        https://git.kernel.org/tip/88dbb6cfb9be0eaaa95c15a4b6d7f49044a2e1b7
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Thu, 28 May 2020 16:53:19 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Sat, 30 May 2020 11:50:12 +02:00

x86/idt: Cleanup trap_init()

No point in having all the IDT cruft in trap_init(). Move it into the IDT
code and fixup the comments.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20200528145522.992376498@linutronix.de

---
 arch/x86/kernel/idt.c   | 18 ++++++++++++++++++
 arch/x86/kernel/traps.c |  9 ---------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/idt.c b/arch/x86/kernel/idt.c
index b6e1a87..902cdd0 100644
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -4,6 +4,7 @@
  */
 #include <linux/interrupt.h>
 
+#include <asm/cpu_entry_area.h>
 #include <asm/traps.h>
 #include <asm/proto.h>
 #include <asm/desc.h>
@@ -281,6 +282,19 @@ void __init idt_setup_ist_traps(void)
 }
 #endif
 
+static void __init idt_map_in_cea(void)
+{
+	/*
+	 * Set the IDT descriptor to a fixed read-only location in the cpu
+	 * entry area, so that the "sidt" instruction will not leak the
+	 * location of the kernel, and to defend the IDT against arbitrary
+	 * memory write vulnerabilities.
+	 */
+	cea_set_pte(CPU_ENTRY_AREA_RO_IDT_VADDR, __pa_symbol(idt_table),
+		    PAGE_KERNEL_RO);
+	idt_descr.address = CPU_ENTRY_AREA_RO_IDT;
+}
+
 /**
  * idt_setup_apic_and_irq_gates - Setup APIC/SMP and normal interrupt gates
  */
@@ -307,6 +321,10 @@ void __init idt_setup_apic_and_irq_gates(void)
 		set_intr_gate(i, entry);
 	}
 #endif
+	/* Map IDT into CPU entry area and reload it. */
+	idt_map_in_cea();
+	load_idt(&idt_descr);
+
 	idt_setup_done = true;
 }
 
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 79af913..5566fe5 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -1056,15 +1056,6 @@ void __init trap_init(void)
 	idt_setup_traps();
 
 	/*
-	 * Set the IDT descriptor to a fixed read-only location, so that the
-	 * "sidt" instruction will not leak the location of the kernel, and
-	 * to defend the IDT against arbitrary memory write vulnerabilities.
-	 * It will be reloaded in cpu_init() */
-	cea_set_pte(CPU_ENTRY_AREA_RO_IDT_VADDR, __pa_symbol(idt_table),
-		    PAGE_KERNEL_RO);
-	idt_descr.address = CPU_ENTRY_AREA_RO_IDT;
-
-	/*
 	 * Should be a barrier for any external CPU state:
 	 */
 	cpu_init();

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [tip: x86/entry] x86/idt: Add comments about early #PF handling
  2020-05-28 14:53 ` [patch 2/5] x86/idt: Add comments about early #PF handling Thomas Gleixner
  2020-05-28 16:14   ` Peter Zijlstra
@ 2020-05-30  9:57   ` tip-bot2 for Thomas Gleixner
  1 sibling, 0 replies; 16+ messages in thread
From: tip-bot2 for Thomas Gleixner @ 2020-05-30  9:57 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Thomas Gleixner, x86, LKML

The following commit has been merged into the x86/entry branch of tip:

Commit-ID:     66d2e706c0cecd09b1f3de4844574d30e5469c28
Gitweb:        https://git.kernel.org/tip/66d2e706c0cecd09b1f3de4844574d30e5469c28
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Thu, 28 May 2020 16:53:17 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Sat, 30 May 2020 11:50:11 +02:00

x86/idt: Add comments about early #PF handling

The difference between 32 and 64 bit vs. early #PF handling is not
documented. Replace the FIXME at idt_setup_early_pf() with proper comments.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20200528145522.807135882@linutronix.de

---
 arch/x86/kernel/idt.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/idt.c b/arch/x86/kernel/idt.c
index 4b99f7b..5ef82fc 100644
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -61,7 +61,11 @@ static bool idt_setup_done __initdata;
 static const __initconst struct idt_data early_idts[] = {
 	INTG(X86_TRAP_DB,		asm_exc_debug),
 	SYSG(X86_TRAP_BP,		asm_exc_int3),
+
 #ifdef CONFIG_X86_32
+	/*
+	 * Not possible on 64-bit. See idt_setup_early_pf() for details.
+	 */
 	INTG(X86_TRAP_PF,		asm_exc_page_fault),
 #endif
 };
@@ -256,8 +260,10 @@ void __init idt_setup_traps(void)
  * 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?
+ * Note, that X86_64 cannot install the real #PF handler in
+ * idt_setup_early_traps() because the memory intialization needs the #PF
+ * handler from the early_idt_handler_array to initialize the early page
+ * tables.
  */
 void __init idt_setup_early_pf(void)
 {

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [tip: x86/entry] x86/idt: Use proper constants for table size
  2020-05-28 14:53 ` [patch 3/5] x86/idt: Use proper constants for table sizes Thomas Gleixner
@ 2020-05-30  9:57   ` tip-bot2 for Thomas Gleixner
  0 siblings, 0 replies; 16+ messages in thread
From: tip-bot2 for Thomas Gleixner @ 2020-05-30  9:57 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Thomas Gleixner, x86, LKML

The following commit has been merged into the x86/entry branch of tip:

Commit-ID:     ab46346736ed50ed90f4bfb854f4bec61fecee9e
Gitweb:        https://git.kernel.org/tip/ab46346736ed50ed90f4bfb854f4bec61fecee9e
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Thu, 28 May 2020 16:53:18 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Sat, 30 May 2020 11:50:12 +02:00

x86/idt: Use proper constants for table size

Use the actual struct size to calculate the IDT table size instead of
hardcoded values.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20200528145522.898591501@linutronix.de

---
 arch/x86/kernel/idt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/idt.c b/arch/x86/kernel/idt.c
index 5ef82fc..b6e1a87 100644
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -51,6 +51,7 @@ struct idt_data {
 #define TSKG(_vector, _gdt)				\
 	G(_vector, NULL, DEFAULT_STACK, GATE_TASK, DPL0, _gdt << 3)
 
+#define IDT_TABLE_SIZE		(IDT_ENTRIES * sizeof(gate_desc))
 
 static bool idt_setup_done __initdata;
 
@@ -168,7 +169,7 @@ static const __initconst struct idt_data early_pf_idts[] = {
 gate_desc idt_table[IDT_ENTRIES] __page_aligned_bss;
 
 struct desc_ptr idt_descr __ro_after_init = {
-	.size		= (IDT_ENTRIES * 2 * sizeof(unsigned long)) - 1,
+	.size		= IDT_TABLE_SIZE - 1,
 	.address	= (unsigned long) idt_table,
 };
 

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [tip: x86/entry] x86/idt: Mark init only functions __init
  2020-05-28 14:53 ` [patch 1/5] x86/idt: Mark init only functions __init Thomas Gleixner
@ 2020-05-30  9:57   ` tip-bot2 for Thomas Gleixner
  0 siblings, 0 replies; 16+ messages in thread
From: tip-bot2 for Thomas Gleixner @ 2020-05-30  9:57 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Thomas Gleixner, x86, LKML

The following commit has been merged into the x86/entry branch of tip:

Commit-ID:     f841aea13b3ba6d7ad19b1d0744593e2d2448d2f
Gitweb:        https://git.kernel.org/tip/f841aea13b3ba6d7ad19b1d0744593e2d2448d2f
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Thu, 28 May 2020 16:53:16 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Sat, 30 May 2020 11:50:11 +02:00

x86/idt: Mark init only functions __init

Since 8175cfbbbfcb ("x86/idt: Remove update_intr_gate()") set_intr_gate()
and idt_setup_from_table() are only called from __init functions. Mark them
as well.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20200528145522.715816477@linutronix.de

---
 arch/x86/kernel/idt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/idt.c b/arch/x86/kernel/idt.c
index 226c992..4b99f7b 100644
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -197,7 +197,7 @@ static inline void idt_init_desc(gate_desc *gate, const struct idt_data *d)
 #endif
 }
 
-static void
+static __init void
 idt_setup_from_table(gate_desc *idt, const struct idt_data *t, int size, bool sys)
 {
 	gate_desc desc;
@@ -210,7 +210,7 @@ idt_setup_from_table(gate_desc *idt, const struct idt_data *t, int size, bool sy
 	}
 }
 
-static void set_intr_gate(unsigned int n, const void *addr)
+static __init void set_intr_gate(unsigned int n, const void *addr)
 {
 	struct idt_data data;
 

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [tip: x86/entry] x86/idt: Consolidate idt functionality
  2020-05-28 14:53 ` [patch 5/5] x86/idt: Consolidate idt functionality Thomas Gleixner
                     ` (2 preceding siblings ...)
  2020-05-30  9:57   ` [tip: x86/entry] " tip-bot2 for Thomas Gleixner
@ 2020-06-01 12:55   ` tip-bot2 for Thomas Gleixner
  3 siblings, 0 replies; 16+ messages in thread
From: tip-bot2 for Thomas Gleixner @ 2020-06-01 12:55 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Thomas Gleixner, x86, LKML

The following commit has been merged into the x86/entry branch of tip:

Commit-ID:     5f98f9eee4d41e012f6a768cec1bd6a143572c09
Gitweb:        https://git.kernel.org/tip/5f98f9eee4d41e012f6a768cec1bd6a143572c09
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Thu, 28 May 2020 16:53:20 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 01 Jun 2020 14:40:14 +02:00

x86/idt: Consolidate idt functionality

 - Move load_current_idt() out of line and replace the hideous comment with
   a lockdep assert. This allows to make idt_table and idt_descr static.

 - Mark idt_table read only after the IDT initialization is complete.

 - Shuffle code around to consolidate the #ifdef sections into one.

 - Adapt the F00F bug code.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20200528145523.084915381@linutronix.de
---
 arch/x86/include/asm/desc.h | 17 +---------
 arch/x86/kernel/idt.c       | 63 +++++++++++++++++++++---------------
 arch/x86/mm/fault.c         | 16 ++-------
 3 files changed, 44 insertions(+), 52 deletions(-)

diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
index 07632f3..1ced11d 100644
--- a/arch/x86/include/asm/desc.h
+++ b/arch/x86/include/asm/desc.h
@@ -40,9 +40,6 @@ static inline void fill_ldt(struct desc_struct *desc, const struct user_desc *in
 	desc->l			= 0;
 }
 
-extern struct desc_ptr idt_descr;
-extern gate_desc idt_table[];
-
 struct gdt_page {
 	struct desc_struct gdt[GDT_ENTRIES];
 } __attribute__((aligned(PAGE_SIZE)));
@@ -388,22 +385,12 @@ void alloc_intr_gate(unsigned int n, const void *addr);
 
 extern unsigned long system_vectors[];
 
-/*
- * The load_current_idt() must be called with interrupts disabled
- * to avoid races. That way the IDT will always be set back to the expected
- * descriptor. It's also called when a CPU is being initialized, and
- * that doesn't need to disable interrupts, as nothing should be
- * bothering the CPU then.
- */
-static __always_inline void load_current_idt(void)
-{
-	load_idt((const struct desc_ptr *)&idt_descr);
-}
-
+extern 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);
+extern bool idt_is_f00f_address(unsigned long address);
 
 #ifdef CONFIG_X86_64
 extern void idt_setup_early_pf(void);
diff --git a/arch/x86/kernel/idt.c b/arch/x86/kernel/idt.c
index 902cdd0..0db2120 100644
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -5,6 +5,7 @@
 #include <linux/interrupt.h>
 
 #include <asm/cpu_entry_area.h>
+#include <asm/set_memory.h>
 #include <asm/traps.h>
 #include <asm/proto.h>
 #include <asm/desc.h>
@@ -156,37 +157,25 @@ static const __initconst struct idt_data apic_idts[] = {
 #endif
 };
 
-#ifdef CONFIG_X86_64
-/*
- * Early traps running on the DEFAULT_STACK because the other interrupt
- * stacks work only after cpu_init().
- */
-static const __initconst struct idt_data early_pf_idts[] = {
-	INTG(X86_TRAP_PF,		asm_exc_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;
+/* Must be page-aligned because the real IDT is used in the cpu entry area */
+static gate_desc idt_table[IDT_ENTRIES] __page_aligned_bss;
 
 struct desc_ptr idt_descr __ro_after_init = {
 	.size		= IDT_TABLE_SIZE - 1,
 	.address	= (unsigned long) idt_table,
 };
 
-#ifdef CONFIG_X86_64
-/*
- * The exceptions which use Interrupt stacks. They are setup after
- * cpu_init() when the TSS has been initialized.
- */
-static const __initconst struct idt_data ist_idts[] = {
-	ISTG(X86_TRAP_DB,	asm_exc_debug,		IST_INDEX_DB),
-	ISTG(X86_TRAP_NMI,	asm_exc_nmi,		IST_INDEX_NMI),
-	ISTG(X86_TRAP_DF,	asm_exc_double_fault,	IST_INDEX_DF),
-#ifdef CONFIG_X86_MCE
-	ISTG(X86_TRAP_MC,	asm_exc_machine_check,	IST_INDEX_MCE),
-#endif
-};
+void load_current_idt(void)
+{
+	lockdep_assert_irqs_disabled();
+	load_idt(&idt_descr);
+}
+
+#ifdef CONFIG_X86_F00F_BUG
+bool idt_is_f00f_address(unsigned long address)
+{
+	return ((address - idt_descr.address) >> 3) == 6;
+}
 #endif
 
 static inline void idt_init_desc(gate_desc *gate, const struct idt_data *d)
@@ -255,6 +244,27 @@ void __init idt_setup_traps(void)
 }
 
 #ifdef CONFIG_X86_64
+/*
+ * Early traps running on the DEFAULT_STACK because the other interrupt
+ * stacks work only after cpu_init().
+ */
+static const __initconst struct idt_data early_pf_idts[] = {
+	INTG(X86_TRAP_PF,		asm_exc_page_fault),
+};
+
+/*
+ * The exceptions which use Interrupt stacks. They are setup after
+ * cpu_init() when the TSS has been initialized.
+ */
+static const __initconst struct idt_data ist_idts[] = {
+	ISTG(X86_TRAP_DB,	asm_exc_debug,		IST_INDEX_DB),
+	ISTG(X86_TRAP_NMI,	asm_exc_nmi,		IST_INDEX_NMI),
+	ISTG(X86_TRAP_DF,	asm_exc_double_fault,	IST_INDEX_DF),
+#ifdef CONFIG_X86_MCE
+	ISTG(X86_TRAP_MC,	asm_exc_machine_check,	IST_INDEX_MCE),
+#endif
+};
+
 /**
  * idt_setup_early_pf - Initialize the idt table with early pagefault handler
  *
@@ -325,6 +335,9 @@ void __init idt_setup_apic_and_irq_gates(void)
 	idt_map_in_cea();
 	load_idt(&idt_descr);
 
+	/* Make the IDT table read only */
+	set_memory_ro((unsigned long)&idt_table, 1);
+
 	idt_setup_done = true;
 }
 
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 9c57fb8..e4625f4 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -555,21 +555,13 @@ static int is_errata100(struct pt_regs *regs, unsigned long address)
 	return 0;
 }
 
+/* Pentium F0 0F C7 C8 bug workaround: */
 static int is_f00f_bug(struct pt_regs *regs, unsigned long address)
 {
 #ifdef CONFIG_X86_F00F_BUG
-	unsigned long nr;
-
-	/*
-	 * Pentium F0 0F C7 C8 bug workaround:
-	 */
-	if (boot_cpu_has_bug(X86_BUG_F00F)) {
-		nr = (address - idt_descr.address) >> 3;
-
-		if (nr == 6) {
-			handle_invalid_op(regs);
-			return 1;
-		}
+	if (boot_cpu_has_bug(X86_BUG_F00F) && idt_is_f00f_address(address)) {
+		handle_invalid_op(regs);
+		return 1;
 	}
 #endif
 	return 0;

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-06-01 12:55 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-28 14:53 [patch 0/5] x86/idt: Cleanups and consolidation Thomas Gleixner
2020-05-28 14:53 ` [patch 1/5] x86/idt: Mark init only functions __init Thomas Gleixner
2020-05-30  9:57   ` [tip: x86/entry] " tip-bot2 for Thomas Gleixner
2020-05-28 14:53 ` [patch 2/5] x86/idt: Add comments about early #PF handling Thomas Gleixner
2020-05-28 16:14   ` Peter Zijlstra
2020-05-28 18:44     ` Thomas Gleixner
2020-05-30  9:57   ` [tip: x86/entry] " tip-bot2 for Thomas Gleixner
2020-05-28 14:53 ` [patch 3/5] x86/idt: Use proper constants for table sizes Thomas Gleixner
2020-05-30  9:57   ` [tip: x86/entry] x86/idt: Use proper constants for table size tip-bot2 for Thomas Gleixner
2020-05-28 14:53 ` [patch 4/5] x86/idt: Cleanup trap_init() Thomas Gleixner
2020-05-30  9:57   ` [tip: x86/entry] " tip-bot2 for Thomas Gleixner
2020-05-28 14:53 ` [patch 5/5] x86/idt: Consolidate idt functionality Thomas Gleixner
2020-05-28 16:16   ` Peter Zijlstra
2020-05-29  2:58   ` kbuild test robot
2020-05-30  9:57   ` [tip: x86/entry] " tip-bot2 for Thomas Gleixner
2020-06-01 12:55   ` tip-bot2 for Thomas Gleixner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.