Messages in this thread Patch in this message | | | Date | Thu, 22 Dec 2022 16:31:16 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH v9 1/4] crypto: ccp - Name -1 return value as SEV_RET_NO_FW_CALL |
| |
On Wed, Dec 07, 2022 at 01:02:07AM +0000, Dionna Glaze wrote: > From: Peter Gonda <pgonda@google.com> > > The PSP can return a "firmware error" code of -1 in circumstances where > the PSP is not actually called. To make this protocol unambiguous, the > value is named SEV_RET_NO_FW_CALL. > > Cc: Thomas Lendacky <Thomas.Lendacky@amd.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Joerg Roedel <jroedel@suse.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Andy Lutomirsky <luto@kernel.org> > Cc: John Allen <john.allen@amd.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Borislav Petkov <bp@alien8.de> > > Signed-off-by: Peter Gonda <pgonda@google.com> > Signed-off-by: Dionna Glaze <dionnaglaze@google.com> > --- > Documentation/virt/coco/sev-guest.rst | 2 +- > drivers/crypto/ccp/sev-dev.c | 22 ++++++++++++++-------- > include/uapi/linux/psp-sev.h | 7 +++++++ > 3 files changed, 22 insertions(+), 9 deletions(-)
Looks good, did some minor touch ups ontop, see below.
@Tom: Ack/Rev-by?
@Herbert: ok to carry this through tip?
Thx.
--- From: Peter Gonda <pgonda@google.com> Date: Wed, 7 Dec 2022 01:02:07 +0000 Subject: [PATCH] crypto: ccp - Name -1 return value as SEV_RET_NO_FW_CALL
The PSP can return a "firmware error" code of -1 in circumstances where the PSP has not actually been called. To make this protocol unambiguous, the name the value SEV_RET_NO_FW_CALL.
[ bp: Massage a bit, get rid of the funky ->init_function local function pointer. ]
Signed-off-by: Peter Gonda <pgonda@google.com> Signed-off-by: Dionna Glaze <dionnaglaze@google.com> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de> Link: https://lore.kernel.org/r/20221207010210.2563293-2-dionnaglaze@google.com --- Documentation/virt/coco/sev-guest.rst | 4 ++-- drivers/crypto/ccp/sev-dev.c | 22 ++++++++++++++-------- include/uapi/linux/psp-sev.h | 7 +++++++ 3 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/Documentation/virt/coco/sev-guest.rst b/Documentation/virt/coco/sev-guest.rst index bf593e88cfd9..aa3e4c6a1f90 100644 --- a/Documentation/virt/coco/sev-guest.rst +++ b/Documentation/virt/coco/sev-guest.rst @@ -40,8 +40,8 @@ along with a description: 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 the -fw_err will be set to 0x00000000000000ff. +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 firmware checks that the message sequence counter is one greater than the guests message sequence counter. If guest driver fails to increment message diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c index 06fc7156c04f..ac205f78a595 100644 --- a/drivers/crypto/ccp/sev-dev.c +++ b/drivers/crypto/ccp/sev-dev.c @@ -440,12 +440,19 @@ 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) { struct psp_device *psp = psp_master; struct sev_device *sev; - int rc = 0, psp_ret = -1; - int (*init_function)(int *error); + int rc = 0, psp_ret = SEV_RET_NO_FW_CALL; if (!psp || !psp->sev_data) return -ENODEV; @@ -456,15 +463,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 @@ -473,9 +477,11 @@ static int __sev_platform_init_locked(int *error) * initialization function should succeed by replacing the state * with a reset state. */ - 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); + dev_err(sev->dev, +"SEV: retrying INIT command because of SECURE_DATA_INVALID error. Retrying once to reset PSP SEV state."); + rc = __sev_do_init_locked(&psp_ret); } + if (error) *error = psp_ret; diff --git a/include/uapi/linux/psp-sev.h b/include/uapi/linux/psp-sev.h index 91b4c63d5cbf..f44514653f5f 100644 --- a/include/uapi/linux/psp-sev.h +++ b/include/uapi/linux/psp-sev.h @@ -36,6 +36,13 @@ enum { * SEV Firmware status code */ typedef enum { + /* + * This error code is not in the SEV spec but is added to convey that + * there was an error that prevented the SEV firmware from being called. + * The SEV API error codes are 16 bits, so the -1 value will not overlap + * with possible values from the specification. + */ + SEV_RET_NO_FW_CALL = -1, SEV_RET_SUCCESS = 0, SEV_RET_INVALID_PLATFORM_STATE, SEV_RET_INVALID_GUEST_STATE, -- 2.35.1
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |