[PATCH 2/2] virt/coco/sev-guest: Add throttling awareness

Khalid Elmously khalid.elmously at canonical.com
Thu Mar 30 04:56:28 UTC 2023


From: Dionna Glaze <dionnaglaze at google.com>

BugLink: https://bugs.launchpad.net/bugs/2013198

The host is permitted and encouraged to throttle guest requests to the
AMD-SP since it is a shared resource across all VMs. Without
throttling-awareness, the host returning an error will immediately lock
out access to the VMPCK, which makes the VM less useful as it can't
attest itself. Since throttling is expected to be a common occurrence, a
cooperative host can return a VMM error code that the request was
throttled.

The driver interprets the upper 32 bits of exitinfo2 as a VMM error code.
For safety, since the encryption algorithm in GHCBv2 is AES_GCM, control
must remain in the kernel to complete the request with the current
sequence number. Returning without finishing the request allows the the
guest to make another request but with different message contents. This
is IV reuse, and breaks cryptographic protections.

A guest request may not make it to the AMD-SP before the host returns to
the guest, so the err local variable in handle_guest_request must be
initialized the same way fw_err is. snp_issue_guest_request similarly
should set fw_err whether or not the value is non-zero, in order to
appropriately clear the error value when zero.

Cc: Tom Lendacky <Thomas.Lendacky at amd.com>
Cc: Paolo Bonzini <pbonzini at redhat.com>
Cc: Joerg Roedel <jroedel at suse.de>
Cc: Peter Gonda <pgonda at google.com>
Cc: Thomas Gleixner <tglx at linutronix.de>
Cc: Dave Hansen <dave.hansen at linux.intel.com>
Cc: Ingo Molnar <mingo at redhat.com>
Cc: Borislav Petkov <Borislav.Petkov at amd.com>
Cc: "H. Peter Anvin" <hpa at zytor.com>
Cc: Venu Busireddy <venu.busireddy at oracle.com>
Cc: Michael Roth <michael.roth at amd.com>
Cc: "Kirill A. Shutemov" <kirill at shutemov.name>
Cc: Michael Sterritt <sterritt at google.com>

Fixes: d5af44dde546 ("x86/sev: Provide support for SNP guest request
NAEs")

Signed-off-by: Dionna Glaze <dionnaglaze at google.com>
(backported from commit 72f7754dcf31c87c92c0c353dcf747814cc5ce10)
[ kmously: Simplified implementation based on an initial proposed
 version of this patch ]
Signed-off-by: Khalid Elmously <khalid.elmously at canonical.com>
---
 arch/x86/include/asm/sev-common.h     |  3 ++-
 arch/x86/kernel/sev.c                 |  4 +---
 drivers/virt/coco/sevguest/sevguest.c | 11 ++++++++++-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index e6e75f19f6fd9..5c8319798954d 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -126,8 +126,9 @@ struct snp_psc_desc {
 	struct psc_entry entries[VMGEXIT_PSC_MAX_ENTRY];
 } __packed;
 
-/* Guest message request error code */
+/* Guest message request error codes */
 #define SNP_GUEST_REQ_INVALID_LEN	BIT_ULL(32)
+#define SNP_GUEST_REQ_ERR_BUSY		BIT_ULL(33)
 
 #define GHCB_MSR_TERM_REQ		0x100
 #define GHCB_MSR_TERM_REASON_SET_POS	12
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 2a99145fd3aee..db4f84b1f8423 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2208,15 +2208,13 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned
 	if (ret)
 		goto e_put;
 
+	*fw_err = ghcb->save.sw_exit_info_2;
 	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;
 	}
 
diff --git a/drivers/virt/coco/sevguest/sevguest.c b/drivers/virt/coco/sevguest/sevguest.c
index f7991bc920e4d..5f0db4a28a25c 100644
--- a/drivers/virt/coco/sevguest/sevguest.c
+++ b/drivers/virt/coco/sevguest/sevguest.c
@@ -324,7 +324,7 @@ 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 *fw_err)
 {
-	unsigned long err;
+	unsigned long err = 0xff;
 	u64 seqno;
 	int rc;
 
@@ -340,6 +340,7 @@ 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. In this function the encrypted
 	 * message enters shared memory with the host. So after this call the
@@ -348,6 +349,14 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
 	 */
 	rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err);
 
+	/*
+	 * The host may return SNP_GUEST_REQ_ERR_EBUSY if the request has been
+	 * throttled. Retry in the driver to avoid returning and reusing the
+	 * message sequence number on a different message.
+	 */
+	if (err == SNP_GUEST_REQ_ERR_BUSY)
+		goto retry;
+
 	/*
 	 * If the extended guest request fails due to having too small of a
 	 * certificate data buffer, retry the same guest request without the
-- 
2.34.1




More information about the kernel-team mailing list