On Mon, Feb 26, 2024 at 02:03:43PM -0500, 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_MEMORY_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@xxxxxxxxxx>
> ---
> .../virt/kvm/x86/amd-memory-encryption.rst | 40 +++++++++++++--
> arch/x86/include/uapi/asm/kvm.h | 9 ++++
> arch/x86/kvm/svm/sev.c | 50 +++++++++++++++++--
> 3 files changed, 92 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/virt/kvm/x86/amd-memory-encryption.rst b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
> index 5ed11bc16b96..b951d82af26c 100644
> --- a/Documentation/virt/kvm/x86/amd-memory-encryption.rst
> +++ b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
> @@ -75,15 +75,49 @@ are defined in ``<linux/psp-dev.h>``.
> KVM implements the following commands to support common lifecycle events of SEV
> guests, such as launching, running, snapshotting, migrating and decommissioning.
>
> -1. KVM_SEV_INIT
> ----------------
> +1. KVM_SEV_INIT2
> +----------------
>
> -The KVM_SEV_INIT command is used by the hypervisor to initialize the SEV platform
> +The KVM_SEV_INIT2 command is used by the hypervisor to initialize the SEV platform
> context. In a typical workflow, this command should be the first command issued.
>
> +For this command to be accepted, either KVM_X86_SEV_VM or KVM_X86_SEV_ES_VM
> +must have been passed to the KVM_CREATE_VM ioctl. A virtual machine created
> +with those machine types in turn cannot be run until KVM_SEV_INIT2 is invoked.
> +
> +Parameters: struct kvm_sev_init (in)
>
> Returns: 0 on success, -negative on error
>
> +::
> +
> + struct struct kvm_sev_init {
Remove the duplicated "struct"
> + __u64 vmsa_features; /* initial value of features field in VMSA */
> + __u32 flags; /* must be 0 */
> + __u32 pad[9];
> + };
> +
> +It is an error if the hypervisor does not support any of the bits that
> +are set in ``flags`` or ``vmsa_features``. ``vmsa_features`` must be
> +0 for SEV virtual machines, as they do not have a VMSA.
> +
> +This command replaces the deprecated KVM_SEV_INIT and KVM_SEV_ES_INIT commands.
> +The commands did not have any parameters (the ```data``` field was unused) and
> +only work for the KVM_X86_DEFAULT_VM machine type (0).
> +
> +They behave as if:
> +
> +* the VM type is KVM_X86_SEV_VM for KVM_SEV_INIT, or KVM_X86_SEV_ES_VM for
> + KVM_SEV_ES_INIT
> +
> +* the ``flags`` and ``vmsa_features`` fields of ``struct kvm_sev_init`` are
> + set to zero
> +
> +If the ``KVM_X86_SEV_VMSA_FEATURES`` attribute does not exist, the hypervisor only
> +supports KVM_SEV_INIT and KVM_SEV_ES_INIT. In that case, note that KVM_SEV_ES_INIT
> +might set the debug swap VMSA feature (bit 5) depending on the value of the
> +``debug_swap`` parameter of ``kvm-amd.ko``.
> +
> 2. KVM_SEV_LAUNCH_START
> -----------------------
>
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index 9d950b0b64c9..51b13080ed4b 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -690,6 +690,9 @@ enum sev_cmd_id {
> /* Guest Migration Extension */
> KVM_SEV_SEND_CANCEL,
>
> + /* Second time is the charm; improved versions of the above ioctls. */
> + KVM_SEV_INIT2,
> +
> KVM_SEV_NR_MAX,
> };
>
> @@ -701,6 +704,12 @@ struct kvm_sev_cmd {
> __u32 sev_fd;
> };
>
> +struct kvm_sev_init {
> + __u64 vmsa_features;
> + __u32 flags;
> + __u32 pad[9];
> +};
> +
> struct kvm_sev_launch_start {
> __u32 handle;
> __u32 policy;
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 1248ccf433e8..909e67a9044b 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -239,23 +239,30 @@ 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;
> + bool es_active = kvm->arch.has_protected_state;
> + u64 valid_vmsa_features = es_active ? sev_supported_vmsa_features : 0;
> int ret;
>
> if (kvm->created_vcpus)
> return -EINVAL;
>
> - if (kvm->arch.vm_type != KVM_X86_DEFAULT_VM)
> + if (data->flags)
> + return -EINVAL;
> +
> + if (data->vmsa_features & ~valid_vmsa_features)
> return -EINVAL;
>
> if (unlikely(sev->active))
> return -EINVAL;
>
> sev->active = true;
> - sev->es_active = argp->id == KVM_SEV_ES_INIT;
> - sev->vmsa_features = 0;
> + sev->es_active = es_active;
> + sev->vmsa_features = data->vmsa_features;
>
> ret = sev_asid_new(sev);
> if (ret)
> @@ -283,6 +290,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 = 0,
> + };
> + unsigned long vm_type;
> +
> + if (kvm->arch.vm_type != KVM_X86_DEFAULT_VM)
^
Same here, KVM_X86_SEV_VM?
Thanks,
Yilun
> + 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, u64_to_user_ptr(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)
> {
> unsigned int asid = sev_get_asid(kvm);
> @@ -1898,6 +1937,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;
> --
> 2.39.1
>
>
>
