Re: [PATCH v3] x86/cpu: clear SME features when not in use

[Tom Lendacky]

On 2/14/22 17:58, Mario Limonciello wrote:
> Currently the SME CPU feature flag is reflective of whether the CPU
> supports the features but not whether they have been activated by the

s/features/feature/
s/they have/it has/

The plural stuff appears left over from the previous patch that was also 
working on SEV-related features.

> kernel.
>
> Change this around to clear the features if the kernel is not using
> them so userspace can determine if they are available and in use
> from `/proc/cpuinfo`.
>
> As the feature flag is now cleared on systems that SME isn't active

s/that SME isn't active/where SME isn't active,/

> use CPUID 0x8000001f to confirm SME availability before calling
> `native_wbinvd`.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
> Changes from v2->v3:
>   * Drop all changes related to SEV, these will be worked out separately.
>   * Only clear SME flag rather than all flags if sme_me_mask is not set
>
>   arch/x86/kernel/cpu/amd.c | 3 +++
>   arch/x86/kernel/process.c | 5 ++++-
>   2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 4edb6f0f628c..8421b9d9f552 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -607,6 +607,9 @@ static void early_detect_mem_encrypt(struct cpuinfo_x86 *c)
>   		if (IS_ENABLED(CONFIG_X86_32))
>   			goto clear_all;
>   
> +		if (!sme_me_mask)
> +			setup_clear_cpu_cap(X86_FEATURE_SME);

The block comment above the "if (cpu_has(c, X86_FEATURE_SME) ..." should 
be updated to match, too.

Thanks,
Tom

> +
>   		rdmsrl(MSR_K7_HWCR, msr);
>   		if (!(msr & MSR_K7_HWCR_SMMLOCK))
>   			goto clear_sev;
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 81d8ef036637..e131d71b3cae 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -765,8 +765,11 @@ void stop_this_cpu(void *dummy)
>   	 * without the encryption bit, they don't race each other when flushed
>   	 * and potentially end up with the wrong entry being committed to
>   	 * memory.
> +	 *
> +	 * Test the CPUID bit directly because the machine might've cleared
> +	 * X86_FEATURE_SME due to cmdline options.
>   	 */
> -	if (boot_cpu_has(X86_FEATURE_SME))
> +	if (cpuid_eax(0x8000001f) & BIT(0))
>   		native_wbinvd();
>   	for (;;) {
>   		/*