Re: [PATCH 2/2] x86/cpu: clear SME/SEV/SEV_ES features when not in use

[Tom Lendacky <thomas.lendacky@amd.com>]

+Brijesh (please copy him, too, on all SEV related things)

On 2/10/22 23:36, Mario Limonciello wrote:
> Currently SME/SEV/SEV_ES features are reflective of whether the CPU
> supports the features but not whether they have been activated by the
> 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`.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   arch/x86/kernel/cpu/amd.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 4edb6f0f628c..4a7d4801fa0b 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -611,12 +611,20 @@ static void early_detect_mem_encrypt(struct cpuinfo_x86 *c)
>   		if (!(msr & MSR_K7_HWCR_SMMLOCK))
>   			goto clear_sev;
>   
> +		if (!cc_platform_has(CC_ATTR_MEM_ENCRYPT))
> +			goto clear_all;
> +		if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> +			goto clear_sev;
> +		if (!cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
> +			goto clear_sev_es;
> +

A host/hypervisor will never see GUEST related attributes return true, you 
need to verify all uses of X86_FEATURE_SEV* in the kernel. I see two files 
where this change will break SEV hypervisor support:

- arch/x86/kvm/svm/sev.c / sev_hardware_setup()
- drivers/crypto/ccp/sev-dev.c / sev_dev_init()

I imagine that some of this has to be re-thought a bit. The current SEV 
and SEV-ES feature attributes are intended for hypervsior side usage.  For 
example, MSR MSR_K7_HWCR_SMMLOCK, which is likely never going to be 
provided to a guest, needs to be checked to be certain that an SEV guest 
can be launched, even though SEV is advertised in CPUID.

Once in the guest, it is the cc_platform_has() support that determines 
feature support and not X86_FEATURE_SEV*

Thanks,
Tom

>   		return;
>   
>   clear_all:
>   		setup_clear_cpu_cap(X86_FEATURE_SME);
>   clear_sev:
>   		setup_clear_cpu_cap(X86_FEATURE_SEV);
> +clear_sev_es:
>   		setup_clear_cpu_cap(X86_FEATURE_SEV_ES);
>   	}
>   }