Re: [PATCH] crypto: ccp - Register the CCP as a DMA resource

2016-04-05 Thread Gary R Hook



On 04/04/2016 04:47 PM, Tom Lendacky wrote:

On 04/04/2016 03:50 PM, Gary R Hook wrote:

The CCP has the ability to provide DMA services to the
kernel using pass-through mode of the device. Register
these services as general purpose DMA channels.
---

You're missing a cc: to David Miller, be sure to check who
should be included when emailing.

D'oh! Of course.

  drivers/crypto/ccp/Kconfig |1
  drivers/crypto/ccp/Makefile|6
  drivers/crypto/ccp/ccp-dev-v3.c|   13 +
  drivers/crypto/ccp/ccp-dev.h   |   49 ++
  drivers/crypto/ccp/ccp-dmaengine.c |  718 
  drivers/crypto/ccp/ccp-ops.c   |   77 
  6 files changed, 856 insertions(+), 8 deletions(-)
  create mode 100644 drivers/crypto/ccp/ccp-dmaengine.c

For some reason the diffstat is missing include/linux/ccp.h


Ack.


  
@@ -408,11 +408,19 @@ static int ccp_init(struct ccp_device *ccp)
  
  	ccp_add_device(ccp);
  
+	/* Register the DMA engine support */

+   ret = ccp_dmaengine_register(ccp);
+   if (ret)
+   goto e_hwrng;
+

This either needs to be before ccp_add_device() or you need to
remove the device in the error path.

Ack. Device registration should be the final step.

/* Enable interrupts */
iowrite32(qim, ccp->io_regs + IRQ_MASK_REG);
  
  	return 0;
  
+e_hwrng:

+   hwrng_unregister(&ccp->hwrng);
+
  e_kthread:
for (i = 0; i < ccp->cmd_q_count; i++)
if (ccp->cmd_q[i].kthread)
@@ -436,6 +444,9 @@ static void ccp_destroy(struct ccp_device *ccp)
/* Remove this device from the list of available units first */
ccp_del_device(ccp);
  
+	/* Unregister the DMA engine */

+   ccp_dmaengine_unregister(ccp);
+
/* Unregister the RNG */
hwrng_unregister(&ccp->hwrng);
  
diff --git a/drivers/crypto/ccp/ccp-dev.h b/drivers/crypto/ccp/ccp-dev.h

index 7745d0b..aa447a7 100644
--- a/drivers/crypto/ccp/ccp-dev.h
+++ b/drivers/crypto/ccp/ccp-dev.h
@@ -22,9 +22,12 @@
  #include 
  #include 
  #include 
+#include 
+#include 
+#include 
  
  #define MAX_CCP_NAME_LEN		16

-#define MAX_DMAPOOL_NAME_LEN   32
+#define MAX_DMA_NAME_LEN   40

Any reason this needed to be increased to 40? Though this change
may not be needed based on comment below.

This will be removed, per the comment below.



diff --git a/drivers/crypto/ccp/ccp-dmaengine.c 
b/drivers/crypto/ccp/ccp-dmaengine.c
new file mode 100644
index 000..241ad8a
--- /dev/null
+++ b/drivers/crypto/ccp/ccp-dmaengine.c
@@ -0,0 +1,718 @@
+/*
+ * AMD Cryptographic Coprocessor (CCP) driver
+ *
+ * Copyright (C) 2015 Advanced Micro Devices, Inc.

2016.


+ *
+ * Author: Tom Lendacky 

This should be your name.

Ack.

...


+int ccp_dmaengine_register(struct ccp_device *ccp)
+{
+   struct ccp_dma_chan *chan;
+   struct dma_device *dma_dev = &ccp->dma_dev;
+   struct dma_chan *dma_chan;
+   char dma_cache_name[MAX_DMA_NAME_LEN];

This can't be a local function variable.  You'll need to allocate
memory for the cache names and track them (or use devm_kasprintf).
While kmem_cache_create() dups the string, a path down to 
sysfs_slab_alias() shows that the pointer is saved elsewhere. 
devm_kasprintf() will be used to build the cache name string.




diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c
index eefdf59..3467a1e 100644
--- a/drivers/crypto/ccp/ccp-ops.c
+++ b/drivers/crypto/ccp/ccp-ops.c
@@ -1311,7 +1311,7 @@ static int ccp_run_passthru_cmd(struct ccp_cmd_queue 
*cmd_q,
if (!pt->final && (pt->src_len & (CCP_PASSTHRU_BLOCKSIZE - 1)))
return -EINVAL;
  
-	if (!pt->src || !pt->dst)

+   if (!pt->src_sg || !pt->dst_sg)
No reason to change this in this patch. If you're trying to distinguish
between sg and dma addr because of the new passthru function you should
change the src and dst name in the new function - src_dma / dst_dma.

Done.


--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] crypto: ccp - Register the CCP as a DMA resource

2016-04-04 Thread Tom Lendacky
On 04/04/2016 03:50 PM, Gary R Hook wrote:
> The CCP has the ability to provide DMA services to the
> kernel using pass-through mode of the device. Register
> these services as general purpose DMA channels.
> ---

You're missing a cc: to David Miller, be sure to check who
should be included when emailing.

>  drivers/crypto/ccp/Kconfig |1 
>  drivers/crypto/ccp/Makefile|6 
>  drivers/crypto/ccp/ccp-dev-v3.c|   13 +
>  drivers/crypto/ccp/ccp-dev.h   |   49 ++
>  drivers/crypto/ccp/ccp-dmaengine.c |  718 
> 
>  drivers/crypto/ccp/ccp-ops.c   |   77 
>  6 files changed, 856 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/crypto/ccp/ccp-dmaengine.c

For some reason the diffstat is missing include/linux/ccp.h

> 
> diff --git a/drivers/crypto/ccp/Kconfig b/drivers/crypto/ccp/Kconfig
> index 6e37845..79cabfb 100644
> --- a/drivers/crypto/ccp/Kconfig
> +++ b/drivers/crypto/ccp/Kconfig
> @@ -3,6 +3,7 @@ config CRYPTO_DEV_CCP_DD
>   depends on CRYPTO_DEV_CCP
>   default m
>   select HW_RANDOM
> + select DMA_ENGINE
>   select CRYPTO_SHA1
>   select CRYPTO_SHA256
>   help
> diff --git a/drivers/crypto/ccp/Makefile b/drivers/crypto/ccp/Makefile
> index b750592..ee4d274 100644
> --- a/drivers/crypto/ccp/Makefile
> +++ b/drivers/crypto/ccp/Makefile
> @@ -1,5 +1,9 @@
>  obj-$(CONFIG_CRYPTO_DEV_CCP_DD) += ccp.o
> -ccp-objs := ccp-dev.o ccp-ops.o ccp-dev-v3.o ccp-platform.o
> +ccp-objs := ccp-dev.o \
> + ccp-ops.o \
> + ccp-dev-v3.o \
> + ccp-platform.o \
> + ccp-dmaengine.o
>  ccp-$(CONFIG_PCI) += ccp-pci.o
>  
>  obj-$(CONFIG_CRYPTO_DEV_CCP_CRYPTO) += ccp-crypto.o
> diff --git a/drivers/crypto/ccp/ccp-dev-v3.c b/drivers/crypto/ccp/ccp-dev-v3.c
> index 7d5eab4..65babf4 100644
> --- a/drivers/crypto/ccp/ccp-dev-v3.c
> +++ b/drivers/crypto/ccp/ccp-dev-v3.c
> @@ -290,7 +290,7 @@ static int ccp_init(struct ccp_device *ccp)
>   struct device *dev = ccp->dev;
>   struct ccp_cmd_queue *cmd_q;
>   struct dma_pool *dma_pool;
> - char dma_pool_name[MAX_DMAPOOL_NAME_LEN];
> + char dma_pool_name[MAX_DMA_NAME_LEN];
>   unsigned int qmr, qim, i;
>   int ret;
>  
> @@ -408,11 +408,19 @@ static int ccp_init(struct ccp_device *ccp)
>  
>   ccp_add_device(ccp);
>  
> + /* Register the DMA engine support */
> + ret = ccp_dmaengine_register(ccp);
> + if (ret)
> + goto e_hwrng;
> +

This either needs to be before ccp_add_device() or you need to
remove the device in the error path.

>   /* Enable interrupts */
>   iowrite32(qim, ccp->io_regs + IRQ_MASK_REG);
>  
>   return 0;
>  
> +e_hwrng:
> + hwrng_unregister(&ccp->hwrng);
> +
>  e_kthread:
>   for (i = 0; i < ccp->cmd_q_count; i++)
>   if (ccp->cmd_q[i].kthread)
> @@ -436,6 +444,9 @@ static void ccp_destroy(struct ccp_device *ccp)
>   /* Remove this device from the list of available units first */
>   ccp_del_device(ccp);
>  
> + /* Unregister the DMA engine */
> + ccp_dmaengine_unregister(ccp);
> +
>   /* Unregister the RNG */
>   hwrng_unregister(&ccp->hwrng);
>  
> diff --git a/drivers/crypto/ccp/ccp-dev.h b/drivers/crypto/ccp/ccp-dev.h
> index 7745d0b..aa447a7 100644
> --- a/drivers/crypto/ccp/ccp-dev.h
> +++ b/drivers/crypto/ccp/ccp-dev.h
> @@ -22,9 +22,12 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>  
>  #define MAX_CCP_NAME_LEN 16
> -#define MAX_DMAPOOL_NAME_LEN 32
> +#define MAX_DMA_NAME_LEN 40

Any reason this needed to be increased to 40? Though this change
may not be needed based on comment below.

>  
>  #define MAX_HW_QUEUES5
>  #define MAX_CMD_QLEN 100
> @@ -167,6 +170,39 @@ extern struct ccp_vdata ccpv3;
>  struct ccp_device;
>  struct ccp_cmd;
>  
> +struct ccp_dma_cmd {
> + struct list_head entry;
> +
> + struct ccp_cmd ccp_cmd;
> +};
> +
> +struct ccp_dma_desc {
> + struct list_head entry;
> +
> + struct ccp_device *ccp;
> +
> + struct list_head pending;
> + struct list_head active;
> +
> + enum dma_status status;
> + struct dma_async_tx_descriptor tx_desc;
> + size_t len;
> +};
> +
> +struct ccp_dma_chan {
> + struct ccp_device *ccp;
> +
> + spinlock_t lock;
> + struct list_head pending;
> + struct list_head active;
> + struct list_head complete;
> +
> + struct tasklet_struct cleanup_tasklet;
> +
> + enum dma_status status;
> + struct dma_chan dma_chan;
> +};
> +
>  struct ccp_cmd_queue {
>   struct ccp_device *ccp;
>  
> @@ -261,6 +297,14 @@ struct ccp_device {
>   unsigned int hwrng_retries;
>  
>   /*
> +  * Support for the CCP DMA capabilities
> +  */
> + struct dma_device dma_dev;
> + struct ccp_dma_chan *ccp_dma_chan;
> + struct kmem_cache *dma_cmd_cache;
> + struct kmem_cache *dma_desc_cache;
> +
> 

[PATCH] crypto: ccp - Register the CCP as a DMA resource

2016-04-04 Thread Gary R Hook
The CCP has the ability to provide DMA services to the
kernel using pass-through mode of the device. Register
these services as general purpose DMA channels.
---
 drivers/crypto/ccp/Kconfig |1 
 drivers/crypto/ccp/Makefile|6 
 drivers/crypto/ccp/ccp-dev-v3.c|   13 +
 drivers/crypto/ccp/ccp-dev.h   |   49 ++
 drivers/crypto/ccp/ccp-dmaengine.c |  718 
 drivers/crypto/ccp/ccp-ops.c   |   77 
 6 files changed, 856 insertions(+), 8 deletions(-)
 create mode 100644 drivers/crypto/ccp/ccp-dmaengine.c

diff --git a/drivers/crypto/ccp/Kconfig b/drivers/crypto/ccp/Kconfig
index 6e37845..79cabfb 100644
--- a/drivers/crypto/ccp/Kconfig
+++ b/drivers/crypto/ccp/Kconfig
@@ -3,6 +3,7 @@ config CRYPTO_DEV_CCP_DD
depends on CRYPTO_DEV_CCP
default m
select HW_RANDOM
+   select DMA_ENGINE
select CRYPTO_SHA1
select CRYPTO_SHA256
help
diff --git a/drivers/crypto/ccp/Makefile b/drivers/crypto/ccp/Makefile
index b750592..ee4d274 100644
--- a/drivers/crypto/ccp/Makefile
+++ b/drivers/crypto/ccp/Makefile
@@ -1,5 +1,9 @@
 obj-$(CONFIG_CRYPTO_DEV_CCP_DD) += ccp.o
-ccp-objs := ccp-dev.o ccp-ops.o ccp-dev-v3.o ccp-platform.o
+ccp-objs := ccp-dev.o \
+   ccp-ops.o \
+   ccp-dev-v3.o \
+   ccp-platform.o \
+   ccp-dmaengine.o
 ccp-$(CONFIG_PCI) += ccp-pci.o
 
 obj-$(CONFIG_CRYPTO_DEV_CCP_CRYPTO) += ccp-crypto.o
diff --git a/drivers/crypto/ccp/ccp-dev-v3.c b/drivers/crypto/ccp/ccp-dev-v3.c
index 7d5eab4..65babf4 100644
--- a/drivers/crypto/ccp/ccp-dev-v3.c
+++ b/drivers/crypto/ccp/ccp-dev-v3.c
@@ -290,7 +290,7 @@ static int ccp_init(struct ccp_device *ccp)
struct device *dev = ccp->dev;
struct ccp_cmd_queue *cmd_q;
struct dma_pool *dma_pool;
-   char dma_pool_name[MAX_DMAPOOL_NAME_LEN];
+   char dma_pool_name[MAX_DMA_NAME_LEN];
unsigned int qmr, qim, i;
int ret;
 
@@ -408,11 +408,19 @@ static int ccp_init(struct ccp_device *ccp)
 
ccp_add_device(ccp);
 
+   /* Register the DMA engine support */
+   ret = ccp_dmaengine_register(ccp);
+   if (ret)
+   goto e_hwrng;
+
/* Enable interrupts */
iowrite32(qim, ccp->io_regs + IRQ_MASK_REG);
 
return 0;
 
+e_hwrng:
+   hwrng_unregister(&ccp->hwrng);
+
 e_kthread:
for (i = 0; i < ccp->cmd_q_count; i++)
if (ccp->cmd_q[i].kthread)
@@ -436,6 +444,9 @@ static void ccp_destroy(struct ccp_device *ccp)
/* Remove this device from the list of available units first */
ccp_del_device(ccp);
 
+   /* Unregister the DMA engine */
+   ccp_dmaengine_unregister(ccp);
+
/* Unregister the RNG */
hwrng_unregister(&ccp->hwrng);
 
diff --git a/drivers/crypto/ccp/ccp-dev.h b/drivers/crypto/ccp/ccp-dev.h
index 7745d0b..aa447a7 100644
--- a/drivers/crypto/ccp/ccp-dev.h
+++ b/drivers/crypto/ccp/ccp-dev.h
@@ -22,9 +22,12 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 #define MAX_CCP_NAME_LEN   16
-#define MAX_DMAPOOL_NAME_LEN   32
+#define MAX_DMA_NAME_LEN   40
 
 #define MAX_HW_QUEUES  5
 #define MAX_CMD_QLEN   100
@@ -167,6 +170,39 @@ extern struct ccp_vdata ccpv3;
 struct ccp_device;
 struct ccp_cmd;
 
+struct ccp_dma_cmd {
+   struct list_head entry;
+
+   struct ccp_cmd ccp_cmd;
+};
+
+struct ccp_dma_desc {
+   struct list_head entry;
+
+   struct ccp_device *ccp;
+
+   struct list_head pending;
+   struct list_head active;
+
+   enum dma_status status;
+   struct dma_async_tx_descriptor tx_desc;
+   size_t len;
+};
+
+struct ccp_dma_chan {
+   struct ccp_device *ccp;
+
+   spinlock_t lock;
+   struct list_head pending;
+   struct list_head active;
+   struct list_head complete;
+
+   struct tasklet_struct cleanup_tasklet;
+
+   enum dma_status status;
+   struct dma_chan dma_chan;
+};
+
 struct ccp_cmd_queue {
struct ccp_device *ccp;
 
@@ -261,6 +297,14 @@ struct ccp_device {
unsigned int hwrng_retries;
 
/*
+* Support for the CCP DMA capabilities
+*/
+   struct dma_device dma_dev;
+   struct ccp_dma_chan *ccp_dma_chan;
+   struct kmem_cache *dma_cmd_cache;
+   struct kmem_cache *dma_desc_cache;
+
+   /*
 * A counter used to generate job-ids for cmds submitted to the CCP
 */
atomic_t current_id cacheline_aligned;
@@ -418,4 +462,7 @@ int ccp_cmd_queue_thread(void *data);
 
 int ccp_run_cmd(struct ccp_cmd_queue *cmd_q, struct ccp_cmd *cmd);
 
+int ccp_dmaengine_register(struct ccp_device *ccp);
+void ccp_dmaengine_unregister(struct ccp_device *ccp);
+
 #endif
diff --git a/drivers/crypto/ccp/ccp-dmaengine.c 
b/drivers/crypto/ccp/ccp-dmaengine.c
new file mode 100644
index 000..241ad8a
--- /dev/null
+++ b/drivers/crypto/ccp/ccp-dmaengine.c
@@ -0,0 +1,7