[PATCH kernel] KVM: SVM: Fix SVM_VMGEXIT_EXT_GUEST_REQUEST to follow the rest of API

From: Alexey Kardashevskiy
Date: Sun Feb 05 2023 - 22:14:14 EST


When SVM VM is up, KVM uses sev_issue_cmd_external_user() with an open
/dev/sev fd which ensures that the SVM initialization was done correctly.
The only helper not following the scheme is snp_guest_ext_guest_request()
which bypasses the fd check.

Change the SEV API to require passing a file.

Handle errors with care in the SNP Extended Guest Request handler
(snp_handle_ext_guest_request()) as there are actually 3 types of errors:
- @rc: return code SEV device's sev_issue_cmd() which is int==int32;
- @err: a psp return code in sev_issue_cmd(), also int==int32 (probably
a mistake but kvm_sev_cmd::error uses __u32 for some time now);
- (added by this) @exitcode: GHCB's exit code sw_exit_info_2, uint64.

Use the right types, remove cast to int* and return ENOSPC from SEV
device for converting it to the GHCB's exit code
SNP_GUEST_REQ_INVALID_LEN==BIT(32).

Fixes: 17f1d0c995ac ("KVM: SVM: Provide support for SNP_GUEST_REQUEST NAE event")
While at this, preserve the original error in snp_cleanup_guest_buf().

Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxx>
---

This can easily be squashed into what it fixes.

The patch is made for
https://github.com/AMDESE/linux/commits/upmv10-host-snp-v7-rfc
---
include/linux/psp-sev.h | 62 +++++++++++---------
arch/x86/kvm/svm/sev.c | 50 +++++++++++-----
drivers/crypto/ccp/sev-dev.c | 11 ++--
3 files changed, 73 insertions(+), 50 deletions(-)

diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 970a9de0ed20..466b1a6e7d7b 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -848,6 +848,36 @@ int sev_platform_status(struct sev_user_data_status *status, int *error);
int sev_issue_cmd_external_user(struct file *filep, unsigned int id,
void *data, int *error);

+/**
+ * sev_issue_cmd_external_user_cert - issue SEV command by other driver with a file
+ * handle and return certificates set onto SEV device via SNP_SET_EXT_CONFIG;
+ * intended for use by the SNP extended guest request command defined
+ * in the GHCB specification.
+ *
+ * @filep - SEV device file pointer
+ * @cmd - command to issue
+ * @data - command buffer
+ * @vaddr: address where the certificate blob need to be copied.
+ * @npages: number of pages for the certificate blob.
+ * If the specified page count is less than the certificate blob size, then the
+ * required page count is returned with ENOSPC error code.
+ * If the specified page count is more than the certificate blob size, then
+ * page count is updated to reflect the amount of valid data copied in the
+ * vaddr.
+ *
+ * @error: SEV command return code
+ *
+ * Returns:
+ * 0 if the sev successfully processed the command
+ * -%ENODEV if the sev device is not available
+ * -%ENOTSUPP if the sev does not support SEV
+ * -%ETIMEDOUT if the sev command timed out
+ * -%EIO if the sev returned a non-zero return code
+ * -%ENOSPC if the specified page count is too small
+ */
+int sev_issue_cmd_external_user_cert(struct file *filep, unsigned int cmd, void *data,
+ unsigned long vaddr, unsigned long *npages, int *error);
+
/**
* sev_guest_deactivate - perform SEV DEACTIVATE command
*
@@ -945,32 +975,6 @@ void snp_free_firmware_page(void *addr);
*/
void snp_mark_pages_offline(unsigned long pfn, unsigned int npages);

-/**
- * snp_guest_ext_guest_request - perform the SNP extended guest request command
- * defined in the GHCB specification.
- *
- * @data: the input guest request structure
- * @vaddr: address where the certificate blob need to be copied.
- * @npages: number of pages for the certificate blob.
- * If the specified page count is less than the certificate blob size, then the
- * required page count is returned with error code defined in the GHCB spec.
- * If the specified page count is more than the certificate blob size, then
- * page count is updated to reflect the amount of valid data copied in the
- * vaddr.
- *
- * @sev_ret: sev command return code
- *
- * Returns:
- * 0 if the sev successfully processed the command
- * -%ENODEV if the sev device is not available
- * -%ENOTSUPP if the sev does not support SEV
- * -%ETIMEDOUT if the sev command timed out
- * -%EIO if the sev returned a non-zero return code
- */
-int snp_guest_ext_guest_request(struct sev_data_snp_guest_request *data,
- unsigned long vaddr, unsigned long *npages,
- unsigned long *error);
-
#else /* !CONFIG_CRYPTO_DEV_SP_PSP */

static inline int
@@ -1013,9 +1017,9 @@ static inline void *snp_alloc_firmware_page(gfp_t mask)

static inline void snp_free_firmware_page(void *addr) { }

-static inline int snp_guest_ext_guest_request(struct sev_data_snp_guest_request *data,
- unsigned long vaddr, unsigned long *n,
- unsigned long *error)
+static inline int sev_issue_cmd_external_user_cert(struct file *filep, unsigned int cmd,
+ void *data, unsigned long vaddr,
+ unsigned long *npages, int *error)
{
return -ENODEV;
}
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index d0e58cffd1ed..b268c35efab4 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -394,6 +394,23 @@ static int sev_issue_cmd(struct kvm *kvm, int id, void *data, int *error)
return __sev_issue_cmd(sev->fd, id, data, error);
}

+static int sev_issue_cmd_cert(struct kvm *kvm, int id, void *data,
+ unsigned long vaddr, unsigned long *npages, int *error)
+{
+ struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+ struct fd f;
+ int ret;
+
+ f = fdget(sev->fd);
+ if (!f.file)
+ return -EBADF;
+
+ ret = sev_issue_cmd_external_user_cert(f.file, id, data, vaddr, npages, error);
+
+ fdput(f);
+ return ret;
+}
+
static int sev_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
{
struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
@@ -3587,11 +3604,11 @@ static void snp_cleanup_guest_buf(struct sev_data_snp_guest_request *data, unsig
int ret;

ret = snp_page_reclaim(pfn);
- if (ret)
+ if (ret && (*rc == SEV_RET_SUCCESS))
*rc = SEV_RET_INVALID_ADDRESS;

ret = rmp_make_shared(pfn, PG_LEVEL_4K);
- if (ret)
+ if (ret && (*rc == SEV_RET_SUCCESS))
*rc = SEV_RET_INVALID_ADDRESS;
}

@@ -3638,8 +3655,9 @@ static void snp_handle_ext_guest_request(struct vcpu_svm *svm, gpa_t req_gpa, gp
struct kvm *kvm = vcpu->kvm;
unsigned long data_npages;
struct kvm_sev_info *sev;
- unsigned long rc, err;
+ unsigned long exitcode;
u64 data_gpa;
+ int err, rc;

if (!sev_snp_guest(vcpu->kvm)) {
rc = SEV_RET_INVALID_GUEST;
@@ -3669,17 +3687,16 @@ static void snp_handle_ext_guest_request(struct vcpu_svm *svm, gpa_t req_gpa, gp
*/
if (sev->snp_certs_len) {
if ((data_npages << PAGE_SHIFT) < sev->snp_certs_len) {
- rc = -EINVAL;
- err = SNP_GUEST_REQ_INVALID_LEN;
+ rc = -ENOSPC;
goto datalen;
}
- rc = sev_issue_cmd(kvm, SEV_CMD_SNP_GUEST_REQUEST, &req,
- (int *)&err);
+ rc = sev_issue_cmd(kvm, SEV_CMD_SNP_GUEST_REQUEST, &req, &err);
} else {
- rc = snp_guest_ext_guest_request(&req,
- (unsigned long)sev->snp_certs_data,
- &data_npages, &err);
+ rc = sev_issue_cmd_cert(kvm, SEV_CMD_SNP_GUEST_REQUEST, &req,
+ (unsigned long)sev->snp_certs_data,
+ &data_npages, &err);
}
+
datalen:
if (sev->snp_certs_len)
data_npages = sev->snp_certs_len >> PAGE_SHIFT;
@@ -3689,27 +3706,30 @@ static void snp_handle_ext_guest_request(struct vcpu_svm *svm, gpa_t req_gpa, gp
* If buffer length is small then return the expected
* length in rbx.
*/
- if (err == SNP_GUEST_REQ_INVALID_LEN)
+ if (rc == -ENOSPC) {
vcpu->arch.regs[VCPU_REGS_RBX] = data_npages;
+ exitcode = SNP_GUEST_REQ_INVALID_LEN;
+ goto cleanup;
+ }

/* pass the firmware error code */
- rc = err;
+ exitcode = err;
goto cleanup;
}

/* Copy the certificate blob in the guest memory */
if (data_npages &&
kvm_write_guest(kvm, data_gpa, sev->snp_certs_data, data_npages << PAGE_SHIFT))
- rc = SEV_RET_INVALID_ADDRESS;
+ exitcode = SEV_RET_INVALID_ADDRESS;

cleanup:
- snp_cleanup_guest_buf(&req, &rc);
+ snp_cleanup_guest_buf(&req, &exitcode);

unlock:
mutex_unlock(&sev->guest_req_lock);

e_fail:
- svm_set_ghcb_sw_exit_info_2(vcpu, rc);
+ svm_set_ghcb_sw_exit_info_2(vcpu, exitcode);
}

static kvm_pfn_t gfn_to_pfn_restricted(struct kvm *kvm, gfn_t gfn)
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 6c4fdcaed72b..73f56c20255c 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -2070,8 +2070,8 @@ int snp_guest_dbg_decrypt_page(u64 gctx_pfn, u64 src_pfn, u64 dst_pfn, int *erro
}
EXPORT_SYMBOL_GPL(snp_guest_dbg_decrypt_page);

-int snp_guest_ext_guest_request(struct sev_data_snp_guest_request *data,
- unsigned long vaddr, unsigned long *npages, unsigned long *fw_err)
+int sev_issue_cmd_external_user_cert(struct file *filep, unsigned int cmd, void *data,
+ unsigned long vaddr, unsigned long *npages, int *error)
{
unsigned long expected_npages;
struct sev_device *sev;
@@ -2093,12 +2093,11 @@ int snp_guest_ext_guest_request(struct sev_data_snp_guest_request *data,
expected_npages = sev->snp_certs_len >> PAGE_SHIFT;
if (*npages < expected_npages) {
*npages = expected_npages;
- *fw_err = SNP_GUEST_REQ_INVALID_LEN;
mutex_unlock(&sev->snp_certs_lock);
- return -EINVAL;
+ return -ENOSPC;
}

- rc = sev_do_cmd(SEV_CMD_SNP_GUEST_REQUEST, data, (int *)fw_err);
+ rc = sev_issue_cmd_external_user(filep, cmd, data, error);
if (rc) {
mutex_unlock(&sev->snp_certs_lock);
return rc;
@@ -2115,7 +2114,7 @@ int snp_guest_ext_guest_request(struct sev_data_snp_guest_request *data,
mutex_unlock(&sev->snp_certs_lock);
return rc;
}
-EXPORT_SYMBOL_GPL(snp_guest_ext_guest_request);
+EXPORT_SYMBOL_GPL(sev_issue_cmd_external_user_cert);

static void sev_exit(struct kref *ref)
{
--
2.39.1