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@xxxxxxxxxx>
>> Cc: "Radim Krčmář" <rkrcmar@xxxxxxxxxx>
>> Cc: Borislav Petkov <bp@xxxxxxx>
>> Cc: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
>> Cc: Gary Hook <gary.hook@xxxxxxx>
>> Cc: Tom Lendacky <thomas.lendacky@xxxxxxx>
>> Cc: linux-crypto@xxxxxxxxxxxxxxx
>> Cc: kvm@xxxxxxxxxxxxxxx
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
>> ---
>> 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.
>
