Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-27 Thread David Woodhouse
On Sat, 2018-01-27 at 08:59 -0500, Konrad Rzeszutek Wilk wrote: > On Mon, Jan 08, 2018 at 05:14:37PM +0100, Peter Zijlstra wrote: > > > > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > > > > > > > > > > > a good suggestion, but we encountered some issues with it either > > >

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-27 Thread David Woodhouse
On Sat, 2018-01-27 at 08:59 -0500, Konrad Rzeszutek Wilk wrote: > On Mon, Jan 08, 2018 at 05:14:37PM +0100, Peter Zijlstra wrote: > > > > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > > > > > > > > > > > a good suggestion, but we encountered some issues with it either > > >

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-27 Thread Konrad Rzeszutek Wilk
On Mon, Jan 08, 2018 at 05:14:37PM +0100, Peter Zijlstra wrote: > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > > > a good suggestion, but we encountered some issues with it either > > > crashing the kernel at boot or not properly turning on/off. > > The below boots, but I

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-27 Thread Konrad Rzeszutek Wilk
On Mon, Jan 08, 2018 at 05:14:37PM +0100, Peter Zijlstra wrote: > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > > > a good suggestion, but we encountered some issues with it either > > > crashing the kernel at boot or not properly turning on/off. > > The below boots, but I

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-09 Thread Tim Chen
On 01/09/2018 10:13 AM, David Woodhouse wrote: > On Tue, 2018-01-09 at 09:55 -0800, Tim Chen wrote: >> >> Thomas, >> >> I'll be sending an updated patchset with boot option opt in for ibrs >> and leave the control varaible out. I agree that we can worry about the >> control variable later. > >

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-09 Thread Tim Chen
On 01/09/2018 10:13 AM, David Woodhouse wrote: > On Tue, 2018-01-09 at 09:55 -0800, Tim Chen wrote: >> >> Thomas, >> >> I'll be sending an updated patchset with boot option opt in for ibrs >> and leave the control varaible out. I agree that we can worry about the >> control variable later. > >

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-09 Thread David Woodhouse
On Tue, 2018-01-09 at 09:55 -0800, Tim Chen wrote: > > Thomas, > > I'll be sending an updated patchset with boot option opt in for ibrs > and leave the control varaible out.  I agree that we can worry about the > control variable later. Please base this on the spectre_v2= option that's already

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-09 Thread David Woodhouse
On Tue, 2018-01-09 at 09:55 -0800, Tim Chen wrote: > > Thomas, > > I'll be sending an updated patchset with boot option opt in for ibrs > and leave the control varaible out.  I agree that we can worry about the > control variable later. Please base this on the spectre_v2= option that's already

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-09 Thread Tim Chen
On 01/08/2018 04:29 PM, Borislav Petkov wrote: > On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: >> From: Tim Chen >> From: Andrea Arcangeli > > There's Co-Developed-by for this: > > - Co-Developed-by: states that the patch was also

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-09 Thread Tim Chen
On 01/08/2018 04:29 PM, Borislav Petkov wrote: > On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: >> From: Tim Chen >> From: Andrea Arcangeli > > There's Co-Developed-by for this: > > - Co-Developed-by: states that the patch was also created by another > developer >along with

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-09 Thread Tim Chen
On 01/09/2018 02:40 AM, Thomas Gleixner wrote: > On Mon, 8 Jan 2018, Peter Zijlstra wrote: >> On Mon, Jan 08, 2018 at 09:28:12AM -0800, Tim Chen wrote: >>> On 01/08/2018 08:14 AM, Peter Zijlstra wrote: On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: >> a good suggestion,

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-09 Thread Tim Chen
On 01/09/2018 02:40 AM, Thomas Gleixner wrote: > On Mon, 8 Jan 2018, Peter Zijlstra wrote: >> On Mon, Jan 08, 2018 at 09:28:12AM -0800, Tim Chen wrote: >>> On 01/08/2018 08:14 AM, Peter Zijlstra wrote: On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: >> a good suggestion,

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-09 Thread Thomas Gleixner
On Mon, 8 Jan 2018, Peter Zijlstra wrote: > On Mon, Jan 08, 2018 at 09:28:12AM -0800, Tim Chen wrote: > > On 01/08/2018 08:14 AM, Peter Zijlstra wrote: > > > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > > >>> a good suggestion, but we encountered some issues with it either > >

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-09 Thread Thomas Gleixner
On Mon, 8 Jan 2018, Peter Zijlstra wrote: > On Mon, Jan 08, 2018 at 09:28:12AM -0800, Tim Chen wrote: > > On 01/08/2018 08:14 AM, Peter Zijlstra wrote: > > > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > > >>> a good suggestion, but we encountered some issues with it either > >

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Borislav Petkov
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > From: Tim Chen > From: Andrea Arcangeli There's Co-Developed-by for this: - Co-Developed-by: states that the patch was also created by another developer along with the original

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Borislav Petkov
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > From: Tim Chen > From: Andrea Arcangeli There's Co-Developed-by for this: - Co-Developed-by: states that the patch was also created by another developer along with the original author. This is useful at times when multiple

RE: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Lu, Hongjiu
> > On Mon, 2018-01-08 at 18:42 +0100, Peter Zijlstra wrote: > > On Mon, Jan 08, 2018 at 09:28:12AM -0800, Tim Chen wrote: > > > On 01/08/2018 08:14 AM, Peter Zijlstra wrote: > > > > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > > > >>> a good suggestion, but we encountered

RE: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Lu, Hongjiu
> > On Mon, 2018-01-08 at 18:42 +0100, Peter Zijlstra wrote: > > On Mon, Jan 08, 2018 at 09:28:12AM -0800, Tim Chen wrote: > > > On 01/08/2018 08:14 AM, Peter Zijlstra wrote: > > > > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > > > >>> a good suggestion, but we encountered

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Woodhouse, David
On Mon, 2018-01-08 at 18:42 +0100, Peter Zijlstra wrote: > On Mon, Jan 08, 2018 at 09:28:12AM -0800, Tim Chen wrote: > > On 01/08/2018 08:14 AM, Peter Zijlstra wrote: > > > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > > >>> a good suggestion, but we encountered some issues

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Woodhouse, David
On Mon, 2018-01-08 at 18:42 +0100, Peter Zijlstra wrote: > On Mon, Jan 08, 2018 at 09:28:12AM -0800, Tim Chen wrote: > > On 01/08/2018 08:14 AM, Peter Zijlstra wrote: > > > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > > >>> a good suggestion, but we encountered some issues

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Andrea Arcangeli
On Mon, Jan 08, 2018 at 08:43:40PM +0300, Alexey Dobriyan wrote: > > + len = sprintf(buf, "%d\n", READ_ONCE(*field)); > > READ_ONCE isn't necessary as there is only one read being made. Others might disagree but you shouldn't ever let gcc touch any memory that can change under gcc without

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Andrea Arcangeli
On Mon, Jan 08, 2018 at 08:43:40PM +0300, Alexey Dobriyan wrote: > > + len = sprintf(buf, "%d\n", READ_ONCE(*field)); > > READ_ONCE isn't necessary as there is only one read being made. Others might disagree but you shouldn't ever let gcc touch any memory that can change under gcc without

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Alexey Dobriyan
> + len = sprintf(buf, "%d\n", READ_ONCE(*field)); READ_ONCE isn't necessary as there is only one read being made. > + len = min(count, sizeof(buf) - 1); > + if (copy_from_user(buf, user_buf, len)) > + return -EFAULT; > + > + buf[len] = '\0'; > + if

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Alexey Dobriyan
> + len = sprintf(buf, "%d\n", READ_ONCE(*field)); READ_ONCE isn't necessary as there is only one read being made. > + len = min(count, sizeof(buf) - 1); > + if (copy_from_user(buf, user_buf, len)) > + return -EFAULT; > + > + buf[len] = '\0'; > + if

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Mon, Jan 08, 2018 at 09:28:12AM -0800, Tim Chen wrote: > On 01/08/2018 08:14 AM, Peter Zijlstra wrote: > > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > >>> a good suggestion, but we encountered some issues with it either > >>> crashing the kernel at boot or not properly

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Mon, Jan 08, 2018 at 09:28:12AM -0800, Tim Chen wrote: > On 01/08/2018 08:14 AM, Peter Zijlstra wrote: > > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > >>> a good suggestion, but we encountered some issues with it either > >>> crashing the kernel at boot or not properly

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Tim Chen
On 01/08/2018 08:14 AM, Peter Zijlstra wrote: > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: >>> a good suggestion, but we encountered some issues with it either >>> crashing the kernel at boot or not properly turning on/off. > > The below boots, but I lack stuff to test the

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Tim Chen
On 01/08/2018 08:14 AM, Peter Zijlstra wrote: > On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: >>> a good suggestion, but we encountered some issues with it either >>> crashing the kernel at boot or not properly turning on/off. > > The below boots, but I lack stuff to test the

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Tim Chen
On 01/08/2018 07:29 AM, Van De Ven, Arjan wrote: >>> + if ((!c->cpu_index) && (boot_cpu_has(X86_FEATURE_SPEC_CTRL))) { >> >> We should test X86_BUG_SPECTRE_V1 here too before default enabling this, >> no? > > > we shouldn't default enable this. > Patch 7 disables ibrs if retpoline is in use.

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Tim Chen
On 01/08/2018 07:29 AM, Van De Ven, Arjan wrote: >>> + if ((!c->cpu_index) && (boot_cpu_has(X86_FEATURE_SPEC_CTRL))) { >> >> We should test X86_BUG_SPECTRE_V1 here too before default enabling this, >> no? > > > we shouldn't default enable this. > Patch 7 disables ibrs if retpoline is in use.

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > > a good suggestion, but we encountered some issues with it either > > crashing the kernel at boot or not properly turning on/off. The below boots, but I lack stuff to test the enabling. --- a/arch/x86/include/asm/spec_ctrl.h +++

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote: > > a good suggestion, but we encountered some issues with it either > > crashing the kernel at boot or not properly turning on/off. The below boots, but I lack stuff to test the enabling. --- a/arch/x86/include/asm/spec_ctrl.h +++

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > +static ssize_t ibrs_enabled_write(struct file *file, > + const char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + char buf[32]; > + ssize_t len; > +

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > +static ssize_t ibrs_enabled_write(struct file *file, > + const char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + char buf[32]; > + ssize_t len; > +

RE: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Van De Ven, Arjan
> > + if ((!c->cpu_index) && (boot_cpu_has(X86_FEATURE_SPEC_CTRL))) { > > We should test X86_BUG_SPECTRE_V1 here too before default enabling this, > no? we shouldn't default enable this.

RE: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Van De Ven, Arjan
> > + if ((!c->cpu_index) && (boot_cpu_has(X86_FEATURE_SPEC_CTRL))) { > > We should test X86_BUG_SPECTRE_V1 here too before default enabling this, > no? we shouldn't default enable this.

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > +static void spec_ctrl_flush_all_cpus(u32 msr_nr, u64 val) > +{ > + int cpu; > + > + get_online_cpus(); > + for_each_online_cpu(cpu) > + wrmsrl_on_cpu(cpu, msr_nr, val); > + put_online_cpus(); > +} > + >

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > +static void spec_ctrl_flush_all_cpus(u32 msr_nr, u64 val) > +{ > + int cpu; > + > + get_online_cpus(); > + for_each_online_cpu(cpu) > + wrmsrl_on_cpu(cpu, msr_nr, val); > + put_online_cpus(); > +} > + >

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > +bool ibrs_inuse(void); > +bool ibrs_inuse(void) > +{ > + return ibrs_enabled == IBRS_ENABLED; > +} > +EXPORT_SYMBOL_GPL(ibrs_inuse); Doesn't appear to actually be used anywhere...

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > +bool ibrs_inuse(void); > +bool ibrs_inuse(void) > +{ > + return ibrs_enabled == IBRS_ENABLED; > +} > +EXPORT_SYMBOL_GPL(ibrs_inuse); Doesn't appear to actually be used anywhere...

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > +void scan_spec_ctrl_feature(struct cpuinfo_x86 *c) > +{ > + if ((!c->cpu_index) && (boot_cpu_has(X86_FEATURE_SPEC_CTRL))) { We should test X86_BUG_SPECTRE_V1 here too before default enabling this, no? > + if

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > +void scan_spec_ctrl_feature(struct cpuinfo_x86 *c) > +{ > + if ((!c->cpu_index) && (boot_cpu_has(X86_FEATURE_SPEC_CTRL))) { We should test X86_BUG_SPECTRE_V1 here too before default enabling this, no? > + if

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Fri, Jan 05, 2018 at 07:12:12PM -0800, Dave Hansen wrote: > On 01/05/2018 06:12 PM, Tim Chen wrote: > > .macro ENABLE_IBRS > > - ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL > > + testl $1, dynamic_ibrs > > + jz .Lskip_\@ > > There was an earlier suggestion to use

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-08 Thread Peter Zijlstra
On Fri, Jan 05, 2018 at 07:12:12PM -0800, Dave Hansen wrote: > On 01/05/2018 06:12 PM, Tim Chen wrote: > > .macro ENABLE_IBRS > > - ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL > > + testl $1, dynamic_ibrs > > + jz .Lskip_\@ > > There was an earlier suggestion to use

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-07 Thread Greg KH
On Sat, Jan 06, 2018 at 04:25:19PM -0500, Konrad Rzeszutek Wilk wrote: > On Sat, Jan 06, 2018 at 10:10:59AM -0800, Tim Chen wrote: > > > > > > On 01/06/2018 12:54 AM, Greg KH wrote: > > > On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > > >> From: Tim Chen

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-07 Thread Greg KH
On Sat, Jan 06, 2018 at 04:25:19PM -0500, Konrad Rzeszutek Wilk wrote: > On Sat, Jan 06, 2018 at 10:10:59AM -0800, Tim Chen wrote: > > > > > > On 01/06/2018 12:54 AM, Greg KH wrote: > > > On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > > >> From: Tim Chen > > >> From: Andrea

RE: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Van De Ven, Arjan
> > it's a mistake though... retpoline is what people should be using > > ... and only in super exception cases should IBRS even be considered > > Like on certain Skylake and Broadwell where using the retpoline won't suffice? skylake is a bit more complex but that was discussed in earlier

RE: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Van De Ven, Arjan
> > it's a mistake though... retpoline is what people should be using > > ... and only in super exception cases should IBRS even be considered > > Like on certain Skylake and Broadwell where using the retpoline won't suffice? skylake is a bit more complex but that was discussed in earlier

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Konrad Rzeszutek Wilk
On Sat, Jan 06, 2018 at 09:34:01PM +, Van De Ven, Arjan wrote: > > I agree. But this is what customers are told to inspect to see if they > > are impacted. And if in the future versions this goes away or such - they > > will freak out and cause needless escalations. > > > it's a mistake

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Konrad Rzeszutek Wilk
On Sat, Jan 06, 2018 at 09:34:01PM +, Van De Ven, Arjan wrote: > > I agree. But this is what customers are told to inspect to see if they > > are impacted. And if in the future versions this goes away or such - they > > will freak out and cause needless escalations. > > > it's a mistake

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Thomas Gleixner
On Sat, 6 Jan 2018, Konrad Rzeszutek Wilk wrote: > On Sat, Jan 06, 2018 at 08:47:19PM +0100, Thomas Gleixner wrote: > > On Sat, 6 Jan 2018, Dave Hansen wrote: > > > > > On 01/06/2018 09:41 AM, Van De Ven, Arjan wrote: > > > .macro DISABLE_IBRS > > > -ALTERNATIVE "jmp .Lskip_\@",

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Thomas Gleixner
On Sat, 6 Jan 2018, Konrad Rzeszutek Wilk wrote: > On Sat, Jan 06, 2018 at 08:47:19PM +0100, Thomas Gleixner wrote: > > On Sat, 6 Jan 2018, Dave Hansen wrote: > > > > > On 01/06/2018 09:41 AM, Van De Ven, Arjan wrote: > > > .macro DISABLE_IBRS > > > -ALTERNATIVE "jmp .Lskip_\@",

RE: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Van De Ven, Arjan
> I agree. But this is what customers are told to inspect to see if they > are impacted. And if in the future versions this goes away or such - they > will freak out and cause needless escalations. it's a mistake though... retpoline is what people should be using ... and only in super

RE: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Van De Ven, Arjan
> I agree. But this is what customers are told to inspect to see if they > are impacted. And if in the future versions this goes away or such - they > will freak out and cause needless escalations. it's a mistake though... retpoline is what people should be using ... and only in super

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Konrad Rzeszutek Wilk
On Sat, Jan 06, 2018 at 08:47:19PM +0100, Thomas Gleixner wrote: > On Sat, 6 Jan 2018, Dave Hansen wrote: > > > On 01/06/2018 09:41 AM, Van De Ven, Arjan wrote: > > .macro DISABLE_IBRS > > - ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL > > + testl $1,

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Konrad Rzeszutek Wilk
On Sat, Jan 06, 2018 at 08:47:19PM +0100, Thomas Gleixner wrote: > On Sat, 6 Jan 2018, Dave Hansen wrote: > > > On 01/06/2018 09:41 AM, Van De Ven, Arjan wrote: > > .macro DISABLE_IBRS > > - ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL > > + testl $1,

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Konrad Rzeszutek Wilk
On Sat, Jan 06, 2018 at 10:10:59AM -0800, Tim Chen wrote: > > > On 01/06/2018 12:54 AM, Greg KH wrote: > > On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > >> From: Tim Chen > >> From: Andrea Arcangeli > >> > >> There are 2 ways to

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Konrad Rzeszutek Wilk
On Sat, Jan 06, 2018 at 10:10:59AM -0800, Tim Chen wrote: > > > On 01/06/2018 12:54 AM, Greg KH wrote: > > On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > >> From: Tim Chen > >> From: Andrea Arcangeli > >> > >> There are 2 ways to control IBRS > >> > >> 1. At boot time > >>

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Thomas Gleixner
On Sat, 6 Jan 2018, Dave Hansen wrote: > On 01/06/2018 09:41 AM, Van De Ven, Arjan wrote: > .macro DISABLE_IBRS > -ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL > +testl $1, dynamic_ibrs > >>> On every system call we end up hammering on this

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Thomas Gleixner
On Sat, 6 Jan 2018, Dave Hansen wrote: > On 01/06/2018 09:41 AM, Van De Ven, Arjan wrote: > .macro DISABLE_IBRS > -ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL > +testl $1, dynamic_ibrs > >>> On every system call we end up hammering on this

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Dave Hansen
On 01/06/2018 09:41 AM, Van De Ven, Arjan wrote: .macro DISABLE_IBRS - ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL + testl $1, dynamic_ibrs >>> On every system call we end up hammering on this 'dynamic_ibrs' >>> variable. And it looks like it can be flipped via the

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Dave Hansen
On 01/06/2018 09:41 AM, Van De Ven, Arjan wrote: .macro DISABLE_IBRS - ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL + testl $1, dynamic_ibrs >>> On every system call we end up hammering on this 'dynamic_ibrs' >>> variable. And it looks like it can be flipped via the

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Tim Chen
On 01/06/2018 09:33 AM, Dave Hansen wrote: > On 01/06/2018 06:41 AM, Konrad Rzeszutek Wilk wrote: >>> .macro DISABLE_IBRS >>> - ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL >>> + testl $1, dynamic_ibrs >> On every system call we end up hammering on this 'dynamic_ibrs' >>

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Tim Chen
On 01/06/2018 09:33 AM, Dave Hansen wrote: > On 01/06/2018 06:41 AM, Konrad Rzeszutek Wilk wrote: >>> .macro DISABLE_IBRS >>> - ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL >>> + testl $1, dynamic_ibrs >> On every system call we end up hammering on this 'dynamic_ibrs' >>

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Tim Chen
On 01/06/2018 06:41 AM, Konrad Rzeszutek Wilk wrote: > On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: >> From: Tim Chen >> From: Andrea Arcangeli >> > >> >> .macro DISABLE_IBRS >> -ALTERNATIVE "jmp .Lskip_\@", "",

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Tim Chen
On 01/06/2018 06:41 AM, Konrad Rzeszutek Wilk wrote: > On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: >> From: Tim Chen >> From: Andrea Arcangeli >> > >> >> .macro DISABLE_IBRS >> -ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL >> +testl $1, dynamic_ibrs > > On

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Tim Chen
On 01/06/2018 12:54 AM, Greg KH wrote: > On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: >> From: Tim Chen >> From: Andrea Arcangeli >> >> There are 2 ways to control IBRS >> >> 1. At boot time >> noibrs kernel boot parameter will

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Tim Chen
On 01/06/2018 12:54 AM, Greg KH wrote: > On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: >> From: Tim Chen >> From: Andrea Arcangeli >> >> There are 2 ways to control IBRS >> >> 1. At boot time >> noibrs kernel boot parameter will disable IBRS usage >> >> Otherwise if the above

RE: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Van De Ven, Arjan
> >> .macro DISABLE_IBRS > >> - ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL > >> + testl $1, dynamic_ibrs > > On every system call we end up hammering on this 'dynamic_ibrs' > > variable. And it looks like it can be flipped via the IPI mechanism. > > > > Would it make sense for

RE: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Van De Ven, Arjan
> >> .macro DISABLE_IBRS > >> - ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL > >> + testl $1, dynamic_ibrs > > On every system call we end up hammering on this 'dynamic_ibrs' > > variable. And it looks like it can be flipped via the IPI mechanism. > > > > Would it make sense for

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Dave Hansen
On 01/06/2018 06:41 AM, Konrad Rzeszutek Wilk wrote: >> .macro DISABLE_IBRS >> -ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL >> +testl $1, dynamic_ibrs > On every system call we end up hammering on this 'dynamic_ibrs' > variable. And it looks like it can be flipped via the IPI

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Dave Hansen
On 01/06/2018 06:41 AM, Konrad Rzeszutek Wilk wrote: >> .macro DISABLE_IBRS >> -ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL >> +testl $1, dynamic_ibrs > On every system call we end up hammering on this 'dynamic_ibrs' > variable. And it looks like it can be flipped via the IPI

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Konrad Rzeszutek Wilk
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > From: Tim Chen > From: Andrea Arcangeli > > There are 2 ways to control IBRS > > 1. At boot time > noibrs kernel boot parameter will disable IBRS usage > > Otherwise if the above

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Konrad Rzeszutek Wilk
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > From: Tim Chen > From: Andrea Arcangeli > > There are 2 ways to control IBRS > > 1. At boot time > noibrs kernel boot parameter will disable IBRS usage > > Otherwise if the above parameters are not specified, the system > will

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Greg KH
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > From: Tim Chen > From: Andrea Arcangeli > > There are 2 ways to control IBRS > > 1. At boot time > noibrs kernel boot parameter will disable IBRS usage > > Otherwise if the above

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-06 Thread Greg KH
On Fri, Jan 05, 2018 at 06:12:19PM -0800, Tim Chen wrote: > From: Tim Chen > From: Andrea Arcangeli > > There are 2 ways to control IBRS > > 1. At boot time > noibrs kernel boot parameter will disable IBRS usage > > Otherwise if the above parameters are not specified, the system > will

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-05 Thread Dave Hansen
On 01/05/2018 06:12 PM, Tim Chen wrote: > .macro ENABLE_IBRS > - ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL > + testl $1, dynamic_ibrs > + jz .Lskip_\@ There was an earlier suggestion to use STATIC_JUMP_IF_... here. That's a good suggestion, but we encountered

Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

2018-01-05 Thread Dave Hansen
On 01/05/2018 06:12 PM, Tim Chen wrote: > .macro ENABLE_IBRS > - ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL > + testl $1, dynamic_ibrs > + jz .Lskip_\@ There was an earlier suggestion to use STATIC_JUMP_IF_... here. That's a good suggestion, but we encountered