On 10/12/17 1:48 PM, Borislav Petkov wrote: ... > On Fri, Oct 06, 2017 at 08:06:04PM -0500, Brijesh Singh wrote: >> The SEV_PDH_GEN command is used to re-generate the Platform >> Diffie-Hellman (PDH) key. The command is defined in SEV spec section >> 5.9. >> >> Cc: Paolo Bonzini <pbonz...@redhat.com> >> Cc: "Radim Krčmář" <rkrc...@redhat.com> >> Cc: Borislav Petkov <b...@suse.de> >> Cc: Herbert Xu <herb...@gondor.apana.org.au> >> Cc: Gary Hook <gary.h...@amd.com> >> Cc: Tom Lendacky <thomas.lenda...@amd.com> >> Cc: linux-crypto@vger.kernel.org >> Cc: k...@vger.kernel.org >> Cc: linux-ker...@vger.kernel.org >> Signed-off-by: Brijesh Singh <brijesh.si...@amd.com> >> --- >> drivers/crypto/ccp/psp-dev.c | 32 ++++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c >> index 03d7bd03ad58..28efb7a9245a 100644 >> --- a/drivers/crypto/ccp/psp-dev.c >> +++ b/drivers/crypto/ccp/psp-dev.c >> @@ -271,6 +271,34 @@ static int sev_ioctl_pek_gen(struct sev_issue_cmd *argp) >> return ret; >> } >> >> +static int sev_ioctl_pdh_gen(struct sev_issue_cmd *argp) >> +{ >> + int ret, state, do_shutdown = 0; >> + >> + /* >> + * PDH_GEN command can be issued when platform is in INIT or WORKING >> + * state. If we are in UNINIT state then transition in INIT state >> + * before issuing the command. >> + */ >> + ret = sev_platform_get_state(&state, &argp->error); >> + if (ret) >> + return ret; >> + > Why isn't this function doing: > > if (state == SEV_STATE_WORKING) { > return -EBUSY; > > like the PEK_GEN one?
We need to follow the platform state machine logic defined in SEV spec section 5.1.2. The PEK_GEN can not be issued when platform is in WORKING state because the command actually re-generate the identity of the platform itself (in other words re-generate the Platform Endorsement Key). Whereas, the PDH_GEN command is used for re-generating Platform Diffie-Hellman Key which can be changed while the guest is running. > Because if so, you can convert it and the PEK_GEN one into a single > function doing the work and wrappers handing in the command to avoid the > code duplication. > >> + if (state == SEV_STATE_UNINIT) { >> + ret = sev_firmware_init(&argp->error); >> + if (ret) >> + return ret; >> + do_shutdown = 1; >> + } >> + >> + ret = sev_handle_cmd(SEV_CMD_PDH_GEN, 0, &argp->error); >> + >> + if (do_shutdown) >> + sev_handle_cmd(SEV_CMD_SHUTDOWN, 0, NULL); >> + >> + return ret; >> +} >> + >> static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long >> arg) >> { >> void __user *argp = (void __user *)arg; >> @@ -300,6 +328,10 @@ static long sev_ioctl(struct file *file, unsigned int >> ioctl, unsigned long arg) >> ret = sev_ioctl_pek_gen(&input); >> break; >> } >> + case SEV_PDH_GEN: { >> + ret = sev_ioctl_pdh_gen(&input); >> + break; >> + } > And those curly braces can go, as before. >