| Message ID | 20240621171519.3180965-1-michael.roth@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1-revised,1/5] KVM: SEV: Provide support for SNP_GUEST_REQUEST NAE event | expand |
On 21/06/2024 18:15, Michael Roth wrote: > From: Brijesh Singh <brijesh.singh@amd.com> > > Version 2 of GHCB specification added support for the SNP Guest Request > Message NAE event. The event allows for an SEV-SNP guest to make > requests to the SEV-SNP firmware through hypervisor using the > SNP_GUEST_REQUEST API defined in the SEV-SNP firmware specification. > > This is used by guests primarily to request attestation reports from > firmware. There are other request types are available as well, but the > specifics of what guest requests are being made are opaque to the > hypervisor, which only serves as a proxy for the guest requests and > firmware responses. > > Implement handling for these events. > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > Co-developed-by: Alexey Kardashevskiy <aik@amd.com> > Signed-off-by: Alexey Kardashevskiy <aik@amd.com> > Co-developed-by: Ashish Kalra <ashish.kalra@amd.com> > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com> > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> > [mdr: ensure FW command failures are indicated to guest, drop extended > request handling to be re-written as separate patch, massage commit] > Signed-off-by: Michael Roth <michael.roth@amd.com> > Message-ID: <20240501085210.2213060-19-michael.roth@amd.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Liam Merwick <liam.merwick@oracle.com> > --- > arch/x86/kvm/svm/sev.c | 73 ++++++++++++++++++++++++++++++++++ > include/uapi/linux/sev-guest.h | 9 +++++ > 2 files changed, 82 insertions(+) >
On Fri, Jun 21, 2024, Michael Roth wrote: > diff --git a/include/uapi/linux/sev-guest.h b/include/uapi/linux/sev-guest.h > index 154a87a1eca9..7bd78e258569 100644 > --- a/include/uapi/linux/sev-guest.h > +++ b/include/uapi/linux/sev-guest.h > @@ -89,8 +89,17 @@ struct snp_ext_report_req { > #define SNP_GUEST_FW_ERR_MASK GENMASK_ULL(31, 0) > #define SNP_GUEST_VMM_ERR_SHIFT 32 > #define SNP_GUEST_VMM_ERR(x) (((u64)x) << SNP_GUEST_VMM_ERR_SHIFT) > +#define SNP_GUEST_FW_ERR(x) ((x) & SNP_GUEST_FW_ERR_MASK) > +#define SNP_GUEST_ERR(vmm_err, fw_err) (SNP_GUEST_VMM_ERR(vmm_err) | \ > + SNP_GUEST_FW_ERR(fw_err)) > > +/* > + * The GHCB spec only formally defines INVALID_LEN/BUSY VMM errors, but define > + * a GENERIC error code such that it won't ever conflict with GHCB-defined > + * errors if any get added in the future. > + */ > #define SNP_GUEST_VMM_ERR_INVALID_LEN 1 > #define SNP_GUEST_VMM_ERR_BUSY 2 > +#define SNP_GUEST_VMM_ERR_GENERIC BIT(31) Related to my suggestion to not have KVM-defined error codes, if we go that route, then I believe SNP_GUEST_VMM_ERR_GENERIC is unnecessary. For snp_handle_guest_req(), if sev_issue_cmd() fails, KVM can/should do something like: /* Forward non-firmware errors to userspace, e.g. if the PSP is dead. */ if (ret && !fw_err) goto release_req; ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, SNP_GUEST_ERR(0, fw_err)); And then in snp_complete_req_certs(), we could either let userspace shove in any error code whatsoever, or restrict userspace to known, GHCB-defined error codes, e.g. int err; err = READ_ONCE(vcpu->run->coco.req_certs.ret); if (err) if (err != SNP_GUEST_VMM_ERR_INVALID_LEN && err != SNP_GUEST_VMM_ERR_BUSY) return -EINVAL; if (err == SNP_GUEST_VMM_ERR_INVALID_LEN) vcpu->arch.regs[VCPU_REGS_RBX] = vcpu->run->coco.req_certs.npages; ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, SNP_GUEST_ERR(err, 0)); return 1; } > > #endif /* __UAPI_LINUX_SEV_GUEST_H_ */ > -- > 2.25.1 >
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index df8818759698..d9921ea87a81 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -19,6 +19,7 @@ #include <linux/misc_cgroup.h> #include <linux/processor.h> #include <linux/trace_events.h> +#include <uapi/linux/sev-guest.h> #include <asm/pkru.h> #include <asm/trapnr.h> @@ -3321,6 +3322,10 @@ static int sev_es_validate_vmgexit(struct vcpu_svm *svm) if (!sev_snp_guest(vcpu->kvm) || !kvm_ghcb_sw_scratch_is_valid(svm)) goto vmgexit_err; break; + case SVM_VMGEXIT_GUEST_REQUEST: + if (!sev_snp_guest(vcpu->kvm)) + goto vmgexit_err; + break; default: reason = GHCB_ERR_INVALID_EVENT; goto vmgexit_err; @@ -3939,6 +3944,71 @@ static int sev_snp_ap_creation(struct vcpu_svm *svm) return ret; } +static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_gpa) +{ + struct sev_data_snp_guest_request data = {0}; + struct kvm *kvm = svm->vcpu.kvm; + kvm_pfn_t req_pfn, resp_pfn; + sev_ret_code fw_err = 0; + int ret; + + if (!sev_snp_guest(kvm) || !PAGE_ALIGNED(req_gpa) || !PAGE_ALIGNED(resp_gpa)) + return -EINVAL; + + req_pfn = gfn_to_pfn(kvm, gpa_to_gfn(req_gpa)); + if (is_error_noslot_pfn(req_pfn)) + return -EINVAL; + + resp_pfn = gfn_to_pfn(kvm, gpa_to_gfn(resp_gpa)); + if (is_error_noslot_pfn(resp_pfn)) { + ret = EINVAL; + goto release_req; + } + + if (rmp_make_private(resp_pfn, 0, PG_LEVEL_4K, 0, true)) { + ret = -EINVAL; + kvm_release_pfn_clean(resp_pfn); + goto release_req; + } + + data.gctx_paddr = __psp_pa(to_kvm_sev_info(kvm)->snp_context); + data.req_paddr = __sme_set(req_pfn << PAGE_SHIFT); + data.res_paddr = __sme_set(resp_pfn << PAGE_SHIFT); + + /* Firmware failures are propagated on to guest. */ + ret = sev_issue_cmd(kvm, SEV_CMD_SNP_GUEST_REQUEST, &data, &fw_err); + if (ret) + pr_debug("%s: guest request failed, ret %d fw_err %d", + __func__, ret, fw_err); + + /* + * If reclaim fails then there's a good chance the guest will no longer + * be runnable so just let userspace terminate the guest. Don't try to + * release the resp_pfn page reference in that case since it is no + * longer usable for future allocations. + */ + if (snp_page_reclaim(kvm, resp_pfn)) { + ret = -EIO; + goto release_req; + } + + /* + * As per GHCB spec, firmware failures should be communicated back to + * the guest via SW_EXITINFO2 rather than be treated as immediately + * fatal. + */ + ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, + SNP_GUEST_ERR(ret ? SNP_GUEST_VMM_ERR_GENERIC : 0, + fw_err)); + + ret = 1; /* resume guest */ + kvm_release_pfn_dirty(resp_pfn); + +release_req: + kvm_release_pfn_clean(req_pfn); + return ret; +} + static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm) { struct vmcb_control_area *control = &svm->vmcb->control; @@ -4213,6 +4283,9 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu) ret = 1; break; + case SVM_VMGEXIT_GUEST_REQUEST: + ret = snp_handle_guest_req(svm, control->exit_info_1, control->exit_info_2); + break; case SVM_VMGEXIT_UNSUPPORTED_EVENT: vcpu_unimpl(vcpu, "vmgexit: unsupported event - exit_info_1=%#llx, exit_info_2=%#llx\n", diff --git a/include/uapi/linux/sev-guest.h b/include/uapi/linux/sev-guest.h index 154a87a1eca9..7bd78e258569 100644 --- a/include/uapi/linux/sev-guest.h +++ b/include/uapi/linux/sev-guest.h @@ -89,8 +89,17 @@ struct snp_ext_report_req { #define SNP_GUEST_FW_ERR_MASK GENMASK_ULL(31, 0) #define SNP_GUEST_VMM_ERR_SHIFT 32 #define SNP_GUEST_VMM_ERR(x) (((u64)x) << SNP_GUEST_VMM_ERR_SHIFT) +#define SNP_GUEST_FW_ERR(x) ((x) & SNP_GUEST_FW_ERR_MASK) +#define SNP_GUEST_ERR(vmm_err, fw_err) (SNP_GUEST_VMM_ERR(vmm_err) | \ + SNP_GUEST_FW_ERR(fw_err)) +/* + * The GHCB spec only formally defines INVALID_LEN/BUSY VMM errors, but define + * a GENERIC error code such that it won't ever conflict with GHCB-defined + * errors if any get added in the future. + */ #define SNP_GUEST_VMM_ERR_INVALID_LEN 1 #define SNP_GUEST_VMM_ERR_BUSY 2 +#define SNP_GUEST_VMM_ERR_GENERIC BIT(31) #endif /* __UAPI_LINUX_SEV_GUEST_H_ */