Messages in this thread Patch in this message | | | Date | Thu, 15 Feb 2024 15:14:15 -0600 | Subject | Re: [PATCH 09/10] KVM: SEV: introduce KVM_SEV_INIT2 operation | From | Tom Lendacky <> |
| |
On 2/9/24 12:37, Paolo Bonzini wrote: > The idea that no parameter would ever be necessary when enabling SEV or > SEV-ES for a VM was decidedly optimistic. In fact, in some sense it's > already a parameter whether SEV or SEV-ES is desired. Another possible > source of variability is the desired set of VMSA features, as that affects > the measurement of the VM's initial state and cannot be changed > arbitrarily by the hypervisor. > > Create a new sub-operation for KVM_MEM_ENCRYPT_OP that can take a struct, > and put the new op to work by including the VMSA features as a field of the > struct. The existing KVM_SEV_INIT and KVM_SEV_ES_INIT use the full set of > supported VMSA features for backwards compatibility. > > The struct also includes the usual bells and whistles for future > extensibility: a flags field that must be zero for now, and some padding > at the end. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > .../virt/kvm/x86/amd-memory-encryption.rst | 41 ++++++++++++++-- > arch/x86/include/uapi/asm/kvm.h | 10 ++++ > arch/x86/kvm/svm/sev.c | 48 +++++++++++++++++-- > 3 files changed, 92 insertions(+), 7 deletions(-) >
..
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index acf5c45ef14e..78c52764453f 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -252,7 +252,9 @@ static void sev_unbind_asid(struct kvm *kvm, unsigned int handle) > sev_decommission(handle); > } > > -static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) > +static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp, > + struct kvm_sev_init *data, > + unsigned long vm_type) > { > struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > int asid, ret; > @@ -260,7 +262,10 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) > if (kvm->created_vcpus) > return -EINVAL; > > - if (kvm->arch.vm_type != KVM_X86_DEFAULT_VM) > + if (data->flags) > + return -EINVAL; > + > + if (data->vmsa_features & ~sev_supported_vmsa_features)
An SEV guest doesn't have protected state and so it doesn't have a VMSA to which you can apply features. So this should be:
if (vm_type != KVM_X86_SEV_VM && (data->vmsa_features & ~sev_supported_vmsa_features))
Thanks, Tom
> return -EINVAL; > > ret = -EBUSY; > @@ -268,8 +273,8 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) > return ret; > > sev->active = true; > - sev->es_active = argp->id == KVM_SEV_ES_INIT; > - sev->vmsa_features = sev_supported_vmsa_features; > + sev->es_active = (vm_type & __KVM_X86_PROTECTED_STATE_TYPE) != 0; > + sev->vmsa_features = data->vmsa_features; > > asid = sev_asid_new(sev); > if (asid < 0) > @@ -298,6 +303,38 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) > return ret; > } > > +static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) > +{ > + struct kvm_sev_init data = { > + .vmsa_features = sev_supported_vmsa_features, > + }; > + unsigned long vm_type; > + > + if (kvm->arch.vm_type != KVM_X86_DEFAULT_VM) > + return -EINVAL; > + > + vm_type = (argp->id == KVM_SEV_INIT ? KVM_X86_SEV_VM : KVM_X86_SEV_ES_VM); > + return __sev_guest_init(kvm, argp, &data, vm_type); > +} > + > +static int sev_guest_init2(struct kvm *kvm, struct kvm_sev_cmd *argp) > +{ > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > + struct kvm_sev_init data; > + > + if (!sev->need_init) > + return -EINVAL; > + > + if (kvm->arch.vm_type != KVM_X86_SEV_VM && > + kvm->arch.vm_type != KVM_X86_SEV_ES_VM) > + return -EINVAL; > + > + if (copy_from_user(&data, (void __user *)(uintptr_t)argp->data, sizeof(data))) > + return -EFAULT; > + > + return __sev_guest_init(kvm, argp, &data, kvm->arch.vm_type); > +} > + > static int sev_bind_asid(struct kvm *kvm, unsigned int handle, int *error) > { > struct sev_data_activate activate; > @@ -1915,6 +1952,9 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp) > case KVM_SEV_INIT: > r = sev_guest_init(kvm, &sev_cmd); > break; > + case KVM_SEV_INIT2: > + r = sev_guest_init2(kvm, &sev_cmd); > + break; > case KVM_SEV_LAUNCH_START: > r = sev_launch_start(kvm, &sev_cmd); > break;
| |