| Message ID | 20231230172351.574091-16-michael.roth@amd.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support | expand |
On Sat, Dec 30, 2023 at 6:26 PM Michael Roth <michael.roth@amd.com> wrote: > > From: Brijesh Singh <brijesh.singh@amd.com> > > The KVM_SNP_INIT command is used by the hypervisor to initialize the > SEV-SNP platform context. In a typical workflow, this command should be > the first command issued. When creating SEV-SNP guest, the VMM must use > this command instead of the KVM_SEV_INIT or KVM_SEV_ES_INIT. > > The flags value must be zero, it will be extended in future SNP support > to communicate the optional features (such as restricted INT injection > etc). We have a (preexisting) problem in that KVM_SEV_INIT and KVM_SEV_ES_INIT are not flexible enough. debug_swap has broken measurements of the VMSA because it changed the contents of the VMSA under userspace's feet, therefore VMSA features need to be passed into the API somehow. It's preexisting but we need to fix it before the new KVM_SNP_INIT API makes it worse. I have started prototyping a change to move SEV-ES/SEV-SNP to KVM_CREATE_VM, and introduce a single KVM_SEV_INIT_VM operation that can be used for the PSP initialization. > +The flags bitmap is defined as:: > + > + /* enable the restricted injection */ > + #define KVM_SEV_SNP_RESTRICTED_INJET (1<<0) > + > + /* enable the restricted injection timer */ > + #define KVM_SEV_SNP_RESTRICTED_TIMER_INJET (1<<1) These are not yet supported, so they might as well not be documented. If you want to document them, you need to provide an API to query SEV_SNP_SUPPORTED_FLAGS. Let's do that later. > + if (params.flags & ~SEV_SNP_SUPPORTED_FLAGS) > + ret = -EOPNOTSUPP; > + > + params.flags = SEV_SNP_SUPPORTED_FLAGS; This assignment is not necessary. Paolo
On Sat, Dec 30, 2023 at 6:26 PM Michael Roth <michael.roth@amd.com> wrote: > + struct kvm_snp_init { > + __u64 flags; > + }; > + > +The flags bitmap is defined as:: > + > + /* enable the restricted injection */ > + #define KVM_SEV_SNP_RESTRICTED_INJET (1<<0) > + > + /* enable the restricted injection timer */ > + #define KVM_SEV_SNP_RESTRICTED_TIMER_INJET (1<<1) The flags are the same as the vmsa_features introduced by KVM_SEV_INIT2, which is great - SNP does not need any change in this department and this patch almost entirely goes away. > if (sev_es_debug_swap_enabled) > save->sev_features |= SVM_SEV_FEAT_DEBUG_SWAP; > > + /* Enable the SEV-SNP feature */ > + if (sev_snp_guest(svm->vcpu.kvm)) > + save->sev_features |= SVM_SEV_FEAT_SNP_ACTIVE; ... on the other hand this begs the question whether SVM_SEV_FEAT_SNP_ACTIVE should be exposed in the KVM_X86_SEV_VMSA_FEATURES attribute. I think it shouldn't. This means that this patch becomes a two-liner change to sev_guest_init() that you can squash in patch 14 ("KVM: SEV: Add initial SEV-SNP support"): sev->es_active = es_active; sev->vmsa_features = data->vmsa_features; + if (vm_type == KVM_X86_SNP_VM) + sev->vmsa_features |= SVM_SEV_FEAT_SNP_ACTIVE Also, since there is now sev->vmsa_features (that wasn't there at the time of your posting), I'd even drop sev->snp_active in favor of "sev->vmsa_features & SVM_SEV_FEAT_SNP_ACTIVE". It's only ever used in sev_snp_guest() so it's a useless duplication. Looking forward to see v12. :) If you have any problems rebasing on top of https://lore.kernel.org/kvm/20240227232100.478238-1-pbonzini@redhat.com/, please shout. Paolo > pr_debug("Virtual Machine Save Area (VMSA):\n"); > print_hex_dump_debug("", DUMP_PREFIX_NONE, 16, 1, save, sizeof(*save), false); > > @@ -1883,6 +1914,12 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp) > } > > switch (sev_cmd.id) { > + case KVM_SEV_SNP_INIT: > + if (!sev_snp_enabled) { > + r = -ENOTTY; > + goto out; > + } > + fallthrough; > case KVM_SEV_ES_INIT: > if (!sev_es_enabled) { > r = -ENOTTY; > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h > index a3e27c82866b..07a9eb5b6ce5 100644 > --- a/arch/x86/kvm/svm/svm.h > +++ b/arch/x86/kvm/svm/svm.h > @@ -76,6 +76,9 @@ enum { > /* TPR and CR2 are always written before VMRUN */ > #define VMCB_ALWAYS_DIRTY_MASK ((1U << VMCB_INTR) | (1U << VMCB_CR2)) > > +/* Supported init feature flags */ > +#define SEV_SNP_SUPPORTED_FLAGS 0x0 > + > struct kvm_sev_info { > bool active; /* SEV enabled guest */ > bool es_active; /* SEV-ES enabled guest */ > @@ -91,6 +94,7 @@ struct kvm_sev_info { > struct list_head mirror_entry; /* Use as a list entry of mirrors */ > struct misc_cg *misc_cg; /* For misc cgroup accounting */ > atomic_t migration_in_progress; > + u64 snp_init_flags; > }; > > struct kvm_svm { > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index c3308536482b..73702e9b9d76 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -1869,6 +1869,9 @@ enum sev_cmd_id { > /* Guest Migration Extension */ > KVM_SEV_SEND_CANCEL, > > + /* SNP specific commands */ > + KVM_SEV_SNP_INIT, > + > KVM_SEV_NR_MAX, > }; > > @@ -1965,6 +1968,16 @@ struct kvm_sev_receive_update_data { > __u32 trans_len; > }; > > +/* enable the restricted injection */ > +#define KVM_SEV_SNP_RESTRICTED_INJET (1 << 0) > + > +/* enable the restricted injection timer */ > +#define KVM_SEV_SNP_RESTRICTED_TIMER_INJET (1 << 1) > + > +struct kvm_snp_init { > + __u64 flags; > +}; > + > #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0) > #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1) > #define KVM_DEV_ASSIGN_MASK_INTX (1 << 2) > -- > 2.25.1 >
diff --git a/Documentation/virt/kvm/x86/amd-memory-encryption.rst b/Documentation/virt/kvm/x86/amd-memory-encryption.rst index 995780088eb2..b1a19c9a577a 100644 --- a/Documentation/virt/kvm/x86/amd-memory-encryption.rst +++ b/Documentation/virt/kvm/x86/amd-memory-encryption.rst @@ -434,6 +434,33 @@ issued by the hypervisor to make the guest ready for execution. Returns: 0 on success, -negative on error +18. KVM_SNP_INIT +---------------- + +The KVM_SNP_INIT command can be used by the hypervisor to initialize SEV-SNP +context. In a typical workflow, this command should be the first command issued. + +Parameters (in/out): struct kvm_snp_init + +Returns: 0 on success, -negative on error + +:: + + struct kvm_snp_init { + __u64 flags; + }; + +The flags bitmap is defined as:: + + /* enable the restricted injection */ + #define KVM_SEV_SNP_RESTRICTED_INJET (1<<0) + + /* enable the restricted injection timer */ + #define KVM_SEV_SNP_RESTRICTED_TIMER_INJET (1<<1) + +If the specified flags is not supported then return -EOPNOTSUPP, and the supported +flags are returned. + References ========== diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h index 87a7b917d30e..ba8ce15b27d7 100644 --- a/arch/x86/include/asm/svm.h +++ b/arch/x86/include/asm/svm.h @@ -286,6 +286,7 @@ static_assert((X2AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AVIC_ #define AVIC_HPA_MASK ~((0xFFFULL << 52) | 0xFFF) #define SVM_SEV_FEAT_DEBUG_SWAP BIT(5) +#define SVM_SEV_FEAT_SNP_ACTIVE BIT(0) struct vmcb_seg { u16 selector; diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 18c09863377b..43b8ae7b74f8 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -250,6 +250,25 @@ static void sev_unbind_asid(struct kvm *kvm, unsigned int handle) sev_decommission(handle); } +static int verify_snp_init_flags(struct kvm *kvm, struct kvm_sev_cmd *argp) +{ + struct kvm_snp_init params; + int ret = 0; + + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data, sizeof(params))) + return -EFAULT; + + if (params.flags & ~SEV_SNP_SUPPORTED_FLAGS) + ret = -EOPNOTSUPP; + + params.flags = SEV_SNP_SUPPORTED_FLAGS; + + if (copy_to_user((void __user *)(uintptr_t)argp->data, ¶ms, sizeof(params))) + ret = -EFAULT; + + return ret; +} + static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) { struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; @@ -264,12 +283,19 @@ 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->es_active = (argp->id == KVM_SEV_ES_INIT || argp->id == KVM_SEV_SNP_INIT); + sev->snp_active = argp->id == KVM_SEV_SNP_INIT; asid = sev_asid_new(sev); if (asid < 0) goto e_no_asid; sev->asid = asid; + if (sev->snp_active) { + ret = verify_snp_init_flags(kvm, argp); + if (ret) + goto e_free; + } + init_args.probe = false; ret = sev_platform_init(&init_args); if (ret) @@ -287,6 +313,7 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) sev_asid_free(sev); sev->asid = 0; e_no_asid: + sev->snp_active = false; sev->es_active = false; sev->active = false; return ret; @@ -625,6 +652,10 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm) if (sev_es_debug_swap_enabled) save->sev_features |= SVM_SEV_FEAT_DEBUG_SWAP; + /* Enable the SEV-SNP feature */ + if (sev_snp_guest(svm->vcpu.kvm)) + save->sev_features |= SVM_SEV_FEAT_SNP_ACTIVE; + pr_debug("Virtual Machine Save Area (VMSA):\n"); print_hex_dump_debug("", DUMP_PREFIX_NONE, 16, 1, save, sizeof(*save), false); @@ -1883,6 +1914,12 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp) } switch (sev_cmd.id) { + case KVM_SEV_SNP_INIT: + if (!sev_snp_enabled) { + r = -ENOTTY; + goto out; + } + fallthrough; case KVM_SEV_ES_INIT: if (!sev_es_enabled) { r = -ENOTTY; diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h index a3e27c82866b..07a9eb5b6ce5 100644 --- a/arch/x86/kvm/svm/svm.h +++ b/arch/x86/kvm/svm/svm.h @@ -76,6 +76,9 @@ enum { /* TPR and CR2 are always written before VMRUN */ #define VMCB_ALWAYS_DIRTY_MASK ((1U << VMCB_INTR) | (1U << VMCB_CR2)) +/* Supported init feature flags */ +#define SEV_SNP_SUPPORTED_FLAGS 0x0 + struct kvm_sev_info { bool active; /* SEV enabled guest */ bool es_active; /* SEV-ES enabled guest */ @@ -91,6 +94,7 @@ struct kvm_sev_info { struct list_head mirror_entry; /* Use as a list entry of mirrors */ struct misc_cg *misc_cg; /* For misc cgroup accounting */ atomic_t migration_in_progress; + u64 snp_init_flags; }; struct kvm_svm { diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index c3308536482b..73702e9b9d76 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -1869,6 +1869,9 @@ enum sev_cmd_id { /* Guest Migration Extension */ KVM_SEV_SEND_CANCEL, + /* SNP specific commands */ + KVM_SEV_SNP_INIT, + KVM_SEV_NR_MAX, }; @@ -1965,6 +1968,16 @@ struct kvm_sev_receive_update_data { __u32 trans_len; }; +/* enable the restricted injection */ +#define KVM_SEV_SNP_RESTRICTED_INJET (1 << 0) + +/* enable the restricted injection timer */ +#define KVM_SEV_SNP_RESTRICTED_TIMER_INJET (1 << 1) + +struct kvm_snp_init { + __u64 flags; +}; + #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0) #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1) #define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)