Re: [Part2 PATCH v7 20/38] crypto: ccp: Implement SEV_PDH_CERT_EXPORT ioctl command

2017-11-05 Thread Borislav Petkov
On Wed, Nov 01, 2017 at 04:16:05PM -0500, Brijesh Singh wrote: > The SEV_PDH_CERT_EXPORT command can be used to export the PDH and its > certificate chain. The command is defined in SEV spec section 5.10. ... > --- > drivers/crypto/ccp/psp-dev.c | 98 >

Re: [Part2 PATCH v7 18/38] crypto: ccp: Implement SEV_PEK_CSR ioctl command

2017-11-03 Thread Borislav Petkov
quot; <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 >

Re: [Part2 PATCH v7 14/38] crypto: ccp: Implement SEV_FACTORY_RESET ioctl command

2017-11-03 Thread Borislav Petkov
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.o

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

2017-11-02 Thread Borislav Petkov
- an in-kernel API to communicate with the SEV firmware. The API can be >used by the hypervisor to create encryption context for a SEV guest. > > - a userspace IOCTL to manage the platform certificates. > > Cc: Paolo Bonzini <pbonz...@redhat.com> > Cc: "Radim Krčmář&q

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

2017-10-31 Thread Borislav Petkov
On Mon, Oct 30, 2017 at 08:29:25PM -0500, Brijesh Singh wrote: > Okay, Just tried static global with CONFIG_VMAP_STACK=y and I am getting > wrong physical address with __pa. PSP command fails with error code > "INVALID_ADDRESS". The same thing works fine with kmalloc() buffer. Ah, right, module

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

2017-10-30 Thread Borislav Petkov
On Mon, Oct 30, 2017 at 12:49:14PM -0500, Brijesh Singh wrote: > If the buffer is allocated on the stack then there is no guarantee that static global is not allocated on the stack. > I can certainly move the allocation outside, but then it may increase the > code size in other functions. If its

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

2017-10-30 Thread Borislav Petkov
On Sun, Oct 29, 2017 at 03:48:25PM -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-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 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 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 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 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 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 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.1 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command

2017-10-26 Thread Borislav Petkov
ter reason for the failure. > > drivers/crypto/ccp/psp-dev.c | 31 +++ > 1 file changed, 31 insertions(+) Reviewed-by: Borislav Petkov <b...@suse.de> -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --

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 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command

2017-10-23 Thread Borislav Petkov
On Mon, Oct 23, 2017 at 08:32:57AM -0500, Brijesh Singh wrote: > If both the command fails then we return status from the last command. > IIRC, in my previous patches I was returning status from sev_do_cmd() > instead of sev_platform_shutdown() but based on our previous > communication I thought

Re: [Part2 PATCH v6 18/38] crypto: ccp: Implement SEV_PEK_CSR ioctl command

2017-10-23 Thread Borislav Petkov
quot; <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.or

Re: [Part2 PATCH v6 17/38] crypto: ccp: Implement SEV_PDH_GEN ioctl command

2017-10-23 Thread Borislav Petkov
ář" <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

Re: [Part2 PATCH v6 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command

2017-10-23 Thread Borislav Petkov
On Mon, Oct 23, 2017 at 07:15:30AM -0500, Brijesh Singh wrote: > I am not sure if I am able to understand your feedback. The > sev_platform_shutdown() is called unconditionally. How's that: If sev_do_cmd() fails and sev_do_cmd(SEV_CMD_SHUTDOWN, ...) in sev_platform_shutdown() fails, then the

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 15/38] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command

2017-10-23 Thread Borislav Petkov
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.o

Re: [Part2 PATCH v6 14/38] crypto: ccp: Implement SEV_FACTORY_RESET ioctl command

2017-10-23 Thread Borislav Petkov
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.o

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 v5.1 12.9/31] crypto: ccp: Implement SEV_PDH_CERT_EXPORT ioctl command

2017-10-13 Thread Borislav Petkov
quot; <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.or

Re: [Part2 PATCH v5.1 12.8/31] crypto: ccp: Implement SEV_PEK_CERT_IMPORT ioctl command

2017-10-13 Thread Borislav Petkov
On Fri, Oct 06, 2017 at 08:06:06PM -0500, Brijesh Singh wrote: > The SEV_PEK_CERT_IMPORT command can be used to import the signed PEK > certificate. The command is defined in SEV spec section 5.8. > > Cc: Paolo Bonzini <pbonz...@redhat.com> > Cc: "Radim Krčmář" <

Re: [Part2 PATCH v5.1 12.7/31] crypto: ccp: Implement SEV_PEK_CSR ioctl command

2017-10-13 Thread Borislav Petkov
On Thu, Oct 12, 2017 at 11:13:44PM -0500, Brijesh Singh wrote: > As per the spec, its perfectly legal to pass input.address = 0x0 and > input.length = 0x0. If userspace wants to query CSR length then it will > fill all the fields with. In response, FW will return error > (LENGTH_TO_SMALL) and

Re: [Part2 PATCH v5.1 12.7/31] crypto: ccp: Implement SEV_PEK_CSR ioctl command

2017-10-13 Thread Borislav Petkov
On Thu, Oct 12, 2017 at 09:24:01PM -0500, Brijesh Singh wrote: > I assume you mean performing the SEV state check before allocating the > memory for the CSR blob, right ? I mean, do those first: if (copy_from_user(, (void __user *)argp->data, sizeof(input))) return

Re: [Part2 PATCH v5.2 12.2/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-12 Thread Borislav Petkov
On Thu, Oct 12, 2017 at 04:52:32PM -0500, Brijesh Singh wrote: > See my above comment, I think the simplest solution is remove psp->sev_misc Ok, so far so good. But now you still need to track which is the last psp device and to call misc_deregister() only when the last device exits. Because if

Re: [Part2 PATCH v5.2 12.2/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-12 Thread Borislav Petkov
On Thu, Oct 12, 2017 at 04:11:18PM -0500, Brijesh Singh wrote: > The sev_exit() will be called for all the psp_device instance. we need > to set psp_misc_dev = NULL after deregistering the device. > > if (psp_misc_dev) { >   misc_deregister(psp_misc_dev); >    psp_misc_dev = NULL; Right, except

Re: [Part2 PATCH v5.1 12.6/31] crypto: ccp: Implement SEV_PDH_GEN ioctl command

2017-10-12 Thread Borislav Petkov
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

Re: [Part2 PATCH v5.1 12.5/31] crypto: ccp: Implement SEV_PEK_GEN ioctl command

2017-10-12 Thread Borislav Petkov
On Thu, Oct 12, 2017 at 03:11:07PM -0500, Brijesh Singh wrote: > Lets  consider this scenario > 1- platform is in uninit state, we transition it to INIT > 2- PEK_GEN command failed > 3- since we have transitioned the platform in INIT state hence we must > call the shutdown otherwise we will leave

Re: [Part2 PATCH v5.1 12.7/31] crypto: ccp: Implement SEV_PEK_CSR ioctl command

2017-10-12 Thread Borislav Petkov
quot; <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.or

Re: [Part2 PATCH v5.1 12.6/31] crypto: ccp: Implement SEV_PDH_GEN ioctl command

2017-10-12 Thread Borislav Petkov
ář" <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

Re: [Part2 PATCH v5.1 12.5/31] crypto: ccp: Implement SEV_PEK_GEN ioctl command

2017-10-12 Thread Borislav Petkov
On Fri, Oct 06, 2017 at 08:06:03PM -0500, Brijesh Singh wrote: > The SEV_PEK_GEN command is used to generate a new Platform Endorsement > Key (PEK). The command is defined in SEV spec section 5.6. > > Cc: Paolo Bonzini <pbonz...@redhat.com> > Cc: "Radim Krčmář" <

Re: [Part2 PATCH v5.2 12.3/31] crypto: ccp: Implement SEV_FACTORY_RESET ioctl command

2017-10-12 Thread Borislav Petkov
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.o

Re: [Part2 PATCH v5.2 12.2/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-12 Thread Borislav Petkov
- an in-kernel API to communicate with the SEV firmware. The API can be >used by the hypervisor to create encryption context for a SEV guest. > > - a userspace IOCTL to manage the platform certificates. > > Cc: Paolo Bonzini <pbonz...@redhat.com> > Cc: "Radim Krčmář&q

Re: [Part2 PATCH v5.1 12.4/31] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command

2017-10-11 Thread Borislav Petkov
On Wed, Oct 11, 2017 at 03:45:00PM -0500, Brijesh Singh wrote: > OK, if userspace is going to pick bits apart then how about this: > > struct sev_data_status { > u8 api_major; /* Out */ > u8 api_minor; /* Out */ > u8

Re: [Part2 PATCH v5.1 12.4/31] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command

2017-10-11 Thread Borislav Petkov
On Wed, Oct 11, 2017 at 03:10:49PM -0500, Brijesh Singh wrote: > The current 'struct sev_data_status' matches with the firmware names and the > bit fields. Only thing I did was the fields with no name is called as > "reservedX" Ok, I see it. So what you actually wanna do is: struct

Re: [Part2 PATCH v5.1 12.4/31] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command

2017-10-11 Thread Borislav Petkov
On Wed, Oct 11, 2017 at 10:04:44PM +0200, Borislav Petkov wrote: > Then it is easy: > >&= GENMASK(23, 1); [1:23] range cleared, of course: &= ~GENMASK(23, 1); -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norto

Re: [Part2 PATCH v5.1 12.4/31] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command

2017-10-11 Thread Borislav Petkov
On Wed, Oct 11, 2017 at 02:49:55PM -0500, Brijesh Singh wrote: > This is OK for now. But in future if FW steals another bit from reserved1 > field to expose a new flag then 'owner' name will no longer be valid. If you > don't to use bit field then we have to use some generic name instead of >

Re: [Part2 PATCH v5.1 12.4/31] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command

2017-10-11 Thread Borislav Petkov
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

Re: [Part2 PATCH v5.1 12.3/31] crypto: ccp: Implement SEV_FACTORY_RESET ioctl command

2017-10-11 Thread Borislav Petkov
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

Re: [Part2 PATCH v5.1 12.1/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-11 Thread Borislav Petkov
On Sun, Oct 08, 2017 at 08:30:47AM -0500, Brijesh Singh wrote: > Basically we need some variable which is outside the per-device > structure so that we don't end up creating multiple /dev/sev nodes. If > needed, I think we can remove 'has_sev_fops' variable from struct > psp_device if we decide to

Re: [Part2 PATCH v5.1 12.1/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-10 Thread Borislav Petkov
On Tue, Oct 10, 2017 at 01:43:22PM -0500, Tom Lendacky wrote: > Maybe for the very first implementation we could do that and that was what > was originally done for the CCP. But as you can see the CCP does not have > a set register offset between various iterations of the device and it can > be

Re: [Part2 PATCH v5.1 12.1/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-09 Thread Borislav Petkov
On Sun, Oct 08, 2017 at 07:11:04PM -0500, Brijesh Singh wrote: > There is a single security processor driver (ccp) which provides the > complete functionality including PSP.  But the driver should be able to > work with multiple devices. e.g In my 2P EPYC configuration, security > processor driver

Re: [Part2 PATCH v5.1 12.1/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-08 Thread Borislav Petkov
On Sun, Oct 08, 2017 at 08:30:47AM -0500, Brijesh Singh wrote: > During the device probe, sev_ops_init() will be called for every device > instance which claims to support the SEV.  One of the device will be > 'master' but we don't the master until we probe all the instances. Hence > the probe for

Re: [Part2 PATCH v5.1 12.1/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-07 Thread Borislav Petkov
- an in-kernel API to communicate with the SEV firmware. The API can be >used by the hypervisor to create encryption context for a SEV guest. > > - a userspace IOCTL to manage the platform certificates. > > Cc: Paolo Bonzini <pbonz...@redhat.com> > Cc: "Radim Krčmář&q

Re: [Part2 PATCH v5.1 12.2/31] crypto: ccp: Define SEV userspace ioctl and command id

2017-10-07 Thread Borislav Petkov
On Fri, Oct 06, 2017 at 08:06:00PM -0500, Brijesh Singh wrote: > Add a include file which defines the ioctl and command id used for > issuing SEV platform management specific commands. > > Cc: Paolo Bonzini <pbonz...@redhat.com> > Cc: "Radim Krčmář" <rkrc...@re

Re: [Part2 PATCH v5 12/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-06 Thread Borislav Petkov
space IOCTL to manage the platform certificates." > > 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 <

Re: [Part2 PATCH v5 09/31] crypto: ccp: Build the AMD secure processor driver only with AMD CPU support

2017-10-04 Thread Borislav Petkov
On Wed, Oct 04, 2017 at 08:13:50AM -0500, Brijesh Singh wrote: > From: Borislav Petkov <b...@suse.de> > > This is AMD-specific hardware so present it in Kconfig only when AMD > CPU support is enabled or on ARM64 where it is also used. > > Signed-off-by: <brijesh.

Re: [Part2 PATCH v4.1 07/29] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-04 Thread Borislav Petkov
On Wed, Oct 04, 2017 at 03:24:36PM +0530, P J P wrote: > It appears to cross 80 columns limit, checkpatch.pl throws warnings. Adding > new line would be consistent with coding style. The 80 cols rule is not a hard one and checkpatch should not override common sense. This is a function which maps

Re: [Part2 PATCH v4.1 07/29] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-04 Thread Borislav Petkov
On Wed, Oct 04, 2017 at 12:26:11PM +0530, P J P wrote: > Each return above needs to be on its own line. ... because? -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

Re: [Part2 Patch v4.2] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-10-04 Thread Borislav Petkov
On Wed, Oct 04, 2017 at 12:06:42PM +0530, P J P wrote: > Needs to kfree(sp->psp_data) before setting to NULL. Not if it is allocated with devm_kzalloc(). -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

Re: [Part2 PATCH v4 05/29] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-10-03 Thread Borislav Petkov
Your patch should concentrate only on adding the PSP and its dependencies. Thx. --- From: Borislav Petkov <b...@suse.de> Date: Sat, 30 Sep 2017 10:06:27 +0200 Subject: [PATCH] crypto: ccp: Build the AMD secure processor driver only with AMD CPU support This is AMD-specific hardware so pr

Re: [Part2 PATCH v4 05/29] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-09-30 Thread Borislav Petkov
On Sat, Sep 30, 2017 at 10:55:25AM -0500, Brijesh Singh wrote: > CRYPTO_DEV_CCP_DD is supported on aarch64 and x86. Whereas the PSP > interface I am adding is available on x86 only hence its safe to add add > depend on CPU_SUP_AMD for CRYPTO_DEV_SP_PSP. I think just from having CRYPTO_DEV_CCP_DD

Re: [PATCH] crypto: ccp: Build the AMD secure processor driver only with AMD CPU support

2017-09-30 Thread Borislav Petkov
On Sat, Sep 30, 2017 at 09:06:26AM -0500, Brijesh Singh wrote: > > diff --git a/drivers/crypto/ccp/Kconfig b/drivers/crypto/ccp/Kconfig > > index 627f3e61dcac..f58a6521270b 100644 > > --- a/drivers/crypto/ccp/Kconfig > > +++ b/drivers/crypto/ccp/Kconfig > > @@ -1,5 +1,6 @@ > > config

[PATCH] crypto: ccp: Build the AMD secure processor driver only with AMD CPU support

2017-09-30 Thread Borislav Petkov
Hi, just a small Kconfig correction. Feel free to add it to your patchset. Thx. --- From: Borislav Petkov <b...@suse.de> This is AMD-specific hardware so present it in Kconfig only when AMD CPU support is enabled. Signed-off-by: Borislav Petkov <b...@suse.de> Cc: Brijesh Singh

Re: [Part2 PATCH v4 05/29] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-09-29 Thread Borislav Petkov
offon", 3 * , 3) in my .vimrc And I'm pretty sure you can do a similar thing with other editors. > software-based Trusted Executation Environment (TEE) to enable the > third-party trusted applications. > > Cc: Paolo Bonzini <pbonz...@redhat.com> > Cc: "Radim Krčmář&qu

Re: [RFC Part2 PATCH v3 03/26] crypto: ccp: Add Secure Encrypted Virtualization (SEV) device support

2017-09-13 Thread Borislav Petkov
On Mon, Jul 24, 2017 at 03:02:40PM -0500, Brijesh Singh wrote: > AMDs new Secure Encrypted Virtualization (SEV) feature allows the memory > contents of a virtual machine to be transparently encrypted with a key > unique to the guest VM. The programming and management of the encryption > keys are

Re: [RFC Part2 PATCH v3 03/26] crypto: ccp: Add Secure Encrypted Virtualization (SEV) device support

2017-09-12 Thread Borislav Petkov
On Tue, Sep 12, 2017 at 10:32:13AM -0500, Brijesh Singh wrote: > The debug statement is very helpful during development, it gives me the full > view of what command we send to PSP, data dump of command buffer before and > after the request completion. e.g when dyndbg is enabled the output looks

Re: [RFC Part2 PATCH v3 03/26] crypto: ccp: Add Secure Encrypted Virtualization (SEV) device support

2017-09-12 Thread Borislav Petkov
Just a cursory review: more indepth after the redesign. On Mon, Jul 24, 2017 at 03:02:40PM -0500, Brijesh Singh wrote: > AMDs new Secure Encrypted Virtualization (SEV) feature allows the memory > contents of a virtual machine to be transparently encrypted with a key > unique to the guest VM. The

Re: [RFC Part2 PATCH v3 02/26] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-09-08 Thread Borislav Petkov
On Thu, Sep 07, 2017 at 05:19:32PM -0500, Brijesh Singh wrote: > At high level, AMD-SP (AMD Secure Processor) (i.e CCP driver) will provide the > support for CCP, SEV and TEE FW commands. > > > +--- CCP > | > AMD-SP --| > |+--- SEV > ||

Re: [RFC Part2 PATCH v3 02/26] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-09-08 Thread Borislav Petkov
On Thu, Sep 07, 2017 at 06:15:55PM -0500, Gary R Hook wrote: > I would prefer that we not shorten this. The prior incarnation, > ccp_alloc_struct(), has/had been around for a while. And there are a > number of similarly named allocation functions in the driver that we > like to keep sorted. If

Re: [RFC Part2 PATCH v3 02/26] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-09-07 Thread Borislav Petkov
On Wed, Sep 06, 2017 at 04:26:52PM -0500, Gary R Hook wrote: > They were included in a pull request (for 4.14) from Herbert, dated Monday. Right. I rebased the SEV pile ontop of latest upstream and now it applies much better: checking file drivers/crypto/ccp/Kconfig Hunk #1 succeeded at 32

Re: [RFC Part2 PATCH v3 02/26] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-09-06 Thread Borislav Petkov
On Wed, Sep 06, 2017 at 03:38:38PM -0500, Brijesh Singh wrote: > This bit of my struggle -- tip/master is not in sync with cryptodev-2.6 [1]. Aaha. > In order to expand the CCP driver we need the following commits from the > cryptodev-2.6 > > 57de3aefb73f crypto: ccp - remove ccp_present()

Re: [RFC Part2 PATCH v3 02/26] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-09-06 Thread Borislav Petkov
On Mon, Jul 24, 2017 at 03:02:39PM -0500, Brijesh Singh wrote: > Platform Security Processor (PSP) is part of AMD Secure Processor (AMD-SP), > PSP is a dedicated processor that provides the support for key management > commands in a Secure Encrypted Virtualiztion (SEV) mode, along with >

Re: [RFC PATCH v2 16/32] x86: kvm: Provide support to create Guest and HV shared per-CPU variables

2017-03-29 Thread Borislav Petkov
On Wed, Mar 29, 2017 at 05:21:13PM +0200, Paolo Bonzini wrote: > The GHCB would have to be allocated much earlier, possibly even by > firmware depending on how things will be designed. How about a statically allocated page like we do with the early pagetable pages in head_64.S? > I think it's

Re: [RFC PATCH v2 18/32] kvm: svm: Use the hardware provided GPA instead of page walk

2017-03-29 Thread Borislav Petkov
we won't know > which memory address was translated into EXITINFO2. > > Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com> > Reviewed-by: Borislav Petkov <b...@suse.de> I think I already asked you to remove Revewed-by tags when you have to change an already reviewed

Re: [RFC PATCH v2 16/32] x86: kvm: Provide support to create Guest and HV shared per-CPU variables

2017-03-28 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:15:36AM -0500, Brijesh Singh wrote: > Some KVM specific MSR's (steal-time, asyncpf, avic_eio) allocates per-CPU > variable at compile time and share its physical address with hypervisor. > It presents a challege when SEV is active in guest OS. When SEV is active, > guest

Re: [RFC PATCH v2 15/32] x86: Add support for changing memory encryption attribute in early boot

2017-03-24 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:15:28AM -0500, Brijesh Singh wrote: > Some KVM-specific custom MSRs shares the guest physical address with > hypervisor. When SEV is active, the shared physical address must be mapped > with encryption attribute cleared so that both hypervsior and guest can > access the

Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array

2017-03-17 Thread Borislav Petkov
On Fri, Mar 17, 2017 at 07:47:33PM +0100, Dmitry Vyukov wrote: > This problem is more general and is not specific to clang. It equally > applies to different versions of gcc, different arches and different > configs (namely, anything else than what a developer used for > testing). I guess. We do

Re: [RFC PATCH v2 14/32] x86: mm: Provide support to use memblock when spliting large pages

2017-03-16 Thread Borislav Petkov
On Fri, Mar 10, 2017 at 04:41:56PM -0600, Brijesh Singh wrote: > I can take a look at fixing those warning. In my initial attempt was to create > a new function to clear encryption bit but it ended up looking very similar to > __change_page_attr_set_clr() hence decided to extend the exiting

Re: [RFC PATCH v2 12/32] x86: Add early boot support when running with SEV active

2017-03-16 Thread Borislav Petkov
On Thu, Mar 16, 2017 at 11:11:26AM -0500, Tom Lendacky wrote: > Not quite. The guest still needs to understand about the encryption mask > so that it can protect memory by setting the encryption mask in the > pagetable entries. It can also decide when to share memory with the > hypervisor by not

Re: [RFC PATCH v2 12/32] x86: Add early boot support when running with SEV active

2017-03-16 Thread Borislav Petkov
On Thu, Mar 16, 2017 at 09:28:58AM -0500, Tom Lendacky wrote: > Because there are differences between how SME and SEV behave > (instruction fetches are always decrypted under SEV, DMA to an > encrypted location is not supported under SEV, etc.) we need to > determine which mode we are in so that

Re: [RFC PATCH v2 12/32] x86: Add early boot support when running with SEV active

2017-03-16 Thread Borislav Petkov
On Fri, Mar 10, 2017 at 10:35:30AM -0600, Brijesh Singh wrote: > We could update this patch to use the below logic: > > * CPUID(0) - Check for AuthenticAMD > * CPID(1) - Check if under hypervisor > * CPUID(0x8000) - Check for highest supported leaf > * CPUID(0x801F).EAX - Check for

Re: [RFC PATCH v2 14/32] x86: mm: Provide support to use memblock when spliting large pages

2017-03-10 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:15:15AM -0500, Brijesh Singh wrote: > If kernel_maps_pages_in_pgd is called early in boot process to change the kernel_map_pages_in_pgd() > memory attributes then it fails to allocate memory when spliting large > pages. The patch extends the cpa_data to provide the

Re: [RFC PATCH v2 13/32] KVM: SVM: Enable SEV by setting the SEV_ENABLE CPU feature

2017-03-09 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:15:01AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > Modify the SVM cpuid update function to indicate if Secure Encrypted > Virtualization (SEV) is active in the guest by setting the SEV KVM CPU > features bit. SEV is active if Secure

Re: [RFC PATCH v2 12/32] x86: Add early boot support when running with SEV active

2017-03-09 Thread Borislav Petkov
On Thu, Mar 09, 2017 at 05:13:33PM +0100, Paolo Bonzini wrote: > This is not how you check if running under a hypervisor; you should > check the HYPERVISOR bit, i.e. bit 31 of cpuid(1).ecx. This in turn > tells you if leaf 0x4000 is valid. Ah, good point, I already do that in the microcode

Re: [RFC PATCH v2 12/32] x86: Add early boot support when running with SEV active

2017-03-09 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:14:48AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > Early in the boot process, add checks to determine if the kernel is > running with Secure Encrypted Virtualization (SEV) active by issuing > a CPUID instruction. > > During early

Re: [RFC PATCH v2 10/32] x86: DMA support for SEV memory encryption

2017-03-08 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:14:25AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > DMA access to memory mapped as encrypted while SEV is active can not be > encrypted during device write or decrypted during device read. In order > for DMA to properly work when SEV

Re: [RFC PATCH v2 02/32] x86: Secure Encrypted Virtualization (SEV) support

2017-03-08 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:12:20AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > Provide support for Secure Encyrpted Virtualization (SEV). This initial > support defines a flag that is used by the kernel to determine if it is > running with SEV active. > >

Re: [RFC PATCH v2 09/32] x86: Change early_ioremap to early_memremap for BOOT data

2017-03-08 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:13:53AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > In order to map BOOT data with the proper encryption bit, the Btw, what does that all-caps spelling "BOOT" denote? Something I'm missing? > early_ioremap() function calls are

Re: [RFC PATCH v2 08/32] x86: Use PAGE_KERNEL protection for ioremap of memory page

2017-03-07 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:13:32AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > In order for memory pages to be properly mapped when SEV is active, we > need to use the PAGE_KERNEL protection attribute as the base protection. > This will insure that memory

Re: [RFC PATCH v2 07/32] x86/efi: Access EFI data as encrypted when SEV is active

2017-03-07 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:13:21AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > EFI data is encrypted when the kernel is run under SEV. Update the > page table references to be sure the EFI memory areas are accessed > encrypted. > > Signed-off-by: Tom Lendacky

Re: [RFC PATCH v2 02/32] x86: Secure Encrypted Virtualization (SEV) support

2017-03-07 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:12:20AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > Provide support for Secure Encyrpted Virtualization (SEV). This initial > support defines a flag that is used by the kernel to determine if it is > running with SEV active. > >

Re: [RFC PATCH v2 05/32] x86: Use encrypted access of BOOT related data with SEV

2017-03-07 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:12:59AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > When Secure Encrypted Virtualization (SEV) is active, BOOT data (such as > EFI related data, setup data) is encrypted and needs to be accessed as > such when mapped. Update the

Re: [RFC PATCH v2 04/32] KVM: SVM: Add SEV feature definitions to KVM

2017-03-06 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:12:48AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > Define a new KVM CPU feature for Secure Encrypted Virtualization (SEV). > The kernel will check for the presence of this feature to determine if > it is running with SEV active. > >

Re: [RFC PATCH v2 01/32] x86: Add the Secure Encrypted Virtualization CPU feature

2017-03-06 Thread Borislav Petkov
On Mon, Mar 06, 2017 at 01:11:03PM -0500, Brijesh Singh wrote: > Sending it through stg mail to avoid line wrapping. Please let me know if > something > is still messed up. I have tried applying it and it seems to apply okay. Yep, thanks. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF:

Re: [RFC PATCH v2 01/32] x86: Add the Secure Encrypted Virtualization CPU feature

2017-03-04 Thread Borislav Petkov
On Fri, Mar 03, 2017 at 03:01:23PM -0600, Brijesh Singh wrote: > +merely enables SME (sets bit 23 of the MSR_K8_SYSCFG), then Linux can > activate > +memory encryption by default (CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT=y) > or > +by supplying mem_encrypt=on on the kernel command line. However,

Re: [RFC PATCH v2 00/32] x86: Secure Encrypted Virtualization (AMD)

2017-03-03 Thread Borislav Petkov
On Fri, Mar 03, 2017 at 02:33:23PM -0600, Bjorn Helgaas wrote: > On Thu, Mar 02, 2017 at 10:12:01AM -0500, Brijesh Singh wrote: > > This RFC series provides support for AMD's new Secure Encrypted > > Virtualization > > (SEV) feature. This RFC is build upon Secure Memory Encryption (SME) RFCv4 >

Re: [RFC PATCH v2 01/32] x86: Add the Secure Encrypted Virtualization CPU feature

2017-03-03 Thread Borislav Petkov
On Thu, Mar 02, 2017 at 10:12:09AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > Update the CPU features to include identifying and reporting on the > Secure Encrypted Virtualization (SEV) feature. SME is identified by > CPUID 0x801f, but requires BIOS

Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active

2016-09-22 Thread Borislav Petkov
On Thu, Sep 22, 2016 at 02:49:22PM -0500, Tom Lendacky wrote: > > I thought that reduction is the reservation of bits for the SME mask. > > > > What other reduction is there? > > There is a reduction in physical address space for the SME mask and the > bits used to aid in identifying the ASID

Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active

2016-09-22 Thread Borislav Petkov
On Thu, Sep 22, 2016 at 08:23:36PM +0200, Paolo Bonzini wrote: > Unless this is part of some spec, it's easier if things are the same in > SME and SEV. Yeah, I was pondering over how sprinkling sev_active checks might not be so clean. I'm wondering if we could make the EFI regions presented to

Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active

2016-09-22 Thread Borislav Petkov
On Thu, Sep 22, 2016 at 07:08:50PM +0200, Paolo Bonzini wrote: > That's not how I read it. I just figured that the BIOS has some magic > things high in the physical address space and if you reduce the physical > address space the BIOS (which is called from e.g. EFI runtime services) > would have

Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active

2016-09-22 Thread Borislav Petkov
On Thu, Sep 22, 2016 at 05:05:54PM +0200, Paolo Bonzini wrote: > Which paragraph? "Linux relies on BIOS to set this bit if BIOS has determined that the reduction in the physical address space as a result of enabling memory encryption..." Basically, you can enable SME in the BIOS and you're all

Re: [RFC PATCH v1 04/28] x86: Secure Encrypted Virtualization (SEV) support

2016-09-22 Thread Borislav Petkov
On Mon, Aug 22, 2016 at 07:24:19PM -0400, Brijesh Singh wrote: > From: Tom Lendacky Subject: [RFC PATCH v1 04/28] x86: Secure Encrypted Virtualization (SEV) support Please start patch commit heading with a verb, i.e.: "x86: Add AMD Secure Encrypted Virtualization (SEV)

Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active

2016-09-22 Thread Borislav Petkov
On Thu, Sep 22, 2016 at 04:45:51PM +0200, Paolo Bonzini wrote: > The main difference between the SME and SEV encryption, from the point > of view of the kernel, is that real-mode always writes unencrypted in > SME and always writes encrypted in SEV. But UEFI can run in 64-bit mode > and learn

Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active

2016-09-22 Thread Borislav Petkov
On Mon, Aug 22, 2016 at 07:25:25PM -0400, Brijesh Singh wrote: > From: Tom Lendacky > > EFI data is encrypted when the kernel is run under SEV. Update the > page table references to be sure the EFI memory areas are accessed > encrypted. > > Signed-off-by: Tom Lendacky

Re: [RFC PATCH v1 03/28] kvm: svm: Use the hardware provided GPA instead of page walk

2016-09-21 Thread Borislav Petkov
|2 ++ > arch/x86/kvm/x86.c | 17 - > 4 files changed, 24 insertions(+), 1 deletion(-) FWIW, LGTM. (Gotta love replying in acronyms :-)) Reviewed-by: Borislav Petkov <b...@suse.de> -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, J

  1   2   >