Re: [WIP/RFC] crypto: add support for Orion5X crypto engine
* Evgeniy Polyakov | 2009-03-18 01:42:43 [+0300]:
>Hi.
Hi Evgeniy,
>On Tue, Mar 17, 2009 at 10:58:44PM +0100, Sebastian Andrzej Siewior
>(arm-ker...@ml.breakpoint.cc) wrote:
>> +struct crypto_priv {
>
>Please use less generic names over the file.
will do.
>> +void __iomem *reg;
>> +void __iomem *sram;
>> +int irq;
>> +struct task_struct *queue_th;
>> +
>> +spinlock_t lock;
>> +struct crypto_queue queue;
>> +enum engine_status eng_st;
>> +struct ablkcipher_request *cur_req;
>> +struct req_progress p;
>> +};
>> +
>> +static struct crypto_priv *cpg;
>> +
>
>This rises several questions: why some of its fields are accessed under
>the lock and others are modified without. Some of them are only used in
>the kernel thread, while others are used in request context.
>Please document locking bits in the code.
Will do. Right now, I could switch to the _bh spinlock since it is only
required to access the queue. Earlier I planned to enqueue the first
request directly on the hw but then I got into the scatter walk.
>> +static void reg_write(void __iomem *mem, u32 val)
>> +{
>> +__raw_writel(val, mem);
>> +}
>> +
>> +static u32 reg_read(void __iomem *mem)
>> +{
>> +return __raw_readl(mem);
>> +}
>> +
>
>Seems like you do not like underscores otherwise you would use those
>functions directly.
Yes, I could do so. I wasn't sure whether those are the correct
functions to access the memory. If there were for some reason I could
replace them in one place. I switch to __.
>> +
>> +#define MAX_REQ_SIZE(8000)
>> +
>
>Parentheses are not needed.
yup.
>> +irqreturn_t crypto_int(int irq, void *priv)
>> +{
>> +// struct crypto_priv *cp = priv;
>> +u32 val;
>> +
>> +val = reg_read(cpg->reg + SEC_ACCEL_INT_STATUS);
>> +reg_write(cpg->reg + SEC_ACCEL_INT_MASK, 0);
>
>Why do you ack interrupt before checking if interrupt belongs to
>this driver?
I don't know. I better fix this.
>> +if (!(val & SEC_INT_ACCEL0_DONE))
>> +return IRQ_NONE;
>> +
>> +BUG_ON(cpg->eng_st != engine_busy);
>> +cpg->eng_st = engine_w_dequeue;
>> +wake_up_process(cpg->queue_th);
>> +return IRQ_HANDLED;
>> +}
>
>--
> Evgeniy Polyakov
Thanks for looking over.
Sebastian
--
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: [WIP/RFC] crypto: add support for Orion5X crypto engine
* Paulius Zaleckas [2009-03-18 17:55]: > > +config CRYPTO_DEV_MARVELL_CRYPTO_ENGINE > > CRYPTO_DEV...CRYPTO_ENGINE > Maybe CRYPTO_DEV_MARVELL would be enough? ... and since we're talking about naming issues, I think you should replace "mav" with "mv" to be in line with other uses inside the kernel (e.g. sata-mv). Thanks for working on this driver! -- Martin Michlmayr http://www.cyrius.com/ -- 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: [WIP/RFC] crypto: add support for Orion5X crypto engine
Sebastian Andrzej Siewior wrote: > This is version two of the the driver. New things: > - aes-ecb passes selftests > - aes-cbc passes selftests > > The driver still does memcpy() from/to sram. To solve this, a dma driver > would be required but first I wanted to compare the performance between > now and nothing/generic aes. However I managed to crash cryptsetup with > luksOpen. Got look into this... > > Signed-off-by: Sebastian Andrzej Siewior > --- > drivers/crypto/Kconfig |9 + > drivers/crypto/Makefile |1 + > drivers/crypto/mav_crypto.c | 724 > +++ > 3 files changed, 734 insertions(+), 0 deletions(-) > create mode 100644 drivers/crypto/mav_crypto.c > > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig > index 01afd75..514fe78 100644 > --- a/drivers/crypto/Kconfig > +++ b/drivers/crypto/Kconfig > @@ -157,6 +157,15 @@ config S390_PRNG > ANSI X9.17 standard. The PRNG is usable via the char device > /dev/prandom. > > +config CRYPTO_DEV_MARVELL_CRYPTO_ENGINE CRYPTO_DEV...CRYPTO_ENGINE Maybe CRYPTO_DEV_MARVELL would be enough? > + tristate "Marvell's Cryptographic Engine" > + depends on PLAT_ORION > + select CRYPTO_ALGAPI > + select CRYPTO_AES > + help > + This driver allows you utilize the cryptographic engine which can be > + found on certain SoC like QNAP's TS-209. > + -- 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: [WIP/RFC] crypto: add support for Orion5X crypto engine
Hi.
On Tue, Mar 17, 2009 at 10:58:44PM +0100, Sebastian Andrzej Siewior
(arm-ker...@ml.breakpoint.cc) wrote:
> +struct crypto_priv {
Please use less generic names over the file.
> + void __iomem *reg;
> + void __iomem *sram;
> + int irq;
> + struct task_struct *queue_th;
> +
> + spinlock_t lock;
> + struct crypto_queue queue;
> + enum engine_status eng_st;
> + struct ablkcipher_request *cur_req;
> + struct req_progress p;
> +};
> +
> +static struct crypto_priv *cpg;
> +
This rises several questions: why some of its fields are accessed under
the lock and others are modified without. Some of them are only used in
the kernel thread, while others are used in request context.
Please document locking bits in the code.
> +static void reg_write(void __iomem *mem, u32 val)
> +{
> + __raw_writel(val, mem);
> +}
> +
> +static u32 reg_read(void __iomem *mem)
> +{
> + return __raw_readl(mem);
> +}
> +
Seems like you do not like underscores otherwise you would use those
functions directly.
> +
> +#define MAX_REQ_SIZE (8000)
> +
Parentheses are not needed.
> +irqreturn_t crypto_int(int irq, void *priv)
> +{
> +// struct crypto_priv *cp = priv;
> + u32 val;
> +
> + val = reg_read(cpg->reg + SEC_ACCEL_INT_STATUS);
> + reg_write(cpg->reg + SEC_ACCEL_INT_MASK, 0);
Why do you ack interrupt before checking if interrupt belongs to
this driver?
> + if (!(val & SEC_INT_ACCEL0_DONE))
> + return IRQ_NONE;
> +
> + BUG_ON(cpg->eng_st != engine_busy);
> + cpg->eng_st = engine_w_dequeue;
> + wake_up_process(cpg->queue_th);
> + return IRQ_HANDLED;
> +}
--
Evgeniy Polyakov
--
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
[WIP/RFC] crypto: add support for Orion5X crypto engine
This is version two of the the driver. New things:
- aes-ecb passes selftests
- aes-cbc passes selftests
The driver still does memcpy() from/to sram. To solve this, a dma driver
would be required but first I wanted to compare the performance between
now and nothing/generic aes. However I managed to crash cryptsetup with
luksOpen. Got look into this...
Signed-off-by: Sebastian Andrzej Siewior
---
drivers/crypto/Kconfig |9 +
drivers/crypto/Makefile |1 +
drivers/crypto/mav_crypto.c | 724 +++
3 files changed, 734 insertions(+), 0 deletions(-)
create mode 100644 drivers/crypto/mav_crypto.c
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 01afd75..514fe78 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -157,6 +157,15 @@ config S390_PRNG
ANSI X9.17 standard. The PRNG is usable via the char device
/dev/prandom.
+config CRYPTO_DEV_MARVELL_CRYPTO_ENGINE
+ tristate "Marvell's Cryptographic Engine"
+ depends on PLAT_ORION
+ select CRYPTO_ALGAPI
+ select CRYPTO_AES
+ help
+ This driver allows you utilize the cryptographic engine which can be
+ found on certain SoC like QNAP's TS-209.
+
config CRYPTO_DEV_HIFN_795X
tristate "Driver HIFN 795x crypto accelerator chips"
select CRYPTO_DES
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index 9bf4a2b..9c7053c 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_CRYPTO_DEV_PADLOCK_AES) += padlock-aes.o
obj-$(CONFIG_CRYPTO_DEV_PADLOCK_SHA) += padlock-sha.o
obj-$(CONFIG_CRYPTO_DEV_GEODE) += geode-aes.o
obj-$(CONFIG_CRYPTO_DEV_HIFN_795X) += hifn_795x.o
+obj-$(CONFIG_CRYPTO_DEV_MARVELL_CRYPTO_ENGINE) += mav_crypto.o
obj-$(CONFIG_CRYPTO_DEV_TALITOS) += talitos.o
obj-$(CONFIG_CRYPTO_DEV_IXP4XX) += ixp4xx_crypto.o
obj-$(CONFIG_CRYPTO_DEV_PPC4XX) += amcc/
diff --git a/drivers/crypto/mav_crypto.c b/drivers/crypto/mav_crypto.c
new file mode 100644
index 000..07152e7
--- /dev/null
+++ b/drivers/crypto/mav_crypto.c
@@ -0,0 +1,724 @@
+/*
+ * Support for Marvell's crypto engine which can be found on some Orion5X
+ * boards.
+ *
+ * Author: Sebastian Andrzej Siewior < sebastian at breakpoint dot cc >
+ * License: GPL
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+enum engine_status {
+ engine_idle,
+ engine_busy,
+ engine_w_dequeue,
+};
+
+struct req_progress {
+ struct sg_mapping_iter src_sg_it;
+ struct sg_mapping_iter dst_sg_it;
+
+ /* src mostly */
+ int this_sg_b_left;
+ int src_start;
+ int crypt_len;
+ /* dst mostly */
+ int this_dst_sg_b_left;
+ int dst_start;
+ int total_req_bytes;
+};
+
+struct crypto_priv {
+ void __iomem *reg;
+ void __iomem *sram;
+ int irq;
+ struct task_struct *queue_th;
+
+ spinlock_t lock;
+ struct crypto_queue queue;
+ enum engine_status eng_st;
+ struct ablkcipher_request *cur_req;
+ struct req_progress p;
+};
+
+static struct crypto_priv *cpg;
+
+static void reg_write(void __iomem *mem, u32 val)
+{
+ __raw_writel(val, mem);
+}
+
+static u32 reg_read(void __iomem *mem)
+{
+ return __raw_readl(mem);
+}
+
+#define DIGEST_INITIAL_VAL_A 0xdd00
+#define DES_CMD_REG0xdd58
+
+#define SEC_ACCEL_CMD 0xde00
+#define SEC_CMD_EN_SEC_ACCL0 (1 << 0)
+#define SEC_CMD_EN_SEC_ACCL1 (1 << 1)
+#define SEC_CMD_DISABLE_SEC(1 << 2)
+
+#define SEC_ACCEL_DESC_P0 0xde04
+#define SEC_DESC_P0_PTR(x) (x)
+
+#define SEC_ACCEL_DESC_P1 0xde14
+#define SEC_DESC_P1_PTR(x) (x)
+
+#define SEC_ACCEL_CFG 0xde08
+#define SEC_CFG_STOP_DIG_ERR (1 << 0)
+#define SEC_CFG_CH0_W_IDMA (1 << 7)
+#define SEC_CFG_CH1_W_IDMA (1 << 8)
+#define SEC_CFG_ACT_CH0_IDMA (1 << 9)
+#define SEC_CFG_ACT_CH1_IDMA (1 << 10)
+
+#define SEC_ACCEL_STATUS 0xde0c
+#define SEC_ST_ACT_0 (1 << 0)
+#define SEC_ST_ACT_1 (1 << 1)
+
+
+#define SEC_ACCEL_INT_STATUS 0xde20
+#define SEC_INT_AUTH_DONE (1 << 0)
+#define SEC_INT_DES_E_DONE (1 << 1)
+#define SEC_INT_AES_E_DONE (1 << 2)
+#define SEC_INT_AES_D_DONE (1 << 3)
+#define SEC_INT_ENC_DONE (1 << 4)
+#define SEC_INT_ACCEL0_DONE(1 << 5)
+#define SEC_INT_ACCEL1_DONE(1 << 6)
+#define SEC_INT_ACC0_IDMA_DONE (1 << 7)
+#define SEC_INT_ACC1_IDMA_DONE (1 << 8)
+
+#define SEC_ACCEL_INT_MASK 0xde24
+
+#define AES_KEY_LEN(8 * 4)
+
+struct sec_accel_config {
+
+ u32 config;
+#define CFG_OP_MAC_ONLY(0)
+#define CFG_OP_CRYPT_ONLY (1)
+#define CFG_OP_MAC_CRYPT (2)
+#define CFG_OP_CRYPT_MAC (3)
+#define CFG_MACM_MD5 (4 << 4)
+#define CFG_MACM_SHA1 (5 << 4)
+#define CFG_MACM_HMAC_MD5 (6 << 4)
+#define CFG_MACM_HMAC_SHA1 (7 <<
