linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses
@ 2019-03-12 23:00 Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 01/18] x86/common: Align cpu_caps_cleared and cpu_caps_set to unsigned long Fenghua Yu
                   ` (17 more replies)
  0 siblings, 18 replies; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

==Introduction==

A split lock is any atomic operation whose operand crosses two cache
lines. Since the operand spans two cache lines and the operation must
be atomic, the system locks the bus while the CPU accesses the two cache
lines.

During bus locking, request from other CPUs or bus agents for control
of the bus are blocked. Blocking bus access from other CPUs plus
overhead of configuring bus locking protocol degrade not only the
performance of one CPU but overall system performance.

If operand is cacheable and completely contained in one cache line, atomic
operation is optimized by less expensive cache locking on Intel P6 and
recent processors. If split lock is detected and the two cache lines in the
operand can be merged into one cache line, cache locking instead of
more expensive bus locking will be used for atomic operation. Removing
split lock can improve overall performance.

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

More information about split lock, bus locking, and cache locking can be
found in the latest Intel 64 and IA-32 Architecture Software Developer's
Manual.

==Split lock detection==

Currently we can trace split lock event counter sq_misc.split_lock
for debug purpose. But for system deployed in the field, this event
counter after the fact is insufficient. We need a mechanism that
detects split lock before it happens to ensure that bus lock is never
incurred due to split lock.

Intel introduces a mechanism to detect split lock via Alignment Check
(#AC) exception before badly aligned atomic instructions might impact
whole system performance in Tremont and other future processors. 

This capability is critical for real time system designers who build
consolidated real time systems. These systems run hard real time
code on some cores and run "untrusted" user processes on some
other cores. The hard real time cannot afford to have any bus lock from
the untrusted processes to hurt real time performance. To date the
designers have been unable to deploy these solutions as they have no
way to prevent the "untrusted" user code from generating split lock and
bus lock to block the hard real time code to access memory during bus
locking.

This capability may also find usage in cloud. A user process with split
lock running in one guest can block other cores from accessing shared
memory during its split locked memory access. That may cause overall
system performance degradation.

Split lock may open a security hole where malicious user code may slow
down overall system by executing instructions with split lock.

==Enumerate #AC for split lock==

A 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 exception for split locked accesses at all CPL.

The split lock detection feature is enumerated through bit 5 in
MSR IA32_CORE_CAPABILITY (0xCF). The MSR 0xCF itself is enumerated by
CPUID.(EAX=0x7,ECX=0):EDX[30].

The enumeration method is published in the latest Intel Software
Developer's Manual.

A few processors don't follow the above enumeration. For those processors,
we can enable #AC for split lock based on their FMS numbers. We will
enable split lock detection on those processors in the future once their
FMS numbers are public.

==Handle Split Lock===

There may be different considerations to handle split lock, e.g. how
to handle split lock issue in firmware after kernel enables #AC for
split lock.

But we use a simple way to handle split lock which is suggested
by Thomas Gleixner and Dave Hansen:

- If split lock happens in kernel, a warning is issued and #AC for
split lock is disabled on the local CPU. The split lock issue should
be fixed in kernel.

- If split lock happens in user process, the process is killed by
SIGBUS. Unless the issue is fixed, the process cannot run in the
system.

- If split lock happens in firmware, system may hang in firmware. The
issue should be fixed in firmware.

- Enable split lock detection by default once the feature is enumerated.

- Disable split lock detection by "clearcpuid=split_lock_detect" during
boot time.

- Disable/enable split lock detection by interface
/sys/devices/system/cpu/split_lock_detect during run time.

==Expose to guest==

To expose this feature to guest, we need to
1. report the new CPUID bit to guest.
2. emulate IA32_CORE_CAPABILITIES MSR.
3. emulate TEST_CTL MSR. We must do it. Since this patch
series enable split lock detection in host kernel by default, if we do
not emualte TEST_CTL MSR for guest, guest will run with the value set
by host without knowing that. So guest will run with split lock detection
enabled due to the host's value. Thus guest running with buggy firmware
and old kernel will fail because they lack the ability to handle #AC for
split lock. So we should emulate TEST_CTL MSR and separate its value
between host and guest.

==Patches==
Patch 1-4: Fix a few existing split lock issues.
Patch 5-8: Enumerate features and define MSR TEST_CTL.
Patch 9: Handle #AC for split lock
Patch 10-11: Enable split lock detection in KVM.
Patch 12: Enable #AC for split lock by default after #AC handler and KVM
are installed
Patch 13: /sys interface to enable/disable split lock detection
Patch 14-18: Extend "clearcpuid=" option.

==Changelog==
v5:
- Fix wlcore issue from Paolo Bonzini
- Fix b44 issue from Peter Zijlstra
- Change init sequence by Dave Hansen
- Fix KVM issues from Paolo Bonzini
- Re-order patch sequence

v4:
- Remove "setcpuid=" option
- Enable IA32_CORE_CAPABILITY enumeration for split lock
- Handle CPUID faulting by Peter Zijlstra
- Enable /sys interface to enable/disable split lock detection

v3:
- Handle split lock as suggested by Thomas Gleixner.
- Fix a few potential spit lock issues suggested by Thomas Gleixner.
- Support kernel option "setcpuid=" suggested by Dave Hanson and Thomas
Gleixner.
- Support flag string in "clearcpuid=" suggested by Dave Hanson and
Thomas Gleixner.

v2:
- Remove code that handles split lock issue in firmware and fix
x86_capability issue mainly based on comments from Thomas Gleixner and
Peter Zijlstra.

In previous version:
Comments from Dave Hansen:
- Enumerate feature in X86_FEATURE_SPLIT_LOCK_AC
- Separate #AC handler from do_error_trap
- Use CONFIG to configure inherit BIOS setting, enable, or disable split
  lock. Remove kernel parameter "split_lock_ac="
- Change config interface to debugfs from sysfs
- Fix a few bisectable issues
- Other changes.

Comment from Tony Luck and Dave Hansen:
- Dump right information in #AC handler

Comment from Alan Cox and Dave Hansen:
- Description of split lock in patch 0

Others:
- Remove tracing because we can trace split lock in existing
  sq_misc.split_lock.
- Add CONFIG to configure either panic or re-execute faulting instruction
  for split lock in kernel.
- other minor changes.

Fenghua Yu (13):
  x86/common: Align cpu_caps_cleared and cpu_caps_set to unsigned long
  x86/split_lock: Align x86_capability to unsigned long to avoid split
    locked access
  x86/cpufeatures: Enumerate IA32_CORE_CAPABILITIES MSR
  x86/msr-index: Define IA32_CORE_CAPABILITY MSR and #AC exception for
    split lock bit
  x86/split_lock: Enumerate #AC for split lock by MSR
    IA32_CORE_CAPABILITY
  x86/split_lock: Define MSR TEST_CTL register
  x86/split_lock: Handle #AC exception for split lock
  x86/split_lock: Enable #AC for split lock by default
  x86/split_lock: Add a sysfs interface to allow user to enable or
    disable split lock detection on all CPUs during run time
  x86/clearcpuid: Support multiple clearcpuid options
  x86/clearcpuid: Support feature flag string in kernel option
    clearcpuid
  x86/clearcpuid: Apply cleared feature bits that are forced set before
  Change document for kernel option clearcpuid

Paolo Bonzini (1):
  wlcore: simplify/fix/optimize reg_ch_conf_pending operations

Peter Zijlstra (2):
  drivers/net/b44: Align pwol_mask to unsigned long for better
    performance
  x86/clearcpuid: Clear CPUID bit in CPUID faulting

Xiaoyao Li (2):
  kvm/x86: Emulate MSR IA32_CORE_CAPABILITY
  kvm/vmx: Emulate MSR TEST_CTL

 .../admin-guide/kernel-parameters.txt         |   6 +-
 arch/x86/include/asm/cmdline.h                |   3 +
 arch/x86/include/asm/cpu.h                    |   7 +
 arch/x86/include/asm/cpufeature.h             |   5 +
 arch/x86/include/asm/cpufeatures.h            |   2 +
 arch/x86/include/asm/kvm_host.h               |   2 +
 arch/x86/include/asm/msr-index.h              |   7 +
 arch/x86/include/asm/processor.h              |   4 +-
 arch/x86/kernel/cpu/common.c                  |  14 +-
 arch/x86/kernel/cpu/cpuid-deps.c              | 157 ++++++++++++----
 arch/x86/kernel/cpu/intel.c                   | 177 +++++++++++++++++-
 arch/x86/kernel/cpu/scattered.c               |  17 ++
 arch/x86/kernel/fpu/init.c                    |  33 +++-
 arch/x86/kernel/process.c                     |   3 +
 arch/x86/kernel/traps.c                       |  42 +++++
 arch/x86/kvm/cpuid.c                          |   6 +
 arch/x86/kvm/vmx/vmx.c                        |  35 ++++
 arch/x86/kvm/vmx/vmx.h                        |   1 +
 arch/x86/kvm/x86.c                            |  39 ++++
 arch/x86/lib/cmdline.c                        |  17 +-
 drivers/net/ethernet/broadcom/b44.c           |   4 +-
 drivers/net/wireless/ti/wlcore/cmd.c          |  15 +-
 drivers/net/wireless/ti/wlcore/wlcore.h       |   4 +-
 23 files changed, 526 insertions(+), 74 deletions(-)

-- 
2.19.1


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

* [PATCH v5 01/18] x86/common: Align cpu_caps_cleared and cpu_caps_set to unsigned long
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 02/18] drivers/net/b44: Align pwol_mask to unsigned long for better performance Fenghua Yu
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

cpu_caps_cleared and cpu_caps_set may not be aligned to unsigned long.
Atomic operations (i.e. set_bit and clear_bit) on the bitmaps may access
two cache lines (a.k.a. split lock) and lock bus to block all memory
accesses from other processors to ensure atomicity.

To avoid the overall performance degradation from the bus locking, align
the two variables to unsigned long.

Defining the variables as unsigned long may also fix the issue because
they are naturally aligned to unsigned long. But that needs additional
code changes. Adding __aligned(unsigned long) are simpler fixes.

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

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index cb28e98a0659..51ab37ba5f64 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -488,8 +488,9 @@ static const char *table_lookup_model(struct cpuinfo_x86 *c)
 	return NULL;		/* Not found */
 }
 
-__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS];
-__u32 cpu_caps_set[NCAPINTS + NBUGINTS];
+/* Unsigned long alignment to avoid split lock in atomic bitmap ops */
+__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
+__u32 cpu_caps_set[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
 
 void load_percpu_segment(int cpu)
 {
-- 
2.19.1


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

* [PATCH v5 02/18] drivers/net/b44: Align pwol_mask to unsigned long for better performance
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 01/18] x86/common: Align cpu_caps_cleared and cpu_caps_set to unsigned long Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 03/18] wlcore: simplify/fix/optimize reg_ch_conf_pending operations Fenghua Yu
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

From: Peter Zijlstra <peterz@infradead.org>

A bit in pwol_mask is set in b44_magic_pattern by atomic set_bit.
But since pwol_mask is local and never exposed to concurrency, there is
no need to set bit in pwol_mask atomically.

set_bit sets the bit in a single unsigned long location. Because pwol_mask
may not be aligned to unsigned long, the location may cross two cache
lines and accessing the location degradates performance. On x86, accessing
two cache lines in locked instruction in set_bit is called split lock and
can cause overall performance degradation.

So we use non atomic __set_bit to set pwol_mask bits. __set_bit won't
hit split lock issue on x86.

Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 drivers/net/ethernet/broadcom/b44.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index 97ab0dd25552..5738ab963dfb 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -1520,7 +1520,7 @@ static int b44_magic_pattern(u8 *macaddr, u8 *ppattern, u8 *pmask, int offset)
 
 	memset(ppattern + offset, 0xff, magicsync);
 	for (j = 0; j < magicsync; j++)
-		set_bit(len++, (unsigned long *) pmask);
+		__set_bit(len++, (unsigned long *)pmask);
 
 	for (j = 0; j < B44_MAX_PATTERNS; j++) {
 		if ((B44_PATTERN_SIZE - len) >= ETH_ALEN)
@@ -1532,7 +1532,7 @@ static int b44_magic_pattern(u8 *macaddr, u8 *ppattern, u8 *pmask, int offset)
 		for (k = 0; k< ethaddr_bytes; k++) {
 			ppattern[offset + magicsync +
 				(j * ETH_ALEN) + k] = macaddr[k];
-			set_bit(len++, (unsigned long *) pmask);
+			__set_bit(len++, (unsigned long *)pmask);
 		}
 	}
 	return len - 1;
-- 
2.19.1


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

* [PATCH v5 03/18] wlcore: simplify/fix/optimize reg_ch_conf_pending operations
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 01/18] x86/common: Align cpu_caps_cleared and cpu_caps_set to unsigned long Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 02/18] drivers/net/b44: Align pwol_mask to unsigned long for better performance Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-14 13:16   ` Kalle Valo
  2019-03-12 23:00 ` [PATCH v5 04/18] x86/split_lock: Align x86_capability to unsigned long to avoid split locked access Fenghua Yu
                   ` (14 subsequent siblings)
  17 siblings, 1 reply; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

From: Paolo Bonzini <pbonzini@redhat.com>

Bitmaps are defined on unsigned longs, so the usage of u32[2] in the
wlcore driver is incorrect.  As noted by Peter Zijlstra, casting arrays
to a bitmap is incorrect for big-endian architectures.

When looking at it I observed that:

- operations on reg_ch_conf_pending is always under the wl_lock mutex,
so set_bit is overkill

- the only case where reg_ch_conf_pending is accessed a u32 at a time is
unnecessary too.

This patch cleans up everything in this area, and changes tmp_ch_bitmap
to have the proper alignment.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 drivers/net/wireless/ti/wlcore/cmd.c    | 15 ++++++---------
 drivers/net/wireless/ti/wlcore/wlcore.h |  4 ++--
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
index 348be0aed97e..3b9266989afe 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.c
+++ b/drivers/net/wireless/ti/wlcore/cmd.c
@@ -1700,14 +1700,14 @@ void wlcore_set_pending_regdomain_ch(struct wl1271 *wl, u16 channel,
 	ch_bit_idx = wlcore_get_reg_conf_ch_idx(band, channel);
 
 	if (ch_bit_idx >= 0 && ch_bit_idx <= WL1271_MAX_CHANNELS)
-		set_bit(ch_bit_idx, (long *)wl->reg_ch_conf_pending);
+		__set_bit_le(ch_bit_idx, (long *)wl->reg_ch_conf_pending);
 }
 
 int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl)
 {
 	struct wl12xx_cmd_regdomain_dfs_config *cmd = NULL;
 	int ret = 0, i, b, ch_bit_idx;
-	u32 tmp_ch_bitmap[2];
+	u32 tmp_ch_bitmap[2] __aligned(sizeof(unsigned long));
 	struct wiphy *wiphy = wl->hw->wiphy;
 	struct ieee80211_supported_band *band;
 	bool timeout = false;
@@ -1717,7 +1717,7 @@ int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl)
 
 	wl1271_debug(DEBUG_CMD, "cmd reg domain config");
 
-	memset(tmp_ch_bitmap, 0, sizeof(tmp_ch_bitmap));
+	memcpy(tmp_ch_bitmap, wl->reg_ch_conf_pending, sizeof(tmp_ch_bitmap));
 
 	for (b = NL80211_BAND_2GHZ; b <= NL80211_BAND_5GHZ; b++) {
 		band = wiphy->bands[b];
@@ -1738,13 +1738,10 @@ int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl)
 			if (ch_bit_idx < 0)
 				continue;
 
-			set_bit(ch_bit_idx, (long *)tmp_ch_bitmap);
+			__set_bit_le(ch_bit_idx, (long *)tmp_ch_bitmap);
 		}
 	}
 
-	tmp_ch_bitmap[0] |= wl->reg_ch_conf_pending[0];
-	tmp_ch_bitmap[1] |= wl->reg_ch_conf_pending[1];
-
 	if (!memcmp(tmp_ch_bitmap, wl->reg_ch_conf_last, sizeof(tmp_ch_bitmap)))
 		goto out;
 
@@ -1754,8 +1751,8 @@ int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl)
 		goto out;
 	}
 
-	cmd->ch_bit_map1 = cpu_to_le32(tmp_ch_bitmap[0]);
-	cmd->ch_bit_map2 = cpu_to_le32(tmp_ch_bitmap[1]);
+	cmd->ch_bit_map1 = tmp_ch_bitmap[0];
+	cmd->ch_bit_map2 = tmp_ch_bitmap[1];
 	cmd->dfs_region = wl->dfs_region;
 
 	wl1271_debug(DEBUG_CMD,
diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h b/drivers/net/wireless/ti/wlcore/wlcore.h
index dd14850b0603..870eea3e7a27 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore.h
@@ -320,9 +320,9 @@ struct wl1271 {
 	bool watchdog_recovery;
 
 	/* Reg domain last configuration */
-	u32 reg_ch_conf_last[2]  __aligned(8);
+	DECLARE_BITMAP(reg_ch_conf_last, 64);
 	/* Reg domain pending configuration */
-	u32 reg_ch_conf_pending[2];
+	DECLARE_BITMAP(reg_ch_conf_pending, 64);
 
 	/* Pointer that holds DMA-friendly block for the mailbox */
 	void *mbox;
-- 
2.19.1


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

* [PATCH v5 04/18] x86/split_lock: Align x86_capability to unsigned long to avoid split locked access
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (2 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 03/18] wlcore: simplify/fix/optimize reg_ch_conf_pending operations Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 05/18] x86/cpufeatures: Enumerate IA32_CORE_CAPABILITIES MSR Fenghua Yu
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

set_cpu_cap() calls locked BTS and clear_cpu_cap() calls locked BTR to
operate on bitmap defined in x86_capability.

Locked BTS/BTR accesses a single unsigned long location. In 64-bit mode,
the location is at:
base address of x86_capability + (bit offset in x86_capability % 64) * 8

Since base address of x86_capability may not aligned to unsigned long,
the single unsigned long location may cross two cache lines and
accessing the location by locked BTS/BTR introductions will trigger #AC.

To fix the split lock issue, align x86_capability to unsigned long so that
the location will be always within one cache line.

Changing x86_capability[]'s type to unsigned long may also fix the issue
because x86_capability[] will be naturally aligned to unsigned long. But
this needs additional code changes. So we choose the simpler solution
by enforcing alignment using __aligned(unsigned long).

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

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 2bb3a648fc12..b92296595fbe 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -93,7 +93,9 @@ struct cpuinfo_x86 {
 	__u32			extended_cpuid_level;
 	/* Maximum supported CPUID level, -1=no CPUID: */
 	int			cpuid_level;
-	__u32			x86_capability[NCAPINTS + NBUGINTS];
+	/* Unsigned long alignment to avoid split lock in atomic bitmap ops */
+	__u32			x86_capability[NCAPINTS + NBUGINTS]
+				__aligned(sizeof(unsigned long));
 	char			x86_vendor_id[16];
 	char			x86_model_id[64];
 	/* in KB - valid for CPUS which support this call: */
-- 
2.19.1


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

* [PATCH v5 05/18] x86/cpufeatures: Enumerate IA32_CORE_CAPABILITIES MSR
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (3 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 04/18] x86/split_lock: Align x86_capability to unsigned long to avoid split locked access Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 06/18] x86/msr-index: Define IA32_CORE_CAPABILITY MSR and #AC exception for split lock bit Fenghua Yu
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

MSR register IA32_CORE_CAPABILITIES (0xCF) contains bits that enumerate
some model specific features.

The MSR 0xCF itself is enumerated by CPUID.(EAX=0x7,ECX=0):EDX[30].
When this bit is 1, the MSR 0xCF exists.

Detailed information for the CPUID bit and the MSR can be found in the
latest Intel Architecture Instruction Set Extensions and Future Features
Programming Reference.

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

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 981ff9479648..65b6af6cdc19 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -350,6 +350,7 @@
 #define X86_FEATURE_INTEL_STIBP		(18*32+27) /* "" Single Thread Indirect Branch Predictors */
 #define X86_FEATURE_FLUSH_L1D		(18*32+28) /* Flush L1D cache */
 #define X86_FEATURE_ARCH_CAPABILITIES	(18*32+29) /* IA32_ARCH_CAPABILITIES MSR (Intel) */
+#define X86_FEATURE_CORE_CAPABILITY	(18*32+30) /* IA32_CORE_CAPABILITY MSR */
 #define X86_FEATURE_SPEC_CTRL_SSBD	(18*32+31) /* "" Speculative Store Bypass Disable */
 
 /*
-- 
2.19.1


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

* [PATCH v5 06/18] x86/msr-index: Define IA32_CORE_CAPABILITY MSR and #AC exception for split lock bit
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (4 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 05/18] x86/cpufeatures: Enumerate IA32_CORE_CAPABILITIES MSR Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 07/18] x86/split_lock: Enumerate #AC for split lock by MSR IA32_CORE_CAPABILITY Fenghua Yu
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

A new IA32_CORE_CAPABILITY MSR (0xCF) is defined. Each bit in
the MSR enumerates a model specific feature. Currently bit 5 enumerates
#AC exception for split locked accesses. When bit 5 is 1, split locked
accesses will generate #AC exception. When bit 5 is 0, split locked
accesses will not generate #AC exception.

Please check the latest Intel Architecture Instruction Set Extensions
and Future Features Programming Reference for more detailed information
on the MSR and the split lock bit.

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

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index ca5bc0eacb95..f65ef6f783d2 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -59,6 +59,9 @@
 #define MSR_PLATFORM_INFO_CPUID_FAULT_BIT	31
 #define MSR_PLATFORM_INFO_CPUID_FAULT		BIT_ULL(MSR_PLATFORM_INFO_CPUID_FAULT_BIT)
 
+#define MSR_IA32_CORE_CAPABILITY	0x000000cf
+#define CORE_CAP_SPLIT_LOCK_DETECT	BIT(5)     /* Detect split lock */
+
 #define MSR_PKG_CST_CONFIG_CONTROL	0x000000e2
 #define NHM_C3_AUTO_DEMOTE		(1UL << 25)
 #define NHM_C1_AUTO_DEMOTE		(1UL << 26)
-- 
2.19.1


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

* [PATCH v5 07/18] x86/split_lock: Enumerate #AC for split lock by MSR IA32_CORE_CAPABILITY
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (5 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 06/18] x86/msr-index: Define IA32_CORE_CAPABILITY MSR and #AC exception for split lock bit Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:52   ` Dave Hansen
  2019-03-12 23:00 ` [PATCH v5 08/18] x86/split_lock: Define MSR TEST_CTL register Fenghua Yu
                   ` (10 subsequent siblings)
  17 siblings, 1 reply; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

Bits in MSR IA32_CORE_CAPABILITY enumerate features that are not
enumerated through CPUID. Currently bit 5 is defined to enumerate
feature of #AC for split lock accesses. All other bits are reserved now.

When the bit 5 is 1, the feature is supported and feature bit
X86_FEATURE_SPLIT_LOCK_DETECT is set. Otherwise, the feature is not
available.

The MSR IA32_CORE_CAPABILITY itself is enumerated by
CPUID.(EAX=0x7,ECX=0):EDX[30].

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/cpu.h         |  5 ++
 arch/x86/include/asm/cpufeatures.h |  1 +
 arch/x86/kernel/cpu/common.c       |  2 +
 arch/x86/kernel/cpu/cpuid-deps.c   | 79 +++++++++++++++---------------
 arch/x86/kernel/cpu/intel.c        | 22 +++++++++
 5 files changed, 70 insertions(+), 39 deletions(-)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index adc6cc86b062..4e03f53fc079 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_CPU_SUP_INTEL
+void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c);
+#else
+static inline void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c) {}
+#endif
 #endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 65b6af6cdc19..661719cbf458 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -221,6 +221,7 @@
 #define X86_FEATURE_ZEN			( 7*32+28) /* "" CPU is AMD family 0x17 (Zen) */
 #define X86_FEATURE_L1TF_PTEINV		( 7*32+29) /* "" L1TF workaround PTE inversion */
 #define X86_FEATURE_IBRS_ENHANCED	( 7*32+30) /* Enhanced IBRS */
+#define X86_FEATURE_SPLIT_LOCK_DETECT	( 7*32+31) /* #AC for split lock */
 
 /* Virtualization flags: Linux defined, word 8 */
 #define X86_FEATURE_TPR_SHADOW		( 8*32+ 0) /* Intel TPR Shadow */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 51ab37ba5f64..16b091bb62bd 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1105,6 +1105,8 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
 
 	cpu_set_bug_bits(c);
 
+	cpu_set_core_cap_bits(c);
+
 	fpu__init_system(c);
 
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 2c0bd38a44ab..3d633f67fbd7 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -20,45 +20,46 @@ struct cpuid_dep {
  * but it's difficult to tell that to the init reference checker.
  */
 static const struct cpuid_dep cpuid_deps[] = {
-	{ X86_FEATURE_XSAVEOPT,		X86_FEATURE_XSAVE     },
-	{ X86_FEATURE_XSAVEC,		X86_FEATURE_XSAVE     },
-	{ X86_FEATURE_XSAVES,		X86_FEATURE_XSAVE     },
-	{ X86_FEATURE_AVX,		X86_FEATURE_XSAVE     },
-	{ X86_FEATURE_PKU,		X86_FEATURE_XSAVE     },
-	{ X86_FEATURE_MPX,		X86_FEATURE_XSAVE     },
-	{ X86_FEATURE_XGETBV1,		X86_FEATURE_XSAVE     },
-	{ X86_FEATURE_FXSR_OPT,		X86_FEATURE_FXSR      },
-	{ X86_FEATURE_XMM,		X86_FEATURE_FXSR      },
-	{ X86_FEATURE_XMM2,		X86_FEATURE_XMM       },
-	{ X86_FEATURE_XMM3,		X86_FEATURE_XMM2      },
-	{ X86_FEATURE_XMM4_1,		X86_FEATURE_XMM2      },
-	{ X86_FEATURE_XMM4_2,		X86_FEATURE_XMM2      },
-	{ X86_FEATURE_XMM3,		X86_FEATURE_XMM2      },
-	{ X86_FEATURE_PCLMULQDQ,	X86_FEATURE_XMM2      },
-	{ X86_FEATURE_SSSE3,		X86_FEATURE_XMM2,     },
-	{ X86_FEATURE_F16C,		X86_FEATURE_XMM2,     },
-	{ X86_FEATURE_AES,		X86_FEATURE_XMM2      },
-	{ X86_FEATURE_SHA_NI,		X86_FEATURE_XMM2      },
-	{ X86_FEATURE_FMA,		X86_FEATURE_AVX       },
-	{ X86_FEATURE_AVX2,		X86_FEATURE_AVX,      },
-	{ X86_FEATURE_AVX512F,		X86_FEATURE_AVX,      },
-	{ X86_FEATURE_AVX512IFMA,	X86_FEATURE_AVX512F   },
-	{ X86_FEATURE_AVX512PF,		X86_FEATURE_AVX512F   },
-	{ X86_FEATURE_AVX512ER,		X86_FEATURE_AVX512F   },
-	{ X86_FEATURE_AVX512CD,		X86_FEATURE_AVX512F   },
-	{ X86_FEATURE_AVX512DQ,		X86_FEATURE_AVX512F   },
-	{ X86_FEATURE_AVX512BW,		X86_FEATURE_AVX512F   },
-	{ X86_FEATURE_AVX512VL,		X86_FEATURE_AVX512F   },
-	{ X86_FEATURE_AVX512VBMI,	X86_FEATURE_AVX512F   },
-	{ X86_FEATURE_AVX512_VBMI2,	X86_FEATURE_AVX512VL  },
-	{ X86_FEATURE_GFNI,		X86_FEATURE_AVX512VL  },
-	{ X86_FEATURE_VAES,		X86_FEATURE_AVX512VL  },
-	{ X86_FEATURE_VPCLMULQDQ,	X86_FEATURE_AVX512VL  },
-	{ X86_FEATURE_AVX512_VNNI,	X86_FEATURE_AVX512VL  },
-	{ X86_FEATURE_AVX512_BITALG,	X86_FEATURE_AVX512VL  },
-	{ X86_FEATURE_AVX512_4VNNIW,	X86_FEATURE_AVX512F   },
-	{ X86_FEATURE_AVX512_4FMAPS,	X86_FEATURE_AVX512F   },
-	{ X86_FEATURE_AVX512_VPOPCNTDQ, X86_FEATURE_AVX512F   },
+	{ X86_FEATURE_XSAVEOPT,			X86_FEATURE_XSAVE     },
+	{ X86_FEATURE_XSAVEC,			X86_FEATURE_XSAVE     },
+	{ X86_FEATURE_XSAVES,			X86_FEATURE_XSAVE     },
+	{ X86_FEATURE_AVX,			X86_FEATURE_XSAVE     },
+	{ X86_FEATURE_PKU,			X86_FEATURE_XSAVE     },
+	{ X86_FEATURE_MPX,			X86_FEATURE_XSAVE     },
+	{ X86_FEATURE_XGETBV1,			X86_FEATURE_XSAVE     },
+	{ X86_FEATURE_FXSR_OPT,			X86_FEATURE_FXSR      },
+	{ X86_FEATURE_XMM,			X86_FEATURE_FXSR      },
+	{ X86_FEATURE_XMM2,			X86_FEATURE_XMM       },
+	{ X86_FEATURE_XMM3,			X86_FEATURE_XMM2      },
+	{ X86_FEATURE_XMM4_1,			X86_FEATURE_XMM2      },
+	{ X86_FEATURE_XMM4_2,			X86_FEATURE_XMM2      },
+	{ X86_FEATURE_XMM3,			X86_FEATURE_XMM2      },
+	{ X86_FEATURE_PCLMULQDQ,		X86_FEATURE_XMM2      },
+	{ X86_FEATURE_SSSE3,			X86_FEATURE_XMM2,     },
+	{ X86_FEATURE_F16C,			X86_FEATURE_XMM2,     },
+	{ X86_FEATURE_AES,			X86_FEATURE_XMM2      },
+	{ X86_FEATURE_SHA_NI,			X86_FEATURE_XMM2      },
+	{ X86_FEATURE_FMA,			X86_FEATURE_AVX       },
+	{ X86_FEATURE_AVX2,			X86_FEATURE_AVX,      },
+	{ X86_FEATURE_AVX512F,			X86_FEATURE_AVX,      },
+	{ X86_FEATURE_AVX512IFMA,		X86_FEATURE_AVX512F   },
+	{ X86_FEATURE_AVX512PF,			X86_FEATURE_AVX512F   },
+	{ X86_FEATURE_AVX512ER,			X86_FEATURE_AVX512F   },
+	{ X86_FEATURE_AVX512CD,			X86_FEATURE_AVX512F   },
+	{ X86_FEATURE_AVX512DQ,			X86_FEATURE_AVX512F   },
+	{ X86_FEATURE_AVX512BW,			X86_FEATURE_AVX512F   },
+	{ X86_FEATURE_AVX512VL,			X86_FEATURE_AVX512F   },
+	{ X86_FEATURE_AVX512VBMI,		X86_FEATURE_AVX512F   },
+	{ X86_FEATURE_AVX512_VBMI2,		X86_FEATURE_AVX512VL  },
+	{ X86_FEATURE_GFNI,			X86_FEATURE_AVX512VL  },
+	{ X86_FEATURE_VAES,			X86_FEATURE_AVX512VL  },
+	{ X86_FEATURE_VPCLMULQDQ,		X86_FEATURE_AVX512VL  },
+	{ X86_FEATURE_AVX512_VNNI,		X86_FEATURE_AVX512VL  },
+	{ X86_FEATURE_AVX512_BITALG,		X86_FEATURE_AVX512VL  },
+	{ X86_FEATURE_AVX512_4VNNIW,		X86_FEATURE_AVX512F   },
+	{ X86_FEATURE_AVX512_4FMAPS,		X86_FEATURE_AVX512F   },
+	{ X86_FEATURE_AVX512_VPOPCNTDQ,		X86_FEATURE_AVX512F   },
+	{ X86_FEATURE_SPLIT_LOCK_DETECT,	X86_FEATURE_CORE_CAPABILITY},
 	{}
 };
 
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index fc3c07fe7df5..889390d51c17 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -1029,3 +1029,25 @@ static const struct cpu_dev intel_cpu_dev = {
 
 cpu_dev_register(intel_cpu_dev);
 
+/**
+ * set_cpu_core_cap_bits - enumerate features supported in IA32_CORE_CAPABILITY
+ * @c: pointer to cpuinfo_x86
+ *
+ * Return: void
+ */
+void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c)
+{
+	u64 ia32_core_cap = 0;
+
+	if (!cpu_has(c, X86_FEATURE_CORE_CAPABILITY))
+		return;
+
+	/*
+	 * If MSR_IA32_CORE_CAPABILITY exists, enumerate features that are
+	 * reported in the MSR.
+	 */
+	rdmsrl(MSR_IA32_CORE_CAPABILITY, ia32_core_cap);
+
+	if (ia32_core_cap & CORE_CAP_SPLIT_LOCK_DETECT)
+		setup_force_cpu_cap(X86_FEATURE_SPLIT_LOCK_DETECT);
+}
-- 
2.19.1


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

* [PATCH v5 08/18] x86/split_lock: Define MSR TEST_CTL register
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (6 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 07/18] x86/split_lock: Enumerate #AC for split lock by MSR IA32_CORE_CAPABILITY Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 09/18] x86/split_lock: Handle #AC exception for split lock Fenghua Yu
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

Setting bit 29 in MSR TEST_CTL register 0x33 enables split lock issue
detection and clearing the bit disables split lock issue detection.

Define the MSR and the bit. The definitions will be used in enabling or
disabling split lock detection.

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

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index f65ef6f783d2..25fa808de9e2 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -39,6 +39,10 @@
 
 /* Intel MSRs. Some also available on other CPUs */
 
+#define MSR_TEST_CTL					0x00000033
+#define TEST_CTL_ENABLE_SPLIT_LOCK_DETECT_SHIFT	29
+#define TEST_CTL_ENABLE_SPLIT_LOCK_DETECT		BIT(29)
+
 #define MSR_IA32_SPEC_CTRL		0x00000048 /* Speculation Control */
 #define SPEC_CTRL_IBRS			(1 << 0)   /* Indirect Branch Restricted Speculation */
 #define SPEC_CTRL_STIBP_SHIFT		1	   /* Single Thread Indirect Branch Predictor (STIBP) bit */
-- 
2.19.1


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

* [PATCH v5 09/18] x86/split_lock: Handle #AC exception for split lock
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (7 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 08/18] x86/split_lock: Define MSR TEST_CTL register Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:51   ` Dave Hansen
  2019-03-12 23:00 ` [PATCH v5 10/18] kvm/x86: Emulate MSR IA32_CORE_CAPABILITY Fenghua Yu
                   ` (8 subsequent siblings)
  17 siblings, 1 reply; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

There may be different considerations on how to handle #AC for split lock,
e.g. how to handle system hang caused by split lock issue in firmware,
how to emulate faulting instruction, etc. We use a simple method to
handle user and kernel split lock and may extend the method in the future.

When #AC exception for split lock is triggered from user process, the
process is killed by SIGBUS. To execute the process properly, user
application developer needs to fix the split lock issue.

When #AC exception for split lock is triggered from a kernel instruction,
disable #AC for split lock on local CPU and warn the split lock issue.
After the exception, the faulting instruction will be executed and kernel
execution continues. #AC for split lock is only disabled on the local CPU
not globally. It will be re-enabled if the CPU is offline and then online.

Kernel developer should check the warning, which contains helpful faulting
address, context, and callstack info, and fix the split lock issue
one by one. Then further split lock may be captured and fixed.

After bit 29 in MSR_TEST_CTL is set as one in kernel, firmware inherits
the setting when firmware is executed in S4, S5, run time services, SMI,
etc. Split lock issue in firmware triggers #AC and may hang the system
depending on how firmware handles the #AC. It's up to firmware developer
to fix the split lock issues in firmware.

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

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index d26f9e9c3d83..69b6233e783e 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>
@@ -293,7 +294,37 @@ DO_ERROR(X86_TRAP_OLD_MF, SIGFPE,           0, NULL, "coprocessor segment overru
 DO_ERROR(X86_TRAP_TS,     SIGSEGV,          0, NULL, "invalid TSS",         invalid_TSS)
 DO_ERROR(X86_TRAP_NP,     SIGBUS,           0, NULL, "segment not present", segment_not_present)
 DO_ERROR(X86_TRAP_SS,     SIGBUS,           0, NULL, "stack segment",       stack_segment)
+#ifndef CONFIG_CPU_SUP_INTEL
 DO_ERROR(X86_TRAP_AC,     SIGBUS,  BUS_ADRALN, NULL, "alignment check",     alignment_check)
+#else
+dotraplinkage void do_alignment_check(struct pt_regs *regs, long error_code)
+{
+	unsigned int trapnr = X86_TRAP_AC;
+	char str[] = "alignment check";
+	int signr = SIGBUS;
+
+	RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
+
+	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr) !=
+			NOTIFY_STOP) {
+		cond_local_irq_enable(regs);
+		if (!user_mode(regs)) {
+			/*
+			 * Only split lock can generate #AC from kernel. Warn
+			 * and disable #AC for split lock on current CPU.
+			 */
+			msr_clear_bit(MSR_TEST_CTL,
+				      TEST_CTL_ENABLE_SPLIT_LOCK_DETECT_SHIFT);
+			WARN_ONCE(1, "A split lock issue is detected.\n");
+
+			return;
+		}
+		/* Handle #AC generated from user code. */
+		do_trap(X86_TRAP_AC, SIGBUS, "alignment check", regs,
+			error_code, BUS_ADRALN, NULL);
+	}
+}
+#endif
 #undef IP
 
 #ifdef CONFIG_VMAP_STACK
-- 
2.19.1


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

* [PATCH v5 10/18] kvm/x86: Emulate MSR IA32_CORE_CAPABILITY
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (8 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 09/18] x86/split_lock: Handle #AC exception for split lock Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-13  8:15   ` Paolo Bonzini
  2019-03-12 23:00 ` [PATCH v5 11/18] kvm/vmx: Emulate MSR TEST_CTL Fenghua Yu
                   ` (7 subsequent siblings)
  17 siblings, 1 reply; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Xiaoyao Li, Fenghua Yu

From: Xiaoyao Li <xiaoyao.li@linux.intel.com>

MSR IA32_CORE_CAPABILITY is a feature-enumerating MSR, bit 5 of which
reports the capability of enabling detection of split locks (will be
supported on future processors based on Tremont microarchitecture and
later).

CPUID.(EAX=7H,ECX=0):EDX[30] will enumerate the presence of the
IA32_CORE_CAPABILITY MSR.

Please check the latest Intel 64 and IA-32 Architectures Software
Developer's Manual for more detailed information on the MSR and
the split lock bit.

Since MSR_IA32_CORE_CAPABILITY is a feature-enumerating MSR, we can
emulate it in software regardless of host's capability. What we need to
do is to set the right value of it to report the capability of guest.

In this patch we just set the guest's core_capability as 0, because we
haven't added support of the features it indicates to guest. It's for
bisectability.

Signed-off-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/kvm_host.h |  2 ++
 arch/x86/kvm/cpuid.c            |  6 ++++++
 arch/x86/kvm/x86.c              | 24 ++++++++++++++++++++++++
 3 files changed, 32 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 180373360e34..2c53df4a5a2a 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -570,6 +570,7 @@ struct kvm_vcpu_arch {
 	bool tpr_access_reporting;
 	u64 ia32_xss;
 	u64 microcode_version;
+	u64 core_capability;
 
 	/*
 	 * Paging state of the vcpu
@@ -1527,6 +1528,7 @@ int kvm_pv_send_ipi(struct kvm *kvm, unsigned long ipi_bitmap_low,
 		    unsigned long icr, int op_64_bit);
 
 u64 kvm_get_arch_capabilities(void);
+u64 kvm_get_core_capability(void);
 void kvm_define_shared_msr(unsigned index, u32 msr);
 int kvm_set_shared_msr(unsigned index, u64 val, u64 mask);
 
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index c07958b59f50..b7fb1822a1e5 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -505,6 +505,12 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 			 * if the host doesn't support it.
 			 */
 			entry->edx |= F(ARCH_CAPABILITIES);
+			/*
+			 * Since we emulate MSR IA32_CORE_CAPABILITY in
+			 * software, we can always enable it for guest
+			 * regardless of host's capability.
+			 */
+			entry->edx |= F(CORE_CAPABILITY);
 		} else {
 			entry->ebx = 0;
 			entry->ecx = 0;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 941f932373d0..e20cbb8c2b74 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1158,6 +1158,7 @@ static u32 emulated_msrs[] = {
 
 	MSR_IA32_TSC_ADJUST,
 	MSR_IA32_TSCDEADLINE,
+	MSR_IA32_CORE_CAPABILITY,
 	MSR_IA32_MISC_ENABLE,
 	MSR_IA32_MCG_STATUS,
 	MSR_IA32_MCG_CTL,
@@ -1197,6 +1198,7 @@ static u32 msr_based_features[] = {
 
 	MSR_F10H_DECFG,
 	MSR_IA32_UCODE_REV,
+	MSR_IA32_CORE_CAPABILITY,
 	MSR_IA32_ARCH_CAPABILITIES,
 };
 
@@ -1224,9 +1226,18 @@ u64 kvm_get_arch_capabilities(void)
 }
 EXPORT_SYMBOL_GPL(kvm_get_arch_capabilities);
 
+u64 kvm_get_core_capability(void)
+{
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kvm_get_core_capability);
+
 static int kvm_get_msr_feature(struct kvm_msr_entry *msr)
 {
 	switch (msr->index) {
+	case MSR_IA32_CORE_CAPABILITY:
+		msr->data = kvm_get_core_capability();
+		break;
 	case MSR_IA32_ARCH_CAPABILITIES:
 		msr->data = kvm_get_arch_capabilities();
 		break;
@@ -2445,6 +2456,12 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		break;
 	case MSR_EFER:
 		return set_efer(vcpu, data);
+	case MSR_IA32_CORE_CAPABILITY:
+		if (!msr_info->host_initiated)
+			return 1;
+
+		vcpu->arch.core_capability = data;
+		break;
 	case MSR_K7_HWCR:
 		data &= ~(u64)0x40;	/* ignore flush filter disable */
 		data &= ~(u64)0x100;	/* ignore ignne emulation enable */
@@ -2750,6 +2767,12 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_IA32_TSC:
 		msr_info->data = kvm_scale_tsc(vcpu, rdtsc()) + vcpu->arch.tsc_offset;
 		break;
+	case MSR_IA32_CORE_CAPABILITY:
+		if (!msr_info->host_initiated &&
+		    !guest_cpuid_has(vcpu, X86_FEATURE_CORE_CAPABILITY))
+			return 1;
+		msr_info->data = vcpu->arch.core_capability;
+		break;
 	case MSR_MTRRcap:
 	case 0x200 ... 0x2ff:
 		return kvm_mtrr_get_msr(vcpu, msr_info->index, &msr_info->data);
@@ -8725,6 +8748,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
 
 int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 {
+	vcpu->arch.core_capability = kvm_get_core_capability();
 	vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;
 	kvm_vcpu_mtrr_init(vcpu);
 	vcpu_load(vcpu);
-- 
2.19.1


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

* [PATCH v5 11/18] kvm/vmx: Emulate MSR TEST_CTL
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (9 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 10/18] kvm/x86: Emulate MSR IA32_CORE_CAPABILITY Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-13  8:15   ` Paolo Bonzini
  2019-03-12 23:00 ` [PATCH v5 12/18] x86/split_lock: Enable #AC for split lock by default Fenghua Yu
                   ` (6 subsequent siblings)
  17 siblings, 1 reply; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Xiaoyao Li, Fenghua Yu

From: Xiaoyao Li <xiaoyao.li@linux.intel.com>

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

Please check the latest Intel 64 and IA-32 Architectures Software
Developer's Manual for more detailed information on the MSR and
the split lock bit.

This patch emulate MSR TEST_CTL with vmx->msr_test_ctl and does the
following:
1. As we emulate MSR TEST_CTL of guest, we should enable the related bits
in CORE_CAPABILITY to corretly report this feature to guest.

2. Differentiate MSR TEST_CTL between host and guest.

Signed-off-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 35 +++++++++++++++++++++++++++++++++++
 arch/x86/kvm/vmx/vmx.h |  1 +
 arch/x86/kvm/x86.c     | 17 ++++++++++++++++-
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 30a6bcd735ec..270c6566fd5a 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1659,6 +1659,12 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	u32 index;
 
 	switch (msr_info->index) {
+	case MSR_TEST_CTL:
+		if (!msr_info->host_initiated &&
+		    !(vcpu->arch.core_capability & CORE_CAP_SPLIT_LOCK_DETECT))
+			return 1;
+		msr_info->data = vmx->msr_test_ctl;
+		break;
 #ifdef CONFIG_X86_64
 	case MSR_FS_BASE:
 		msr_info->data = vmcs_readl(GUEST_FS_BASE);
@@ -1799,6 +1805,14 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	u32 index;
 
 	switch (msr_index) {
+	case MSR_TEST_CTL:
+		if (!(vcpu->arch.core_capability & CORE_CAP_SPLIT_LOCK_DETECT))
+			return 1;
+
+		if (data & ~TEST_CTL_ENABLE_SPLIT_LOCK_DETECT)
+			return 1;
+		vmx->msr_test_ctl = data;
+		break;
 	case MSR_EFER:
 		ret = kvm_set_msr_common(vcpu, msr_info);
 		break;
@@ -4085,6 +4099,9 @@ static void vmx_vcpu_setup(struct vcpu_vmx *vmx)
 
 	vmx->arch_capabilities = kvm_get_arch_capabilities();
 
+	/* disable AC split lock by default */
+	vmx->msr_test_ctl = 0;
+
 	vm_exit_controls_init(vmx, vmx_vmexit_ctrl());
 
 	/* 22.2.1, 20.8.1 */
@@ -4122,6 +4139,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 
 	vmx->rmode.vm86_active = 0;
 	vmx->spec_ctrl = 0;
+	vmx->msr_test_ctl = 0;
 
 	vcpu->arch.microcode_version = 0x100000000ULL;
 	vmx->vcpu.arch.regs[VCPU_REGS_RDX] = get_rdx_init_val();
@@ -6321,6 +6339,21 @@ static void atomic_switch_perf_msrs(struct vcpu_vmx *vmx)
 					msrs[i].host, false);
 }
 
+static void atomic_switch_msr_test_ctl(struct vcpu_vmx *vmx)
+{
+	u64 host_msr_test_ctl;
+
+	/* if TEST_CTL MSR doesn't exist on the hardware, we do nothing */
+	if (rdmsrl_safe(MSR_TEST_CTL, &host_msr_test_ctl))
+		return;
+
+	if (host_msr_test_ctl == vmx->msr_test_ctl)
+		clear_atomic_switch_msr(vmx, MSR_TEST_CTL);
+	else
+		add_atomic_switch_msr(vmx, MSR_TEST_CTL, vmx->msr_test_ctl,
+				      host_msr_test_ctl, false);
+}
+
 static void vmx_arm_hv_timer(struct vcpu_vmx *vmx, u32 val)
 {
 	vmcs_write32(VMX_PREEMPTION_TIMER_VALUE, val);
@@ -6562,6 +6595,8 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
 
 	atomic_switch_perf_msrs(vmx);
 
+	atomic_switch_msr_test_ctl(vmx);
+
 	vmx_update_hv_timer(vcpu);
 
 	/*
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 0ac0a64c7790..8549cba0fb75 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -191,6 +191,7 @@ struct vcpu_vmx {
 	u64		      msr_guest_kernel_gs_base;
 #endif
 
+	u64		      msr_test_ctl;
 	u64		      arch_capabilities;
 	u64		      spec_ctrl;
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e20cbb8c2b74..ad1df965574e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1228,7 +1228,22 @@ EXPORT_SYMBOL_GPL(kvm_get_arch_capabilities);
 
 u64 kvm_get_core_capability(void)
 {
-	return 0;
+	u64 data;
+
+	rdmsrl_safe(MSR_IA32_CORE_CAPABILITY, &data);
+
+	/* mask non-virtualizable functions */
+	data &= CORE_CAP_SPLIT_LOCK_DETECT;
+
+	/*
+	 * There will be a list of FMS values that have split lock detection
+	 * but lack the CORE CAPABILITY MSR. In this case, we can set
+	 * CORE_CAP_SPLIT_LOCK_DETECT since we emulate MSR CORE_CAPABILITY.
+	 */
+	if (boot_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT))
+		data |= CORE_CAP_SPLIT_LOCK_DETECT;
+
+	return data;
 }
 EXPORT_SYMBOL_GPL(kvm_get_core_capability);
 
-- 
2.19.1


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

* [PATCH v5 12/18] x86/split_lock: Enable #AC for split lock by default
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (10 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 11/18] kvm/vmx: Emulate MSR TEST_CTL Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:43   ` Dave Hansen
  2019-03-12 23:00 ` [PATCH v5 13/18] x86/split_lock: Add a sysfs interface to allow user to enable or disable split lock detection on all CPUs during run time Fenghua Yu
                   ` (5 subsequent siblings)
  17 siblings, 1 reply; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

Split locked access locks bus and degradates overall memory access
performance. When split lock detection feature is enumerated, we want to
enable the feature by default to find any split lock issue and then fix
the issue.

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

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 889390d51c17..561f7d50246a 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -31,6 +31,11 @@
 #include <asm/apic.h>
 #endif
 
+#define DISABLE_SPLIT_LOCK_DETECT 0
+#define ENABLE_SPLIT_LOCK_DETECT  1
+
+static int split_lock_detect_val;
+
 /*
  * Just in case our CPU detection goes bad, or you have a weird system,
  * allow a way to override the automatic disabling of MPX.
@@ -161,10 +166,35 @@ static bool bad_spectre_microcode(struct cpuinfo_x86 *c)
 	return false;
 }
 
+static u32 new_sp_test_ctl_val(u32 test_ctl_val)
+{
+	/* Change the split lock setting. */
+	if (split_lock_detect_val == DISABLE_SPLIT_LOCK_DETECT)
+		test_ctl_val &= ~TEST_CTL_ENABLE_SPLIT_LOCK_DETECT;
+	else
+		test_ctl_val |= TEST_CTL_ENABLE_SPLIT_LOCK_DETECT;
+
+	return test_ctl_val;
+}
+
+static void init_split_lock_detect(struct cpuinfo_x86 *c)
+{
+	if (cpu_has(c, X86_FEATURE_SPLIT_LOCK_DETECT)) {
+		u32 l, h;
+
+		rdmsr(MSR_TEST_CTL, l, h);
+		l = new_sp_test_ctl_val(l);
+		wrmsr(MSR_TEST_CTL, l, h);
+		pr_info_once("#AC for split lock is enabled\n");
+	}
+}
+
 static void early_init_intel(struct cpuinfo_x86 *c)
 {
 	u64 misc_enable;
 
+	init_split_lock_detect(c);
+
 	/* Unmask CPUID levels if masked: */
 	if (c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xd)) {
 		if (msr_clear_bit(MSR_IA32_MISC_ENABLE,
@@ -1048,6 +1078,8 @@ void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c)
 	 */
 	rdmsrl(MSR_IA32_CORE_CAPABILITY, ia32_core_cap);
 
-	if (ia32_core_cap & CORE_CAP_SPLIT_LOCK_DETECT)
+	if (ia32_core_cap & CORE_CAP_SPLIT_LOCK_DETECT) {
 		setup_force_cpu_cap(X86_FEATURE_SPLIT_LOCK_DETECT);
+		split_lock_detect_val = 1;
+	}
 }
-- 
2.19.1


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

* [PATCH v5 13/18] x86/split_lock: Add a sysfs interface to allow user to enable or disable split lock detection on all CPUs during run time
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (11 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 12/18] x86/split_lock: Enable #AC for split lock by default Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:48   ` Dave Hansen
  2019-03-12 23:00 ` [PATCH v5 14/18] x86/clearcpuid: Support multiple clearcpuid options Fenghua Yu
                   ` (4 subsequent siblings)
  17 siblings, 1 reply; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

The interface /sys/device/system/cpu/split_lock_detect is added
to allow user to control split lock detection and show current split
lock detection setting.

Writing 1 to the file enables split lock detection and writing 0
disables split lock detection. Split lock detection is enabled or
disabled on all CPUs.

Reading the file shows current global split lock detection setting:
disabled when 0 and enabled when 1.

Please note the interface only shows global setting. During run time,
split lock detection on one CPU may be disabled if split lock
in kernel code happens on the CPU. The interface doesn't show split
lock detection status on individual CPU. User can run rdmsr on 0x33
to check split lock detection on each CPU.

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

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 561f7d50246a..1b25ff8c75eb 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -34,6 +34,7 @@
 #define DISABLE_SPLIT_LOCK_DETECT 0
 #define ENABLE_SPLIT_LOCK_DETECT  1
 
+static DEFINE_MUTEX(split_lock_detect_mutex);
 static int split_lock_detect_val;
 
 /*
@@ -1083,3 +1084,69 @@ void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c)
 		split_lock_detect_val = 1;
 	}
 }
+
+static ssize_t
+split_lock_detect_show(struct device *dev, struct device_attribute *attr,
+		       char *buf)
+{
+	return sprintf(buf, "%u\n", READ_ONCE(split_lock_detect_val));
+}
+
+static ssize_t
+split_lock_detect_store(struct device *dev, struct device_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_DETECT && val != ENABLE_SPLIT_LOCK_DETECT)
+		return -EINVAL;
+
+	/*
+	 * Since split lock could be disabled by kernel #AC handler or user
+	 * may directly change bit 29 in MSR_TEST_CTL, split lock setting on
+	 * each CPU may be different from global setting split_lock_detect_val
+	 * by now. Update MSR on each CPU, so all of CPUs will have same split
+	 * lock setting.
+	 */
+	mutex_lock(&split_lock_detect_mutex);
+
+	WRITE_ONCE(split_lock_detect_val, val);
+
+	/*
+	 * Get MSR_TEST_CTL on this CPU, assuming all CPUs have same value
+	 * in the MSR except split lock detection bit (bit 29).
+	 */
+	rdmsr(MSR_TEST_CTL, l, h);
+	l = new_sp_test_ctl_val(l);
+	/* Update the split lock detection setting on all online CPUs. */
+	for_each_online_cpu(cpu)
+		wrmsr_on_cpu(cpu, MSR_TEST_CTL, l, h);
+
+	mutex_unlock(&split_lock_detect_mutex);
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(split_lock_detect);
+
+static int __init split_lock_init(void)
+{
+	int ret;
+
+	if (!boot_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT))
+		return -ENODEV;
+
+	ret = device_create_file(cpu_subsys.dev_root,
+				 &dev_attr_split_lock_detect);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+subsys_initcall(split_lock_init);
-- 
2.19.1


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

* [PATCH v5 14/18] x86/clearcpuid: Support multiple clearcpuid options
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (12 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 13/18] x86/split_lock: Add a sysfs interface to allow user to enable or disable split lock detection on all CPUs during run time Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 15/18] x86/clearcpuid: Support feature flag string in kernel option clearcpuid Fenghua Yu
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

Currently only one kernel option "clearcpuid=" can be picked up by
kernel during boot time.

In some cases, user may want to clear a few cpu caps. This may be
useful to replace endless (new) kernel options like nosmep, nosmap,
etc.

We add support of multiple clearcpuid options to allow user to clear
multiple cpu caps.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/cmdline.h |  3 +++
 arch/x86/kernel/fpu/init.c     | 30 ++++++++++++++++++++----------
 arch/x86/lib/cmdline.c         | 17 +++++++++++++++--
 3 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/cmdline.h b/arch/x86/include/asm/cmdline.h
index 6faaf27e8899..059e29558bb3 100644
--- a/arch/x86/include/asm/cmdline.h
+++ b/arch/x86/include/asm/cmdline.h
@@ -5,5 +5,8 @@
 int cmdline_find_option_bool(const char *cmdline_ptr, const char *option);
 int cmdline_find_option(const char *cmdline_ptr, const char *option,
 			char *buffer, int bufsize);
+int cmdline_find_option_in_range(const char *cmdline_ptr, int cmdline_size,
+				 const char *option, char *buffer, int bufsize,
+				 char **arg_pos);
 
 #endif /* _ASM_X86_CMDLINE_H */
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index 6abd83572b01..88bbba7ee96a 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -243,16 +243,31 @@ static void __init fpu__init_system_ctx_switch(void)
 	WARN_ON_FPU(current->thread.fpu.initialized);
 }
 
+static void __init clear_cpuid(void)
+{
+	char arg[32], *argptr, *option_pos, clearcpuid_option[] = "clearcpuid";
+	int bit, cmdline_size;
+
+	/* Find each option in boot_command_line and clear specified cpu cap. */
+	cmdline_size = COMMAND_LINE_SIZE;
+	while (cmdline_find_option_in_range(boot_command_line, cmdline_size,
+					    clearcpuid_option, arg,
+					    sizeof(arg), &option_pos) >= 0) {
+		/* Chang command line range for next search. */
+		cmdline_size = option_pos - boot_command_line + 1;
+		argptr = arg;
+		if (get_option(&argptr, &bit) &&
+		    bit >= 0 && bit < NCAPINTS * 32)
+			setup_clear_cpu_cap(bit);
+	}
+}
+
 /*
  * We parse fpu parameters early because fpu__init_system() is executed
  * before parse_early_param().
  */
 static void __init fpu__init_parse_early_param(void)
 {
-	char arg[32];
-	char *argptr = arg;
-	int bit;
-
 	if (cmdline_find_option_bool(boot_command_line, "no387"))
 		setup_clear_cpu_cap(X86_FEATURE_FPU);
 
@@ -271,12 +286,7 @@ static void __init fpu__init_parse_early_param(void)
 	if (cmdline_find_option_bool(boot_command_line, "noxsaves"))
 		setup_clear_cpu_cap(X86_FEATURE_XSAVES);
 
-	if (cmdline_find_option(boot_command_line, "clearcpuid", arg,
-				sizeof(arg)) &&
-	    get_option(&argptr, &bit) &&
-	    bit >= 0 &&
-	    bit < NCAPINTS * 32)
-		setup_clear_cpu_cap(bit);
+	clear_cpuid();
 }
 
 /*
diff --git a/arch/x86/lib/cmdline.c b/arch/x86/lib/cmdline.c
index 3261abb21ef4..9cf1a0773877 100644
--- a/arch/x86/lib/cmdline.c
+++ b/arch/x86/lib/cmdline.c
@@ -114,13 +114,15 @@ __cmdline_find_option_bool(const char *cmdline, int max_cmdline_size,
  * @option: option string to look for
  * @buffer: memory buffer to return the option argument
  * @bufsize: size of the supplied memory buffer
+ * @option_pos: pointer to the option if found
  *
  * Returns the length of the argument (regardless of if it was
  * truncated to fit in the buffer), or -1 on not found.
  */
 static int
 __cmdline_find_option(const char *cmdline, int max_cmdline_size,
-		      const char *option, char *buffer, int bufsize)
+		      const char *option, char *buffer, int bufsize,
+		      char **arg_pos)
 {
 	char c;
 	int pos = 0, len = -1;
@@ -164,6 +166,9 @@ __cmdline_find_option(const char *cmdline, int max_cmdline_size,
 				len = 0;
 				bufptr = buffer;
 				state = st_bufcpy;
+				if (arg_pos)
+					*arg_pos = (char *)cmdline -
+						   strlen(option) - 1;
 				break;
 			} else if (c == *opptr++) {
 				/*
@@ -211,5 +216,13 @@ int cmdline_find_option(const char *cmdline, const char *option, char *buffer,
 			int bufsize)
 {
 	return __cmdline_find_option(cmdline, COMMAND_LINE_SIZE, option,
-				     buffer, bufsize);
+				     buffer, bufsize, NULL);
+}
+
+int cmdline_find_option_in_range(const char *cmdline, int cmdline_size,
+				 char *option, char *buffer, int bufsize,
+				 char **arg_pos)
+{
+	return __cmdline_find_option(cmdline, cmdline_size, option, buffer,
+				     bufsize, arg_pos);
 }
-- 
2.19.1


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

* [PATCH v5 15/18] x86/clearcpuid: Support feature flag string in kernel option clearcpuid
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (13 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 14/18] x86/clearcpuid: Support multiple clearcpuid options Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 16/18] x86/clearcpuid: Apply cleared feature bits that are forced set before Fenghua Yu
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

The kernel option clearcpuid currently only takes feature bit which
can be changed from kernel to kernel.

Extend clearcpuid to use cap flag string, which is defined in
x86_cap_flags[] and won't be changed from kernel to kernel.
And user can easily get the cap flag string from /proc/cpuinfo.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/cpufeature.h |  1 +
 arch/x86/kernel/cpu/cpuid-deps.c  | 26 ++++++++++++++++++++++++++
 arch/x86/kernel/fpu/init.c        |  3 ++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index ce95b8cbd229..6792088525e3 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -132,6 +132,7 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
 
 extern void setup_clear_cpu_cap(unsigned int bit);
 extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);
+bool find_cpu_cap(char *cap_flag, unsigned int *pfeature);
 
 #define setup_force_cpu_cap(bit) do { \
 	set_cpu_cap(&boot_cpu_data, bit);	\
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 3d633f67fbd7..1a71434f7b46 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -120,3 +120,29 @@ void setup_clear_cpu_cap(unsigned int feature)
 {
 	do_clear_cpu_cap(NULL, feature);
 }
+
+/**
+ * find_cpu_cap - Given a cap flag string, find its corresponding feature bit.
+ * @cap_flag:	cap flag string as defined in x86_cap_flags[]
+ * @pfeature:	feature bit
+ *
+ * Return: true if the feature is found. false if not found
+ */
+bool find_cpu_cap(char *cap_flag, unsigned int *pfeature)
+{
+#ifdef CONFIG_X86_FEATURE_NAMES
+	unsigned int feature;
+
+	for (feature = 0; feature < NCAPINTS * 32; feature++) {
+		if (!x86_cap_flags[feature])
+			continue;
+
+		if (strcmp(cap_flag, x86_cap_flags[feature]) == 0) {
+			*pfeature = feature;
+
+			return true;
+		}
+	}
+#endif
+	return false;
+}
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index 88bbba7ee96a..99b895eea166 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -256,7 +256,8 @@ static void __init clear_cpuid(void)
 		/* Chang command line range for next search. */
 		cmdline_size = option_pos - boot_command_line + 1;
 		argptr = arg;
-		if (get_option(&argptr, &bit) &&
+		/* cpu cap can be specified by either feature bit or string */
+		if ((get_option(&argptr, &bit) || find_cpu_cap(arg, &bit)) &&
 		    bit >= 0 && bit < NCAPINTS * 32)
 			setup_clear_cpu_cap(bit);
 	}
-- 
2.19.1


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

* [PATCH v5 16/18] x86/clearcpuid: Apply cleared feature bits that are forced set before
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (14 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 15/18] x86/clearcpuid: Support feature flag string in kernel option clearcpuid Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 17/18] x86/clearcpuid: Clear CPUID bit in CPUID faulting Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 18/18] Change document for kernel option clearcpuid Fenghua Yu
  17 siblings, 0 replies; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

Some CPU feature bits are forced set and stored in cpuinfo_x86 before
handling clearcpuid options. To clear those bits from cpuinfo_x86,
apply_forced_cap() is called after handling the options.

Please note, apply_forced_cap() is called twice on boot CPU. But this code
is simple and there is no functionality issue.

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

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 4e03f53fc079..bfa5ac6b7502 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -26,6 +26,8 @@ struct x86_cpu {
 	struct cpu cpu;
 };
 
+void apply_forced_caps(struct cpuinfo_x86 *c);
+
 #ifdef CONFIG_HOTPLUG_CPU
 extern int arch_register_cpu(int num);
 extern void arch_unregister_cpu(int);
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 16b091bb62bd..546f92aa61ca 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -758,13 +758,14 @@ void cpu_detect(struct cpuinfo_x86 *c)
 	}
 }
 
-static void apply_forced_caps(struct cpuinfo_x86 *c)
+void apply_forced_caps(struct cpuinfo_x86 *c)
 {
 	int i;
 
 	for (i = 0; i < NCAPINTS + NBUGINTS; i++) {
-		c->x86_capability[i] &= ~cpu_caps_cleared[i];
+		/* Bits may be cleared after they are set. */
 		c->x86_capability[i] |= cpu_caps_set[i];
+		c->x86_capability[i] &= ~cpu_caps_cleared[i];
 	}
 }
 
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index 99b895eea166..9c2801b605e3 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -5,6 +5,7 @@
 #include <asm/tlbflush.h>
 #include <asm/setup.h>
 #include <asm/cmdline.h>
+#include <asm/cpu.h>
 
 #include <linux/sched.h>
 #include <linux/sched/task.h>
@@ -261,6 +262,7 @@ static void __init clear_cpuid(void)
 		    bit >= 0 && bit < NCAPINTS * 32)
 			setup_clear_cpu_cap(bit);
 	}
+	apply_forced_caps(&boot_cpu_data);
 }
 
 /*
-- 
2.19.1


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

* [PATCH v5 17/18] x86/clearcpuid: Clear CPUID bit in CPUID faulting
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (15 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 16/18] x86/clearcpuid: Apply cleared feature bits that are forced set before Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  2019-03-12 23:00 ` [PATCH v5 18/18] Change document for kernel option clearcpuid Fenghua Yu
  17 siblings, 0 replies; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

From: Peter Zijlstra <peterz@infradead.org>

After kernel clears a CPUID bit through clearcpuid or other kernel options,
CPUID instruction executed from user space should see the same value for
the bit. The CPUID faulting handler returns the cleared bit to user.

Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/cpufeature.h |  4 +++
 arch/x86/kernel/cpu/common.c      |  2 ++
 arch/x86/kernel/cpu/cpuid-deps.c  | 52 ++++++++++++++++++++++++++++
 arch/x86/kernel/cpu/intel.c       | 56 +++++++++++++++++++++++++++++--
 arch/x86/kernel/cpu/scattered.c   | 17 ++++++++++
 arch/x86/kernel/process.c         |  3 ++
 arch/x86/kernel/traps.c           | 11 ++++++
 7 files changed, 142 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 6792088525e3..c4ac787d9b85 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -227,5 +227,9 @@ static __always_inline __pure bool _static_cpu_has(u16 bit)
 #define CPU_FEATURE_TYPEVAL		boot_cpu_data.x86_vendor, boot_cpu_data.x86, \
 					boot_cpu_data.x86_model
 
+extern int cpuid_fault;
+u32 scattered_cpuid_mask(u32 leaf, u32 count, enum cpuid_regs_idx reg);
+u32 cpuid_cap_mask(u32 leaf, u32 count, enum cpuid_regs_idx reg);
+
 #endif /* defined(__KERNEL__) && !defined(__ASSEMBLY__) */
 #endif /* _ASM_X86_CPUFEATURE_H */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 546f92aa61ca..02ea518580fa 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1503,6 +1503,8 @@ void print_cpu_info(struct cpuinfo_x86 *c)
 		pr_cont(")\n");
 }
 
+int cpuid_fault;
+
 /*
  * clearcpuid= was already parsed in fpu__init_parse_early_param.
  * But we need to keep a dummy __setup around otherwise it would
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 1a71434f7b46..d42aa4fa3021 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -113,9 +113,61 @@ static void do_clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int feature)
 
 void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int feature)
 {
+	if (boot_cpu_has(feature))
+		cpuid_fault = 1;
 	do_clear_cpu_cap(c, feature);
 }
 
+u32 cpuid_cap_mask(u32 leaf, u32 count, enum cpuid_regs_idx reg)
+{
+	switch (leaf) {
+	case 0x1:
+		if (reg == CPUID_EDX)
+			return ~cpu_caps_cleared[CPUID_1_EDX];
+		if (reg == CPUID_ECX)
+			return ~cpu_caps_cleared[CPUID_1_ECX];
+		break;
+
+	case 0x6:
+		if (reg == CPUID_EAX)
+			return ~cpu_caps_cleared[CPUID_6_EAX];
+		break;
+
+	case 0x7:
+		if (reg == CPUID_EDX)
+			return ~cpu_caps_cleared[CPUID_7_EDX];
+		if (reg == CPUID_ECX)
+			return ~cpu_caps_cleared[CPUID_7_ECX];
+		if (reg == CPUID_EBX && count == 0)
+			return ~cpu_caps_cleared[CPUID_7_0_EBX];
+		break;
+
+	case 0xD:
+		if (reg == CPUID_EAX)
+			return ~cpu_caps_cleared[CPUID_D_1_EAX];
+		break;
+
+	case 0xF:
+		if (reg == CPUID_EDX) {
+			if (count == 0)
+				return ~cpu_caps_cleared[CPUID_F_0_EDX];
+			if (count == 1)
+				return ~cpu_caps_cleared[CPUID_F_0_EDX];
+		}
+		break;
+
+	case 0x80000007:
+		if (reg == CPUID_EDX) {
+			if (test_bit(X86_FEATURE_CONSTANT_TSC,
+				     (unsigned long *)cpu_caps_cleared))
+				return ~(1 << 8);
+		}
+		break;
+	}
+
+	return scattered_cpuid_mask(leaf, count, reg);
+}
+
 void setup_clear_cpu_cap(unsigned int feature)
 {
 	do_clear_cpu_cap(NULL, feature);
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 1b25ff8c75eb..13044b3f426d 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -19,6 +19,9 @@
 #include <asm/microcode_intel.h>
 #include <asm/hwcap2.h>
 #include <asm/elf.h>
+#include <asm/insn.h>
+#include <asm/insn-eval.h>
+#include <asm/inat.h>
 
 #ifdef CONFIG_X86_64
 #include <linux/topology.h>
@@ -657,13 +660,60 @@ static void intel_bsp_resume(struct cpuinfo_x86 *c)
 	init_intel_energy_perf(c);
 }
 
+bool fixup_cpuid_exception(struct pt_regs *regs)
+{
+	unsigned int leaf, count, eax, ebx, ecx, edx;
+	unsigned long seg_base = 0;
+	unsigned char buf[2];
+	int not_copied;
+
+	if (!cpuid_fault)
+		return false;
+
+	if (test_thread_flag(TIF_NOCPUID))
+		return false;
+
+	if (!user_64bit_mode(regs))
+		seg_base = insn_get_seg_base(regs, INAT_SEG_REG_CS);
+
+	if (seg_base == -1L)
+		return false;
+
+	not_copied = copy_from_user(buf, (void __user *)(seg_base + regs->ip),
+				    sizeof(buf));
+	if (not_copied)
+		return false;
+
+	if (buf[0] != 0x0F || buf[1] != 0xA2) /* CPUID - OF A2 */
+		return false;
+
+	leaf = regs->ax;
+	count = regs->cx;
+
+	cpuid_count(leaf, count, &eax, &ebx, &ecx, &edx);
+
+	regs->ip += 2;
+	regs->ax = eax & cpuid_cap_mask(leaf, count, CPUID_EAX);
+	regs->bx = ebx & cpuid_cap_mask(leaf, count, CPUID_EBX);
+	regs->cx = ecx & cpuid_cap_mask(leaf, count, CPUID_ECX);
+	regs->dx = edx & cpuid_cap_mask(leaf, count, CPUID_EDX);
+
+	return true;
+}
+
 static void init_cpuid_fault(struct cpuinfo_x86 *c)
 {
 	u64 msr;
 
-	if (!rdmsrl_safe(MSR_PLATFORM_INFO, &msr)) {
-		if (msr & MSR_PLATFORM_INFO_CPUID_FAULT)
-			set_cpu_cap(c, X86_FEATURE_CPUID_FAULT);
+	if (rdmsrl_safe(MSR_PLATFORM_INFO, &msr))
+		return;
+
+	if (msr & MSR_PLATFORM_INFO_CPUID_FAULT) {
+		set_cpu_cap(c, X86_FEATURE_CPUID_FAULT);
+		if (cpuid_fault) {
+			this_cpu_or(msr_misc_features_shadow,
+				    MSR_MISC_FEATURES_ENABLES_CPUID_FAULT);
+		}
 	}
 }
 
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 94aa1c72ca98..353756c27096 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -62,3 +62,20 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
 			set_cpu_cap(c, cb->feature);
 	}
 }
+
+u32 scattered_cpuid_mask(u32 leaf, u32 count, enum cpuid_regs_idx reg)
+{
+	const struct cpuid_bit *cb;
+	u32 mask = ~0U;
+
+	for (cb = cpuid_bits; cb->feature; cb++) {
+		if (cb->level == leaf && cb->sub_leaf == count &&
+		    cb->reg == reg) {
+			if (test_bit(cb->feature,
+				     (unsigned long *)cpu_caps_cleared))
+				mask &= ~BIT(cb->bit);
+		}
+	}
+
+	return mask;
+}
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 58ac7be52c7a..2b1dfd7ae65d 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -196,6 +196,9 @@ static void set_cpuid_faulting(bool on)
 {
 	u64 msrval;
 
+	if (cpuid_fault)
+		return;
+
 	msrval = this_cpu_read(msr_misc_features_shadow);
 	msrval &= ~MSR_MISC_FEATURES_ENABLES_CPUID_FAULT;
 	msrval |= (on << MSR_MISC_FEATURES_ENABLES_CPUID_FAULT_BIT);
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 69b6233e783e..36b2c87a2bce 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -549,6 +549,12 @@ dotraplinkage void do_bounds(struct pt_regs *regs, long error_code)
 	do_trap(X86_TRAP_BR, SIGSEGV, "bounds", regs, error_code, 0, NULL);
 }
 
+#ifdef CONFIG_CPU_SUP_INTEL
+extern bool fixup_cpuid_exception(struct pt_regs *regs);
+#else
+static inline bool fixup_cpuid_exception(struct pt_regs *regs) { return false; }
+#endif
+
 dotraplinkage void
 do_general_protection(struct pt_regs *regs, long error_code)
 {
@@ -563,6 +569,11 @@ do_general_protection(struct pt_regs *regs, long error_code)
 			return;
 	}
 
+	if (static_cpu_has(X86_FEATURE_CPUID_FAULT)) {
+		if (user_mode(regs) && fixup_cpuid_exception(regs))
+			return;
+	}
+
 	if (v8086_mode(regs)) {
 		local_irq_enable();
 		handle_vm86_fault((struct kernel_vm86_regs *) regs, error_code);
-- 
2.19.1


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

* [PATCH v5 18/18] Change document for kernel option clearcpuid
  2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
                   ` (16 preceding siblings ...)
  2019-03-12 23:00 ` [PATCH v5 17/18] x86/clearcpuid: Clear CPUID bit in CPUID faulting Fenghua Yu
@ 2019-03-12 23:00 ` Fenghua Yu
  17 siblings, 0 replies; 34+ messages in thread
From: Fenghua Yu @ 2019-03-12 23:00 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Fenghua Yu

Since kernel option clearcpuid now supports multiple options and CPU
capability flags, the document needs to be changed.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2b8ee90bb644..7e65ae67d573 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -563,10 +563,12 @@
 			loops can be debugged more effectively on production
 			systems.
 
-	clearcpuid=BITNUM [X86]
+	clearcpuid=BITNUM | FLAG [X86]
 			Disable CPUID feature X for the kernel. See
 			arch/x86/include/asm/cpufeatures.h for the valid bit
-			numbers. Note the Linux specific bits are not necessarily
+			numbers or /proc/cpuinfo for valid CPU flags.
+			Multiple options can be used to disable a few features.
+			Note the Linux specific bits are not necessarily
 			stable over kernel options, but the vendor specific
 			ones should be.
 			Also note that user programs calling CPUID directly
-- 
2.19.1


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

* Re: [PATCH v5 12/18] x86/split_lock: Enable #AC for split lock by default
  2019-03-12 23:00 ` [PATCH v5 12/18] x86/split_lock: Enable #AC for split lock by default Fenghua Yu
@ 2019-03-12 23:43   ` Dave Hansen
  0 siblings, 0 replies; 34+ messages in thread
From: Dave Hansen @ 2019-03-12 23:43 UTC (permalink / raw)
  To: Fenghua Yu, Thomas Gleixner, Ingo Molnar, H Peter Anvin,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm

On 3/12/19 4:00 PM, Fenghua Yu wrote:
> Split locked access locks bus and degradates overall memory access

Either "split locked accesses lock" or "A split lock access locks".

s/degradates/degrades/

> performance. When split lock detection feature is enumerated, we want to

		   ^ the

Also, you need to go back and remove all the "we"'s.  Our friendly x86
maintainers prefer phrasing this like:

	 When split lock detection feature is enumerated, enable the 	
	feature...

> enable the feature by default to find any split lock issue and then fix
> the issue.

Generally, the changelogs here are passable but pretty rough.  They have
a ton of issues like this and I'm sure they can be improved.  I'm sure
that with some love an attention they can be brought up to the highest
standards.

> +#define DISABLE_SPLIT_LOCK_DETECT 0
> +#define ENABLE_SPLIT_LOCK_DETECT  1

These are a bit overkill, but oh well.

> +static int split_lock_detect_val;
> +
>  /*
>   * Just in case our CPU detection goes bad, or you have a weird system,
>   * allow a way to override the automatic disabling of MPX.
> @@ -161,10 +166,35 @@ static bool bad_spectre_microcode(struct cpuinfo_x86 *c)
>  	return false;
>  }
>  
> +static u32 new_sp_test_ctl_val(u32 test_ctl_val)
> +{
> +	/* Change the split lock setting. */
> +	if (split_lock_detect_val == DISABLE_SPLIT_LOCK_DETECT)
> +		test_ctl_val &= ~TEST_CTL_ENABLE_SPLIT_LOCK_DETECT;
> +	else
> +		test_ctl_val |= TEST_CTL_ENABLE_SPLIT_LOCK_DETECT;
> +
> +	return test_ctl_val;
> +}
> +
> +static void init_split_lock_detect(struct cpuinfo_x86 *c)
> +{
> +	if (cpu_has(c, X86_FEATURE_SPLIT_LOCK_DETECT)) {
> +		u32 l, h;
> +
> +		rdmsr(MSR_TEST_CTL, l, h);
> +		l = new_sp_test_ctl_val(l);
> +		wrmsr(MSR_TEST_CTL, l, h);
> +		pr_info_once("#AC for split lock is enabled\n");
> +	}
> +}

Please make sure you've removed all the "#AC for split lock"
nomenclature in these patches.  It's acronym nonsense and there's no
reason to be so oblique.  Just say "split lock detection enabled".  This
also needs a proper prefix, like "x86/intel:".  Look around for examples.



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

* Re: [PATCH v5 13/18] x86/split_lock: Add a sysfs interface to allow user to enable or disable split lock detection on all CPUs during run time
  2019-03-12 23:00 ` [PATCH v5 13/18] x86/split_lock: Add a sysfs interface to allow user to enable or disable split lock detection on all CPUs during run time Fenghua Yu
@ 2019-03-12 23:48   ` Dave Hansen
  2019-03-13  0:53     ` Fenghua Yu
  0 siblings, 1 reply; 34+ messages in thread
From: Dave Hansen @ 2019-03-12 23:48 UTC (permalink / raw)
  To: Fenghua Yu, Thomas Gleixner, Ingo Molnar, H Peter Anvin,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm

On 3/12/19 4:00 PM, Fenghua Yu wrote:
> The interface /sys/device/system/cpu/split_lock_detect is added
> to allow user to control split lock detection and show current split
> lock detection setting.

Did I miss the Documentation/ for this new sysfs feature?

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

* Re: [PATCH v5 09/18] x86/split_lock: Handle #AC exception for split lock
  2019-03-12 23:00 ` [PATCH v5 09/18] x86/split_lock: Handle #AC exception for split lock Fenghua Yu
@ 2019-03-12 23:51   ` Dave Hansen
  2019-03-13  0:49     ` Fenghua Yu
  0 siblings, 1 reply; 34+ messages in thread
From: Dave Hansen @ 2019-03-12 23:51 UTC (permalink / raw)
  To: Fenghua Yu, Thomas Gleixner, Ingo Molnar, H Peter Anvin,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm

On 3/12/19 4:00 PM, Fenghua Yu wrote:
> +#ifndef CONFIG_CPU_SUP_INTEL
>  DO_ERROR(X86_TRAP_AC,     SIGBUS,  BUS_ADRALN, NULL, "alignment check",     alignment_check)
> +#else
> +dotraplinkage void do_alignment_check(struct pt_regs *regs, long error_code)
> +{
> +	unsigned int trapnr = X86_TRAP_AC;
> +	char str[] = "alignment check";
> +	int signr = SIGBUS;
> +
> +	RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
> +
> +	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr) !=
> +			NOTIFY_STOP) {
> +		cond_local_irq_enable(regs);
> +		if (!user_mode(regs)) {
> +			/*
> +			 * Only split lock can generate #AC from kernel. Warn
> +			 * and disable #AC for split lock on current CPU.
> +			 */
> +			msr_clear_bit(MSR_TEST_CTL,
> +				      TEST_CTL_ENABLE_SPLIT_LOCK_DETECT_SHIFT)

I don't see any feature checking here.  Don't we need to see if this MSR
is supported?

Shouldn't the code here on systems that don't support split lock
disabling be the same as on CONFIG_CPU_SUP_INTEL=n systems?

> +			WARN_ONCE(1, "A split lock issue is detected.\n");
> +
> +			return;
> +		}
> +		/* Handle #AC generated from user code. */
> +		do_trap(X86_TRAP_AC, SIGBUS, "alignment check", regs,
> +			error_code, BUS_ADRALN, NULL);
> +	}
> +}
> +#endif

So...  Do we really need an Intel-specific #ifdef for this sucker?

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

* Re: [PATCH v5 07/18] x86/split_lock: Enumerate #AC for split lock by MSR IA32_CORE_CAPABILITY
  2019-03-12 23:00 ` [PATCH v5 07/18] x86/split_lock: Enumerate #AC for split lock by MSR IA32_CORE_CAPABILITY Fenghua Yu
@ 2019-03-12 23:52   ` Dave Hansen
  2019-03-13  0:56     ` Fenghua Yu
  0 siblings, 1 reply; 34+ messages in thread
From: Dave Hansen @ 2019-03-12 23:52 UTC (permalink / raw)
  To: Fenghua Yu, Thomas Gleixner, Ingo Molnar, H Peter Anvin,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li,
	Michael Chan, Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm

On 3/12/19 4:00 PM, Fenghua Yu wrote:
> +/**
> + * set_cpu_core_cap_bits - enumerate features supported in IA32_CORE_CAPABILITY
> + * @c: pointer to cpuinfo_x86
> + *
> + * Return: void
> + */
> +void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c)

I think this is kerneldoc overkill.  It just wastes space and adds no
value here.

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

* Re: [PATCH v5 09/18] x86/split_lock: Handle #AC exception for split lock
  2019-03-12 23:51   ` Dave Hansen
@ 2019-03-13  0:49     ` Fenghua Yu
  2019-03-13 16:22       ` Dave Hansen
  0 siblings, 1 reply; 34+ messages in thread
From: Fenghua Yu @ 2019-03-13  0:49 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Paolo Bonzini,
	Ashok Raj, Peter Zijlstra, Xiaoyao Li, Michael Chan,
	Ravi V Shankar, linux-kernel, x86, linux-wireless, netdev, kvm

On Tue, Mar 12, 2019 at 04:51:22PM -0700, Dave Hansen wrote:
> On 3/12/19 4:00 PM, Fenghua Yu wrote:
> I don't see any feature checking here.  Don't we need to see if this MSR
> is supported?
> 
> Shouldn't the code here on systems that don't support split lock
> disabling be the same as on CONFIG_CPU_SUP_INTEL=n systems?

You are right. Is the following #AC handler code better?

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index d26f9e9c3d83..5296021509c7 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>
@@ -293,7 +294,37 @@ DO_ERROR(X86_TRAP_OLD_MF, SIGFPE,           0, NULL, "coprocessor segment overru
 DO_ERROR(X86_TRAP_TS,     SIGSEGV,          0, NULL, "invalid TSS",         invalid_TSS)
 DO_ERROR(X86_TRAP_NP,     SIGBUS,           0, NULL, "segment not present", segment_not_present)
 DO_ERROR(X86_TRAP_SS,     SIGBUS,           0, NULL, "stack segment",       stack_segment)
-DO_ERROR(X86_TRAP_AC,     SIGBUS,  BUS_ADRALN, NULL, "alignment check",     alignment_check)
+dotraplinkage void do_alignment_check(struct pt_regs *regs, long error_code)
+{
+	unsigned int trapnr = X86_TRAP_AC;
+	char str[] = "alignment check";
+	int signr = SIGBUS;
+
+	RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
+
+	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr) !=
+			NOTIFY_STOP) {
+		cond_local_irq_enable(regs);
+		if (!user_mode(regs)) {
+			if (!this_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT))
+				return;
+
+			/*
+			 * Only split lock can generate #AC from kernel. Warn
+			 * and disable #AC for split lock on current CPU.
+			 */
+			msr_clear_bit(MSR_TEST_CTL,
+				      TEST_CTL_ENABLE_SPLIT_LOCK_DETECT_SHIFT);
+			WARN_ONCE(1, "A split lock issue is detected.\n");
+
+
+			return;
+		}
+		/* Handle #AC generated from user code. */
+		do_trap(X86_TRAP_AC, SIGBUS, "alignment check", regs,
+			error_code, BUS_ADRALN, NULL);
+	}
+}
 #undef IP
 
 #ifdef CONFIG_VMAP_STACK



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

* Re: [PATCH v5 13/18] x86/split_lock: Add a sysfs interface to allow user to enable or disable split lock detection on all CPUs during run time
  2019-03-12 23:48   ` Dave Hansen
@ 2019-03-13  0:53     ` Fenghua Yu
  2019-03-13  1:08       ` Dave Hansen
  0 siblings, 1 reply; 34+ messages in thread
From: Fenghua Yu @ 2019-03-13  0:53 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Paolo Bonzini,
	Ashok Raj, Peter Zijlstra, Xiaoyao Li, Michael Chan,
	Ravi V Shankar, linux-kernel, x86, linux-wireless, netdev, kvm

On Tue, Mar 12, 2019 at 04:48:09PM -0700, Dave Hansen wrote:
> On 3/12/19 4:00 PM, Fenghua Yu wrote:
> > The interface /sys/device/system/cpu/split_lock_detect is added
> > to allow user to control split lock detection and show current split
> > lock detection setting.
> 
> Did I miss the Documentation/ for this new sysfs feature?

I don't have a doc for the interface.

Should I create a new file in Documentation/ or can I add the interface
doc in an existing doc file?

Thanks.

-Fenghua

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

* Re: [PATCH v5 07/18] x86/split_lock: Enumerate #AC for split lock by MSR IA32_CORE_CAPABILITY
  2019-03-12 23:52   ` Dave Hansen
@ 2019-03-13  0:56     ` Fenghua Yu
  0 siblings, 0 replies; 34+ messages in thread
From: Fenghua Yu @ 2019-03-13  0:56 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Paolo Bonzini,
	Ashok Raj, Peter Zijlstra, Xiaoyao Li, Michael Chan,
	Ravi V Shankar, linux-kernel, x86, linux-wireless, netdev, kvm

On Tue, Mar 12, 2019 at 04:52:37PM -0700, Dave Hansen wrote:
> On 3/12/19 4:00 PM, Fenghua Yu wrote:
> > +/**
> > + * set_cpu_core_cap_bits - enumerate features supported in IA32_CORE_CAPABILITY
> > + * @c: pointer to cpuinfo_x86
> > + *
> > + * Return: void
> > + */
> > +void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c)
> 
> I think this is kerneldoc overkill.  It just wastes space and adds no
> value here.

OK. I will not add the function in kerneldoc.

Thanks.

-Fenghua

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

* Re: [PATCH v5 13/18] x86/split_lock: Add a sysfs interface to allow user to enable or disable split lock detection on all CPUs during run time
  2019-03-13  0:53     ` Fenghua Yu
@ 2019-03-13  1:08       ` Dave Hansen
  0 siblings, 0 replies; 34+ messages in thread
From: Dave Hansen @ 2019-03-13  1:08 UTC (permalink / raw)
  To: Fenghua Yu
  Cc: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Paolo Bonzini,
	Ashok Raj, Peter Zijlstra, Xiaoyao Li, Michael Chan,
	Ravi V Shankar, linux-kernel, x86, linux-wireless, netdev, kvm

On 3/12/19 5:53 PM, Fenghua Yu wrote:
> On Tue, Mar 12, 2019 at 04:48:09PM -0700, Dave Hansen wrote:
>> On 3/12/19 4:00 PM, Fenghua Yu wrote:
>>> The interface /sys/device/system/cpu/split_lock_detect is added
>>> to allow user to control split lock detection and show current split
>>> lock detection setting.
>> Did I miss the Documentation/ for this new sysfs feature?
> I don't have a doc for the interface.
> 
> Should I create a new file in Documentation/ or can I add the interface
> doc in an existing doc file?

I'd suggest spending a few minutes familiarizing yourself with
Documentation/.  Maybe you can even grep for some other similar files in
there and see if those are documented.

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

* Re: [PATCH v5 10/18] kvm/x86: Emulate MSR IA32_CORE_CAPABILITY
  2019-03-12 23:00 ` [PATCH v5 10/18] kvm/x86: Emulate MSR IA32_CORE_CAPABILITY Fenghua Yu
@ 2019-03-13  8:15   ` Paolo Bonzini
  0 siblings, 0 replies; 34+ messages in thread
From: Paolo Bonzini @ 2019-03-13  8:15 UTC (permalink / raw)
  To: Fenghua Yu, Thomas Gleixner, Ingo Molnar, H Peter Anvin,
	Dave Hansen, Ashok Raj, Peter Zijlstra, Xiaoyao Li, Michael Chan,
	Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Xiaoyao Li

On 13/03/19 00:00, Fenghua Yu wrote:
> From: Xiaoyao Li <xiaoyao.li@linux.intel.com>
> 
> MSR IA32_CORE_CAPABILITY is a feature-enumerating MSR, bit 5 of which
> reports the capability of enabling detection of split locks (will be
> supported on future processors based on Tremont microarchitecture and
> later).
> 
> CPUID.(EAX=7H,ECX=0):EDX[30] will enumerate the presence of the
> IA32_CORE_CAPABILITY MSR.
> 
> Please check the latest Intel 64 and IA-32 Architectures Software
> Developer's Manual for more detailed information on the MSR and
> the split lock bit.
> 
> Since MSR_IA32_CORE_CAPABILITY is a feature-enumerating MSR, we can
> emulate it in software regardless of host's capability. What we need to
> do is to set the right value of it to report the capability of guest.
> 
> In this patch we just set the guest's core_capability as 0, because we
> haven't added support of the features it indicates to guest. It's for
> bisectability.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  2 ++
>  arch/x86/kvm/cpuid.c            |  6 ++++++
>  arch/x86/kvm/x86.c              | 24 ++++++++++++++++++++++++
>  3 files changed, 32 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 180373360e34..2c53df4a5a2a 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -570,6 +570,7 @@ struct kvm_vcpu_arch {
>  	bool tpr_access_reporting;
>  	u64 ia32_xss;
>  	u64 microcode_version;
> +	u64 core_capability;
>  
>  	/*
>  	 * Paging state of the vcpu
> @@ -1527,6 +1528,7 @@ int kvm_pv_send_ipi(struct kvm *kvm, unsigned long ipi_bitmap_low,
>  		    unsigned long icr, int op_64_bit);
>  
>  u64 kvm_get_arch_capabilities(void);
> +u64 kvm_get_core_capability(void);
>  void kvm_define_shared_msr(unsigned index, u32 msr);
>  int kvm_set_shared_msr(unsigned index, u64 val, u64 mask);
>  
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index c07958b59f50..b7fb1822a1e5 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -505,6 +505,12 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  			 * if the host doesn't support it.
>  			 */
>  			entry->edx |= F(ARCH_CAPABILITIES);
> +			/*
> +			 * Since we emulate MSR IA32_CORE_CAPABILITY in
> +			 * software, we can always enable it for guest
> +			 * regardless of host's capability.
> +			 */
> +			entry->edx |= F(CORE_CAPABILITY);
>  		} else {
>  			entry->ebx = 0;
>  			entry->ecx = 0;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 941f932373d0..e20cbb8c2b74 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1158,6 +1158,7 @@ static u32 emulated_msrs[] = {
>  
>  	MSR_IA32_TSC_ADJUST,
>  	MSR_IA32_TSCDEADLINE,
> +	MSR_IA32_CORE_CAPABILITY,
>  	MSR_IA32_MISC_ENABLE,
>  	MSR_IA32_MCG_STATUS,
>  	MSR_IA32_MCG_CTL,
> @@ -1197,6 +1198,7 @@ static u32 msr_based_features[] = {
>  
>  	MSR_F10H_DECFG,
>  	MSR_IA32_UCODE_REV,
> +	MSR_IA32_CORE_CAPABILITY,
>  	MSR_IA32_ARCH_CAPABILITIES,
>  };
>  
> @@ -1224,9 +1226,18 @@ u64 kvm_get_arch_capabilities(void)
>  }
>  EXPORT_SYMBOL_GPL(kvm_get_arch_capabilities);
>  
> +u64 kvm_get_core_capability(void)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(kvm_get_core_capability);
> +
>  static int kvm_get_msr_feature(struct kvm_msr_entry *msr)
>  {
>  	switch (msr->index) {
> +	case MSR_IA32_CORE_CAPABILITY:
> +		msr->data = kvm_get_core_capability();
> +		break;
>  	case MSR_IA32_ARCH_CAPABILITIES:
>  		msr->data = kvm_get_arch_capabilities();
>  		break;
> @@ -2445,6 +2456,12 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		break;
>  	case MSR_EFER:
>  		return set_efer(vcpu, data);
> +	case MSR_IA32_CORE_CAPABILITY:
> +		if (!msr_info->host_initiated)
> +			return 1;
> +
> +		vcpu->arch.core_capability = data;
> +		break;
>  	case MSR_K7_HWCR:
>  		data &= ~(u64)0x40;	/* ignore flush filter disable */
>  		data &= ~(u64)0x100;	/* ignore ignne emulation enable */
> @@ -2750,6 +2767,12 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  	case MSR_IA32_TSC:
>  		msr_info->data = kvm_scale_tsc(vcpu, rdtsc()) + vcpu->arch.tsc_offset;
>  		break;
> +	case MSR_IA32_CORE_CAPABILITY:
> +		if (!msr_info->host_initiated &&
> +		    !guest_cpuid_has(vcpu, X86_FEATURE_CORE_CAPABILITY))
> +			return 1;
> +		msr_info->data = vcpu->arch.core_capability;
> +		break;
>  	case MSR_MTRRcap:
>  	case 0x200 ... 0x2ff:
>  		return kvm_mtrr_get_msr(vcpu, msr_info->index, &msr_info->data);
> @@ -8725,6 +8748,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
>  
>  int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>  {
> +	vcpu->arch.core_capability = kvm_get_core_capability();
>  	vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;
>  	kvm_vcpu_mtrr_init(vcpu);
>  	vcpu_load(vcpu);
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

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

* Re: [PATCH v5 11/18] kvm/vmx: Emulate MSR TEST_CTL
  2019-03-12 23:00 ` [PATCH v5 11/18] kvm/vmx: Emulate MSR TEST_CTL Fenghua Yu
@ 2019-03-13  8:15   ` Paolo Bonzini
  0 siblings, 0 replies; 34+ messages in thread
From: Paolo Bonzini @ 2019-03-13  8:15 UTC (permalink / raw)
  To: Fenghua Yu, Thomas Gleixner, Ingo Molnar, H Peter Anvin,
	Dave Hansen, Ashok Raj, Peter Zijlstra, Xiaoyao Li, Michael Chan,
	Ravi V Shankar
  Cc: linux-kernel, x86, linux-wireless, netdev, kvm, Xiaoyao Li

On 13/03/19 00:00, Fenghua Yu wrote:
> From: Xiaoyao Li <xiaoyao.li@linux.intel.com>
> 
> A control bit (bit 29) in TEST_CTL MSR 0x33 will be introduced in
> future x86 processors. When bit 29 is set, the processor causes #AC
> exception for split locked accesses at all CPL.
> 
> Please check the latest Intel 64 and IA-32 Architectures Software
> Developer's Manual for more detailed information on the MSR and
> the split lock bit.
> 
> This patch emulate MSR TEST_CTL with vmx->msr_test_ctl and does the
> following:
> 1. As we emulate MSR TEST_CTL of guest, we should enable the related bits
> in CORE_CAPABILITY to corretly report this feature to guest.
> 
> 2. Differentiate MSR TEST_CTL between host and guest.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 35 +++++++++++++++++++++++++++++++++++
>  arch/x86/kvm/vmx/vmx.h |  1 +
>  arch/x86/kvm/x86.c     | 17 ++++++++++++++++-
>  3 files changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 30a6bcd735ec..270c6566fd5a 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1659,6 +1659,12 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  	u32 index;
>  
>  	switch (msr_info->index) {
> +	case MSR_TEST_CTL:
> +		if (!msr_info->host_initiated &&
> +		    !(vcpu->arch.core_capability & CORE_CAP_SPLIT_LOCK_DETECT))
> +			return 1;
> +		msr_info->data = vmx->msr_test_ctl;
> +		break;
>  #ifdef CONFIG_X86_64
>  	case MSR_FS_BASE:
>  		msr_info->data = vmcs_readl(GUEST_FS_BASE);
> @@ -1799,6 +1805,14 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  	u32 index;
>  
>  	switch (msr_index) {
> +	case MSR_TEST_CTL:
> +		if (!(vcpu->arch.core_capability & CORE_CAP_SPLIT_LOCK_DETECT))
> +			return 1;
> +
> +		if (data & ~TEST_CTL_ENABLE_SPLIT_LOCK_DETECT)
> +			return 1;
> +		vmx->msr_test_ctl = data;
> +		break;
>  	case MSR_EFER:
>  		ret = kvm_set_msr_common(vcpu, msr_info);
>  		break;
> @@ -4085,6 +4099,9 @@ static void vmx_vcpu_setup(struct vcpu_vmx *vmx)
>  
>  	vmx->arch_capabilities = kvm_get_arch_capabilities();
>  
> +	/* disable AC split lock by default */
> +	vmx->msr_test_ctl = 0;
> +
>  	vm_exit_controls_init(vmx, vmx_vmexit_ctrl());
>  
>  	/* 22.2.1, 20.8.1 */
> @@ -4122,6 +4139,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>  
>  	vmx->rmode.vm86_active = 0;
>  	vmx->spec_ctrl = 0;
> +	vmx->msr_test_ctl = 0;
>  
>  	vcpu->arch.microcode_version = 0x100000000ULL;
>  	vmx->vcpu.arch.regs[VCPU_REGS_RDX] = get_rdx_init_val();
> @@ -6321,6 +6339,21 @@ static void atomic_switch_perf_msrs(struct vcpu_vmx *vmx)
>  					msrs[i].host, false);
>  }
>  
> +static void atomic_switch_msr_test_ctl(struct vcpu_vmx *vmx)
> +{
> +	u64 host_msr_test_ctl;
> +
> +	/* if TEST_CTL MSR doesn't exist on the hardware, we do nothing */
> +	if (rdmsrl_safe(MSR_TEST_CTL, &host_msr_test_ctl))
> +		return;
> +
> +	if (host_msr_test_ctl == vmx->msr_test_ctl)
> +		clear_atomic_switch_msr(vmx, MSR_TEST_CTL);
> +	else
> +		add_atomic_switch_msr(vmx, MSR_TEST_CTL, vmx->msr_test_ctl,
> +				      host_msr_test_ctl, false);
> +}
> +
>  static void vmx_arm_hv_timer(struct vcpu_vmx *vmx, u32 val)
>  {
>  	vmcs_write32(VMX_PREEMPTION_TIMER_VALUE, val);
> @@ -6562,6 +6595,8 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
>  
>  	atomic_switch_perf_msrs(vmx);
>  
> +	atomic_switch_msr_test_ctl(vmx);
> +
>  	vmx_update_hv_timer(vcpu);
>  
>  	/*
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index 0ac0a64c7790..8549cba0fb75 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -191,6 +191,7 @@ struct vcpu_vmx {
>  	u64		      msr_guest_kernel_gs_base;
>  #endif
>  
> +	u64		      msr_test_ctl;
>  	u64		      arch_capabilities;
>  	u64		      spec_ctrl;
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index e20cbb8c2b74..ad1df965574e 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1228,7 +1228,22 @@ EXPORT_SYMBOL_GPL(kvm_get_arch_capabilities);
>  
>  u64 kvm_get_core_capability(void)
>  {
> -	return 0;
> +	u64 data;
> +
> +	rdmsrl_safe(MSR_IA32_CORE_CAPABILITY, &data);
> +
> +	/* mask non-virtualizable functions */
> +	data &= CORE_CAP_SPLIT_LOCK_DETECT;
> +
> +	/*
> +	 * There will be a list of FMS values that have split lock detection
> +	 * but lack the CORE CAPABILITY MSR. In this case, we can set
> +	 * CORE_CAP_SPLIT_LOCK_DETECT since we emulate MSR CORE_CAPABILITY.
> +	 */
> +	if (boot_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT))
> +		data |= CORE_CAP_SPLIT_LOCK_DETECT;
> +
> +	return data;
>  }
>  EXPORT_SYMBOL_GPL(kvm_get_core_capability);
>  
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

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

* Re: [PATCH v5 09/18] x86/split_lock: Handle #AC exception for split lock
  2019-03-13  0:49     ` Fenghua Yu
@ 2019-03-13 16:22       ` Dave Hansen
  0 siblings, 0 replies; 34+ messages in thread
From: Dave Hansen @ 2019-03-13 16:22 UTC (permalink / raw)
  To: Fenghua Yu
  Cc: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Paolo Bonzini,
	Ashok Raj, Peter Zijlstra, Xiaoyao Li, Michael Chan,
	Ravi V Shankar, linux-kernel, x86, linux-wireless, netdev, kvm

On 3/12/19 5:49 PM, Fenghua Yu wrote:
> On Tue, Mar 12, 2019 at 04:51:22PM -0700, Dave Hansen wrote:
>> On 3/12/19 4:00 PM, Fenghua Yu wrote:
>> I don't see any feature checking here.  Don't we need to see if this MSR
>> is supported?
>>
>> Shouldn't the code here on systems that don't support split lock
>> disabling be the same as on CONFIG_CPU_SUP_INTEL=n systems?
> 
> You are right. Is the following #AC handler code better?

Fenghua, I'd really appreciate if you could take a deep breath and slow
down.  The most important thing is getting the right patch out and being
as respectful as possible with reviewer bandwidth.

> @@ -293,7 +294,37 @@ DO_ERROR(X86_TRAP_OLD_MF, SIGFPE,           0, NULL, "coprocessor segment overru
>  DO_ERROR(X86_TRAP_TS,     SIGSEGV,          0, NULL, "invalid TSS",         invalid_TSS)
>  DO_ERROR(X86_TRAP_NP,     SIGBUS,           0, NULL, "segment not present", segment_not_present)
>  DO_ERROR(X86_TRAP_SS,     SIGBUS,           0, NULL, "stack segment",       stack_segment)
> -DO_ERROR(X86_TRAP_AC,     SIGBUS,  BUS_ADRALN, NULL, "alignment check",     alignment_check)
> +dotraplinkage void do_alignment_check(struct pt_regs *regs, long error_code)

Is this really an appropriate place to stick this function?  Without any
whitespace, and even pushing out the "#undef" that was here before?

> +{
> +	unsigned int trapnr = X86_TRAP_AC;
> +	char str[] = "alignment check";
> +	int signr = SIGBUS;
> +
> +	RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
> +
> +	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr) !=
> +			NOTIFY_STOP) {

Please unindent this code block.

> +		cond_local_irq_enable(regs);
> +		if (!user_mode(regs)) {

Comments please.

The comment about #AC being impossible in the kernel without the split
lock detection feature belongs here, not below.

> +			if (!this_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT))
> +				return;

Is this consistent with the code that was here before?  Basically, if we
are in the kernel, get an #AC and end up here, we just return from this
function?  Is that what DO_ERROR() did?

> +			/*
> +			 * Only split lock can generate #AC from kernel. Warn
> +			 * and disable #AC for split lock on current CPU.
> +			 */
> +			msr_clear_bit(MSR_TEST_CTL,
> +				      TEST_CTL_ENABLE_SPLIT_LOCK_DETECT_SHIFT);
> +			WARN_ONCE(1, "A split lock issue is detected.\n");

Is it an issue?  I'd probably say: "split lock operation detected"

> +
> +
> +			return;

Extra whitespace.

> +		}
> +		/* Handle #AC generated from user code. */
> +		do_trap(X86_TRAP_AC, SIGBUS, "alignment check", regs,
> +			error_code, BUS_ADRALN, NULL);
> +	}
> +}
>  #undef IP

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

* Re: [PATCH v5 03/18] wlcore: simplify/fix/optimize reg_ch_conf_pending operations
  2019-03-12 23:00 ` [PATCH v5 03/18] wlcore: simplify/fix/optimize reg_ch_conf_pending operations Fenghua Yu
@ 2019-03-14 13:16   ` Kalle Valo
  2019-03-14 23:16     ` Fenghua Yu
  0 siblings, 1 reply; 34+ messages in thread
From: Kalle Valo @ 2019-03-14 13:16 UTC (permalink / raw)
  To: Fenghua Yu
  Cc: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li ,
	Michael Chan, Ravi V Shankar, linux-kernel, x86, linux-wireless,
	netdev, kvm

Fenghua Yu <fenghua.yu@intel.com> writes:

> From: Paolo Bonzini <pbonzini@redhat.com>
>
> Bitmaps are defined on unsigned longs, so the usage of u32[2] in the
> wlcore driver is incorrect.  As noted by Peter Zijlstra, casting arrays
> to a bitmap is incorrect for big-endian architectures.
>
> When looking at it I observed that:
>
> - operations on reg_ch_conf_pending is always under the wl_lock mutex,
> so set_bit is overkill
>
> - the only case where reg_ch_conf_pending is accessed a u32 at a time is
> unnecessary too.
>
> This patch cleans up everything in this area, and changes tmp_ch_bitmap
> to have the proper alignment.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>

[...]

>  int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl)
>  {
>  	struct wl12xx_cmd_regdomain_dfs_config *cmd = NULL;
>  	int ret = 0, i, b, ch_bit_idx;
> -	u32 tmp_ch_bitmap[2];
> +	u32 tmp_ch_bitmap[2] __aligned(sizeof(unsigned long));
>  	struct wiphy *wiphy = wl->hw->wiphy;
>  	struct ieee80211_supported_band *band;
>  	bool timeout = false;

[...]

> @@ -1754,8 +1751,8 @@ int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl)
>  		goto out;
>  	}
>  
> -	cmd->ch_bit_map1 = cpu_to_le32(tmp_ch_bitmap[0]);
> -	cmd->ch_bit_map2 = cpu_to_le32(tmp_ch_bitmap[1]);
> +	cmd->ch_bit_map1 = tmp_ch_bitmap[0];
> +	cmd->ch_bit_map2 = tmp_ch_bitmap[1];

Will sparse still be happy? AFAICS you are now assigning u32 to __le32:

struct wl12xx_cmd_regdomain_dfs_config {
       struct wl1271_cmd_header header;

       __le32 ch_bit_map1;
       __le32 ch_bit_map2;

Also this doesn't depend on anything else from this patchset, right? So
I could apply this directly?

-- 
Kalle Valo

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

* Re: [PATCH v5 03/18] wlcore: simplify/fix/optimize reg_ch_conf_pending operations
  2019-03-14 13:16   ` Kalle Valo
@ 2019-03-14 23:16     ` Fenghua Yu
  2019-03-15 17:17       ` Paolo Bonzini
  0 siblings, 1 reply; 34+ messages in thread
From: Fenghua Yu @ 2019-03-14 23:16 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Paolo Bonzini, Ashok Raj, Peter Zijlstra, Xiaoyao Li,
	Michael Chan, Ravi V Shankar, linux-kernel, x86, linux-wireless,
	netdev, kvm

Hi, Valo,

On Thu, Mar 14, 2019 at 03:16:33PM +0200, Kalle Valo wrote:
> Fenghua Yu <fenghua.yu@intel.com> writes:
> 
> > From: Paolo Bonzini <pbonzini@redhat.com>
> >
> > Bitmaps are defined on unsigned longs, so the usage of u32[2] in the
> > wlcore driver is incorrect.  As noted by Peter Zijlstra, casting arrays
> > to a bitmap is incorrect for big-endian architectures.
> >
> > When looking at it I observed that:
> >
> > - operations on reg_ch_conf_pending is always under the wl_lock mutex,
> > so set_bit is overkill
> >
> > - the only case where reg_ch_conf_pending is accessed a u32 at a time is
> > unnecessary too.
> >
> > This patch cleans up everything in this area, and changes tmp_ch_bitmap
> > to have the proper alignment.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> 
> [...]
> 
> >  int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl)
> >  {
> >  	struct wl12xx_cmd_regdomain_dfs_config *cmd = NULL;
> >  	int ret = 0, i, b, ch_bit_idx;
> > -	u32 tmp_ch_bitmap[2];
> > +	u32 tmp_ch_bitmap[2] __aligned(sizeof(unsigned long));
> >  	struct wiphy *wiphy = wl->hw->wiphy;
> >  	struct ieee80211_supported_band *band;
> >  	bool timeout = false;
> 
> [...]
> 
> > @@ -1754,8 +1751,8 @@ int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl)
> >  		goto out;
> >  	}
> >  
> > -	cmd->ch_bit_map1 = cpu_to_le32(tmp_ch_bitmap[0]);
> > -	cmd->ch_bit_map2 = cpu_to_le32(tmp_ch_bitmap[1]);
> > +	cmd->ch_bit_map1 = tmp_ch_bitmap[0];
> > +	cmd->ch_bit_map2 = tmp_ch_bitmap[1];
> 
> Will sparse still be happy? AFAICS you are now assigning u32 to __le32:
> 
> struct wl12xx_cmd_regdomain_dfs_config {
>        struct wl1271_cmd_header header;
> 
>        __le32 ch_bit_map1;
>        __le32 ch_bit_map2;

Discussion between Peter and Paolo (https://lkml.org/lkml/2019/3/4/521)
may answer your question.

(Sorry I didn't send to you v4 patch set)

> 
> Also this doesn't depend on anything else from this patchset, right? So
> I could apply this directly?

You are right. This patch doesn't rely on other patches from this patchset.
This patch just fixes a split lock issue. You could apply this directly
without other patches.

Thanks.

-Fenghua

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

* Re: [PATCH v5 03/18] wlcore: simplify/fix/optimize reg_ch_conf_pending operations
  2019-03-14 23:16     ` Fenghua Yu
@ 2019-03-15 17:17       ` Paolo Bonzini
  2019-03-26  7:55         ` Kalle Valo
  0 siblings, 1 reply; 34+ messages in thread
From: Paolo Bonzini @ 2019-03-15 17:17 UTC (permalink / raw)
  To: Fenghua Yu, Kalle Valo
  Cc: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Dave Hansen,
	Ashok Raj, Peter Zijlstra, Xiaoyao Li, Michael Chan,
	Ravi V Shankar, linux-kernel, x86, linux-wireless, netdev, kvm

On 15/03/19 00:16, Fenghua Yu wrote:
> Hi, Valo,
> 
> On Thu, Mar 14, 2019 at 03:16:33PM +0200, Kalle Valo wrote:
>> Fenghua Yu <fenghua.yu@intel.com> writes:
>>
>>> From: Paolo Bonzini <pbonzini@redhat.com>
>>>
>>> Bitmaps are defined on unsigned longs, so the usage of u32[2] in the
>>> wlcore driver is incorrect.  As noted by Peter Zijlstra, casting arrays
>>> to a bitmap is incorrect for big-endian architectures.
>>>
>>> When looking at it I observed that:
>>>
>>> - operations on reg_ch_conf_pending is always under the wl_lock mutex,
>>> so set_bit is overkill
>>>
>>> - the only case where reg_ch_conf_pending is accessed a u32 at a time is
>>> unnecessary too.
>>>
>>> This patch cleans up everything in this area, and changes tmp_ch_bitmap
>>> to have the proper alignment.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
>>
>> [...]
>>
>>>  int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl)
>>>  {
>>>  	struct wl12xx_cmd_regdomain_dfs_config *cmd = NULL;
>>>  	int ret = 0, i, b, ch_bit_idx;
>>> -	u32 tmp_ch_bitmap[2];
>>> +	u32 tmp_ch_bitmap[2] __aligned(sizeof(unsigned long));
>>>  	struct wiphy *wiphy = wl->hw->wiphy;
>>>  	struct ieee80211_supported_band *band;
>>>  	bool timeout = false;
>>
>> [...]
>>
>>> @@ -1754,8 +1751,8 @@ int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl)
>>>  		goto out;
>>>  	}
>>>  
>>> -	cmd->ch_bit_map1 = cpu_to_le32(tmp_ch_bitmap[0]);
>>> -	cmd->ch_bit_map2 = cpu_to_le32(tmp_ch_bitmap[1]);
>>> +	cmd->ch_bit_map1 = tmp_ch_bitmap[0];
>>> +	cmd->ch_bit_map2 = tmp_ch_bitmap[1];
>>
>> Will sparse still be happy? AFAICS you are now assigning u32 to __le32:
>>
>> struct wl12xx_cmd_regdomain_dfs_config {
>>        struct wl1271_cmd_header header;
>>
>>        __le32 ch_bit_map1;
>>        __le32 ch_bit_map2;
> 
> Discussion between Peter and Paolo (https://lkml.org/lkml/2019/3/4/521)
> may answer your question.

No, Kalle is right.  You do need to change

-	u32 tmp_ch_bitmap[2];
+	u32 tmp_ch_bitmap[2] __aligned(sizeof(unsigned long));

into

-	u32 tmp_ch_bitmap[2];
+	__le32 tmp_ch_bitmap[2] __aligned(sizeof(unsigned long));

The assignment from wl->reg_ch_conf_pending to tmp_ch_bitmap is fine
because it goes through memcpy.

Paolo

> (Sorry I didn't send to you v4 patch set)
> 
>>
>> Also this doesn't depend on anything else from this patchset, right? So
>> I could apply this directly?
> 
> You are right. This patch doesn't rely on other patches from this patchset.
> This patch just fixes a split lock issue. You could apply this directly
> without other patches.
> 
> Thanks.
> 
> -Fenghua
> 


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

* Re: [PATCH v5 03/18] wlcore: simplify/fix/optimize reg_ch_conf_pending operations
  2019-03-15 17:17       ` Paolo Bonzini
@ 2019-03-26  7:55         ` Kalle Valo
  0 siblings, 0 replies; 34+ messages in thread
From: Kalle Valo @ 2019-03-26  7:55 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Fenghua Yu, Thomas Gleixner, Ingo Molnar, H Peter Anvin,
	Dave Hansen, Ashok Raj, Peter Zijlstra, Xiaoyao Li, Michael Chan,
	Ravi V Shankar, linux-kernel, x86, linux-wireless, netdev, kvm

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 15/03/19 00:16, Fenghua Yu wrote:
>> Hi, Valo,
>> 
>> On Thu, Mar 14, 2019 at 03:16:33PM +0200, Kalle Valo wrote:
>>> Fenghua Yu <fenghua.yu@intel.com> writes:
>>>
>>>> From: Paolo Bonzini <pbonzini@redhat.com>
>>>>
>>>> Bitmaps are defined on unsigned longs, so the usage of u32[2] in the
>>>> wlcore driver is incorrect.  As noted by Peter Zijlstra, casting arrays
>>>> to a bitmap is incorrect for big-endian architectures.
>>>>
>>>> When looking at it I observed that:
>>>>
>>>> - operations on reg_ch_conf_pending is always under the wl_lock mutex,
>>>> so set_bit is overkill
>>>>
>>>> - the only case where reg_ch_conf_pending is accessed a u32 at a time is
>>>> unnecessary too.
>>>>
>>>> This patch cleans up everything in this area, and changes tmp_ch_bitmap
>>>> to have the proper alignment.
>>>>
>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
>>>
>>> [...]
>>>
>>>>  int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl)
>>>>  {
>>>>  	struct wl12xx_cmd_regdomain_dfs_config *cmd = NULL;
>>>>  	int ret = 0, i, b, ch_bit_idx;
>>>> -	u32 tmp_ch_bitmap[2];
>>>> +	u32 tmp_ch_bitmap[2] __aligned(sizeof(unsigned long));
>>>>  	struct wiphy *wiphy = wl->hw->wiphy;
>>>>  	struct ieee80211_supported_band *band;
>>>>  	bool timeout = false;
>>>
>>> [...]
>>>
>>>> @@ -1754,8 +1751,8 @@ int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl)
>>>>  		goto out;
>>>>  	}
>>>>  
>>>> -	cmd->ch_bit_map1 = cpu_to_le32(tmp_ch_bitmap[0]);
>>>> -	cmd->ch_bit_map2 = cpu_to_le32(tmp_ch_bitmap[1]);
>>>> +	cmd->ch_bit_map1 = tmp_ch_bitmap[0];
>>>> +	cmd->ch_bit_map2 = tmp_ch_bitmap[1];
>>>
>>> Will sparse still be happy? AFAICS you are now assigning u32 to __le32:
>>>
>>> struct wl12xx_cmd_regdomain_dfs_config {
>>>        struct wl1271_cmd_header header;
>>>
>>>        __le32 ch_bit_map1;
>>>        __le32 ch_bit_map2;
>> 
>> Discussion between Peter and Paolo (https://lkml.org/lkml/2019/3/4/521)
>> may answer your question.
>
> No, Kalle is right.  You do need to change
>
> -	u32 tmp_ch_bitmap[2];
> +	u32 tmp_ch_bitmap[2] __aligned(sizeof(unsigned long));
>
> into
>
> -	u32 tmp_ch_bitmap[2];
> +	__le32 tmp_ch_bitmap[2] __aligned(sizeof(unsigned long));
>
> The assignment from wl->reg_ch_conf_pending to tmp_ch_bitmap is fine
> because it goes through memcpy.

Thanks for confirming, I'll then drop patch 3 and wait for a new
version. IMHO it would be easier to submit this patch separately to
linux-wireless, no need to have within this bigger patchset.

-- 
Kalle Valo

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

end of thread, other threads:[~2019-03-26  7:55 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-12 23:00 [PATCH v5 00/18] x86/split_lock: Enable #AC exception for split locked accesses Fenghua Yu
2019-03-12 23:00 ` [PATCH v5 01/18] x86/common: Align cpu_caps_cleared and cpu_caps_set to unsigned long Fenghua Yu
2019-03-12 23:00 ` [PATCH v5 02/18] drivers/net/b44: Align pwol_mask to unsigned long for better performance Fenghua Yu
2019-03-12 23:00 ` [PATCH v5 03/18] wlcore: simplify/fix/optimize reg_ch_conf_pending operations Fenghua Yu
2019-03-14 13:16   ` Kalle Valo
2019-03-14 23:16     ` Fenghua Yu
2019-03-15 17:17       ` Paolo Bonzini
2019-03-26  7:55         ` Kalle Valo
2019-03-12 23:00 ` [PATCH v5 04/18] x86/split_lock: Align x86_capability to unsigned long to avoid split locked access Fenghua Yu
2019-03-12 23:00 ` [PATCH v5 05/18] x86/cpufeatures: Enumerate IA32_CORE_CAPABILITIES MSR Fenghua Yu
2019-03-12 23:00 ` [PATCH v5 06/18] x86/msr-index: Define IA32_CORE_CAPABILITY MSR and #AC exception for split lock bit Fenghua Yu
2019-03-12 23:00 ` [PATCH v5 07/18] x86/split_lock: Enumerate #AC for split lock by MSR IA32_CORE_CAPABILITY Fenghua Yu
2019-03-12 23:52   ` Dave Hansen
2019-03-13  0:56     ` Fenghua Yu
2019-03-12 23:00 ` [PATCH v5 08/18] x86/split_lock: Define MSR TEST_CTL register Fenghua Yu
2019-03-12 23:00 ` [PATCH v5 09/18] x86/split_lock: Handle #AC exception for split lock Fenghua Yu
2019-03-12 23:51   ` Dave Hansen
2019-03-13  0:49     ` Fenghua Yu
2019-03-13 16:22       ` Dave Hansen
2019-03-12 23:00 ` [PATCH v5 10/18] kvm/x86: Emulate MSR IA32_CORE_CAPABILITY Fenghua Yu
2019-03-13  8:15   ` Paolo Bonzini
2019-03-12 23:00 ` [PATCH v5 11/18] kvm/vmx: Emulate MSR TEST_CTL Fenghua Yu
2019-03-13  8:15   ` Paolo Bonzini
2019-03-12 23:00 ` [PATCH v5 12/18] x86/split_lock: Enable #AC for split lock by default Fenghua Yu
2019-03-12 23:43   ` Dave Hansen
2019-03-12 23:00 ` [PATCH v5 13/18] x86/split_lock: Add a sysfs interface to allow user to enable or disable split lock detection on all CPUs during run time Fenghua Yu
2019-03-12 23:48   ` Dave Hansen
2019-03-13  0:53     ` Fenghua Yu
2019-03-13  1:08       ` Dave Hansen
2019-03-12 23:00 ` [PATCH v5 14/18] x86/clearcpuid: Support multiple clearcpuid options Fenghua Yu
2019-03-12 23:00 ` [PATCH v5 15/18] x86/clearcpuid: Support feature flag string in kernel option clearcpuid Fenghua Yu
2019-03-12 23:00 ` [PATCH v5 16/18] x86/clearcpuid: Apply cleared feature bits that are forced set before Fenghua Yu
2019-03-12 23:00 ` [PATCH v5 17/18] x86/clearcpuid: Clear CPUID bit in CPUID faulting Fenghua Yu
2019-03-12 23:00 ` [PATCH v5 18/18] Change document for kernel option clearcpuid Fenghua Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).