Messages in this thread | | | Date | Mon, 24 Oct 2022 09:22:14 -0500 | Subject | Re: [PATCH v3 4/4] virt/coco/sev-guest: interpret VMM errors from guest request | From | Tom Lendacky <> |
| |
On 10/21/22 11:59, Dionna Glaze wrote: > The GHCB specification states that the upper 32 bits of exitinfo2 are > for the VMM's error codes. The sev-guest ABI has already locked in > that the fw_err status of the input will be 64 bits, and that > BIT_ULL(32) means that the extended guest request's data buffer was too > small, so we have to keep that ABI. > > We can still interpret the upper 32 bits of exitinfo2 for the user > anyway in case the request gets throttled. For safety, since the > encryption algorithm in GHCBv2 is AES_GCM, we cannot return to user > space without having completed the request with the current sequence > number. If we were to return and the guest were to make another request > but with different message contents, then that would be IV reuse. > > When throttled, the driver will reschedule itself and then try > again. The ioctl may block indefinitely, but that has always been the > case when deferring these requests to the host. > > Cc: Tom Lendacky <Thomas.Lendacky@amd.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Joerg Roedel <jroedel@suse.de> > Cc: Peter Gonda <pgonda@google.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > > Signed-off-by: Dionna Glaze <dionnaglaze@google.com> > --- > drivers/virt/coco/sev-guest/sev-guest.c | 32 ++++++++++++++++++++----- > include/uapi/linux/sev-guest.h | 18 ++++++++++++-- > 2 files changed, 42 insertions(+), 8 deletions(-) > > diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c > index 0508c2f46f6b..7abf4c3daa6d 100644 > --- a/drivers/virt/coco/sev-guest/sev-guest.c > +++ b/drivers/virt/coco/sev-guest/sev-guest.c > @@ -305,9 +305,12 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in > u8 type, void *req_buf, size_t req_sz, void *resp_buf, > u32 resp_sz, __u64 *exitinfo2) > { > + unsigned int vmm_err; > u64 seqno; > int rc; > > + might_resched(); > + > /* Get message sequence and verify that its a non-zero */ > seqno = snp_get_msg_seqno(snp_dev); > if (!seqno) > @@ -320,9 +323,26 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in > if (rc) > return rc; > > +retry: > /* Call firmware to process the request */ > rc = snp_issue_guest_request(exit_code, &snp_dev->input, exitinfo2); > > + vmm_err = *exitinfo2 >> SNP_GUEST_VMM_ERR_SHIFT; > + /* > + * The host may return EBUSY if the request has been throttled. > + * We retry in the driver to avoid returning and reusing the message > + * sequence number on a different message. > + */ > + if (vmm_err == SNP_GUEST_VMM_ERR_BUSY) { > + cond_resched();
I would think there should be some form of delay here. Otherwise, it is likely that the request could be re-issued almost immediately, which may make the hypervisor think even more requests are being issued and try to rate limit the guest even more.
Thanks, Tom
> + goto retry; > + } > + > + if (vmm_err && vmm_err != SNP_GUEST_VMM_ERR_INVALID_LEN) { > + pr_err("sev-guest: host returned unknown error code: %d\n", > + vmm_err); > + return -EINVAL; > + } > if (rc) > return rc; > > @@ -375,7 +395,7 @@ static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_io > > rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg->msg_version, > SNP_MSG_REPORT_REQ, &req, sizeof(req), resp->data, > - resp_len, &arg->fw_err); > + resp_len, &arg->exitinfo2); > if (rc) > goto e_free; > > @@ -415,7 +435,7 @@ static int get_derived_key(struct snp_guest_dev *snp_dev, struct snp_guest_reque > > rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg->msg_version, > SNP_MSG_KEY_REQ, &req, sizeof(req), buf, resp_len, > - &arg->fw_err); > + &arg->exitinfo2); > if (rc) > return rc; > > @@ -477,10 +497,10 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques > snp_dev->input.data_npages = npages; > ret = handle_guest_request(snp_dev, SVM_VMGEXIT_EXT_GUEST_REQUEST, arg->msg_version, > SNP_MSG_REPORT_REQ, &req.data, > - sizeof(req.data), resp->data, resp_len, &arg->fw_err); > + sizeof(req.data), resp->data, resp_len, &arg->exitinfo2); > > /* If certs length is invalid then copy the returned length */ > - if (arg->fw_err == SNP_GUEST_REQ_INVALID_LEN) { > + if (arg->vmm_error == SNP_GUEST_VMM_ERR_INVALID_LEN) { > req.certs_len = snp_dev->input.data_npages << PAGE_SHIFT; > > if (copy_to_user((void __user *)arg->req_data, &req, sizeof(req))) > @@ -515,7 +535,7 @@ static long snp_guest_ioctl(struct file *file, unsigned int ioctl, unsigned long > if (copy_from_user(&input, argp, sizeof(input))) > return -EFAULT; > > - input.fw_err = 0xff; > + input.exitinfo2 = SEV_RET_NO_FW_CALL; > > /* Message version must be non-zero */ > if (!input.msg_version) > @@ -546,7 +566,7 @@ static long snp_guest_ioctl(struct file *file, unsigned int ioctl, unsigned long > > mutex_unlock(&snp_cmd_mutex); > > - if (input.fw_err && copy_to_user(argp, &input, sizeof(input))) > + if (input.exitinfo2 && copy_to_user(argp, &input, sizeof(input))) > return -EFAULT; > > return ret; > diff --git a/include/uapi/linux/sev-guest.h b/include/uapi/linux/sev-guest.h > index 256aaeff7e65..8e4144aa78c9 100644 > --- a/include/uapi/linux/sev-guest.h > +++ b/include/uapi/linux/sev-guest.h > @@ -52,8 +52,15 @@ struct snp_guest_request_ioctl { > __u64 req_data; > __u64 resp_data; > > - /* firmware error code on failure (see psp-sev.h) */ > - __u64 fw_err; > + /* bits[63:32]: VMM error code, bits[31:0] firmware error code (see psp-sev.h) */ > + union { > + __u64 exitinfo2; > + __u64 fw_err; /* Name deprecated in favor of others */ > + struct { > + __u32 fw_error; > + __u32 vmm_error; > + }; > + }; > }; > > struct snp_ext_report_req { > @@ -77,4 +84,11 @@ struct snp_ext_report_req { > /* Get SNP extended report as defined in the GHCB specification version 2. */ > #define SNP_GET_EXT_REPORT _IOWR(SNP_GUEST_REQ_IOC_TYPE, 0x2, struct snp_guest_request_ioctl) > > +/* Guest message request EXIT_INFO_2 constants */ > +#define SNP_GUEST_FW_ERR_MASK GENMASK_ULL(31, 0) > +#define SNP_GUEST_VMM_ERR_SHIFT 32 > + > +#define SNP_GUEST_VMM_ERR_INVALID_LEN 1 > +#define SNP_GUEST_VMM_ERR_BUSY 2 > + > #endif /* __UAPI_LINUX_SEV_GUEST_H_ */
| |