[MODERATED] Re: [patch V9 00/16] SSB

[Jon Masters <jcm@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 7931 bytes --]

Sorry for lag yesterday - CFL box I was using in the office randomly
ceased functioning so I swore loudly then ran out and bought a Kaby Lake
box for which I have ucode (still waiting on SKL ucode for normal box).
I've got the v9 patches running quite nicely on this new box and will do
a bunch more testing over the next few days. Will test v10 tomorrow but
it's a bit academic given the changes. In addition to RHEL patches, I'll
see if we can throw the speck-v10 into a 4.17-rc3 test and run it
through a bunch of the usual tests we do internally to shake out.

On userspace. So far we're still mostly worried about Java though our QE
folks are working through lists of packages that have execmem and other
privs looking for additional JITs and sandboxes to prctl.

Jon.

On 05/01/2018 11:24 AM, speck for Thomas Gleixner wrote:
> Changes vs. V8:
> 
> Addressed review feed back and added Linus variant of
> alternative_write_msr().
> 
>  - Use EINVAL in the prctl
> 
>  - Fix the sbb typos
> 
>  - Make AMD default to prctl as well (Borislav)
> 
>  - Add the not affected CPU types (Tim)
> 
> I've updated the git repository as well
> 
>   ssh://git@cvs.ou.linutronix.de/linux/speck/linux master (forced update)
> 
> There are also two stable backport branches now:
> 
>   ssh://git@cvs.ou.linutronix.de/linux/speck/linux linux-4.16.y
> 
>   ssh://git@cvs.ou.linutronix.de/linux/speck/linux linux-4.14.y
> 
> 
> Delta patch against V8 below. Git bundle against 4.17-rc3 follows in
> separate mail.
> 
> Thanks,
> 
> 	tglx
> 
> 8<-------------------
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index de1ca7510697..29984fd3dd18 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4052,10 +4052,10 @@
>  			on     - Unconditionally disable Speculative Store Bypass
>  			off    - Unconditionally enable Speculative Store Bypass
>  			auto   - Kernel detects whether the CPU model contains a
> -			         vulnerable implementation of Speculative Store
> -			         Bypass and picks the most appropriate mitigation
> +				 implementation of Speculative Store Bypass and
> +				 picks the most appropriate mitigation
>  			prctl  - Control Speculative Store Bypass for a thread
> -			         via prctl. By default it is enabled. The state
> +				 via prctl. By default it is enabled. The state
>  				 is inherited on fork.
>  
>  			Not specifying this option is equivalent to
> diff --git a/Documentation/userspace-api/spec_ctrl.rst b/Documentation/userspace-api/spec_ctrl.rst
> index 867b8b435baa..8ff39a26a992 100644
> --- a/Documentation/userspace-api/spec_ctrl.rst
> +++ b/Documentation/userspace-api/spec_ctrl.rst
> @@ -56,11 +56,10 @@ Common error codes
>  ======= =================================================================
>  Value   Meaning
>  ======= =================================================================
> -EINVAL  The prctl is not implemented by the architecture
> +EINVAL  The prctl is not implemented by the architecture or unused
> +        prctl(2) arguments are not 0
>  
>  ENODEV  arg2 is selecting a not supported speculation misfeature
> -
> -EUCLEAN The unused prctl() arguments are not 0
>  ======= =================================================================
>  
>  PR_SET_SPECULATION_CTRL error codes
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index 7a1be0b2c264..023e2edc0f3c 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -259,22 +259,22 @@ static inline void vmexit_fill_RSB(void)
>  #endif
>  }
>  
> -#define alternative_msr_write(_msr, _val, _feature)		\
> -	asm volatile(ALTERNATIVE("",				\
> -				 "movl %[msr], %%ecx\n\t"	\
> -				 "movl %[val], %%eax\n\t"	\
> -				 "movl $0, %%edx\n\t"		\
> -				 "wrmsr",			\
> -				 _feature)			\
> -		     : : [msr] "i" (_msr), [val] "m" (_val)	\
> -		     : "eax", "ecx", "edx", "memory")
> +static __always_inline
> +void alternative_msr_write(unsigned int msr, u64 val, unsigned int feature)
> +{
> +	asm volatile(ALTERNATIVE("", "wrmsr", %c[feature])
> +		: : "c" (msr),
> +		    "a" (val),
> +		    "d" (val >> 32),
> +		    [feature] "i" (feature)
> +		: "memory");
> +}
>  
>  static inline void indirect_branch_prediction_barrier(void)
>  {
>  	u64 val = PRED_CMD_IBPB;
>  
> -	alternative_msr_write(MSR_IA32_PRED_CMD, val,
> -			      X86_FEATURE_USE_IBPB);
> +	alternative_msr_write(MSR_IA32_PRED_CMD, val, X86_FEATURE_USE_IBPB);
>  }
>  
>  /*
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index d52ccd0ce507..c28856e475c8 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -476,11 +476,6 @@ static enum ssb_mitigation_cmd __init __ssb_select_mitigation(void)
>  
>  	switch (cmd) {
>  	case SPEC_STORE_BYPASS_CMD_AUTO:
> -		/*
> -		 * AMD platforms by default don't need SSB mitigation.
> -		 */
> -		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
> -			break;
>  		/* Choose prctl as the default mode */
>  		mode = SPEC_STORE_BYPASS_PRCTL;
>  		break;
> @@ -531,7 +526,7 @@ static void ssb_select_mitigation()
>  
>  #undef pr_fmt
>  
> -static int sbb_prctl_set(unsigned long ctrl)
> +static int ssb_prctl_set(unsigned long ctrl)
>  {
>  	bool rds = !!test_tsk_thread_flag(current, TIF_RDS);
>  
> @@ -549,7 +544,7 @@ static int sbb_prctl_set(unsigned long ctrl)
>  	return 0;
>  }
>  
> -static int sbb_prctl_get(void)
> +static int ssb_prctl_get(void)
>  {
>  	switch (ssb_mode) {
>  	case SPEC_STORE_BYPASS_DISABLE:
> @@ -572,7 +567,7 @@ int arch_prctl_set_spec_ctrl(unsigned long which, unsigned long ctrl)
>  
>  	switch (which) {
>  	case PR_SPEC_STORE_BYPASS:
> -		return sbb_prctl_set(ctrl);
> +		return ssb_prctl_set(ctrl);
>  	default:
>  		return -ENODEV;
>  	}
> @@ -582,7 +577,7 @@ int arch_prctl_get_spec_ctrl(unsigned long which)
>  {
>  	switch (which) {
>  	case PR_SPEC_STORE_BYPASS:
> -		return sbb_prctl_get();
> +		return ssb_prctl_get();
>  	default:
>  		return -ENODEV;
>  	}
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index c3b53bc30a29..6101e3dba2f6 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -931,6 +931,9 @@ static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = {
>  	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_CEDARVIEW },
>  	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_SILVERMONT1 },
>  	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_AIRMONT },
> +	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_SILVERMONT2 },
> +	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_MERRIFIELD },
> +	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_CORE_YONAH },
>  	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_XEON_PHI_KNL },
>  	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_XEON_PHI_KNM },
>  	{ X86_VENDOR_CENTAUR,	5 },
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 8bb8051bd9a1..d7afe29319f1 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -2464,15 +2464,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
>  		break;
>  	case PR_SET_SPECULATION_CTRL:
>  		if (arg4 || arg5)
> -			error = -EUCLEAN;
> -		else
> -			error = arch_prctl_set_spec_ctrl(arg2, arg3);
> +			return -EINVAL;
> +		error = arch_prctl_set_spec_ctrl(arg2, arg3);
>  		break;
>  	case PR_GET_SPECULATION_CTRL:
>  		if (arg3 || arg4 || arg5)
> -			error = -EUCLEAN;
> -		else
> -			error = arch_prctl_get_spec_ctrl(arg2);
> +			return -EINVAL;
> +		error = arch_prctl_get_spec_ctrl(arg2);
>  		break;
>  	default:
>  		error = -EINVAL;
> 


-- 
Computer Architect | Sent from my Fedora powered laptop