Re: [PATCH v2 0/4] Intel MPX support

Re: [PATCH v2 0/4] Intel MPX support

[David Rientjes]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2528 bytes --]

On Wed, 22 Jan 2014, Qiaowei Ren wrote:

> Changes since v1:
>   * check to see if #BR occurred in userspace or kernel space.
>   * use generic structure and macro as much as possible when
>     decode mpx instructions.
> 
> Qiaowei Ren (4):
>   x86, mpx: add documentation on Intel MPX
>   x86, mpx: hook #BR exception handler to allocate bound tables
>   x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
>   x86, mpx: extend siginfo structure to include bound violation
>     information
> 
>  Documentation/x86/intel_mpx.txt    |   76 +++++++
>  arch/x86/Kconfig                   |    4 +
>  arch/x86/include/asm/mpx.h         |   63 ++++++
>  arch/x86/include/asm/processor.h   |   16 ++
>  arch/x86/kernel/Makefile           |    1 +
>  arch/x86/kernel/mpx.c              |  417 ++++++++++++++++++++++++++++++++++++
>  arch/x86/kernel/traps.c            |   61 +++++-
>  include/uapi/asm-generic/siginfo.h |    9 +-
>  include/uapi/linux/prctl.h         |    6 +
>  kernel/signal.c                    |    4 +
>  kernel/sys.c                       |   12 +
>  11 files changed, 667 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/x86/intel_mpx.txt
>  create mode 100644 arch/x86/include/asm/mpx.h
>  create mode 100644 arch/x86/kernel/mpx.c
> 

There's compiler warnings spread amongst the various patches with x86_64 
defconfig:

arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’:
arch/x86/kernel/mpx.c:415:2: warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘void *’ [-Wformat]
arch/x86/kernel/mpx.c:415:2: warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘void *’ [-Wformat]
arch/x86/kernel/mpx.c: In function ‘do_mpx_bt_fault’:
arch/x86/kernel/mpx.c:373:5: warning: ‘old_val’ may be used uninitialized in this function [-Wuninitialized]
arch/x86/kernel/mpx.c:360:25: note: ‘old_val’ was declared here

and I had to resolve the second patch manually because of "x86/traps: 
Clean up error exception handler definitions" in the x86 tree.

With 32-bit, we get casting warnings:

arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’:
arch/x86/kernel/mpx.c:407:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
arch/x86/kernel/mpx.c:409:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

and I'm pretty sure you want this to be available for such a config.

There's also whitespace in the fourth patch.

RE: [PATCH v2 0/4] Intel MPX support

[Ren, Qiaowei]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3297 bytes --]

> -----Original Message-----
> From: David Rientjes [mailto:rientjes@google.com]
> Sent: Wednesday, January 22, 2014 4:01 PM
> To: Ren, Qiaowei
> Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; x86@kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 0/4] Intel MPX support
> 
> On Wed, 22 Jan 2014, Qiaowei Ren wrote:
> 
> > Changes since v1:
> >   * check to see if #BR occurred in userspace or kernel space.
> >   * use generic structure and macro as much as possible when
> >     decode mpx instructions.
> >
> > Qiaowei Ren (4):
> >   x86, mpx: add documentation on Intel MPX
> >   x86, mpx: hook #BR exception handler to allocate bound tables
> >   x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
> >   x86, mpx: extend siginfo structure to include bound violation
> >     information
> >
> >  Documentation/x86/intel_mpx.txt    |   76 +++++++
> >  arch/x86/Kconfig                   |    4 +
> >  arch/x86/include/asm/mpx.h         |   63 ++++++
> >  arch/x86/include/asm/processor.h   |   16 ++
> >  arch/x86/kernel/Makefile           |    1 +
> >  arch/x86/kernel/mpx.c              |  417
> ++++++++++++++++++++++++++++++++++++
> >  arch/x86/kernel/traps.c            |   61 +++++-
> >  include/uapi/asm-generic/siginfo.h |    9 +-
> >  include/uapi/linux/prctl.h         |    6 +
> >  kernel/signal.c                    |    4 +
> >  kernel/sys.c                       |   12 +
> >  11 files changed, 667 insertions(+), 2 deletions(-)  create mode
> > 100644 Documentation/x86/intel_mpx.txt  create mode 100644
> > arch/x86/include/asm/mpx.h  create mode 100644 arch/x86/kernel/mpx.c
> >
> 
> There's compiler warnings spread amongst the various patches with x86_64
> defconfig:
> 
> arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’:
> arch/x86/kernel/mpx.c:415:2: warning: format ‘%lx’ expects argument of
> type ‘long unsigned int’, but argument 3 has type ‘void *’ [-Wformat]
> arch/x86/kernel/mpx.c:415:2: warning: format ‘%lx’ expects argument of
> type ‘long unsigned int’, but argument 4 has type ‘void *’ [-Wformat]
> arch/x86/kernel/mpx.c: In function ‘do_mpx_bt_fault’:
> arch/x86/kernel/mpx.c:373:5: warning: ‘old_val’ may be used uninitialized in
> this function [-Wuninitialized]
> arch/x86/kernel/mpx.c:360:25: note: ‘old_val’ was declared here
> 
I will fix these warnings.
old_val will be set by user_atomic_cmpxchg_inatomic().

> and I had to resolve the second patch manually because of "x86/traps:
> Clean up error exception handler definitions" in the x86 tree.
> 
There is a conflict with latest kernel. I just fixed it and built successfully.

> With 32-bit, we get casting warnings:
> 
> arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’:
> arch/x86/kernel/mpx.c:407:3: warning: cast to pointer from integer of different
> size [-Wint-to-pointer-cast]
> arch/x86/kernel/mpx.c:409:3: warning: cast to pointer from integer of different
> size [-Wint-to-pointer-cast]
> 
> and I'm pretty sure you want this to be available for such a config.
> 
> There's also whitespace in the fourth patch.
Thanks much for your feedback.

Qiaowei

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[PATCH v2 0/4] Intel MPX support

[Qiaowei Ren]

Changes since v1:
  * check to see if #BR occurred in userspace or kernel space.
  * use generic structure and macro as much as possible when
    decode mpx instructions.

Qiaowei Ren (4):
  x86, mpx: add documentation on Intel MPX
  x86, mpx: hook #BR exception handler to allocate bound tables
  x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
  x86, mpx: extend siginfo structure to include bound violation
    information

 Documentation/x86/intel_mpx.txt    |   76 +++++++
 arch/x86/Kconfig                   |    4 +
 arch/x86/include/asm/mpx.h         |   63 ++++++
 arch/x86/include/asm/processor.h   |   16 ++
 arch/x86/kernel/Makefile           |    1 +
 arch/x86/kernel/mpx.c              |  417 ++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/traps.c            |   61 +++++-
 include/uapi/asm-generic/siginfo.h |    9 +-
 include/uapi/linux/prctl.h         |    6 +
 kernel/signal.c                    |    4 +
 kernel/sys.c                       |   12 +
 11 files changed, 667 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/x86/intel_mpx.txt
 create mode 100644 arch/x86/include/asm/mpx.h
 create mode 100644 arch/x86/kernel/mpx.c

[PATCH v2 1/4] x86, mpx: add documentation on Intel MPX

[Qiaowei Ren]

This patch adds the Documentation/x86/intel_mpx.txt file with some
information about Intel MPX.

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
---
 Documentation/x86/intel_mpx.txt |   76 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 76 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/x86/intel_mpx.txt

diff --git a/Documentation/x86/intel_mpx.txt b/Documentation/x86/intel_mpx.txt
new file mode 100644
index 0000000..778d06e
--- /dev/null
+++ b/Documentation/x86/intel_mpx.txt
@@ -0,0 +1,76 @@
+Intel(R) MPX Overview:
+=====================
+
+Intel(R) Memory Protection Extensions (Intel(R) MPX) is a new
+capability introduced into Intel Architecture. Intel MPX can
+increase the robustness of software when it is used in conjunction
+with compiler changes to check that memory references intended
+at compile time do not become unsafe at runtime.
+
+Two of the most important goals of Intel MPX are to provide
+this capability at very low performance overhead for newly
+compiled code, and to provide compatibility mechanisms with
+legacy software components. A direct benefit Intel MPX provides
+is hardening software against malicious attacks designed to
+cause or exploit buffer overruns.
+
+For details about the Intel MPX instructions, see "Intel(R)
+Architecture Instruction Set Extensions Programming Reference".
+
+Intel(R) MPX Programming Model
+------------------------------
+
+Intel MPX introduces new registers and new instructions that
+operate on these registers. Some of the registers added are
+bounds registers which store a pointer's lower bound and upper
+bound limits. Whenever the pointer is used, the requested
+reference is checked against the pointer's associated bounds,
+thereby preventing out-of-bound memory access (such as buffer
+overflows and overruns). Out-of-bounds memory references
+initiate a #BR exception which can then be handled in an
+appropriate manner.
+
+Loading and Storing Bounds using Translation
+--------------------------------------------
+
+Intel MPX defines two instructions for load/store of the linear
+address of a pointer to a buffer, along with the bounds of the
+buffer into a paging structure of extended bounds. Specifically
+when storing extended bounds, the processor will perform address
+translation of the address where the pointer is stored to an
+address in the Bound Table (BT) to determine the store location
+of extended bounds. Loading of an extended bounds performs the
+reverse sequence.
+
+The structure in memory to load/store an extended bound is a
+4-tuple consisting of lower bound, upper bound, pointer value
+and a reserved field. Bound loads and stores access 32-bit or
+64-bit operand size according to the operation mode. Thus,
+a bound table entry is 4*32 bits in 32-bit mode and 4*64 bits
+in 64-bit mode.
+
+The linear address of a bound table is stored in a Bound
+Directory (BD) entry. And the linear address of the bound
+directory is derived from either BNDCFGU or BNDCFGS registers.
+Bounds in memory are stored in Bound Tables (BT) as an extended
+bound, which are accessed via Bound Directory (BD) and address
+translation performed by BNDLDX/BNDSTX instructions.
+
+Bounds Directory (BD) and Bounds Tables (BT) are stored in
+application memory and are allocated by the application (in case
+of kernel use, the structures will be in kernel memory). The
+bound directory and each instance of bound table are in contiguous
+linear memory.
+
+XSAVE/XRESTOR Support of Intel MPX State
+----------------------------------------
+
+Enabling Intel MPX requires an OS to manage two bits in XCR0:
+  - BNDREGS for saving and restoring registers BND0-BND3,
+  - BNDCSR for saving and restoring the user-mode configuration
+(BNDCFGU) and the status register (BNDSTATUS).
+
+The reason for having two separate bits is that BND0-BND3 is
+likely to be volatile state, while BNDCFGU and BNDSTATUS are not.
+Therefore, an OS has flexibility in handling these two states
+differently in saving or restoring them.
-- 
1.7.1

[PATCH v2 2/4] x86, mpx: hook #BR exception handler to allocate bound tables

[Qiaowei Ren]

An access to an invalid bound directory entry will cause a #BR
exception. This patch hook #BR exception handler to allocate
one bound table and bind it with that buond directory entry.

This will avoid the need of forwarding the #BR exception
to the user space when bound directory has invalid entry.

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
---
 arch/x86/include/asm/mpx.h |   35 ++++++++++++++++++++++++++++
 arch/x86/kernel/Makefile   |    1 +
 arch/x86/kernel/mpx.c      |   44 +++++++++++++++++++++++++++++++++++
 arch/x86/kernel/traps.c    |   55 +++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 134 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/include/asm/mpx.h
 create mode 100644 arch/x86/kernel/mpx.c

diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h
new file mode 100644
index 0000000..d074153
--- /dev/null
+++ b/arch/x86/include/asm/mpx.h
@@ -0,0 +1,35 @@
+#ifndef _ASM_X86_MPX_H
+#define _ASM_X86_MPX_H
+
+#include <linux/types.h>
+#include <asm/ptrace.h>
+
+#ifdef CONFIG_X86_64
+
+#define MPX_L1_BITS	28
+#define MPX_L1_SHIFT	3
+#define MPX_L2_BITS	17
+#define MPX_L2_SHIFT	5
+#define MPX_IGN_BITS	3
+#define MPX_L2_NODE_ADDR_MASK	0xfffffffffffffff8UL
+
+#define MPX_BNDSTA_ADDR_MASK	0xfffffffffffffffcUL
+#define MPX_BNDCFG_ADDR_MASK	0xfffffffffffff000UL
+
+#else
+
+#define MPX_L1_BITS	20
+#define MPX_L1_SHIFT	2
+#define MPX_L2_BITS	10
+#define MPX_L2_SHIFT	4
+#define MPX_IGN_BITS	2
+#define MPX_L2_NODE_ADDR_MASK	0xfffffffcUL
+
+#define MPX_BNDSTA_ADDR_MASK	0xfffffffcUL
+#define MPX_BNDCFG_ADDR_MASK	0xfffff000UL
+
+#endif
+
+void do_mpx_bt_fault(struct xsave_struct *xsave_buf);
+
+#endif /* _ASM_X86_MPX_H */
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index a5408b9..bba7a71 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -38,6 +38,7 @@ obj-y			+= resource.o
 
 obj-y				+= process.o
 obj-y				+= i387.o xsave.o
+obj-y				+= mpx.o
 obj-y				+= ptrace.o
 obj-$(CONFIG_X86_32)		+= tls.o
 obj-$(CONFIG_IA32_EMULATION)	+= tls.o
diff --git a/arch/x86/kernel/mpx.c b/arch/x86/kernel/mpx.c
new file mode 100644
index 0000000..767b3bf
--- /dev/null
+++ b/arch/x86/kernel/mpx.c
@@ -0,0 +1,44 @@
+#include <linux/kernel.h>
+#include <linux/syscalls.h>
+#include <asm/processor.h>
+#include <asm/mpx.h>
+#include <asm/mman.h>
+#include <asm/i387.h>
+#include <asm/fpu-internal.h>
+#include <asm/alternative.h>
+
+static bool allocate_bt(unsigned long bd_entry)
+{
+	unsigned long bt_size = 1UL << (MPX_L2_BITS+MPX_L2_SHIFT);
+	unsigned long bt_addr, old_val;
+
+	bt_addr = sys_mmap_pgoff(0, bt_size, PROT_READ | PROT_WRITE,
+			MAP_ANONYMOUS | MAP_PRIVATE | MAP_POPULATE, -1, 0);
+	if (bt_addr == -1) {
+		pr_err("L2 Node Allocation Failed at L1 addr %lx\n",
+				bd_entry);
+		return false;
+	}
+	bt_addr = (bt_addr & MPX_L2_NODE_ADDR_MASK) | 0x01;
+
+	user_atomic_cmpxchg_inatomic(&old_val,
+			(long __user *)bd_entry, 0, bt_addr);
+	if (old_val)
+		vm_munmap(bt_addr & MPX_L2_NODE_ADDR_MASK, bt_size);
+
+	return true;
+}
+
+void do_mpx_bt_fault(struct xsave_struct *xsave_buf)
+{
+	unsigned long status;
+	unsigned long bd_entry, bd_base;
+	unsigned long bd_size = 1UL << (MPX_L1_BITS+MPX_L1_SHIFT);
+
+	bd_base = xsave_buf->bndcsr.cfg_reg_u & MPX_BNDCFG_ADDR_MASK;
+	status = xsave_buf->bndcsr.status_reg;
+
+	bd_entry = status & MPX_BNDSTA_ADDR_MASK;
+	if ((bd_entry >= bd_base) && (bd_entry < bd_base + bd_size))
+		allocate_bt(bd_entry);
+}
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 8c8093b..78e9c16 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -59,6 +59,7 @@
 #include <asm/fixmap.h>
 #include <asm/mach_traps.h>
 #include <asm/alternative.h>
+#include <asm/mpx.h>
 
 #ifdef CONFIG_X86_64
 #include <asm/x86_init.h>
@@ -214,7 +215,6 @@ dotraplinkage void do_##name(struct pt_regs *regs, long error_code)	\
 DO_ERROR_INFO(X86_TRAP_DE, SIGFPE, "divide error", divide_error, FPE_INTDIV,
 		regs->ip)
 DO_ERROR(X86_TRAP_OF, SIGSEGV, "overflow", overflow)
-DO_ERROR(X86_TRAP_BR, SIGSEGV, "bounds", bounds)
 DO_ERROR_INFO(X86_TRAP_UD, SIGILL, "invalid opcode", invalid_op, ILL_ILLOPN,
 		regs->ip)
 DO_ERROR(X86_TRAP_OLD_MF, SIGFPE, "coprocessor segment overrun",
@@ -267,6 +267,59 @@ dotraplinkage void do_double_fault(struct pt_regs *regs, long error_code)
 }
 #endif
 
+dotraplinkage void do_bounds(struct pt_regs *regs, long error_code)
+{
+	enum ctx_state prev_state;
+	unsigned long status;
+	struct xsave_struct *xsave_buf;
+	struct task_struct *tsk = current;
+
+	prev_state = exception_enter();
+	if (notify_die(DIE_TRAP, "bounds", regs, error_code,
+			X86_TRAP_BR, SIGSEGV) == NOTIFY_STOP)
+		goto exit;
+	conditional_sti(regs);
+
+	if (!user_mode(regs)) {
+		if (!fixup_exception(regs)) {
+			tsk->thread.error_code = error_code;
+			tsk->thread.trap_nr = X86_TRAP_BR;
+			die("bounds", regs, error_code);
+		}
+		goto exit;
+	}
+
+	if (!boot_cpu_has(X86_FEATURE_MPX)) {
+		do_trap(X86_TRAP_BR, SIGSEGV, "bounds", regs, error_code, NULL);
+		goto exit;
+	}
+
+	fpu_xsave(&tsk->thread.fpu);
+	xsave_buf = &(tsk->thread.fpu.state->xsave);
+	status = xsave_buf->bndcsr.status_reg;
+
+	switch (status & 0x3) {
+	case 2:
+		/*
+		 * Bound directory has invalid entry.
+		 * No signal will be sent to the user space.
+		 */
+		do_mpx_bt_fault(xsave_buf);
+		break;
+
+	case 1: /* Bound violation. */
+	case 0: /* No MPX exception. */
+		do_trap(X86_TRAP_BR, SIGSEGV, "bounds", regs, error_code, NULL);
+		break;
+
+	default:
+		break;
+	}
+
+exit:
+	exception_exit(prev_state);
+}
+
 dotraplinkage void __kprobes
 do_general_protection(struct pt_regs *regs, long error_code)
 {
-- 
1.7.1

[PATCH v2 3/4] x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE

[Qiaowei Ren]

This patch adds the PR_MPX_INIT and PR_MPX_RELEASE prctl()
commands on the x86 platform. These commands can be used to
init and release MPX related resource.

A MMU notifier will be registered during PR_MPX_INIT
command execution. So the bound tables can be automatically
deallocated when one memory area is unmapped.

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
---
 arch/x86/Kconfig                 |    4 ++
 arch/x86/include/asm/mpx.h       |    9 ++++
 arch/x86/include/asm/processor.h |   16 +++++++
 arch/x86/kernel/mpx.c            |   84 ++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/prctl.h       |    6 +++
 kernel/sys.c                     |   12 +++++
 6 files changed, 131 insertions(+), 0 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ee2fb9d..695101a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -233,6 +233,10 @@ config HAVE_INTEL_TXT
 	def_bool y
 	depends on INTEL_IOMMU && ACPI
 
+config HAVE_INTEL_MPX
+	def_bool y
+	select MMU_NOTIFIER
+
 config X86_32_SMP
 	def_bool y
 	depends on X86_32 && SMP
diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h
index d074153..9652e9e 100644
--- a/arch/x86/include/asm/mpx.h
+++ b/arch/x86/include/asm/mpx.h
@@ -30,6 +30,15 @@
 
 #endif
 
+typedef union {
+	struct {
+		unsigned long ignored:MPX_IGN_BITS;
+		unsigned long l2index:MPX_L2_BITS;
+		unsigned long l1index:MPX_L1_BITS;
+	};
+	unsigned long addr;
+} mpx_addr;
+
 void do_mpx_bt_fault(struct xsave_struct *xsave_buf);
 
 #endif /* _ASM_X86_MPX_H */
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 43be6f6..ea4e72d 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -963,6 +963,22 @@ extern void start_thread(struct pt_regs *regs, unsigned long new_ip,
 extern int get_tsc_mode(unsigned long adr);
 extern int set_tsc_mode(unsigned int val);
 
+#ifdef CONFIG_HAVE_INTEL_MPX
+
+/* Init/release a process' MPX related resource */
+#define MPX_INIT(tsk)		mpx_init((tsk))
+#define MPX_RELEASE(tsk)	mpx_release((tsk))
+
+extern int mpx_init(struct task_struct *tsk);
+extern int mpx_release(struct task_struct *tsk);
+
+#else /* CONFIG_HAVE_INTEL_MPX */
+
+#define MPX_INIT(tsk)		(-EINVAL)
+#define MPX_RELEASE(tsk)	(-EINVAL)
+
+#endif /* CONFIG_HAVE_INTEL_MPX */
+
 extern u16 amd_get_nb_id(int cpu);
 
 static inline uint32_t hypervisor_cpuid_base(const char *sig, uint32_t leaves)
diff --git a/arch/x86/kernel/mpx.c b/arch/x86/kernel/mpx.c
index 767b3bf..ffe5aee 100644
--- a/arch/x86/kernel/mpx.c
+++ b/arch/x86/kernel/mpx.c
@@ -1,5 +1,7 @@
 #include <linux/kernel.h>
 #include <linux/syscalls.h>
+#include <linux/prctl.h>
+#include <linux/mmu_notifier.h>
 #include <asm/processor.h>
 #include <asm/mpx.h>
 #include <asm/mman.h>
@@ -7,6 +9,88 @@
 #include <asm/fpu-internal.h>
 #include <asm/alternative.h>
 
+static struct mmu_notifier mpx_mn;
+
+static void mpx_invl_range_end(struct mmu_notifier *mn,
+		struct mm_struct *mm,
+		unsigned long start, unsigned long end)
+{
+	struct xsave_struct *xsave_buf;
+	unsigned long bt_size = 1UL << (MPX_L2_BITS+MPX_L2_SHIFT);
+	unsigned long bt_addr;
+	unsigned long bd_base;
+	unsigned long bd_entry, bde_start, bde_end;
+	mpx_addr lap;
+
+	pgd_t *pgd;
+	pud_t *pud;
+	pmd_t *pmd;
+	pte_t *pte;
+
+	/* ignore swap notifications */
+	pgd = pgd_offset(mm, start);
+	pud = pud_offset(pgd, start);
+	pmd = pmd_offset(pud, start);
+	pte = pte_offset_kernel(pmd, start);
+	if (!pte_present(*pte) && !pte_none(*pte) && !pte_file(*pte))
+		return;
+
+	/* get bound directory base address */
+	fpu_xsave(&current->thread.fpu);
+	xsave_buf = &(current->thread.fpu.state->xsave);
+	bd_base = xsave_buf->bndcsr.cfg_reg_u & MPX_BNDCFG_ADDR_MASK;
+
+	/* get related bde range */
+	lap.addr = start;
+	bde_start = bd_base + (lap.l1index << MPX_L1_SHIFT);
+
+	lap.addr = end;
+	if (lap.ignored || lap.l2index)
+		bde_end = bd_base + (lap.l1index<<MPX_L1_SHIFT) + sizeof(long);
+	else
+		bde_end = bd_base + (lap.l1index<<MPX_L1_SHIFT);
+
+	for (bd_entry = bde_start; bd_entry < bde_end;
+		bd_entry += sizeof(unsigned long)) {
+
+		if (get_user(bt_addr, (long __user *)bd_entry))
+			return;
+
+		if (bt_addr & 0x1) {
+			user_atomic_cmpxchg_inatomic(&bt_addr,
+					(long __user *)bd_entry, bt_addr, 0);
+			do_munmap(mm, bt_addr & MPX_L2_NODE_ADDR_MASK, bt_size);
+		}
+	}
+}
+
+static struct mmu_notifier_ops mpx_mmuops = {
+	.invalidate_range_end = mpx_invl_range_end,
+};
+
+int mpx_init(struct task_struct *tsk)
+{
+	if (!boot_cpu_has(X86_FEATURE_MPX))
+		return -EINVAL;
+
+	/* register mmu_notifier to delallocate the bound tables */
+	mpx_mn.ops = &mpx_mmuops;
+	mmu_notifier_register(&mpx_mn, current->mm);
+
+	return 0;
+}
+
+int mpx_release(struct task_struct *tsk)
+{
+	if (!boot_cpu_has(X86_FEATURE_MPX))
+		return -EINVAL;
+
+	/* unregister mmu_notifier */
+	mmu_notifier_unregister(&mpx_mn, current->mm);
+
+	return 0;
+}
+
 static bool allocate_bt(unsigned long bd_entry)
 {
 	unsigned long bt_size = 1UL << (MPX_L2_BITS+MPX_L2_SHIFT);
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 289760f..19ab881 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -149,4 +149,10 @@
 
 #define PR_GET_TID_ADDRESS	40
 
+/*
+ * Init/release MPX related resource.
+ */
+#define PR_MPX_INIT	41
+#define PR_MPX_RELEASE	42
+
 #endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index c18ecca..bbaf573 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -92,6 +92,12 @@
 #ifndef SET_TSC_CTL
 # define SET_TSC_CTL(a)		(-EINVAL)
 #endif
+#ifndef MPX_INIT
+# define MPX_INIT(a)		(-EINVAL)
+#endif
+#ifndef MPX_RELEASE
+# define MPX_RELEASE(a)		(-EINVAL)
+#endif
 
 /*
  * this is where the system-wide overflow UID and GID are defined, for
@@ -1999,6 +2005,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		if (arg2 || arg3 || arg4 || arg5)
 			return -EINVAL;
 		return current->no_new_privs ? 1 : 0;
+	case PR_MPX_INIT:
+		error = MPX_INIT(me);
+		break;
+	case PR_MPX_RELEASE:
+		error = MPX_RELEASE(me);
+		break;
 	default:
 		error = -EINVAL;
 		break;
-- 
1.7.1

[PATCH v2 4/4] x86, mpx: extend siginfo structure to include bound violation information

[Qiaowei Ren]

This patch adds new fields about bound violation into siginfo
structure. si_lower and si_upper are respectively lower bound
and upper bound when bound violation is caused.

These fields will be set in #BR exception handler by decoding
the user instruction and constructing the faulting pointer.
A userspace application can get violation address, lower bound
and upper bound for bound violation from this new siginfo structure.

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
---
 arch/x86/include/asm/mpx.h         |   19 +++
 arch/x86/kernel/mpx.c              |  289 ++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/traps.c            |    6 +
 include/uapi/asm-generic/siginfo.h |    9 +-
 kernel/signal.c                    |    4 +
 5 files changed, 326 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h
index 9652e9e..e099573 100644
--- a/arch/x86/include/asm/mpx.h
+++ b/arch/x86/include/asm/mpx.h
@@ -3,6 +3,7 @@
 
 #include <linux/types.h>
 #include <asm/ptrace.h>
+#include <asm/insn.h>
 
 #ifdef CONFIG_X86_64
 
@@ -30,6 +31,22 @@
 
 #endif
 
+struct mpx_insn {
+	struct insn_field rex_prefix;	/* REX prefix */
+	struct insn_field modrm;
+	struct insn_field sib;
+	struct insn_field displacement;
+
+	unsigned char addr_bytes;	/* effective address size */
+	unsigned char limit;
+	unsigned char x86_64;
+
+	const unsigned char *kaddr;	/* kernel address of insn to analyze */
+	const unsigned char *next_byte;
+};
+
+#define MAX_MPX_INSN_SIZE	15
+
 typedef union {
 	struct {
 		unsigned long ignored:MPX_IGN_BITS;
@@ -40,5 +57,7 @@ typedef union {
 } mpx_addr;
 
 void do_mpx_bt_fault(struct xsave_struct *xsave_buf);
+void do_mpx_bounds(struct pt_regs *regs, siginfo_t *info,
+		struct xsave_struct *xsave_buf);
 
 #endif /* _ASM_X86_MPX_H */
diff --git a/arch/x86/kernel/mpx.c b/arch/x86/kernel/mpx.c
index ffe5aee..3770991 100644
--- a/arch/x86/kernel/mpx.c
+++ b/arch/x86/kernel/mpx.c
@@ -91,6 +91,269 @@ int mpx_release(struct task_struct *tsk)
 	return 0;
 }
 
+typedef enum {REG_TYPE_RM, REG_TYPE_INDEX, REG_TYPE_BASE} reg_type_t;
+static unsigned long get_reg(struct mpx_insn *insn, struct pt_regs *regs,
+			     reg_type_t type)
+{
+	int regno = 0;
+	unsigned char modrm = (unsigned char)insn->modrm.value;
+	unsigned char sib = (unsigned char)insn->sib.value;
+
+	static const int regoff[] = {
+		offsetof(struct pt_regs, ax),
+		offsetof(struct pt_regs, cx),
+		offsetof(struct pt_regs, dx),
+		offsetof(struct pt_regs, bx),
+		offsetof(struct pt_regs, sp),
+		offsetof(struct pt_regs, bp),
+		offsetof(struct pt_regs, si),
+		offsetof(struct pt_regs, di),
+#ifdef CONFIG_X86_64
+		offsetof(struct pt_regs, r8),
+		offsetof(struct pt_regs, r9),
+		offsetof(struct pt_regs, r10),
+		offsetof(struct pt_regs, r11),
+		offsetof(struct pt_regs, r12),
+		offsetof(struct pt_regs, r13),
+		offsetof(struct pt_regs, r14),
+		offsetof(struct pt_regs, r15),
+#endif
+	};
+
+	switch (type) {
+	case REG_TYPE_RM:
+		regno = X86_MODRM_RM(modrm);
+		if (X86_REX_B(insn->rex_prefix.value) == 1)
+			regno += 8;
+		break;
+
+	case REG_TYPE_INDEX:
+		regno = X86_SIB_INDEX(sib);
+		if (X86_REX_X(insn->rex_prefix.value) == 1)
+			regno += 8;
+		break;
+
+	case REG_TYPE_BASE:
+		regno = X86_SIB_BASE(sib);
+		if (X86_REX_B(insn->rex_prefix.value) == 1)
+			regno += 8;
+		break;
+
+	default:
+		break;
+	}
+
+	return regs_get_register(regs, regoff[regno]);
+}
+
+/*
+ * return the address being referenced be instruction
+ * for rm=3 returning the content of the rm reg
+ * for rm!=3 calculates the address using SIB and Disp
+ */
+static unsigned long get_addr_ref(struct mpx_insn *insn, struct pt_regs *regs)
+{
+	unsigned long addr;
+	unsigned long base;
+	unsigned long indx;
+	unsigned char modrm = (unsigned char)insn->modrm.value;
+	unsigned char sib = (unsigned char)insn->sib.value;
+
+	if (X86_MODRM_MOD(modrm) == 3) {
+		addr = get_reg(insn, regs, REG_TYPE_RM);
+	} else {
+		if (insn->sib.nbytes) {
+			base = get_reg(insn, regs, REG_TYPE_BASE);
+			indx = get_reg(insn, regs, REG_TYPE_INDEX);
+			addr = base + indx * (1 << X86_SIB_SCALE(sib));
+		} else {
+			addr = get_reg(insn, regs, REG_TYPE_RM);
+		}
+		addr += insn->displacement.value;
+	}
+
+	return addr;
+}
+
+/* Verify next sizeof(t) bytes can be on the same instruction */
+#define validate_next(t, insn, n)	\
+	((insn)->next_byte + sizeof(t) + n - (insn)->kaddr <= (insn)->limit)
+
+#define __get_next(t, insn)		\
+({					\
+	t r = *(t*)insn->next_byte;	\
+	insn->next_byte += sizeof(t);	\
+	r;				\
+})
+
+#define __peek_next(t, insn)		\
+({					\
+	t r = *(t*)insn->next_byte;	\
+	r;				\
+})
+
+#define get_next(t, insn)		\
+({					\
+	if (unlikely(!validate_next(t, insn, 0)))	\
+		goto err_out;		\
+	__get_next(t, insn);		\
+})
+
+#define peek_next(t, insn)		\
+({					\
+	if (unlikely(!validate_next(t, insn, 0)))	\
+		goto err_out;		\
+	__peek_next(t, insn);		\
+})
+
+static void mpx_insn_get_prefixes(struct mpx_insn *insn)
+{
+	unsigned char b;
+
+	/* Decode legacy prefix and REX prefix */
+	b = peek_next(unsigned char, insn);
+	while (b != 0x0f) {
+		/*
+		 * look for a rex prefix
+		 * a REX prefix cannot be followed by a legacy prefix.
+		 */
+		if (insn->x86_64 && (b&0xf0)==0x40) {
+			insn->rex_prefix.value = b;
+			insn->rex_prefix.nbytes = 1;
+			insn->next_byte++;
+			break;
+		}
+
+		/* check the other legacy prefixes */
+		switch (b) {
+		case 0xf2:
+		case 0xf3:
+		case 0xf0:
+		case 0x64:
+		case 0x65:
+		case 0x2e:
+		case 0x3e:
+		case 0x26:
+		case 0x36:
+		case 0x66:
+		case 0x67:
+			insn->next_byte++;
+			break;
+		default: /* everything else is garbage */
+			goto err_out;
+		}
+		b = peek_next(unsigned char, insn);
+	}
+
+err_out:
+	return;
+}
+
+static void mpx_insn_get_modrm(struct mpx_insn *insn)
+{
+	insn->modrm.value = get_next(unsigned char, insn);
+	insn->modrm.nbytes = 1;
+
+err_out:
+	return;
+}
+
+static void mpx_insn_get_sib(struct mpx_insn *insn)
+{
+	unsigned char modrm = (unsigned char)insn->modrm.value;
+
+	if (X86_MODRM_MOD(modrm) != 3 && X86_MODRM_RM(modrm) == 4) {
+		insn->sib.value = get_next(unsigned char, insn);
+		insn->sib.nbytes = 1;
+	}
+
+err_out:
+	return;
+}
+
+static void mpx_insn_get_displacement(struct mpx_insn *insn)
+{
+	unsigned char mod, rm, base;
+
+	/*
+	 * Interpreting the modrm byte:
+	 * mod = 00 - no displacement fields (exceptions below)
+	 * mod = 01 - 1-byte displacement field
+	 * mod = 10 - displacement field is 4 bytes
+	 * mod = 11 - no memory operand
+	 *
+	 * mod != 11, r/m = 100 - SIB byte exists
+	 * mod = 00, SIB base = 101 - displacement field is 4 bytes
+	 * mod = 00, r/m = 101 - rip-relative addressing, displacement
+	 * 	field is 4 bytes
+	 */
+	mod = X86_MODRM_MOD(insn->modrm.value);
+	rm = X86_MODRM_RM(insn->modrm.value);
+	base = X86_SIB_BASE(insn->sib.value);
+	if (mod == 3)
+		return;
+	if (mod == 1) {
+		insn->displacement.value = get_next(unsigned char, insn);
+		insn->displacement.nbytes = 1;
+	} else if ((mod == 0 && rm == 5) || mod == 2 ||
+			(mod == 0 && base == 5)) {
+		insn->displacement.value = get_next(int, insn);
+		insn->displacement.nbytes = 4;
+	}
+
+err_out:
+	return;
+}
+
+static void mpx_insn_init(struct mpx_insn *insn, struct pt_regs *regs)
+{
+	unsigned char buf[MAX_MPX_INSN_SIZE];
+	int bytes;
+
+	memset(insn, 0, sizeof(*insn));
+
+	bytes = copy_from_user(buf, (void __user *)regs->ip, MAX_MPX_INSN_SIZE);
+	insn->limit = MAX_MPX_INSN_SIZE - bytes;
+	insn->kaddr = buf;
+	insn->next_byte = buf;
+
+	/*
+	 * In 64-bit Mode, all Intel MPX instructions use 64-bit
+	 * operands for bounds and 64 bit addressing, i.e. REX.W &
+	 * 67H have no effect on data or address size.
+	 *
+	 * In compatibility and legacy modes (including 16-bit code
+	 * segments, real and virtual 8086 modes) all Intel MPX
+	 * instructions use 32-bit operands for bounds and 32 bit
+	 * addressing.
+	 */
+#ifdef CONFIG_X86_64
+	insn->x86_64 = 1;
+	insn->addr_bytes = 8;
+#else
+	insn->x86_64 = 0;
+	insn->addr_bytes = 4;
+#endif
+}
+
+static unsigned long mpx_insn_decode(struct mpx_insn *insn, struct pt_regs *regs)
+{
+	mpx_insn_init(insn, regs);
+
+	/*
+	 * In this case, we only need decode bndcl/bndcn/bndcu,
+	 * so we can use private diassembly interfaces to get
+	 * prefixes, modrm, sib, displacement, etc..
+	 */
+	mpx_insn_get_prefixes(insn);
+	insn->next_byte += 2; /* ignore opcode */
+	mpx_insn_get_modrm(insn);
+	mpx_insn_get_sib(insn);
+	mpx_insn_get_displacement(insn);
+
+	return get_addr_ref(insn, regs);
+}
+
 static bool allocate_bt(unsigned long bd_entry)
 {
 	unsigned long bt_size = 1UL << (MPX_L2_BITS+MPX_L2_SHIFT);
@@ -126,3 +389,29 @@ void do_mpx_bt_fault(struct xsave_struct *xsave_buf)
 	if ((bd_entry >= bd_base) && (bd_entry < bd_base + bd_size))
 		allocate_bt(bd_entry);
 }
+ 
+void do_mpx_bounds(struct pt_regs *regs, siginfo_t *info,
+		struct xsave_struct *xsave_buf)
+{
+	struct mpx_insn insn;
+	uint8_t bndregno;
+	unsigned long addr_vio;
+
+	addr_vio = mpx_insn_decode(&insn, regs);
+
+	bndregno = X86_MODRM_REG(insn.modrm.value);
+	if (bndregno > 3)
+		return;
+
+	info->si_lower =
+		(void __user *)(xsave_buf->bndregs.bndregs[2*bndregno]);
+	info->si_upper =
+		(void __user *)(~xsave_buf->bndregs.bndregs[2*bndregno+1]);
+	info->si_addr_lsb = 0;
+	info->si_signo = SIGSEGV;
+	info->si_errno = 0;
+	info->si_code = SEGV_BNDERR;
+	info->si_addr = (void __user *)addr_vio;
+	pr_debug("bound violation addr 0x%lx, lower 0x%lx, upper 0x%lx\n",
+			addr_vio, info->si_lower, info->si_upper);
+}
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 78e9c16..8bcb63b 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -273,6 +273,7 @@ dotraplinkage void do_bounds(struct pt_regs *regs, long error_code)
 	unsigned long status;
 	struct xsave_struct *xsave_buf;
 	struct task_struct *tsk = current;
+	siginfo_t info;
 
 	prev_state = exception_enter();
 	if (notify_die(DIE_TRAP, "bounds", regs, error_code,
@@ -308,6 +309,11 @@ dotraplinkage void do_bounds(struct pt_regs *regs, long error_code)
 		break;
 
 	case 1: /* Bound violation. */
+		do_mpx_bounds(regs, &info, xsave_buf);
+		do_trap(X86_TRAP_BR, SIGSEGV, "bounds", regs,
+				error_code, &info);
+		break;
+
 	case 0: /* No MPX exception. */
 		do_trap(X86_TRAP_BR, SIGSEGV, "bounds", regs, error_code, NULL);
 		break;
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index ba5be7f..1e35520 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -91,6 +91,10 @@ typedef struct siginfo {
 			int _trapno;	/* TRAP # which caused the signal */
 #endif
 			short _addr_lsb; /* LSB of the reported address */
+			struct {
+				void __user *_lower;
+				void __user *_upper;
+			} _addr_bnd;
 		} _sigfault;
 
 		/* SIGPOLL */
@@ -131,6 +135,8 @@ typedef struct siginfo {
 #define si_trapno	_sifields._sigfault._trapno
 #endif
 #define si_addr_lsb	_sifields._sigfault._addr_lsb
+#define si_lower	_sifields._sigfault._addr_bnd._lower
+#define si_upper	_sifields._sigfault._addr_bnd._upper
 #define si_band		_sifields._sigpoll._band
 #define si_fd		_sifields._sigpoll._fd
 #ifdef __ARCH_SIGSYS
@@ -199,7 +205,8 @@ typedef struct siginfo {
  */
 #define SEGV_MAPERR	(__SI_FAULT|1)	/* address not mapped to object */
 #define SEGV_ACCERR	(__SI_FAULT|2)	/* invalid permissions for mapped object */
-#define NSIGSEGV	2
+#define SEGV_BNDERR	(__SI_FAULT|3)  /* failed address bound checks */
+#define NSIGSEGV	3
 
 /*
  * SIGBUS si_codes
diff --git a/kernel/signal.c b/kernel/signal.c
index ded28b9..3cd6eaf 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2771,6 +2771,10 @@ int copy_siginfo_to_user(siginfo_t __user *to, siginfo_t *from)
 		if (from->si_code == BUS_MCEERR_AR || from->si_code == BUS_MCEERR_AO)
 			err |= __put_user(from->si_addr_lsb, &to->si_addr_lsb);
 #endif
+#ifdef SEGV_BNDERR
+		err |= __put_user(from->si_lower, &to->si_lower);
+		err |= __put_user(from->si_upper, &to->si_upper);
+#endif
 		break;
 	case __SI_CHLD:
 		err |= __put_user(from->si_pid, &to->si_pid);
-- 
1.7.1

Re: [PATCH v2 0/4] Intel MPX support

[Ingo Molnar]

* Qiaowei Ren <qiaowei.ren@intel.com> wrote:

> Changes since v1:
>   * check to see if #BR occurred in userspace or kernel space.
>   * use generic structure and macro as much as possible when
>     decode mpx instructions.
> 
> Qiaowei Ren (4):
>   x86, mpx: add documentation on Intel MPX
>   x86, mpx: hook #BR exception handler to allocate bound tables
>   x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
>   x86, mpx: extend siginfo structure to include bound violation
>     information
> 
>  Documentation/x86/intel_mpx.txt    |   76 +++++++
>  arch/x86/Kconfig                   |    4 +
>  arch/x86/include/asm/mpx.h         |   63 ++++++
>  arch/x86/include/asm/processor.h   |   16 ++
>  arch/x86/kernel/Makefile           |    1 +
>  arch/x86/kernel/mpx.c              |  417 ++++++++++++++++++++++++++++++++++++
>  arch/x86/kernel/traps.c            |   61 +++++-
>  include/uapi/asm-generic/siginfo.h |    9 +-
>  include/uapi/linux/prctl.h         |    6 +
>  kernel/signal.c                    |    4 +
>  kernel/sys.c                       |   12 +
>  11 files changed, 667 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/x86/intel_mpx.txt
>  create mode 100644 arch/x86/include/asm/mpx.h
>  create mode 100644 arch/x86/kernel/mpx.c

Such a patch submission is absolutely inadequate!

Please outline:

  - a short summary of what the feature does

  - a short description of what hardware supports it today or will
    support it in the future

  - a short description of whether the feature needs any
    configuration from the user or it's entirely auto-enabled on
    hardware that supports it.

  - a cost/benefit description to unrelated code: is this slowing down
    anything else?

  - how does user-space compiler support stand, what's the expected
    status there, etc.

Only a small fraction of that information can be found in 
Documentation/x86/intel_mpx.txt. in

I'm absolutely sick of these semi-anonymous patch submissions from 
Intel, so I'm NAK-ing it until it's communicated properly.

Thanks,

	Ingo

RE: [PATCH v2 0/4] Intel MPX support

[Ren, Qiaowei]

> -----Original Message-----
> From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of Ingo
> Molnar
> Sent: Wednesday, January 22, 2014 7:53 PM
> To: Ren, Qiaowei
> Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; x86@kernel.org;
> linux-kernel@vger.kernel.org; Peter Zijlstra
> Subject: Re: [PATCH v2 0/4] Intel MPX support
> 
> 
> * Qiaowei Ren <qiaowei.ren@intel.com> wrote:
> 
> > Changes since v1:
> >   * check to see if #BR occurred in userspace or kernel space.
> >   * use generic structure and macro as much as possible when
> >     decode mpx instructions.
> >
> > Qiaowei Ren (4):
> >   x86, mpx: add documentation on Intel MPX
> >   x86, mpx: hook #BR exception handler to allocate bound tables
> >   x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
> >   x86, mpx: extend siginfo structure to include bound violation
> >     information
> >
> >  Documentation/x86/intel_mpx.txt    |   76 +++++++
> >  arch/x86/Kconfig                   |    4 +
> >  arch/x86/include/asm/mpx.h         |   63 ++++++
> >  arch/x86/include/asm/processor.h   |   16 ++
> >  arch/x86/kernel/Makefile           |    1 +
> >  arch/x86/kernel/mpx.c              |  417
> ++++++++++++++++++++++++++++++++++++
> >  arch/x86/kernel/traps.c            |   61 +++++-
> >  include/uapi/asm-generic/siginfo.h |    9 +-
> >  include/uapi/linux/prctl.h         |    6 +
> >  kernel/signal.c                    |    4 +
> >  kernel/sys.c                       |   12 +
> >  11 files changed, 667 insertions(+), 2 deletions(-)  create mode
> > 100644 Documentation/x86/intel_mpx.txt  create mode 100644
> > arch/x86/include/asm/mpx.h  create mode 100644 arch/x86/kernel/mpx.c
> 
> Such a patch submission is absolutely inadequate!
> 
> Please outline:
> 
>   - a short summary of what the feature does
> 
>   - a short description of what hardware supports it today or will
>     support it in the future
> 
>   - a short description of whether the feature needs any
>     configuration from the user or it's entirely auto-enabled on
>     hardware that supports it.
> 
>   - a cost/benefit description to unrelated code: is this slowing down
>     anything else?
> 
>   - how does user-space compiler support stand, what's the expected
>     status there, etc.
> 
> Only a small fraction of that information can be found in
> Documentation/x86/intel_mpx.txt. in
> 
> I'm absolutely sick of these semi-anonymous patch submissions from Intel, so
> I'm NAK-ing it until it's communicated properly.
> 
Ok. I will add related content into this documentation.

Thanks,
Qiaowei

Re: [PATCH v2 0/4] Intel MPX support

[Ingo Molnar]

* Ren, Qiaowei <qiaowei.ren@intel.com> wrote:

> 
> 
> > -----Original Message-----
> > From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of Ingo
> > Molnar
> > Sent: Wednesday, January 22, 2014 7:53 PM
> > To: Ren, Qiaowei
> > Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; x86@kernel.org;
> > linux-kernel@vger.kernel.org; Peter Zijlstra
> > Subject: Re: [PATCH v2 0/4] Intel MPX support
> > 
> > 
> > * Qiaowei Ren <qiaowei.ren@intel.com> wrote:
> > 
> > > Changes since v1:
> > >   * check to see if #BR occurred in userspace or kernel space.
> > >   * use generic structure and macro as much as possible when
> > >     decode mpx instructions.
> > >
> > > Qiaowei Ren (4):
> > >   x86, mpx: add documentation on Intel MPX
> > >   x86, mpx: hook #BR exception handler to allocate bound tables
> > >   x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
> > >   x86, mpx: extend siginfo structure to include bound violation
> > >     information
> > >
> > >  Documentation/x86/intel_mpx.txt    |   76 +++++++
> > >  arch/x86/Kconfig                   |    4 +
> > >  arch/x86/include/asm/mpx.h         |   63 ++++++
> > >  arch/x86/include/asm/processor.h   |   16 ++
> > >  arch/x86/kernel/Makefile           |    1 +
> > >  arch/x86/kernel/mpx.c              |  417
> > ++++++++++++++++++++++++++++++++++++
> > >  arch/x86/kernel/traps.c            |   61 +++++-
> > >  include/uapi/asm-generic/siginfo.h |    9 +-
> > >  include/uapi/linux/prctl.h         |    6 +
> > >  kernel/signal.c                    |    4 +
> > >  kernel/sys.c                       |   12 +
> > >  11 files changed, 667 insertions(+), 2 deletions(-)  create mode
> > > 100644 Documentation/x86/intel_mpx.txt  create mode 100644
> > > arch/x86/include/asm/mpx.h  create mode 100644 arch/x86/kernel/mpx.c
> > 
> > Such a patch submission is absolutely inadequate!
> > 
> > Please outline:
> > 
> >   - a short summary of what the feature does
> > 
> >   - a short description of what hardware supports it today or will
> >     support it in the future
> > 
> >   - a short description of whether the feature needs any
> >     configuration from the user or it's entirely auto-enabled on
> >     hardware that supports it.
> > 
> >   - a cost/benefit description to unrelated code: is this slowing down
> >     anything else?
> > 
> >   - how does user-space compiler support stand, what's the expected
> >     status there, etc.
> > 
> > Only a small fraction of that information can be found in
> > Documentation/x86/intel_mpx.txt. in
> > 
> > I'm absolutely sick of these semi-anonymous patch submissions from Intel, so
> > I'm NAK-ing it until it's communicated properly.
> > 
> Ok. I will add related content into this documentation.

More importantly, put it into the 0/X mail! That's how people can 
review such a patch set effectively.

Thanks,

	Ingo

Re: [PATCH v2 0/4] Intel MPX support

[Ren Qiaowei]

On 01/22/2014 08:30 PM, Ingo Molnar wrote:
>
> * Ren, Qiaowei <qiaowei.ren@intel.com> wrote:
>
>>
>>
>>> -----Original Message-----
>>> From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of Ingo
>>> Molnar
>>> Sent: Wednesday, January 22, 2014 7:53 PM
>>> To: Ren, Qiaowei
>>> Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; x86@kernel.org;
>>> linux-kernel@vger.kernel.org; Peter Zijlstra
>>> Subject: Re: [PATCH v2 0/4] Intel MPX support
>>>
>>>
>>> * Qiaowei Ren <qiaowei.ren@intel.com> wrote:
>>>
>>>> Changes since v1:
>>>>    * check to see if #BR occurred in userspace or kernel space.
>>>>    * use generic structure and macro as much as possible when
>>>>      decode mpx instructions.
>>>>
>>>> Qiaowei Ren (4):
>>>>    x86, mpx: add documentation on Intel MPX
>>>>    x86, mpx: hook #BR exception handler to allocate bound tables
>>>>    x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
>>>>    x86, mpx: extend siginfo structure to include bound violation
>>>>      information
>>>>
>>>>   Documentation/x86/intel_mpx.txt    |   76 +++++++
>>>>   arch/x86/Kconfig                   |    4 +
>>>>   arch/x86/include/asm/mpx.h         |   63 ++++++
>>>>   arch/x86/include/asm/processor.h   |   16 ++
>>>>   arch/x86/kernel/Makefile           |    1 +
>>>>   arch/x86/kernel/mpx.c              |  417
>>> ++++++++++++++++++++++++++++++++++++
>>>>   arch/x86/kernel/traps.c            |   61 +++++-
>>>>   include/uapi/asm-generic/siginfo.h |    9 +-
>>>>   include/uapi/linux/prctl.h         |    6 +
>>>>   kernel/signal.c                    |    4 +
>>>>   kernel/sys.c                       |   12 +
>>>>   11 files changed, 667 insertions(+), 2 deletions(-)  create mode
>>>> 100644 Documentation/x86/intel_mpx.txt  create mode 100644
>>>> arch/x86/include/asm/mpx.h  create mode 100644 arch/x86/kernel/mpx.c
>>>
>>> Such a patch submission is absolutely inadequate!
>>>
>>> Please outline:
>>>
>>>    - a short summary of what the feature does
>>>
>>>    - a short description of what hardware supports it today or will
>>>      support it in the future
>>>
>>>    - a short description of whether the feature needs any
>>>      configuration from the user or it's entirely auto-enabled on
>>>      hardware that supports it.
>>>
>>>    - a cost/benefit description to unrelated code: is this slowing down
>>>      anything else?
>>>
>>>    - how does user-space compiler support stand, what's the expected
>>>      status there, etc.
>>>
>>> Only a small fraction of that information can be found in
>>> Documentation/x86/intel_mpx.txt. in
>>>
>>> I'm absolutely sick of these semi-anonymous patch submissions from Intel, so
>>> I'm NAK-ing it until it's communicated properly.
>>>
>> Ok. I will add related content into this documentation.
>
> More importantly, put it into the 0/X mail! That's how people can
> review such a patch set effectively.
>
Ok. Thanks for your feedback. I will do it.

Thanks,
Qiaowei