[v8,36/40] x86/sev: Provide support for SNP guest request NAEs

Commit Message

Brijesh Singh Dec. 10, 2021, 3:43 p.m. UTC

Version 2 of GHCB specification provides SNP_GUEST_REQUEST and
SNP_EXT_GUEST_REQUEST NAE that can be used by the SNP guest to communicate
with the PSP.

While at it, add a snp_issue_guest_request() helper that can be used by
driver or other subsystem to issue the request to PSP.

See SEV-SNP and GHCB spec for more details.

Signed-off-by: Brijesh Singh <[e-mail]>
---
 arch/x86/include/asm/sev-common.h |  3 ++
 arch/x86/include/asm/sev.h        | 14 +++++++++
 arch/x86/include/uapi/asm/svm.h   |  4 +++
 arch/x86/kernel/sev.c             | 51 +++++++++++++++++++++++++++++++
 4 files changed, 72 insertions(+)

Comments

Brijesh Singh Jan. 27, 2022, 5:02 p.m. UTC | #1

On 1/27/22 10:21 AM, Borislav Petkov wrote:
> On Fri, Dec 10, 2021 at 09:43:28AM -0600, Brijesh Singh wrote:
>> Version 2 of GHCB specification provides SNP_GUEST_REQUEST and
>> SNP_EXT_GUEST_REQUEST NAE that can be used by the SNP guest to communicate
>> with the PSP.
>>
>> While at it, add a snp_issue_guest_request() helper that can be used by
> 
> Not "that can" but "that will".
> 
Noted.

>>   
>> +/* Guest message request error code */
>> +#define SNP_GUEST_REQ_INVALID_LEN	BIT_ULL(32)
> 
> SZ_4G is more descriptive, perhaps...
> 

I am okay with using SZ_4G but per the spec they don't spell that its 4G 
size. It says bit 32 will should be set on error.



>> +
>> +	ret = sev_es_ghcb_hv_call(ghcb, true, NULL, exit_code, input->req_gpa, input->resp_gpa);
> 					      ^^^^^
> 
> That's ctxt which is accessed without a NULL check in
> verify_exception_info().
> 
> Why aren't you allocating a ctxt on stack like the other callers do?

Typically the sev_es_ghcb_hv_handler() is called from #VC handler, which 
provides the context structure. But in this and PSC case, the caller is 
not a #VC handler, so we don't have a context structure. But as you 
pointed, we could allocate context structure on the stack and pass it 
down so that verify_exception_info() does not cause a panic with NULL 
deference (when HV violates the spec and inject exception while handling 
this NAE).

thanks

Brijesh Singh Jan. 29, 2022, 11:49 a.m. UTC | #2

On 1/29/22 4:27 AM, Borislav Petkov wrote:
> On Thu, Jan 27, 2022 at 11:02:13AM -0600, Brijesh Singh wrote:
>> I am okay with using SZ_4G but per the spec they don't spell that its 4G
>> size. It says bit 32 will should be set on error.
> What does the speck call it exactly? Is it "length"? Because that's what
> confused me: SNP_GUEST_REQ_INVALID_LEN - that's a length and length you
> don't usually specify with a bit position...

Here is the text from the spec:

----------

The hypervisor must validate that the guest has supplied enough pages to
hold the certificates that will be returned before performing the SNP
guest request. If there are not enough guest pages to hold the
certificate table and certificate data, the hypervisor will return the
required number of pages needed to hold the certificate table and
certificate data in the RBX register and set the SW_EXITINFO2 field to
0x0000000100000000.

---------

It does not spell it as invalid length. However, for *similar* failure,
the SEV-SNP spec spells out it as INVALID_LENGTH, so, I choose macro
name as INVALID_LENGTH.

thanks

Patch

diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index 673e6778194b..346600724b84 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -128,6 +128,9 @@  struct snp_psc_desc {
 	struct psc_entry entries[VMGEXIT_PSC_MAX_ENTRY];
 } __packed;
 
+/* Guest message request error code */
+#define SNP_GUEST_REQ_INVALID_LEN	BIT_ULL(32)
+
 #define GHCB_MSR_TERM_REQ		0x100
 #define GHCB_MSR_TERM_REASON_SET_POS	12
 #define GHCB_MSR_TERM_REASON_SET_MASK	0xf
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 76a208fd451b..a47fa0f2547e 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -81,6 +81,14 @@  extern bool handle_vc_boot_ghcb(struct pt_regs *regs);
 
 #define RMPADJUST_VMSA_PAGE_BIT		BIT(16)
 
+/* SNP Guest message request */
+struct snp_req_data {
+	unsigned long req_gpa;
+	unsigned long resp_gpa;
+	unsigned long data_gpa;
+	unsigned int data_npages;
+};
+
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 extern struct static_key_false sev_es_enable_key;
 extern void __sev_es_ist_enter(struct pt_regs *regs);
@@ -148,6 +156,7 @@  void snp_set_memory_private(unsigned long vaddr, unsigned int npages);
 void snp_set_wakeup_secondary_cpu(void);
 bool snp_init(struct boot_params *bp);
 void snp_abort(void);
+int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned long *fw_err);
 #else
 static inline void sev_es_ist_enter(struct pt_regs *regs) { }
 static inline void sev_es_ist_exit(void) { }
@@ -167,6 +176,11 @@  static inline void snp_set_memory_private(unsigned long vaddr, unsigned int npag
 static inline void snp_set_wakeup_secondary_cpu(void) { }
 static inline bool snp_init(struct boot_params *bp) { return false; }
 static inline void snp_abort(void) { }
+static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input,
+					  unsigned long *fw_err)
+{
+	return -ENOTTY;
+}
 #endif
 
 #endif
diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h
index 8b4c57baec52..5b8bc2b65a5e 100644
--- a/arch/x86/include/uapi/asm/svm.h
+++ b/arch/x86/include/uapi/asm/svm.h
@@ -109,6 +109,8 @@ 
 #define SVM_VMGEXIT_SET_AP_JUMP_TABLE		0
 #define SVM_VMGEXIT_GET_AP_JUMP_TABLE		1
 #define SVM_VMGEXIT_PSC				0x80000010
+#define SVM_VMGEXIT_GUEST_REQUEST		0x80000011
+#define SVM_VMGEXIT_EXT_GUEST_REQUEST		0x80000012
 #define SVM_VMGEXIT_AP_CREATION			0x80000013
 #define SVM_VMGEXIT_AP_CREATE_ON_INIT		0
 #define SVM_VMGEXIT_AP_CREATE			1
@@ -225,6 +227,8 @@ 
 	{ SVM_VMGEXIT_AP_HLT_LOOP,	"vmgexit_ap_hlt_loop" }, \
 	{ SVM_VMGEXIT_AP_JUMP_TABLE,	"vmgexit_ap_jump_table" }, \
 	{ SVM_VMGEXIT_PSC,	"vmgexit_page_state_change" }, \
+	{ SVM_VMGEXIT_GUEST_REQUEST,		"vmgexit_guest_request" }, \
+	{ SVM_VMGEXIT_EXT_GUEST_REQUEST,	"vmgexit_ext_guest_request" }, \
 	{ SVM_VMGEXIT_AP_CREATION,	"vmgexit_ap_creation" }, \
 	{ SVM_VMGEXIT_HV_FEATURES,	"vmgexit_hypervisor_feature" }, \
 	{ SVM_EXIT_ERR,         "invalid_guest_state" }
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 70e18b98bb68..289f93e1ab80 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2051,3 +2051,54 @@  static int __init snp_cpuid_check_status(void)
 }
 
 arch_initcall(snp_cpuid_check_status);
+
+int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned long *fw_err)
+{
+	struct ghcb_state state;
+	unsigned long flags;
+	struct ghcb *ghcb;
+	int ret;
+
+	if (!cc_platform_has(CC_ATTR_SEV_SNP))
+		return -ENODEV;
+
+	/* __sev_get_ghcb() need to run with IRQs disabled because it using per-cpu GHCB */
+	local_irq_save(flags);
+
+	ghcb = __sev_get_ghcb(&state);
+	if (!ghcb) {
+		ret = -EIO;
+		goto e_restore_irq;
+	}
+
+	vc_ghcb_invalidate(ghcb);
+
+	if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST) {
+		ghcb_set_rax(ghcb, input->data_gpa);
+		ghcb_set_rbx(ghcb, input->data_npages);
+	}
+
+	ret = sev_es_ghcb_hv_call(ghcb, true, NULL, exit_code, input->req_gpa, input->resp_gpa);
+	if (ret)
+		goto e_put;
+
+	if (ghcb->save.sw_exit_info_2) {
+		/* Number of expected pages are returned in RBX */
+		if (exit_code == SVM_VMGEXIT_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);