From: clabbe.montjoie@gmail.com (Corentin LABBE)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
Date: Thu, 22 May 2014 20:13:03 +0200 [thread overview]
Message-ID: <537E3E2F.7000407@gmail.com> (raw)
In-Reply-To: <7774492.yVryDeoI4M@wuerfel>
Le 22/05/2014 17:28, Arnd Bergmann a ?crit :
> On Thursday 22 May 2014 17:09:56 LABBE Corentin wrote:
>> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
>> ---
>> drivers/crypto/Kconfig | 49 ++
>> drivers/crypto/Makefile | 1 +
>> drivers/crypto/sunxi-ss.c | 1476 +++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 1526 insertions(+)
>> create mode 100644 drivers/crypto/sunxi-ss.c
>>
>> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
>> index 03ccdb0..5ea0922 100644
>> --- a/drivers/crypto/Kconfig
>> +++ b/drivers/crypto/Kconfig
>> @@ -418,4 +418,53 @@ config CRYPTO_DEV_MXS_DCP
>> To compile this driver as a module, choose M here: the module
>> will be called mxs-dcp.
>>
>> +config CRYPTO_DEV_SUNXI_SS
>> + tristate "Support for Allwinner Security System cryptographic accelerator"
>> + depends on ARCH_SUNXI
>> + help
>> + Some Allwinner processors have a crypto accelerator named
>> + Security System. Select this if you want to use it.
>> +
>> + To compile this driver as a module, choose M here: the module
>> + will be called sunxi-ss.
>> +
>> +if CRYPTO_DEV_SUNXI_SS
>> +config CRYPTO_DEV_SUNXI_SS_PRNG
>> + bool "Security System PRNG"
>> + select CRYPTO_RNG2
>> + help
>> + If you enable this option, the SS will provide a pseudo random
>> + number generator.
>> +config CRYPTO_DEV_SUNXI_SS_MD5
>> + bool "Security System MD5"
>> + select CRYPTO_MD5
>> + help
>> + If you enable this option, the SS will provide MD5 hardware
>> + acceleration.
>
> My feeling is that this should either be one driver that provides
> all five algorithms unconditionally, or five drivers that are each
> separate loadable modules and based on top of a common module
> that only exports functions but has no active logic itself
I agree for the split.
It was my first intention but I feared to add too many files.
So I propose to split in 6, sunxi-ss-hash.c, sunxi-ss-des.c, sunxi-ss-aes.c, sunxi-ss-rng.c, sunxi-ss-common.c and sunxi-ss.h
Does can I add a sunxi-ss directory in drivers/crypto ?
>
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
>> +#include <crypto/md5.h>
>> +#define SUNXI_SS_HASH_COMMON
>> +#endif
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
>> +#include <crypto/sha.h>
>> +#define SUNXI_SS_HASH_COMMON
>> +#endif
>> +#ifdef SUNXI_SS_HASH_COMMON
>> +#include <crypto/hash.h>
>> +#include <crypto/internal/hash.h>
>> +#endif
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
>> +#include <crypto/aes.h>
>> +#endif
>> +
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
>> +#define SUNXI_SS_DES
>> +#endif
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
>> +#define SUNXI_SS_DES
>> +#endif
>> +#ifdef SUNXI_SS_DES
>> +#include <crypto/des.h>
>> +#endif
>> +
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
>> +#include <crypto/internal/rng.h>
>
> That would cleanup this #ifdef chain either way.
>
>> +/* General notes:
>> + * I cannot use a key/IV cache because each time one of these change ALL stuff
>> + * need to be re-writed.
>> + * And for example, with dm-crypt IV changes on each request.
>> + *
>> + * After each request the device must be disabled.
>> + *
>> + * For performance reason, we use writel_relaxed/read_relaxed for all
>> + * operations on RX and TX FIFO.
>> + * For all other registers, we use writel.
>> + * See http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117644
>> + * and http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117640
>> + * */
>> +
>> +static struct sunxi_ss_ctx {
>> + void *base;
>
> __iomem ?
ok
>
>> + int irq;
>> + struct clk *busclk;
>> + struct clk *ssclk;
>> + struct device *dev;
>> + struct resource *res;
>> + void *buf_in; /* pointer to data to be uploaded to the device */
>> + size_t buf_in_size; /* size of buf_in */
>> + void *buf_out;
>> + size_t buf_out_size;
>> +} _ss_ctx, *ss_ctx = &_ss_ctx;
>> +
>> +static DEFINE_MUTEX(lock);
>> +static DEFINE_MUTEX(bufout_lock);
>> +static DEFINE_MUTEX(bufin_lock);
>
> I guess the mutexes should be part of sunxi_ss_ctx.
>
> Are you sure you need the global _ss_ctx structure?
> Normally you should get a pointer from whoever calls you.
I will use platform_get_drvdata/platform_set_drvdata for cleaning all thoses global structures..
>
>> +struct sunxi_req_ctx {
>> + u8 key[AES_MAX_KEY_SIZE * 8];
>> + u32 keylen;
>> + u32 mode;
>> + u64 byte_count; /* number of bytes "uploaded" to the device */
>> + u32 waitbuf; /* a partial word waiting to be completed and
>> + uploaded to the device */
>> + /* number of bytes to be uploaded in the waitbuf word */
>> + unsigned int nbwait;
>> +};
>> +
>> +#ifdef SUNXI_SS_HASH_COMMON
>> +/*============================================================================*/
>> +/*============================================================================*/
>
> Please remove all the ======== lines.
ok
>
>> + /* If we have more than one SG, we cannot use kmap_atomic since
>> + * we hold the mapping too long*/
>> + src_addr = kmap(sg_page(in_sg)) + in_sg->offset;
>> + if (src_addr == NULL) {
>> + dev_err(ss_ctx->dev, "KMAP error for src SG\n");
>> + return -1;
>> + }
>> + dst_addr = kmap(sg_page(out_sg)) + out_sg->offset;
>> + if (dst_addr == NULL) {
>> + kunmap(src_addr);
>> + dev_err(ss_ctx->dev, "KMAP error for dst SG\n");
>> + return -1;
>> + }
>> + src32 = (u32 *)src_addr;
>> + dst32 = (u32 *)dst_addr;
>> + ileft = nbytes / 4;
>> + oleft = nbytes / 4;
>> + sgileft = in_sg->length / 4;
>> + sgoleft = out_sg->length / 4;
>> + do {
>> + tmp = readl_relaxed(ss_ctx->base + SUNXI_SS_FCSR);
>> + rx_cnt = SS_RXFIFO_SPACES(tmp);
>> + tx_cnt = SS_TXFIFO_SPACES(tmp);
>> + todo = min3(rx_cnt, ileft, sgileft);
>> + if (todo > 0) {
>> + ileft -= todo;
>> + sgileft -= todo;
>> + }
>> + while (todo > 0) {
>> + writel_relaxed(*src32++, ss_ctx->base + SS_RXFIFO);
>> + todo--;
>> + }
>
> I wonder if this is meant to be used in combination with a dma engine
> rather than accessed with writel/readl.
You could do both, but the dmaengine driver is under development.
When it will be ready, I will add DMA support.
But my intention is to keep both mode, since poll mode is better than DMA for small request.
>
> How does the original driver do it?
There are no original driver, this driver is the first for the Security System.
>
> Arnd
>
Thanks for your review.
next prev parent reply other threads:[~2014-05-22 18:13 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-22 15:09 crypto: Add Allwinner Security System crypto accelerator LABBE Corentin
2014-05-22 15:09 ` [PATCH 1/3] dt: Add DT bindings documentation for SUNXI Security System LABBE Corentin
2014-05-24 11:21 ` Marek Vasut
2014-05-24 19:20 ` Tomasz Figa
2014-05-24 19:43 ` Marek Vasut
2014-05-24 19:51 ` Tomasz Figa
2014-05-24 19:59 ` Marek Vasut
2014-05-25 13:09 ` Maxime Ripard
2014-05-25 13:07 ` Maxime Ripard
2014-05-22 15:09 ` [PATCH 2/3] ARM: sun7i: dt: Add Security System to A20 SoC DTS LABBE Corentin
2014-05-24 11:23 ` Marek Vasut
2014-05-22 15:09 ` [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator LABBE Corentin
2014-05-22 15:28 ` Arnd Bergmann
2014-05-22 18:13 ` Corentin LABBE [this message]
2014-05-23 10:46 ` Arnd Bergmann
2014-05-24 11:26 ` Marek Vasut
2014-05-24 12:00 ` Marek Vasut
2014-05-25 11:58 ` Corentin LABBE
2014-05-25 14:30 ` Marek Vasut
2014-07-23 13:57 ` Herbert Xu
2014-07-23 14:07 ` Marek Vasut
2014-07-23 14:13 ` Herbert Xu
2014-07-23 15:51 ` Marek Vasut
2014-07-23 18:52 ` Corentin LABBE
2014-07-23 19:38 ` Marek Vasut
2014-07-24 1:40 ` Herbert Xu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=537E3E2F.7000407@gmail.com \
--to=clabbe.montjoie@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).