Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

On 10/27/17 2:56 AM, Borislav Petkov wrote:

On Thu, Oct 26, 2017 at 03:59:32PM -0500, Brijesh Singh wrote:

quoted

we can workaround #1 by adding some hooks in sp_pci_init() to invoke the PSP
initialization routines after pci_register_driver() is done but #2 can get
painful because it will require us calling the SHUTDOWN outside the
sp_pci_exit() code flow.

Ok, do that and init the PSP master and then put the device in UNINIT
state only in the functions which execute those commands which need the
device to be in UNINIT state, e.g., wrap the SEV_CMD_FACTORY_RESET glue
in a command function which does put the device in the UNINIT state as a
first step.

transiting a platform in UINIT state to handle the FACTORY_RESET can
have a negative consequence.

Consider this scenario:

Process A
---------
sev_launch_start(...)

while (count < 10000) {
    sev_launch_update(...)
}

sev_launch_finish()
...
...

Process B:
---------
....
sev_factory_reset();
....

If in order to handle the FACTORY_RESET we  transition a platform in
UINIT state then it will results as unexpected failure from the
sev_launch_update() because the FACTORY_RESET command remove all the
state information created by sev_launch_start() etc. 

I think our design so far is simple, if command require INIT state then
caller executes sev_platform_init(), then command and finish with
sev_platform_shutdown(). If command does not require INIT state, then
simply issue the command. e.g currently, when caller issues
FACTORY_RESET then we pass command directly to PSP and if FW is in INIT
state then FACTORY_RESET returns error (INVALID_STATE/EBUSY) and we
propagate the error code to userspace.  User can retry the command
sometime later when nobody else is using the PSP.

Then, when that function is done, put the device in the mode which the
other commands would expect it to be in, e.g., INIT state.

This way you'll simplify the whole command flow considerably and won't
have to "toggle" the device each time and will save yourself a lot of
time on command execution.

Thx.