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

2017-10-28 Thread Brijesh Singh
On 10/27/17 7:00 PM, Borislav Petkov wrote: > On Fri, Oct 27, 2017 at 05:59:23PM -0500, Brijesh Singh wrote: >> Yes it is typo. PEK_GEN wants FW to be in INIT state hence someone need >> to transition from UNINIT -> INIT. > Which, once you've done it once on driver init, is there. > >> That's

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

2017-10-28 Thread Brijesh Singh
On 10/27/17 7:00 PM, Borislav Petkov wrote: > On Fri, Oct 27, 2017 at 05:59:23PM -0500, Brijesh Singh wrote: >> Yes it is typo. PEK_GEN wants FW to be in INIT state hence someone need >> to transition from UNINIT -> INIT. > Which, once you've done it once on driver init, is there. > >> That's

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

2017-10-27 Thread Borislav Petkov
On Fri, Oct 27, 2017 at 05:59:23PM -0500, Brijesh Singh wrote: > Yes it is typo. PEK_GEN wants FW to be in INIT state hence someone need > to transition from UNINIT -> INIT. Which, once you've done it once on driver init, is there. > That's what I am doing except FACTORY_RESET. Well, not

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

2017-10-27 Thread Borislav Petkov
On Fri, Oct 27, 2017 at 05:59:23PM -0500, Brijesh Singh wrote: > Yes it is typo. PEK_GEN wants FW to be in INIT state hence someone need > to transition from UNINIT -> INIT. Which, once you've done it once on driver init, is there. > That's what I am doing except FACTORY_RESET. Well, not

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

2017-10-27 Thread Brijesh Singh
On 10/27/17 4:49 PM, Borislav Petkov wrote: > On Fri, Oct 27, 2017 at 04:28:31PM -0500, Brijesh Singh wrote: >> This will fail because PEK_GEN require the platform in INIT state and >> nobody has done the state transition from INIT -> UINIT. > Huh, FW is in INIT state and PEK_GEN wants it to be

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

2017-10-27 Thread Brijesh Singh
On 10/27/17 4:49 PM, Borislav Petkov wrote: > On Fri, Oct 27, 2017 at 04:28:31PM -0500, Brijesh Singh wrote: >> This will fail because PEK_GEN require the platform in INIT state and >> nobody has done the state transition from INIT -> UINIT. > Huh, FW is in INIT state and PEK_GEN wants it to be

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

2017-10-27 Thread Borislav Petkov
On Fri, Oct 27, 2017 at 04:28:31PM -0500, Brijesh Singh wrote: > This will fail because PEK_GEN require the platform in INIT state and > nobody has done the state transition from INIT -> UINIT. Huh, FW is in INIT state and PEK_GEN wants it to be in INIT state. Typo? Aaanyway, I don't like this

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

2017-10-27 Thread Borislav Petkov
On Fri, Oct 27, 2017 at 04:28:31PM -0500, Brijesh Singh wrote: > This will fail because PEK_GEN require the platform in INIT state and > nobody has done the state transition from INIT -> UINIT. Huh, FW is in INIT state and PEK_GEN wants it to be in INIT state. Typo? Aaanyway, I don't like this

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

2017-10-27 Thread Brijesh Singh
On 10/27/17 3:27 PM, Borislav Petkov wrote: > On Fri, Oct 27, 2017 at 03:25:24PM -0500, Brijesh Singh wrote: >> Yep, we are doing state transition only when we really need to. At least >> so far I have tried to avoid making any unnecessary state transitions. > So change all those which do INIT

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

2017-10-27 Thread Brijesh Singh
On 10/27/17 3:27 PM, Borislav Petkov wrote: > On Fri, Oct 27, 2017 at 03:25:24PM -0500, Brijesh Singh wrote: >> Yep, we are doing state transition only when we really need to. At least >> so far I have tried to avoid making any unnecessary state transitions. > So change all those which do INIT

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

2017-10-27 Thread Borislav Petkov
On Fri, Oct 27, 2017 at 03:25:24PM -0500, Brijesh Singh wrote: > Yep, we are doing state transition only when we really need to. At least > so far I have tried to avoid making any unnecessary state transitions. So change all those which do INIT -> CMD -> SHUTDOWN to do only the command as the

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

2017-10-27 Thread Borislav Petkov
On Fri, Oct 27, 2017 at 03:25:24PM -0500, Brijesh Singh wrote: > Yep, we are doing state transition only when we really need to. At least > so far I have tried to avoid making any unnecessary state transitions. So change all those which do INIT -> CMD -> SHUTDOWN to do only the command as the

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

2017-10-27 Thread Brijesh Singh
On 10/27/17 3:15 PM, Borislav Petkov wrote: > On Fri, Oct 27, 2017 at 06:28:38AM -0500, Brijesh Singh wrote: >> ... User can retry the command sometime later when nobody else is >> using the PSP. > That still doesn't prevent you from doing two things: > > * make that fw_init_count a proper kref

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

2017-10-27 Thread Brijesh Singh
On 10/27/17 3:15 PM, Borislav Petkov wrote: > On Fri, Oct 27, 2017 at 06:28:38AM -0500, Brijesh Singh wrote: >> ... User can retry the command sometime later when nobody else is >> using the PSP. > That still doesn't prevent you from doing two things: > > * make that fw_init_count a proper kref

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

2017-10-27 Thread Borislav Petkov
On Fri, Oct 27, 2017 at 06:28:38AM -0500, Brijesh Singh wrote: > ... User can retry the command sometime later when nobody else is > using the PSP. That still doesn't prevent you from doing two things: * make that fw_init_count a proper kref instead of your homegrown thing * do not preemptively

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

2017-10-27 Thread Borislav Petkov
On Fri, Oct 27, 2017 at 06:28:38AM -0500, Brijesh Singh wrote: > ... User can retry the command sometime later when nobody else is > using the PSP. That still doesn't prevent you from doing two things: * make that fw_init_count a proper kref instead of your homegrown thing * do not preemptively

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

2017-10-27 Thread Brijesh Singh
On 10/27/17 2:56 AM, Borislav Petkov wrote: > On Thu, Oct 26, 2017 at 03:59:32PM -0500, Brijesh Singh wrote: >> 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

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

2017-10-27 Thread Brijesh Singh
On 10/27/17 2:56 AM, Borislav Petkov wrote: > On Thu, Oct 26, 2017 at 03:59:32PM -0500, Brijesh Singh wrote: >> 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

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

2017-10-27 Thread Borislav Petkov
On Thu, Oct 26, 2017 at 03:59:32PM -0500, Brijesh Singh wrote: > 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 >

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

2017-10-27 Thread Borislav Petkov
On Thu, Oct 26, 2017 at 03:59:32PM -0500, Brijesh Singh wrote: > 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 >

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

2017-10-26 Thread Brijesh Singh
On 10/26/2017 03:13 PM, Borislav Petkov wrote: On Thu, Oct 26, 2017 at 02:26:15PM -0500, Brijesh Singh wrote: SHUTDOWN command unconditionally transitions a platform to uninitialized state. The command does not care how many processes are actively using the PSP. We don't want to shutdown the

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

2017-10-26 Thread Brijesh Singh
On 10/26/2017 03:13 PM, Borislav Petkov wrote: On Thu, Oct 26, 2017 at 02:26:15PM -0500, Brijesh Singh wrote: SHUTDOWN command unconditionally transitions a platform to uninitialized state. The command does not care how many processes are actively using the PSP. We don't want to shutdown the

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

2017-10-26 Thread Borislav Petkov
On Thu, Oct 26, 2017 at 02:26:15PM -0500, Brijesh Singh wrote: > SHUTDOWN command unconditionally transitions a platform to uninitialized > state. The command does not care how many processes are actively using the > PSP. We don't want to shutdown the firmware while other process is still > using

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

2017-10-26 Thread Borislav Petkov
On Thu, Oct 26, 2017 at 02:26:15PM -0500, Brijesh Singh wrote: > SHUTDOWN command unconditionally transitions a platform to uninitialized > state. The command does not care how many processes are actively using the > PSP. We don't want to shutdown the firmware while other process is still > using

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

2017-10-26 Thread Brijesh Singh
On 10/26/2017 12:44 PM, Borislav Petkov wrote: On Thu, Oct 26, 2017 at 11:56:57AM -0500, Brijesh Singh wrote: The variable is used as ref counter. ... and it can't be converted to a boolean because...? SHUTDOWN command unconditionally transitions a platform to uninitialized state. The

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

2017-10-26 Thread Brijesh Singh
On 10/26/2017 12:44 PM, Borislav Petkov wrote: On Thu, Oct 26, 2017 at 11:56:57AM -0500, Brijesh Singh wrote: The variable is used as ref counter. ... and it can't be converted to a boolean because...? SHUTDOWN command unconditionally transitions a platform to uninitialized state. The

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

2017-10-26 Thread Borislav Petkov
On Thu, Oct 26, 2017 at 11:56:57AM -0500, Brijesh Singh wrote: > The variable is used as ref counter. ... and it can't be converted to a boolean because...? > In your previous reply you comments on global semaphore (fw_init_mutex) and > in response I tried to highlight why we need the global

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

2017-10-26 Thread Borislav Petkov
On Thu, Oct 26, 2017 at 11:56:57AM -0500, Brijesh Singh wrote: > The variable is used as ref counter. ... and it can't be converted to a boolean because...? > In your previous reply you comments on global semaphore (fw_init_mutex) and > in response I tried to highlight why we need the global

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

2017-10-26 Thread Brijesh Singh
On 10/26/2017 08:56 AM, Borislav Petkov wrote: On Mon, Oct 23, 2017 at 02:57:04PM -0500, Brijesh Singh wrote: Calling PLATFORM_GET_STATUS is not required, we can manage the state through a simple ref count variable. Issuing PSP commands will always be much more expensive compare to accessing

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

2017-10-26 Thread Brijesh Singh
On 10/26/2017 08:56 AM, Borislav Petkov wrote: On Mon, Oct 23, 2017 at 02:57:04PM -0500, Brijesh Singh wrote: Calling PLATFORM_GET_STATUS is not required, we can manage the state through a simple ref count variable. Issuing PSP commands will always be much more expensive compare to accessing

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

2017-10-26 Thread Borislav Petkov
On Mon, Oct 23, 2017 at 02:57:04PM -0500, Brijesh Singh wrote: > Calling PLATFORM_GET_STATUS is not required, we can manage the state through > a simple ref count variable. Issuing PSP commands will always be much more > expensive compare to accessing a protected global variable. What does

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

2017-10-26 Thread Borislav Petkov
On Mon, Oct 23, 2017 at 02:57:04PM -0500, Brijesh Singh wrote: > Calling PLATFORM_GET_STATUS is not required, we can manage the state through > a simple ref count variable. Issuing PSP commands will always be much more > expensive compare to accessing a protected global variable. What does

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

2017-10-23 Thread Brijesh Singh
On 10/23/2017 02:34 AM, Borislav Petkov wrote: ... Just minor cleanups: Thanks Boris, I have applied your cleanups. -Brijesh --- diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c index e9966d5fc6c4..f9a9a6e6ab99 100644 --- a/drivers/crypto/ccp/psp-dev.c +++

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

2017-10-23 Thread Brijesh Singh
On 10/23/2017 02:34 AM, Borislav Petkov wrote: ... Just minor cleanups: Thanks Boris, I have applied your cleanups. -Brijesh --- diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c index e9966d5fc6c4..f9a9a6e6ab99 100644 --- a/drivers/crypto/ccp/psp-dev.c +++

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

2017-10-23 Thread Brijesh Singh
On 10/23/2017 04:20 AM, Borislav Petkov wrote: On Thu, Oct 19, 2017 at 09:33:48PM -0500, Brijesh Singh wrote: +static int __sev_platform_init(struct sev_data_init *data, int *error) +{ + int rc = 0; + + mutex_lock(_init_mutex); + + if (!fw_init_count) { I still don't like

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

2017-10-23 Thread Brijesh Singh
On 10/23/2017 04:20 AM, Borislav Petkov wrote: On Thu, Oct 19, 2017 at 09:33:48PM -0500, Brijesh Singh wrote: +static int __sev_platform_init(struct sev_data_init *data, int *error) +{ + int rc = 0; + + mutex_lock(_init_mutex); + + if (!fw_init_count) { I still don't like

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

2017-10-23 Thread Borislav Petkov
On Thu, Oct 19, 2017 at 09:33:48PM -0500, Brijesh Singh wrote: > +static int __sev_platform_init(struct sev_data_init *data, int *error) > +{ > + int rc = 0; > + > + mutex_lock(_init_mutex); > + > + if (!fw_init_count) { I still don't like global semaphores. Can you get the status and

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

2017-10-23 Thread Borislav Petkov
On Thu, Oct 19, 2017 at 09:33:48PM -0500, Brijesh Singh wrote: > +static int __sev_platform_init(struct sev_data_init *data, int *error) > +{ > + int rc = 0; > + > + mutex_lock(_init_mutex); > + > + if (!fw_init_count) { I still don't like global semaphores. Can you get the status and

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

2017-10-23 Thread Borislav Petkov
On Thu, Oct 19, 2017 at 09:33:48PM -0500, Brijesh Singh wrote: > AMD's new Secure Encrypted Virtualization (SEV) feature allows the > memory contents of virtual machines to be transparently encrypted with a > key unique to the VM. The programming and management of the encryption > keys are handled

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

2017-10-23 Thread Borislav Petkov
On Thu, Oct 19, 2017 at 09:33:48PM -0500, Brijesh Singh wrote: > AMD's new Secure Encrypted Virtualization (SEV) feature allows the > memory contents of virtual machines to be transparently encrypted with a > key unique to the VM. The programming and management of the encryption > keys are handled

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

2017-10-19 Thread Brijesh Singh
AMD's new Secure Encrypted Virtualization (SEV) feature allows the memory contents of virtual machines to be transparently encrypted with a key unique to the VM. The programming and management of the encryption keys are handled by the AMD Secure Processor (AMD-SP) which exposes the commands for

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

2017-10-19 Thread Brijesh Singh
AMD's new Secure Encrypted Virtualization (SEV) feature allows the memory contents of virtual machines to be transparently encrypted with a key unique to the VM. The programming and management of the encryption keys are handled by the AMD Secure Processor (AMD-SP) which exposes the commands for