| Message ID | 20171007010607.78088-6-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <pbonzini@redhat.com> > Cc: "Radim Krčmář" <rkrcmar@redhat.com> > Cc: Borislav Petkov <bp@suse.de> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: Gary Hook <gary.hook@amd.com> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Cc: linux-crypto@vger.kernel.org > Cc: kvm@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Brijesh Singh <brijesh.singh@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? 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.
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 <pbonzini@redhat.com> >> Cc: "Radim Krčmář" <rkrcmar@redhat.com> >> Cc: Borislav Petkov <bp@suse.de> >> Cc: Herbert Xu <herbert@gondor.apana.org.au> >> Cc: Gary Hook <gary.hook@amd.com> >> Cc: Tom Lendacky <thomas.lendacky@amd.com> >> Cc: linux-crypto@vger.kernel.org >> Cc: kvm@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Brijesh Singh <brijesh.singh@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. >
On Thu, Oct 12, 2017 at 03:21:04PM -0500, Brijesh Singh wrote: > 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. I see. So the proposition to carve out and split the platform *init commands might come in handy here too...
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; + + 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; + } default: ret = -EINVAL; break;
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 <pbonzini@redhat.com> Cc: "Radim Krčmář" <rkrcmar@redhat.com> Cc: Borislav Petkov <bp@suse.de> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: Gary Hook <gary.hook@amd.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: linux-crypto@vger.kernel.org Cc: kvm@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- drivers/crypto/ccp/psp-dev.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)