Re: [PATCH RFC v3 2/4] x86/bus_lock: Handle warn and fatal in #DB for bus lock

2020-11-06 Thread Fenghua Yu
Hi, Xiaoyao,

On Tue, Nov 03, 2020 at 08:15:27PM +0800, Xiaoyao Li wrote:
> On 10/31/2020 8:27 AM, Fenghua Yu wrote:
> > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> > index 3c70fb34028b..1c3442000972 100644
> > --- a/arch/x86/kernel/traps.c
> > +++ b/arch/x86/kernel/traps.c
> > @@ -953,6 +953,13 @@ static __always_inline void exc_debug_user(struct 
> > pt_regs *regs,
> > goto out_irq;
> > }
> > +   /*
> > +* Handle bus lock. #DB for bus lock can only be triggered from
> > +* userspace.
> > +*/
> > +   if (!(dr6 & DR_BUS_LOCK))
> 
> it should be
> 
>   if (dr6 & DR_BUS_LOCK)
> 
> since you keep DR6.[bit 11] reserved in this version. bit 11 of
> debug_read_clear_dr6() being set to 1 means bus lock detected.

You are right. Will fix it in the next version.

Thank you very much!

-Fenghua


Re: [PATCH RFC v3 2/4] x86/bus_lock: Handle warn and fatal in #DB for bus lock

2020-11-03 Thread Xiaoyao Li

On 10/31/2020 8:27 AM, Fenghua Yu wrote:

...


diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 3c70fb34028b..1c3442000972 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -953,6 +953,13 @@ static __always_inline void exc_debug_user(struct pt_regs 
*regs,
goto out_irq;
}
  
+	/*

+* Handle bus lock. #DB for bus lock can only be triggered from
+* userspace.
+*/
+   if (!(dr6 & DR_BUS_LOCK))


it should be

if (dr6 & DR_BUS_LOCK)

since you keep DR6.[bit 11] reserved in this version. bit 11 of 
debug_read_clear_dr6() being set to 1 means bus lock detected.




+   handle_bus_lock(regs);
+
/* Add the virtual_dr6 bits for signals. */
dr6 |= current->thread.virtual_dr6;
if (dr6 & (DR_STEP | DR_TRAP_BITS) || icebp)





[PATCH RFC v3 2/4] x86/bus_lock: Handle warn and fatal in #DB for bus lock

2020-10-30 Thread Fenghua Yu
#DB for bus lock is enabled by bus lock detection bit 2 in DEBUGCTL MSR
while #AC for split lock is enabled by split lock detection bit 29 in
TEST_CTRL MSR.

Delivery of #DB for bus lock in userspace clears DR6[11]. To avoid
confusion in identifying #DB, #DB handler sets the bit to 1 before
returning to the interrupted task.

Use the existing kernel command line option "split_lock_detect=" to handle
#DB for bus lock:

split_lock_detect=
#AC for split lock  #DB for bus lock

off Do nothing  Do nothing

warnKernel OOPs Warn once per task and
Warn once per task and  and continues to run.
disable future checking When both features are
supported, warn in #DB

fatal   Kernel OOPs Send SIGBUS to user
Send SIGBUS to user
When both features are
supported, fatal in #AC.

Default option is "warn".

Hardware only generates #DB for bus lock detect when CPL>0 to avoid
nested #DB from multiple bus locks while the first #DB is being handled.
So no need to handle #DB for bus lock detected in the kernel.

Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
Change Log:
RFC v3:
- Remove DR6_RESERVED change (PeterZ).

 arch/x86/include/asm/cpu.h   |  10 ++-
 arch/x86/include/asm/msr-index.h |   1 +
 arch/x86/include/uapi/asm/debugreg.h |   1 +
 arch/x86/kernel/cpu/common.c |   2 +-
 arch/x86/kernel/cpu/intel.c  | 114 ++-
 arch/x86/kernel/traps.c  |   7 ++
 6 files changed, 113 insertions(+), 22 deletions(-)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index da78ccbd493b..e74de9fccc0b 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -41,12 +41,13 @@ 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
-extern void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c);
+extern void __init sld_setup(struct cpuinfo_x86 *c);
 extern void switch_to_sld(unsigned long tifn);
 extern bool handle_user_split_lock(struct pt_regs *regs, long error_code);
 extern bool handle_guest_split_lock(unsigned long ip);
+extern bool handle_bus_lock(struct pt_regs *regs);
 #else
-static inline void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c) {}
+static inline void __init sld_setup(struct cpuinfo_x86 *c) {}
 static inline void switch_to_sld(unsigned long tifn) {}
 static inline bool handle_user_split_lock(struct pt_regs *regs, long 
error_code)
 {
@@ -57,6 +58,11 @@ static inline bool handle_guest_split_lock(unsigned long ip)
 {
return false;
 }
+
+static inline bool handle_bus_lock(struct pt_regs *regs)
+{
+   return false;
+}
 #endif
 #ifdef CONFIG_IA32_FEAT_CTL
 void init_ia32_feat_ctl(struct cpuinfo_x86 *c);
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 972a34d93505..62f7534e0855 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -264,6 +264,7 @@
 #define DEBUGCTLMSR_LBR(1UL <<  0) /* last branch 
recording */
 #define DEBUGCTLMSR_BTF_SHIFT  1
 #define DEBUGCTLMSR_BTF(1UL <<  1) /* single-step on 
branches */
+#define DEBUGCTLMSR_BUS_LOCK_DETECT(1UL <<  2) /* Bus lock detect */
 #define DEBUGCTLMSR_TR (1UL <<  6)
 #define DEBUGCTLMSR_BTS(1UL <<  7)
 #define DEBUGCTLMSR_BTINT  (1UL <<  8)
diff --git a/arch/x86/include/uapi/asm/debugreg.h 
b/arch/x86/include/uapi/asm/debugreg.h
index d95d080b30e3..0007ba077c0c 100644
--- a/arch/x86/include/uapi/asm/debugreg.h
+++ b/arch/x86/include/uapi/asm/debugreg.h
@@ -24,6 +24,7 @@
 #define DR_TRAP3   (0x8)   /* db3 */
 #define DR_TRAP_BITS   (DR_TRAP0|DR_TRAP1|DR_TRAP2|DR_TRAP3)
 
+#define DR_BUS_LOCK(0x800) /* bus_lock */
 #define DR_STEP(0x4000)/* single-step */
 #define DR_SWITCH  (0x8000)/* task switch */
 
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 35ad8480c464..92705ac301ec 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1327,7 +1327,7 @@ static void __init early_identify_cpu(struct cpuinfo_x86 
*c)
 
cpu_set_bug_bits(c);
 
-   cpu_set_core_cap_bits(c);
+   sld_setup(c);
 
fpu__init_system(c);
 
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 59a1e3ce3f14..3aa57199484b 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -44,11 +44,15 @@ enum split_lock_detect_state {
 
 /*
  * Default to sld_off because most systems do not support split lock detection
- * split_lock_setup() will switch this to sld_warn on systems tha