[PULL,17/19] KVM: SEV: Provide support for SNP_GUEST_REQUEST NAE event

Commit Message

Michael Roth May 10, 2024, 9:10 p.m. UTC

From: Brijesh Singh <[e-mail]>

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 <[e-mail]>
Co-developed-by: Alexey Kardashevskiy <[e-mail]>
Signed-off-by: Alexey Kardashevskiy <[e-mail]>
Co-developed-by: Ashish Kalra <[e-mail]>
Signed-off-by: Ashish Kalra <[e-mail]>
Reviewed-by: Tom Lendacky <[e-mail]>
[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 <[e-mail]>
Message-ID: <[e-mail]>
Signed-off-by: Paolo Bonzini <[e-mail]>
---
 arch/x86/kvm/svm/sev.c         | 86 ++++++++++++++++++++++++++++++++++
 include/uapi/linux/sev-guest.h |  9 ++++
 2 files changed, 95 insertions(+)

Comments

Rick Edgecombe May 17, 2024, 8:41 p.m. UTC | #1

On Fri, 2024-05-10 at 16:10 -0500, Michael Roth wrote:
> +
> +static int __snp_handle_guest_req(struct kvm *kvm, gpa_t req_gpa, gpa_t
> resp_gpa,
> +                                 sev_ret_code *fw_err)
> +{
> +       struct sev_data_snp_guest_request data = {0};
> +       struct kvm_sev_info *sev;
> +       int ret;
> +
> +       if (!sev_snp_guest(kvm))
> +               return -EINVAL;
> +
> +       sev = &to_kvm_svm(kvm)->sev_info;
> +
> +       ret = snp_setup_guest_buf(kvm, &data, req_gpa, resp_gpa);
> +       if (ret)
> +               return ret;
> +
> +       ret = sev_issue_cmd(kvm, SEV_CMD_SNP_GUEST_REQUEST, &data, fw_err);
> +       if (ret)
> +               return ret;
> +
> +       ret = snp_cleanup_guest_buf(&data);
> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}

I get a build error in kvm-coco-queue with W=1:

arch/x86/kvm/svm/sev.c: In function ‘__snp_handle_guest_req’:
arch/x86/kvm/svm/sev.c:3968:30: error: variable ‘sev’ set but not used [-
Werror=unused-but-set-variable]
 3968 |         struct kvm_sev_info *sev;
      |                              ^~~
cc1: all warnings being treated as errors

To fix it:

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 57c2c8025547..6beaa6d42de9 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3965,14 +3965,11 @@ static int __snp_handle_guest_req(struct kvm *kvm, gpa_t
req_gpa, gpa_t resp_gpa
                                  sev_ret_code *fw_err)
 {
        struct sev_data_snp_guest_request data = {0};
-       struct kvm_sev_info *sev;
        int ret;
 
        if (!sev_snp_guest(kvm))
                return -EINVAL;
 
-       sev = &to_kvm_svm(kvm)->sev_info;
-
        ret = snp_setup_guest_buf(kvm, &data, req_gpa, resp_gpa);
        if (ret)
                return ret;

Paolo Bonzini May 17, 2024, 10:01 p.m. UTC | #2

On 5/17/24 22:41, Edgecombe, Rick P wrote:
> I get a build error in kvm-coco-queue with W=1:
> 
> arch/x86/kvm/svm/sev.c: In function ‘__snp_handle_guest_req’:
> arch/x86/kvm/svm/sev.c:3968:30: error: variable ‘sev’ set but not used [-
> Werror=unused-but-set-variable]
>   3968 |         struct kvm_sev_info *sev;
>        |                              ^~~
> cc1: all warnings being treated as errors
> 
> To fix it:
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 57c2c8025547..6beaa6d42de9 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -3965,14 +3965,11 @@ static int __snp_handle_guest_req(struct kvm *kvm, gpa_t
> req_gpa, gpa_t resp_gpa
>                                    sev_ret_code *fw_err)
>   {
>          struct sev_data_snp_guest_request data = {0};
> -       struct kvm_sev_info *sev;
>          int ret;
>   
>          if (!sev_snp_guest(kvm))
>                  return -EINVAL;
>   
> -       sev = &to_kvm_svm(kvm)->sev_info;
> -
>          ret = snp_setup_guest_buf(kvm, &data, req_gpa, resp_gpa);
>          if (ret)
>                  return ret;

I'll post a fully updated version tomorrow with all the pending fixes. 
Or today depending on the timezone.

Paolo

Patch

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index eb397ec22a47..00d29d278f6e 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>
@@ -3292,6 +3293,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;
@@ -3914,6 +3919,83 @@  static int sev_snp_ap_creation(struct vcpu_svm *svm)
 	return ret;
 }
 
+static int snp_setup_guest_buf(struct kvm *kvm, struct sev_data_snp_guest_request *data,
+			       gpa_t req_gpa, gpa_t resp_gpa)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	kvm_pfn_t req_pfn, resp_pfn;
+
+	if (!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))
+		return -EINVAL;
+
+	if (rmp_make_private(resp_pfn, 0, PG_LEVEL_4K, 0, true))
+		return -EINVAL;
+
+	data->gctx_paddr = __psp_pa(sev->snp_context);
+	data->req_paddr = __sme_set(req_pfn << PAGE_SHIFT);
+	data->res_paddr = __sme_set(resp_pfn << PAGE_SHIFT);
+
+	return 0;
+}
+
+static int snp_cleanup_guest_buf(struct sev_data_snp_guest_request *data)
+{
+	u64 pfn = __sme_clr(data->res_paddr) >> PAGE_SHIFT;
+
+	if (snp_page_reclaim(pfn) || rmp_make_shared(pfn, PG_LEVEL_4K))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int __snp_handle_guest_req(struct kvm *kvm, gpa_t req_gpa, gpa_t resp_gpa,
+				  sev_ret_code *fw_err)
+{
+	struct sev_data_snp_guest_request data = {0};
+	struct kvm_sev_info *sev;
+	int ret;
+
+	if (!sev_snp_guest(kvm))
+		return -EINVAL;
+
+	sev = &to_kvm_svm(kvm)->sev_info;
+
+	ret = snp_setup_guest_buf(kvm, &data, req_gpa, resp_gpa);
+	if (ret)
+		return ret;
+
+	ret = sev_issue_cmd(kvm, SEV_CMD_SNP_GUEST_REQUEST, &data, fw_err);
+	if (ret)
+		return ret;
+
+	ret = snp_cleanup_guest_buf(&data);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_gpa)
+{
+	struct kvm_vcpu *vcpu = &svm->vcpu;
+	struct kvm *kvm = vcpu->kvm;
+	sev_ret_code fw_err = 0;
+	int vmm_ret = 0;
+
+	if (__snp_handle_guest_req(kvm, req_gpa, resp_gpa, &fw_err))
+		vmm_ret = SNP_GUEST_VMM_ERR_GENERIC;
+
+	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, SNP_GUEST_ERR(vmm_ret, fw_err));
+}
+
 static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
 {
 	struct vmcb_control_area *control = &svm->vmcb->control;
@@ -4186,6 +4268,10 @@  int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
 			ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_ERR_INVALID_INPUT);
 		}
 
+		ret = 1;
+		break;
+	case SVM_VMGEXIT_GUEST_REQUEST:
+		snp_handle_guest_req(svm, control->exit_info_1, control->exit_info_2);
 		ret = 1;
 		break;
 	case SVM_VMGEXIT_UNSUPPORTED_EVENT:
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_ */