| Message ID | 20190710201244.25195-2-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add AMD SEV guest live migration support | expand |
On Wed, Jul 10, 2019 at 08:13:00PM +0000, Singh, Brijesh wrote: > The command is used to create an outgoing SEV guest encryption context. > > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: "Radim Krčmář" <rkrcmar@redhat.com> > Cc: Joerg Roedel <joro@8bytes.org> > Cc: Borislav Petkov <bp@suse.de> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Cc: x86@kernel.org > Cc: kvm@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > .../virtual/kvm/amd-memory-encryption.rst | 27 +++++ > arch/x86/kvm/svm.c | 105 ++++++++++++++++++ > include/uapi/linux/kvm.h | 12 ++ > 3 files changed, 144 insertions(+) > > diff --git a/Documentation/virtual/kvm/amd-memory-encryption.rst b/Documentation/virtual/kvm/amd-memory-encryption.rst > index d18c97b4e140..0e9e1e9f9687 100644 > --- a/Documentation/virtual/kvm/amd-memory-encryption.rst > +++ b/Documentation/virtual/kvm/amd-memory-encryption.rst Do a s/virtual/virt/g for the next revision because this path got changed recently: 2f5947dfcaec ("Documentation: move Documentation/virtual to Documentation/virt") > @@ -238,6 +238,33 @@ Returns: 0 on success, -negative on error > __u32 trans_len; > }; > > +10. KVM_SEV_SEND_START > +---------------------- > + > +The KVM_SEV_SEND_START command can be used by the hypervisor to create an > +outgoing guest encryption context. > + > +Parameters (in): struct kvm_sev_send_start > + > +Returns: 0 on success, -negative on error > + > +:: > + struct kvm_sev_send_start { > + __u32 policy; /* guest policy */ > + > + __u64 pdh_cert_uaddr; /* platform Diffie-Hellman certificate */ > + __u32 pdh_cert_len; > + > + __u64 plat_cert_uaddr; /* platform certificate chain */ > + __u32 plat_cert_len; > + > + __u64 amd_cert_uaddr; /* AMD certificate */ > + __u32 amd_cert_len; > + > + __u64 session_uaddr; /* Guest session information */ > + __u32 session_len; > + }; SEV API doc has "CERT" for PDH members but "CERTS" for the others. Judging by the description, you should do the same here too. Just so that there's no discrepancy from the docs. > + > References > ========== > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 48c865a4e5dd..0b0937f53520 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -6957,6 +6957,108 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp) > return ret; > } > > +static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp) > +{ > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > + void *amd_cert = NULL, *session_data = NULL; > + void *pdh_cert = NULL, *plat_cert = NULL; > + struct sev_data_send_start *data = NULL; Why are you initializing those to NULL? Also, SEV API text on SEND_START talks about a bunch of requirements in section "6.8.1 Actions" like "The platform must be in the PSTATE.WORKING state. The guest must be in the GSTATE.RUNNING state. GCTX.POLICY.NOSEND must be zero. Otherwise, an error is returned. ..." Where are we checking/verifying those? > + struct kvm_sev_send_start params; > + int ret; > + > + if (!sev_guest(kvm)) > + return -ENOTTY; > + > + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data, > + sizeof(struct kvm_sev_send_start))) > + return -EFAULT; > + > + data = kzalloc(sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; Move that allocation... > + > + /* userspace wants to query the session length */ > + if (!params.session_len) > + goto cmd; > + > + if (!params.pdh_cert_uaddr || !params.pdh_cert_len || > + !params.session_uaddr) > + return -EINVAL; > + > + /* copy the certificate blobs from userspace */ > + pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr, params.pdh_cert_len); > + if (IS_ERR(pdh_cert)) { > + ret = PTR_ERR(pdh_cert); > + goto e_free; > + } ... here so that it doesn't happen unnecessarily if the above fail. > + > + data->pdh_cert_address = __psp_pa(pdh_cert); > + data->pdh_cert_len = params.pdh_cert_len; > + > + plat_cert = psp_copy_user_blob(params.plat_cert_uaddr, params.plat_cert_len); > + if (IS_ERR(plat_cert)) { > + ret = PTR_ERR(plat_cert); > + goto e_free_pdh; > + } > + > + data->plat_cert_address = __psp_pa(plat_cert); > + data->plat_cert_len = params.plat_cert_len; > + > + amd_cert = psp_copy_user_blob(params.amd_cert_uaddr, params.amd_cert_len); > + if (IS_ERR(amd_cert)) { > + ret = PTR_ERR(amd_cert); > + goto e_free_plat_cert; > + } > + > + data->amd_cert_address = __psp_pa(amd_cert); > + data->amd_cert_len = params.amd_cert_len; > + > + ret = -EINVAL; > + if (params.session_len > SEV_FW_BLOB_MAX_SIZE) > + goto e_free_amd_cert; That check could go up where the other params.session_len check is happening and you can save yourself the cert alloc+freeing. > + > + ret = -ENOMEM; > + session_data = kmalloc(params.session_len, GFP_KERNEL); > + if (!session_data) > + goto e_free_amd_cert; Ditto. ...
On 8/22/19 5:08 AM, Borislav Petkov wrote: > On Wed, Jul 10, 2019 at 08:13:00PM +0000, Singh, Brijesh wrote: >> The command is used to create an outgoing SEV guest encryption context. >> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: "H. Peter Anvin" <hpa@zytor.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: "Radim Krčmář" <rkrcmar@redhat.com> >> Cc: Joerg Roedel <joro@8bytes.org> >> Cc: Borislav Petkov <bp@suse.de> >> Cc: Tom Lendacky <thomas.lendacky@amd.com> >> Cc: x86@kernel.org >> Cc: kvm@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> --- >> .../virtual/kvm/amd-memory-encryption.rst | 27 +++++ >> arch/x86/kvm/svm.c | 105 ++++++++++++++++++ >> include/uapi/linux/kvm.h | 12 ++ >> 3 files changed, 144 insertions(+) >> >> diff --git a/Documentation/virtual/kvm/amd-memory-encryption.rst b/Documentation/virtual/kvm/amd-memory-encryption.rst >> index d18c97b4e140..0e9e1e9f9687 100644 >> --- a/Documentation/virtual/kvm/amd-memory-encryption.rst >> +++ b/Documentation/virtual/kvm/amd-memory-encryption.rst > > Do a > > s/virtual/virt/g > > for the next revision because this path got changed recently: > > 2f5947dfcaec ("Documentation: move Documentation/virtual to Documentation/virt") > >> @@ -238,6 +238,33 @@ Returns: 0 on success, -negative on error >> __u32 trans_len; >> }; >> >> +10. KVM_SEV_SEND_START >> +---------------------- >> + >> +The KVM_SEV_SEND_START command can be used by the hypervisor to create an >> +outgoing guest encryption context. >> + >> +Parameters (in): struct kvm_sev_send_start >> + >> +Returns: 0 on success, -negative on error >> + >> +:: >> + struct kvm_sev_send_start { >> + __u32 policy; /* guest policy */ >> + >> + __u64 pdh_cert_uaddr; /* platform Diffie-Hellman certificate */ >> + __u32 pdh_cert_len; >> + >> + __u64 plat_cert_uaddr; /* platform certificate chain */ >> + __u32 plat_cert_len; >> + >> + __u64 amd_cert_uaddr; /* AMD certificate */ >> + __u32 amd_cert_len; >> + >> + __u64 session_uaddr; /* Guest session information */ >> + __u32 session_len; >> + }; > > SEV API doc has "CERT" for PDH members but "CERTS" for the others. > Judging by the description, you should do the same here too. Just so that > there's no discrepancy from the docs. > Noted. >> + >> References >> ========== >> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index 48c865a4e5dd..0b0937f53520 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -6957,6 +6957,108 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp) >> return ret; >> } >> >> +static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp) >> +{ >> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; >> + void *amd_cert = NULL, *session_data = NULL; >> + void *pdh_cert = NULL, *plat_cert = NULL; >> + struct sev_data_send_start *data = NULL; > > Why are you initializing those to NULL? > It simplifies the error handling path where we can do kfree() without knowing whether the buffers were allocated or not. > Also, SEV API text on SEND_START talks about a bunch of requirements in > section > > "6.8.1 Actions" > > like > > "The platform must be in the PSTATE.WORKING state. > The guest must be in the GSTATE.RUNNING state. > GCTX.POLICY.NOSEND must be zero. Otherwise, an error is returned. > ..." > > Where are we checking/verifying those? > The kernel driver does not keep track of all those SEV VM states. The userspace application (e.g qemu) will ensure that VM is in proper state before calling those commands. If userspace does not check states and makes a blind calls then we let the FW error out and propagate the correct error message to the caller so that it can take the required action. >> + struct kvm_sev_send_start params; >> + int ret; >> + >> + if (!sev_guest(kvm)) >> + return -ENOTTY; >> + >> + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data, >> + sizeof(struct kvm_sev_send_start))) >> + return -EFAULT; >> + >> + data = kzalloc(sizeof(*data), GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; > > Move that allocation... > >> + >> + /* userspace wants to query the session length */ >> + if (!params.session_len) >> + goto cmd; >> + >> + if (!params.pdh_cert_uaddr || !params.pdh_cert_len || >> + !params.session_uaddr) >> + return -EINVAL; >> + >> + /* copy the certificate blobs from userspace */ >> + pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr, params.pdh_cert_len); >> + if (IS_ERR(pdh_cert)) { >> + ret = PTR_ERR(pdh_cert); >> + goto e_free; >> + } > > ... here so that it doesn't happen unnecessarily if the above fail. > Noted. >> + >> + data->pdh_cert_address = __psp_pa(pdh_cert); >> + data->pdh_cert_len = params.pdh_cert_len; >> + >> + plat_cert = psp_copy_user_blob(params.plat_cert_uaddr, params.plat_cert_len); >> + if (IS_ERR(plat_cert)) { >> + ret = PTR_ERR(plat_cert); >> + goto e_free_pdh; >> + } >> + >> + data->plat_cert_address = __psp_pa(plat_cert); >> + data->plat_cert_len = params.plat_cert_len; >> + >> + amd_cert = psp_copy_user_blob(params.amd_cert_uaddr, params.amd_cert_len); >> + if (IS_ERR(amd_cert)) { >> + ret = PTR_ERR(amd_cert); >> + goto e_free_plat_cert; >> + } >> + >> + data->amd_cert_address = __psp_pa(amd_cert); >> + data->amd_cert_len = params.amd_cert_len; >> + >> + ret = -EINVAL; >> + if (params.session_len > SEV_FW_BLOB_MAX_SIZE) >> + goto e_free_amd_cert; > > That check could go up where the other params.session_len check is > happening and you can save yourself the cert alloc+freeing. > I will take a look. >> + >> + ret = -ENOMEM; >> + session_data = kmalloc(params.session_len, GFP_KERNEL); >> + if (!session_data) >> + goto e_free_amd_cert; > > Ditto. > > ... >
On Wed, Jul 10, 2019 at 1:13 PM Singh, Brijesh <brijesh.singh@amd.com> wrote: > > +static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp) > +{ > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > + void *amd_cert = NULL, *session_data = NULL; > + void *pdh_cert = NULL, *plat_cert = NULL; > + struct sev_data_send_start *data = NULL; > + struct kvm_sev_send_start params; > + int ret; > + > + if (!sev_guest(kvm)) > + return -ENOTTY; > + > + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data, > + sizeof(struct kvm_sev_send_start))) > + return -EFAULT; > + > + data = kzalloc(sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + /* userspace wants to query the session length */ > + if (!params.session_len) > + goto cmd; > + > + if (!params.pdh_cert_uaddr || !params.pdh_cert_len || > + !params.session_uaddr) > + return -EINVAL; I think pdh_cert is only required if the guest policy SEV bit is set. Can pdh_cert be optional? > + > + /* copy the certificate blobs from userspace */ > + pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr, params.pdh_cert_len); > + if (IS_ERR(pdh_cert)) { > + ret = PTR_ERR(pdh_cert); > + goto e_free; > + } > + > + data->pdh_cert_address = __psp_pa(pdh_cert); > + data->pdh_cert_len = params.pdh_cert_len; > + > + plat_cert = psp_copy_user_blob(params.plat_cert_uaddr, params.plat_cert_len); > + if (IS_ERR(plat_cert)) { > + ret = PTR_ERR(plat_cert); > + goto e_free_pdh; > + } I think plat_cert is also only required if the guest policy SEV bit is set. Can plat_cert also be optional? > + > + data->plat_cert_address = __psp_pa(plat_cert); > + data->plat_cert_len = params.plat_cert_len; > + > + amd_cert = psp_copy_user_blob(params.amd_cert_uaddr, params.amd_cert_len); > + if (IS_ERR(amd_cert)) { > + ret = PTR_ERR(amd_cert); > + goto e_free_plat_cert; > + } I think amd_cert is also only required if the guest policy SEV bit is set. Can amd_cert also be optional? > + > + data->amd_cert_address = __psp_pa(amd_cert); > + data->amd_cert_len = params.amd_cert_len; > + > + ret = -EINVAL; > + if (params.session_len > SEV_FW_BLOB_MAX_SIZE) > + goto e_free_amd_cert; > + > + ret = -ENOMEM; > + session_data = kmalloc(params.session_len, GFP_KERNEL); > + if (!session_data) > + goto e_free_amd_cert; This pattern of returning -EINVAL if a length is greater than SEV_FW_BLOB_MAX_SIZE and -ENOMEM if kmalloc fails is used at sev_launch_measure. And I think in your later patches you do similar, did you consider factoring this out into a helper function similar to psp_copy_user_blob?
On 11/12/19 12:35 PM, Peter Gonda wrote: > On Wed, Jul 10, 2019 at 1:13 PM Singh, Brijesh <brijesh.singh@amd.com> wrote: >> +static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp) >> +{ >> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; >> + void *amd_cert = NULL, *session_data = NULL; >> + void *pdh_cert = NULL, *plat_cert = NULL; >> + struct sev_data_send_start *data = NULL; >> + struct kvm_sev_send_start params; >> + int ret; >> + >> + if (!sev_guest(kvm)) >> + return -ENOTTY; >> + >> + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data, >> + sizeof(struct kvm_sev_send_start))) >> + return -EFAULT; >> + >> + data = kzalloc(sizeof(*data), GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + /* userspace wants to query the session length */ >> + if (!params.session_len) >> + goto cmd; >> + >> + if (!params.pdh_cert_uaddr || !params.pdh_cert_len || >> + !params.session_uaddr) >> + return -EINVAL; > I think pdh_cert is only required if the guest policy SEV bit is set. > Can pdh_cert be optional? We don't cache the policy information in kernel, having said so we can try caching it during the LAUNCH_START to optimize this case. I have to check with FW folks but I believe all those fields are required. IIRC, When I passed NULL then SEND_START failed for me. But I double check it and update you on this. > >> + >> + /* copy the certificate blobs from userspace */ >> + pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr, params.pdh_cert_len); >> + if (IS_ERR(pdh_cert)) { >> + ret = PTR_ERR(pdh_cert); >> + goto e_free; >> + } >> + >> + data->pdh_cert_address = __psp_pa(pdh_cert); >> + data->pdh_cert_len = params.pdh_cert_len; >> + >> + plat_cert = psp_copy_user_blob(params.plat_cert_uaddr, params.plat_cert_len); >> + if (IS_ERR(plat_cert)) { >> + ret = PTR_ERR(plat_cert); >> + goto e_free_pdh; >> + } > I think plat_cert is also only required if the guest policy SEV bit is > set. Can plat_cert also be optional? Same as above, I believe its required. > >> + >> + data->plat_cert_address = __psp_pa(plat_cert); >> + data->plat_cert_len = params.plat_cert_len; >> + >> + amd_cert = psp_copy_user_blob(params.amd_cert_uaddr, params.amd_cert_len); >> + if (IS_ERR(amd_cert)) { >> + ret = PTR_ERR(amd_cert); >> + goto e_free_plat_cert; >> + } > I think amd_cert is also only required if the guest policy SEV bit is > set. Can amd_cert also be optional? Same as above, I believe its required. I will double check it. >> + >> + data->amd_cert_address = __psp_pa(amd_cert); >> + data->amd_cert_len = params.amd_cert_len; >> + >> + ret = -EINVAL; >> + if (params.session_len > SEV_FW_BLOB_MAX_SIZE) >> + goto e_free_amd_cert; >> + >> + ret = -ENOMEM; >> + session_data = kmalloc(params.session_len, GFP_KERNEL); >> + if (!session_data) >> + goto e_free_amd_cert; > This pattern of returning -EINVAL if a length is greater than > SEV_FW_BLOB_MAX_SIZE and -ENOMEM if kmalloc fails is used at > sev_launch_measure. And I think in your later patches you do similar, > did you consider factoring this out into a helper function similar to > psp_copy_user_blob? Yes, we could factor out this check into a separate function. Let me see what I can do in next iteration. thanks for the feedbacks. -Brijesh
On Tue, Nov 12, 2019 at 2:27 PM Brijesh Singh <brijesh.singh@amd.com> wrote: > > > On 11/12/19 12:35 PM, Peter Gonda wrote: > > On Wed, Jul 10, 2019 at 1:13 PM Singh, Brijesh <brijesh.singh@amd.com> wrote: > >> +static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp) > >> +{ > >> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > >> + void *amd_cert = NULL, *session_data = NULL; > >> + void *pdh_cert = NULL, *plat_cert = NULL; > >> + struct sev_data_send_start *data = NULL; > >> + struct kvm_sev_send_start params; > >> + int ret; > >> + > >> + if (!sev_guest(kvm)) > >> + return -ENOTTY; > >> + > >> + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data, > >> + sizeof(struct kvm_sev_send_start))) > >> + return -EFAULT; > >> + > >> + data = kzalloc(sizeof(*data), GFP_KERNEL); > >> + if (!data) > >> + return -ENOMEM; > >> + > >> + /* userspace wants to query the session length */ > >> + if (!params.session_len) > >> + goto cmd; > >> + > >> + if (!params.pdh_cert_uaddr || !params.pdh_cert_len || > >> + !params.session_uaddr) > >> + return -EINVAL; > > I think pdh_cert is only required if the guest policy SEV bit is set. > > Can pdh_cert be optional? > > > We don't cache the policy information in kernel, having said so we can > try caching it during the LAUNCH_START to optimize this case. I have to > check with FW folks but I believe all those fields are required. IIRC, > When I passed NULL then SEND_START failed for me. But I double check it > and update you on this. I must have misinterpreted the this line of the spec: "If GCTX.POLICY.SEV is 1, the PDH, PEK, CEK, ASK, and ARK certificates are validated." I thought that since they were not validated they were not needed.
On 11/14/19 1:27 PM, Peter Gonda wrote: > On Tue, Nov 12, 2019 at 2:27 PM Brijesh Singh <brijesh.singh@amd.com> wrote: >> >> >> On 11/12/19 12:35 PM, Peter Gonda wrote: >>> On Wed, Jul 10, 2019 at 1:13 PM Singh, Brijesh <brijesh.singh@amd.com> wrote: >>>> +static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp) >>>> +{ >>>> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; >>>> + void *amd_cert = NULL, *session_data = NULL; >>>> + void *pdh_cert = NULL, *plat_cert = NULL; >>>> + struct sev_data_send_start *data = NULL; >>>> + struct kvm_sev_send_start params; >>>> + int ret; >>>> + >>>> + if (!sev_guest(kvm)) >>>> + return -ENOTTY; >>>> + >>>> + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data, >>>> + sizeof(struct kvm_sev_send_start))) >>>> + return -EFAULT; >>>> + >>>> + data = kzalloc(sizeof(*data), GFP_KERNEL); >>>> + if (!data) >>>> + return -ENOMEM; >>>> + >>>> + /* userspace wants to query the session length */ >>>> + if (!params.session_len) >>>> + goto cmd; >>>> + >>>> + if (!params.pdh_cert_uaddr || !params.pdh_cert_len || >>>> + !params.session_uaddr) >>>> + return -EINVAL; >>> I think pdh_cert is only required if the guest policy SEV bit is set. >>> Can pdh_cert be optional? >> >> >> We don't cache the policy information in kernel, having said so we can >> try caching it during the LAUNCH_START to optimize this case. I have to >> check with FW folks but I believe all those fields are required. IIRC, >> When I passed NULL then SEND_START failed for me. But I double check it >> and update you on this. > > > I must have misinterpreted the this line of the spec: > "If GCTX.POLICY.SEV is 1, the PDH, PEK, CEK, ASK, and ARK certificates > are validated." > I thought that since they were not validated they were not needed. > I have confirmed that these fields are required by the SEND_START even when GCTX.POLICY.SEV=0. The FW needs us to pass certificate chain, if POLICY.SEV=1 then FW will validating the chain otherwise ignore it.
diff --git a/Documentation/virtual/kvm/amd-memory-encryption.rst b/Documentation/virtual/kvm/amd-memory-encryption.rst index d18c97b4e140..0e9e1e9f9687 100644 --- a/Documentation/virtual/kvm/amd-memory-encryption.rst +++ b/Documentation/virtual/kvm/amd-memory-encryption.rst @@ -238,6 +238,33 @@ Returns: 0 on success, -negative on error __u32 trans_len; }; +10. KVM_SEV_SEND_START +---------------------- + +The KVM_SEV_SEND_START command can be used by the hypervisor to create an +outgoing guest encryption context. + +Parameters (in): struct kvm_sev_send_start + +Returns: 0 on success, -negative on error + +:: + struct kvm_sev_send_start { + __u32 policy; /* guest policy */ + + __u64 pdh_cert_uaddr; /* platform Diffie-Hellman certificate */ + __u32 pdh_cert_len; + + __u64 plat_cert_uaddr; /* platform certificate chain */ + __u32 plat_cert_len; + + __u64 amd_cert_uaddr; /* AMD certificate */ + __u32 amd_cert_len; + + __u64 session_uaddr; /* Guest session information */ + __u32 session_len; + }; + References ========== diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 48c865a4e5dd..0b0937f53520 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -6957,6 +6957,108 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp) return ret; } +static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp) +{ + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; + void *amd_cert = NULL, *session_data = NULL; + void *pdh_cert = NULL, *plat_cert = NULL; + struct sev_data_send_start *data = NULL; + struct kvm_sev_send_start params; + int ret; + + if (!sev_guest(kvm)) + return -ENOTTY; + + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data, + sizeof(struct kvm_sev_send_start))) + return -EFAULT; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + /* userspace wants to query the session length */ + if (!params.session_len) + goto cmd; + + if (!params.pdh_cert_uaddr || !params.pdh_cert_len || + !params.session_uaddr) + return -EINVAL; + + /* copy the certificate blobs from userspace */ + pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr, params.pdh_cert_len); + if (IS_ERR(pdh_cert)) { + ret = PTR_ERR(pdh_cert); + goto e_free; + } + + data->pdh_cert_address = __psp_pa(pdh_cert); + data->pdh_cert_len = params.pdh_cert_len; + + plat_cert = psp_copy_user_blob(params.plat_cert_uaddr, params.plat_cert_len); + if (IS_ERR(plat_cert)) { + ret = PTR_ERR(plat_cert); + goto e_free_pdh; + } + + data->plat_cert_address = __psp_pa(plat_cert); + data->plat_cert_len = params.plat_cert_len; + + amd_cert = psp_copy_user_blob(params.amd_cert_uaddr, params.amd_cert_len); + if (IS_ERR(amd_cert)) { + ret = PTR_ERR(amd_cert); + goto e_free_plat_cert; + } + + data->amd_cert_address = __psp_pa(amd_cert); + data->amd_cert_len = params.amd_cert_len; + + ret = -EINVAL; + if (params.session_len > SEV_FW_BLOB_MAX_SIZE) + goto e_free_amd_cert; + + ret = -ENOMEM; + session_data = kmalloc(params.session_len, GFP_KERNEL); + if (!session_data) + goto e_free_amd_cert; + + data->session_address = __psp_pa(session_data); + data->session_len = params.session_len; +cmd: + data->handle = sev->handle; + ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error); + + /* if we queried the session length, FW responded with expected data */ + if (!params.session_len) + goto done; + + if (copy_to_user((void __user *)(uintptr_t) params.session_uaddr, + session_data, params.session_len)) { + ret = -EFAULT; + goto e_free_session; + } + + params.policy = data->policy; + +done: + params.session_len = data->session_len; + if (copy_to_user((void __user *)(uintptr_t)argp->data, ¶ms, + sizeof(struct kvm_sev_send_start))) + ret = -EFAULT; + +e_free_session: + kfree(session_data); +e_free_amd_cert: + kfree(amd_cert); +e_free_plat_cert: + kfree(plat_cert); +e_free_pdh: + kfree(pdh_cert); +e_free: + kfree(data); + return ret; +} + static int svm_mem_enc_op(struct kvm *kvm, void __user *argp) { struct kvm_sev_cmd sev_cmd; @@ -6998,6 +7100,9 @@ static int svm_mem_enc_op(struct kvm *kvm, void __user *argp) case KVM_SEV_LAUNCH_SECRET: r = sev_launch_secret(kvm, &sev_cmd); break; + case KVM_SEV_SEND_START: + r = sev_send_start(kvm, &sev_cmd); + break; default: r = -EINVAL; goto out; diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 2fe12b40d503..4e9e7a5b2066 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -1531,6 +1531,18 @@ struct kvm_sev_dbg { __u32 len; }; +struct kvm_sev_send_start { + __u32 policy; + __u64 pdh_cert_uaddr; + __u32 pdh_cert_len; + __u64 plat_cert_uaddr; + __u32 plat_cert_len; + __u64 amd_cert_uaddr; + __u32 amd_cert_len; + __u64 session_uaddr; + __u32 session_len; +}; + #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)
The command is used to create an outgoing SEV guest encryption context. Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: "Radim Krčmář" <rkrcmar@redhat.com> Cc: Joerg Roedel <joro@8bytes.org> Cc: Borislav Petkov <bp@suse.de> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: x86@kernel.org Cc: kvm@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- .../virtual/kvm/amd-memory-encryption.rst | 27 +++++ arch/x86/kvm/svm.c | 105 ++++++++++++++++++ include/uapi/linux/kvm.h | 12 ++ 3 files changed, 144 insertions(+)