Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 14 Dec 2018 21:50:23 +0100
From:   Borislav Petkov <bp@alien8.de>
To:     "Lendacky, Thomas" <Thomas.Lendacky@amd.com>
Cc:     "x86@kernel.org" <x86@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
        Jiri Kosina <jkosina@suse.cz>, Ingo Molnar <mingo@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        David Woodhouse <dwmw@amazon.co.uk>
Subject: Re: [PATCH v3] x86/speculation: Add support for STIBP always-on
 preferred mode
Message-ID: <20181214205023.GJ11710@zn.tnic>
References: <20181213230352.6937.74943.stgit@tlendack-t1.amdoffice.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20181213230352.6937.74943.stgit@tlendack-t1.amdoffice.net>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Dec 13, 2018 at 11:03:54PM +0000, Lendacky, Thomas wrote:
> Different AMD processors may have different implementations of STIBP.
> When STIBP is conditionally enabled, some implementations would benefit
> from having STIBP always on instead of toggling the STIBP bit through MSR
> writes. This preference is advertised through a CPUID feature bit.
> 
> When conditional STIBP support is requested at boot and the CPU advertises
> STIBP always-on mode as preferred, switch to STIBP "on" support. To show
> that this transition has occurred, create a new spectre_v2_user_mitigation
> value and a new spectre_v2_user_strings message. The new mitigation value
> is used in spectre_v2_user_select_mitigation() to print the new mitigation
> message as well as to return a new string from stibp_state().
> 
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> 
> This patch is against the x86/pti branch of the tip tree:
>   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/pti
> 
> Changes from v2:
> - Reverted back to original version of the patch
> - Updated the messages that are issued. I'm not terribly happy with the
>   wording, so if there are any suggestions...
> 
> Changes from v1:
> - Removed explicit SPECTRE_V2_USER_STRICT_PREFERRED mode
> - Added a message when switching to always-on mode
> - Set and used a static boolean for the string in stibp_state()
> 
>  arch/x86/include/asm/cpufeatures.h   |    1 +
>  arch/x86/include/asm/nospec-branch.h |    1 +
>  arch/x86/kernel/cpu/bugs.c           |   28 ++++++++++++++++++++++------
>  3 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 28c4a50..df8e94e2 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -284,6 +284,7 @@
>  #define X86_FEATURE_AMD_IBPB		(13*32+12) /* "" Indirect Branch Prediction Barrier */
>  #define X86_FEATURE_AMD_IBRS		(13*32+14) /* "" Indirect Branch Restricted Speculation */
>  #define X86_FEATURE_AMD_STIBP		(13*32+15) /* "" Single Thread Indirect Branch Predictors */
> +#define X86_FEATURE_AMD_STIBP_ALWAYS_ON	(13*32+17) /* "" Single Thread Indirect Branch Predictors always-on preferred */
>  #define X86_FEATURE_AMD_SSBD		(13*32+24) /* "" Speculative Store Bypass Disable */
>  #define X86_FEATURE_VIRT_SSBD		(13*32+25) /* Virtualized Speculative Store Bypass Disable */
>  #define X86_FEATURE_AMD_SSB_NO		(13*32+26) /* "" Speculative Store Bypass is fixed in hardware. */
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index 032b600..dad12b7 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -232,6 +232,7 @@ enum spectre_v2_mitigation {
>  enum spectre_v2_user_mitigation {
>  	SPECTRE_V2_USER_NONE,
>  	SPECTRE_V2_USER_STRICT,
> +	SPECTRE_V2_USER_STRICT_PREFERRED,
>  	SPECTRE_V2_USER_PRCTL,
>  	SPECTRE_V2_USER_SECCOMP,
>  };
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 58689ac..77bf225 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -262,10 +262,11 @@ enum spectre_v2_user_cmd {
>  };
>  
>  static const char * const spectre_v2_user_strings[] = {
> -	[SPECTRE_V2_USER_NONE]		= "User space: Vulnerable",
> -	[SPECTRE_V2_USER_STRICT]	= "User space: Mitigation: STIBP protection",
> -	[SPECTRE_V2_USER_PRCTL]		= "User space: Mitigation: STIBP via prctl",
> -	[SPECTRE_V2_USER_SECCOMP]	= "User space: Mitigation: STIBP via seccomp and prctl",
> +	[SPECTRE_V2_USER_NONE]			= "User space: Vulnerable",
> +	[SPECTRE_V2_USER_STRICT]		= "User space: Mitigation: STIBP protection",
> +	[SPECTRE_V2_USER_STRICT_PREFERRED]	= "User space: Mitigation: STIBP always-on protection",

Barring the "preferred" vs "always-on" bikeshed controversy :-P, I don't
see anything wrong with this version.

The only thing that could be improved maybe is having the mode name
and the user visible string match for ease of code staring during bug
hunting...

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.