2021-10-28 17:59:15

by Peter Gonda

[permalink] [raw]
Subject: [PATCH 0/4] Add SEV_INIT_EX support

SEV_INIT requires users to unlock their SPI bus for the PSP's non
volatile (NV) storage. Users may wish to lock their SPI bus for numerous
reasons, to support this the PSP firmware supports SEV_INIT_EX. INIT_EX
allows the firmware to use a region of memory for its NV storage leaving
the kernel responsible for actually storing the data in a persistent
way. This series adds a new module parameter to ccp allowing users to
specify a path to a file for use as the PSP's NV storage. The ccp driver
then reads the file into memory for the PSP to use and is responsible
for writing the file whenever the PSP modifies the memory region.

Signed-off-by: Peter Gonda <[email protected]>
Acked-by: David Rientjes <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: Brijesh Singh <[email protected]>
Cc: Marc Orr <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: John Allen <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Paolo Bonzini <[email protected]> (
Cc: [email protected]
Cc: [email protected]

David Rientjes (1):
crypto: ccp - Add SEV_INIT_EX support

Peter Gonda (3):
crypto: ccp - Fix SEV_INIT error logging on init
crypto: ccp - Move SEV_INIT retry for corrupted data
crypto: ccp - Refactor out sev_fw_alloc()

drivers/crypto/ccp/sev-dev.c | 235 ++++++++++++++++++++++++++++++-----
include/linux/psp-sev.h | 21 ++++
2 files changed, 222 insertions(+), 34 deletions(-)

--
2.33.1.1089.g2158813163f-goog


2021-10-28 17:59:39

by Peter Gonda

[permalink] [raw]
Subject: [PATCH 2/4] crypto: ccp - Move SEV_INIT retry for corrupted data

This change moves the data corrupted retry of SEV_INIT into the
__sev_platform_init_locked() function. This is for upcoming INIT_EX
support as well as helping direct callers of
__sev_platform_init_locked() which currently do not support the
retry.

Signed-off-by: Peter Gonda <[email protected]>
Acked-by: David Rientjes <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: Brijesh Singh <[email protected]>
Cc: Marc Orr <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: John Allen <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Paolo Bonzini <[email protected]> (
Cc: [email protected]
Cc: [email protected]
---
drivers/crypto/ccp/sev-dev.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index ec89a82ba267..e4bc833949a0 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -267,6 +267,18 @@ static int __sev_platform_init_locked(int *error)
}

rc = __sev_do_cmd_locked(SEV_CMD_INIT, &data, error);
+ if (rc && *error == SEV_RET_SECURE_DATA_INVALID) {
+ /*
+ * INIT command returned an integrity check failure
+ * status code, meaning that firmware load and
+ * validation of SEV related persistent data has
+ * failed and persistent state has been erased.
+ * Retrying INIT command here should succeed.
+ */
+ dev_dbg(sev->dev, "SEV: retrying INIT command");
+ rc = __sev_do_cmd_locked(SEV_CMD_INIT, &data, error);
+ }
+
if (rc)
return rc;

@@ -1091,18 +1103,6 @@ void sev_pci_init(void)

/* Initialize the platform */
rc = sev_platform_init(&error);
- if (rc && (error == SEV_RET_SECURE_DATA_INVALID)) {
- /*
- * INIT command returned an integrity check failure
- * status code, meaning that firmware load and
- * validation of SEV related persistent data has
- * failed and persistent state has been erased.
- * Retrying INIT command here should succeed.
- */
- dev_dbg(sev->dev, "SEV: retrying INIT command");
- rc = sev_platform_init(&error);
- }
-
if (rc) {
dev_err(sev->dev, "SEV: failed to INIT error %#x, rc %d\n",
error, rc);
--
2.33.1.1089.g2158813163f-goog

2021-11-01 16:29:05

by Marc Orr

[permalink] [raw]
Subject: Re: [PATCH 2/4] crypto: ccp - Move SEV_INIT retry for corrupted data

On Thu, Oct 28, 2021 at 10:58 AM Peter Gonda <[email protected]> wrote:
>
> This change moves the data corrupted retry of SEV_INIT into the
> __sev_platform_init_locked() function. This is for upcoming INIT_EX
> support as well as helping direct callers of
> __sev_platform_init_locked() which currently do not support the
> retry.
>
> Signed-off-by: Peter Gonda <[email protected]>
> Acked-by: David Rientjes <[email protected]>
> Cc: Tom Lendacky <[email protected]>
> Cc: Brijesh Singh <[email protected]>
> Cc: Marc Orr <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Herbert Xu <[email protected]>
> Cc: David Rientjes <[email protected]>
> Cc: John Allen <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Paolo Bonzini <[email protected]> (
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/crypto/ccp/sev-dev.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index ec89a82ba267..e4bc833949a0 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -267,6 +267,18 @@ static int __sev_platform_init_locked(int *error)
> }
>
> rc = __sev_do_cmd_locked(SEV_CMD_INIT, &data, error);
> + if (rc && *error == SEV_RET_SECURE_DATA_INVALID) {
> + /*
> + * INIT command returned an integrity check failure
> + * status code, meaning that firmware load and
> + * validation of SEV related persistent data has
> + * failed and persistent state has been erased.
> + * Retrying INIT command here should succeed.
> + */
> + dev_dbg(sev->dev, "SEV: retrying INIT command");
> + rc = __sev_do_cmd_locked(SEV_CMD_INIT, &data, error);
> + }
> +
> if (rc)
> return rc;
>
> @@ -1091,18 +1103,6 @@ void sev_pci_init(void)
>
> /* Initialize the platform */
> rc = sev_platform_init(&error);
> - if (rc && (error == SEV_RET_SECURE_DATA_INVALID)) {
> - /*
> - * INIT command returned an integrity check failure
> - * status code, meaning that firmware load and
> - * validation of SEV related persistent data has
> - * failed and persistent state has been erased.
> - * Retrying INIT command here should succeed.
> - */
> - dev_dbg(sev->dev, "SEV: retrying INIT command");
> - rc = sev_platform_init(&error);
> - }
> -
> if (rc) {
> dev_err(sev->dev, "SEV: failed to INIT error %#x, rc %d\n",
> error, rc);
> --
> 2.33.1.1089.g2158813163f-goog
>

Reviewed-by: Marc Orr <[email protected]>

2021-11-01 16:31:45

by Marc Orr

[permalink] [raw]
Subject: Re: [PATCH 0/4] Add SEV_INIT_EX support

On Thu, Oct 28, 2021 at 10:57 AM Peter Gonda <[email protected]> wrote:
>
> SEV_INIT requires users to unlock their SPI bus for the PSP's non
> volatile (NV) storage. Users may wish to lock their SPI bus for numerous
> reasons, to support this the PSP firmware supports SEV_INIT_EX. INIT_EX
> allows the firmware to use a region of memory for its NV storage leaving
> the kernel responsible for actually storing the data in a persistent
> way. This series adds a new module parameter to ccp allowing users to
> specify a path to a file for use as the PSP's NV storage. The ccp driver
> then reads the file into memory for the PSP to use and is responsible
> for writing the file whenever the PSP modifies the memory region.
>
> Signed-off-by: Peter Gonda <[email protected]>
> Acked-by: David Rientjes <[email protected]>
> Cc: Tom Lendacky <[email protected]>
> Cc: Brijesh Singh <[email protected]>
> Cc: Marc Orr <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Herbert Xu <[email protected]>
> Cc: David Rientjes <[email protected]>
> Cc: John Allen <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Paolo Bonzini <[email protected]> (
> Cc: [email protected]
> Cc: [email protected]
>
> David Rientjes (1):
> crypto: ccp - Add SEV_INIT_EX support
>
> Peter Gonda (3):
> crypto: ccp - Fix SEV_INIT error logging on init
> crypto: ccp - Move SEV_INIT retry for corrupted data
> crypto: ccp - Refactor out sev_fw_alloc()
>
> drivers/crypto/ccp/sev-dev.c | 235 ++++++++++++++++++++++++++++++-----
> include/linux/psp-sev.h | 21 ++++
> 2 files changed, 222 insertions(+), 34 deletions(-)
>
> --
> 2.33.1.1089.g2158813163f-goog
>

I've just replied with my Reviewed-by tag to all of the patches
because I had reviewed the v1 internally, before Peter posted
externally. Thank you, Tom, for the excellent reviews! I'm looking
forward to seeing the v2 with all of this feedback incorporated.