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

2017-10-03 Thread Brijesh Singh 



On 10/03/2017 11:17 AM, Borislav Petkov wrote:
...



No, please add my patch below to your set for the CRYPTO_DEV_CCP_DD
dependency as it is a separate thing. Your patch should concentrate only
on adding the PSP and its dependencies.



Sure, I will include your patch in my series. thanks



---
From: Borislav Petkov 
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 present it in Kconfig only when AMD
CPU support is enabled or on ARM64 where it is also used.

Signed-off-by: Borislav Petkov 
Cc: Brijesh Singh 
Cc: Tom Lendacky 
Cc: Gary Hook 
Cc: Herbert Xu 
Cc: "David S. Miller" 
Cc: linux-crypto@vger.kernel.org
---
  drivers/crypto/ccp/Kconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/ccp/Kconfig b/drivers/crypto/ccp/Kconfig
index 627f3e61dcac..f19f57162225 100644
--- a/drivers/crypto/ccp/Kconfig
+++ b/drivers/crypto/ccp/Kconfig
@@ -1,5 +1,6 @@
  config CRYPTO_DEV_CCP_DD
tristate "Secure Processor device driver"
+   depends on CPU_SUP_AMD || ARM64
default m
help
  Provides AMD Secure Processor device driver.

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

2017-10-03 Thread Borislav Petkov 
On Sun, Oct 01, 2017 at 03:05:11PM -0500, Brijesh Singh wrote:
> I think theoretically a 32-bit host OS can invoke a PSP commands but
> currently PSP interface is exposing only the SEV FW command. And SEV

Let's cross that bridge when we get to it.

> feature is available when we are in 64-bit mode hence for now its okay
> to have depends on X86_64. I will add CRYPTO_DEV_CCP_DD depend on
> CPU_SUP_AMD || ARM64 and CRYPTO_DEV_SP_PSP depend on X86_64 and send you
> v4.2.

No, please add my patch below to your set for the CRYPTO_DEV_CCP_DD
dependency as it is a separate thing. Your patch should concentrate only
on adding the PSP and its dependencies.

Thx.

---
From: Borislav Petkov 
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 present it in Kconfig only when AMD
CPU support is enabled or on ARM64 where it is also used.

Signed-off-by: Borislav Petkov 
Cc: Brijesh Singh 
Cc: Tom Lendacky 
Cc: Gary Hook 
Cc: Herbert Xu 
Cc: "David S. Miller" 
Cc: linux-crypto@vger.kernel.org
---
 drivers/crypto/ccp/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/ccp/Kconfig b/drivers/crypto/ccp/Kconfig
index 627f3e61dcac..f19f57162225 100644
--- a/drivers/crypto/ccp/Kconfig
+++ b/drivers/crypto/ccp/Kconfig
@@ -1,5 +1,6 @@
 config CRYPTO_DEV_CCP_DD
tristate "Secure Processor device driver"
+   depends on CPU_SUP_AMD || ARM64
default m
help
  Provides AMD Secure Processor device driver.
-- 
2.13.0

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-01 Thread Brijesh Singh 


On 9/30/17 11:11 AM, Borislav Petkov wrote:
> I think just from having CRYPTO_DEV_CCP_DD depend on CPU_SUP_AMD ||
> ARM64, CRYPTO_DEV_SP_PSP gets almost the same dependency transitively.
> But sure, let's make the PSP build only on x86. It should depend on
> X86_64, to be precise.

I think theoretically a 32-bit host OS can invoke a PSP commands but
currently PSP interface is exposing only the SEV FW command. And SEV
feature is available when we are in 64-bit mode hence for now its okay
to have depends on X86_64. I will add CRYPTO_DEV_CCP_DD depend on
CPU_SUP_AMD || ARM64 and CRYPTO_DEV_SP_PSP depend on X86_64 and send you
v4.2. thanks

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 depend on CPU_SUP_AMD ||
ARM64, CRYPTO_DEV_SP_PSP gets almost the same dependency transitively.
But sure, let's make the PSP build only on x86. It should depend on
X86_64, to be precise.

> Yes its very much possible. The SEV FW provides two sets of commands 1)
> platform certificate management and 2) guest management
> 
> The platform certificate management commands is used outside the
> CONFIG_KVM_AMD.

Ok, please state that in the commit message so that it is written down
somewhere.

Thx.

-- 
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-09-30 Thread Brijesh Singh 


On 9/29/17 10:16 AM, Borislav Petkov wrote:
...

> +
>> +config CRYPTO_DEV_SP_PSP
>> +bool "Platform Security Processor (PSP) device"
>> +default y
>> +depends on CRYPTO_DEV_CCP_DD
> So this last symbol CRYPTO_DEV_CCP_DD is default m and it doesn't depend
> on anything. And I'm pretty sure it should depend on CPU_SUP_AMD as this
> is AMD-specific hw. You can add that dependency in a prepatch.


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.


> And what happened to adding dependencies on CONFIG_KVM_AMD? Or can you
> use the PSP without virtualization in any sensible way?

Yes its very much possible. The SEV FW provides two sets of commands 1)
platform certificate management and 2) guest management

The platform certificate management commands is used outside the
CONFIG_KVM_AMD.

-Brijesh

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

2017-09-29 Thread Borislav Petkov 
On Tue, Sep 19, 2017 at 03:46:03PM -0500, Brijesh Singh wrote:
> Platform Security Processor (PSP) is part of AMD Secure Processor (AMD-SP),

The Platform...

> PSP is a dedicated processor that provides the support for key management
> commands in a Secure Encrypted Virtualiztion (SEV) mode, along with

Virtualization

Is integrating that spellchecker hard? Because what I do, for example,
is press F7 in vim when I've written the commit message. And F7 is
mapped to:

map  :set spell! spelllang=en_us 
spellfile=~/.vim/spellfile.add:echo "spellcheck: " . strpart("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 
> Cc: "Radim Krčmář" 
> Cc: Borislav Petkov 
> Cc: Herbert Xu 
> Cc: Gary Hook 
> Cc: Tom Lendacky 
> Cc: linux-crypto@vger.kernel.org
> Cc: k...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Signed-off-by: Brijesh Singh 
> ---
>  drivers/crypto/ccp/Kconfig   |  11 +
>  drivers/crypto/ccp/Makefile  |   1 +
>  drivers/crypto/ccp/psp-dev.c | 111 
> +++
>  drivers/crypto/ccp/psp-dev.h |  61 
>  drivers/crypto/ccp/sp-dev.c  |  32 +
>  drivers/crypto/ccp/sp-dev.h  |  27 ++-
>  drivers/crypto/ccp/sp-pci.c  |  46 ++
>  7 files changed, 288 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/crypto/ccp/psp-dev.c
>  create mode 100644 drivers/crypto/ccp/psp-dev.h
> 
> diff --git a/drivers/crypto/ccp/Kconfig b/drivers/crypto/ccp/Kconfig
> index 6d626606b9c5..1d927e13bf31 100644
> --- a/drivers/crypto/ccp/Kconfig
> +++ b/drivers/crypto/ccp/Kconfig
> @@ -32,3 +32,14 @@ config CRYPTO_DEV_CCP_CRYPTO
> Support for using the cryptographic API with the AMD Cryptographic
> Coprocessor. This module supports offload of SHA and AES algorithms.
> If you choose 'M' here, this module will be called ccp_crypto.
> +
> +config CRYPTO_DEV_SP_PSP
> + bool "Platform Security Processor (PSP) device"
> + default y
> + depends on CRYPTO_DEV_CCP_DD

So this last symbol CRYPTO_DEV_CCP_DD is default m and it doesn't depend
on anything. And I'm pretty sure it should depend on CPU_SUP_AMD as this
is AMD-specific hw. You can add that dependency in a prepatch.

And what happened to adding dependencies on CONFIG_KVM_AMD? Or can you
use the PSP without virtualization in any sensible way?

> + help
> +  Provide the support for AMD Platform Security Processor (PSP). PSP is

... for the AMD ... The PSP 
... 

> +  a dedicated processor that provides the support for key management

that provides support for

> +  commands in in a Secure Encrypted Virtualiztion (SEV) mode, along with

... in Secure Encrypted Virtualization

> +  software-based Trusted Executation Environment (TEE) to enable the
> +  third-party trusted applications.
> diff --git a/drivers/crypto/ccp/Makefile b/drivers/crypto/ccp/Makefile
> index 57f8debfcfb3..008bae7e26ec 100644
> --- a/drivers/crypto/ccp/Makefile
> +++ b/drivers/crypto/ccp/Makefile
> @@ -7,6 +7,7 @@ ccp-$(CONFIG_CRYPTO_DEV_SP_CCP) += ccp-dev.o \
>   ccp-dmaengine.o \
>   ccp-debugfs.o
>  ccp-$(CONFIG_PCI) += sp-pci.o
> +ccp-$(CONFIG_CRYPTO_DEV_SP_PSP) += psp-dev.o
>  
>  obj-$(CONFIG_CRYPTO_DEV_CCP_CRYPTO) += ccp-crypto.o
>  ccp-crypto-objs := ccp-crypto-main.o \
> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
> new file mode 100644
> index ..e60e53272e71
> --- /dev/null
> +++ b/drivers/crypto/ccp/psp-dev.c
> @@ -0,0 +1,111 @@
> +/*
> + * AMD Platform Security Processor (PSP) interface
> + *
> + * Copyright (C) 2016-2017 Advanced Micro Devices, Inc.
> + *
> + * Author: Brijesh Singh 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "sp-dev.h"
> +#include "psp-dev.h"
> +
> +const struct psp_vdata psp_entry = {
> + .offset = 0x10500,
> +};
> +
> +static struct psp_device *psp_alloc_struct(struct sp_device *sp)
> +{
> + struct device *dev = sp->dev;
> + struct psp_device *psp;
> +
> + psp = devm_kzalloc(dev, sizeof(*psp), GFP_KERNEL);
> + if (!psp)
> + return NULL;
> +
> + psp->dev = dev;
> + psp->sp = sp;
> +
> + snprintf(psp->name, sizeof(psp->name), "psp-%u",

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

2017-09-19 Thread Brijesh Singh 
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
software-based Trusted Executation Environment (TEE) to enable the
third-party trusted applications.

Cc: Paolo Bonzini 
Cc: "Radim Krčmář" 
Cc: Borislav Petkov 
Cc: Herbert Xu 
Cc: Gary Hook 
Cc: Tom Lendacky 
Cc: linux-crypto@vger.kernel.org
Cc: k...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Brijesh Singh 
---
 drivers/crypto/ccp/Kconfig   |  11 +
 drivers/crypto/ccp/Makefile  |   1 +
 drivers/crypto/ccp/psp-dev.c | 111 +++
 drivers/crypto/ccp/psp-dev.h |  61 
 drivers/crypto/ccp/sp-dev.c  |  32 +
 drivers/crypto/ccp/sp-dev.h  |  27 ++-
 drivers/crypto/ccp/sp-pci.c  |  46 ++
 7 files changed, 288 insertions(+), 1 deletion(-)
 create mode 100644 drivers/crypto/ccp/psp-dev.c
 create mode 100644 drivers/crypto/ccp/psp-dev.h

diff --git a/drivers/crypto/ccp/Kconfig b/drivers/crypto/ccp/Kconfig
index 6d626606b9c5..1d927e13bf31 100644
--- a/drivers/crypto/ccp/Kconfig
+++ b/drivers/crypto/ccp/Kconfig
@@ -32,3 +32,14 @@ config CRYPTO_DEV_CCP_CRYPTO
  Support for using the cryptographic API with the AMD Cryptographic
  Coprocessor. This module supports offload of SHA and AES algorithms.
  If you choose 'M' here, this module will be called ccp_crypto.
+
+config CRYPTO_DEV_SP_PSP
+   bool "Platform Security Processor (PSP) device"
+   default y
+   depends on CRYPTO_DEV_CCP_DD
+   help
+Provide the support for AMD Platform Security Processor (PSP). PSP is
+a dedicated processor that provides the support for key management
+commands in in a Secure Encrypted Virtualiztion (SEV) mode, along with
+software-based Trusted Executation Environment (TEE) to enable the
+third-party trusted applications.
diff --git a/drivers/crypto/ccp/Makefile b/drivers/crypto/ccp/Makefile
index 57f8debfcfb3..008bae7e26ec 100644
--- a/drivers/crypto/ccp/Makefile
+++ b/drivers/crypto/ccp/Makefile
@@ -7,6 +7,7 @@ ccp-$(CONFIG_CRYPTO_DEV_SP_CCP) += ccp-dev.o \
ccp-dmaengine.o \
ccp-debugfs.o
 ccp-$(CONFIG_PCI) += sp-pci.o
+ccp-$(CONFIG_CRYPTO_DEV_SP_PSP) += psp-dev.o
 
 obj-$(CONFIG_CRYPTO_DEV_CCP_CRYPTO) += ccp-crypto.o
 ccp-crypto-objs := ccp-crypto-main.o \
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
new file mode 100644
index ..e60e53272e71
--- /dev/null
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -0,0 +1,111 @@
+/*
+ * AMD Platform Security Processor (PSP) interface
+ *
+ * Copyright (C) 2016-2017 Advanced Micro Devices, Inc.
+ *
+ * Author: Brijesh Singh 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "sp-dev.h"
+#include "psp-dev.h"
+
+const struct psp_vdata psp_entry = {
+   .offset = 0x10500,
+};
+
+static struct psp_device *psp_alloc_struct(struct sp_device *sp)
+{
+   struct device *dev = sp->dev;
+   struct psp_device *psp;
+
+   psp = devm_kzalloc(dev, sizeof(*psp), GFP_KERNEL);
+   if (!psp)
+   return NULL;
+
+   psp->dev = dev;
+   psp->sp = sp;
+
+   snprintf(psp->name, sizeof(psp->name), "psp-%u", sp->ord);
+
+   return psp;
+}
+
+irqreturn_t psp_irq_handler(int irq, void *data)
+{
+   return IRQ_HANDLED;
+}
+
+int psp_dev_init(struct sp_device *sp)
+{
+   struct device *dev = sp->dev;
+   struct psp_device *psp;
+   int ret;
+
+   ret = -ENOMEM;
+   psp = psp_alloc_struct(sp);
+   if (!psp)
+   goto e_err;
+   sp->psp_data = psp;
+
+   psp->vdata = (struct psp_vdata *)sp->dev_vdata->psp_vdata;
+   if (!psp->vdata) {
+   ret = -ENODEV;
+   dev_err(dev, "missing driver data\n");
+   goto e_err;
+   }
+
+   psp->io_regs = sp->io_map + psp->vdata->offset;
+
+   /* Disable and clear interrupts until ready */
+   iowrite32(0, psp->io_regs + PSP_P2CMSG_INTEN);
+   iowrite32(-1, psp->io_regs + PSP_P2CMSG_INTSTS);
+
+   dev_dbg(dev, "requesting an IRQ ...\n");
+   /* Request an irq */
+   ret = sp_request_psp_irq(psp->sp, psp_irq_handler, psp->name, psp);
+   if (ret) {
+   dev_err(dev, "psp: unable to allocate an IRQ\n");
+   goto e_err;
+   }
+
+   sp_set_psp_master(sp);
+
+   /* Enable