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

2014-09-18 Thread Dave Hansen
On 09/18/2014 12:17 AM, Kevin Easton wrote: > I was assuming that if an application did want to enable MPX after threads > had already been created, it would generally want to enable it > simultaneously across all threads. This would be a lot easier for the > kernel than for the application. The

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

2014-09-18 Thread Kevin Easton
On Wed, Sep 17, 2014 at 09:43:09PM -0700, Dave Hansen wrote: > On 09/17/2014 08:23 PM, Kevin Easton wrote: > > I was actually thinking that the kernel would take care of the xsave / > > xrstor (for current), updating tsk->thread.fpu.state (for non-running > > threads) and sending an IPI for

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

2014-09-18 Thread Kevin Easton
On Wed, Sep 17, 2014 at 09:43:09PM -0700, Dave Hansen wrote: On 09/17/2014 08:23 PM, Kevin Easton wrote: I was actually thinking that the kernel would take care of the xsave / xrstor (for current), updating tsk-thread.fpu.state (for non-running threads) and sending an IPI for threads

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

2014-09-18 Thread Dave Hansen
On 09/18/2014 12:17 AM, Kevin Easton wrote: I was assuming that if an application did want to enable MPX after threads had already been created, it would generally want to enable it simultaneously across all threads. This would be a lot easier for the kernel than for the application. The

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

2014-09-17 Thread Dave Hansen
On 09/17/2014 08:23 PM, Kevin Easton wrote: > I was actually thinking that the kernel would take care of the xsave / > xrstor (for current), updating tsk->thread.fpu.state (for non-running > threads) and sending an IPI for threads running on other CPUs. > > Of course userspace can always then

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

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

2014-09-17 Thread Kevin Easton
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 not too important as long as it's

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 Kevin Easton
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 not too important as long as it's

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 not

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

2014-09-17 Thread Dave Hansen
On 09/17/2014 08:23 PM, Kevin Easton wrote: I was actually thinking that the kernel would take care of the xsave / xrstor (for current), updating tsk-thread.fpu.state (for non-running threads) and sending an IPI for threads running on other CPUs. Of course userspace can always then manually

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

2014-09-16 Thread Kevin Easton
On Thu, Sep 11, 2014 at 04:46:48PM +0800, Qiaowei Ren wrote: > +static __user void *task_get_bounds_dir(struct task_struct *tsk) > +{ > + struct xsave_struct *xsave_buf; > + > + fpu_xsave(>thread.fpu); > + xsave_buf = &(tsk->thread.fpu.state->xsave); > + if

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

2014-09-16 Thread Kevin Easton
On Thu, Sep 11, 2014 at 04:46:48PM +0800, Qiaowei Ren wrote: +static __user void *task_get_bounds_dir(struct task_struct *tsk) +{ + struct xsave_struct *xsave_buf; + + fpu_xsave(tsk-thread.fpu); + xsave_buf = (tsk-thread.fpu.state-xsave); + if (!(xsave_buf-bndcsr.cfg_reg_u

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

2014-09-15 Thread Dave Hansen
On 09/15/2014 08:20 PM, Ren, Qiaowei wrote: >> What are the semantics across execve() ? >> > This will not impact on the semantics of execve(). One runtime > library > for MPX will be provided (or merged into Glibc), and when the > application starts, this runtime will be called to initialize MPX

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 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER

2014-09-15 Thread Dave Hansen
On 09/15/2014 08:20 PM, Ren, Qiaowei wrote: What are the semantics across execve() ? This will not impact on the semantics of execve(). One runtime library for MPX will be provided (or merged into Glibc), and when the application starts, this runtime will be called to initialize MPX

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

2014-09-14 Thread One Thousand Gnomes
> 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 decided to unregister it. > +int

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

2014-09-14 Thread One Thousand Gnomes
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 decided to unregister it. +int

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

2014-09-13 Thread Thomas Gleixner
On Fri, 12 Sep 2014, Dave Hansen wrote: > On 09/12/2014 10:34 AM, Thomas Gleixner wrote: > > On Fri, 12 Sep 2014, Dave Hansen wrote: > >> There are two mappings in play: > >> 1. The mapping with the actual data, which userspace is munmap()ing or > >>brk()ing away, etc... (never tagged VM_MPX)

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

2014-09-13 Thread Thomas Gleixner
On Fri, 12 Sep 2014, Dave Hansen wrote: On 09/12/2014 10:34 AM, Thomas Gleixner wrote: On Fri, 12 Sep 2014, Dave Hansen wrote: There are two mappings in play: 1. The mapping with the actual data, which userspace is munmap()ing or brk()ing away, etc... (never tagged VM_MPX) It's

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

2014-09-12 Thread Dave Hansen
On 09/12/2014 11:42 AM, Thomas Gleixner wrote: > On Fri, 12 Sep 2014, Thomas Gleixner wrote: >> On Fri, 12 Sep 2014, Dave Hansen wrote: >> The proper solution to this problem is: >> >> do_bounds() >> bd_addr = get_bd_addr_from_xsave(); >> bd_entry = bndstatus & ADDR_MASK: > > Just

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

2014-09-12 Thread Dave Hansen
On 09/12/2014 10:42 AM, Thomas Gleixner wrote: > On Fri, 12 Sep 2014, Dave Hansen wrote: >> The prctl(register) is meant to be a signal from userspace to the kernel >> to say, "I would like your help in managing these bounds tables". >> prctl(unregister) is the opposite, meaning "I don't want your

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

2014-09-12 Thread Dave Hansen
On 09/12/2014 10:34 AM, Thomas Gleixner wrote: > On Fri, 12 Sep 2014, Dave Hansen wrote: >> There are two mappings in play: >> 1. The mapping with the actual data, which userspace is munmap()ing or >>brk()ing away, etc... (never tagged VM_MPX) > > It's not tagged that way because it is mapped

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

2014-09-12 Thread Thomas Gleixner
On Fri, 12 Sep 2014, Thomas Gleixner wrote: > On Fri, 12 Sep 2014, Dave Hansen wrote: > The proper solution to this problem is: > > do_bounds() > bd_addr = get_bd_addr_from_xsave(); > bd_entry = bndstatus & ADDR_MASK: Just for clarification. You CANNOT avoid the xsave here

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

2014-09-12 Thread Thomas Gleixner
On Fri, 12 Sep 2014, Dave Hansen wrote: > On 09/12/2014 01:11 AM, Thomas Gleixner wrote: > > So what you are saying is, that if user space sets the pointer to NULL > > via the unregister prctl, kernel can safely ignore vmas which have the > > VM_MPX flag set. I really can't follow that logic. > >

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

2014-09-12 Thread Thomas Gleixner
On Fri, 12 Sep 2014, Dave Hansen wrote: > On 09/12/2014 02:24 AM, Thomas Gleixner wrote: > > On Fri, 12 Sep 2014, Thomas Gleixner wrote: > >> On Thu, 11 Sep 2014, Dave Hansen wrote: > >>> Well, we use it to figure out whether we _potentially_ need to tear down > >>> an VM_MPX-flagged area.

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

2014-09-12 Thread Dave Hansen
On 09/12/2014 01:11 AM, Thomas Gleixner wrote: > So what you are saying is, that if user space sets the pointer to NULL > via the unregister prctl, kernel can safely ignore vmas which have the > VM_MPX flag set. I really can't follow that logic. > > mmap_mpx(); > prctl(enable mpx); >

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

2014-09-12 Thread Dave Hansen
On 09/12/2014 02:24 AM, Thomas Gleixner wrote: > On Fri, 12 Sep 2014, Thomas Gleixner wrote: >> On Thu, 11 Sep 2014, Dave Hansen wrote: >>> Well, we use it to figure out whether we _potentially_ need to tear down >>> an VM_MPX-flagged area. There's no guarantee that there will be one. >> >> So

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

2014-09-12 Thread Thomas Gleixner
On Fri, 12 Sep 2014, Thomas Gleixner wrote: > On Thu, 11 Sep 2014, Dave Hansen wrote: > > Well, we use it to figure out whether we _potentially_ need to tear down > > an VM_MPX-flagged area. There's no guarantee that there will be one. > > So what you are saying is, that if user space sets the

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

2014-09-12 Thread Thomas Gleixner
On Thu, 11 Sep 2014, Dave Hansen wrote: > On 09/11/2014 04:28 PM, Thomas Gleixner wrote: > > On Thu, 11 Sep 2014, Qiaowei Ren wrote: > >> 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

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

2014-09-12 Thread Thomas Gleixner
On Thu, 11 Sep 2014, Dave Hansen wrote: On 09/11/2014 04:28 PM, Thomas Gleixner wrote: On Thu, 11 Sep 2014, Qiaowei Ren wrote: 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

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

2014-09-12 Thread Thomas Gleixner
On Fri, 12 Sep 2014, Thomas Gleixner wrote: On Thu, 11 Sep 2014, Dave Hansen wrote: Well, we use it to figure out whether we _potentially_ need to tear down an VM_MPX-flagged area. There's no guarantee that there will be one. So what you are saying is, that if user space sets the pointer

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

2014-09-12 Thread Dave Hansen
On 09/12/2014 02:24 AM, Thomas Gleixner wrote: On Fri, 12 Sep 2014, Thomas Gleixner wrote: On Thu, 11 Sep 2014, Dave Hansen wrote: Well, we use it to figure out whether we _potentially_ need to tear down an VM_MPX-flagged area. There's no guarantee that there will be one. So what you are

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

2014-09-12 Thread Dave Hansen
On 09/12/2014 01:11 AM, Thomas Gleixner wrote: So what you are saying is, that if user space sets the pointer to NULL via the unregister prctl, kernel can safely ignore vmas which have the VM_MPX flag set. I really can't follow that logic. mmap_mpx(); prctl(enable mpx);

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

2014-09-12 Thread Thomas Gleixner
On Fri, 12 Sep 2014, Dave Hansen wrote: On 09/12/2014 02:24 AM, Thomas Gleixner wrote: On Fri, 12 Sep 2014, Thomas Gleixner wrote: On Thu, 11 Sep 2014, Dave Hansen wrote: Well, we use it to figure out whether we _potentially_ need to tear down an VM_MPX-flagged area. There's no guarantee

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

2014-09-12 Thread Thomas Gleixner
On Fri, 12 Sep 2014, Dave Hansen wrote: On 09/12/2014 01:11 AM, Thomas Gleixner wrote: So what you are saying is, that if user space sets the pointer to NULL via the unregister prctl, kernel can safely ignore vmas which have the VM_MPX flag set. I really can't follow that logic.

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

2014-09-12 Thread Thomas Gleixner
On Fri, 12 Sep 2014, Thomas Gleixner wrote: On Fri, 12 Sep 2014, Dave Hansen wrote: The proper solution to this problem is: do_bounds() bd_addr = get_bd_addr_from_xsave(); bd_entry = bndstatus ADDR_MASK: Just for clarification. You CANNOT avoid the xsave here because it's

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

2014-09-12 Thread Dave Hansen
On 09/12/2014 10:34 AM, Thomas Gleixner wrote: On Fri, 12 Sep 2014, Dave Hansen wrote: There are two mappings in play: 1. The mapping with the actual data, which userspace is munmap()ing or brk()ing away, etc... (never tagged VM_MPX) It's not tagged that way because it is mapped by user

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

2014-09-12 Thread Dave Hansen
On 09/12/2014 10:42 AM, Thomas Gleixner wrote: On Fri, 12 Sep 2014, Dave Hansen wrote: The prctl(register) is meant to be a signal from userspace to the kernel to say, I would like your help in managing these bounds tables. prctl(unregister) is the opposite, meaning I don't want your help any

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

2014-09-12 Thread Dave Hansen
On 09/12/2014 11:42 AM, Thomas Gleixner wrote: On Fri, 12 Sep 2014, Thomas Gleixner wrote: On Fri, 12 Sep 2014, Dave Hansen wrote: The proper solution to this problem is: do_bounds() bd_addr = get_bd_addr_from_xsave(); bd_entry = bndstatus ADDR_MASK: Just for

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 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER

2014-09-11 Thread Dave Hansen
On 09/11/2014 04:28 PM, Thomas Gleixner wrote: > On Thu, 11 Sep 2014, Qiaowei Ren wrote: >> 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. > > I cant see anything

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

2014-09-11 Thread Thomas Gleixner
On Thu, 11 Sep 2014, Qiaowei Ren wrote: > 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. I cant see anything which is registered/unregistered. > The base of the

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

2014-09-11 Thread Dave Hansen
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 '(unsigned long)' out. -- To

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

2014-09-11 Thread Thomas Gleixner
On Thu, 11 Sep 2014, Qiaowei Ren wrote: 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. I cant see anything which is registered/unregistered. The base of the bounds

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

2014-09-11 Thread Dave Hansen
On 09/11/2014 04:28 PM, Thomas Gleixner wrote: On Thu, 11 Sep 2014, Qiaowei Ren wrote: 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. I cant see anything which is

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 v8 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER

2014-09-11 Thread Dave Hansen
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 '(unsigned long)' out. -- To unsubscribe from