Re: [PATCH v9 09/12] x86, mpx: decode MPX instruction to get bound violation information

2014-10-30 Thread Ren Qiaowei
On 10/31/2014 06:38 AM, Dave Hansen wrote: +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(, regs); + +

Re: [PATCH v9 09/12] x86, mpx: decode MPX instruction to get bound violation information

2014-10-30 Thread Ren Qiaowei
On 10/31/2014 06:38 AM, Dave Hansen wrote: +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); + +

Re: [PATCH v9 09/12] x86, mpx: decode MPX instruction to get bound violation information

2014-10-28 Thread Ren Qiaowei
On 10/28/2014 04:36 AM, Thomas Gleixner wrote: On Mon, 27 Oct 2014, Ren, Qiaowei wrote: On 2014-10-24, Thomas Gleixner wrote: On Sun, 12 Oct 2014, Qiaowei Ren wrote: This patch sets bound violation fields of siginfo struct in #BR exception handler by decoding the user instruction

Re: [PATCH v9 11/12] x86, mpx: cleanup unused bound tables

2014-10-28 Thread Ren Qiaowei
On 10/28/2014 04:49 AM, Thomas Gleixner wrote: On Mon, 27 Oct 2014, Ren Qiaowei wrote: If so, I guess that there are some questions needed to be considered: 1) Almost all palces which call do_munmap() will need to add mpx_pre_unmap/post_unmap calls, like vm_munmap(), mremap(), shmdt(), etc

Re: [PATCH v9 10/12] x86, mpx: add prctl commands PR_MPX_ENABLE_MANAGEMENT, PR_MPX_DISABLE_MANAGEMENT

2014-10-28 Thread Ren Qiaowei
On 10/28/2014 04:38 AM, Thomas Gleixner wrote: On Mon, 27 Oct 2014, Ren, Qiaowei wrote: On 2014-10-24, Thomas Gleixner wrote: On Sun, 12 Oct 2014, Qiaowei Ren wrote: +int mpx_enable_management(struct task_struct *tsk) { + struct mm_struct *mm = tsk->mm; + void __user *bd_b

Re: [PATCH v9 11/12] x86, mpx: cleanup unused bound tables

2014-10-28 Thread Ren Qiaowei
On 10/28/2014 04:49 AM, Thomas Gleixner wrote: On Mon, 27 Oct 2014, Ren Qiaowei wrote: If so, I guess that there are some questions needed to be considered: 1) Almost all palces which call do_munmap() will need to add mpx_pre_unmap/post_unmap calls, like vm_munmap(), mremap(), shmdt(), etc

Re: [PATCH v9 10/12] x86, mpx: add prctl commands PR_MPX_ENABLE_MANAGEMENT, PR_MPX_DISABLE_MANAGEMENT

2014-10-28 Thread Ren Qiaowei
On 10/28/2014 04:38 AM, Thomas Gleixner wrote: On Mon, 27 Oct 2014, Ren, Qiaowei wrote: On 2014-10-24, Thomas Gleixner wrote: On Sun, 12 Oct 2014, Qiaowei Ren wrote: +int mpx_enable_management(struct task_struct *tsk) { + struct mm_struct *mm = tsk-mm; + void __user *bd_base

Re: [PATCH v9 09/12] x86, mpx: decode MPX instruction to get bound violation information

2014-10-28 Thread Ren Qiaowei
On 10/28/2014 04:36 AM, Thomas Gleixner wrote: On Mon, 27 Oct 2014, Ren, Qiaowei wrote: On 2014-10-24, Thomas Gleixner wrote: On Sun, 12 Oct 2014, Qiaowei Ren wrote: This patch sets bound violation fields of siginfo struct in #BR exception handler by decoding the user instruction

Re: [PATCH v9 05/12] x86, mpx: on-demand kernel allocation of bounds tables

2014-10-26 Thread Ren Qiaowei
On 10/24/2014 08:08 PM, Thomas Gleixner wrote: On Sun, 12 Oct 2014, Qiaowei Ren wrote: + /* +* Go poke the address of the new bounds table in to the +* bounds directory entry out in userspace memory. Note: +* we may race with another CPU instantiating the same

Re: [PATCH v9 11/12] x86, mpx: cleanup unused bound tables

2014-10-26 Thread Ren Qiaowei
On 10/24/2014 10:40 PM, Thomas Gleixner wrote: On Sun, 12 Oct 2014, Qiaowei Ren wrote: Since we are doing the freeing from munmap() (and other paths like it), we hold mmap_sem for write. If we fault, the page fault handler will attempt to acquire mmap_sem for read and we will deadlock. For now,

RE: [PATCH v9 10/12] x86, mpx: add prctl commands PR_MPX_ENABLE_MANAGEMENT, PR_MPX_DISABLE_MANAGEMENT

2014-10-26 Thread Ren, Qiaowei
On 2014-10-24, Thomas Gleixner wrote: > On Sun, 12 Oct 2014, Qiaowei Ren wrote: >> +int mpx_enable_management(struct task_struct *tsk) { >> +struct mm_struct *mm = tsk->mm; >> +void __user *bd_base = MPX_INVALID_BOUNDS_DIR; > > What's the point of initializing bd_base here. I had to

RE: [PATCH v9 09/12] x86, mpx: decode MPX instruction to get bound violation information

2014-10-26 Thread Ren, Qiaowei
On 2014-10-24, Thomas Gleixner wrote: > On Sun, 12 Oct 2014, Qiaowei Ren wrote: > >> This patch sets bound violation fields of siginfo struct in #BR >> exception handler by decoding the user instruction and constructing >> the faulting pointer. >> >> This patch does't use the generic decoder,

RE: [PATCH v9 09/12] x86, mpx: decode MPX instruction to get bound violation information

2014-10-26 Thread Ren, Qiaowei
On 2014-10-24, Thomas Gleixner wrote: On Sun, 12 Oct 2014, Qiaowei Ren wrote: This patch sets bound violation fields of siginfo struct in #BR exception handler by decoding the user instruction and constructing the faulting pointer. This patch does't use the generic decoder, and

RE: [PATCH v9 10/12] x86, mpx: add prctl commands PR_MPX_ENABLE_MANAGEMENT, PR_MPX_DISABLE_MANAGEMENT

2014-10-26 Thread Ren, Qiaowei
On 2014-10-24, Thomas Gleixner wrote: On Sun, 12 Oct 2014, Qiaowei Ren wrote: +int mpx_enable_management(struct task_struct *tsk) { +struct mm_struct *mm = tsk-mm; +void __user *bd_base = MPX_INVALID_BOUNDS_DIR; What's the point of initializing bd_base here. I had to look twice to

Re: [PATCH v9 11/12] x86, mpx: cleanup unused bound tables

2014-10-26 Thread Ren Qiaowei
On 10/24/2014 10:40 PM, Thomas Gleixner wrote: On Sun, 12 Oct 2014, Qiaowei Ren wrote: Since we are doing the freeing from munmap() (and other paths like it), we hold mmap_sem for write. If we fault, the page fault handler will attempt to acquire mmap_sem for read and we will deadlock. For now,

Re: [PATCH v9 05/12] x86, mpx: on-demand kernel allocation of bounds tables

2014-10-26 Thread Ren Qiaowei
On 10/24/2014 08:08 PM, Thomas Gleixner wrote: On Sun, 12 Oct 2014, Qiaowei Ren wrote: + /* +* Go poke the address of the new bounds table in to the +* bounds directory entry out in userspace memory. Note: +* we may race with another CPU instantiating the same

RE: [PATCH v7 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER

2014-10-13 Thread Ren, Qiaowei
On 2014-10-14, Hansen, Dave wrote: > On 07/20/2014 11:09 PM, Andi Kleen wrote: >> Qiaowei Ren writes: >>> This patch adds the PR_MPX_REGISTER and PR_MPX_UNREGISTER prctl() >>> commands. These commands can be used to register and unregister MPX >>> related resource on the x86 platform. >> >>

RE: [PATCH v7 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER

2014-10-13 Thread Ren, Qiaowei
On 2014-10-14, Hansen, Dave wrote: On 07/20/2014 11:09 PM, Andi Kleen wrote: Qiaowei Ren qiaowei@intel.com writes: This patch adds the PR_MPX_REGISTER and PR_MPX_UNREGISTER prctl() commands. These commands can be used to register and unregister MPX related resource on the x86 platform

RE: [PATCH v8 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER

2014-09-17 Thread Ren, Qiaowei
On 2014-09-18, Kevin Easton wrote: > On Thu, Sep 18, 2014 at 12:40:29AM +0000, Ren, Qiaowei wrote: >>> Would it be prudent to use an error code other than EINVAL for the >>> "hardware doesn't support it" case? >>> >> Seems like no specific error

RE: [PATCH v8 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER

2014-09-17 Thread Ren, Qiaowei
On 2014-09-16, Kevin Easton wrote: > On Thu, Sep 11, 2014 at 04:46:48PM +0800, Qiaowei Ren wrote: >> + >> +int mpx_register(struct task_struct *tsk) { >> +struct mm_struct *mm = tsk->mm; >> + >> +if (!cpu_has_mpx) >> +return -EINVAL; >> + >> +/* >> + * runtime in the

RE: [PATCH v8 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER

2014-09-17 Thread Ren, Qiaowei
On 2014-09-16, Kevin Easton wrote: On Thu, Sep 11, 2014 at 04:46:48PM +0800, Qiaowei Ren wrote: + +int mpx_register(struct task_struct *tsk) { +struct mm_struct *mm = tsk-mm; + +if (!cpu_has_mpx) +return -EINVAL; + +/* + * runtime in the userspace will be

RE: [PATCH v8 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER

2014-09-17 Thread Ren, Qiaowei
On 2014-09-18, Kevin Easton wrote: On Thu, Sep 18, 2014 at 12:40:29AM +, Ren, Qiaowei wrote: Would it be prudent to use an error code other than EINVAL for the hardware doesn't support it case? Seems like no specific error code for this case. ENXIO would probably be OK. It's

RE: [PATCH v8 09/10] x86, mpx: cleanup unused bound tables

2014-09-16 Thread Ren, Qiaowei
On 2014-09-16, Hansen, Dave wrote: > On 09/11/2014 01:46 AM, Qiaowei Ren wrote: >> +/* >> + * Free the backing physical pages of bounds table 'bt_addr'. >> + * Assume start...end is within that bounds table. >> + */ >> +static int __must_check zap_bt_entries(struct mm_struct *mm, >> +

RE: [PATCH v8 09/10] x86, mpx: cleanup unused bound tables

2014-09-16 Thread Ren, Qiaowei
On 2014-09-16, Hansen, Dave wrote: On 09/11/2014 01:46 AM, Qiaowei Ren wrote: +/* + * Free the backing physical pages of bounds table 'bt_addr'. + * Assume start...end is within that bounds table. + */ +static int __must_check zap_bt_entries(struct mm_struct *mm, +unsigned

RE: [PATCH v8 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER

2014-09-15 Thread Ren, Qiaowei
On 2014-09-15, One Thousand Gnomes wrote: >> The base of the bounds directory is set into mm_struct during >> PR_MPX_REGISTER command execution. This member can be used to check >> whether one application is mpx enabled. > > Not really because by the time you ask the question another thread >

RE: [PATCH v8 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER

2014-09-15 Thread Ren, Qiaowei
On 2014-09-15, One Thousand Gnomes wrote: The base of the bounds directory is set into mm_struct during PR_MPX_REGISTER command execution. This member can be used to check whether one application is mpx enabled. Not really because by the time you ask the question another thread might have

RE: [PATCH v8 04/10] x86, mpx: hook #BR exception handler to allocate bound tables

2014-09-13 Thread Ren, Qiaowei
On 2014-09-13, Hansen, Dave wrote: > On 09/11/2014 01:46 AM, Qiaowei Ren wrote: >> +static int allocate_bt(long __user *bd_entry) { >> +unsigned long bt_addr, old_val = 0; >> +int ret = 0; >> + >> +bt_addr = mpx_mmap(MPX_BT_SIZE_BYTES); >> +if (IS_ERR((void *)bt_addr)) >> +

RE: [PATCH v8 06/10] mips: sync struct siginfo with general version

2014-09-13 Thread Ren, Qiaowei
On 2014-09-12, Thomas Gleixner wrote: > On Fri, 12 Sep 2014, Ren, Qiaowei wrote: >> On 2014-09-12, Thomas Gleixner wrote: >>> On Thu, 11 Sep 2014, Qiaowei Ren wrote: >>> >>>> Due to new fields about bound violation added into struct >>>>

RE: [PATCH v8 06/10] mips: sync struct siginfo with general version

2014-09-13 Thread Ren, Qiaowei
On 2014-09-12, Thomas Gleixner wrote: On Fri, 12 Sep 2014, Ren, Qiaowei wrote: On 2014-09-12, Thomas Gleixner wrote: On Thu, 11 Sep 2014, Qiaowei Ren wrote: Due to new fields about bound violation added into struct siginfo, this patch syncs it with general version to avoid build issue

RE: [PATCH v8 04/10] x86, mpx: hook #BR exception handler to allocate bound tables

2014-09-13 Thread Ren, Qiaowei
On 2014-09-13, Hansen, Dave wrote: On 09/11/2014 01:46 AM, Qiaowei Ren wrote: +static int allocate_bt(long __user *bd_entry) { +unsigned long bt_addr, old_val = 0; +int ret = 0; + +bt_addr = mpx_mmap(MPX_BT_SIZE_BYTES); +if (IS_ERR((void *)bt_addr)) +return

RE: [PATCH v8 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER

2014-09-11 Thread Ren, Qiaowei
On 2014-09-11, Hansen, Dave wrote: > On 09/11/2014 01:46 AM, Qiaowei Ren wrote: >> + >> +return (void __user *)(unsigned long)(xsave_buf->bndcsr.cfg_reg_u & >> +MPX_BNDCFG_ADDR_MASK); >> +} > > I don't think casting a u64 to a ulong, then to a pointer is useful. > Just

RE: [PATCH v8 09/10] x86, mpx: cleanup unused bound tables

2014-09-11 Thread Ren, Qiaowei
On 2014-09-11, Hansen, Dave wrote: > On 09/11/2014 01:46 AM, Qiaowei Ren wrote: >> + * This function will be called by do_munmap(), and the VMAs >> + covering >> + * the virtual address region start...end have already been split >> + if >> + * necessary and remvoed from the VMA list. > >

RE: [PATCH v8 06/10] mips: sync struct siginfo with general version

2014-09-11 Thread Ren, Qiaowei
On 2014-09-12, Thomas Gleixner wrote: > On Thu, 11 Sep 2014, Qiaowei Ren wrote: > >> Due to new fields about bound violation added into struct siginfo, >> this patch syncs it with general version to avoid build issue. > > You completely fail to explain which build issue is addressed by this >

RE: [PATCH v8 06/10] mips: sync struct siginfo with general version

2014-09-11 Thread Ren, Qiaowei
when arch=mips. In addition, only MIPS arch define its own struct siginfo, so this is only affecting MIPS. Thanks, Qiaowei Signed-off-by: Qiaowei Ren qiaowei@intel.com --- arch/mips/include/uapi/asm/siginfo.h |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git

RE: [PATCH v8 09/10] x86, mpx: cleanup unused bound tables

2014-09-11 Thread Ren, Qiaowei
On 2014-09-11, Hansen, Dave wrote: On 09/11/2014 01:46 AM, Qiaowei Ren wrote: + * This function will be called by do_munmap(), and the VMAs + covering + * the virtual address region start...end have already been split + if + * necessary and remvoed from the VMA list. remvoed - removed

RE: [PATCH v8 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER

2014-09-11 Thread Ren, Qiaowei
On 2014-09-11, Hansen, Dave wrote: On 09/11/2014 01:46 AM, Qiaowei Ren wrote: + +return (void __user *)(unsigned long)(xsave_buf-bndcsr.cfg_reg_u +MPX_BNDCFG_ADDR_MASK); +} I don't think casting a u64 to a ulong, then to a pointer is useful. Just take the

RE: [PATCH v7 03/10] x86, mpx: add macro cpu_has_mpx

2014-07-23 Thread Ren, Qiaowei
On 2014-07-24, Hansen, Dave wrote: > On 07/23/2014 05:56 PM, Ren, Qiaowei wrote: >> On 2014-07-24, Hansen, Dave wrote: >>> On 07/22/2014 07:35 PM, Ren, Qiaowei wrote: >>>> The checking about MPX feature should be as follow: >>>> &

RE: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables

2014-07-23 Thread Ren, Qiaowei
On 2014-07-24, Hansen, Dave wrote: > On 07/23/2014 05:49 PM, Ren, Qiaowei wrote: >> I can check a lot of debug information when one VMA and related >> bounds tables are allocated and freed through adding a lot of >> print() like log into kernel/runtime. Do you think t

RE: [PATCH v7 03/10] x86, mpx: add macro cpu_has_mpx

2014-07-23 Thread Ren, Qiaowei
On 2014-07-24, Hansen, Dave wrote: > On 07/22/2014 07:35 PM, Ren, Qiaowei wrote: >> The checking about MPX feature should be as follow: >> >> if (pcntxt_mask & XSTATE_EAGER) { >> if (eagerfpu == DISABLE) { >>

RE: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables

2014-07-23 Thread Ren, Qiaowei
On 2014-07-24, Hansen, Dave wrote: > On 07/20/2014 10:38 PM, Qiaowei Ren wrote: >> Since the kernel allocated those tables on-demand without userspace >> knowledge, it is also responsible for freeing them when the >> associated mappings go away. >> >> Here, the solution for this issue is to

RE: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables

2014-07-23 Thread Ren, Qiaowei
On 2014-07-24, Hansen, Dave wrote: On 07/20/2014 10:38 PM, Qiaowei Ren wrote: Since the kernel allocated those tables on-demand without userspace knowledge, it is also responsible for freeing them when the associated mappings go away. Here, the solution for this issue is to hook

RE: [PATCH v7 03/10] x86, mpx: add macro cpu_has_mpx

2014-07-23 Thread Ren, Qiaowei
On 2014-07-24, Hansen, Dave wrote: On 07/22/2014 07:35 PM, Ren, Qiaowei wrote: The checking about MPX feature should be as follow: if (pcntxt_mask XSTATE_EAGER) { if (eagerfpu == DISABLE) { pr_err(eagerfpu not present, disabling some

RE: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables

2014-07-23 Thread Ren, Qiaowei
On 2014-07-24, Hansen, Dave wrote: On 07/23/2014 05:49 PM, Ren, Qiaowei wrote: I can check a lot of debug information when one VMA and related bounds tables are allocated and freed through adding a lot of print() like log into kernel/runtime. Do you think this is enough? I thought

RE: [PATCH v7 03/10] x86, mpx: add macro cpu_has_mpx

2014-07-23 Thread Ren, Qiaowei
On 2014-07-24, Hansen, Dave wrote: On 07/23/2014 05:56 PM, Ren, Qiaowei wrote: On 2014-07-24, Hansen, Dave wrote: On 07/22/2014 07:35 PM, Ren, Qiaowei wrote: The checking about MPX feature should be as follow: if (pcntxt_mask XSTATE_EAGER) { if (eagerfpu

RE: [PATCH v7 03/10] x86, mpx: add macro cpu_has_mpx

2014-07-22 Thread Ren, Qiaowei
On 2014-07-23, Hansen, Dave wrote: > On 07/20/2014 10:38 PM, Qiaowei Ren wrote: >> +#ifdef CONFIG_X86_INTEL_MPX >> +#define cpu_has_mpx boot_cpu_has(X86_FEATURE_MPX) #else #define >> +cpu_has_mpx 0 #endif /* CONFIG_X86_INTEL_MPX */ > > Is this enough checking? Looking at the extension

RE: [PATCH v7 03/10] x86, mpx: add macro cpu_has_mpx

2014-07-22 Thread Ren, Qiaowei
On 2014-07-23, Hansen, Dave wrote: On 07/20/2014 10:38 PM, Qiaowei Ren wrote: +#ifdef CONFIG_X86_INTEL_MPX +#define cpu_has_mpx boot_cpu_has(X86_FEATURE_MPX) #else #define +cpu_has_mpx 0 #endif /* CONFIG_X86_INTEL_MPX */ Is this enough checking? Looking at the extension reference, it

RE: [PATCH v7 07/10] x86, mpx: decode MPX instruction to get bound violation information

2014-07-21 Thread Ren, Qiaowei
On 2014-07-21, Andi Kleen wrote: > Qiaowei Ren writes: >> + */ >> +#ifdef CONFIG_X86_64 >> +insn->x86_64 = 1; >> +insn->addr_bytes = 8; >> +#else >> +insn->x86_64 = 0; >> +insn->addr_bytes = 4; >> +#endif > > How would that handle compat mode on a 64bit kernel? > Should

RE: [PATCH v7 07/10] x86, mpx: decode MPX instruction to get bound violation information

2014-07-21 Thread Ren, Qiaowei
On 2014-07-21, Andi Kleen wrote: Qiaowei Ren qiaowei@intel.com writes: + */ +#ifdef CONFIG_X86_64 +insn-x86_64 = 1; +insn-addr_bytes = 8; +#else +insn-x86_64 = 0; +insn-addr_bytes = 4; +#endif How would that handle compat mode on a 64bit kernel? Should likely

RE: [PATCH v6 02/10] x86, mpx: add MPX specific mmap interface

2014-06-24 Thread Ren, Qiaowei
On 2014-06-25, Andy Lutomirski wrote: > On Mon, Jun 23, 2014 at 10:53 PM, Ren, Qiaowei > wrote: >> On 2014-06-24, Andy Lutomirski wrote: >>>> On 06/23/2014 01:06 PM, Andy Lutomirski wrote: >>>>> Can the new vm_operation "name" be use for thi

RE: [PATCH v6 02/10] x86, mpx: add MPX specific mmap interface

2014-06-24 Thread Ren, Qiaowei
On 2014-06-25, Andy Lutomirski wrote: On Mon, Jun 23, 2014 at 10:53 PM, Ren, Qiaowei qiaowei@intel.com wrote: On 2014-06-24, Andy Lutomirski wrote: On 06/23/2014 01:06 PM, Andy Lutomirski wrote: Can the new vm_operation name be use for this? The magic always written to core dumps

RE: [PATCH v6 02/10] x86, mpx: add MPX specific mmap interface

2014-06-23 Thread Ren, Qiaowei
On 2014-06-24, Andy Lutomirski wrote: >> On 06/23/2014 01:06 PM, Andy Lutomirski wrote: >>> Can the new vm_operation "name" be use for this? The magic "always >>> written to core dumps" feature might need to be reconsidered. >> >> One thing I'd like to avoid is an MPX vma getting merged with a

RE: [PATCH v6 02/10] x86, mpx: add MPX specific mmap interface

2014-06-23 Thread Ren, Qiaowei
On 2014-06-24, Andy Lutomirski wrote: >> +/* Make bounds tables and bouds directory unlocked. */ >> +if (vm_flags & VM_LOCKED) >> +vm_flags &= ~VM_LOCKED; > > Why? I would expect MCL_FUTURE to lock these. > Andy, I was just a little confused about LOCKED & POPULATE earlier

RE: [PATCH v6 04/10] x86, mpx: hook #BR exception handler to allocate bound tables

2014-06-23 Thread Ren, Qiaowei
On 2014-06-24, Andy Lutomirski wrote: > On 06/18/2014 02:44 AM, Qiaowei Ren wrote: >> +/* >> + * The error code field of the BNDSTATUS register communicates status >> + * information of a bound range exception #BR or operation involving >> + * bound directory. >> + */ >> +

RE: [PATCH v6 04/10] x86, mpx: hook #BR exception handler to allocate bound tables

2014-06-23 Thread Ren, Qiaowei
On 2014-06-24, Andy Lutomirski wrote: On 06/18/2014 02:44 AM, Qiaowei Ren wrote: +/* + * The error code field of the BNDSTATUS register communicates status + * information of a bound range exception #BR or operation involving + * bound directory. + */ +switch (status

RE: [PATCH v6 02/10] x86, mpx: add MPX specific mmap interface

2014-06-23 Thread Ren, Qiaowei
On 2014-06-24, Andy Lutomirski wrote: +/* Make bounds tables and bouds directory unlocked. */ +if (vm_flags VM_LOCKED) +vm_flags = ~VM_LOCKED; Why? I would expect MCL_FUTURE to lock these. Andy, I was just a little confused about LOCKED POPULATE earlier and I

RE: [PATCH v6 02/10] x86, mpx: add MPX specific mmap interface

2014-06-23 Thread Ren, Qiaowei
On 2014-06-24, Andy Lutomirski wrote: On 06/23/2014 01:06 PM, Andy Lutomirski wrote: Can the new vm_operation name be use for this? The magic always written to core dumps feature might need to be reconsidered. One thing I'd like to avoid is an MPX vma getting merged with a non-MPX vma. I

RE: [PATCH v6 07/10] x86, mpx: decode MPX instruction to get bound violation information

2014-06-19 Thread Ren, Qiaowei
On 2014-06-20, H. Peter Anvin wrote: > On 06/19/2014 10:04 AM, Dave Hansen wrote: >> >> Could you please support this position with some data? I'm a bit >> skeptical that instruction decoding is going to be a >> performance-critical path. >> >> I also don't see the extra field that you talked

RE: [PATCH v6 07/10] x86, mpx: decode MPX instruction to get bound violation information

2014-06-19 Thread Ren, Qiaowei
On 2014-06-19, Borislav Petkov wrote: > On Thu, Jun 19, 2014 at 01:13:48AM +0000, Ren, Qiaowei wrote: >> On 2014-06-18, Borislav Petkov wrote: >>> On Wed, Jun 18, 2014 at 05:44:13PM +0800, Qiaowei Ren wrote: >>> >>> This whole insn decoding machinery above look

RE: [PATCH v6 07/10] x86, mpx: decode MPX instruction to get bound violation information

2014-06-19 Thread Ren, Qiaowei
On 2014-06-19, Borislav Petkov wrote: On Thu, Jun 19, 2014 at 01:13:48AM +, Ren, Qiaowei wrote: On 2014-06-18, Borislav Petkov wrote: On Wed, Jun 18, 2014 at 05:44:13PM +0800, Qiaowei Ren wrote: This whole insn decoding machinery above looks like adapted from arch/x86/lib/insn.c. You

RE: [PATCH v6 07/10] x86, mpx: decode MPX instruction to get bound violation information

2014-06-19 Thread Ren, Qiaowei
On 2014-06-20, H. Peter Anvin wrote: On 06/19/2014 10:04 AM, Dave Hansen wrote: Could you please support this position with some data? I'm a bit skeptical that instruction decoding is going to be a performance-critical path. I also don't see the extra field that you talked about in the

RE: [PATCH v6 07/10] x86, mpx: decode MPX instruction to get bound violation information

2014-06-18 Thread Ren, Qiaowei
On 2014-06-18, Borislav Petkov wrote: > On Wed, Jun 18, 2014 at 05:44:13PM +0800, Qiaowei Ren wrote: > > This whole insn decoding machinery above looks like adapted from > arch/x86/lib/insn.c. You should merge it with the generic code in > insn.c instead of homegrowing it here only for the

RE: [PATCH v6 07/10] x86, mpx: decode MPX instruction to get bound violation information

2014-06-18 Thread Ren, Qiaowei
On 2014-06-18, Borislav Petkov wrote: On Wed, Jun 18, 2014 at 05:44:13PM +0800, Qiaowei Ren wrote: This whole insn decoding machinery above looks like adapted from arch/x86/lib/insn.c. You should merge it with the generic code in insn.c instead of homegrowing it here only for the purposes of

Re: [PATCH v5 1/3] x86, mpx: add documentation on Intel MPX

2014-02-26 Thread Ren Qiaowei
On 02/27/2014 05:44 AM, H. Peter Anvin wrote: On 02/26/2014 11:17 AM, Dave Hansen wrote: On 02/23/2014 05:27 AM, Qiaowei Ren wrote: +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

Re: [PATCH v5 1/3] x86, mpx: add documentation on Intel MPX

2014-02-26 Thread Ren Qiaowei
On 02/27/2014 05:19 AM, Dave Hansen wrote: On 02/23/2014 05:27 AM, Qiaowei Ren wrote: +The other case that generates a #BR is when a BNDSTX instruction +attempts to save bounds to a BD entry marked as invalid. This is +an indication that no BT exists for this entry. In this case the +fault

Re: [PATCH v5 1/3] x86, mpx: add documentation on Intel MPX

2014-02-26 Thread Ren Qiaowei
On 02/27/2014 05:19 AM, Dave Hansen wrote: On 02/23/2014 05:27 AM, Qiaowei Ren wrote: +The other case that generates a #BR is when a BNDSTX instruction +attempts to save bounds to a BD entry marked as invalid. This is +an indication that no BT exists for this entry. In this case the +fault

Re: [PATCH v5 1/3] x86, mpx: add documentation on Intel MPX

2014-02-26 Thread Ren Qiaowei
On 02/27/2014 05:44 AM, H. Peter Anvin wrote: On 02/26/2014 11:17 AM, Dave Hansen wrote: On 02/23/2014 05:27 AM, Qiaowei Ren wrote: +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

RE: [PATCH v5 2/3] x86, mpx: hook #BR exception handler to allocate bound tables

2014-02-25 Thread Ren, Qiaowei
> -Original Message- > From: H. Peter Anvin [mailto:h...@zytor.com] > Sent: Tuesday, February 25, 2014 1:52 AM > To: Hansen, Dave; Ren, Qiaowei; Thomas Gleixner; Ingo Molnar > Cc: x...@kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v5 2/3] x86, mpx:

RE: [PATCH v5 2/3] x86, mpx: hook #BR exception handler to allocate bound tables

2014-02-25 Thread Ren, Qiaowei
-Original Message- From: H. Peter Anvin [mailto:h...@zytor.com] Sent: Tuesday, February 25, 2014 1:52 AM To: Hansen, Dave; Ren, Qiaowei; Thomas Gleixner; Ingo Molnar Cc: x...@kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 2/3] x86, mpx: hook #BR exception handler

Re: [PATCH v4 2/3] x86, mpx: hook #BR exception handler to allocate bound tables

2014-02-20 Thread Ren Qiaowei
On 02/13/2014 04:19 AM, Andy Lutomirski wrote: On 02/12/2014 10:36 AM, Qiaowei Ren wrote: 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

Re: [PATCH v4 2/3] x86, mpx: hook #BR exception handler to allocate bound tables

2014-02-20 Thread Ren Qiaowei
the need of forwarding the #BR exception to the user space when bound directory has invalid entry. Signed-off-by: Qiaowei Ren qiaowei@intel.com --- +void do_mpx_bt_fault(struct xsave_struct *xsave_buf) +{ + unsigned long status; + unsigned long bd_entry, bd_base; + unsigned long

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

2014-01-27 Thread Ren Qiaowei
On 01/28/2014 02:42 PM, Ingo Molnar wrote: * Ren Qiaowei wrote: MPX kernel code, namely this patchset, has mainly the 2 responsibilities: provide handlers for bounds faults (#BR), and manage bounds memory. AFAICS the kernel side implementation causes no runtime overhead for non-MPX

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

2014-01-27 Thread Ren Qiaowei
On 01/28/2014 02:42 PM, Andy Lutomirski wrote: I just read it. do_trap_no_signal presumably calls fixup_exception because #UD uses it and #UD needs that handling. (I'm guessing that there is actually a legitimate use for a kernel fixup on #UD somewhere -- there's probably something that isn't

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

2014-01-27 Thread Ren Qiaowei
On 01/28/2014 01:21 PM, Andy Lutomirski wrote: On Mon, Jan 27, 2014 at 7:35 PM, Ren Qiaowei wrote: On 01/28/2014 04:36 AM, Andy Lutomirski wrote: + bd_entry = status & MPX_BNDSTA_ADDR_MASK; + if ((bd_entry >= bd_base) && (bd_entry < bd_base + bd_size)) +

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

2014-01-27 Thread Ren Qiaowei
On 01/28/2014 04:27 AM, Andy Lutomirski wrote: On 01/26/2014 01:08 AM, Qiaowei Ren wrote: +1) Providing handlers for bounds faults (#BR). + +When MPX is enabled, there are 2 new situations that can generate +#BR faults. If a bounds overflow occurs then a #BR is generated. +The fault handler

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

2014-01-27 Thread Ren Qiaowei
On 01/28/2014 04:36 AM, Andy Lutomirski wrote: + bd_entry = status & MPX_BNDSTA_ADDR_MASK; + if ((bd_entry >= bd_base) && (bd_entry < bd_base + bd_size)) + allocate_bt(bd_entry); What happens if this fails? Retrying forever isn't very nice. If allocation of the

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

2014-01-27 Thread Ren Qiaowei
On 01/28/2014 05:58 AM, Andy Lutomirski wrote: On 01/26/2014 01:08 AM, Qiaowei Ren wrote: (Why on earth does Intel not expose this stuff in cr2 or an MSR or something?) I guess it is due to some design reason. Thanks, Qiaowei -- To unsubscribe from this list: send the line "unsubscribe

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

2014-01-27 Thread Ren Qiaowei
On 01/28/2014 05:58 AM, Andy Lutomirski wrote: On 01/26/2014 01:08 AM, Qiaowei Ren wrote: (Why on earth does Intel not expose this stuff in cr2 or an MSR or something?) I guess it is due to some design reason. Thanks, Qiaowei -- To unsubscribe from this list: send the line unsubscribe

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

2014-01-27 Thread Ren Qiaowei
On 01/28/2014 04:36 AM, Andy Lutomirski wrote: + bd_entry = status MPX_BNDSTA_ADDR_MASK; + if ((bd_entry = bd_base) (bd_entry bd_base + bd_size)) + allocate_bt(bd_entry); What happens if this fails? Retrying forever isn't very nice. If allocation of the bound

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

2014-01-27 Thread Ren Qiaowei
On 01/28/2014 04:27 AM, Andy Lutomirski wrote: On 01/26/2014 01:08 AM, Qiaowei Ren wrote: +1) Providing handlers for bounds faults (#BR). + +When MPX is enabled, there are 2 new situations that can generate +#BR faults. If a bounds overflow occurs then a #BR is generated. +The fault handler

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

2014-01-27 Thread Ren Qiaowei
On 01/28/2014 01:21 PM, Andy Lutomirski wrote: On Mon, Jan 27, 2014 at 7:35 PM, Ren Qiaowei qiaowei@intel.com wrote: On 01/28/2014 04:36 AM, Andy Lutomirski wrote: + bd_entry = status MPX_BNDSTA_ADDR_MASK; + if ((bd_entry = bd_base) (bd_entry bd_base + bd_size

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

2014-01-27 Thread Ren Qiaowei
On 01/28/2014 02:42 PM, Andy Lutomirski wrote: I just read it. do_trap_no_signal presumably calls fixup_exception because #UD uses it and #UD needs that handling. (I'm guessing that there is actually a legitimate use for a kernel fixup on #UD somewhere -- there's probably something that isn't

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

2014-01-27 Thread Ren Qiaowei
On 01/28/2014 02:42 PM, Ingo Molnar wrote: * Ren Qiaowei qiaowei@intel.com wrote: MPX kernel code, namely this patchset, has mainly the 2 responsibilities: provide handlers for bounds faults (#BR), and manage bounds memory. AFAICS the kernel side implementation causes no runtime

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

2014-01-26 Thread Ren Qiaowei
On 01/27/2014 10:10 AM, H. Peter Anvin wrote: On 01/26/2014 05:55 PM, Ren Qiaowei wrote: Peter, you mean we should remove these two call and do what they do in user-space, right? Unless we think there is a benefit to the kernel to have a on/off switch for the #BR exception (if disabled, all

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

2014-01-26 Thread Ren Qiaowei
On 01/26/2014 11:14 PM, Ingo Molnar wrote: * Ren, Qiaowei wrote: The size of one bound table is 4M bytes for 64bit, and 16K bytes for 32bit. It can not be accessed by user-space, and it will be accessed automatically by hardware. So, here's the bound-table allocation AFAICS: +static bool

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

2014-01-26 Thread Ren Qiaowei
On 01/27/2014 09:53 AM, H. Peter Anvin wrote: On 01/26/2014 05:34 PM, Ren Qiaowei wrote: I agree with you and we should suppress all the warnings as possible. If I use (unsgined long) to cast like the following code, what do you think about it? sizeof(long) will be 4 for 32-bit. info

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

2014-01-26 Thread Ren Qiaowei
On 01/27/2014 09:50 AM, H. Peter Anvin wrote: On 01/26/2014 12:39 AM, Ingo Molnar wrote: It will be only once per startup. In that case it would be more efficient to make this part of the binary execution environment so that exec() sets it up automatically, not a separate prctl() syscall.

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

2014-01-26 Thread Ren Qiaowei
On 01/27/2014 05:38 AM, David Rientjes wrote: On Sun, 26 Jan 2014, Ren Qiaowei wrote: 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

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

2014-01-26 Thread Ren, Qiaowei
> -Original Message- > From: Ingo Molnar [mailto:mingo.kernel@gmail.com] On Behalf Of Ingo > Molnar > Sent: Sunday, January 26, 2014 5:08 PM > To: Ren, Qiaowei > Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; x...@kernel.org; > linux-kernel@vger.kernel.org;

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

2014-01-26 Thread Ren, Qiaowei
> -Original Message- > From: linux-kernel-ow...@vger.kernel.org > [mailto:linux-kernel-ow...@vger.kernel.org] On Behalf Of Ingo Molnar > Sent: Sunday, January 26, 2014 4:40 PM > To: Ren, Qiaowei > Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; x...@kernel.o

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

2014-01-26 Thread Ren Qiaowei
On 01/26/2014 04:22 PM, Ingo Molnar wrote: * Qiaowei Ren wrote: 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

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

2014-01-26 Thread Ren Qiaowei
On 01/26/2014 04:19 PM, Ingo Molnar wrote: * Qiaowei Ren wrote: This patchset adds support for the Memory Protection Extensions (MPX) feature found in future Intel processors. MPX can be used in conjunction with compiler changes to check memory references, for those references whose

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

2014-01-26 Thread Ren Qiaowei
On 01/26/2014 04:19 PM, Ingo Molnar wrote: * Qiaowei Ren qiaowei@intel.com wrote: This patchset adds support for the Memory Protection Extensions (MPX) feature found in future Intel processors. MPX can be used in conjunction with compiler changes to check memory references, for those

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

2014-01-26 Thread Ren Qiaowei
On 01/26/2014 04:22 PM, Ingo Molnar wrote: * Qiaowei Ren qiaowei@intel.com wrote: 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

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

2014-01-26 Thread Ren, Qiaowei
-Original Message- From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel-ow...@vger.kernel.org] On Behalf Of Ingo Molnar Sent: Sunday, January 26, 2014 4:40 PM To: Ren, Qiaowei Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; x...@kernel.org; linux-kernel@vger.kernel.org

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

2014-01-26 Thread Ren, Qiaowei
-Original Message- From: Ingo Molnar [mailto:mingo.kernel@gmail.com] On Behalf Of Ingo Molnar Sent: Sunday, January 26, 2014 5:08 PM To: Ren, Qiaowei Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; x...@kernel.org; linux-kernel@vger.kernel.org; Peter Zijlstra; Linus Torvalds

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

2014-01-26 Thread Ren Qiaowei
On 01/27/2014 05:38 AM, David Rientjes wrote: On Sun, 26 Jan 2014, Ren Qiaowei wrote: 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

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

2014-01-26 Thread Ren Qiaowei
On 01/27/2014 09:50 AM, H. Peter Anvin wrote: On 01/26/2014 12:39 AM, Ingo Molnar wrote: It will be only once per startup. In that case it would be more efficient to make this part of the binary execution environment so that exec() sets it up automatically, not a separate prctl() syscall.

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

2014-01-26 Thread Ren Qiaowei
On 01/27/2014 09:53 AM, H. Peter Anvin wrote: On 01/26/2014 05:34 PM, Ren Qiaowei wrote: I agree with you and we should suppress all the warnings as possible. If I use (unsgined long) to cast like the following code, what do you think about it? sizeof(long) will be 4 for 32-bit. info

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

2014-01-26 Thread Ren Qiaowei
On 01/26/2014 11:14 PM, Ingo Molnar wrote: * Ren, Qiaowei qiaowei@intel.com wrote: The size of one bound table is 4M bytes for 64bit, and 16K bytes for 32bit. It can not be accessed by user-space, and it will be accessed automatically by hardware. So, here's the bound-table allocation

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

2014-01-26 Thread Ren Qiaowei
On 01/27/2014 10:10 AM, H. Peter Anvin wrote: On 01/26/2014 05:55 PM, Ren Qiaowei wrote: Peter, you mean we should remove these two call and do what they do in user-space, right? Unless we think there is a benefit to the kernel to have a on/off switch for the #BR exception (if disabled, all

  1   2   >