[patch 0/5] x86/idt: Cleanups and consolidation

[patch 0/5] x86/idt: Cleanups and consolidation

[Thomas Gleixner]

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(-)

[patch 1/5] x86/idt: Mark init only functions __init

[Thomas Gleixner]

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;
 

[tip: x86/entry] x86/idt: Mark init only functions __init

[tip-bot2 for Thomas Gleixner]

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;
 

[patch 2/5] x86/idt: Add comments about early #PF handling

[Thomas Gleixner]

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)
 {

Re: [patch 2/5] x86/idt: Add comments about early #PF handling

[Peter Zijlstra]

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)
>  {
> 

Re: [patch 2/5] x86/idt: Add comments about early #PF handling

[Thomas Gleixner]

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.

[tip: x86/entry] x86/idt: Add comments about early #PF handling

[tip-bot2 for Thomas Gleixner]

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)
 {

[patch 3/5] x86/idt: Use proper constants for table sizes

[Thomas Gleixner]

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
 

[tip: x86/entry] x86/idt: Use proper constants for table size

[tip-bot2 for Thomas Gleixner]

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,
 };
 

[patch 4/5] x86/idt: Cleanup trap_init()

[Thomas Gleixner]

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();
 }

[tip: x86/entry] x86/idt: Cleanup trap_init()

[tip-bot2 for Thomas Gleixner]

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();

[patch 5/5] x86/idt: Consolidate idt functionality

[Thomas Gleixner]

 - 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();
 }
 
 /**

Re: [patch 5/5] x86/idt: Consolidate idt functionality

[Peter Zijlstra]

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 :-)

[tip: x86/entry] x86/idt: Consolidate idt functionality

[tip-bot2 for Thomas Gleixner]

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;

[tip: x86/entry] x86/idt: Consolidate idt functionality

[tip-bot2 for Thomas Gleixner]

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;