2023-03-07 19:38:02

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v3 00/11] SEV: Cleanup sev-guest a bit and add throttling

From: "Borislav Petkov (AMD)" <[email protected]>

From: "Borislav Petkov (AMD)" <[email protected]>

Hi,

ok, here's v3, rebased ontop of -rc1 and hopefully with all the stuff
addressed. There's always room for improvement but that should come
later and ontop.

Thx.

Changelog:
----------

v2:

ok, here's v2 with the pending stuff fixed.

Thx.

v1:
---
so I've been looking at Dionna's patches adding the SEV guest throttling
and that request issuing spaghetti was getting on my nerves. And it
would've become even worse with more stuff piling ontop so here's
a first round of cleanups before adding more stuff and making it an
unmaintainable mess.

The final result is a lot easier to read with proper separation of
functionality between functions. I want to get rid of more input/output
params being passed back'n'forth and use a struct instead and Nikunj's
patches have stuff which goes in that direction but first things first.

After the cleanup, the new stuff being added is a lot less code and
almost trivial. But I've been staring at this for a while now so it
could be only me who thinks it is trivial now. But we'll see.

Initial smoke testing seems to work ok but I might've introduced some
funky bugs, ofc.

Comments and suggestions are appreciated, as always.

Thanks and thanks, Tom, for the help!


Borislav Petkov (AMD) (7):
virt/coco/sev-guest: Check SEV_SNP attribute at probe time
virt/coco/sev-guest: Simplify extended guest request handling
virt/coco/sev-guest: Remove the disable_vmpck label in
handle_guest_request()
virt/coco/sev-guest: Carve out the request issuing logic into a helper
virt/coco/sev-guest: Do some code style cleanups
virt/coco/sev-guest: Convert the sw_exit_info_2 checking to a
switch-case
crypto: ccp: Get rid of __sev_platform_init_locked()'s local function
pointer

Dionna Glaze (3):
virt/coco/sev-guest: Add throttling awareness
virt/coco/sev-guest: Double-buffer messages
x86/sev: Change snp_guest_issue_request()'s fw_err argument

Peter Gonda (1):
crypto: ccp - Name -1 return value as SEV_RET_NO_FW_CALL

Documentation/virt/coco/sev-guest.rst | 20 ++-
arch/x86/include/asm/sev-common.h | 3 -
arch/x86/include/asm/sev.h | 10 +-
arch/x86/kernel/sev.c | 33 ++--
drivers/crypto/ccp/sev-dev.c | 22 ++-
drivers/virt/coco/sev-guest/sev-guest.c | 190 +++++++++++++++---------
include/uapi/linux/psp-sev.h | 7 +
include/uapi/linux/sev-guest.h | 18 ++-
8 files changed, 199 insertions(+), 104 deletions(-)

--
2.35.1



2023-03-07 19:38:12

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v3 02/11] virt/coco/sev-guest: Check SEV_SNP attribute at probe time

From: "Borislav Petkov (AMD)" <[email protected]>

No need to check it on every ioctl. And yes, this is a common SEV driver
but it does only SNP-specific operations currently. This can be
revisited later, when more use cases appear.

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
arch/x86/kernel/sev.c | 3 ---
drivers/virt/coco/sev-guest/sev-guest.c | 3 +++
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 679026a640ef..c644c34372e8 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2183,9 +2183,6 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned
struct ghcb *ghcb;
int ret;

- if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
- return -ENODEV;
-
if (!fw_err)
return -EINVAL;

diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 7b4e9009f335..ed5d6ae1a144 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -703,6 +703,9 @@ static int __init sev_guest_probe(struct platform_device *pdev)
void __iomem *mapping;
int ret;

+ if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
+ return -ENODEV;
+
if (!dev->platform_data)
return -ENODEV;

--
2.35.1


2023-03-07 19:38:15

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v3 04/11] virt/coco/sev-guest: Remove the disable_vmpck label in handle_guest_request()

From: "Borislav Petkov (AMD)" <[email protected]>

Call the function directly instead.

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
drivers/virt/coco/sev-guest/sev-guest.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index e61db0b15b7a..a51bd4afd5ab 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -405,7 +405,8 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
dev_alert(snp_dev->dev,
"Detected error from ASP request. rc: %d, fw_err: %llu\n",
rc, *fw_err);
- goto disable_vmpck;
+ snp_disable_vmpck(snp_dev);
+ return rc;
}

rc = verify_and_dec_payload(snp_dev, resp_buf, resp_sz);
@@ -413,14 +414,11 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
dev_alert(snp_dev->dev,
"Detected unexpected decode failure from ASP. rc: %d\n",
rc);
- goto disable_vmpck;
+ snp_disable_vmpck(snp_dev);
+ return rc;
}

return 0;
-
-disable_vmpck:
- snp_disable_vmpck(snp_dev);
- return rc;
}

static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_ioctl *arg)
--
2.35.1


2023-03-07 19:38:20

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v3 03/11] virt/coco/sev-guest: Simplify extended guest request handling

From: "Borislav Petkov (AMD)" <[email protected]>

Return a specific error code - -ENOSPC - to signal the too small cert
data buffer instead of checking exit code and exitinfo2.

While at it, hoist the *fw_err assignment in snp_issue_guest_request()
so that a proper error value is returned to the callers.

[ Tom: check override_err instead of err. ]

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Signed-off-by: Tom Lendacky <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
arch/x86/kernel/sev.c | 11 +++---
drivers/virt/coco/sev-guest/sev-guest.c | 48 +++++++++++++------------
2 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index c644c34372e8..6a3e1425ba17 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2209,15 +2209,16 @@ 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)
+ ghcb->save.sw_exit_info_2 == SNP_GUEST_REQ_INVALID_LEN) {
input->data_npages = ghcb_get_rbx(ghcb);
-
- *fw_err = ghcb->save.sw_exit_info_2;
-
- ret = -EIO;
+ ret = -ENOSPC;
+ } else {
+ ret = -EIO;
+ }
}

e_put:
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index ed5d6ae1a144..e61db0b15b7a 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -322,7 +322,8 @@ 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, override_err = 0;
+ unsigned int override_npages = 0;
u64 seqno;
int rc;

@@ -338,6 +339,7 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
if (rc)
return rc;

+retry_request:
/*
* Call firmware to process the request. In this function the encrypted
* message enters shared memory with the host. So after this call the
@@ -346,17 +348,24 @@ 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);

- /*
- * If the extended guest request fails due to having too small of a
- * certificate data buffer, retry the same guest request without the
- * extended data request in order to increment the sequence number
- * and thus avoid IV reuse.
- */
- if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST &&
- err == SNP_GUEST_REQ_INVALID_LEN) {
- const unsigned int certs_npages = snp_dev->input.data_npages;
+ switch (rc) {
+ case -ENOSPC:
+ /*
+ * If the extended guest request fails due to having too
+ * small of a certificate data buffer, retry the same
+ * guest request without the extended data request in
+ * order to increment the sequence number and thus avoid
+ * IV reuse.
+ */
+ override_npages = snp_dev->input.data_npages;
+ exit_code = SVM_VMGEXIT_GUEST_REQUEST;

- exit_code = SVM_VMGEXIT_GUEST_REQUEST;
+ /*
+ * Override the error to inform callers the given extended
+ * request buffer size was too small and give the caller the
+ * required buffer size.
+ */
+ override_err = SNP_GUEST_REQ_INVALID_LEN;

/*
* If this call to the firmware succeeds, the sequence number can
@@ -366,15 +375,7 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
* of the VMPCK and the error code being propagated back to the
* user as an ioctl() return code.
*/
- rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err);
-
- /*
- * Override the error to inform callers the given extended
- * request buffer size was too small and give the caller the
- * required buffer size.
- */
- err = SNP_GUEST_REQ_INVALID_LEN;
- snp_dev->input.data_npages = certs_npages;
+ goto retry_request;
}

/*
@@ -386,7 +387,10 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
snp_inc_msg_seqno(snp_dev);

if (fw_err)
- *fw_err = err;
+ *fw_err = override_err ?: err;
+
+ if (override_npages)
+ snp_dev->input.data_npages = override_npages;

/*
* If an extended guest request was issued and the supplied certificate
@@ -394,7 +398,7 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
* prevent IV reuse. If the standard request was successful, return -EIO
* back to the caller as would have originally been returned.
*/
- if (!rc && err == SNP_GUEST_REQ_INVALID_LEN)
+ if (!rc && override_err == SNP_GUEST_REQ_INVALID_LEN)
return -EIO;

if (rc) {
--
2.35.1


2023-03-07 19:38:23

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v3 06/11] virt/coco/sev-guest: Do some code style cleanups

From: "Borislav Petkov (AMD)" <[email protected]>

Remove unnecessary linebreaks, make the code more compact.

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
drivers/virt/coco/sev-guest/sev-guest.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 07dafe22b27a..81a53c31ff46 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -412,18 +412,14 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
if (rc == -EIO && *fw_err == SNP_GUEST_REQ_INVALID_LEN)
return rc;

- dev_alert(snp_dev->dev,
- "Detected error from ASP request. rc: %d, fw_err: %llu\n",
- rc, *fw_err);
+ dev_alert(snp_dev->dev, "Detected error from ASP request. rc: %d, fw_err: %llu\n", rc, *fw_err);
snp_disable_vmpck(snp_dev);
return rc;
}

rc = verify_and_dec_payload(snp_dev, resp_buf, resp_sz);
if (rc) {
- dev_alert(snp_dev->dev,
- "Detected unexpected decode failure from ASP. rc: %d\n",
- rc);
+ dev_alert(snp_dev->dev, "Detected unexpected decode failure from ASP. rc: %d\n", rc);
snp_disable_vmpck(snp_dev);
return rc;
}
--
2.35.1


2023-03-07 19:38:27

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v3 05/11] virt/coco/sev-guest: Carve out the request issuing logic into a helper

From: "Borislav Petkov (AMD)" <[email protected]>

This makes the code flow a lot easier to follow.

No functional changes.

[ Tom: touchups. ]

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Signed-off-by: Tom Lendacky <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
drivers/virt/coco/sev-guest/sev-guest.c | 44 +++++++++++++++----------
1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index a51bd4afd5ab..07dafe22b27a 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -318,27 +318,12 @@ static int enc_payload(struct snp_guest_dev *snp_dev, u64 seqno, int version, u8
return __enc_payload(snp_dev, req, payload, sz);
}

-static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, int msg_ver,
- u8 type, void *req_buf, size_t req_sz, void *resp_buf,
- u32 resp_sz, __u64 *fw_err)
+static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, __u64 *fw_err)
{
unsigned long err, override_err = 0;
unsigned int override_npages = 0;
- u64 seqno;
int rc;

- /* Get message sequence and verify that its a non-zero */
- seqno = snp_get_msg_seqno(snp_dev);
- if (!seqno)
- return -EIO;
-
- memset(snp_dev->response, 0, sizeof(struct snp_guest_msg));
-
- /* Encrypt the userspace provided payload */
- rc = enc_payload(snp_dev, seqno, msg_ver, type, req_buf, req_sz);
- if (rc)
- return rc;
-
retry_request:
/*
* Call firmware to process the request. In this function the encrypted
@@ -347,7 +332,6 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
* prevent reuse of the IV.
*/
rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err);
-
switch (rc) {
case -ENOSPC:
/*
@@ -401,7 +385,33 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
if (!rc && override_err == SNP_GUEST_REQ_INVALID_LEN)
return -EIO;

+ return rc;
+}
+
+static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, int msg_ver,
+ u8 type, void *req_buf, size_t req_sz, void *resp_buf,
+ u32 resp_sz, __u64 *fw_err)
+{
+ u64 seqno;
+ int rc;
+
+ /* Get message sequence and verify that its a non-zero */
+ seqno = snp_get_msg_seqno(snp_dev);
+ if (!seqno)
+ return -EIO;
+
+ memset(snp_dev->response, 0, sizeof(struct snp_guest_msg));
+
+ /* Encrypt the userspace provided payload */
+ rc = enc_payload(snp_dev, seqno, msg_ver, type, req_buf, req_sz);
+ if (rc)
+ return rc;
+
+ rc = __handle_guest_request(snp_dev, exit_code, fw_err);
if (rc) {
+ if (rc == -EIO && *fw_err == SNP_GUEST_REQ_INVALID_LEN)
+ return rc;
+
dev_alert(snp_dev->dev,
"Detected error from ASP request. rc: %d, fw_err: %llu\n",
rc, *fw_err);
--
2.35.1


2023-03-07 19:38:33

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v3 07/11] virt/coco/sev-guest: Convert the sw_exit_info_2 checking to a switch-case

From: "Borislav Petkov (AMD)" <[email protected]>

snp_issue_guest_request() checks the value returned by the hypervisor in
sw_exit_info_2 and returns a different error depending on it.

Convert those checks into a switch-case to make it more readable when
more error values are going to be checked in the future.

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
arch/x86/kernel/sev.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 6a3e1425ba17..d67884fb38c1 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2210,15 +2210,21 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned
goto e_put;

*fw_err = ghcb->save.sw_exit_info_2;
- if (ghcb->save.sw_exit_info_2) {
+ switch (*fw_err) {
+ case 0:
+ break;
+
+ case SNP_GUEST_REQ_INVALID_LEN:
/* 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) {
+ if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST) {
input->data_npages = ghcb_get_rbx(ghcb);
ret = -ENOSPC;
- } else {
- ret = -EIO;
+ break;
}
+ fallthrough;
+ default:
+ ret = -EIO;
+ break;
}

e_put:
--
2.35.1


2023-03-07 19:38:37

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v3 08/11] crypto: ccp: Get rid of __sev_platform_init_locked()'s local function pointer

From: "Borislav Petkov (AMD)" <[email protected]>

Add a wrapper instead.

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
drivers/crypto/ccp/sev-dev.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 823c67a43c38..e346c00b132a 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -442,11 +442,18 @@ static int __sev_init_ex_locked(int *error)
return __sev_do_cmd_locked(SEV_CMD_INIT_EX, &data, error);
}

+static inline int __sev_do_init_locked(int *psp_ret)
+{
+ if (sev_init_ex_buffer)
+ return __sev_init_ex_locked(psp_ret);
+ else
+ return __sev_init_locked(psp_ret);
+}
+
static int __sev_platform_init_locked(int *error)
{
int rc = 0, psp_ret = SEV_RET_NO_FW_CALL;
struct psp_device *psp = psp_master;
- int (*init_function)(int *error);
struct sev_device *sev;

if (!psp || !psp->sev_data)
@@ -458,15 +465,12 @@ static int __sev_platform_init_locked(int *error)
return 0;

if (sev_init_ex_buffer) {
- init_function = __sev_init_ex_locked;
rc = sev_read_init_ex_file();
if (rc)
return rc;
- } else {
- init_function = __sev_init_locked;
}

- rc = init_function(&psp_ret);
+ rc = __sev_do_init_locked(&psp_ret);
if (rc && psp_ret == SEV_RET_SECURE_DATA_INVALID) {
/*
* Initialization command returned an integrity check failure
@@ -477,7 +481,7 @@ static int __sev_platform_init_locked(int *error)
*/
dev_err(sev->dev,
"SEV: retrying INIT command because of SECURE_DATA_INVALID error. Retrying once to reset PSP SEV state.");
- rc = init_function(&psp_ret);
+ rc = __sev_do_init_locked(&psp_ret);
}

if (error)
--
2.35.1


2023-03-07 19:38:41

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v3 09/11] virt/coco/sev-guest: Add throttling awareness

From: Dionna Glaze <[email protected]>

A potentially malicious SEV guest can constantly hammer the hypervisor
using this driver to send down requests and thus prevent or at least
considerably hinder other guests from issuing requests to the secure
processor which is a shared platform resource.

Therefore, the host is permitted and encouraged to throttle such guest
requests.

Add the capability to handle the case when the hypervisor throttles
excessive numbers of requests issued by the guest. Otherwise, the VM
platform communication key will be disabled, preventing the guest from
attesting itself.

Realistically speaking, a well-behaved guest should not even care about
throttling. During its lifetime, it would end up issuing a handful of
requests which the hardware can easily handle.

This is more to address the case of a malicious guest. Such guest should
get throttled and if its VMPCK gets disabled, then that's its own
wrongdoing and perhaps that guest even deserves it.

To the implementation: the hypervisor signals with SNP_GUEST_REQ_ERR_BUSY
that the guest requests should be throttled. That error code is returned
in the upper 32-bit half of exitinfo2 and this is part of the GHCB spec
v2.

So the guest is given a throttling period of 1 minute in which it
retries the request every 2 seconds. This is a good default but if it
turns out to not pan out in practice, it can be tweaked later.

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
guest to make another request but with different message contents. This
is IV reuse, and breaks cryptographic protections.

[ bp: Rewrite commit message and do a simplified version. ]

Fixes: d5af44dde546 ("x86/sev: Provide support for SNP guest request NAEs")
Signed-off-by: Dionna Glaze <[email protected]>
Co-developed-by: Borislav Petkov (AMD) <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/sev-common.h | 3 ++-
arch/x86/kernel/sev.c | 4 ++++
drivers/virt/coco/sev-guest/sev-guest.c | 19 ++++++++++++++++++-
3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index b8357d6ecd47..b63be696b776 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -128,8 +128,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 d67884fb38c1..3f664ab277c4 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2214,6 +2214,10 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned
case 0:
break;

+ case SNP_GUEST_REQ_ERR_BUSY:
+ ret = -EAGAIN;
+ break;
+
case SNP_GUEST_REQ_INVALID_LEN:
/* Number of expected pages are returned in RBX */
if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST) {
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 81a53c31ff46..46f1a8d558b0 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -31,6 +31,9 @@
#define AAD_LEN 48
#define MSG_HDR_VER 1

+#define SNP_REQ_MAX_RETRY_DURATION (60*HZ)
+#define SNP_REQ_RETRY_DELAY (2*HZ)
+
struct snp_guest_crypto {
struct crypto_aead *tfm;
u8 *iv, *authtag;
@@ -320,7 +323,8 @@ static int enc_payload(struct snp_guest_dev *snp_dev, u64 seqno, int version, u8

static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, __u64 *fw_err)
{
- unsigned long err, override_err = 0;
+ unsigned long err = 0xff, override_err = 0;
+ unsigned long req_start = jiffies;
unsigned int override_npages = 0;
int rc;

@@ -360,6 +364,19 @@ static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code,
* user as an ioctl() return code.
*/
goto retry_request;
+
+ /*
+ * 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.
+ */
+ case -EAGAIN:
+ if (jiffies - req_start > SNP_REQ_MAX_RETRY_DURATION) {
+ rc = -ETIMEDOUT;
+ break;
+ }
+ schedule_timeout_killable(SNP_REQ_RETRY_DELAY);
+ goto retry_request;
}

/*
--
2.35.1


2023-03-07 19:38:45

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v3 10/11] virt/coco/sev-guest: Double-buffer messages

From: Dionna Glaze <[email protected]>

The encryption algorithms read and write directly to shared unencrypted
memory, which may leak information as well as permit the host to tamper
with the message integrity. Instead, copy whole messages in or out as
needed before doing any computation on them.

Fixes: d5af44dde546 ("x86/sev: Provide support for SNP guest request NAEs")
Signed-off-by: Dionna Glaze <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/virt/coco/sev-guest/sev-guest.c | 27 +++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 46f1a8d558b0..0c7b47acba2a 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -46,7 +46,15 @@ struct snp_guest_dev {

void *certs_data;
struct snp_guest_crypto *crypto;
+ /* request and response are in unencrypted memory */
struct snp_guest_msg *request, *response;
+
+ /*
+ * Avoid information leakage by double-buffering shared messages
+ * in fields that are in regular encrypted memory.
+ */
+ struct snp_guest_msg secret_request, secret_response;
+
struct snp_secrets_page_layout *layout;
struct snp_req_data input;
u32 *os_area_msg_seqno;
@@ -266,14 +274,17 @@ static int dec_payload(struct snp_guest_dev *snp_dev, struct snp_guest_msg *msg,
static int verify_and_dec_payload(struct snp_guest_dev *snp_dev, void *payload, u32 sz)
{
struct snp_guest_crypto *crypto = snp_dev->crypto;
- struct snp_guest_msg *resp = snp_dev->response;
- struct snp_guest_msg *req = snp_dev->request;
+ struct snp_guest_msg *resp = &snp_dev->secret_response;
+ struct snp_guest_msg *req = &snp_dev->secret_request;
struct snp_guest_msg_hdr *req_hdr = &req->hdr;
struct snp_guest_msg_hdr *resp_hdr = &resp->hdr;

dev_dbg(snp_dev->dev, "response [seqno %lld type %d version %d sz %d]\n",
resp_hdr->msg_seqno, resp_hdr->msg_type, resp_hdr->msg_version, resp_hdr->msg_sz);

+ /* Copy response from shared memory to encrypted memory. */
+ memcpy(resp, snp_dev->response, sizeof(*resp));
+
/* Verify that the sequence counter is incremented by 1 */
if (unlikely(resp_hdr->msg_seqno != (req_hdr->msg_seqno + 1)))
return -EBADMSG;
@@ -297,7 +308,7 @@ static int verify_and_dec_payload(struct snp_guest_dev *snp_dev, void *payload,
static int enc_payload(struct snp_guest_dev *snp_dev, u64 seqno, int version, u8 type,
void *payload, size_t sz)
{
- struct snp_guest_msg *req = snp_dev->request;
+ struct snp_guest_msg *req = &snp_dev->secret_request;
struct snp_guest_msg_hdr *hdr = &req->hdr;

memset(req, 0, sizeof(*req));
@@ -417,13 +428,21 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
if (!seqno)
return -EIO;

+ /* Clear shared memory's response for the host to populate. */
memset(snp_dev->response, 0, sizeof(struct snp_guest_msg));

- /* Encrypt the userspace provided payload */
+ /* Encrypt the userspace provided payload in snp_dev->secret_request. */
rc = enc_payload(snp_dev, seqno, msg_ver, type, req_buf, req_sz);
if (rc)
return rc;

+ /*
+ * Write the fully encrypted request to the shared unencrypted
+ * request page.
+ */
+ memcpy(snp_dev->request, &snp_dev->secret_request,
+ sizeof(snp_dev->secret_request));
+
rc = __handle_guest_request(snp_dev, exit_code, fw_err);
if (rc) {
if (rc == -EIO && *fw_err == SNP_GUEST_REQ_INVALID_LEN)
--
2.35.1


2023-03-07 19:38:50

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v3 11/11] x86/sev: Change snp_guest_issue_request()'s fw_err argument

From: Dionna Glaze <[email protected]>

The GHCB specification declares that the firmware error value for
a guest request will be stored in the lower 32 bits of EXIT_INFO_2. The
upper 32 bits are for the VMM's own error code. The fw_err argument to
snp_guest_issue_request() is thus a misnomer, and callers will need
access to all 64 bits.

The type of unsigned long also causes problems, since sw_exit_info2 is
u64 (unsigned long long) vs the argument's unsigned long*. Change this
type for issuing the guest request. Pass the ioctl command struct's error
field directly instead of in a local variable, since an incomplete guest
request may not set the error code, and uninitialized stack memory would
be written back to user space.

The firmware might not even be called, so bookend the call with the no
firmware call error and clear the error.

Since the "fw_err" field is really exitinfo2 split into the upper bits'
vmm error code and lower bits' firmware error code, convert the 64 bit
value to a union.

[ bp:
- Massage commit message
- adjust code
- Fix a build issue as
Reported-by: kernel test robot <[email protected]>
Link: https://lore.kernel.org/oe-kbuild-all/[email protected]
- print exitinfo2 in hex
Tom:
- Correct -EIO exit case. ]

Signed-off-by: Dionna Glaze <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Signed-off-by: Tom Lendacky <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
Documentation/virt/coco/sev-guest.rst | 20 ++++---
arch/x86/include/asm/sev-common.h | 4 --
arch/x86/include/asm/sev.h | 10 ++--
arch/x86/kernel/sev.c | 15 +++---
drivers/virt/coco/sev-guest/sev-guest.c | 71 +++++++++++++------------
include/uapi/linux/sev-guest.h | 18 ++++++-
6 files changed, 82 insertions(+), 56 deletions(-)

diff --git a/Documentation/virt/coco/sev-guest.rst b/Documentation/virt/coco/sev-guest.rst
index aa3e4c6a1f90..68b0d2363af8 100644
--- a/Documentation/virt/coco/sev-guest.rst
+++ b/Documentation/virt/coco/sev-guest.rst
@@ -37,11 +37,11 @@ along with a description:
the return value. General error numbers (-ENOMEM, -EINVAL)
are not detailed, but errors with specific meanings are.

-The guest ioctl should be issued on a file descriptor of the /dev/sev-guest device.
-The ioctl accepts struct snp_user_guest_request. The input and output structure is
-specified through the req_data and resp_data field respectively. If the ioctl fails
-to execute due to a firmware error, then fw_err code will be set. Otherwise, fw_err
-will be set to 0x00000000ffffffff, i.e., the lower 32-bits are -1.
+The guest ioctl should be issued on a file descriptor of the /dev/sev-guest
+device. The ioctl accepts struct snp_user_guest_request. The input and
+output structure is specified through the req_data and resp_data field
+respectively. If the ioctl fails to execute due to a firmware error, then
+the fw_error code will be set, otherwise fw_error will be set to -1.

The firmware checks that the message sequence counter is one greater than
the guests message sequence counter. If guest driver fails to increment message
@@ -57,8 +57,14 @@ counter (e.g. counter overflow), then -EIO will be returned.
__u64 req_data;
__u64 resp_data;

- /* firmware error code on failure (see psp-sev.h) */
- __u64 fw_err;
+ /* bits[63:32]: VMM error code, bits[31:0] firmware error code (see psp-sev.h) */
+ union {
+ __u64 exitinfo2;
+ struct {
+ __u32 fw_error;
+ __u32 vmm_error;
+ };
+ };
};

2.1 SNP_GET_REPORT
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index b63be696b776..0759af9b1acf 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -128,10 +128,6 @@ struct snp_psc_desc {
struct psc_entry entries[VMGEXIT_PSC_MAX_ENTRY];
} __packed;

-/* 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
#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 ebc271bb6d8e..13dc2a9d23c1 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -9,6 +9,8 @@
#define __ASM_ENCRYPTED_STATE_H

#include <linux/types.h>
+#include <linux/sev-guest.h>
+
#include <asm/insn.h>
#include <asm/sev-common.h>
#include <asm/bootparam.h>
@@ -185,6 +187,9 @@ static inline int pvalidate(unsigned long vaddr, bool rmp_psize, bool validate)

return rc;
}
+
+struct snp_guest_request_ioctl;
+
void setup_ghcb(void);
void __init early_snp_set_memory_private(unsigned long vaddr, unsigned long paddr,
unsigned int npages);
@@ -196,7 +201,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 __init __noreturn snp_abort(void);
-int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned long *fw_err);
+int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio);
#else
static inline void sev_es_ist_enter(struct pt_regs *regs) { }
static inline void sev_es_ist_exit(void) { }
@@ -216,8 +221,7 @@ 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)
+static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio)
{
return -ENOTTY;
}
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 3f664ab277c4..b031244d6d2d 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -22,6 +22,8 @@
#include <linux/efi.h>
#include <linux/platform_device.h>
#include <linux/io.h>
+#include <linux/psp-sev.h>
+#include <uapi/linux/sev-guest.h>

#include <asm/cpu_entry_area.h>
#include <asm/stacktrace.h>
@@ -2175,7 +2177,7 @@ static int __init init_sev_config(char *str)
}
__setup("sev=", init_sev_config);

-int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned long *fw_err)
+int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio)
{
struct ghcb_state state;
struct es_em_ctxt ctxt;
@@ -2183,8 +2185,7 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned
struct ghcb *ghcb;
int ret;

- if (!fw_err)
- return -EINVAL;
+ rio->exitinfo2 = SEV_RET_NO_FW_CALL;

/*
* __sev_get_ghcb() needs to run with IRQs disabled because it is using
@@ -2209,16 +2210,16 @@ 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;
- switch (*fw_err) {
+ rio->exitinfo2 = ghcb->save.sw_exit_info_2;
+ switch (rio->exitinfo2) {
case 0:
break;

- case SNP_GUEST_REQ_ERR_BUSY:
+ case SNP_GUEST_VMM_ERR(SNP_GUEST_VMM_ERR_BUSY):
ret = -EAGAIN;
break;

- case SNP_GUEST_REQ_INVALID_LEN:
+ case SNP_GUEST_VMM_ERR(SNP_GUEST_VMM_ERR_INVALID_LEN):
/* Number of expected pages are returned in RBX */
if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST) {
input->data_npages = ghcb_get_rbx(ghcb);
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 0c7b47acba2a..2764ede9b256 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -332,11 +332,12 @@ static int enc_payload(struct snp_guest_dev *snp_dev, u64 seqno, int version, u8
return __enc_payload(snp_dev, req, payload, sz);
}

-static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, __u64 *fw_err)
+static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code,
+ struct snp_guest_request_ioctl *rio)
{
- unsigned long err = 0xff, override_err = 0;
unsigned long req_start = jiffies;
unsigned int override_npages = 0;
+ u64 override_err = 0;
int rc;

retry_request:
@@ -346,7 +347,7 @@ static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code,
* sequence number must be incremented or the VMPCK must be deleted to
* prevent reuse of the IV.
*/
- rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err);
+ rc = snp_issue_guest_request(exit_code, &snp_dev->input, rio);
switch (rc) {
case -ENOSPC:
/*
@@ -364,7 +365,7 @@ static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code,
* request buffer size was too small and give the caller the
* required buffer size.
*/
- override_err = SNP_GUEST_REQ_INVALID_LEN;
+ override_err = SNP_GUEST_VMM_ERR(SNP_GUEST_VMM_ERR_INVALID_LEN);

/*
* If this call to the firmware succeeds, the sequence number can
@@ -377,7 +378,7 @@ static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code,
goto retry_request;

/*
- * The host may return SNP_GUEST_REQ_ERR_EBUSY if the request has been
+ * The host may return SNP_GUEST_VMM_ERR_BUSY if the request has been
* throttled. Retry in the driver to avoid returning and reusing the
* message sequence number on a different message.
*/
@@ -398,27 +399,28 @@ static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code,
*/
snp_inc_msg_seqno(snp_dev);

- if (fw_err)
- *fw_err = override_err ?: err;
+ if (override_err) {
+ rio->exitinfo2 = override_err;
+
+ /* If an extended guest request was issued and the supplied certificate
+ * buffer was not large enough, a standard guest request was issued to
+ * prevent IV reuse. If the standard request was successful, return -EIO
+ * back to the caller as would have originally been returned.
+ */
+ if (!rc && override_err == SNP_GUEST_VMM_ERR(SNP_GUEST_VMM_ERR_INVALID_LEN))
+ rc = -EIO;
+ }

if (override_npages)
snp_dev->input.data_npages = override_npages;

- /*
- * If an extended guest request was issued and the supplied certificate
- * buffer was not large enough, a standard guest request was issued to
- * prevent IV reuse. If the standard request was successful, return -EIO
- * back to the caller as would have originally been returned.
- */
- if (!rc && override_err == SNP_GUEST_REQ_INVALID_LEN)
- return -EIO;
-
return rc;
}

-static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, int msg_ver,
- u8 type, void *req_buf, size_t req_sz, void *resp_buf,
- u32 resp_sz, __u64 *fw_err)
+static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code,
+ struct snp_guest_request_ioctl *rio, u8 type,
+ void *req_buf, size_t req_sz, void *resp_buf,
+ u32 resp_sz)
{
u64 seqno;
int rc;
@@ -432,7 +434,7 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
memset(snp_dev->response, 0, sizeof(struct snp_guest_msg));

/* Encrypt the userspace provided payload in snp_dev->secret_request. */
- rc = enc_payload(snp_dev, seqno, msg_ver, type, req_buf, req_sz);
+ rc = enc_payload(snp_dev, seqno, rio->msg_version, type, req_buf, req_sz);
if (rc)
return rc;

@@ -443,12 +445,16 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
memcpy(snp_dev->request, &snp_dev->secret_request,
sizeof(snp_dev->secret_request));

- rc = __handle_guest_request(snp_dev, exit_code, fw_err);
+ rc = __handle_guest_request(snp_dev, exit_code, rio);
if (rc) {
- if (rc == -EIO && *fw_err == SNP_GUEST_REQ_INVALID_LEN)
+ if (rc == -EIO &&
+ rio->exitinfo2 == SNP_GUEST_VMM_ERR(SNP_GUEST_VMM_ERR_INVALID_LEN))
return rc;

- dev_alert(snp_dev->dev, "Detected error from ASP request. rc: %d, fw_err: %llu\n", rc, *fw_err);
+ dev_alert(snp_dev->dev,
+ "Detected error from ASP request. rc: %d, exitinfo2: 0x%llx\n",
+ rc, rio->exitinfo2);
+
snp_disable_vmpck(snp_dev);
return rc;
}
@@ -488,9 +494,9 @@ static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_io
if (!resp)
return -ENOMEM;

- rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg->msg_version,
+ rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg,
SNP_MSG_REPORT_REQ, &req, sizeof(req), resp->data,
- resp_len, &arg->fw_err);
+ resp_len);
if (rc)
goto e_free;

@@ -528,9 +534,8 @@ static int get_derived_key(struct snp_guest_dev *snp_dev, struct snp_guest_reque
if (copy_from_user(&req, (void __user *)arg->req_data, sizeof(req)))
return -EFAULT;

- rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg->msg_version,
- SNP_MSG_KEY_REQ, &req, sizeof(req), buf, resp_len,
- &arg->fw_err);
+ rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg,
+ SNP_MSG_KEY_REQ, &req, sizeof(req), buf, resp_len);
if (rc)
return rc;

@@ -590,12 +595,12 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
return -ENOMEM;

snp_dev->input.data_npages = npages;
- ret = handle_guest_request(snp_dev, SVM_VMGEXIT_EXT_GUEST_REQUEST, arg->msg_version,
+ ret = handle_guest_request(snp_dev, SVM_VMGEXIT_EXT_GUEST_REQUEST, arg,
SNP_MSG_REPORT_REQ, &req.data,
- sizeof(req.data), resp->data, resp_len, &arg->fw_err);
+ sizeof(req.data), resp->data, resp_len);

/* If certs length is invalid then copy the returned length */
- if (arg->fw_err == SNP_GUEST_REQ_INVALID_LEN) {
+ if (arg->vmm_error == SNP_GUEST_VMM_ERR_INVALID_LEN) {
req.certs_len = snp_dev->input.data_npages << PAGE_SHIFT;

if (copy_to_user((void __user *)arg->req_data, &req, sizeof(req)))
@@ -630,7 +635,7 @@ static long snp_guest_ioctl(struct file *file, unsigned int ioctl, unsigned long
if (copy_from_user(&input, argp, sizeof(input)))
return -EFAULT;

- input.fw_err = 0xff;
+ input.exitinfo2 = 0xff;

/* Message version must be non-zero */
if (!input.msg_version)
@@ -661,7 +666,7 @@ static long snp_guest_ioctl(struct file *file, unsigned int ioctl, unsigned long

mutex_unlock(&snp_cmd_mutex);

- if (input.fw_err && copy_to_user(argp, &input, sizeof(input)))
+ if (input.exitinfo2 && copy_to_user(argp, &input, sizeof(input)))
return -EFAULT;

return ret;
diff --git a/include/uapi/linux/sev-guest.h b/include/uapi/linux/sev-guest.h
index 256aaeff7e65..2aa39112cf8d 100644
--- a/include/uapi/linux/sev-guest.h
+++ b/include/uapi/linux/sev-guest.h
@@ -52,8 +52,14 @@ struct snp_guest_request_ioctl {
__u64 req_data;
__u64 resp_data;

- /* firmware error code on failure (see psp-sev.h) */
- __u64 fw_err;
+ /* bits[63:32]: VMM error code, bits[31:0] firmware error code (see psp-sev.h) */
+ union {
+ __u64 exitinfo2;
+ struct {
+ __u32 fw_error;
+ __u32 vmm_error;
+ };
+ };
};

struct snp_ext_report_req {
@@ -77,4 +83,12 @@ struct snp_ext_report_req {
/* Get SNP extended report as defined in the GHCB specification version 2. */
#define SNP_GET_EXT_REPORT _IOWR(SNP_GUEST_REQ_IOC_TYPE, 0x2, struct snp_guest_request_ioctl)

+/* Guest message request EXIT_INFO_2 constants */
+#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_VMM_ERR_INVALID_LEN 1
+#define SNP_GUEST_VMM_ERR_BUSY 2
+
#endif /* __UAPI_LINUX_SEV_GUEST_H_ */
--
2.35.1


2023-03-07 20:28:36

by Dionna Amalie Glaze

[permalink] [raw]
Subject: Re: [PATCH -v3 09/11] virt/coco/sev-guest: Add throttling awareness

> @@ -320,7 +323,8 @@ static int enc_payload(struct snp_guest_dev *snp_dev, u64 seqno, int version, u8
>
> static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, __u64 *fw_err)
> {
> - unsigned long err, override_err = 0;
> + unsigned long err = 0xff, override_err = 0;
> + unsigned long req_start = jiffies;
> unsigned int override_npages = 0;
> int rc;
>

Not blocking, but I do think in the previous version we discussed
changing this to the SEV_RET_NO_FW_CALL value instead of 0xff. I agree
that should be a later cleanup.

--
-Dionna Glaze, PhD (she/her)

2023-03-08 20:34:33

by Tom Lendacky

[permalink] [raw]
Subject: Re: [PATCH -v3 00/11] SEV: Cleanup sev-guest a bit and add throttling

On 3/7/23 13:24, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <[email protected]>
>
> From: "Borislav Petkov (AMD)" <[email protected]>
>
> Hi,
>
> ok, here's v3, rebased ontop of -rc1 and hopefully with all the stuff
> addressed. There's always room for improvement but that should come
> later and ontop.
>
> Thx.

For the series:
Reviewed-by: Tom Lendacky <[email protected]>

One thing I noticed is that patch #3 fixes an issue where the fw_err field
returns uninitialized stack data when success (0) is returned by the
ioctl(). There's no Fixes: tag on that commit. But I guess the later
Fixes: tag on patch #9 would cause #3 to be pulled, too, right?

Thanks,
Tom

>
> Changelog:
> ----------
>
> v2:
>
> ok, here's v2 with the pending stuff fixed.
>
> Thx.
>
> v1:
> ---
> so I've been looking at Dionna's patches adding the SEV guest throttling
> and that request issuing spaghetti was getting on my nerves. And it
> would've become even worse with more stuff piling ontop so here's
> a first round of cleanups before adding more stuff and making it an
> unmaintainable mess.
>
> The final result is a lot easier to read with proper separation of
> functionality between functions. I want to get rid of more input/output
> params being passed back'n'forth and use a struct instead and Nikunj's
> patches have stuff which goes in that direction but first things first.
>
> After the cleanup, the new stuff being added is a lot less code and
> almost trivial. But I've been staring at this for a while now so it
> could be only me who thinks it is trivial now. But we'll see.
>
> Initial smoke testing seems to work ok but I might've introduced some
> funky bugs, ofc.
>
> Comments and suggestions are appreciated, as always.
>
> Thanks and thanks, Tom, for the help!
>
>
> Borislav Petkov (AMD) (7):
> virt/coco/sev-guest: Check SEV_SNP attribute at probe time
> virt/coco/sev-guest: Simplify extended guest request handling
> virt/coco/sev-guest: Remove the disable_vmpck label in
> handle_guest_request()
> virt/coco/sev-guest: Carve out the request issuing logic into a helper
> virt/coco/sev-guest: Do some code style cleanups
> virt/coco/sev-guest: Convert the sw_exit_info_2 checking to a
> switch-case
> crypto: ccp: Get rid of __sev_platform_init_locked()'s local function
> pointer
>
> Dionna Glaze (3):
> virt/coco/sev-guest: Add throttling awareness
> virt/coco/sev-guest: Double-buffer messages
> x86/sev: Change snp_guest_issue_request()'s fw_err argument
>
> Peter Gonda (1):
> crypto: ccp - Name -1 return value as SEV_RET_NO_FW_CALL
>
> Documentation/virt/coco/sev-guest.rst | 20 ++-
> arch/x86/include/asm/sev-common.h | 3 -
> arch/x86/include/asm/sev.h | 10 +-
> arch/x86/kernel/sev.c | 33 ++--
> drivers/crypto/ccp/sev-dev.c | 22 ++-
> drivers/virt/coco/sev-guest/sev-guest.c | 190 +++++++++++++++---------
> include/uapi/linux/psp-sev.h | 7 +
> include/uapi/linux/sev-guest.h | 18 ++-
> 8 files changed, 199 insertions(+), 104 deletions(-)
>

2023-03-09 12:12:17

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH -v3 00/11] SEV: Cleanup sev-guest a bit and add throttling

On Wed, Mar 08, 2023 at 02:33:58PM -0600, Tom Lendacky wrote:
> One thing I noticed is that patch #3 fixes an issue where the fw_err field
> returns uninitialized stack data when success (0) is returned by the
> ioctl(). There's no Fixes: tag on that commit. But I guess the later Fixes:
> tag on patch #9 would cause #3 to be pulled, too, right?

Yeah, that's the plan...

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2023-03-13 12:58:19

by tip-bot2 for Tony Luck

[permalink] [raw]
Subject: [tip: x86/urgent] virt/coco/sev-guest: Remove the disable_vmpck label in handle_guest_request()

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: c5a338274bdb894f088767bea856be344d0ccaef
Gitweb: https://git.kernel.org/tip/c5a338274bdb894f088767bea856be344d0ccaef
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Wed, 15 Feb 2023 11:43:43 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Mon, 13 Mar 2023 11:33:41 +01:00

virt/coco/sev-guest: Remove the disable_vmpck label in handle_guest_request()

Call the function directly instead.

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Tom Lendacky <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/virt/coco/sev-guest/sev-guest.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index e61db0b..a51bd4a 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -405,7 +405,8 @@ retry_request:
dev_alert(snp_dev->dev,
"Detected error from ASP request. rc: %d, fw_err: %llu\n",
rc, *fw_err);
- goto disable_vmpck;
+ snp_disable_vmpck(snp_dev);
+ return rc;
}

rc = verify_and_dec_payload(snp_dev, resp_buf, resp_sz);
@@ -413,14 +414,11 @@ retry_request:
dev_alert(snp_dev->dev,
"Detected unexpected decode failure from ASP. rc: %d\n",
rc);
- goto disable_vmpck;
+ snp_disable_vmpck(snp_dev);
+ return rc;
}

return 0;
-
-disable_vmpck:
- snp_disable_vmpck(snp_dev);
- return rc;
}

static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_ioctl *arg)

2023-03-13 12:58:21

by tip-bot2 for Tony Luck

[permalink] [raw]
Subject: [tip: x86/urgent] virt/coco/sev-guest: Do some code style cleanups

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: d25bae7dc7b0668cb2a1325c64eb32d5fea4e5a9
Gitweb: https://git.kernel.org/tip/d25bae7dc7b0668cb2a1325c64eb32d5fea4e5a9
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Wed, 15 Feb 2023 11:54:59 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Mon, 13 Mar 2023 12:47:55 +01:00

virt/coco/sev-guest: Do some code style cleanups

Remove unnecessary linebreaks, make the code more compact.

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Tom Lendacky <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/virt/coco/sev-guest/sev-guest.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 07dafe2..81a53c3 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -412,18 +412,14 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
if (rc == -EIO && *fw_err == SNP_GUEST_REQ_INVALID_LEN)
return rc;

- dev_alert(snp_dev->dev,
- "Detected error from ASP request. rc: %d, fw_err: %llu\n",
- rc, *fw_err);
+ dev_alert(snp_dev->dev, "Detected error from ASP request. rc: %d, fw_err: %llu\n", rc, *fw_err);
snp_disable_vmpck(snp_dev);
return rc;
}

rc = verify_and_dec_payload(snp_dev, resp_buf, resp_sz);
if (rc) {
- dev_alert(snp_dev->dev,
- "Detected unexpected decode failure from ASP. rc: %d\n",
- rc);
+ dev_alert(snp_dev->dev, "Detected unexpected decode failure from ASP. rc: %d\n", rc);
snp_disable_vmpck(snp_dev);
return rc;
}

2023-03-13 12:58:25

by tip-bot2 for Tony Luck

[permalink] [raw]
Subject: [tip: x86/urgent] virt/coco/sev-guest: Convert the sw_exit_info_2 checking to a switch-case

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: fa4ae42cc60a7dea30e8f2db444b808d80862345
Gitweb: https://git.kernel.org/tip/fa4ae42cc60a7dea30e8f2db444b808d80862345
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Thu, 16 Feb 2023 10:50:11 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Mon, 13 Mar 2023 12:55:34 +01:00

virt/coco/sev-guest: Convert the sw_exit_info_2 checking to a switch-case

snp_issue_guest_request() checks the value returned by the hypervisor in
sw_exit_info_2 and returns a different error depending on it.

Convert those checks into a switch-case to make it more readable when
more error values are going to be checked in the future.

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Tom Lendacky <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/sev.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 6a3e142..d67884f 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2210,15 +2210,21 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned
goto e_put;

*fw_err = ghcb->save.sw_exit_info_2;
- if (ghcb->save.sw_exit_info_2) {
+ switch (*fw_err) {
+ case 0:
+ break;
+
+ case SNP_GUEST_REQ_INVALID_LEN:
/* 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) {
+ if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST) {
input->data_npages = ghcb_get_rbx(ghcb);
ret = -ENOSPC;
- } else {
- ret = -EIO;
+ break;
}
+ fallthrough;
+ default:
+ ret = -EIO;
+ break;
}

e_put:

2023-03-13 12:58:32

by tip-bot2 for Tony Luck

[permalink] [raw]
Subject: [tip: x86/urgent] virt/coco/sev-guest: Carve out the request issuing logic into a helper

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: 0fdb6cc7c89cb5e0cbc45dbdbafb8e3fb92ddc95
Gitweb: https://git.kernel.org/tip/0fdb6cc7c89cb5e0cbc45dbdbafb8e3fb92ddc95
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Tue, 07 Mar 2023 09:19:19 -06:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Mon, 13 Mar 2023 12:35:02 +01:00

virt/coco/sev-guest: Carve out the request issuing logic into a helper

This makes the code flow a lot easier to follow.

No functional changes.

[ Tom: touchups. ]

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Signed-off-by: Tom Lendacky <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/virt/coco/sev-guest/sev-guest.c | 44 ++++++++++++++----------
1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index a51bd4a..07dafe2 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -318,27 +318,12 @@ static int enc_payload(struct snp_guest_dev *snp_dev, u64 seqno, int version, u8
return __enc_payload(snp_dev, req, payload, sz);
}

-static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, int msg_ver,
- u8 type, void *req_buf, size_t req_sz, void *resp_buf,
- u32 resp_sz, __u64 *fw_err)
+static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, __u64 *fw_err)
{
unsigned long err, override_err = 0;
unsigned int override_npages = 0;
- u64 seqno;
int rc;

- /* Get message sequence and verify that its a non-zero */
- seqno = snp_get_msg_seqno(snp_dev);
- if (!seqno)
- return -EIO;
-
- memset(snp_dev->response, 0, sizeof(struct snp_guest_msg));
-
- /* Encrypt the userspace provided payload */
- rc = enc_payload(snp_dev, seqno, msg_ver, type, req_buf, req_sz);
- if (rc)
- return rc;
-
retry_request:
/*
* Call firmware to process the request. In this function the encrypted
@@ -347,7 +332,6 @@ retry_request:
* prevent reuse of the IV.
*/
rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err);
-
switch (rc) {
case -ENOSPC:
/*
@@ -401,7 +385,33 @@ retry_request:
if (!rc && override_err == SNP_GUEST_REQ_INVALID_LEN)
return -EIO;

+ return rc;
+}
+
+static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, int msg_ver,
+ u8 type, void *req_buf, size_t req_sz, void *resp_buf,
+ u32 resp_sz, __u64 *fw_err)
+{
+ u64 seqno;
+ int rc;
+
+ /* Get message sequence and verify that its a non-zero */
+ seqno = snp_get_msg_seqno(snp_dev);
+ if (!seqno)
+ return -EIO;
+
+ memset(snp_dev->response, 0, sizeof(struct snp_guest_msg));
+
+ /* Encrypt the userspace provided payload */
+ rc = enc_payload(snp_dev, seqno, msg_ver, type, req_buf, req_sz);
+ if (rc)
+ return rc;
+
+ rc = __handle_guest_request(snp_dev, exit_code, fw_err);
if (rc) {
+ if (rc == -EIO && *fw_err == SNP_GUEST_REQ_INVALID_LEN)
+ return rc;
+
dev_alert(snp_dev->dev,
"Detected error from ASP request. rc: %d, fw_err: %llu\n",
rc, *fw_err);

2023-03-13 12:58:37

by tip-bot2 for Tony Luck

[permalink] [raw]
Subject: [tip: x86/urgent] virt/coco/sev-guest: Check SEV_SNP attribute at probe time

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: d6fd48eff7506bb866a54e40369df8899f2078a9
Gitweb: https://git.kernel.org/tip/d6fd48eff7506bb866a54e40369df8899f2078a9
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Wed, 15 Feb 2023 11:01:42 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Mon, 13 Mar 2023 11:20:20 +01:00

virt/coco/sev-guest: Check SEV_SNP attribute at probe time

No need to check it on every ioctl. And yes, this is a common SEV driver
but it does only SNP-specific operations currently. This can be
revisited later, when more use cases appear.

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Tom Lendacky <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/sev.c | 3 ---
drivers/virt/coco/sev-guest/sev-guest.c | 3 +++
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 679026a..c644c34 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2183,9 +2183,6 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned
struct ghcb *ghcb;
int ret;

- if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
- return -ENODEV;
-
if (!fw_err)
return -EINVAL;

diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 7b4e900..ed5d6ae 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -703,6 +703,9 @@ static int __init sev_guest_probe(struct platform_device *pdev)
void __iomem *mapping;
int ret;

+ if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
+ return -ENODEV;
+
if (!dev->platform_data)
return -ENODEV;


2023-03-13 12:58:47

by tip-bot2 for Tony Luck

[permalink] [raw]
Subject: [tip: x86/urgent] virt/coco/sev-guest: Simplify extended guest request handling

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: 970ab823743fb54b42002ec76c51481f67436444
Gitweb: https://git.kernel.org/tip/970ab823743fb54b42002ec76c51481f67436444
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Wed, 15 Feb 2023 11:39:41 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Mon, 13 Mar 2023 11:27:10 +01:00

virt/coco/sev-guest: Simplify extended guest request handling

Return a specific error code - -ENOSPC - to signal the too small cert
data buffer instead of checking exit code and exitinfo2.

While at it, hoist the *fw_err assignment in snp_issue_guest_request()
so that a proper error value is returned to the callers.

[ Tom: check override_err instead of err. ]

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Signed-off-by: Tom Lendacky <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/sev.c | 11 +++---
drivers/virt/coco/sev-guest/sev-guest.c | 48 +++++++++++++-----------
2 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index c644c34..6a3e142 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2209,15 +2209,16 @@ 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)
+ ghcb->save.sw_exit_info_2 == SNP_GUEST_REQ_INVALID_LEN) {
input->data_npages = ghcb_get_rbx(ghcb);
-
- *fw_err = ghcb->save.sw_exit_info_2;
-
- ret = -EIO;
+ ret = -ENOSPC;
+ } else {
+ ret = -EIO;
+ }
}

e_put:
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index ed5d6ae..e61db0b 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -322,7 +322,8 @@ 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, override_err = 0;
+ unsigned int override_npages = 0;
u64 seqno;
int rc;

@@ -338,6 +339,7 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
if (rc)
return rc;

+retry_request:
/*
* Call firmware to process the request. In this function the encrypted
* message enters shared memory with the host. So after this call the
@@ -346,17 +348,24 @@ 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);

- /*
- * If the extended guest request fails due to having too small of a
- * certificate data buffer, retry the same guest request without the
- * extended data request in order to increment the sequence number
- * and thus avoid IV reuse.
- */
- if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST &&
- err == SNP_GUEST_REQ_INVALID_LEN) {
- const unsigned int certs_npages = snp_dev->input.data_npages;
+ switch (rc) {
+ case -ENOSPC:
+ /*
+ * If the extended guest request fails due to having too
+ * small of a certificate data buffer, retry the same
+ * guest request without the extended data request in
+ * order to increment the sequence number and thus avoid
+ * IV reuse.
+ */
+ override_npages = snp_dev->input.data_npages;
+ exit_code = SVM_VMGEXIT_GUEST_REQUEST;

- exit_code = SVM_VMGEXIT_GUEST_REQUEST;
+ /*
+ * Override the error to inform callers the given extended
+ * request buffer size was too small and give the caller the
+ * required buffer size.
+ */
+ override_err = SNP_GUEST_REQ_INVALID_LEN;

/*
* If this call to the firmware succeeds, the sequence number can
@@ -366,15 +375,7 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
* of the VMPCK and the error code being propagated back to the
* user as an ioctl() return code.
*/
- rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err);
-
- /*
- * Override the error to inform callers the given extended
- * request buffer size was too small and give the caller the
- * required buffer size.
- */
- err = SNP_GUEST_REQ_INVALID_LEN;
- snp_dev->input.data_npages = certs_npages;
+ goto retry_request;
}

/*
@@ -386,7 +387,10 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
snp_inc_msg_seqno(snp_dev);

if (fw_err)
- *fw_err = err;
+ *fw_err = override_err ?: err;
+
+ if (override_npages)
+ snp_dev->input.data_npages = override_npages;

/*
* If an extended guest request was issued and the supplied certificate
@@ -394,7 +398,7 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
* prevent IV reuse. If the standard request was successful, return -EIO
* back to the caller as would have originally been returned.
*/
- if (!rc && err == SNP_GUEST_REQ_INVALID_LEN)
+ if (!rc && override_err == SNP_GUEST_REQ_INVALID_LEN)
return -EIO;

if (rc) {

2023-03-21 15:28:59

by tip-bot2 for Tony Luck

[permalink] [raw]
Subject: [tip: x86/sev] crypto: ccp: Get rid of __sev_platform_init_locked()'s local function pointer

The following commit has been merged into the x86/sev branch of tip:

Commit-ID: dbf07b544ca12c0ab8bd3fc1ea3509ea713a8bf5
Gitweb: https://git.kernel.org/tip/dbf07b544ca12c0ab8bd3fc1ea3509ea713a8bf5
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Tue, 07 Mar 2023 20:24:46 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Tue, 21 Mar 2023 12:08:07 +01:00

crypto: ccp: Get rid of __sev_platform_init_locked()'s local function pointer

Add a wrapper instead.

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/crypto/ccp/sev-dev.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 823c67a..e346c00 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -442,11 +442,18 @@ static int __sev_init_ex_locked(int *error)
return __sev_do_cmd_locked(SEV_CMD_INIT_EX, &data, error);
}

+static inline int __sev_do_init_locked(int *psp_ret)
+{
+ if (sev_init_ex_buffer)
+ return __sev_init_ex_locked(psp_ret);
+ else
+ return __sev_init_locked(psp_ret);
+}
+
static int __sev_platform_init_locked(int *error)
{
int rc = 0, psp_ret = SEV_RET_NO_FW_CALL;
struct psp_device *psp = psp_master;
- int (*init_function)(int *error);
struct sev_device *sev;

if (!psp || !psp->sev_data)
@@ -458,15 +465,12 @@ static int __sev_platform_init_locked(int *error)
return 0;

if (sev_init_ex_buffer) {
- init_function = __sev_init_ex_locked;
rc = sev_read_init_ex_file();
if (rc)
return rc;
- } else {
- init_function = __sev_init_locked;
}

- rc = init_function(&psp_ret);
+ rc = __sev_do_init_locked(&psp_ret);
if (rc && psp_ret == SEV_RET_SECURE_DATA_INVALID) {
/*
* Initialization command returned an integrity check failure
@@ -477,7 +481,7 @@ static int __sev_platform_init_locked(int *error)
*/
dev_err(sev->dev,
"SEV: retrying INIT command because of SECURE_DATA_INVALID error. Retrying once to reset PSP SEV state.");
- rc = init_function(&psp_ret);
+ rc = __sev_do_init_locked(&psp_ret);
}

if (error)