[PATCH 0/15] x86/split_lock: Enable #AC exception for split locked accesses

[PATCH 0/15] x86/split_lock: Enable #AC exception for split locked accesses

[Fenghua Yu]

When executing an atomic instruction, sometimes the data access crosses
the cache line boundary. This causes performance degradation. Intel
introduces mechanism to detect such split lock issue via alignment check
fault.

Instructions that may cause split lock issue include lock add, lock btc,
xchg, lsl, far call, ltr, etc.

To detect split lock, a new control bit (bit 29) in TEST_CTL MSR 0x33
will be introduced in future x86 processors. When the bit 29 is set,
the processor causes #AC(0) exception for split locked accesses at all
CPL.

BIOS or hardware may set or clear the control bit depending on
platforms. To avoid disturbing BIOS/hardware setting, by default,
kernel keeps split lock BIOS setting.

Kernel can override BIOS setting by explicitly enabling or disabling
the feature using kernel parameters "split_lock_ac=on" or
"split_lock_ac=off".

When an instruction accesses split locked data and triggers #AC
exception, the faulting instruction is handled as follows:
- the faulting instruction is re-executed when the instruction is
  from kernel.
- user process gets SIGBUS signal when the faulting instruction is
  from the user process. This behavior can be changed to re-execute
  the faulting user instruction by sysfs interface
  /sys/kernel/split_lock/user_mode.

We do see #AC exception is triggered and causes system hang in BIOS path
(e.g. during system reboot) after kernel enables the feature. Instead of
debugging potential system hangs due to split locked accesses in various
buggy BIOSes, kernel only maintains enabled feature in the kernel domain.
Once it's out of the kernel domain (i.e. S3, S4, S5, efi runtime
services, kexec, kdump, CPU offline, etc), kernel restores to BIOS
setting. When returning from BIOS, kernel restores to kernel setting.

Since kernel doesn't know when SMI comes, it's impossible for kernel
to disable split lock #AC before entering SMI. So SMI handler may
inherit kernel's split lock setting and kernel tester may end up
debug split lock issues in SMI.

In cases when user does want to detect and fix split lock #AC bang
in BIOS (e.g. in real time), the user can enable split lock #AC
using sysfs interface /sys/kernel/split_lock/bios.

The bit 29 specification in MSR TEST_CTL is published in the latest
Intel Architecture Instruction Set Extensions and Future Features
Programming Reference.

Tests:
- /sys/kernel/split_lock/test_kernel (in patche 14) tests kernel
  space split lock.
- selftest (in patch 15) tests user space split lock.
- perf traces two events exceptions:split_lock_user and exceptions:
  split_lock_kernel.
- S3, S4, S5, CPU hotplug, kexec tests with split lock eanbled.

Fenghua Yu (15):
  x86/split_lock: Add CONFIG and enumerate #AC exception for split
    locked access feature
  x86/split_lock: Set up #AC exception for split locked accesses
  x86/split_lock: Handle #AC exception for split lock in kernel mode
  x86/split_lock: Use non locked bit set instruction in set_cpu_cap
  x86/split_lock: Use non atomic set and clear bit instructions to clear
    cpufeature
  x86/split_lock: Save #AC setting for split lock in BIOS in boot time
    and restore the setting in reboot
  x86/split_lock: Handle suspend/hibernate and resume
  x86/split_lock: Set split lock during EFI runtime service
  x86/split_lock: Explicitly enable or disable #AC for split locked
    accesses
  x86/split_lock: Add a sysfs interface to allow user to enable or
    disable split lock during run time
  x86/split_lock: Add sysfs interface to control user mode behavior
  x86/split_lock: Add sysfs interface to show and control BIOS split
    lock setting
  x86/split_lock: Trace #AC exception for split lock
  x86/split_lock: Add CONFIG and testing sysfs interface
  x86/split_lock: Add split lock user space test in selftest

 Documentation/admin-guide/kernel-parameters.txt    |  11 +
 arch/x86/Kconfig                                   |  22 +
 arch/x86/include/asm/cpu.h                         |  14 +
 arch/x86/include/asm/cpufeature.h                  |   3 +-
 arch/x86/include/asm/efi.h                         |   5 +
 arch/x86/include/asm/msr-index.h                   |   5 +
 arch/x86/include/asm/trace/common.h                |   8 +
 arch/x86/include/asm/trace/exceptions.h            |  21 +-
 arch/x86/kernel/cpu/Makefile                       |   1 +
 arch/x86/kernel/cpu/common.c                       |   3 +
 arch/x86/kernel/cpu/cpuid-deps.c                   |  10 +-
 arch/x86/kernel/cpu/split_lock.c                   | 606 +++++++++++++++++++++
 arch/x86/kernel/traps.c                            |  12 +
 tools/testing/selftests/x86/Makefile               |   3 +-
 tools/testing/selftests/x86/split_lock_user_test.c | 187 +++++++
 15 files changed, 901 insertions(+), 10 deletions(-)
 create mode 100644 arch/x86/kernel/cpu/split_lock.c
 create mode 100644 tools/testing/selftests/x86/split_lock_user_test.c

-- 
2.5.0

[PATCH 01/15] x86/split_lock: Add CONFIG and enumerate #AC exception for split locked access feature

[Fenghua Yu]

Add CONFIG_SPLIT_LOCK_AC (default: y, dependent on X86 and CPU_SUP_INTEL)
to control inclusion of the feature.

Bit 29 in MSR TEST_CTL 0x33 can only be set on processors that support
the feature. On processors not supporting the feature, the bit is reserved
i.e. can not be set as one) or the MSR doesn't exist.

To detect the feature, attempt to set the bit in the MSR. If the writing
succeeds, the feature is available. Otherwise, the feature is not
supported on this platform.

And the enumeration happens before SMP so all processors can use
enumerated result when SMP boots.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/Kconfig                 | 12 ++++++++
 arch/x86/include/asm/cpu.h       |  5 ++++
 arch/x86/include/asm/msr-index.h |  5 ++++
 arch/x86/kernel/cpu/Makefile     |  1 +
 arch/x86/kernel/cpu/common.c     |  1 +
 arch/x86/kernel/cpu/split_lock.c | 62 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 86 insertions(+)
 create mode 100644 arch/x86/kernel/cpu/split_lock.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c07f492b871a..38baf5fb8556 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -448,6 +448,18 @@ config INTEL_RDT
 
 	  Say N if unsure.
 
+config SPLIT_LOCK_AC
+	bool "#AC exception for split locked accesses support"
+	default y
+	depends on X86 && CPU_SUP_INTEL
+	help
+	  Select to support #AC exception for split locked accesses. More
+	  detailed information about the feature can be found in
+	  Intel Architecture Instruction Set Extensions and Future Feature
+	  Programming Reference.
+
+	  Say N if unsure.
+
 if X86_32
 config X86_BIGSMP
 	bool "Support for big SMP systems with more than 8 CPUs"
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index adc6cc86b062..c73b6d369047 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -40,4 +40,9 @@ int mwait_usable(const struct cpuinfo_x86 *);
 unsigned int x86_family(unsigned int sig);
 unsigned int x86_model(unsigned int sig);
 unsigned int x86_stepping(unsigned int sig);
+#ifdef CONFIG_SPLIT_LOCK_AC
+int __init enumerate_split_lock(void);
+#else /* CONFIG_SPLIT_LOCK_AC */
+static inline int enumerate_split_lock(void) { return 0; }
+#endif /* CONFIG_SPLIT_LOCK_AC */
 #endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 53d5b1b9255e..c791190c8c71 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -39,6 +39,11 @@
 
 /* Intel MSRs. Some also available on other CPUs */
 
+#define MSR_TEST_CTL					0x00000033
+#define MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK_SHIFT		29
+#define MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK		BIT(29)
+#define MSR_TEST_CTL_DISABLE_LOCK_ASSERT_SPLIT_LOCK	BIT(31)
+
 #define MSR_IA32_SPEC_CTRL		0x00000048 /* Speculation Control */
 #define SPEC_CTRL_IBRS			(1 << 0)   /* Indirect Branch Restricted Speculation */
 #define SPEC_CTRL_STIBP			(1 << 1)   /* Single Thread Indirect Branch Predictors */
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index a66229f51b12..1b633450e372 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_MICROCODE)			+= microcode/
 obj-$(CONFIG_X86_LOCAL_APIC)		+= perfctr-watchdog.o
 
 obj-$(CONFIG_HYPERVISOR_GUEST)		+= vmware.o hypervisor.o mshyperv.o
+obj-$(CONFIG_SPLIT_LOCK_AC)		+= split_lock.o
 
 ifdef CONFIG_X86_FEATURE_NAMES
 quiet_cmd_mkcapflags = MKCAP   $@
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index ce243f7d2d4e..7684e82e254f 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1008,6 +1008,7 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
 	 */
 	setup_clear_cpu_cap(X86_FEATURE_PCID);
 #endif
+	enumerate_split_lock();
 }
 
 void __init early_cpu_init(void)
diff --git a/arch/x86/kernel/cpu/split_lock.c b/arch/x86/kernel/cpu/split_lock.c
new file mode 100644
index 000000000000..2ab28419e080
--- /dev/null
+++ b/arch/x86/kernel/cpu/split_lock.c
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Enable #AC exception for split locked accesses
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Author:
+ *	Fenghua Yu <fenghua.yu@intel.com>
+ */
+
+#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
+
+#include <linux/printk.h>
+#include <asm/msr.h>
+
+static bool split_lock_ac_supported;
+
+/*
+ * On processors not supporting #AC exception for split lock feature,
+ * MSR_TEST_CTL may not exist or MSR_TEST_CTL exists but the bit 29 is
+ * reserved.
+ *
+ * MSR_TEST_CTL exists and the bit 29 can be set as one only on processors
+ * that support the feature.
+ *
+ * To enumerate #AC exception for split lock feature, try to set bit 29 as
+ * one in MSR_TEST_CTL. If the bit is successfully set as 1, the feature
+ * is supported and the variable split_lock_ac_supported is set as true.
+ * Otherwise, the feature is not available.
+ */
+void __init enumerate_split_lock(void)
+{
+	u32 l, h, l_orig;
+	int ret;
+
+	/* Attempt to read the MSR. If the MSR doesn't exist, reading fails. */
+	ret = rdmsr_safe(MSR_TEST_CTL, &l, &h);
+	if (ret)
+		return;
+
+	l_orig = l;
+
+	/* Turn on the split lock bit */
+	l |= MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK;
+
+	/* Set the bit in the MSR */
+	ret = wrmsr_safe(MSR_TEST_CTL, l, h);
+	if (ret)
+		return;
+
+	/* When coming to here, the feature is supported on this platform. */
+	split_lock_ac_supported = true;
+
+	/*
+	 *
+	 * Need to restore split lock setting to original BIOS setting before
+	 * leaving.
+	 */
+	wrmsr(MSR_TEST_CTL, l_orig, h);
+
+	pr_info("#AC exception for split locked accesses is supported\n");
+}
-- 
2.5.0

[PATCH 02/15] x86/split_lock: Set up #AC exception for split locked accesses

[Fenghua Yu]

When bit 29 is set in Test Control MSR register 0x33, #AC exception
is generated for split locked accesses at all CPL.

By default, kernel inherits the bit setting from BIOS.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/cpu.h       |  2 ++
 arch/x86/kernel/cpu/common.c     |  2 ++
 arch/x86/kernel/cpu/split_lock.c | 54 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index c73b6d369047..b4fe6496bb15 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -42,7 +42,9 @@ unsigned int x86_model(unsigned int sig);
 unsigned int x86_stepping(unsigned int sig);
 #ifdef CONFIG_SPLIT_LOCK_AC
 int __init enumerate_split_lock(void);
+void setup_split_lock(void);
 #else /* CONFIG_SPLIT_LOCK_AC */
 static inline int enumerate_split_lock(void) { return 0; }
+static inline void setup_split_lock(void) {}
 #endif /* CONFIG_SPLIT_LOCK_AC */
 #endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 7684e82e254f..daff18ef4f8f 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1304,6 +1304,8 @@ static void identify_cpu(struct cpuinfo_x86 *c)
 	/* Init Machine Check Exception if available. */
 	mcheck_cpu_init(c);
 
+	setup_split_lock();
+
 	select_idle_routine(c);
 
 #ifdef CONFIG_NUMA
diff --git a/arch/x86/kernel/cpu/split_lock.c b/arch/x86/kernel/cpu/split_lock.c
index 2ab28419e080..98bbfb176cf4 100644
--- a/arch/x86/kernel/cpu/split_lock.c
+++ b/arch/x86/kernel/cpu/split_lock.c
@@ -15,6 +15,11 @@
 
 static bool split_lock_ac_supported;
 
+#define	DISABLE_SPLIT_LOCK_AC		0
+#define	ENABLE_SPLIT_LOCK_AC		1
+
+static int split_lock_ac = DISABLE_SPLIT_LOCK_AC;
+
 /*
  * On processors not supporting #AC exception for split lock feature,
  * MSR_TEST_CTL may not exist or MSR_TEST_CTL exists but the bit 29 is
@@ -58,5 +63,54 @@ void __init enumerate_split_lock(void)
 	 */
 	wrmsr(MSR_TEST_CTL, l_orig, h);
 
+	/* Initialize split lock setting from previous BIOS setting. */
+	if (l_orig & MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK)
+		split_lock_ac = ENABLE_SPLIT_LOCK_AC;
+	else
+		split_lock_ac = DISABLE_SPLIT_LOCK_AC;
+
 	pr_info("#AC exception for split locked accesses is supported\n");
 }
+
+static bool _setup_split_lock(int split_lock_ac_val)
+{
+	u32 l, h;
+
+	rdmsr(MSR_TEST_CTL, l, h);
+
+	/* No need to update MSR if same value. */
+	if ((l >> MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK_SHIFT & 0x1) ==
+	    split_lock_ac_val)
+		goto out;
+
+	if (split_lock_ac_val == ENABLE_SPLIT_LOCK_AC)
+		/* Set the split lock bit to enable the feature. */
+		l |= MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK;
+	else if (split_lock_ac_val == DISABLE_SPLIT_LOCK_AC)
+		/* Clear the split lock bit to disable the feature. */
+		l &= ~MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK;
+	else
+		return false;
+
+	wrmsr(MSR_TEST_CTL, l, h);
+out:
+	return true;
+}
+
+void setup_split_lock(void)
+{
+	if (!split_lock_ac_supported)
+		return;
+
+	if (!_setup_split_lock(split_lock_ac))
+		goto out_fail;
+
+	pr_info_once("split lock #AC is %sd\n",
+		     split_lock_ac == ENABLE_SPLIT_LOCK_AC ? "enable"
+		     : "disable");
+
+	return;
+
+out_fail:
+	pr_warn("fail to set split lock #AC\n");
+}
-- 
2.5.0

[PATCH 03/15] x86/split_lock: Handle #AC exception for split lock in kernel mode

[Fenghua Yu]

When #AC exception for split lock happens in kernel code, disable
further #AC exception for split lock in the handler. Then the
faulting instruction is re-executed after exiting from the handler
without triggering another #AC exception. Re-enable #AC exception
for split lock later (after 1 ms).

During the period between disabling and re-enabling #AC exception for
split lock, some split locked accesses may not be captured. And since
the MSR_TEST_CTL is per core, disabling #AC exception for split
lock on one thread disables the feature on all threads in the
same core.

Although it's not an accurate way, the delayed re-enabling code is
simpler and cleaner than another possible method which sets single
step execution in the #AC exception and re-enables #AC for split lock
in debug trap triggered by the next instruction after the faulting
instruction. The delayed re-enabling code can prevent split lock #AC
flood caused by a lot split locks in short time (e.g. faulting
instruction in a loop). And there is no missing split lock because
the following few blocked split locks will show up once the first split
lock issue is fixed

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/cpu.h       |  3 ++
 arch/x86/kernel/cpu/split_lock.c | 67 +++++++++++++++++++++++++++++++++++++++-
 arch/x86/kernel/traps.c          | 12 +++++++
 3 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index b4fe6496bb15..80dc27d73e81 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -43,8 +43,11 @@ unsigned int x86_stepping(unsigned int sig);
 #ifdef CONFIG_SPLIT_LOCK_AC
 int __init enumerate_split_lock(void);
 void setup_split_lock(void);
+bool do_split_lock_exception(struct pt_regs *regs, unsigned long error_code);
 #else /* CONFIG_SPLIT_LOCK_AC */
 static inline int enumerate_split_lock(void) { return 0; }
 static inline void setup_split_lock(void) {}
+static inline bool
+do_split_lock_exception(struct pt_regs *regs, unsigned long error_code) {}
 #endif /* CONFIG_SPLIT_LOCK_AC */
 #endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/kernel/cpu/split_lock.c b/arch/x86/kernel/cpu/split_lock.c
index 98bbfb176cf4..efe6f39353d1 100644
--- a/arch/x86/kernel/cpu/split_lock.c
+++ b/arch/x86/kernel/cpu/split_lock.c
@@ -11,6 +11,8 @@
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
 #include <linux/printk.h>
+#include <linux/workqueue.h>
+#include <linux/cpu.h>
 #include <asm/msr.h>
 
 static bool split_lock_ac_supported;
@@ -20,6 +22,11 @@ static bool split_lock_ac_supported;
 
 static int split_lock_ac = DISABLE_SPLIT_LOCK_AC;
 
+static DEFINE_SPINLOCK(sl_lock);
+
+static void delayed_reenable_split_lock(struct work_struct *w);
+static DECLARE_DELAYED_WORK(delayed_work, delayed_reenable_split_lock);
+
 /*
  * On processors not supporting #AC exception for split lock feature,
  * MSR_TEST_CTL may not exist or MSR_TEST_CTL exists but the bit 29 is
@@ -76,6 +83,13 @@ static bool _setup_split_lock(int split_lock_ac_val)
 {
 	u32 l, h;
 
+	/*
+	 * The MSR_TEST_CTL is per core.
+	 *
+	 * We use spin lock to solve race condition when multiple threads
+	 * on the same core generate #AC exception at the same time.
+	 */
+	spin_lock(&sl_lock);
 	rdmsr(MSR_TEST_CTL, l, h);
 
 	/* No need to update MSR if same value. */
@@ -90,11 +104,17 @@ static bool _setup_split_lock(int split_lock_ac_val)
 		/* Clear the split lock bit to disable the feature. */
 		l &= ~MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK;
 	else
-		return false;
+		goto out_fail;
 
 	wrmsr(MSR_TEST_CTL, l, h);
 out:
+	spin_unlock(&sl_lock);
+
 	return true;
+out_fail:
+	spin_unlock(&sl_lock);
+
+	return false;
 }
 
 void setup_split_lock(void)
@@ -114,3 +134,48 @@ void setup_split_lock(void)
 out_fail:
 	pr_warn("fail to set split lock #AC\n");
 }
+
+#define	delay_ms	1
+
+static void delayed_reenable_split_lock(struct work_struct *w)
+{
+	if (split_lock_ac == ENABLE_SPLIT_LOCK_AC)
+		_setup_split_lock(ENABLE_SPLIT_LOCK_AC);
+}
+
+/* Will the faulting instruction be re-executed? */
+static bool re_execute(struct pt_regs *regs)
+{
+	/*
+	 * The only reason for generating #AC from kernel is because of
+	 * split lock. The kernel faulting instruction will be re-executed.
+	 */
+	if (!user_mode(regs))
+		return true;
+
+	return false;
+}
+
+/*
+ * #AC handler for kernel split lock is called by generic #AC handler.
+ *
+ * Disable #AC for split lock on this CPU so that the faulting instruction
+ * gets executed. The #AC for split lock is re-enabled later.
+ */
+bool do_split_lock_exception(struct pt_regs *regs, unsigned long error_code)
+{
+	unsigned long delay = msecs_to_jiffies(delay_ms);
+	unsigned long address = read_cr2(); /* Get the faulting address */
+	int this_cpu = smp_processor_id();
+
+	if (!re_execute(regs))
+		return false;
+
+	pr_info_ratelimited("Alignment check for split lock at %lx\n", address);
+
+	_setup_split_lock(DISABLE_SPLIT_LOCK_AC);
+
+	schedule_delayed_work_on(this_cpu, &delayed_work, delay);
+
+	return true;
+}
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 03f3d7695dac..c07b817bbbe9 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -61,6 +61,7 @@
 #include <asm/mpx.h>
 #include <asm/vm86.h>
 #include <asm/umip.h>
+#include <asm/cpu.h>
 
 #ifdef CONFIG_X86_64
 #include <asm/x86_init.h>
@@ -286,10 +287,21 @@ static void do_error_trap(struct pt_regs *regs, long error_code, char *str,
 			  unsigned long trapnr, int signr)
 {
 	siginfo_t info;
+	int ret;
 
 	RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
 
 	/*
+	 * #AC exception could be handled by split lock handler.
+	 * If the handler can't handle the exception, go to generic #AC handler.
+	 */
+	if (trapnr == X86_TRAP_AC) {
+		ret = do_split_lock_exception(regs, error_code);
+		if (ret)
+			return;
+	}
+
+	/*
 	 * WARN*()s end up here; fix them up before we call the
 	 * notifier chain.
 	 */
-- 
2.5.0

[PATCH 04/15] x86/split_lock: Use non locked bit set instruction in set_cpu_cap

[Fenghua Yu]

set_bit() called by set_cpu_cap() is a locked bit set instruction for
atomic operation.

Since the c->x86_capability is not aligned to cache line depending on
compiler, the locked bit set instruction falls into split lock
situation which causes #AC exception when #AC exception is enabled by
split locked accesses.

But set_cpu_cap() is only called in early init phase on a CPU and
therefore there is no contention for set_cpu_cap(). So it's unnecessary
to be atomic operation.

Using __set_bit(), which is not a locked instruction, can fix the
kernel split lock issue and is faster than locked instruction.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Acked-by: Dave Hansen <dave.hansen@intel.com>
---
 arch/x86/include/asm/cpufeature.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index b27da9602a6d..604f4c514efa 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -128,7 +128,8 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
 
 #define boot_cpu_has(bit)	cpu_has(&boot_cpu_data, bit)
 
-#define set_cpu_cap(c, bit)	set_bit(bit, (unsigned long *)((c)->x86_capability))
+#define set_cpu_cap(c, bit)	\
+	__set_bit(bit, (unsigned long *)((c)->x86_capability))
 
 extern void setup_clear_cpu_cap(unsigned int bit);
 extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);
-- 
2.5.0

[PATCH 05/15] x86/split_lock: Use non atomic set and clear bit instructions to clear cpufeature

[Fenghua Yu]

When #AC exception is enabled for split locked accesses,
clear_cpufeature() generates #AC exception because of atomic setting
or clearing bits in non cache line aligned x86_capability depending
on compiler.

But kernel clears cpufeatures only when a CPU boots up. Therefore, there
is no racing condition when clear_cpufeature() is called and no need to
atomically clear or set x86_capability.

To avoid #AC exception caused by split lock and get better performance,
call non atomic __set_bit() and __clear_bit().

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/cpuid-deps.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 2c0bd38a44ab..0c02c6e44ea7 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -65,15 +65,15 @@ static const struct cpuid_dep cpuid_deps[] = {
 static inline void clear_feature(struct cpuinfo_x86 *c, unsigned int feature)
 {
 	/*
-	 * Note: This could use the non atomic __*_bit() variants, but the
-	 * rest of the cpufeature code uses atomics as well, so keep it for
-	 * consistency. Cleanup all of it separately.
+	 * Because this code is only called during boot time and there
+	 * is no need to be atomic, use non atomic __*_bit() to avoid
+	 * #AC exception for split locked access.
 	 */
 	if (!c) {
 		clear_cpu_cap(&boot_cpu_data, feature);
-		set_bit(feature, (unsigned long *)cpu_caps_cleared);
+		__set_bit(feature, (unsigned long *)cpu_caps_cleared);
 	} else {
-		clear_bit(feature, (unsigned long *)c->x86_capability);
+		__clear_bit(feature, (unsigned long *)c->x86_capability);
 	}
 }
 
-- 
2.5.0

[PATCH 06/15] x86/split_lock: Save #AC setting for split lock in BIOS in boot time and restore the setting in reboot

[Fenghua Yu]

BIOS may contain split locked instructions. #AC handler in BIOS may
treat split lock as fatal fault and stop execution. If kernel enables
#AC exception for split locked accesses and then kernel returns to BIOS,
the BIOS reboot code may hit #AC exception and block the reboot.

Instead of debugging the buggy BIOS, #AC setting for split lock is
restored to original BIOS setting to hide the potential BIOS issue and
allow kernel reboot succeed.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/cpu.h       |  2 ++
 arch/x86/kernel/cpu/split_lock.c | 56 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 80dc27d73e81..0b00033b6fa8 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -44,10 +44,12 @@ unsigned int x86_stepping(unsigned int sig);
 int __init enumerate_split_lock(void);
 void setup_split_lock(void);
 bool do_split_lock_exception(struct pt_regs *regs, unsigned long error_code);
+bool restore_split_lock_ac_bios(int *enable);
 #else /* CONFIG_SPLIT_LOCK_AC */
 static inline int enumerate_split_lock(void) { return 0; }
 static inline void setup_split_lock(void) {}
 static inline bool
 do_split_lock_exception(struct pt_regs *regs, unsigned long error_code) {}
+static inline bool restore_split_lock_ac_bios(int *enable) { return true; }
 #endif /* CONFIG_SPLIT_LOCK_AC */
 #endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/kernel/cpu/split_lock.c b/arch/x86/kernel/cpu/split_lock.c
index efe6f39353d1..d2735259800b 100644
--- a/arch/x86/kernel/cpu/split_lock.c
+++ b/arch/x86/kernel/cpu/split_lock.c
@@ -13,6 +13,7 @@
 #include <linux/printk.h>
 #include <linux/workqueue.h>
 #include <linux/cpu.h>
+#include <linux/reboot.h>
 #include <asm/msr.h>
 
 static bool split_lock_ac_supported;
@@ -21,6 +22,7 @@ static bool split_lock_ac_supported;
 #define	ENABLE_SPLIT_LOCK_AC		1
 
 static int split_lock_ac = DISABLE_SPLIT_LOCK_AC;
+static int split_lock_ac_bios = DISABLE_SPLIT_LOCK_AC;
 
 static DEFINE_SPINLOCK(sl_lock);
 
@@ -71,10 +73,13 @@ void __init enumerate_split_lock(void)
 	wrmsr(MSR_TEST_CTL, l_orig, h);
 
 	/* Initialize split lock setting from previous BIOS setting. */
-	if (l_orig & MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK)
+	if (l_orig & MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK) {
+		split_lock_ac_bios = ENABLE_SPLIT_LOCK_AC;
 		split_lock_ac = ENABLE_SPLIT_LOCK_AC;
-	else
+	} else {
+		split_lock_ac_bios = DISABLE_SPLIT_LOCK_AC;
 		split_lock_ac = DISABLE_SPLIT_LOCK_AC;
+	}
 
 	pr_info("#AC exception for split locked accesses is supported\n");
 }
@@ -117,6 +122,44 @@ static bool _setup_split_lock(int split_lock_ac_val)
 	return false;
 }
 
+static bool restore_split_lock_ac(int split_lock_ac_val)
+{
+	if (!_setup_split_lock(split_lock_ac_val))
+		return false;
+
+	return true;
+}
+
+/* Restore BIOS setting for #AC exception for split lock. */
+bool restore_split_lock_ac_bios(int *enable)
+{
+	/* Don't restore the BIOS setting if kernel didn't change it. */
+	if (split_lock_ac == split_lock_ac_bios)
+		return false;
+
+	if (enable)
+		*enable = split_lock_ac_bios == ENABLE_SPLIT_LOCK_AC ? 1 : 0;
+
+	return restore_split_lock_ac(split_lock_ac_bios);
+}
+
+static void split_lock_cpu_reboot(void *unused)
+{
+	restore_split_lock_ac_bios(NULL);
+}
+
+static int split_lock_reboot_notify(struct notifier_block *nb,
+				    unsigned long code, void *unused)
+{
+	on_each_cpu_mask(cpu_online_mask, split_lock_cpu_reboot, NULL, 1);
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block split_lock_reboot_nb = {
+	.notifier_call = split_lock_reboot_notify,
+};
+
 void setup_split_lock(void)
 {
 	if (!split_lock_ac_supported)
@@ -179,3 +222,12 @@ bool do_split_lock_exception(struct pt_regs *regs, unsigned long error_code)
 
 	return true;
 }
+
+static int __init split_lock_init(void)
+{
+	register_reboot_notifier(&split_lock_reboot_nb);
+
+	return 0;
+}
+
+late_initcall(split_lock_init);
-- 
2.5.0

[PATCH 07/15] x86/split_lock: Handle suspend/hibernate and resume

[Fenghua Yu]

During suspend or hibernation, system enters BIOS. To avoid potential
BIOS issue that may generate #AC exception for split locked accesses,
handle the #AC as fatal exception, and block suspend or hibernation,
the split lock setting is restored to BIOS setting. When resuming
from suspend or hibernation, the split lock setting is restored to
kernel setting.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/cpu.h       |  2 ++
 arch/x86/kernel/cpu/split_lock.c | 54 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 0b00033b6fa8..89d62d7051fa 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -45,11 +45,13 @@ int __init enumerate_split_lock(void);
 void setup_split_lock(void);
 bool do_split_lock_exception(struct pt_regs *regs, unsigned long error_code);
 bool restore_split_lock_ac_bios(int *enable);
+bool restore_split_lock_ac_kernel(int *enable);
 #else /* CONFIG_SPLIT_LOCK_AC */
 static inline int enumerate_split_lock(void) { return 0; }
 static inline void setup_split_lock(void) {}
 static inline bool
 do_split_lock_exception(struct pt_regs *regs, unsigned long error_code) {}
 static inline bool restore_split_lock_ac_bios(int *enable) { return true; }
+static inline bool restore_split_lock_ac_kernel(int *enable) { return true; }
 #endif /* CONFIG_SPLIT_LOCK_AC */
 #endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/kernel/cpu/split_lock.c b/arch/x86/kernel/cpu/split_lock.c
index d2735259800b..5187a9c6cea6 100644
--- a/arch/x86/kernel/cpu/split_lock.c
+++ b/arch/x86/kernel/cpu/split_lock.c
@@ -14,6 +14,7 @@
 #include <linux/workqueue.h>
 #include <linux/cpu.h>
 #include <linux/reboot.h>
+#include <linux/syscore_ops.h>
 #include <asm/msr.h>
 
 static bool split_lock_ac_supported;
@@ -143,6 +144,15 @@ bool restore_split_lock_ac_bios(int *enable)
 	return restore_split_lock_ac(split_lock_ac_bios);
 }
 
+/* Restore kernel setting for #AC enable bit for split lock. */
+bool restore_split_lock_ac_kernel(int *enable)
+{
+	if (enable)
+		*enable = split_lock_ac == ENABLE_SPLIT_LOCK_AC ?  1 : 0;
+
+	return restore_split_lock_ac(split_lock_ac);
+}
+
 static void split_lock_cpu_reboot(void *unused)
 {
 	restore_split_lock_ac_bios(NULL);
@@ -223,11 +233,55 @@ bool do_split_lock_exception(struct pt_regs *regs, unsigned long error_code)
 	return true;
 }
 
+static int split_lock_offline(unsigned int cpu)
+{
+	int enable;
+
+	if (restore_split_lock_ac_bios(&enable))
+		pr_info("%s split lock on CPU%d\n",
+			enable ? "enable" : "disable", smp_processor_id());
+
+	return 0;
+}
+
+static int split_lock_bsp_suspend(void)
+{
+	restore_split_lock_ac_bios(NULL);
+
+	return 0;
+}
+
+static void split_lock_bsp_resume(void)
+{
+	restore_split_lock_ac_kernel(NULL);
+}
+
+static struct syscore_ops split_lock_syscore_ops = {
+	.suspend	= split_lock_bsp_suspend,
+	.resume		= split_lock_bsp_resume,
+};
+
 static int __init split_lock_init(void)
 {
+	int ret;
+
+	if (!split_lock_ac_supported)
+		return -ENODEV;
+
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
+				"x86/split_lock:online:",
+				NULL, split_lock_offline);
+	if (ret < 0)
+		goto out_fail;
+
+	register_syscore_ops(&split_lock_syscore_ops);
+
 	register_reboot_notifier(&split_lock_reboot_nb);
 
 	return 0;
+
+out_fail:
+	return ret;
 }
 
 late_initcall(split_lock_init);
-- 
2.5.0

[PATCH 08/15] x86/split_lock: Set split lock during EFI runtime service

[Fenghua Yu]

If kernel enables #AC for split locked accesses, EFI runtime service may
hit #AC exception, treat it as fatal fault, and cause system hang.

To avoid debugging potential buggy EFI runtime service code in BIOS,
restore to BIOS split lock setting before entering EFI runtime service
and restore to kernel split lock setting after exiting EFI runtime service.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/efi.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index cec5fae23eb3..6dcd0717a3cf 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -8,6 +8,7 @@
 #include <asm/tlb.h>
 #include <asm/nospec-branch.h>
 #include <asm/mmu_context.h>
+#include <asm/cpu.h>
 
 /*
  * We map the EFI regions needed for runtime services non-contiguously,
@@ -41,12 +42,14 @@ extern asmlinkage unsigned long efi_call_phys(void *, ...);
 #define arch_efi_call_virt_setup()					\
 ({									\
 	kernel_fpu_begin();						\
+	restore_split_lock_ac_bios(NULL);				\
 	firmware_restrict_branch_speculation_start();			\
 })
 
 #define arch_efi_call_virt_teardown()					\
 ({									\
 	firmware_restrict_branch_speculation_end();			\
+	restore_split_lock_ac_kernel(NULL);				\
 	kernel_fpu_end();						\
 })
 
@@ -84,6 +87,7 @@ struct efi_scratch {
 	efi_sync_low_kernel_mappings();					\
 	preempt_disable();						\
 	__kernel_fpu_begin();						\
+	restore_split_lock_ac_bios(NULL);				\
 	firmware_restrict_branch_speculation_start();			\
 									\
 	if (!efi_enabled(EFI_OLD_MEMMAP))				\
@@ -99,6 +103,7 @@ struct efi_scratch {
 		efi_switch_mm(efi_scratch.prev_mm);			\
 									\
 	firmware_restrict_branch_speculation_end();			\
+	restore_split_lock_ac_kernel(NULL);				\
 	__kernel_fpu_end();						\
 	preempt_enable();						\
 })
-- 
2.5.0

[PATCH 09/15] x86/split_lock: Explicitly enable or disable #AC for split locked accesses

[Fenghua Yu]

By default, we don't set or clear the bit 29 in TEST_CTL MSR 0x33 and
the bit is inherited from BIOS/hardware setting.

The kernel parameter "split_lock_ac=on/off" explicitly sets or clears
the bit during boot time.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 11 +++++++++++
 arch/x86/kernel/cpu/split_lock.c                | 16 ++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 11fc28ecdb6d..862758533346 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4030,6 +4030,17 @@
 	spia_pedr=
 	spia_peddr=
 
+	split_lock_ac=	[X86] Enable or disable #AC exception for
+			split locked accesses
+			By default, kernel just inherits setting by
+			hardware and BIOS which may enable or disable
+			the feature. User can explicitly enable or disable
+			the feature by this parameter to override
+			BIOS/hardware setting.
+
+		on	Enable #AC exception for split locked accesses.
+		off	Disable #AC exception for split locked accesses.
+
 	srcutree.counter_wrap_check [KNL]
 			Specifies how frequently to check for
 			grace-period sequence counter wrap for the
diff --git a/arch/x86/kernel/cpu/split_lock.c b/arch/x86/kernel/cpu/split_lock.c
index 5187a9c6cea6..bc66c21b0ca9 100644
--- a/arch/x86/kernel/cpu/split_lock.c
+++ b/arch/x86/kernel/cpu/split_lock.c
@@ -85,6 +85,22 @@ void __init enumerate_split_lock(void)
 	pr_info("#AC exception for split locked accesses is supported\n");
 }
 
+static __init int setup_split_lock_ac(char *str)
+{
+	if (!str)
+		return -EINVAL;
+
+	if (!strncmp(str, "on", 2))
+		split_lock_ac = ENABLE_SPLIT_LOCK_AC;
+	else if (!strncmp(str, "off", 3))
+		split_lock_ac = DISABLE_SPLIT_LOCK_AC;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+__setup("split_lock_ac=", setup_split_lock_ac);
+
 static bool _setup_split_lock(int split_lock_ac_val)
 {
 	u32 l, h;
-- 
2.5.0

[PATCH 10/15] x86/split_lock: Add a sysfs interface to allow user to enable or disable split lock during run time

[Fenghua Yu]

The interface /sys/kernel/split_lock/enable is added to allow user to
control split lock and show current split lock status during run time.

Writing 1 to the file enables split lock and writing 0 disables split
lock.

Reading the file shows current split lock eanble/disable status:
disabled when 0 and enabled when 1.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/split_lock.c | 65 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 64 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/split_lock.c b/arch/x86/kernel/cpu/split_lock.c
index bc66c21b0ca9..5d399b09c1c8 100644
--- a/arch/x86/kernel/cpu/split_lock.c
+++ b/arch/x86/kernel/cpu/split_lock.c
@@ -30,6 +30,8 @@ static DEFINE_SPINLOCK(sl_lock);
 static void delayed_reenable_split_lock(struct work_struct *w);
 static DECLARE_DELAYED_WORK(delayed_work, delayed_reenable_split_lock);
 
+static DEFINE_MUTEX(split_lock_mutex);
+
 /*
  * On processors not supporting #AC exception for split lock feature,
  * MSR_TEST_CTL may not exist or MSR_TEST_CTL exists but the bit 29 is
@@ -277,6 +279,60 @@ static struct syscore_ops split_lock_syscore_ops = {
 	.resume		= split_lock_bsp_resume,
 };
 
+static ssize_t
+enable_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%u\n", split_lock_ac);
+}
+
+static ssize_t enable_store(struct kobject *kobj, struct kobj_attribute *attr,
+			    const char *buf, size_t count)
+{
+	u32 val, l, h;
+	int cpu, ret;
+
+	ret = kstrtou32(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	if (val != DISABLE_SPLIT_LOCK_AC && val != ENABLE_SPLIT_LOCK_AC)
+		return -EINVAL;
+
+	/* No need to update MSR if new setting is the same as old one. */
+	if (val == split_lock_ac)
+		return count;
+
+	mutex_lock(&split_lock_mutex);
+
+	split_lock_ac = val;
+	/* Read split lock setting on the current CPU. */
+	rdmsr(MSR_TEST_CTL, l, h);
+	/* Change the split lock setting. */
+	if (split_lock_ac == DISABLE_SPLIT_LOCK_AC)
+		l &= ~MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK;
+	else
+		l |= MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK;
+	/* Update the split lock setting on all online CPUs. */
+	for_each_online_cpu(cpu)
+		wrmsr_on_cpu(cpu, MSR_TEST_CTL, l, h);
+
+	mutex_unlock(&split_lock_mutex);
+
+	return count;
+}
+
+static struct kobj_attribute split_lock_ac_enable = __ATTR_RW(enable);
+
+static struct attribute *split_lock_attrs[] = {
+	&split_lock_ac_enable.attr,
+	NULL,
+};
+
+static struct attribute_group split_lock_attr_group = {
+	.attrs		= split_lock_attrs,
+	.name		= "split_lock",
+};
+
 static int __init split_lock_init(void)
 {
 	int ret;
@@ -284,11 +340,15 @@ static int __init split_lock_init(void)
 	if (!split_lock_ac_supported)
 		return -ENODEV;
 
+	ret = sysfs_create_group(kernel_kobj, &split_lock_attr_group);
+	if (ret)
+		goto out_fail;
+
 	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
 				"x86/split_lock:online:",
 				NULL, split_lock_offline);
 	if (ret < 0)
-		goto out_fail;
+		goto out_group;
 
 	register_syscore_ops(&split_lock_syscore_ops);
 
@@ -296,6 +356,9 @@ static int __init split_lock_init(void)
 
 	return 0;
 
+out_group:
+	sysfs_remove_group(kernel_kobj, &split_lock_attr_group);
+
 out_fail:
 	return ret;
 }
-- 
2.5.0

[PATCH 11/15] x86/split_lock: Add sysfs interface to control user mode behavior

[Fenghua Yu]

Add the interface /sys/kernel/split_lock/user_mode to allow user to
choose to either generate SIGBUS (default) when hitting split lock in
user or re-execute the user faulting instruction without generating
SIGBUS signal.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/split_lock.c | 89 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/arch/x86/kernel/cpu/split_lock.c b/arch/x86/kernel/cpu/split_lock.c
index 5d399b09c1c8..02b461c48b3c 100644
--- a/arch/x86/kernel/cpu/split_lock.c
+++ b/arch/x86/kernel/cpu/split_lock.c
@@ -32,6 +32,19 @@ static DECLARE_DELAYED_WORK(delayed_work, delayed_reenable_split_lock);
 
 static DEFINE_MUTEX(split_lock_mutex);
 
+enum {
+	USER_MODE_SIGBUS,
+	USER_MODE_RE_EXECUTE,
+	USER_MODE_LAST
+};
+
+static int user_mode_reaction = USER_MODE_SIGBUS;
+
+static const char * const user_modes[USER_MODE_LAST] = {
+	[USER_MODE_SIGBUS]     = "sigbus",
+	[USER_MODE_RE_EXECUTE] = "re-execute",
+};
+
 /*
  * On processors not supporting #AC exception for split lock feature,
  * MSR_TEST_CTL may not exist or MSR_TEST_CTL exists but the bit 29 is
@@ -214,6 +227,16 @@ static void delayed_reenable_split_lock(struct work_struct *w)
 		_setup_split_lock(ENABLE_SPLIT_LOCK_AC);
 }
 
+static unsigned long eflags_ac(struct pt_regs *regs)
+{
+	return regs->flags & X86_EFLAGS_AC;
+}
+
+static unsigned long cr0_am(struct pt_regs *regs)
+{
+	return read_cr0() & X86_CR0_AM;
+}
+
 /* Will the faulting instruction be re-executed? */
 static bool re_execute(struct pt_regs *regs)
 {
@@ -224,6 +247,24 @@ static bool re_execute(struct pt_regs *regs)
 	if (!user_mode(regs))
 		return true;
 
+	/*
+	 * Now check if the user faulting instruction can be re-executed.
+	 *
+	 * If both CR0.AM (Alignment Mask) and EFLAGS.AC (Alignment Check)
+	 * are set in user space, any misalignment including split lock
+	 * can trigger #AC. In this case, we just issue SIGBUS as standard
+	 * #AC handler to the user process because split lock is not the
+	 * definite reason for triggering this #AC.
+	 *
+	 * If either CR0.AM or EFLAGS.AC is zero, the only reason for
+	 * triggering this #AC is split lock. So the faulting instruction
+	 * can be re-executed if required by user.
+	 */
+	if (cr0_am(regs) == 0 || eflags_ac(regs) == 0)
+		/* User faulting instruction will be re-executed if required. */
+		if (user_mode_reaction == USER_MODE_RE_EXECUTE)
+			return true;
+
 	return false;
 }
 
@@ -323,8 +364,56 @@ static ssize_t enable_store(struct kobject *kobj, struct kobj_attribute *attr,
 
 static struct kobj_attribute split_lock_ac_enable = __ATTR_RW(enable);
 
+static ssize_t
+user_mode_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	char *s = buf;
+	int reaction;
+
+	for (reaction = 0; reaction < USER_MODE_LAST; reaction++) {
+		if (reaction == user_mode_reaction)
+			s += sprintf(s, "[%s] ", user_modes[reaction]);
+		else
+			s += sprintf(s, "%s ", user_modes[reaction]);
+	}
+
+	if (s != buf)
+		/* convert the last space to a newline */
+		*(s - 1) = '\n';
+
+	return s - buf;
+}
+
+static ssize_t
+user_mode_store(struct kobject *kobj, struct kobj_attribute *attr,
+		const char *buf, size_t count)
+{
+	int reaction, len, error = -EINVAL;
+	const char * const *s, *p;
+
+	p = memchr(buf, '\n', count);
+	len = p ? p - buf : count;
+
+	mutex_lock(&split_lock_mutex);
+	reaction = USER_MODE_SIGBUS;
+	for (s = &user_modes[reaction]; reaction <= USER_MODE_LAST;
+	     s++, reaction++) {
+		if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) {
+			user_mode_reaction = reaction;
+			error = 0;
+			break;
+		}
+	}
+	mutex_unlock(&split_lock_mutex);
+
+	return error ? error : count;
+}
+
+static struct kobj_attribute split_lock_ac_user = __ATTR_RW(user_mode);
+
 static struct attribute *split_lock_attrs[] = {
 	&split_lock_ac_enable.attr,
+	&split_lock_ac_user.attr,
 	NULL,
 };
 
-- 
2.5.0

[PATCH 12/15] x86/split_lock: Add sysfs interface to show and control BIOS split lock setting

[Fenghua Yu]

By default, the split lock BIOS setting inherits from BIOS setting
before kernel boots.

The sysfs interface /sys/kernel/split_lock/bios shows the BIOS split
lock setting: 0 for disabled and 1 for enabled.

User can override the BIOS setting by writing 1 to enable split
lock in BIOS and write 0 to disable split lock in BIOS.

When control flow comes to BIOS (e.g. in S3, S4, S5, and EFI runtime),
kernel sets the BIOS setting and restores to kernel setting after
coming back to kernel.

In cases (e.g. real time) user wants to identify split lock cases in BIOS
even when there is no BIOS setting before kernel boot, the user may
explicitly enable split lock for BIOS. Getting bang whenever there is
a split lock in BIOS helps identify and fix the BIOS split lock issue.

Please note: System Management Mode (SMM) is out of control of
kernel. So this interface cannot control split lock in SMM.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/split_lock.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/x86/kernel/cpu/split_lock.c b/arch/x86/kernel/cpu/split_lock.c
index 02b461c48b3c..020af331594d 100644
--- a/arch/x86/kernel/cpu/split_lock.c
+++ b/arch/x86/kernel/cpu/split_lock.c
@@ -411,9 +411,43 @@ user_mode_store(struct kobject *kobj, struct kobj_attribute *attr,
 
 static struct kobj_attribute split_lock_ac_user = __ATTR_RW(user_mode);
 
+static ssize_t
+bios_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%u\n", split_lock_ac_bios);
+}
+
+static ssize_t
+bios_store(struct kobject *kobj, struct kobj_attribute *attr,
+	   const char *buf, size_t count)
+{
+	u32 val;
+	int ret;
+
+	ret = kstrtou32(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	if (val != DISABLE_SPLIT_LOCK_AC && val != ENABLE_SPLIT_LOCK_AC)
+		return -EINVAL;
+
+	/* No need to update setting if new setting is the same as old one. */
+	if (val == split_lock_ac_bios)
+		return count;
+
+	mutex_lock(&split_lock_mutex);
+	split_lock_ac_bios = val;
+	mutex_unlock(&split_lock_mutex);
+
+	return count;
+}
+
+static struct kobj_attribute split_lock_ac_bios_enable = __ATTR_RW(bios);
+
 static struct attribute *split_lock_attrs[] = {
 	&split_lock_ac_enable.attr,
 	&split_lock_ac_user.attr,
+	&split_lock_ac_bios_enable.attr,
 	NULL,
 };
 
-- 
2.5.0

[PATCH 13/15] x86/split_lock: Trace #AC exception for split lock

[Fenghua Yu]

Create two events x86_exceptions.split_lock_user to trace #AC
exception for split lock triggered from user space and
x86_exceptions.split_lock_kernel to trace #AC exception for split
lock from triggered from kernel space.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/trace/common.h     |  8 ++++++
 arch/x86/include/asm/trace/exceptions.h | 21 ++++++++++++---
 arch/x86/kernel/cpu/split_lock.c        | 46 +++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/trace/common.h b/arch/x86/include/asm/trace/common.h
index 57c8da027d99..5e61c7348349 100644
--- a/arch/x86/include/asm/trace/common.h
+++ b/arch/x86/include/asm/trace/common.h
@@ -8,9 +8,17 @@ DECLARE_STATIC_KEY_FALSE(trace_pagefault_key);
 DECLARE_STATIC_KEY_FALSE(trace_resched_ipi_key);
 #define trace_resched_ipi_enabled()			\
 	static_branch_unlikely(&trace_resched_ipi_key)
+
+#ifdef CONFIG_SPLIT_LOCK_AC
+DECLARE_STATIC_KEY_FALSE(trace_split_lock_key);
+#define trace_split_lock_enabled()			\
+	static_branch_unlikely(&trace_split_lock_key)
+#endif
+
 #else
 static inline bool trace_pagefault_enabled(void) { return false; }
 static inline bool trace_resched_ipi_enabled(void) { return false; }
+static inline bool trace_split_lock_enabled(void) { return false; }
 #endif
 
 #endif
diff --git a/arch/x86/include/asm/trace/exceptions.h b/arch/x86/include/asm/trace/exceptions.h
index 69615e387973..2a4ea0c70963 100644
--- a/arch/x86/include/asm/trace/exceptions.h
+++ b/arch/x86/include/asm/trace/exceptions.h
@@ -2,8 +2,8 @@
 #undef TRACE_SYSTEM
 #define TRACE_SYSTEM exceptions
 
-#if !defined(_TRACE_PAGE_FAULT_H) || defined(TRACE_HEADER_MULTI_READ)
-#define _TRACE_PAGE_FAULT_H
+#if !defined(_TRACE_EXCEPTIONS_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_EXCEPTIONS_H
 
 #include <linux/tracepoint.h>
 #include <asm/trace/common.h>
@@ -44,10 +44,25 @@ DEFINE_EVENT_FN(x86_exceptions, name,				\
 DEFINE_PAGE_FAULT_EVENT(page_fault_user);
 DEFINE_PAGE_FAULT_EVENT(page_fault_kernel);
 
+#ifdef CONFIG_SPLIT_LOCK_AC
+int trace_split_lock_reg(void);
+void trace_split_lock_unreg(void);
+
+#define DEFINE_SPLIT_LOCK_FAULT_EVENT(name)			\
+DEFINE_EVENT_FN(x86_exceptions, name,				\
+	TP_PROTO(unsigned long address,	struct pt_regs *regs,	\
+		 unsigned long error_code),			\
+	TP_ARGS(address, regs, error_code),			\
+	trace_split_lock_reg, trace_split_lock_unreg)
+
+DEFINE_SPLIT_LOCK_FAULT_EVENT(split_lock_user);
+DEFINE_SPLIT_LOCK_FAULT_EVENT(split_lock_kernel);
+#endif
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #define TRACE_INCLUDE_FILE exceptions
-#endif /*  _TRACE_PAGE_FAULT_H */
+#endif /*  _TRACE_EXCEPTIONS_H */
 
 /* This part must be outside protection */
 #include <trace/define_trace.h>
diff --git a/arch/x86/kernel/cpu/split_lock.c b/arch/x86/kernel/cpu/split_lock.c
index 020af331594d..948a7fa948a2 100644
--- a/arch/x86/kernel/cpu/split_lock.c
+++ b/arch/x86/kernel/cpu/split_lock.c
@@ -15,6 +15,8 @@
 #include <linux/cpu.h>
 #include <linux/reboot.h>
 #include <linux/syscore_ops.h>
+#include <asm-generic/kprobes.h>
+#include <asm/trace/exceptions.h>
 #include <asm/msr.h>
 
 static bool split_lock_ac_supported;
@@ -45,6 +47,8 @@ static const char * const user_modes[USER_MODE_LAST] = {
 	[USER_MODE_RE_EXECUTE] = "re-execute",
 };
 
+DEFINE_STATIC_KEY_FALSE(trace_split_lock_key);
+
 /*
  * On processors not supporting #AC exception for split lock feature,
  * MSR_TEST_CTL may not exist or MSR_TEST_CTL exists but the bit 29 is
@@ -116,6 +120,20 @@ static __init int setup_split_lock_ac(char *str)
 }
 __setup("split_lock_ac=", setup_split_lock_ac);
 
+int trace_split_lock_reg(void)
+{
+	if (split_lock_ac_supported)
+		static_branch_inc(&trace_split_lock_key);
+
+	return 0;
+}
+
+void trace_split_lock_unreg(void)
+{
+	if (split_lock_ac_supported)
+		static_branch_dec(&trace_split_lock_key);
+}
+
 static bool _setup_split_lock(int split_lock_ac_val)
 {
 	u32 l, h;
@@ -268,6 +286,31 @@ static bool re_execute(struct pt_regs *regs)
 	return false;
 }
 
+static nokprobe_inline void
+trace_split_lock_entries(unsigned long address, struct pt_regs *regs,
+			 unsigned long error_code)
+{
+	/*
+	 * If either CR0.AM or EFLAGS.AC is zero in user, the #AC must
+	 * come from split lock. Trace the #AC in split_lock_user event.
+	 *
+	 * For other cases, the #AC could be from split lock or from
+	 * generic cache line misalignment. Don't trace the #AC. In
+	 * theory, that means some split lock events may not be traced.
+	 * But usually EFLAGS.AC is not set for user process; so this
+	 * is not big issue.
+	 */
+	if (user_mode(regs) && (cr0_am(regs) == 0 || eflags_ac(regs) == 0))
+		trace_split_lock_user(address, regs, error_code);
+
+	/*
+	 * Only split lock can trigger #AC from kernel. Trace the #AC in
+	 * split_lock_kernel event.
+	 */
+	if (!user_mode(regs))
+		trace_split_lock_kernel(address, regs, error_code);
+}
+
 /*
  * #AC handler for kernel split lock is called by generic #AC handler.
  *
@@ -280,6 +323,9 @@ bool do_split_lock_exception(struct pt_regs *regs, unsigned long error_code)
 	unsigned long address = read_cr2(); /* Get the faulting address */
 	int this_cpu = smp_processor_id();
 
+	if (trace_split_lock_enabled())
+		trace_split_lock_entries(address, regs, error_code);
+
 	if (!re_execute(regs))
 		return false;
 
-- 
2.5.0

[PATCH 14/15] x86/split_lock: Add CONFIG and testing sysfs interface

[Fenghua Yu]

CONFIG_SPLIT_LOCK_AC_TEST is added to enable sysfs interface
/sys/kernel/split_lock/test_kernel to test split lock in kernel.

Writing 1 to the file triggers a split locked access in kernel.
User can use this interface to test how split locked access
happening in kernel is handled.

The file is not readable.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/Kconfig                 | 10 ++++++
 arch/x86/kernel/cpu/split_lock.c | 71 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 38baf5fb8556..018596d80424 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -460,6 +460,16 @@ config SPLIT_LOCK_AC
 
 	  Say N if unsure.
 
+config SPLIT_LOCK_AC_TEST
+	bool "Test #AC exception for split locked accesses"
+	default n
+	depends on SPLIT_LOCK_AC
+	help
+	  Select to enable testing #AC exception for split lock accesses.
+	  This adds interface /sys/kernel/split_lock/trigger_kernel to
+	  allow user to trigger split locked access in kernel and test
+	  split lock handling.
+
 if X86_32
 config X86_BIGSMP
 	bool "Support for big SMP systems with more than 8 CPUs"
diff --git a/arch/x86/kernel/cpu/split_lock.c b/arch/x86/kernel/cpu/split_lock.c
index 948a7fa948a2..eef69283aa5d 100644
--- a/arch/x86/kernel/cpu/split_lock.c
+++ b/arch/x86/kernel/cpu/split_lock.c
@@ -490,10 +490,81 @@ bios_store(struct kobject *kobj, struct kobj_attribute *attr,
 
 static struct kobj_attribute split_lock_ac_bios_enable = __ATTR_RW(bios);
 
+#ifdef CONFIG_SPLIT_LOCK_AC_TEST
+/* Execute locked cmpxchg with split locked address. */
+static void split_lock_test_kernel(void)
+{
+	char cptr[128] __aligned(64);
+	int *iptr, a = 10, b = 11;
+
+	/* Increment the pointer, making it misaligned */
+	iptr = (int *)(cptr + 61);
+
+	/* Initial value 1 in iptr */
+	*iptr = 1;
+
+	pr_info("split lock test: misaligned address=%lx\n",
+		(unsigned long)iptr);
+	/*
+	 * Since eax is equal to *iptr, the instruction loads value in b
+	 * (i.e. 11) into iptr. If the instruction is executed correctly,
+	 * the content of *iptr is changed to 11 from previous value 1.
+	 *
+	 * Accessing iptr cross two cache lines will trigger #AC in hardware.
+	 * The instruction is re-executed during split lock is disabled and
+	 * re-enabled later.
+	 */
+	asm volatile ("movl %1, %%eax\n\t"
+		      "movl %1, %0\n\t"
+		      "lock\n cmpxchgl %2, %0\n\t"
+		      : "=m" (*iptr)
+		      : "r"(a), "r"(b)
+		      : "%eax");
+
+	if (*iptr == b)
+		pr_info("split lock kernel test passes\n");
+	else
+		pr_info("split lock kernel test fails\n");
+}
+
+/*
+ * Writing 1 to /sys/kernel/split_lock/test_kernel triggers split locked
+ * access in kernel mode.
+ */
+static ssize_t
+test_kernel_store(struct kobject *kobj, struct kobj_attribute *attr,
+		  const char *buf, size_t count)
+{
+	u32 val;
+	int ret;
+
+	if (split_lock_ac == DISABLE_SPLIT_LOCK_AC)
+		return -ENODEV;
+
+	ret = kstrtou32(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	if (val != 1)
+		return -EINVAL;
+
+	mutex_lock(&split_lock_mutex);
+	split_lock_test_kernel();
+	mutex_unlock(&split_lock_mutex);
+
+	return count;
+}
+
+static struct kobj_attribute split_lock_ac_test = __ATTR_WO(test_kernel);
+#endif /* CONFIG_SPLIT_LOCK_AC_TEST */
+
 static struct attribute *split_lock_attrs[] = {
 	&split_lock_ac_enable.attr,
 	&split_lock_ac_user.attr,
 	&split_lock_ac_bios_enable.attr,
+#ifdef CONFIG_SPLIT_LOCK_AC_TEST
+	&split_lock_ac_test.attr,
+#endif
 	NULL,
 };
 
-- 
2.5.0

[PATCH 15/15] x86/split_lock: Add split lock user space test in selftest

[Fenghua Yu]

The test generates split locked access from user space. If #AC
exception is enabled for split lock, a SIGBUS is delivered
to the test.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 tools/testing/selftests/x86/Makefile               |   3 +-
 tools/testing/selftests/x86/split_lock_user_test.c | 187 +++++++++++++++++++++
 2 files changed, 189 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/x86/split_lock_user_test.c

diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index d744991c0f4f..19d65729b100 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -11,7 +11,8 @@ CAN_BUILD_X86_64 := $(shell ./check_cc.sh $(CC) trivial_64bit_program.c)
 
 TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt test_mremap_vdso \
 			check_initial_reg_state sigreturn iopl mpx-mini-test ioperm \
-			protection_keys test_vdso test_vsyscall
+			protection_keys test_vdso test_vsyscall	\
+			split_lock_user_test
 TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso unwind_vdso \
 			test_FCMOV test_FCOMI test_FISTTP \
 			vdso_restorer
diff --git a/tools/testing/selftests/x86/split_lock_user_test.c b/tools/testing/selftests/x86/split_lock_user_test.c
new file mode 100644
index 000000000000..e6f3367dfb6b
--- /dev/null
+++ b/tools/testing/selftests/x86/split_lock_user_test.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Intel Corporation
+ * Author: Fenghua Yu <fenghua.yu@intel.com>
+ *
+ * Pre-request:
+ * Kernel is built with CONFIG_SPLIT_LOCK_AC=y.
+ * Split lock is enabled. If not, enable it by:
+ * #echo 1 >/sys/kernel/split_lock/enable
+ *
+ * Usage:
+ * Run the test alone and it should show:
+ *      TEST PASS: locked instruction is re-executed.
+ *      TEST PASS: Caught SIGBUS/#AC due to split locked access
+ *
+ * Or launch the test from perf and watch "split_lock_user" event count.
+ * #/perf stat -e exceptions:split_lock* /root/split_lock_user_test_64
+ *      TEST PASS: locked instruction is re-executed.
+ *      TEST PASS: Caught SIGBUS/#AC due to split locked access
+ *
+ * Performance counter stats for '/root/split_lock_user_test_64':
+ *
+ *                1      exceptions:split_lock_user
+ *                0      exceptions:split_lock_kernel
+ *
+ *       1.000893458 seconds time elapsed
+ */
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+
+int split_lock_exception;
+
+void catch_sigbus(int sig)
+{
+	split_lock_exception = 1;
+	printf("TEST PASS: Caught SIGBUS/#AC due to split locked access\n");
+
+	exit(-1);
+}
+
+int split_lock_ac_enabled(void)
+{
+	int fd, enable, ret;
+	char buf[16];
+
+	fd = open("/sys/kernel/split_lock/enable", O_RDONLY);
+	if (fd < 0)
+		return 0;
+
+	if (read(fd, buf, sizeof(int)) < 0) {
+		ret = 0;
+		goto out;
+	}
+
+	enable = atoi(buf);
+	if (enable == 1)
+		ret = 1;
+	else
+		ret = 0;
+
+out:
+	close(fd);
+
+	return ret;
+}
+
+int setup_user_mode(char *user_mode_reaction)
+{
+	ssize_t count;
+	int fd;
+
+	if (strcmp(user_mode_reaction, "sigbus") &&
+	    strcmp(user_mode_reaction, "re-execute"))
+		return -1;
+
+	fd = open("/sys/kernel/split_lock/user_mode", O_RDWR);
+	if (fd < 0)
+		return -1;
+
+	count = write(fd, user_mode_reaction, strlen(user_mode_reaction));
+	if (count != strlen(user_mode_reaction))
+		return -1;
+
+	close(fd);
+
+	return 0;
+}
+
+void do_split_locked_inst(int a, int b, int *iptr)
+{
+	/*
+	 * Since eax is equal to *iptr, the instruction loads value in b
+	 * (i.e. 11) into iptr. If the instruction is executed correctly,
+	 * the content of *iptr is changed * to 11 from previous value 1.
+	 *
+	 * Accessing iptr cross two cache lines will trigger #AC in hardware
+	 * and kernel either delivers SIGBUS to this process or re-execute
+	 * the instruction depending on /sys/kernel/split_lock/user_mode
+	 * setting.
+	 */
+	asm volatile ("movl %1, %%eax\n\t"
+		      "movl %1, %0\n\t"
+		      "lock\n cmpxchgl %2, %0\n\t"
+		      : "=m" (*iptr)
+		      : "r"(a), "r"(b)
+		      : "%eax");
+}
+
+void test_re_execute(int a, int b, int *iptr)
+{
+	setup_user_mode("re-execute");
+
+	/* The locked instruction triggers #AC and then it's re-executed. */
+	do_split_locked_inst(a, b, iptr);
+
+	if (*iptr == b) {
+		printf("TEST PASS: locked instruction is re-executed.\n");
+	} else {
+		printf("TEST FAIL: No #AC exception is caught and ");
+		printf("instruction is not executed correctly.\n");
+	}
+}
+
+void test_sigbus(int a, int b, int *iptr)
+{
+	setup_user_mode("sigbus");
+
+	/*
+	 * The locked instruction triggers #AC and kernel delivers SIGBUS
+	 * to this process.
+	 */
+	do_split_locked_inst(a, b, iptr);
+}
+
+int main(int argc, char **argv)
+{
+	int *iptr, a = 10, b = 11;
+	char *cptr;
+
+	if (!split_lock_ac_enabled()) {
+		printf("#AC exception for split lock is NOT enabled!!\n");
+		printf("Please make sure split lock feature is supported,\n");
+		printf("CONFIG_SPLIT_LOCK_AC is turned on,\n");
+		printf("and /sys/kernel/split_lock/enable is 1 before test!\n");
+
+		return 0;
+	}
+
+	signal(SIGBUS, catch_sigbus);
+
+	/*
+	 * Enable Alignment Checking on x86_64.
+	 * This will generate alignment check on not only split lock but also
+	 * on any misalignment.
+	 * Turn on this for reference only.
+	 */
+	/* __asm__("pushf\norl $0x40000,(%rsp)\npopf"); */
+
+	/* aligned_alloc() provides 64-byte aligned memory */
+	cptr = (char *)aligned_alloc(64, 128);
+
+	/* Increment the pointer by 61, making it misaligned */
+	iptr = (int *)(cptr + 61);
+
+	/* Initial value in iptr is 1. */
+	*iptr = 1;
+
+	test_re_execute(a, b, iptr);
+	/*
+	 * The split lock is disabled after the last locked instruction is
+	 * re-executed.
+	 *
+	 * Wait for the split lock is re-enabled again before next test.
+	 */
+	sleep(1);
+	test_sigbus(a, b, iptr);
+
+	free(cptr);
+
+	return 0;
+}
-- 
2.5.0

Re: [PATCH 07/15] x86/split_lock: Handle suspend/hibernate and resume

[Rafael J. Wysocki]

On Monday, May 14, 2018 8:52:17 PM CEST Fenghua Yu wrote:
> During suspend or hibernation, system enters BIOS. To avoid potential
> BIOS issue that may generate #AC exception for split locked accesses,
> handle the #AC as fatal exception, and block suspend or hibernation,
> the split lock setting is restored to BIOS setting. When resuming
> from suspend or hibernation, the split lock setting is restored to
> kernel setting.
> 
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  arch/x86/include/asm/cpu.h       |  2 ++
>  arch/x86/kernel/cpu/split_lock.c | 54 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
> index 0b00033b6fa8..89d62d7051fa 100644
> --- a/arch/x86/include/asm/cpu.h
> +++ b/arch/x86/include/asm/cpu.h
> @@ -45,11 +45,13 @@ int __init enumerate_split_lock(void);
>  void setup_split_lock(void);
>  bool do_split_lock_exception(struct pt_regs *regs, unsigned long error_code);
>  bool restore_split_lock_ac_bios(int *enable);
> +bool restore_split_lock_ac_kernel(int *enable);
>  #else /* CONFIG_SPLIT_LOCK_AC */
>  static inline int enumerate_split_lock(void) { return 0; }
>  static inline void setup_split_lock(void) {}
>  static inline bool
>  do_split_lock_exception(struct pt_regs *regs, unsigned long error_code) {}
>  static inline bool restore_split_lock_ac_bios(int *enable) { return true; }
> +static inline bool restore_split_lock_ac_kernel(int *enable) { return true; }
>  #endif /* CONFIG_SPLIT_LOCK_AC */
>  #endif /* _ASM_X86_CPU_H */
> diff --git a/arch/x86/kernel/cpu/split_lock.c b/arch/x86/kernel/cpu/split_lock.c
> index d2735259800b..5187a9c6cea6 100644
> --- a/arch/x86/kernel/cpu/split_lock.c
> +++ b/arch/x86/kernel/cpu/split_lock.c
> @@ -14,6 +14,7 @@
>  #include <linux/workqueue.h>
>  #include <linux/cpu.h>
>  #include <linux/reboot.h>
> +#include <linux/syscore_ops.h>
>  #include <asm/msr.h>
>  
>  static bool split_lock_ac_supported;
> @@ -143,6 +144,15 @@ bool restore_split_lock_ac_bios(int *enable)
>  	return restore_split_lock_ac(split_lock_ac_bios);
>  }
>  
> +/* Restore kernel setting for #AC enable bit for split lock. */
> +bool restore_split_lock_ac_kernel(int *enable)
> +{
> +	if (enable)
> +		*enable = split_lock_ac == ENABLE_SPLIT_LOCK_AC ?  1 : 0;
> +
> +	return restore_split_lock_ac(split_lock_ac);
> +}
> +
>  static void split_lock_cpu_reboot(void *unused)
>  {
>  	restore_split_lock_ac_bios(NULL);
> @@ -223,11 +233,55 @@ bool do_split_lock_exception(struct pt_regs *regs, unsigned long error_code)
>  	return true;
>  }
>  
> +static int split_lock_offline(unsigned int cpu)
> +{
> +	int enable;
> +
> +	if (restore_split_lock_ac_bios(&enable))
> +		pr_info("%s split lock on CPU%d\n",
> +			enable ? "enable" : "disable", smp_processor_id());
> +
> +	return 0;
> +}
> +
> +static int split_lock_bsp_suspend(void)
> +{
> +	restore_split_lock_ac_bios(NULL);
> +
> +	return 0;
> +}
> +
> +static void split_lock_bsp_resume(void)
> +{
> +	restore_split_lock_ac_kernel(NULL);
> +}
> +
> +static struct syscore_ops split_lock_syscore_ops = {
> +	.suspend	= split_lock_bsp_suspend,
> +	.resume		= split_lock_bsp_resume,
> +};
> +
>  static int __init split_lock_init(void)
>  {
> +	int ret;
> +
> +	if (!split_lock_ac_supported)
> +		return -ENODEV;
> +
> +	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> +				"x86/split_lock:online:",
> +				NULL, split_lock_offline);
> +	if (ret < 0)
> +		goto out_fail;
> +
> +	register_syscore_ops(&split_lock_syscore_ops);
> +
>  	register_reboot_notifier(&split_lock_reboot_nb);
>  
>  	return 0;
> +
> +out_fail:
> +	return ret;
>  }
>  
>  late_initcall(split_lock_init);
> 

Re: [PATCH 0/15] x86/split_lock: Enable #AC exception for split locked accesses

[Dave Hansen]

On 05/14/2018 11:52 AM, Fenghua Yu wrote:
> When executing an atomic instruction, sometimes the data access crosses
> the cache line boundary. This causes performance degradation. Intel
> introduces mechanism to detect such split lock issue via alignment check
> fault.

This is still a pretty sparse description.

Could you describe the performance degradation?  Why is this not a
"doctor it hurts when I do that" situation?

Could you also give us *some* idea which CPUs this will show up on?  Is
it a one-off feature, or will it be available widely?

> Since kernel doesn't know when SMI comes, it's impossible for kernel
> to disable split lock #AC before entering SMI. So SMI handler may
> inherit kernel's split lock setting and kernel tester may end up
> debug split lock issues in SMI.

What's a "kernel tester"?

Re: [PATCH 01/15] x86/split_lock: Add CONFIG and enumerate #AC exception for split locked access feature

[Dave Hansen]

On 05/14/2018 11:52 AM, Fenghua Yu wrote:
> +#include <linux/printk.h>
> +#include <asm/msr.h>
> +
> +static bool split_lock_ac_supported;

Was there a reason not to use an X86_FEATURE* bit for this?

Re: [PATCH 01/15] x86/split_lock: Add CONFIG and enumerate #AC exception for split locked access feature

[Fenghua Yu]

On Tue, May 15, 2018 at 08:36:35AM -0700, Dave Hansen wrote:
> On 05/14/2018 11:52 AM, Fenghua Yu wrote:
> > +#include <linux/printk.h>
> > +#include <asm/msr.h>
> > +
> > +static bool split_lock_ac_supported;
> 
> Was there a reason not to use an X86_FEATURE* bit for this?

The feature is not enumerated in cpuid. It's detected by writing 1
to bit 29 in MSR 0x33.

So it can't fit in X86_FEATURE, right?

Thanks.

-Fenghua

Re: [PATCH 03/15] x86/split_lock: Handle #AC exception for split lock in kernel mode

[Dave Hansen]

On 05/14/2018 11:52 AM, Fenghua Yu wrote:
> +#define	delay_ms	1

That seems like a dangerously-generic name that should not be a #define
anyway.

> +static void delayed_reenable_split_lock(struct work_struct *w)
> +{
> +	if (split_lock_ac == ENABLE_SPLIT_LOCK_AC)
> +		_setup_split_lock(ENABLE_SPLIT_LOCK_AC);
> +}

This seems like it might get confusing.  We have the split lock ac
*mode* (what the kernel is doing overall) and also the *status* (what
mode the CPU is in at the moment).

The naming here doesn't really split up those two concepts very well.

> +/* Will the faulting instruction be re-executed? */
> +static bool re_execute(struct pt_regs *regs)
> +{
> +	/*
> +	 * The only reason for generating #AC from kernel is because of
> +	 * split lock. The kernel faulting instruction will be re-executed.
> +	 */
> +	if (!user_mode(regs))
> +		return true;
> +
> +	return false;
> +}

This helper with a single user is a bit unnecessary.  Just open-code
this and move the comments into the caller.

> +/*
> + * #AC handler for kernel split lock is called by generic #AC handler.
> + *
> + * Disable #AC for split lock on this CPU so that the faulting instruction
> + * gets executed. The #AC for split lock is re-enabled later.
> + */
> +bool do_split_lock_exception(struct pt_regs *regs, unsigned long error_code)
> +{
> +	unsigned long delay = msecs_to_jiffies(delay_ms);
> +	unsigned long address = read_cr2(); /* Get the faulting address */
> +	int this_cpu = smp_processor_id();

How does this end up working?  This seems to depend on this handler not
getting preempted.

> +	if (!re_execute(regs))
> +		return false;
> +
> +	pr_info_ratelimited("Alignment check for split lock at %lx\n", address);

This is a potential KASLR bypass, I believe.  We shouldn't be printing
raw kernel addresses.

We have some nice printk's for page faults that give you kernel symbols.
 Could you copy one of those?

> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index 03f3d7695dac..c07b817bbbe9 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -61,6 +61,7 @@
>  #include <asm/mpx.h>
>  #include <asm/vm86.h>
>  #include <asm/umip.h>
> +#include <asm/cpu.h>
>  
>  #ifdef CONFIG_X86_64
>  #include <asm/x86_init.h>
> @@ -286,10 +287,21 @@ static void do_error_trap(struct pt_regs *regs, long error_code, char *str,
>  			  unsigned long trapnr, int signr)
>  {
>  	siginfo_t info;
> +	int ret;
>  
>  	RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
>  
>  	/*
> +	 * #AC exception could be handled by split lock handler.
> +	 * If the handler can't handle the exception, go to generic #AC handler.
> +	 */
> +	if (trapnr == X86_TRAP_AC) {
> +		ret = do_split_lock_exception(regs, error_code);
> +		if (ret)
> +			return;
> +	}

Why are you hooking into do_error_trap()?  Shouldn't you just be
installing do_split_lock_exception() as *the* #AC handler and put it in
the IDT?

Re: [PATCH 01/15] x86/split_lock: Add CONFIG and enumerate #AC exception for split locked access feature

[Dave Hansen]

On 05/15/2018 08:41 AM, Fenghua Yu wrote:
> On Tue, May 15, 2018 at 08:36:35AM -0700, Dave Hansen wrote:
>> On 05/14/2018 11:52 AM, Fenghua Yu wrote:
>>> +#include <linux/printk.h>
>>> +#include <asm/msr.h>
>>> +
>>> +static bool split_lock_ac_supported;
>> Was there a reason not to use an X86_FEATURE* bit for this?
> The feature is not enumerated in cpuid. It's detected by writing 1
> to bit 29 in MSR 0x33.
> 
> So it can't fit in X86_FEATURE, right?

We have X86_FEATURE_*'s for lots of things, even software constructs
that have no representation in the hardware CPUID.

Re: [PATCH 09/15] x86/split_lock: Explicitly enable or disable #AC for split locked accesses

[Dave Hansen]

On 05/14/2018 11:52 AM, Fenghua Yu wrote:
> By default, we don't set or clear the bit 29 in TEST_CTL MSR 0x33 and
> the bit is inherited from BIOS/hardware setting.
> 
> The kernel parameter "split_lock_ac=on/off" explicitly sets or clears
> the bit during boot time.

The more I think about this...  Why do we need this at boot anyway?
Surely boot-time kernel code can't cause performance issues in the same
way that untrusted repeated userspace can.  Why don't we just let
userspace turn this on?

Re: [PATCH 0/15] x86/split_lock: Enable #AC exception for split locked accesses

[Alan Cox]

> Could you describe the performance degradation?  Why is this not a
> "doctor it hurts when I do that" situation?

They already know it hurts - the question is why and where.

Alan

Re: [PATCH 0/15] x86/split_lock: Enable #AC exception for split locked accesses

[Dave Hansen]

On 05/15/2018 09:26 AM, Alan Cox wrote:
>> Could you describe the performance degradation?  Why is this not a
>> "doctor it hurts when I do that" situation?
> They already know it hurts - the question is why and where.

I thought there were also cross-VM and cross-process concerns where
somebody unprivileged can make the whole system slow down.  They might
even be malicious, so this isn't something that *just* hurts one app or
one VM.

I think that's what I wanted to see hit the changelog.

Re: [PATCH 03/15] x86/split_lock: Handle #AC exception for split lock in kernel mode

[Luck, Tony]

On Tue, May 15, 2018 at 08:51:24AM -0700, Dave Hansen wrote:
> > +	pr_info_ratelimited("Alignment check for split lock at %lx\n", address);
> 
> This is a potential KASLR bypass, I believe.  We shouldn't be printing
> raw kernel addresses.
> 
> We have some nice printk's for page faults that give you kernel symbols.
>  Could you copy one of those?

It's not really all that useful to print the address of the split lock
itself. It's probably in something that was kmalloc()'d. Users will
probably want to see the address of the instruction so they know which
function to go and debug.  Print that with %pF

-Tony

Re: [PATCH 03/15] x86/split_lock: Handle #AC exception for split lock in kernel mode

[Fenghua Yu]

On Tue, May 15, 2018 at 08:51:24AM -0700, Dave Hansen wrote:
> On 05/14/2018 11:52 AM, Fenghua Yu wrote:
> > +#define	delay_ms	1
> 
> That seems like a dangerously-generic name that should not be a #define
> anyway.

Sure. I will change it to
#define split_lock_delay_ms	1

> 
> > +static void delayed_reenable_split_lock(struct work_struct *w)
> > +{
> > +	if (split_lock_ac == ENABLE_SPLIT_LOCK_AC)
> > +		_setup_split_lock(ENABLE_SPLIT_LOCK_AC);
> > +}
> 
> This seems like it might get confusing.  We have the split lock ac
> *mode* (what the kernel is doing overall) and also the *status* (what
> mode the CPU is in at the moment).
> 
> The naming here doesn't really split up those two concepts very well.

To check the status and re-enable split lock is complex because the
MSR is shared among threads in one core and locking could be complex.

I will re-think about this code...

> 
> > +/* Will the faulting instruction be re-executed? */
> > +static bool re_execute(struct pt_regs *regs)
> > +{
> > +	/*
> > +	 * The only reason for generating #AC from kernel is because of
> > +	 * split lock. The kernel faulting instruction will be re-executed.
> > +	 */
> > +	if (!user_mode(regs))
> > +		return true;
> > +
> > +	return false;
> > +}
> 
> This helper with a single user is a bit unnecessary.  Just open-code
> this and move the comments into the caller.

In this patch, this helper is only used for checking kernel mode.
Then in patch #11, this helper will add checking user mode code.
It would be better to have a helper defined and called.

> 
> > +/*
> > + * #AC handler for kernel split lock is called by generic #AC handler.
> > + *
> > + * Disable #AC for split lock on this CPU so that the faulting instruction
> > + * gets executed. The #AC for split lock is re-enabled later.
> > + */
> > +bool do_split_lock_exception(struct pt_regs *regs, unsigned long error_code)
> > +{
> > +	unsigned long delay = msecs_to_jiffies(delay_ms);
> > +	unsigned long address = read_cr2(); /* Get the faulting address */
> > +	int this_cpu = smp_processor_id();
> 
> How does this end up working?  This seems to depend on this handler not
> getting preempted.

Maybe change the handler to:

this_cpu = task_cpu(current);
Then disable split lock on this_cpu.
Re-enable split lock on this_cpu (already in this way).

Does this sound better?

> 
> > +	if (!re_execute(regs))
> > +		return false;
> > +
> > +	pr_info_ratelimited("Alignment check for split lock at %lx\n", address);
> 
> This is a potential KASLR bypass, I believe.  We shouldn't be printing
> raw kernel addresses.
> 
> We have some nice printk's for page faults that give you kernel symbols.
>  Could you copy one of those?

Sure. Will do that.

> 
> > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> > index 03f3d7695dac..c07b817bbbe9 100644
> > --- a/arch/x86/kernel/traps.c
> > +++ b/arch/x86/kernel/traps.c
> > @@ -61,6 +61,7 @@
> >  #include <asm/mpx.h>
> >  #include <asm/vm86.h>
> >  #include <asm/umip.h>
> > +#include <asm/cpu.h>
> >  
> >  #ifdef CONFIG_X86_64
> >  #include <asm/x86_init.h>
> > @@ -286,10 +287,21 @@ static void do_error_trap(struct pt_regs *regs, long error_code, char *str,
> >  			  unsigned long trapnr, int signr)
> >  {
> >  	siginfo_t info;
> > +	int ret;
> >  
> >  	RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
> >  
> >  	/*
> > +	 * #AC exception could be handled by split lock handler.
> > +	 * If the handler can't handle the exception, go to generic #AC handler.
> > +	 */
> > +	if (trapnr == X86_TRAP_AC) {
> > +		ret = do_split_lock_exception(regs, error_code);
> > +		if (ret)
> > +			return;
> > +	}
> 
> Why are you hooking into do_error_trap()?  Shouldn't you just be
> installing do_split_lock_exception() as *the* #AC handler and put it in
> the IDT?
> 

Split lock is not the only reason that causes #AC. #AC can be caused
by user turning on AC bit in EFLAGS, which is just cache line misalignment
and is different from split lock.

So split lock is sharing the handler with another #AC case and can't
be installed seperately from previous #AC handler, right?

Thanks.

-Fenghua

Re: [PATCH 09/15] x86/split_lock: Explicitly enable or disable #AC for split locked accesses

[Fenghua Yu]

On Tue, May 15, 2018 at 09:15:16AM -0700, Dave Hansen wrote:
> On 05/14/2018 11:52 AM, Fenghua Yu wrote:
> > By default, we don't set or clear the bit 29 in TEST_CTL MSR 0x33 and
> > the bit is inherited from BIOS/hardware setting.
> > 
> > The kernel parameter "split_lock_ac=on/off" explicitly sets or clears
> > the bit during boot time.
> 
> The more I think about this...  Why do we need this at boot anyway?
> Surely boot-time kernel code can't cause performance issues in the same
> way that untrusted repeated userspace can.  Why don't we just let
> userspace turn this on?

Turning split lock earlier can captuer split lock performance issue
in the boot path. Actually we did find two split lock issues during
boot time by turning on the feature earlier (see patch 4 and 5).

I guess how to improve boot time is still a concern for client and VM.
If split lock issue can be identified and fixed in boot time, it just
helps and dosn't hurt anything.

Turning on the feature during boot time makes user easier to identify
any split lock issue not just during boot time and also run time.

Having said that, there is a sysfs interface in patch #10 that allows
user to turn on/off the feature during boot time.

Does that sound better?

Thanks.

-Fenghua

Re: [PATCH 09/15] x86/split_lock: Explicitly enable or disable #AC for split locked accesses

[Dave Hansen]

On 05/15/2018 10:29 AM, Fenghua Yu wrote:
> On Tue, May 15, 2018 at 09:15:16AM -0700, Dave Hansen wrote:
>> On 05/14/2018 11:52 AM, Fenghua Yu wrote:
>>> By default, we don't set or clear the bit 29 in TEST_CTL MSR 0x33 and
>>> the bit is inherited from BIOS/hardware setting.
>>>
>>> The kernel parameter "split_lock_ac=on/off" explicitly sets or clears
>>> the bit during boot time.
>>
>> The more I think about this...  Why do we need this at boot anyway?
>> Surely boot-time kernel code can't cause performance issues in the same
>> way that untrusted repeated userspace can.  Why don't we just let
>> userspace turn this on?
> 
> Turning split lock earlier can captuer split lock performance issue
> in the boot path. Actually we did find two split lock issues during
> boot time by turning on the feature earlier (see patch 4 and 5).

Yeah, but you're going to have a pretty hard time arguing that these are
even close to the same class of issues that occur when all the CPUs are
up and userspace is pounding the system with split locks.

It also seems like the kind of thing that deserves to be a debugging
option, or Kconfig thing rather than a kernel command-line option that
has to live forever.

Re: [PATCH 03/15] x86/split_lock: Handle #AC exception for split lock in kernel mode

[Dave Hansen]

On 05/15/2018 10:21 AM, Fenghua Yu wrote:
> On Tue, May 15, 2018 at 08:51:24AM -0700, Dave Hansen wrote:
>> On 05/14/2018 11:52 AM, Fenghua Yu wrote:
>>> +#define	delay_ms	1
>>
>> That seems like a dangerously-generic name that should not be a #define
>> anyway.
> 
> Sure. I will change it to
> #define split_lock_delay_ms	1

Why not:

static unsigned int reenable_split_lock_delay_ms = 1;

?

>>> +/* Will the faulting instruction be re-executed? */
>>> +static bool re_execute(struct pt_regs *regs)
>>> +{
>>> +	/*
>>> +	 * The only reason for generating #AC from kernel is because of
>>> +	 * split lock. The kernel faulting instruction will be re-executed.
>>> +	 */
>>> +	if (!user_mode(regs))
>>> +		return true;
>>> +
>>> +	return false;
>>> +}
>>
>> This helper with a single user is a bit unnecessary.  Just open-code
>> this and move the comments into the caller.
> 
> In this patch, this helper is only used for checking kernel mode.
> Then in patch #11, this helper will add checking user mode code.
> It would be better to have a helper defined and called.

Then introduce the helper later, or call this out in a comment or the
patch description, please.

>>> +/*
>>> + * #AC handler for kernel split lock is called by generic #AC handler.
>>> + *
>>> + * Disable #AC for split lock on this CPU so that the faulting instruction
>>> + * gets executed. The #AC for split lock is re-enabled later.
>>> + */
>>> +bool do_split_lock_exception(struct pt_regs *regs, unsigned long error_code)
>>> +{
>>> +	unsigned long delay = msecs_to_jiffies(delay_ms);
>>> +	unsigned long address = read_cr2(); /* Get the faulting address */
>>> +	int this_cpu = smp_processor_id();
>>
>> How does this end up working?  This seems to depend on this handler not
>> getting preempted.
> 
> Maybe change the handler to:
> 
> this_cpu = task_cpu(current);
> Then disable split lock on this_cpu.
> Re-enable split lock on this_cpu (already in this way).
> 
> Does this sound better?

Actually, as I look at it, interrupts *are* still disabled here, so you
are OK.  But, you can do a local_irq_enable() once you get all of the
per-cpu state settled and go to start handling the fault if you are
going to do anything that takes an appreciable amount of time.

>>> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
>>> index 03f3d7695dac..c07b817bbbe9 100644
>>> --- a/arch/x86/kernel/traps.c
>>> +++ b/arch/x86/kernel/traps.c
>>> @@ -61,6 +61,7 @@
>>>  #include <asm/mpx.h>
>>>  #include <asm/vm86.h>
>>>  #include <asm/umip.h>
>>> +#include <asm/cpu.h>
>>>  
>>>  #ifdef CONFIG_X86_64
>>>  #include <asm/x86_init.h>
>>> @@ -286,10 +287,21 @@ static void do_error_trap(struct pt_regs *regs, long error_code, char *str,
>>>  			  unsigned long trapnr, int signr)
>>>  {
>>>  	siginfo_t info;
>>> +	int ret;
>>>  
>>>  	RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
>>>  
>>>  	/*
>>> +	 * #AC exception could be handled by split lock handler.
>>> +	 * If the handler can't handle the exception, go to generic #AC handler.
>>> +	 */
>>> +	if (trapnr == X86_TRAP_AC) {
>>> +		ret = do_split_lock_exception(regs, error_code);
>>> +		if (ret)
>>> +			return;
>>> +	}
>>
>> Why are you hooking into do_error_trap()?  Shouldn't you just be
>> installing do_split_lock_exception() as *the* #AC handler and put it in
>> the IDT?
> 
> Split lock is not the only reason that causes #AC. #AC can be caused
> by user turning on AC bit in EFLAGS, which is just cache line misalignment
> and is different from split lock.
> 
> So split lock is sharing the handler with another #AC case and can't
> be installed seperately from previous #AC handler, right?

There are lots of exceptions that use do_error_trap().  I'm suggesting
that you make an IDT entry for X86_TRAP_AC that does not use
do_error_trap() since you need something different in there now.

See:

> #define DO_ERROR(trapnr, signr, str, name)                              \
> dotraplinkage void do_##name(struct pt_regs *regs, long error_code)     \
> {                                                                       \
>         do_error_trap(regs, error_code, str, trapnr, signr);            \
> }
> 
> DO_ERROR(X86_TRAP_DE,     SIGFPE,  "divide error",              divide_error)
> DO_ERROR(X86_TRAP_OF,     SIGSEGV, "overflow",                  overflow)
> DO_ERROR(X86_TRAP_UD,     SIGILL,  "invalid opcode",            invalid_op)
> DO_ERROR(X86_TRAP_OLD_MF, SIGFPE,  "coprocessor segment overrun",coprocessor_segment_overrun)
> DO_ERROR(X86_TRAP_TS,     SIGSEGV, "invalid TSS",               invalid_TSS)
> DO_ERROR(X86_TRAP_NP,     SIGBUS,  "segment not present",       segment_not_present)
> DO_ERROR(X86_TRAP_SS,     SIGBUS,  "stack segment",             stack_segment)
> DO_ERROR(X86_TRAP_AC,     SIGBUS,  "alignment check",           alignment_check)

Look at do_general_protection(), for instance.

Re: [PATCH 03/15] x86/split_lock: Handle #AC exception for split lock in kernel mode

[Fenghua Yu]

On Wed, May 16, 2018 at 09:44:59AM -0700, Dave Hansen wrote:
> On 05/15/2018 10:21 AM, Fenghua Yu wrote:
> > On Tue, May 15, 2018 at 08:51:24AM -0700, Dave Hansen wrote:
> >> On 05/14/2018 11:52 AM, Fenghua Yu wrote:
> >>> +#define	delay_ms	1
> >>
> >> That seems like a dangerously-generic name that should not be a #define
> >> anyway.
> > 
> > Sure. I will change it to
> > #define split_lock_delay_ms	1
> 
> Why not:
> 
> static unsigned int reenable_split_lock_delay_ms = 1;
> 
> ?
Sure.

> 
> >>> +/* Will the faulting instruction be re-executed? */
> >>> +static bool re_execute(struct pt_regs *regs)
> >>> +{
> >>> +	/*
> >>> +	 * The only reason for generating #AC from kernel is because of
> >>> +	 * split lock. The kernel faulting instruction will be re-executed.
> >>> +	 */
> >>> +	if (!user_mode(regs))
> >>> +		return true;
> >>> +
> >>> +	return false;
> >>> +}
> >>
> >> This helper with a single user is a bit unnecessary.  Just open-code
> >> this and move the comments into the caller.
> > 
> > In this patch, this helper is only used for checking kernel mode.
> > Then in patch #11, this helper will add checking user mode code.
> > It would be better to have a helper defined and called.
> 
> Then introduce the helper later, or call this out in a comment or the
> patch description, please.

Ok. I will call this out in the patch description.

> 
> >>> +/*
> >>> + * #AC handler for kernel split lock is called by generic #AC handler.
> >>> + *
> >>> + * Disable #AC for split lock on this CPU so that the faulting instruction
> >>> + * gets executed. The #AC for split lock is re-enabled later.
> >>> + */
> >>> +bool do_split_lock_exception(struct pt_regs *regs, unsigned long error_code)
> >>> +{
> >>> +	unsigned long delay = msecs_to_jiffies(delay_ms);
> >>> +	unsigned long address = read_cr2(); /* Get the faulting address */
> >>> +	int this_cpu = smp_processor_id();
> >>
> >> How does this end up working?  This seems to depend on this handler not
> >> getting preempted.
> > 
> > Maybe change the handler to:
> > 
> > this_cpu = task_cpu(current);
> > Then disable split lock on this_cpu.
> > Re-enable split lock on this_cpu (already in this way).
> > 
> > Does this sound better?
> 
> Actually, as I look at it, interrupts *are* still disabled here, so you
> are OK.  But, you can do a local_irq_enable() once you get all of the
> per-cpu state settled and go to start handling the fault if you are
> going to do anything that takes an appreciable amount of time.

Ok.

> 
> >>> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> >>> index 03f3d7695dac..c07b817bbbe9 100644
> >>> --- a/arch/x86/kernel/traps.c
> >>> +++ b/arch/x86/kernel/traps.c
> >>> @@ -61,6 +61,7 @@
> >>>  #include <asm/mpx.h>
> >>>  #include <asm/vm86.h>
> >>>  #include <asm/umip.h>
> >>> +#include <asm/cpu.h>
> >>>  
> >>>  #ifdef CONFIG_X86_64
> >>>  #include <asm/x86_init.h>
> >>> @@ -286,10 +287,21 @@ static void do_error_trap(struct pt_regs *regs, long error_code, char *str,
> >>>  			  unsigned long trapnr, int signr)
> >>>  {
> >>>  	siginfo_t info;
> >>> +	int ret;
> >>>  
> >>>  	RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
> >>>  
> >>>  	/*
> >>> +	 * #AC exception could be handled by split lock handler.
> >>> +	 * If the handler can't handle the exception, go to generic #AC handler.
> >>> +	 */
> >>> +	if (trapnr == X86_TRAP_AC) {
> >>> +		ret = do_split_lock_exception(regs, error_code);
> >>> +		if (ret)
> >>> +			return;
> >>> +	}
> >>
> >> Why are you hooking into do_error_trap()?  Shouldn't you just be
> >> installing do_split_lock_exception() as *the* #AC handler and put it in
> >> the IDT?
> > 
> > Split lock is not the only reason that causes #AC. #AC can be caused
> > by user turning on AC bit in EFLAGS, which is just cache line misalignment
> > and is different from split lock.
> > 
> > So split lock is sharing the handler with another #AC case and can't
> > be installed seperately from previous #AC handler, right?
> 
> There are lots of exceptions that use do_error_trap().  I'm suggesting
> that you make an IDT entry for X86_TRAP_AC that does not use
> do_error_trap() since you need something different in there now.
> 
> See:
> 
> > #define DO_ERROR(trapnr, signr, str, name)                              \
> > dotraplinkage void do_##name(struct pt_regs *regs, long error_code)     \
> > {                                                                       \
> >         do_error_trap(regs, error_code, str, trapnr, signr);            \
> > }
> > 
> > DO_ERROR(X86_TRAP_DE,     SIGFPE,  "divide error",              divide_error)
> > DO_ERROR(X86_TRAP_OF,     SIGSEGV, "overflow",                  overflow)
> > DO_ERROR(X86_TRAP_UD,     SIGILL,  "invalid opcode",            invalid_op)
> > DO_ERROR(X86_TRAP_OLD_MF, SIGFPE,  "coprocessor segment overrun",coprocessor_segment_overrun)
> > DO_ERROR(X86_TRAP_TS,     SIGSEGV, "invalid TSS",               invalid_TSS)
> > DO_ERROR(X86_TRAP_NP,     SIGBUS,  "segment not present",       segment_not_present)
> > DO_ERROR(X86_TRAP_SS,     SIGBUS,  "stack segment",             stack_segment)
> > DO_ERROR(X86_TRAP_AC,     SIGBUS,  "alignment check",           alignment_check)
> 
> Look at do_general_protection(), for instance.

Sure. I will define the #AC separately.

Thanks.

-Fenghua