On Fri, Jun 21, 2024 at 03:52:35PM +0000, Liam Merwick wrote:
> On 21/06/2024 14:40, Michael Roth wrote:
> > From: Brijesh Singh <brijesh.singh@xxxxxxx>
> >
> > 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@xxxxxxx>
> > Co-developed-by: Alexey Kardashevskiy <aik@xxxxxxx>
> > Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxx>
> > Co-developed-by: Ashish Kalra <ashish.kalra@xxxxxxx>
> > Signed-off-by: Ashish Kalra <ashish.kalra@xxxxxxx>
> > Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> > [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@xxxxxxx>
> > Message-ID: <20240501085210.2213060-19-michael.roth@xxxxxxx>
> > Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > ---
> > arch/x86/kvm/svm/sev.c | 69 ++++++++++++++++++++++++++++++++++
> > include/uapi/linux/sev-guest.h | 9 +++++
> > 2 files changed, 78 insertions(+)
> >
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index df8818759698..7338b987cadd 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,67 @@ 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);
> > +
> > + ret = sev_issue_cmd(kvm, SEV_CMD_SNP_GUEST_REQUEST, &data, &fw_err);
> > + if (ret)
> > + return ret;
>
>
> Should this be goto release_req; ?
> Does resp_pfn need to be dealt with as well?
Argh... yes. I folded some helper functions into here and missed updating
some of the return sites. I'll respond to this patch with a revised version.
Sorry for the noise.
-Mike
