Re: [PATCH] crypto: ccp - Assign DMA commands to the channel's CCP

2017-03-16 Thread Herbert Xu
On Fri, Mar 10, 2017 at 12:28:18PM -0600, Gary R Hook wrote:
> From: Gary R Hook 
> 
> The CCP driver generally uses a round-robin approach when
> assigning operations to available CCPs. For the DMA engine,
> however, the DMA mappings of the SGs are associated with a
> specific CCP. When an IOMMU is enabled, the IOMMU is
> programmed based on this specific device.
> 
> If the DMA operations are not performed by that specific
> CCP then addressing errors and I/O page faults will occur.
> 
> Update the CCP driver to allow a specific CCP device to be
> requested for an operation and use this in the DMA engine
> support.
> 
> Cc:  # 4.9.x-
> Signed-off-by: Gary R Hook 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


Re: [PATCH] crypto: ccp - Assign DMA commands to the channel's CCP

2017-03-14 Thread Stephan Müller
Am Montag, 13. März 2017, 20:35:07 CET schrieb Gary R Hook:

Hi Gary,

> On 03/03/2017 7:15 AM, Stephan Mueller wrote:
> > Am Donnerstag, 2. März 2017, 22:26:54 CET schrieb Gary R Hook:
> > 
> > Hi Gary,
> 
> Thanks for your comments, Stephan.
> 
> > > A version 5 device provides the primitive commands
> > > required for AES GCM. This patch adds support for
> > > en/decryption.
> > > 
> > > Signed-off-by: Gary R Hook 
> > > ---
> > > 
> > >  drivers/crypto/ccp/Makefile|1
> > >  drivers/crypto/ccp/ccp-crypto-aes-galois.c |  257
> > > 
> > >  drivers/crypto/ccp/ccp-crypto-main.c  
> > > |
> > > 12 +
> > > 
> > >  drivers/crypto/ccp/ccp-crypto.h|   14 ++
> > >  drivers/crypto/ccp/ccp-dev-v5.c|2
> > >  drivers/crypto/ccp/ccp-dev.h   |1
> > >  drivers/crypto/ccp/ccp-ops.c   |  252
> > > 
> > > +++ include/linux/ccp.h|
> > > 9 +
> > > 
> > >  8 files changed, 548 insertions(+)
> > >  create mode 100644 drivers/crypto/ccp/ccp-crypto-aes-galois.c
> > > 
> > > diff --git a/drivers/crypto/ccp/ccp-crypto-aes-galois.c
> > > b/drivers/crypto/ccp/ccp-crypto-aes-galois.c new file mode 100644
> > > index 000..8bc18c9
> > > --- /dev/null
> > > +++ b/drivers/crypto/ccp/ccp-crypto-aes-galois.c
> > > @@ -0,0 +1,257 @@
> > > +/*
> > > + * AMD Cryptographic Coprocessor (CCP) AES GCM crypto API support
> > > + *
> > > + * Copyright (C) 2016 Advanced Micro Devices, Inc.
> > > + *
> > > + * Author: Gary R Hook 
> > > + *
> > > + * 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 "ccp-crypto.h"
> > > +
> > > +#defineAES_GCM_IVSIZE  12
> > > +
> > > +static int ccp_aes_gcm_complete(struct crypto_async_request *async_req,
> > > int ret) +{
> > > +   return ret;
> > > +}
> > > +
> > > +static int ccp_aes_gcm_setkey(struct crypto_aead *tfm, const u8 *key,
> > > + unsigned int key_len)
> > > +{
> > > +   struct ccp_ctx *ctx = crypto_aead_ctx(tfm);
> > > +
> > > +   switch (key_len) {
> > > +   case AES_KEYSIZE_128:
> > > +   ctx->u.aes.type = CCP_AES_TYPE_128;
> > > +   break;
> > > +   case AES_KEYSIZE_192:
> > > +   ctx->u.aes.type = CCP_AES_TYPE_192;
> > > +   break;
> > > +   case AES_KEYSIZE_256:
> > > +   ctx->u.aes.type = CCP_AES_TYPE_256;
> > > +   break;
> > > +   default:
> > > +   crypto_aead_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
> > > +   return -EINVAL;
> > > +   }
> > > +
> > > +   ctx->u.aes.mode = CCP_AES_MODE_GCM;
> > > +   ctx->u.aes.key_len = key_len;
> > > +
> > > +   memcpy(ctx->u.aes.key, key, key_len);
> > > +   sg_init_one(&ctx->u.aes.key_sg, ctx->u.aes.key, key_len);
> > > +
> > > +   return 0;
> > > +}
> > > +
> > > +static int ccp_aes_gcm_setauthsize(struct crypto_aead *tfm,
> > > +  unsigned int authsize)
> > > +{
> > > +   return 0;
> > > +}
> > > +
> > > +static int ccp_aes_gcm_crypt(struct aead_request *req, bool encrypt)
> > > +{
> > > +   struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> > > +   struct ccp_ctx *ctx = crypto_aead_ctx(tfm);
> > > +   struct ccp_aes_req_ctx *rctx = aead_request_ctx(req);
> > > +   struct scatterlist *iv_sg = NULL;
> > > +   unsigned int iv_len = 0;
> > > +   int i;
> > > +   int ret = 0;
> > > +
> > > +   if (!ctx->u.aes.key_len)
> > > +   return -EINVAL;
> > > +
> > > +   if (ctx->u.aes.mode != CCP_AES_MODE_GCM)
> > > +   return -EINVAL;
> > > +
> > > +   if (!req->iv)
> > > +   return -EINVAL;
> > > +
> > > +   /*
> > > +* 5 parts:
> > > +*   plaintext/ciphertext input
> > > +*   AAD
> > > +*   key
> > > +*   IV
> > > +*   Destination+tag buffer
> > > +*/
> > > +
> > > +   /* According to the way AES GCM has been implemented here,
> > > +* per RFC 4106 it seems, the provided IV is fixed at 12 bytes,
> > 
> > When you have that restriction, should the cipher be called
> > rfc4106(gcm(aes))?
> > 
> > But then, the key is 4 bytes longer than a normal AES key as it contains
> > the leading 32 bits of the IV.
> 
> I had my wires crossed due to an incomplete understanding of an AEAD cipher
> in general, and GCM in particular. I'm hopeful that someone can help me
> understand:
> 
> For the AES GCM encryption tests in testmgr.h, where there is an IV,
> they're all
> 12 bytes in length. As I understand AES GCM the IV can be anywhere from
> 1 to 2^64
> bits in length; the value of 96 makes for convenience and efficiency.
> But it's
> neither a requirement nor restriction.

Re: [PATCH] crypto: ccp - Assign DMA commands to the channel's CCP

2017-03-13 Thread Gary R Hook

On 03/03/2017 7:15 AM, Stephan Mueller wrote:


Am Donnerstag, 2. März 2017, 22:26:54 CET schrieb Gary R Hook:

Hi Gary,


Thanks for your comments, Stephan.



> A version 5 device provides the primitive commands
> required for AES GCM. This patch adds support for
> en/decryption.
>
> Signed-off-by: Gary R Hook 
> ---
>  drivers/crypto/ccp/Makefile|1
>  drivers/crypto/ccp/ccp-crypto-aes-galois.c |  257
>  drivers/crypto/ccp/ccp-crypto-main.c   |
> 12 +
>  drivers/crypto/ccp/ccp-crypto.h|   14 ++
>  drivers/crypto/ccp/ccp-dev-v5.c|2
>  drivers/crypto/ccp/ccp-dev.h   |1
>  drivers/crypto/ccp/ccp-ops.c   |  252
> +++ include/linux/ccp.h|
> 9 +
>  8 files changed, 548 insertions(+)
>  create mode 100644 drivers/crypto/ccp/ccp-crypto-aes-galois.c
>
> diff --git a/drivers/crypto/ccp/ccp-crypto-aes-galois.c
> b/drivers/crypto/ccp/ccp-crypto-aes-galois.c new file mode 100644
> index 000..8bc18c9
> --- /dev/null
> +++ b/drivers/crypto/ccp/ccp-crypto-aes-galois.c
> @@ -0,0 +1,257 @@
> +/*
> + * AMD Cryptographic Coprocessor (CCP) AES GCM crypto API support
> + *
> + * Copyright (C) 2016 Advanced Micro Devices, Inc.
> + *
> + * Author: Gary R Hook 
> + *
> + * 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 "ccp-crypto.h"
> +
> +#defineAES_GCM_IVSIZE  12
> +
> +static int ccp_aes_gcm_complete(struct crypto_async_request *async_req, int
> ret) +{
> +   return ret;
> +}
> +
> +static int ccp_aes_gcm_setkey(struct crypto_aead *tfm, const u8 *key,
> + unsigned int key_len)
> +{
> +   struct ccp_ctx *ctx = crypto_aead_ctx(tfm);
> +
> +   switch (key_len) {
> +   case AES_KEYSIZE_128:
> +   ctx->u.aes.type = CCP_AES_TYPE_128;
> +   break;
> +   case AES_KEYSIZE_192:
> +   ctx->u.aes.type = CCP_AES_TYPE_192;
> +   break;
> +   case AES_KEYSIZE_256:
> +   ctx->u.aes.type = CCP_AES_TYPE_256;
> +   break;
> +   default:
> +   crypto_aead_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
> +   return -EINVAL;
> +   }
> +
> +   ctx->u.aes.mode = CCP_AES_MODE_GCM;
> +   ctx->u.aes.key_len = key_len;
> +
> +   memcpy(ctx->u.aes.key, key, key_len);
> +   sg_init_one(&ctx->u.aes.key_sg, ctx->u.aes.key, key_len);
> +
> +   return 0;
> +}
> +
> +static int ccp_aes_gcm_setauthsize(struct crypto_aead *tfm,
> +  unsigned int authsize)
> +{
> +   return 0;
> +}
> +
> +static int ccp_aes_gcm_crypt(struct aead_request *req, bool encrypt)
> +{
> +   struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> +   struct ccp_ctx *ctx = crypto_aead_ctx(tfm);
> +   struct ccp_aes_req_ctx *rctx = aead_request_ctx(req);
> +   struct scatterlist *iv_sg = NULL;
> +   unsigned int iv_len = 0;
> +   int i;
> +   int ret = 0;
> +
> +   if (!ctx->u.aes.key_len)
> +   return -EINVAL;
> +
> +   if (ctx->u.aes.mode != CCP_AES_MODE_GCM)
> +   return -EINVAL;
> +
> +   if (!req->iv)
> +   return -EINVAL;
> +
> +   /*
> +* 5 parts:
> +*   plaintext/ciphertext input
> +*   AAD
> +*   key
> +*   IV
> +*   Destination+tag buffer
> +*/
> +
> +   /* According to the way AES GCM has been implemented here,
> +* per RFC 4106 it seems, the provided IV is fixed at 12 bytes,

When you have that restriction, should the cipher be called rfc4106(gcm(aes))?

But then, the key is 4 bytes longer than a normal AES key as it contains the
leading 32 bits of the IV.


I had my wires crossed due to an incomplete understanding of an AEAD cipher
in general, and GCM in particular. I'm hopeful that someone can help me
understand:

For the AES GCM encryption tests in testmgr.h, where there is an IV, 
they're all
12 bytes in length. As I understand AES GCM the IV can be anywhere from 
1 to 2^64
bits in length; the value of 96 makes for convenience and efficiency. 
But it's

neither a requirement nor restriction.

There are no tests (in testmgr.h) that use an IV length other than  0 or 96.
My comment about RFC4106 has to do with requiring an IV 0f 96 bits + a 
word that
is incremented for each block (making every nonce unique, per the 
requirement).

But let's ignore that, please.

It looks as if:

What seems to be missing is the ability to register a (GCM) transform 
that can
handle an IV of arbitrary (allowable) length. I have to specify the 
length (ivsize)
when I register an algorithm, and everything I see in the existing code 
appears
to expect a GCM ivsize to be 96 bits, period (or zero). This is what I 
meant when
I referenced RFC4106: I perceive restricti

[PATCH] crypto: ccp - Assign DMA commands to the channel's CCP

2017-03-10 Thread Gary R Hook
From: Gary R Hook 

The CCP driver generally uses a round-robin approach when
assigning operations to available CCPs. For the DMA engine,
however, the DMA mappings of the SGs are associated with a
specific CCP. When an IOMMU is enabled, the IOMMU is
programmed based on this specific device.

If the DMA operations are not performed by that specific
CCP then addressing errors and I/O page faults will occur.

Update the CCP driver to allow a specific CCP device to be
requested for an operation and use this in the DMA engine
support.

Cc:  # 4.9.x-
Signed-off-by: Gary R Hook 
---
 drivers/crypto/ccp/ccp-dev.c   |5 -
 drivers/crypto/ccp/ccp-dmaengine.c |1 +
 include/linux/ccp.h|2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c
index 511ab04..92d1c69 100644
--- a/drivers/crypto/ccp/ccp-dev.c
+++ b/drivers/crypto/ccp/ccp-dev.c
@@ -283,11 +283,14 @@ unsigned int ccp_version(void)
  */
 int ccp_enqueue_cmd(struct ccp_cmd *cmd)
 {
-   struct ccp_device *ccp = ccp_get_device();
+   struct ccp_device *ccp;
unsigned long flags;
unsigned int i;
int ret;
 
+   /* Some commands might need to be sent to a specific device */
+   ccp = cmd->ccp ? cmd->ccp : ccp_get_device();
+
if (!ccp)
return -ENODEV;
 
diff --git a/drivers/crypto/ccp/ccp-dmaengine.c 
b/drivers/crypto/ccp/ccp-dmaengine.c
index e5d9278..8d0eeb4 100644
--- a/drivers/crypto/ccp/ccp-dmaengine.c
+++ b/drivers/crypto/ccp/ccp-dmaengine.c
@@ -390,6 +390,7 @@ static struct ccp_dma_desc *ccp_create_desc(struct dma_chan 
*dma_chan,
goto err;
 
ccp_cmd = &cmd->ccp_cmd;
+   ccp_cmd->ccp = chan->ccp;
ccp_pt = &ccp_cmd->u.passthru_nomap;
ccp_cmd->flags = CCP_CMD_MAY_BACKLOG;
ccp_cmd->flags |= CCP_CMD_PASSTHRU_NO_DMA_MAP;
diff --git a/include/linux/ccp.h b/include/linux/ccp.h
index c71dd8f..c41b8d99 100644
--- a/include/linux/ccp.h
+++ b/include/linux/ccp.h
@@ -556,7 +556,7 @@ enum ccp_engine {
  * struct ccp_cmd - CCP operation request
  * @entry: list element (ccp driver use only)
  * @work: work element used for callbacks (ccp driver use only)
- * @ccp: CCP device to be run on (ccp driver use only)
+ * @ccp: CCP device to be run on
  * @ret: operation return code (ccp driver use only)
  * @flags: cmd processing flags
  * @engine: CCP operation to perform