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

[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;
> +	unsigned int enable;
> +
> +	len = min(count, sizeof(buf) - 1);
> +	if (copy_from_user(buf, user_buf, len))
> +		return -EFAULT;
> +
> +	buf[len] = '\0';
> +	if (kstrtouint(buf, 0, &enable))
> +		return -EINVAL;
> +
> +	if (enable > IBRS_MAX)
> +		return -EINVAL;
> +
> +	if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL)) {
> +		ibrs_enabled = IBRS_DISABLED;
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&spec_ctrl_mutex);
> +
> +	if (enable == IBRS_DISABLED) {
> +		/* disable IBRS usage */
> +		ibrs_admin_disabled = true;
> +		dynamic_ibrs = 0;
> +		spec_ctrl_flush_all_cpus(MSR_IA32_SPEC_CTRL,
> +					 SPEC_CTRL_FEATURE_DISABLE_IBRS);
> +
> +	} else if (enable == IBRS_ENABLED) {
> +		/* enable IBRS usage in kernel */
> +		ibrs_admin_disabled = false;
> +		dynamic_ibrs = 1;

I think you need to do:

		spec_ctrl_flush_all_cpus(MSR_IA32_SPEC_CTRL,
					 SPEC_CTRL_FEATURE_ENABLE_IBRS);

here as well, to force all CPUs into a known state.

> +
> +	} else if (enable == IBRS_ENABLED_USER) {
> +		/* enable IBRS all the time in both userspace and kernel */
> +		ibrs_admin_disabled = false;
> +		dynamic_ibrs = 0;
> +		spec_ctrl_flush_all_cpus(MSR_IA32_SPEC_CTRL,
> +					 SPEC_CTRL_FEATURE_ENABLE_IBRS);
> +	}
> +
> +	ibrs_enabled = enable;
> +
> +	mutex_unlock(&spec_ctrl_mutex);
> +	return count;
> +}