On 8/27/21 12:44 PM, Borislav Petkov wrote:

On Fri, Aug 20, 2021 at 10:19:28AM -0500, Brijesh Singh wrote:

quoted

+int snp_issue_guest_request(int type, struct snp_guest_request_data *input, unsigned long *fw_err)
+{
+	struct ghcb_state state;
+	unsigned long id, flags;
+	struct ghcb *ghcb;
+	int ret;
+
+	if (!sev_feature_enabled(SEV_SNP))
+		return -ENODEV;
+
+	local_irq_save(flags);
+
+	ghcb = __sev_get_ghcb(&state);
+	if (!ghcb) {
+		ret = -EIO;
+		goto e_restore_irq;
+	}
+
+	vc_ghcb_invalidate(ghcb);
+
+	if (type == GUEST_REQUEST) {
+		id = SVM_VMGEXIT_GUEST_REQUEST;
+	} else if (type == EXT_GUEST_REQUEST) {
+		id = SVM_VMGEXIT_EXT_GUEST_REQUEST;
+		ghcb_set_rax(ghcb, input->data_gpa);
+		ghcb_set_rbx(ghcb, input->data_npages);

Hmmm, now I'm not sure. We did enum psc_op where you simply pass in the
op directly to the hardware because the enum uses the same numbers as
the actual command.

But here that @type thing is simply used to translate to the SVM_VMGEXIT
thing. So maybe you don't need it here and you can hand in the exit_code
directly:

int snp_issue_guest_request(u64 exit_code, struct snp_guest_request_data *input,
			    unsigned long *fw_err)

which you then pass in directly to...

Okay, works for me. The main reason why I choose the enum was to not
expose the GHCB exit code to the driver but I guess that attestation
driver need to know which VMGEXIT need to be called, so, its okay to
have it pass the VMGEXIT number instead of enum.

quoted

+	} else {
+		ret = -EINVAL;
+		goto e_put;
+	}
+
+	ret = sev_es_ghcb_hv_call(ghcb, NULL, id, input->req_gpa, input->resp_gpa);

... this guy here:

	ret = sev_es_ghcb_hv_call(ghcb, NULL, exit_code, input->req_gpa, input->resp_gpa);

quoted

+	if (ret)
+		goto e_put;
+
+	if (ghcb->save.sw_exit_info_2) {
+		/* Number of expected pages are returned in RBX */
+		if (id == EXT_GUEST_REQUEST &&
+		    ghcb->save.sw_exit_info_2 == SNP_GUEST_REQ_INVALID_LEN)
+			input->data_npages = ghcb_get_rbx(ghcb);
+
+		if (fw_err)
+			*fw_err = ghcb->save.sw_exit_info_2;
+
+		ret = -EIO;
+	}
+
+e_put:
+	__sev_put_ghcb(&state);
+e_restore_irq:
+	local_irq_restore(flags);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snp_issue_guest_request);

diff --git a/include/linux/sev-guest.h b/include/linux/sev-guest.h

Why is this a separate header in the include/linux/ namespace?

Is SNP available on something which is !x86, all of a sudden?

So far most of the changes were in x86 specific files. However, the
attestation service is available through a driver to the userspace. The
driver needs to use the VMGEXIT routines provides by the x86 core. I
created the said header file so that driver does not need to include
<asm/sev.h/sev-common.h> etc and will compile for !x86.

quoted

new file mode 100644
index 000000000000..24dd17507789

--- /dev/null
+++ b/include/linux/sev-guest.h

@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * AMD Secure Encrypted Virtualization (SEV) guest driver interface
+ *
+ * Copyright (C) 2021 Advanced Micro Devices, Inc.
+ *
+ * Author: Brijesh Singh <brijesh.singh@amd.com>
+ *
+ */
+
+#ifndef __LINUX_SEV_GUEST_H_
+#define __LINUX_SEV_GUEST_H_
+
+#include <linux/types.h>
+
+enum vmgexit_type {
+	GUEST_REQUEST,
+	EXT_GUEST_REQUEST,
+
+	GUEST_REQUEST_MAX
+};
+
+/*
+ * The error code when the data_npages is too small. The error code
+ * is defined in the GHCB specification.
+ */
+#define SNP_GUEST_REQ_INVALID_LEN	0x100000000ULL

so basically

BIT_ULL(32)

Noted.

quoted

+
+struct snp_guest_request_data {

"snp_req_data" I guess is shorter. And having "guest" in there is
probably not needed because snp is always guest-related anyway.

Noted.

quoted

+	unsigned long req_gpa;
+	unsigned long resp_gpa;
+	unsigned long data_gpa;
+	unsigned int data_npages;
+};
+
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+int snp_issue_guest_request(int vmgexit_type, struct snp_guest_request_data *input,
+			    unsigned long *fw_err);
+#else
+
+static inline int snp_issue_guest_request(int type, struct snp_guest_request_data *input,
+					  unsigned long *fw_err)
+{
+	return -ENODEV;
+}
+
+#endif /* CONFIG_AMD_MEM_ENCRYPT */
+#endif /* __LINUX_SEV_GUEST_H__ */
-- 
2.17.1