Re: [PATCH v3 2/2] KVM: SEV: Configure "ALLOWED_SEV_FEATURES" VMCB Field

[Kim Phillips <kim.phillips@amd.com>]

On 2/11/25 3:46 PM, Sean Christopherson wrote:
> On Mon, Feb 10, 2025, Tom Lendacky wrote:
>> On 2/7/25 17:34, Kim Phillips wrote:
>>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>>> index a2a794c32050..a9e16792cac0 100644
>>> --- a/arch/x86/kvm/svm/sev.c
>>> +++ b/arch/x86/kvm/svm/sev.c
>>> @@ -894,9 +894,19 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
>>>   	return 0;
>>>   }
>>>   
>>> +static u64 allowed_sev_features(struct kvm_sev_info *sev)
>>> +{
>>> +	if (cpu_feature_enabled(X86_FEATURE_ALLOWED_SEV_FEATURES) &&
>>
>> Not sure if the cpu_feature_enabled() check is necessary, as init should
>> have failed if SVM_SEV_FEAT_ALLOWED_SEV_FEATURES wasn't set in
>> sev_supported_vmsa_features.
> 
> Two things missing from this series:
> 
>   1: KVM enforcement.  No way is KVM going to rely on userspace to opt-in to
>      preventing the guest from enabling features.
>   2: Backwards compatilibity if KVM unconditionally enforces ALLOWED_SEV_FEATURES.
>      Although maybe there's nothing to do here?  I vaguely recall all of the gated
>      features being unsupported, or something...

This contradicts your review comment from the previous version of the series [1].

If KVM enforces ALLOWED_SEV_FEATURES, it can break existing VMs, thus
the explicit userspace allowed-sev-features=on opt-in [2].

Thanks,

Kim

[1] https://lore.kernel.org/kvm/ZsfKYHFkWA-Rh23C@google.com/
[2] https://lore.kernel.org/kvm/20250207233327.130770-1-kim.phillips@amd.com/