* [PATCH v8 01/17] x86/ptrace: Prevent ptrace from clearing the FS/GS selector
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 02/17] selftests/x86/fsgsbase: Test GS selector on ptracer-induced GS base write Chang S. Bae
` (16 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Thomas Gleixner, Borislav Petkov,
Andy Lutomirski, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen
When a ptracer writes a ptracee's FS/GS base with a different value, the
selector is also cleared. This behavior is not correct as the selector
should be preserved.
Update only the base value and leave the selector intact. To simplify the
code further remove the conditional checking for the same value as this
code is not performance-critical.
The only recognizable downside of this change is when the selector is
already nonzero on write. The base will be reloaded according to the
selector. But the case is highly unexpected in real usages.
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
---
Changes from v7:
* Fixed to call correct helper functions
* Massaged changelog by Thomas
* Used '[FS|GS] base' consistently, instead of '[FS|GS]BASE'
---
arch/x86/kernel/ptrace.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 3c5bbe8..df222e2 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -370,22 +370,12 @@ static int putreg(struct task_struct *child,
case offsetof(struct user_regs_struct,fs_base):
if (value >= TASK_SIZE_MAX)
return -EIO;
- /*
- * When changing the FS base, use do_arch_prctl_64()
- * to set the index to zero and to set the base
- * as requested.
- */
- if (child->thread.fsbase != value)
- return do_arch_prctl_64(child, ARCH_SET_FS, value);
+ x86_fsbase_write_task(child, value);
return 0;
case offsetof(struct user_regs_struct,gs_base):
- /*
- * Exactly the same here as the %fs handling above.
- */
if (value >= TASK_SIZE_MAX)
return -EIO;
- if (child->thread.gsbase != value)
- return do_arch_prctl_64(child, ARCH_SET_GS, value);
+ x86_gsbase_write_task(child, value);
return 0;
#endif
}
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 02/17] selftests/x86/fsgsbase: Test GS selector on ptracer-induced GS base write
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 01/17] x86/ptrace: Prevent ptrace from clearing the FS/GS selector Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 03/17] x86/cpu: Add 'unsafe_fsgsbase' to enable CR4.FSGSBASE Chang S. Bae
` (15 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Thomas Gleixner, Borislav Petkov,
Andy Lutomirski, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen
The test validates that the selector is not changed when a ptracer writes
the ptracee's GS base.
Originally-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
---
Changes from v7:
* Trimmed down the changes as most codes from v7 were already merged
* Included Andy's additional comments and messages when testing old
kernels
* Used 'GS base' consistently, instead of 'GSBASE'
---
tools/testing/selftests/x86/fsgsbase.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/x86/fsgsbase.c b/tools/testing/selftests/x86/fsgsbase.c
index 15a329d..950a48b 100644
--- a/tools/testing/selftests/x86/fsgsbase.c
+++ b/tools/testing/selftests/x86/fsgsbase.c
@@ -465,7 +465,7 @@ static void test_ptrace_write_gsbase(void)
wait(&status);
if (WSTOPSIG(status) == SIGTRAP) {
- unsigned long gs, base;
+ unsigned long gs;
unsigned long gs_offset = USER_REGS_OFFSET(gs);
unsigned long base_offset = USER_REGS_OFFSET(gs_base);
@@ -481,7 +481,6 @@ static void test_ptrace_write_gsbase(void)
err(1, "PTRACE_POKEUSER");
gs = ptrace(PTRACE_PEEKUSER, child, gs_offset, NULL);
- base = ptrace(PTRACE_PEEKUSER, child, base_offset, NULL);
/*
* In a non-FSGSBASE system, the nonzero selector will load
@@ -489,11 +488,21 @@ static void test_ptrace_write_gsbase(void)
* selector value is changed or not by the GSBASE write in
* a ptracer.
*/
- if (gs == 0 && base == 0xFF) {
- printf("[OK]\tGS was reset as expected\n");
- } else {
+ if (gs != *shared_scratch) {
nerrs++;
- printf("[FAIL]\tGS=0x%lx, GSBASE=0x%lx (should be 0, 0xFF)\n", gs, base);
+ printf("[FAIL]\tGS changed to %lx\n", gs);
+
+ /*
+ * On older kernels, poking a nonzero value into the
+ * base would zero the selector. On newer kernels,
+ * this behavior has changed -- poking the base
+ * changes only the base and, if FSGSBASE is not
+ * available, this may not effect.
+ */
+ if (gs == 0)
+ printf("\tNote: this is expected behavior on older kernels.\n");
+ } else {
+ printf("[OK]\tGS remained 0x%hx\n", *shared_scratch);
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 03/17] x86/cpu: Add 'unsafe_fsgsbase' to enable CR4.FSGSBASE
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 01/17] x86/ptrace: Prevent ptrace from clearing the FS/GS selector Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 02/17] selftests/x86/fsgsbase: Test GS selector on ptracer-induced GS base write Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 04/17] x86/fsgsbase/64: Add intrinsics for FSGSBASE instructions Chang S. Bae
` (14 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Andy Lutomirski, Thomas Gleixner,
Borislav Petkov, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen
From: Andy Lutomirski <luto@kernel.org>
This is temporary. It will allow the next few patches to be tested
incrementally.
Setting unsafe_fsgsbase is a root hole. Don't do it.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
---
Changes from v7: none
---
Documentation/admin-guide/kernel-parameters.txt | 3 +++
arch/x86/kernel/cpu/common.c | 24 ++++++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1c67acd..2ff72af 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2875,6 +2875,9 @@
no5lvl [X86-64] Disable 5-level paging mode. Forces
kernel to use 4-level paging instead.
+ unsafe_fsgsbase [X86] Allow FSGSBASE instructions. This will be
+ replaced with a nofsgsbase flag.
+
no_console_suspend
[HW] Never suspend the console
Disable suspending of consoles during suspend and
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 9ae7d1b..9f57fb0 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -438,6 +438,22 @@ static void __init setup_cr_pinning(void)
}
/*
+ * Temporary hack: FSGSBASE is unsafe until a few kernel code paths are
+ * updated. This allows us to get the kernel ready incrementally.
+ *
+ * Once all the pieces are in place, these will go away and be replaced with
+ * a nofsgsbase chicken flag.
+ */
+static bool unsafe_fsgsbase;
+
+static __init int setup_unsafe_fsgsbase(char *arg)
+{
+ unsafe_fsgsbase = true;
+ return 1;
+}
+__setup("unsafe_fsgsbase", setup_unsafe_fsgsbase);
+
+/*
* Protection Keys are not available in 32-bit mode.
*/
static bool pku_disabled;
@@ -1455,6 +1471,14 @@ static void identify_cpu(struct cpuinfo_x86 *c)
setup_smap(c);
setup_umip(c);
+ /* Enable FSGSBASE instructions if available. */
+ if (cpu_has(c, X86_FEATURE_FSGSBASE)) {
+ if (unsafe_fsgsbase)
+ cr4_set_bits(X86_CR4_FSGSBASE);
+ else
+ clear_cpu_cap(c, X86_FEATURE_FSGSBASE);
+ }
+
/*
* The vendor-specific functions might have changed features.
* Now we do "generic changes."
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 04/17] x86/fsgsbase/64: Add intrinsics for FSGSBASE instructions
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (2 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 03/17] x86/cpu: Add 'unsafe_fsgsbase' to enable CR4.FSGSBASE Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 05/17] x86/fsgsbase/64: Enable FSGSBASE instructions in helper functions Chang S. Bae
` (13 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Andi Kleen, Andy Lutomirski,
Thomas Gleixner, Borislav Petkov, Dave Hansen, Tony Luck,
H . Peter Anvin
From: Andi Kleen <ak@linux.intel.com>
[ luto: Rename the variables from FS and GS to FSBASE and GSBASE and
make <asm/fsgsbase.h> safe to include on 32-bit kernels. ]
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Andy Lutomirski <luto@kernel.org>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Andi Kleen <ak@linux.intel.com>
---
Changes from v7:
* No code change
* Trimmed the changelog by Thomas
---
arch/x86/include/asm/fsgsbase.h | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/arch/x86/include/asm/fsgsbase.h b/arch/x86/include/asm/fsgsbase.h
index bca4c74..fdd1177 100644
--- a/arch/x86/include/asm/fsgsbase.h
+++ b/arch/x86/include/asm/fsgsbase.h
@@ -19,6 +19,36 @@ extern unsigned long x86_gsbase_read_task(struct task_struct *task);
extern void x86_fsbase_write_task(struct task_struct *task, unsigned long fsbase);
extern void x86_gsbase_write_task(struct task_struct *task, unsigned long gsbase);
+/* Must be protected by X86_FEATURE_FSGSBASE check. */
+
+static __always_inline unsigned long rdfsbase(void)
+{
+ unsigned long fsbase;
+
+ asm volatile("rdfsbase %0" : "=r" (fsbase) :: "memory");
+
+ return fsbase;
+}
+
+static __always_inline unsigned long rdgsbase(void)
+{
+ unsigned long gsbase;
+
+ asm volatile("rdgsbase %0" : "=r" (gsbase) :: "memory");
+
+ return gsbase;
+}
+
+static __always_inline void wrfsbase(unsigned long fsbase)
+{
+ asm volatile("wrfsbase %0" :: "r" (fsbase) : "memory");
+}
+
+static __always_inline void wrgsbase(unsigned long gsbase)
+{
+ asm volatile("wrgsbase %0" :: "r" (gsbase) : "memory");
+}
+
/* Helper functions for reading/writing FS/GS base */
static inline unsigned long x86_fsbase_read_cpu(void)
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 05/17] x86/fsgsbase/64: Enable FSGSBASE instructions in helper functions
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (3 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 04/17] x86/fsgsbase/64: Add intrinsics for FSGSBASE instructions Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 06/17] x86/fsgsbase/64: Use FSGSBASE in switch_to() if available Chang S. Bae
` (12 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Thomas Gleixner, Borislav Petkov,
Andy Lutomirski, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen, Andrew Cooper
Add CPU feature conditional FS/GS base access to the relevant helper
functions. That allows accelerating certain FS/GS base operations in
subsequent changes.
Note, that while possible, the user space entry/exit GS base operations are
not going to use the new FSGSBASE instructions. The reason is that it would
require additional storage for the user space value which adds more
complexity to the low level code and experiments have shown marginal
benefit. This may be revisited later but for now the SWAPGS based handling
in the entry code is preserved except for the paranoid entry/exit code.
To preserve the SWAPGS entry mechanism introduce __[rd|wr]gsbase_inactive()
helpers. Note, for Xen PV, paravirt hooks can be added later as they might
allow a very efficient but different implementation.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes from v7:
* Added interrupt-related warning messages by Thomas
* Massaged changelog by Thomas
* Used '[FS|GS] base' consistently, instead of '[FS|GS]BASE'
---
arch/x86/include/asm/fsgsbase.h | 27 ++++++++---------
arch/x86/kernel/process_64.c | 66 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 15 deletions(-)
diff --git a/arch/x86/include/asm/fsgsbase.h b/arch/x86/include/asm/fsgsbase.h
index fdd1177..aefd537 100644
--- a/arch/x86/include/asm/fsgsbase.h
+++ b/arch/x86/include/asm/fsgsbase.h
@@ -49,35 +49,32 @@ static __always_inline void wrgsbase(unsigned long gsbase)
asm volatile("wrgsbase %0" :: "r" (gsbase) : "memory");
}
+#include <asm/cpufeature.h>
+
/* Helper functions for reading/writing FS/GS base */
static inline unsigned long x86_fsbase_read_cpu(void)
{
unsigned long fsbase;
- rdmsrl(MSR_FS_BASE, fsbase);
+ if (static_cpu_has(X86_FEATURE_FSGSBASE))
+ fsbase = rdfsbase();
+ else
+ rdmsrl(MSR_FS_BASE, fsbase);
return fsbase;
}
-static inline unsigned long x86_gsbase_read_cpu_inactive(void)
-{
- unsigned long gsbase;
-
- rdmsrl(MSR_KERNEL_GS_BASE, gsbase);
-
- return gsbase;
-}
-
static inline void x86_fsbase_write_cpu(unsigned long fsbase)
{
- wrmsrl(MSR_FS_BASE, fsbase);
+ if (static_cpu_has(X86_FEATURE_FSGSBASE))
+ wrfsbase(fsbase);
+ else
+ wrmsrl(MSR_FS_BASE, fsbase);
}
-static inline void x86_gsbase_write_cpu_inactive(unsigned long gsbase)
-{
- wrmsrl(MSR_KERNEL_GS_BASE, gsbase);
-}
+extern unsigned long x86_gsbase_read_cpu_inactive(void);
+extern void x86_gsbase_write_cpu_inactive(unsigned long gsbase);
#endif /* CONFIG_X86_64 */
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index af64519..c8a67bf 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -152,6 +152,40 @@ enum which_selector {
};
/*
+ * Out of line to be protected from kprobes. It is not used on Xen
+ * paravirt. When paravirt support is needed, it needs to be renamed
+ * with native_ prefix.
+ */
+static noinline unsigned long __rdgsbase_inactive(void)
+{
+ unsigned long gsbase;
+
+ lockdep_assert_irqs_disabled();
+
+ native_swapgs();
+ gsbase = rdgsbase();
+ native_swapgs();
+
+ return gsbase;
+}
+NOKPROBE_SYMBOL(__rdgsbase_inactive);
+
+/*
+ * Out of line to be protected from kprobes. It is not used on Xen
+ * paravirt. When paravirt support is needed, it needs to be renamed
+ * with native_ prefix.
+ */
+static noinline void __wrgsbase_inactive(unsigned long gsbase)
+{
+ lockdep_assert_irqs_disabled();
+
+ native_swapgs();
+ wrgsbase(gsbase);
+ native_swapgs();
+}
+NOKPROBE_SYMBOL(__wrgsbase_inactive);
+
+/*
* Saves the FS or GS base for an outgoing thread if FSGSBASE extensions are
* not available. The goal is to be reasonably fast on non-FSGSBASE systems.
* It's forcibly inlined because it'll generate better code and this function
@@ -329,6 +363,38 @@ static unsigned long x86_fsgsbase_read_task(struct task_struct *task,
return base;
}
+unsigned long x86_gsbase_read_cpu_inactive(void)
+{
+ unsigned long gsbase;
+
+ if (static_cpu_has(X86_FEATURE_FSGSBASE)) {
+ unsigned long flags;
+
+ /* Interrupts are disabled here. */
+ local_irq_save(flags);
+ gsbase = __rdgsbase_inactive();
+ local_irq_restore(flags);
+ } else {
+ rdmsrl(MSR_KERNEL_GS_BASE, gsbase);
+ }
+
+ return gsbase;
+}
+
+void x86_gsbase_write_cpu_inactive(unsigned long gsbase)
+{
+ if (static_cpu_has(X86_FEATURE_FSGSBASE)) {
+ unsigned long flags;
+
+ /* Interrupts are disabled here. */
+ local_irq_save(flags);
+ __wrgsbase_inactive(gsbase);
+ local_irq_restore(flags);
+ } else {
+ wrmsrl(MSR_KERNEL_GS_BASE, gsbase);
+ }
+}
+
unsigned long x86_fsbase_read_task(struct task_struct *task)
{
unsigned long fsbase;
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 06/17] x86/fsgsbase/64: Use FSGSBASE in switch_to() if available
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (4 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 05/17] x86/fsgsbase/64: Enable FSGSBASE instructions in helper functions Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 07/17] x86/fsgsbase/64: Use FSGSBASE instructions on thread copy and ptrace Chang S. Bae
` (11 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Andy Lutomirski, Thomas Gleixner,
Borislav Petkov, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen
From: Andy Lutomirski <luto@kernel.org>
With the new FSGSBASE instructions, FS/GS base can be efficiently read
and written in __switch_to(). Use that capability to preserve the full
state.
This will enable user code to do whatever it wants with the new
instructions without any kernel-induced gotchas. (There can still be
architectural gotchas: movl %gs,%eax; movl %eax,%gs may change GS base
if WRGSBASE was used, but users are expected to read the CPU manual
before doing things like that.)
This is a considerable speedup. It seems to save about 100 cycles per
context switch compared to the baseline 4.6-rc1 behavior on a Skylake
laptop.
[ chang: 5~10% performance improvements were seen by a context switch
benchmark that ran threads with different FS/GS base values (to the
baseline 4.16). ]
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
---
Changes from v7:
* Used appropriate GS base read/write functions depending on interrupt
conditions. This fixes the bug in v7.
* Massaged changelog by Thomas
* Used '[FS|GS] base' consistently, instead of '[FS|GS]BASE'
---
arch/x86/kernel/process_64.c | 40 ++++++++++++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index c8a67bf..4c388817 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -234,8 +234,21 @@ static __always_inline void save_fsgs(struct task_struct *task)
{
savesegment(fs, task->thread.fsindex);
savesegment(gs, task->thread.gsindex);
- save_base_legacy(task, task->thread.fsindex, FS);
- save_base_legacy(task, task->thread.gsindex, GS);
+ if (static_cpu_has(X86_FEATURE_FSGSBASE)) {
+ /*
+ * If FSGSBASE is enabled, we can't make any useful guesses
+ * about the base, and user code expects us to save the current
+ * value. Fortunately, reading the base directly is efficient.
+ */
+ task->thread.fsbase = rdfsbase();
+ if (irqs_disabled())
+ task->thread.gsbase = __rdgsbase_inactive();
+ else
+ task->thread.gsbase = x86_gsbase_read_cpu_inactive();
+ } else {
+ save_base_legacy(task, task->thread.fsindex, FS);
+ save_base_legacy(task, task->thread.gsindex, GS);
+ }
}
#if IS_ENABLED(CONFIG_KVM)
@@ -314,10 +327,25 @@ static __always_inline void load_seg_legacy(unsigned short prev_index,
static __always_inline void x86_fsgsbase_load(struct thread_struct *prev,
struct thread_struct *next)
{
- load_seg_legacy(prev->fsindex, prev->fsbase,
- next->fsindex, next->fsbase, FS);
- load_seg_legacy(prev->gsindex, prev->gsbase,
- next->gsindex, next->gsbase, GS);
+ if (static_cpu_has(X86_FEATURE_FSGSBASE)) {
+ /* Update the FS and GS selectors if they could have changed. */
+ if (unlikely(prev->fsindex || next->fsindex))
+ loadseg(FS, next->fsindex);
+ if (unlikely(prev->gsindex || next->gsindex))
+ loadseg(GS, next->gsindex);
+
+ /* Update the bases. */
+ wrfsbase(next->fsbase);
+ if (irqs_disabled())
+ __wrgsbase_inactive(next->gsbase);
+ else
+ x86_gsbase_write_cpu_inactive(next->gsbase);
+ } else {
+ load_seg_legacy(prev->fsindex, prev->fsbase,
+ next->fsindex, next->fsbase, FS);
+ load_seg_legacy(prev->gsindex, prev->gsbase,
+ next->gsindex, next->gsbase, GS);
+ }
}
static unsigned long x86_fsgsbase_read_task(struct task_struct *task,
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 07/17] x86/fsgsbase/64: Use FSGSBASE instructions on thread copy and ptrace
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (5 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 06/17] x86/fsgsbase/64: Use FSGSBASE in switch_to() if available Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 08/17] x86/entry/64: Clean up paranoid exit Chang S. Bae
` (10 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Thomas Gleixner, Borislav Petkov,
Andy Lutomirski, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen
When FSGSBASE is enabled, copying threads and reading FS/GS base using
ptrace must read the actual values.
When copying a thread, use fsgs_save() and copy the saved values. For
ptrace, the bases must be read from memory regardless of the selector
if FSGSBASE is enabled.
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
---
Changes from v7:
* No code change
* Massaged changelog by Andy Lutomirski
---
arch/x86/kernel/process_64.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 4c388817..7e582be 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -429,7 +429,8 @@ unsigned long x86_fsbase_read_task(struct task_struct *task)
if (task == current)
fsbase = x86_fsbase_read_cpu();
- else if (task->thread.fsindex == 0)
+ else if (static_cpu_has(X86_FEATURE_FSGSBASE) ||
+ (task->thread.fsindex == 0))
fsbase = task->thread.fsbase;
else
fsbase = x86_fsgsbase_read_task(task, task->thread.fsindex);
@@ -443,7 +444,8 @@ unsigned long x86_gsbase_read_task(struct task_struct *task)
if (task == current)
gsbase = x86_gsbase_read_cpu_inactive();
- else if (task->thread.gsindex == 0)
+ else if (static_cpu_has(X86_FEATURE_FSGSBASE) ||
+ (task->thread.gsindex == 0))
gsbase = task->thread.gsbase;
else
gsbase = x86_fsgsbase_read_task(task, task->thread.gsindex);
@@ -483,10 +485,11 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
p->thread.sp = (unsigned long) fork_frame;
p->thread.io_bitmap_ptr = NULL;
- savesegment(gs, p->thread.gsindex);
- p->thread.gsbase = p->thread.gsindex ? 0 : me->thread.gsbase;
- savesegment(fs, p->thread.fsindex);
- p->thread.fsbase = p->thread.fsindex ? 0 : me->thread.fsbase;
+ save_fsgs(me);
+ p->thread.fsindex = me->thread.fsindex;
+ p->thread.fsbase = me->thread.fsbase;
+ p->thread.gsindex = me->thread.gsindex;
+ p->thread.gsbase = me->thread.gsbase;
savesegment(es, p->thread.es);
savesegment(ds, p->thread.ds);
memset(p->thread.ptrace_bps, 0, sizeof(p->thread.ptrace_bps));
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 08/17] x86/entry/64: Clean up paranoid exit
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (6 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 07/17] x86/fsgsbase/64: Use FSGSBASE instructions on thread copy and ptrace Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 09/17] x86/entry/64: Switch CR3 before SWAPGS in paranoid entry Chang S. Bae
` (9 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Andy Lutomirski, Thomas Gleixner,
Borislav Petkov, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen, Vegard Nossum
From: Andy Lutomirski <luto@kernel.org>
All that paranoid exit needs to do is to disable IRQs, handle IRQ tracing,
then restore CR3, and restore GS base. Simply do those actions in that
order. Cleaning up the spaghetti code.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
---
Changes from v7:
* Included as a new patch. Took the cleanup part from the Andy Lutomirski's
original patch [*] and edited its changelog a little bit.
[*] https://lkml.kernel.org/r/59725ceb08977359489fbed979716949ad45f616.1562035429.git.luto@kernel.org
---
arch/x86/entry/entry_64.S | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index b7c3ea4..dd0d62a 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1265,20 +1265,25 @@ END(paranoid_entry)
ENTRY(paranoid_exit)
UNWIND_HINT_REGS
DISABLE_INTERRUPTS(CLBR_ANY)
+
+ /*
+ * The order of operations is important. IRQ tracing requires
+ * kernel GS base and CR3. RESTORE_CR3 requires kernel GS base.
+ *
+ * NB to anyone to try to optimize this code: this code does
+ * not execute at all for exceptions from user mode. Those
+ * exceptions go through error_exit instead.
+ */
TRACE_IRQS_OFF_DEBUG
- testl %ebx, %ebx /* swapgs needed? */
- jnz .Lparanoid_exit_no_swapgs
- TRACE_IRQS_IRETQ
- /* Always restore stashed CR3 value (see paranoid_entry) */
- RESTORE_CR3 scratch_reg=%rbx save_reg=%r14
+ RESTORE_CR3 scratch_reg=%rax save_reg=%r14
+
+ /* If EBX is 0, SWAPGS is required */
+ testl %ebx, %ebx
+ jnz restore_regs_and_return_to_kernel
+
+ /* We are returning to a context with user GS base */
SWAPGS_UNSAFE_STACK
- jmp .Lparanoid_exit_restore
-.Lparanoid_exit_no_swapgs:
- TRACE_IRQS_IRETQ_DEBUG
- /* Always restore stashed CR3 value (see paranoid_entry) */
- RESTORE_CR3 scratch_reg=%rbx save_reg=%r14
-.Lparanoid_exit_restore:
- jmp restore_regs_and_return_to_kernel
+ jmp restore_regs_and_return_to_kernel
END(paranoid_exit)
/*
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 09/17] x86/entry/64: Switch CR3 before SWAPGS in paranoid entry
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (7 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 08/17] x86/entry/64: Clean up paranoid exit Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 10/17] x86/entry/64: Introduce the FIND_PERCPU_BASE macro Chang S. Bae
` (8 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Thomas Gleixner, Borislav Petkov,
Andy Lutomirski, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen, Vegard Nossum
When FSGSBASE is enabled, the GS base handling in paranoid entry will need
to retrieve the kernel GS base which requires that the kernel page table is
active.
As the CR3 switch to the kernel page tables (PTI is active) does not depend
on kernel GS base, move the CR3 switch in front of the GS base handling.
Comment the EBX content while at it.
No functional change.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
---
Changes from v7:
* Rebased onto the LFENCE-based SWAPGS mitigation code
* Dropped the READ_MSR_GSBASE macro by Thomas
* Rewrote changelog and comments by Thomas
* Use 'GS base' consistently, instead of 'GSBASE'
---
arch/x86/entry/entry_64.S | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index dd0d62a..edb4160 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1219,15 +1219,7 @@ ENTRY(paranoid_entry)
cld
PUSH_AND_CLEAR_REGS save_ret=1
ENCODE_FRAME_POINTER 8
- movl $1, %ebx
- movl $MSR_GS_BASE, %ecx
- rdmsr
- testl %edx, %edx
- js 1f /* negative -> in kernel */
- SWAPGS
- xorl %ebx, %ebx
-1:
/*
* Always stash CR3 in %r14. This value will be restored,
* verbatim, at exit. Needed if paranoid_entry interrupted
@@ -1237,16 +1229,31 @@ ENTRY(paranoid_entry)
* This is also why CS (stashed in the "iret frame" by the
* hardware at entry) can not be used: this may be a return
* to kernel code, but with a user CR3 value.
+ *
+ * Switching CR3 does not depend on kernel GS base so it can
+ * be done before switching to the kernel GS base. This is
+ * required for FSGSBASE because the kernel GS base has to
+ * be retrieved from a kernel internal table.
*/
SAVE_AND_SWITCH_TO_KERNEL_CR3 scratch_reg=%rax save_reg=%r14
+ /* EBX = 1 -> kernel GSBASE active, no restore required */
+ movl $1, %ebx
/*
- * The above SAVE_AND_SWITCH_TO_KERNEL_CR3 macro doesn't do an
- * unconditional CR3 write, even in the PTI case. So do an lfence
- * to prevent GS speculation, regardless of whether PTI is enabled.
+ * The kernel-enforced convention is a negative GS base indicates
+ * a kernel value. No SWAPGS needed on entry and exit.
*/
- FENCE_SWAPGS_KERNEL_ENTRY
+ movl $MSR_GS_BASE, %ecx
+ rdmsr
+ testl %edx, %edx
+ jns .Lparanoid_entry_swapgs
+ ret
+.Lparanoid_entry_swapgs:
+ SWAPGS
+ FENCE_SWAPGS_KERNEL_ENTRY
+ /* EBX = 0 -> SWAPGS required on exit */
+ xorl %ebx, %ebx
ret
END(paranoid_entry)
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 10/17] x86/entry/64: Introduce the FIND_PERCPU_BASE macro
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (8 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 09/17] x86/entry/64: Switch CR3 before SWAPGS in paranoid entry Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 11/17] x86/entry/64: Handle FSGSBASE enabled paranoid entry/exit Chang S. Bae
` (7 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Thomas Gleixner, Borislav Petkov,
Andy Lutomirski, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen, Vegard Nossum
GS base is used to find per-CPU data in the kernel. But when GS base is
unknown, the per-CPU base can be found from the per_cpu_offset table with a
CPU NR. The CPU NR is extracted from the limit field of the CPUNODE entry
in GDT, or by the RDPID instruction. This is a prerequisite for using
FSGSBASE in the low level entry code.
Also, add the GAS-compatible RDPID macro as binutils 2.21 does not support
it. Support is added in version 2.27.
Suggested-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
---
Changes from v7:
* No code change
* Massaged changelog by Thomas
* Used 'GS base' consistently, instead of 'GSBASE'
---
arch/x86/entry/calling.h | 34 ++++++++++++++++++++++++++++++++++
arch/x86/include/asm/inst.h | 15 +++++++++++++++
2 files changed, 49 insertions(+)
diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index 515c0ce..c222302 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -6,6 +6,7 @@
#include <asm/percpu.h>
#include <asm/asm-offsets.h>
#include <asm/processor-flags.h>
+#include <asm/inst.h>
/*
@@ -347,6 +348,39 @@ For 32-bit we have the following conventions - kernel is built with
#endif
.endm
+#ifdef CONFIG_SMP
+
+/*
+ * CPU/node NR is loaded from the limit (size) field of a special segment
+ * descriptor entry in GDT.
+ */
+.macro LOAD_CPU_AND_NODE_SEG_LIMIT reg:req
+ movq $__CPUNODE_SEG, \reg
+ lsl \reg, \reg
+.endm
+
+/*
+ * Fetch the per-CPU GS base value for this processor and put it in @reg.
+ * We normally use %gs for accessing per-CPU data, but we are setting up
+ * %gs here and obviously can not use %gs itself to access per-CPU data.
+ */
+.macro GET_PERCPU_BASE reg:req
+ ALTERNATIVE \
+ "LOAD_CPU_AND_NODE_SEG_LIMIT \reg", \
+ "RDPID \reg", \
+ X86_FEATURE_RDPID
+ andq $VDSO_CPUNODE_MASK, \reg
+ movq __per_cpu_offset(, \reg, 8), \reg
+.endm
+
+#else
+
+.macro GET_PERCPU_BASE reg:req
+ movq pcpu_unit_offsets(%rip), \reg
+.endm
+
+#endif /* CONFIG_SMP */
+
/*
* This does 'call enter_from_user_mode' unless we can avoid it based on
* kernel config or using the static jump infrastructure.
diff --git a/arch/x86/include/asm/inst.h b/arch/x86/include/asm/inst.h
index f5a796d..d063841 100644
--- a/arch/x86/include/asm/inst.h
+++ b/arch/x86/include/asm/inst.h
@@ -306,6 +306,21 @@
.endif
MODRM 0xc0 movq_r64_xmm_opd1 movq_r64_xmm_opd2
.endm
+
+.macro RDPID opd
+ REG_TYPE rdpid_opd_type \opd
+ .if rdpid_opd_type == REG_TYPE_R64
+ R64_NUM rdpid_opd \opd
+ .else
+ R32_NUM rdpid_opd \opd
+ .endif
+ .byte 0xf3
+ .if rdpid_opd > 7
+ PFX_REX rdpid_opd 0
+ .endif
+ .byte 0x0f, 0xc7
+ MODRM 0xc0 rdpid_opd 0x7
+.endm
#endif
#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 11/17] x86/entry/64: Handle FSGSBASE enabled paranoid entry/exit
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (9 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 10/17] x86/entry/64: Introduce the FIND_PERCPU_BASE macro Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 12/17] x86/entry/64: Document GSBASE handling in the paranoid path Chang S. Bae
` (6 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Thomas Gleixner, Borislav Petkov,
Andy Lutomirski, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen, Vegard Nossum
Without FSGSBASE, user space cannot change GS base other than through a
PRCTL. The kernel enforces that the user space GS base value is positive
as negative values are used for detecting the kernel space GS base value
in the paranoid entry code.
If FSGSBASE is enabled, user space can set arbitrary GS base values without
kernel intervention, including negative ones, which breaks the paranoid
entry assumptions.
To avoid this, paranoid entry needs to unconditionally save the current
GS base value independent of the interrupted context, retrieve and write
the kernel GS base and unconditionally restore the saved value on exit.
The restore happens either in paranoid exit or in the special exit path of
the NMI low level code.
All other entry code paths which use unconditional SWAPGS are not affected
as they do not depend on the actual content.
The new logic for paranoid entry, when FSGSBASE is enabled, removes SWAPGS
and replaces with unconditional WRGSBASE. Hence no fences are needed.
Suggested-by: H. Peter Anvin <hpa@zytor.com>
Suggested-by: Andy Lutomirski <luto@kernel.org>
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
---
Changes from v7:
* Rebased paranoid exit changes on the precedent cleanup patch
* Massaged changelog and comment by Thomas
* Added comments related to the SWAPGS mitigation
* Used 'GS base' consistently, instead of 'GSBASE'
---
arch/x86/entry/calling.h | 6 ++++
arch/x86/entry/entry_64.S | 78 +++++++++++++++++++++++++++++++++++++++++------
2 files changed, 75 insertions(+), 9 deletions(-)
diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index c222302..673d086 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -340,6 +340,12 @@ For 32-bit we have the following conventions - kernel is built with
#endif
.endm
+.macro SAVE_AND_SET_GSBASE scratch_reg:req save_reg:req
+ rdgsbase \save_reg
+ GET_PERCPU_BASE \scratch_reg
+ wrgsbase \scratch_reg
+.endm
+
#endif /* CONFIG_X86_64 */
.macro STACKLEAK_ERASE
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index edb4160..d554754 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -38,6 +38,7 @@
#include <asm/export.h>
#include <asm/frame.h>
#include <asm/nospec-branch.h>
+#include <asm/fsgsbase.h>
#include <linux/err.h>
#include "calling.h"
@@ -1210,9 +1211,14 @@ idtentry machine_check do_mce has_error_code=0 paranoid=1
#endif
/*
- * Save all registers in pt_regs, and switch gs if needed.
- * Use slow, but surefire "are we in kernel?" check.
- * Return: ebx=0: need swapgs on exit, ebx=1: otherwise
+ * Save all registers in pt_regs. Return GS base related information
+ * in EBX depending on the availability of the FSGSBASE instructions:
+ *
+ * FSGSBASE R/EBX
+ * N 0 -> SWAPGS on exit
+ * 1 -> no SWAPGS on exit
+ *
+ * Y GS base value at entry, must be restored in paranoid_exit
*/
ENTRY(paranoid_entry)
UNWIND_HINT_FUNC
@@ -1237,7 +1243,29 @@ ENTRY(paranoid_entry)
*/
SAVE_AND_SWITCH_TO_KERNEL_CR3 scratch_reg=%rax save_reg=%r14
- /* EBX = 1 -> kernel GSBASE active, no restore required */
+ /*
+ * Handling GS base depends on the availability of FSGSBASE.
+ *
+ * Without FSGSBASE the kernel enforces that negative GS base
+ * values indicate kernel GS base. With FSGSBASE no assumptions
+ * can be made about the GS base value when entering from user
+ * space.
+ */
+ ALTERNATIVE "jmp .Lparanoid_entry_checkgs", "", X86_FEATURE_FSGSBASE
+
+ /*
+ * Read the current GS base and store it in %rbx unconditionally,
+ * retrieve and set the current CPUs kernel GS base. The stored value
+ * has to be restored in paranoid_exit unconditionally.
+ *
+ * This unconditional write of GS base ensures no subsequent load
+ * based on a mispredicted GS base.
+ */
+ SAVE_AND_SET_GSBASE scratch_reg=%rax save_reg=%rbx
+ ret
+
+.Lparanoid_entry_checkgs:
+ /* EBX = 1 -> kernel GS base active, no restore required */
movl $1, %ebx
/*
* The kernel-enforced convention is a negative GS base indicates
@@ -1264,10 +1292,17 @@ END(paranoid_entry)
*
* We may be returning to very strange contexts (e.g. very early
* in syscall entry), so checking for preemption here would
- * be complicated. Fortunately, we there's no good reason
- * to try to handle preemption here.
+ * be complicated. Fortunately, there's no good reason to try
+ * to handle preemption here.
+ *
+ * R/EBX contains the GS base related information depending on the
+ * availability of the FSGSBASE instructions:
+ *
+ * FSGSBASE R/EBX
+ * N 0 -> SWAPGS on exit
+ * 1 -> no SWAPGS on exit
*
- * On entry, ebx is "no swapgs" flag (1: don't need swapgs, 0: need it)
+ * Y User space GS base, must be restored unconditionally
*/
ENTRY(paranoid_exit)
UNWIND_HINT_REGS
@@ -1284,7 +1319,15 @@ ENTRY(paranoid_exit)
TRACE_IRQS_OFF_DEBUG
RESTORE_CR3 scratch_reg=%rax save_reg=%r14
- /* If EBX is 0, SWAPGS is required */
+ /* Handle the three GS base cases */
+ ALTERNATIVE "jmp .Lparanoid_exit_checkgs", "", X86_FEATURE_FSGSBASE
+
+ /* With FSGSBASE enabled, unconditionally resotre GS base */
+ wrgsbase %rbx
+ jmp restore_regs_and_return_to_kernel
+
+.Lparanoid_exit_checkgs:
+ /* On non-FSGSBASE systems, conditionally do SWAPGS */
testl %ebx, %ebx
jnz restore_regs_and_return_to_kernel
@@ -1698,10 +1741,27 @@ end_repeat_nmi:
/* Always restore stashed CR3 value (see paranoid_entry) */
RESTORE_CR3 scratch_reg=%r15 save_reg=%r14
- testl %ebx, %ebx /* swapgs needed? */
+ /*
+ * The above invocation of paranoid_entry stored the GS base
+ * related information in R/EBX depending on the availability
+ * of FSGSBASE.
+ *
+ * If FSGSBASE is enabled, restore the saved GS base value
+ * unconditionally, otherwise take the conditional SWAPGS path.
+ */
+ ALTERNATIVE "jmp nmi_no_fsgsbase", "", X86_FEATURE_FSGSBASE
+
+ wrgsbase %rbx
+ jmp nmi_restore
+
+nmi_no_fsgsbase:
+ /* EBX == 0 -> invoke SWAPGS */
+ testl %ebx, %ebx
jnz nmi_restore
+
nmi_swapgs:
SWAPGS_UNSAFE_STACK
+
nmi_restore:
POP_REGS
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 12/17] x86/entry/64: Document GSBASE handling in the paranoid path
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (10 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 11/17] x86/entry/64: Handle FSGSBASE enabled paranoid entry/exit Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 13/17] x86/speculation/swapgs: Check FSGSBASE in enabling SWAPGS mitigation Chang S. Bae
` (5 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Thomas Gleixner, Borislav Petkov,
Andy Lutomirski, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen
On FSGSBASE systems, the way to handle GS base in the paranoid path is
different from the existing SWAPGS-based entry/exit path handling. Document
the reason and what has to be done for FSGSBASE enabled systems.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
---
Changes from v7:
* Massaged doc and changelog by Thomas
* Used 'GS base' consistently, instead of 'GSBASE'
---
Documentation/x86/entry_64.rst | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/x86/entry_64.rst b/Documentation/x86/entry_64.rst
index a48b3f6..0499a40 100644
--- a/Documentation/x86/entry_64.rst
+++ b/Documentation/x86/entry_64.rst
@@ -108,3 +108,12 @@ We try to only use IST entries and the paranoid entry code for vectors
that absolutely need the more expensive check for the GS base - and we
generate all 'normal' entry points with the regular (faster) paranoid=0
variant.
+
+On FSGSBASE systems, however, user space can set GS without kernel
+interaction. It means the value of GS base itself does not imply anything,
+whether a kernel value or a user space value. So, there is no longer a safe
+way to check whether the exception is entering from user mode or kernel
+mode in the paranoid entry code path. So the GS base value needs to be read
+out, saved and the kernel GS base value written. On exit, the saved GS base
+value needs to be restored unconditionally. The non-paranoid entry/exit
+code still uses SWAPGS unconditionally as the state is known.
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 13/17] x86/speculation/swapgs: Check FSGSBASE in enabling SWAPGS mitigation
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (11 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 12/17] x86/entry/64: Document GSBASE handling in the paranoid path Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 14/17] selftests/x86/fsgsbase: Test ptracer-induced GS base write with FSGSBASE Chang S. Bae
` (4 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Tony Luck, Thomas Gleixner,
Borislav Petkov, Andy Lutomirski, H . Peter Anvin, Dave Hansen,
Andi Kleen
From: Tony Luck <tony.luck@intel.com>
Before enabling FSGSBASE the kernel could safely assume that the content
of GS base was a user address. Thus any speculative access as the result
of a mispredicted branch controlling the execution of SWAPGS would be to
a user address. So systems with speculation-proof SMAP did not need to
add additional LFENCE instructions to mitigate.
With FSGSBASE enabled a hostile user can set GS base to a kernel address.
So they can make the kernel speculatively access data they wish to leak
via a side channel. This means that SMAP provides no protection.
Add FSGSBASE as an additional condition to enable the fence-based SWAPGS
mitigation.
Signed-off-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
---
Changes from v7:
* Included as a new patch.
---
arch/x86/kernel/cpu/bugs.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 91c2561..e06356f 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -321,14 +321,12 @@ static void __init spectre_v1_select_mitigation(void)
* If FSGSBASE is enabled, the user can put a kernel address in
* GS, in which case SMAP provides no protection.
*
- * [ NOTE: Don't check for X86_FEATURE_FSGSBASE until the
- * FSGSBASE enablement patches have been merged. ]
- *
* If FSGSBASE is disabled, the user can only put a user space
* address in GS. That makes an attack harder, but still
* possible if there's no SMAP protection.
*/
- if (!smap_works_speculatively()) {
+ if (boot_cpu_has(X86_FEATURE_FSGSBASE) ||
+ !smap_works_speculatively()) {
/*
* Mitigation can be provided from SWAPGS itself or
* PTI as the CR3 write in the Meltdown mitigation
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 14/17] selftests/x86/fsgsbase: Test ptracer-induced GS base write with FSGSBASE
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (12 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 13/17] x86/speculation/swapgs: Check FSGSBASE in enabling SWAPGS mitigation Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 15/17] x86/fsgsbase/64: Enable FSGSBASE on 64bit by default and add a chicken bit Chang S. Bae
` (3 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Thomas Gleixner, Borislav Petkov,
Andy Lutomirski, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen
This validates that GS selector and base are independently preserved in
ptrace commands.
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
---
Changes from v7:
* Fixed the test message
---
tools/testing/selftests/x86/fsgsbase.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/x86/fsgsbase.c b/tools/testing/selftests/x86/fsgsbase.c
index 950a48b..9a43498 100644
--- a/tools/testing/selftests/x86/fsgsbase.c
+++ b/tools/testing/selftests/x86/fsgsbase.c
@@ -465,7 +465,7 @@ static void test_ptrace_write_gsbase(void)
wait(&status);
if (WSTOPSIG(status) == SIGTRAP) {
- unsigned long gs;
+ unsigned long gs, base;
unsigned long gs_offset = USER_REGS_OFFSET(gs);
unsigned long base_offset = USER_REGS_OFFSET(gs_base);
@@ -481,6 +481,7 @@ static void test_ptrace_write_gsbase(void)
err(1, "PTRACE_POKEUSER");
gs = ptrace(PTRACE_PEEKUSER, child, gs_offset, NULL);
+ base = ptrace(PTRACE_PEEKUSER, child, base_offset, NULL);
/*
* In a non-FSGSBASE system, the nonzero selector will load
@@ -501,8 +502,14 @@ static void test_ptrace_write_gsbase(void)
*/
if (gs == 0)
printf("\tNote: this is expected behavior on older kernels.\n");
+ } else if (have_fsgsbase && (base != 0xFF)) {
+ nerrs++;
+ printf("[FAIL]\tGSBASE changed to %lx\n", base);
} else {
- printf("[OK]\tGS remained 0x%hx\n", *shared_scratch);
+ printf("[OK]\tGS remained 0x%hx", *shared_scratch);
+ if (have_fsgsbase)
+ printf(" and GSBASE changed to 0xFF");
+ printf("\n");
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 15/17] x86/fsgsbase/64: Enable FSGSBASE on 64bit by default and add a chicken bit
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (13 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 14/17] selftests/x86/fsgsbase: Test ptracer-induced GS base write with FSGSBASE Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 16/17] x86/elf: Enumerate kernel FSGSBASE capability in AT_HWCAP2 Chang S. Bae
` (2 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Andy Lutomirski, Thomas Gleixner,
Borislav Petkov, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen
From: Andy Lutomirski <luto@kernel.org>
Now that FSGSBASE is fully supported, remove unsafe_fsgsbase, enable
FSGSBASE by default, and add nofsgsbase to disable it.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
---
Changes from v7:
* No code change
* Massaged title by Thomas
---
Documentation/admin-guide/kernel-parameters.txt | 3 +--
arch/x86/kernel/cpu/common.c | 32 +++++++++++--------------
2 files changed, 15 insertions(+), 20 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2ff72af..ce11a6a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2875,8 +2875,7 @@
no5lvl [X86-64] Disable 5-level paging mode. Forces
kernel to use 4-level paging instead.
- unsafe_fsgsbase [X86] Allow FSGSBASE instructions. This will be
- replaced with a nofsgsbase flag.
+ nofsgsbase [X86] Disables FSGSBASE instructions.
no_console_suspend
[HW] Never suspend the console
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 9f57fb0..0fe75fa 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -437,21 +437,21 @@ static void __init setup_cr_pinning(void)
static_key_enable(&cr_pinning.key);
}
-/*
- * Temporary hack: FSGSBASE is unsafe until a few kernel code paths are
- * updated. This allows us to get the kernel ready incrementally.
- *
- * Once all the pieces are in place, these will go away and be replaced with
- * a nofsgsbase chicken flag.
- */
-static bool unsafe_fsgsbase;
-
-static __init int setup_unsafe_fsgsbase(char *arg)
+static __init int x86_nofsgsbase_setup(char *arg)
{
- unsafe_fsgsbase = true;
+ /* Require an exact match without trailing characters. */
+ if (strlen(arg))
+ return 0;
+
+ /* Do not emit a message if the feature is not present. */
+ if (!boot_cpu_has(X86_FEATURE_FSGSBASE))
+ return 1;
+
+ setup_clear_cpu_cap(X86_FEATURE_FSGSBASE);
+ pr_info("nofsgsbase: FSGSBASE disabled\n");
return 1;
}
-__setup("unsafe_fsgsbase", setup_unsafe_fsgsbase);
+__setup("nofsgsbase", x86_nofsgsbase_setup);
/*
* Protection Keys are not available in 32-bit mode.
@@ -1472,12 +1472,8 @@ static void identify_cpu(struct cpuinfo_x86 *c)
setup_umip(c);
/* Enable FSGSBASE instructions if available. */
- if (cpu_has(c, X86_FEATURE_FSGSBASE)) {
- if (unsafe_fsgsbase)
- cr4_set_bits(X86_CR4_FSGSBASE);
- else
- clear_cpu_cap(c, X86_FEATURE_FSGSBASE);
- }
+ if (cpu_has(c, X86_FEATURE_FSGSBASE))
+ cr4_set_bits(X86_CR4_FSGSBASE);
/*
* The vendor-specific functions might have changed features.
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 16/17] x86/elf: Enumerate kernel FSGSBASE capability in AT_HWCAP2
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (14 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 15/17] x86/fsgsbase/64: Enable FSGSBASE on 64bit by default and add a chicken bit Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-12 20:06 ` [PATCH v8 17/17] Documentation/x86/64: Add documentation for GS/FS addressing mode Chang S. Bae
2019-09-13 4:10 ` [PATCH v8 00/17] Enable FSGSBASE instructions Andy Lutomirski
17 siblings, 0 replies; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Andi Kleen, Thomas Gleixner,
Borislav Petkov, Andy Lutomirski, H . Peter Anvin, Dave Hansen,
Tony Luck
From: Andi Kleen <ak@linux.intel.com>
The kernel needs to explicitly enable FSGSBASE. So, the application needs
to know if it can safely use these instructions. Just looking at the CPUID
bit is not enough because it may be running in a kernel that does not
enable the instructions.
One way for the application would be to just try and catch the SIGILL.
But that is difficult to do in libraries which may not want to overwrite
the signal handlers of the main application.
Enumerate the enabled FSGSBASE capability in bit 1 of AT_HWCAP2 in the ELF
aux vector. AT_HWCAP2 is already used by PPC for similar purposes.
The application can access it open coded or by using the getauxval()
function in newer versions of glibc.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
---
Changes from v7:
* No code change
* Massaged changelog by Thomas
---
arch/x86/include/uapi/asm/hwcap2.h | 3 +++
arch/x86/kernel/cpu/common.c | 4 +++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/uapi/asm/hwcap2.h b/arch/x86/include/uapi/asm/hwcap2.h
index 8b2effe..5fdfcb4 100644
--- a/arch/x86/include/uapi/asm/hwcap2.h
+++ b/arch/x86/include/uapi/asm/hwcap2.h
@@ -5,4 +5,7 @@
/* MONITOR/MWAIT enabled in Ring 3 */
#define HWCAP2_RING3MWAIT (1 << 0)
+/* Kernel allows FSGSBASE instructions available in Ring 3 */
+#define HWCAP2_FSGSBASE BIT(1)
+
#endif
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 0fe75fa..43b5946 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1472,8 +1472,10 @@ static void identify_cpu(struct cpuinfo_x86 *c)
setup_umip(c);
/* Enable FSGSBASE instructions if available. */
- if (cpu_has(c, X86_FEATURE_FSGSBASE))
+ if (cpu_has(c, X86_FEATURE_FSGSBASE)) {
cr4_set_bits(X86_CR4_FSGSBASE);
+ elf_hwcap2 |= HWCAP2_FSGSBASE;
+ }
/*
* The vendor-specific functions might have changed features.
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 17/17] Documentation/x86/64: Add documentation for GS/FS addressing mode
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (15 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 16/17] x86/elf: Enumerate kernel FSGSBASE capability in AT_HWCAP2 Chang S. Bae
@ 2019-09-12 20:06 ` Chang S. Bae
2019-09-27 21:25 ` Randy Dunlap
2019-09-13 4:10 ` [PATCH v8 00/17] Enable FSGSBASE instructions Andy Lutomirski
17 siblings, 1 reply; 23+ messages in thread
From: Chang S. Bae @ 2019-09-12 20:06 UTC (permalink / raw)
To: linux-kernel
Cc: ravi.v.shankar, chang.seok.bae, Thomas Gleixner, Borislav Petkov,
Andy Lutomirski, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen, Randy Dunlap, Jonathan Corbet
From: Thomas Gleixner <tglx@linutronix.de>
Explain how the GS/FS based addressing can be utilized in user space
applications along with the differences between the generic prctl() based
GS/FS base control and the FSGSBASE version available on newer CPUs.
Originally-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Jonathan Corbet <corbet@lwn.net>
---
Changes from v7:
* Rewritten the documentation and changelog by Thomas
* Included compiler version info additionally
---
Documentation/x86/x86_64/fsgs.rst | 200 +++++++++++++++++++++++++++++++++++++
Documentation/x86/x86_64/index.rst | 1 +
2 files changed, 201 insertions(+)
create mode 100644 Documentation/x86/x86_64/fsgs.rst
diff --git a/Documentation/x86/x86_64/fsgs.rst b/Documentation/x86/x86_64/fsgs.rst
new file mode 100644
index 0000000..791063c
--- /dev/null
+++ b/Documentation/x86/x86_64/fsgs.rst
@@ -0,0 +1,200 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Using FS and GS segments in user space applications
+===================================================
+
+The x86 architecture supports segmentation. Instructions which access
+memory can use segment register based addressing mode. The following
+notation is used to address a byte within a segment:
+
+ Segment-register:Byte-address
+
+The segment base address is added to the Byte-address to compute the
+resulting virtual address which is accessed. This allows to access multiple
+instances of data with the identical Byte-address, i.e. the same code. The
+selection of a particular instance is purely based on the base-address in
+the segment register.
+
+In 32-bit mode the CPU provides 6 segments, which also support segment
+limits. The limits can be used to enforce address space protections.
+
+In 64-bit mode the CS/SS/DS/ES segments are ignored and the base address is
+always 0 to provide a full 64bit address space. The FS and GS segments are
+still functional in 64-bit mode.
+
+Common FS and GS usage
+------------------------------
+
+The FS segment is commonly used to address Thread Local Storage (TLS). FS
+is usually managed by runtime code or a threading library. Variables
+declared with the '__thread' storage class specifier are instantiated per
+thread and the compiler emits the FS: address prefix for accesses to these
+variables. Each thread has its own FS base address so common code can be
+used without complex address offset calculations to access the per thread
+instances. Applications should not use FS for other purposes when they use
+runtimes or threading libraries which manage the per thread FS.
+
+The GS segment has no common use and can be used freely by
+applications. There is no storage class specifier similar to __thread which
+would cause the compiler to use GS based addressing modes. Newer versions
+of GCC and Clang support GS based addressing via address space identifiers.
+
+
+Reading and writing the FS/GS base address
+------------------------------------------
+
+There exist two mechanisms to read and write the FS/FS base address:
+
+ - the arch_prctl() system call
+
+ - the FSGSBASE instruction family
+
+Accessing FS/GS base with arch_prctl()
+--------------------------------------
+
+ The arch_prctl(2) based mechanism is available on all 64bit CPUs and all
+ kernel versions.
+
+ Reading the base:
+
+ arch_prctl(ARCH_GET_FS, &fsbase);
+ arch_prctl(ARCH_GET_GS, &gsbase);
+
+ Writing the base:
+
+ arch_prctl(ARCH_SET_FS, fsbase);
+ arch_prctl(ARCH_SET_GS, gsbase);
+
+ The ARCH_SET_GS prctl may be disabled depending on kernel configuration
+ and security settings.
+
+Accessing FS/GS base with the FSGSBASE instructions
+---------------------------------------------------
+
+ With the Ivy Bridge CPU generation Intel introduced a new set of
+ instructions to access the FS and GS base registers directly from user
+ space. These instructions are also supported on AMD Family 17H CPUs. The
+ following instructions are available:
+
+ =============== ===========================
+ RDFSBASE %reg Read the FS base register
+ RDGSBASE %reg Read the GS base register
+ WRFSBASE %reg Write the FS base register
+ WRGSBASE %reg Write the GS base register
+ =============== ===========================
+
+ The instructions avoid the overhead of the arch_prctl() syscall and allow
+ more flexible usage of the FS/GS addressing modes in user space
+ applications. This does not prevent conflicts between threading libraries
+ and runtimes which utilize FS and applications which want to use it for
+ their own purpose.
+
+FSGSBASE instructions enablement
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ The instructions are enumerated in CPUID leaf 7, bit 0 of EBX. If
+ available /proc/cpuinfo shows 'fsgsbase' in the flag entry of the CPUs.
+
+ The availability of the instructions is not enabling them
+ automatically. The kernel has to enable them explicitly in CR4. The
+ reason for this is that older kernels make assumptions about the values in
+ the GS register and enforce them when GS base is set via
+ arch_prctl(). Allowing user space to write arbitrary values to GS base
+ would violate these assumptions and cause malfunction.
+
+ On kernels which do not enable FSGSBASE the execution of the FSGSBASE
+ instructions will fault with a #UD exception.
+
+ The kernel provides reliable information about the enabled state in the
+ ELF AUX vector. If the HWCAP2_FSGSBASE bit is set in the AUX vector, the
+ kernel has FSGSBASE instructions enabled and applications can use them.
+ The following code example shows how this detection works::
+
+ #include <sys/auxv.h>
+ #include <elf.h>
+
+ /* Will be eventually in asm/hwcap.h */
+ #ifndef HWCAP2_FSGSBASE
+ #define HWCAP2_FSGSBASE (1 << 1)
+ #endif
+
+ ....
+
+ unsigned val = getauxval(AT_HWCAP2);
+
+ if (val & HWCAP2_FSGSBASE)
+ printf("FSGSBASE enabled\n");
+
+FSGSBASE instructions compiler support
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+GCC version 4.6.4 and newer provide instrinsics for the FSGSBASE
+instructions. Clang 5 supports them as well.
+
+ =================== ===========================
+ _readfsbase_u64() Read the FS base register
+ _readfsbase_u64() Read the GS base register
+ _writefsbase_u64() Write the FS base register
+ _writegsbase_u64() Write the GS base register
+ =================== ===========================
+
+To utilize these instrinsics <immintrin.h> must be included in the source
+code and the compiler option -mfsgsbase has to be added.
+
+Compiler support for FS/GS based addressing
+-------------------------------------------
+
+GCC version 6 and newer provide support for FS/GS based addressing via
+Named Address Spaces. GCC implements the following address space
+identifiers for x86:
+
+ ========= ====================================
+ __seg_fs Variable is addressed relative to FS
+ __seg_gs Variable is addressed relative to GS
+ ========= ====================================
+
+The preprocessor symbols __SEG_FS and __SEG_GS are defined when these
+address spaces are supported. Code which implements fallback modes should
+check whether these symbols are defined. Usage example::
+
+ #ifdef __SEG_GS
+
+ long data0 = 0;
+ long data1 = 1;
+
+ long __seg_gs *ptr;
+
+ /* Check whether FSGSBASE is enabled by the kernel (HWCAP2_FSGSBASE) */
+ ....
+
+ /* Set GS to point to data0 */
+ _writegsbase_u64(&data0);
+
+ /* Access offset 0 of GS */
+ ptr = 0;
+ print("data0 = %ld\n", *ptr);
+
+ /* Set GS to point to data1 */
+ _writegsbase_u64(&data1);
+ /* ptr still addresses offset 0! */
+ print("data1 = %ld\n", *ptr);
+
+
+Clang does not provide these address space identifiers, but it provides
+an attribute based mechanism in Clang 2.6 and newer versions:
+
+ ==================================== =====================================
+ __attribute__((address_space(256)) Variable is addressed relative to GS
+ __attribute__((address_space(257)) Variable is addressed relative to FS
+ ==================================== =====================================
+
+FS/GS based addressing with inline assembly
+-------------------------------------------
+
+In case the compiler does not support address spaces, inline assembly can
+be used for FS/GS based addressing mode::
+
+ mov %fs:offset, %reg
+ mov %gs:offset, %reg
+
+ mov %reg, %fs:offset
+ mov %reg, %gs:offset
\ No newline at end of file
diff --git a/Documentation/x86/x86_64/index.rst b/Documentation/x86/x86_64/index.rst
index d6eaaa5..a56070f 100644
--- a/Documentation/x86/x86_64/index.rst
+++ b/Documentation/x86/x86_64/index.rst
@@ -14,3 +14,4 @@ x86_64 Support
fake-numa-for-cpusets
cpu-hotplug-spec
machinecheck
+ fsgs
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v8 17/17] Documentation/x86/64: Add documentation for GS/FS addressing mode
2019-09-12 20:06 ` [PATCH v8 17/17] Documentation/x86/64: Add documentation for GS/FS addressing mode Chang S. Bae
@ 2019-09-27 21:25 ` Randy Dunlap
2019-09-27 21:50 ` Bae, Chang Seok
0 siblings, 1 reply; 23+ messages in thread
From: Randy Dunlap @ 2019-09-27 21:25 UTC (permalink / raw)
To: Chang S. Bae, linux-kernel
Cc: ravi.v.shankar, Thomas Gleixner, Borislav Petkov,
Andy Lutomirski, H . Peter Anvin, Dave Hansen, Tony Luck,
Andi Kleen, Jonathan Corbet
Hi,
Some doc comments/fixes below...
On 9/12/19 1:06 PM, Chang S. Bae wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> Explain how the GS/FS based addressing can be utilized in user space
> applications along with the differences between the generic prctl() based
> GS/FS base control and the FSGSBASE version available on newer CPUs.
>
> ---
>
> Changes from v7:
> * Rewritten the documentation and changelog by Thomas
> * Included compiler version info additionally
> ---
> Documentation/x86/x86_64/fsgs.rst | 200 +++++++++++++++++++++++++++++++++++++
> Documentation/x86/x86_64/index.rst | 1 +
> 2 files changed, 201 insertions(+)
> create mode 100644 Documentation/x86/x86_64/fsgs.rst
>
> diff --git a/Documentation/x86/x86_64/fsgs.rst b/Documentation/x86/x86_64/fsgs.rst
> new file mode 100644
> index 0000000..791063c
> --- /dev/null
> +++ b/Documentation/x86/x86_64/fsgs.rst
> @@ -0,0 +1,200 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Using FS and GS segments in user space applications
> +===================================================
> +
> +The x86 architecture supports segmentation. Instructions which access
> +memory can use segment register based addressing mode. The following
> +notation is used to address a byte within a segment:
> +
> + Segment-register:Byte-address
> +
> +The segment base address is added to the Byte-address to compute the
> +resulting virtual address which is accessed. This allows to access multiple
> +instances of data with the identical Byte-address, i.e. the same code. The
> +selection of a particular instance is purely based on the base-address in
> +the segment register.
> +
> +In 32-bit mode the CPU provides 6 segments, which also support segment
> +limits. The limits can be used to enforce address space protections.
> +
> +In 64-bit mode the CS/SS/DS/ES segments are ignored and the base address is
> +always 0 to provide a full 64bit address space. The FS and GS segments are
> +still functional in 64-bit mode.
> +
> +Common FS and GS usage
> +------------------------------
> +
> +The FS segment is commonly used to address Thread Local Storage (TLS). FS
> +is usually managed by runtime code or a threading library. Variables
> +declared with the '__thread' storage class specifier are instantiated per
> +thread and the compiler emits the FS: address prefix for accesses to these
> +variables. Each thread has its own FS base address so common code can be
> +used without complex address offset calculations to access the per thread
> +instances. Applications should not use FS for other purposes when they use
> +runtimes or threading libraries which manage the per thread FS.
> +
> +The GS segment has no common use and can be used freely by
> +applications. There is no storage class specifier similar to __thread which
> +would cause the compiler to use GS based addressing modes. Newer versions
> +of GCC and Clang support GS based addressing via address space identifiers.
> +
> +
> +Reading and writing the FS/GS base address
> +------------------------------------------
> +
> +There exist two mechanisms to read and write the FS/FS base address:
FS/GS
> +
> + - the arch_prctl() system call
> +
> + - the FSGSBASE instruction family
> +
> +Accessing FS/GS base with arch_prctl()
> +--------------------------------------
> +
> + The arch_prctl(2) based mechanism is available on all 64bit CPUs and all
64-bit
> + kernel versions.
> +
> + Reading the base:
> +
> + arch_prctl(ARCH_GET_FS, &fsbase);
> + arch_prctl(ARCH_GET_GS, &gsbase);
> +
> + Writing the base:
> +
> + arch_prctl(ARCH_SET_FS, fsbase);
> + arch_prctl(ARCH_SET_GS, gsbase);
> +
> + The ARCH_SET_GS prctl may be disabled depending on kernel configuration
> + and security settings.
> +
> +Accessing FS/GS base with the FSGSBASE instructions
> +---------------------------------------------------
> +
> + With the Ivy Bridge CPU generation Intel introduced a new set of
> + instructions to access the FS and GS base registers directly from user
> + space. These instructions are also supported on AMD Family 17H CPUs. The
> + following instructions are available:
> +
> + =============== ===========================
> + RDFSBASE %reg Read the FS base register
> + RDGSBASE %reg Read the GS base register
> + WRFSBASE %reg Write the FS base register
> + WRGSBASE %reg Write the GS base register
> + =============== ===========================
> +
> + The instructions avoid the overhead of the arch_prctl() syscall and allow
> + more flexible usage of the FS/GS addressing modes in user space
> + applications. This does not prevent conflicts between threading libraries
> + and runtimes which utilize FS and applications which want to use it for
> + their own purpose.
> +
> +FSGSBASE instructions enablement
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> + The instructions are enumerated in CPUID leaf 7, bit 0 of EBX. If
> + available /proc/cpuinfo shows 'fsgsbase' in the flag entry of the CPUs.
> +
> + The availability of the instructions is not enabling them
> + automatically. The kernel has to enable them explicitly in CR4. The
> + reason for this is that older kernels make assumptions about the values in
> + the GS register and enforce them when GS base is set via
> + arch_prctl(). Allowing user space to write arbitrary values to GS base
> + would violate these assumptions and cause malfunction.
> +
> + On kernels which do not enable FSGSBASE the execution of the FSGSBASE
> + instructions will fault with a #UD exception.
> +
> + The kernel provides reliable information about the enabled state in the
> + ELF AUX vector. If the HWCAP2_FSGSBASE bit is set in the AUX vector, the
> + kernel has FSGSBASE instructions enabled and applications can use them.
> + The following code example shows how this detection works::
> +
> + #include <sys/auxv.h>
> + #include <elf.h>
> +
> + /* Will be eventually in asm/hwcap.h */
> + #ifndef HWCAP2_FSGSBASE
> + #define HWCAP2_FSGSBASE (1 << 1)
> + #endif
> +
> + ....
> +
> + unsigned val = getauxval(AT_HWCAP2);
> +
> + if (val & HWCAP2_FSGSBASE)
> + printf("FSGSBASE enabled\n");
> +
> +FSGSBASE instructions compiler support
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +GCC version 4.6.4 and newer provide instrinsics for the FSGSBASE
> +instructions. Clang 5 supports them as well.
> +
> + =================== ===========================
> + _readfsbase_u64() Read the FS base register
> + _readfsbase_u64() Read the GS base register
> + _writefsbase_u64() Write the FS base register
> + _writegsbase_u64() Write the GS base register
> + =================== ===========================
> +
> +To utilize these instrinsics <immintrin.h> must be included in the source
> +code and the compiler option -mfsgsbase has to be added.
> +
> +Compiler support for FS/GS based addressing
> +-------------------------------------------
> +
> +GCC version 6 and newer provide support for FS/GS based addressing via
> +Named Address Spaces. GCC implements the following address space
> +identifiers for x86:
> +
> + ========= ====================================
> + __seg_fs Variable is addressed relative to FS
> + __seg_gs Variable is addressed relative to GS
> + ========= ====================================
> +
> +The preprocessor symbols __SEG_FS and __SEG_GS are defined when these
> +address spaces are supported. Code which implements fallback modes should
> +check whether these symbols are defined. Usage example::
> +
> + #ifdef __SEG_GS
> +
> + long data0 = 0;
> + long data1 = 1;
> +
> + long __seg_gs *ptr;
> +
> + /* Check whether FSGSBASE is enabled by the kernel (HWCAP2_FSGSBASE) */
> + ....
> +
> + /* Set GS to point to data0 */
> + _writegsbase_u64(&data0);
> +
> + /* Access offset 0 of GS */
> + ptr = 0;
> + print("data0 = %ld\n", *ptr);
> +
> + /* Set GS to point to data1 */
> + _writegsbase_u64(&data1);
> + /* ptr still addresses offset 0! */
> + print("data1 = %ld\n", *ptr);
> +
> +
> +Clang does not provide these address space identifiers, but it provides
> +an attribute based mechanism in Clang 2.6 and newer versions:
> +
> + ==================================== =====================================
> + __attribute__((address_space(256)) Variable is addressed relative to GS
> + __attribute__((address_space(257)) Variable is addressed relative to FS
> + ==================================== =====================================
> +
> +FS/GS based addressing with inline assembly
> +-------------------------------------------
> +
> +In case the compiler does not support address spaces, inline assembly can
> +be used for FS/GS based addressing mode::
> +
> + mov %fs:offset, %reg
> + mov %gs:offset, %reg
> +
> + mov %reg, %fs:offset
> + mov %reg, %gs:offset
> \ No newline at end of file
oops.
--
~Randy
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v8 17/17] Documentation/x86/64: Add documentation for GS/FS addressing mode
2019-09-27 21:25 ` Randy Dunlap
@ 2019-09-27 21:50 ` Bae, Chang Seok
0 siblings, 0 replies; 23+ messages in thread
From: Bae, Chang Seok @ 2019-09-27 21:50 UTC (permalink / raw)
To: Randy Dunlap
Cc: LKML, Shankar, Ravi V, Thomas Gleixner, Borislav Petkov,
Andy Lutomirski, H . Peter Anvin, Hansen, Dave, Luck, Tony,
Andi Kleen, Jonathan Corbet
> On Sep 27, 2019, at 14:25, Randy Dunlap <rdunlap@infradead.org> wrote:
>
> Hi,
> Some doc comments/fixes below...
>
> On 9/12/19 1:06 PM, Chang S. Bae wrote:
>> From: Thomas Gleixner <tglx@linutronix.de>
>>
[snip]
>> +There exist two mechanisms to read and write the FS/FS base address:
>
> FS/GS
[snip]
>> + The arch_prctl(2) based mechanism is available on all 64bit CPUs and all
>
> 64-bit
[snip]
>> + mov %reg, %gs:offset
>> \ No newline at end of file
> oops.
Thanks. Let me include this in my v9 submission.
Chang
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v8 00/17] Enable FSGSBASE instructions
2019-09-12 20:06 [PATCH v8 00/17] Enable FSGSBASE instructions Chang S. Bae
` (16 preceding siblings ...)
2019-09-12 20:06 ` [PATCH v8 17/17] Documentation/x86/64: Add documentation for GS/FS addressing mode Chang S. Bae
@ 2019-09-13 4:10 ` Andy Lutomirski
2019-09-16 9:21 ` Thomas Gleixner
2019-09-18 21:02 ` Bae, Chang Seok
17 siblings, 2 replies; 23+ messages in thread
From: Andy Lutomirski @ 2019-09-13 4:10 UTC (permalink / raw)
To: Chang S. Bae, linux-kernel, the arch/x86 maintainers,
Borislav Petkov, dave.hansen, tony.luck, ak
Cc: ravi.v.shankar
On 9/12/19 1:06 PM, Chang S. Bae wrote:
> Updates from v7 [7]:
> (1) Consider FSGSBASE when determining which Spectre SWAPGS mitigations are
> required.
> (2) Fixed save_fsgs() to be aware of interrupt conditions
> (3) Made selftest changes based on Andy's previous fixes and cleanups
> (4) Included Andy's paranoid exit cleanup
> (5) Included documentation rewritten by Thomas
> (6) Carried on Thomas' edits on multiple changelogs and comments
> (7) Used '[FS|GS] base' consistently, except for selftest where GSBASE has
> been already used in its test messages
> (8) Dropped the READ_MSR_GSBASE macro
>
This looks unpleasant to review. I wonder if it would be better to
unrevert the reversion, merge up to Linus' tree or -tip, and then base
the changes on top of that.
I also think that, before this series can have my ack, it needs an
actual gdb maintainer to chime in, publicly, and state that they have
thought about and tested the ABI changes and that gdb still works on
patched kernels with and without FSGSBASE enabled. I realize that there
were all kinds of discussions, but they were all quite theoretical, and
I think that the actual patches need to be considered by people who
understand the concerns. Specific test cases would be nice, too.
Finally, I wrote up some notes here:
https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes&id=70a7d284989e3539ee84f9d709d6450099f773fb
I want to make sure that they're accounted for, and that patch should
possibly be applied. The parent (broken link, but should fix itself soon):
https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes&id=166324e907f8a71c823b41bbc2e1b5bc711532d8
may also help understand the relevant code.
--Andy
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v8 00/17] Enable FSGSBASE instructions
2019-09-13 4:10 ` [PATCH v8 00/17] Enable FSGSBASE instructions Andy Lutomirski
@ 2019-09-16 9:21 ` Thomas Gleixner
2019-09-18 21:02 ` Bae, Chang Seok
1 sibling, 0 replies; 23+ messages in thread
From: Thomas Gleixner @ 2019-09-16 9:21 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Chang S. Bae, linux-kernel, the arch/x86 maintainers,
Borislav Petkov, dave.hansen, tony.luck, ak, ravi.v.shankar
On Thu, 12 Sep 2019, Andy Lutomirski wrote:
Thanks, for adding me and the others on Cc. I had to dig out the cover
letter from my LKML archive ....
> On 9/12/19 1:06 PM, Chang S. Bae wrote:
>
> > Updates from v7 [7]:
> > (1) Consider FSGSBASE when determining which Spectre SWAPGS mitigations are
> > required.
> > (2) Fixed save_fsgs() to be aware of interrupt conditions
> > (3) Made selftest changes based on Andy's previous fixes and cleanups
> > (4) Included Andy's paranoid exit cleanup
> > (5) Included documentation rewritten by Thomas
> > (6) Carried on Thomas' edits on multiple changelogs and comments
> > (7) Used '[FS|GS] base' consistently, except for selftest where GSBASE has
> > been already used in its test messages
> > (8) Dropped the READ_MSR_GSBASE macro
> >
>
> This looks unpleasant to review. I wonder if it would be better to unrevert
> the reversion, merge up to Linus' tree or -tip, and then base the changes on
> top of that.
I don't think that's a good idea. The old code is broken in several ways
and not bisectable. So we really better start from scratch.
Thanks,
tglx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v8 00/17] Enable FSGSBASE instructions
2019-09-13 4:10 ` [PATCH v8 00/17] Enable FSGSBASE instructions Andy Lutomirski
2019-09-16 9:21 ` Thomas Gleixner
@ 2019-09-18 21:02 ` Bae, Chang Seok
1 sibling, 0 replies; 23+ messages in thread
From: Bae, Chang Seok @ 2019-09-18 21:02 UTC (permalink / raw)
To: Andy Lutomirski
Cc: LKML, the arch/x86 maintainers, Borislav Petkov, Hansen, Dave,
Luck, Tony, Andi Kleen, Shankar, Ravi V, gdb-patches,
Joel Brobecker, Pedro Alves
> On Sep 12, 2019, at 21:10, Andy Lutomirski <luto@kernel.org> wrote:
>
> I also think that, before this series can have my ack, it needs an actual gdb maintainer to chime in, publicly, and state that they have thought about and tested the ABI changes and that gdb still works on patched kernels with and without FSGSBASE enabled. I realize that there were all kinds of discussions, but they were all quite theoretical, and I think that the actual patches need to be considered by people who understand the concerns. Specific test cases would be nice, too.
>
> Finally, I wrote up some notes here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes&id=70a7d284989e3539ee84f9d709d6450099f773fb
>
> I want to make sure that they're accounted for, and that patch should possibly be applied. The parent (broken link, but should fix itself soon):
>
> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes&id=166324e907f8a71c823b41bbc2e1b5bc711532d8
>
> may also help understand the relevant code.
Adds GDB folks and here is a link to the patch0 entry in this series for them:
https://lore.kernel.org/lkml/1568318818-4091-1-git-send-email-chang.seok.bae@intel.com/
Thanks,
Chang
^ permalink raw reply [flat|nested] 23+ messages in thread