* Re: [PATCH 0/7] arm64: marvell: add cryptographic engine support for 7k/8k
From: Herbert Xu @ 2017-04-12 6:11 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Antoine Tenart, davem, jason, andrew, sebastian.hesselbarth,
linux-crypto, linux-arm-kernel, thomas.petazzoni, boris.brezillon,
oferh, igall, nadavh
In-Reply-To: <87efwyj53q.fsf@free-electrons.com>
On Tue, Apr 11, 2017 at 06:12:25PM +0200, Gregory CLEMENT wrote:
>
> > I have no problems with the crypto bits.
>
> Does it means that you agree that I apply the arm-soc related patches
> (from 4 to 7) to my mvebu trees?
Yes sure.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH v3 RESEND] dm: switch dm-verity to async hash crypto API
From: Gilad Ben-Yossef @ 2017-04-12 7:25 UTC (permalink / raw)
To: dm-devel, Alasdair Kergon, Mike Snitzer
Cc: linux-crypto, gilad.benyossef, ofir.drang, Milan Broz,
linux-kernel, Eric Biggers, Ondrej Mosnáček
Use of the synchronous digest API limits dm-verity to using pure
CPU based algorithm providers and rules out the use of off CPU
algorithm providers which are normally asynchronous by nature,
potentially freeing CPU cycles.
This can reduce performance per Watt in situations such as during
boot time when a lot of concurrent file accesses are made to the
protected volume.
Move DM_VERITY to the asynchronous hash API.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Tested-by: Milan Broz <gmazyland@gmail.com>
CC: Eric Biggers <ebiggers3@gmail.com>
CC: Ondrej Mosnáček <omosnacek+linux-crypto@gmail.com>
---
Resending since I did not get any feedback beyond Milan confirmation that
the latest version passes his simple sanity test.
The patch was tested by me on an Armv7 based dual core Zynq ZC706 development
board with SHA256-asm, SHA256-neon synchronous providers with no visible
degradation of performance, with cryptd based asynchronous versions of the
same and with an off tree Arm CryptoCell asynchronous provider.
Changes from v2:
- Use completion to potentially wait also on crypto_ahash_init() as it
may finish asynchronously as well in some drivers, such as cryptd, as
discovered by Milan Broz.
- Use sg_init_one() where apropriate as pointed out by Milan Broz.
Changes from v1:
- Use a 0 mask to allocate crypto alg indicating we welcome async algo
providers, as suggested by Ondrej Mosnáček.
- Fix use of un-initialized completion when using async provider for IO
blocks hashing
- Pass flag indicating we are OK with crypto provider backlog
- Re-factor checking for need to wait into a common function
drivers/md/dm-verity-fec.c | 4 +-
drivers/md/dm-verity-target.c | 202 +++++++++++++++++++++++++++++-------------
drivers/md/dm-verity.h | 23 +++--
3 files changed, 158 insertions(+), 71 deletions(-)
drivers/md/dm-verity-fec.c | 4 +-
drivers/md/dm-verity-target.c | 201 +++++++++++++++++++++++++++++-------------
drivers/md/dm-verity.h | 23 +++--
3 files changed, 157 insertions(+), 71 deletions(-)
diff --git a/drivers/md/dm-verity-fec.c b/drivers/md/dm-verity-fec.c
index 0f0eb8a..dab98fe 100644
--- a/drivers/md/dm-verity-fec.c
+++ b/drivers/md/dm-verity-fec.c
@@ -188,7 +188,7 @@ static int fec_decode_bufs(struct dm_verity *v, struct dm_verity_fec_io *fio,
static int fec_is_erasure(struct dm_verity *v, struct dm_verity_io *io,
u8 *want_digest, u8 *data)
{
- if (unlikely(verity_hash(v, verity_io_hash_desc(v, io),
+ if (unlikely(verity_hash(v, verity_io_hash_req(v, io),
data, 1 << v->data_dev_block_bits,
verity_io_real_digest(v, io))))
return 0;
@@ -397,7 +397,7 @@ static int fec_decode_rsb(struct dm_verity *v, struct dm_verity_io *io,
}
/* Always re-validate the corrected block against the expected hash */
- r = verity_hash(v, verity_io_hash_desc(v, io), fio->output,
+ r = verity_hash(v, verity_io_hash_req(v, io), fio->output,
1 << v->data_dev_block_bits,
verity_io_real_digest(v, io));
if (unlikely(r < 0))
diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index 7335d8a..97de961 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -93,81 +93,123 @@ static sector_t verity_position_at_level(struct dm_verity *v, sector_t block,
}
/*
- * Wrapper for crypto_shash_init, which handles verity salting.
+ * Callback function for asynchrnous crypto API completion notification
*/
-static int verity_hash_init(struct dm_verity *v, struct shash_desc *desc)
+static void verity_op_done(struct crypto_async_request *base, int err)
{
- int r;
+ struct verity_result *res = (struct verity_result *)base->data;
- desc->tfm = v->tfm;
- desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+ if (err == -EINPROGRESS)
+ return;
- r = crypto_shash_init(desc);
+ res->err = err;
+ complete(&res->completion);
+}
- if (unlikely(r < 0)) {
- DMERR("crypto_shash_init failed: %d", r);
- return r;
- }
+/*
+ * Wait for async crypto API callback
+ */
+static inline int verity_complete_op(struct verity_result *res, int ret)
+{
+ switch (ret) {
+ case 0:
+ break;
- if (likely(v->version >= 1)) {
- r = crypto_shash_update(desc, v->salt, v->salt_size);
+ case -EINPROGRESS:
+ case -EBUSY:
+ ret = wait_for_completion_interruptible(&res->completion);
+ if (!ret)
+ ret = res->err;
+ reinit_completion(&res->completion);
+ break;
- if (unlikely(r < 0)) {
- DMERR("crypto_shash_update failed: %d", r);
- return r;
- }
+ default:
+ DMERR("verity_wait_hash: crypto op submission failed: %d", ret);
}
- return 0;
+ if (unlikely(ret < 0))
+ DMERR("verity_wait_hash: crypto op failed: %d", ret);
+
+ return ret;
}
-static int verity_hash_update(struct dm_verity *v, struct shash_desc *desc,
- const u8 *data, size_t len)
+static int verity_hash_update(struct dm_verity *v, struct ahash_request *req,
+ const u8 *data, size_t len,
+ struct verity_result *res)
{
- int r = crypto_shash_update(desc, data, len);
+ struct scatterlist sg;
- if (unlikely(r < 0))
- DMERR("crypto_shash_update failed: %d", r);
+ sg_init_one(&sg, data, len);
+ ahash_request_set_crypt(req, &sg, NULL, len);
+
+ return verity_complete_op(res, crypto_ahash_update(req));
+}
+
+/*
+ * Wrapper for crypto_ahash_init, which handles verity salting.
+ */
+static int verity_hash_init(struct dm_verity *v, struct ahash_request *req,
+ struct verity_result *res)
+{
+ int r;
+
+ ahash_request_set_tfm(req, v->tfm);
+ ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP |
+ CRYPTO_TFM_REQ_MAY_BACKLOG,
+ verity_op_done, (void *)res);
+ init_completion(&res->completion);
+
+ r = verity_complete_op(res, crypto_ahash_init(req));
+
+ if (unlikely(r < 0)) {
+ DMERR("crypto_ahash_init failed: %d", r);
+ return r;
+ }
+
+ if (likely(v->version >= 1))
+ r = verity_hash_update(v, req, v->salt, v->salt_size, res);
return r;
}
-static int verity_hash_final(struct dm_verity *v, struct shash_desc *desc,
- u8 *digest)
+static int verity_hash_final(struct dm_verity *v, struct ahash_request *req,
+ u8 *digest, struct verity_result *res)
{
int r;
if (unlikely(!v->version)) {
- r = crypto_shash_update(desc, v->salt, v->salt_size);
+ r = verity_hash_update(v, req, v->salt, v->salt_size, res);
if (r < 0) {
- DMERR("crypto_shash_update failed: %d", r);
- return r;
+ DMERR("verity_hash_final failed updating salt: %d", r);
+ goto out;
}
}
- r = crypto_shash_final(desc, digest);
-
- if (unlikely(r < 0))
- DMERR("crypto_shash_final failed: %d", r);
-
+ ahash_request_set_crypt(req, NULL, digest, 0);
+ r = verity_complete_op(res, crypto_ahash_final(req));
+out:
return r;
}
-int verity_hash(struct dm_verity *v, struct shash_desc *desc,
+int verity_hash(struct dm_verity *v, struct ahash_request *req,
const u8 *data, size_t len, u8 *digest)
{
int r;
+ struct verity_result res;
- r = verity_hash_init(v, desc);
+ r = verity_hash_init(v, req, &res);
if (unlikely(r < 0))
- return r;
+ goto out;
- r = verity_hash_update(v, desc, data, len);
+ r = verity_hash_update(v, req, data, len, &res);
if (unlikely(r < 0))
- return r;
+ goto out;
+
+ r = verity_hash_final(v, req, digest, &res);
- return verity_hash_final(v, desc, digest);
+out:
+ return r;
}
static void verity_hash_at_level(struct dm_verity *v, sector_t block, int level,
@@ -275,7 +317,7 @@ static int verity_verify_level(struct dm_verity *v, struct dm_verity_io *io,
goto release_ret_r;
}
- r = verity_hash(v, verity_io_hash_desc(v, io),
+ r = verity_hash(v, verity_io_hash_req(v, io),
data, 1 << v->hash_dev_block_bits,
verity_io_real_digest(v, io));
if (unlikely(r < 0))
@@ -344,6 +386,49 @@ int verity_hash_for_block(struct dm_verity *v, struct dm_verity_io *io,
}
/*
+ * Calculates the digest for the given bio
+ */
+int verity_for_io_block(struct dm_verity *v, struct dm_verity_io *io,
+ struct bvec_iter *iter, struct verity_result *res)
+{
+ unsigned int todo = 1 << v->data_dev_block_bits;
+ struct bio *bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size);
+ struct scatterlist sg;
+ struct ahash_request *req = verity_io_hash_req(v, io);
+
+ do {
+ int r;
+ unsigned int len;
+ struct bio_vec bv = bio_iter_iovec(bio, *iter);
+
+ sg_init_table(&sg, 1);
+
+ len = bv.bv_len;
+
+ if (likely(len >= todo))
+ len = todo;
+ /*
+ * Operating on a single page at a time looks suboptimal
+ * until you consider the typical block size is 4,096B.
+ * Going through this loops twice should be very rare.
+ */
+ sg_set_page(&sg, bv.bv_page, len, bv.bv_offset);
+ ahash_request_set_crypt(req, &sg, NULL, len);
+ r = verity_complete_op(res, crypto_ahash_update(req));
+
+ if (unlikely(r < 0)) {
+ DMERR("verity_for_io_block crypto op failed: %d", r);
+ return r;
+ }
+
+ bio_advance_iter(bio, iter, len);
+ todo -= len;
+ } while (todo);
+
+ return 0;
+}
+
+/*
* Calls function process for 1 << v->data_dev_block_bits bytes in the bio_vec
* starting from iter.
*/
@@ -381,12 +466,6 @@ int verity_for_bv_block(struct dm_verity *v, struct dm_verity_io *io,
return 0;
}
-static int verity_bv_hash_update(struct dm_verity *v, struct dm_verity_io *io,
- u8 *data, size_t len)
-{
- return verity_hash_update(v, verity_io_hash_desc(v, io), data, len);
-}
-
static int verity_bv_zero(struct dm_verity *v, struct dm_verity_io *io,
u8 *data, size_t len)
{
@@ -403,10 +482,11 @@ static int verity_verify_io(struct dm_verity_io *io)
struct dm_verity *v = io->v;
struct bvec_iter start;
unsigned b;
+ struct verity_result res;
for (b = 0; b < io->n_blocks; b++) {
int r;
- struct shash_desc *desc = verity_io_hash_desc(v, io);
+ struct ahash_request *req = verity_io_hash_req(v, io);
r = verity_hash_for_block(v, io, io->block + b,
verity_io_want_digest(v, io),
@@ -427,16 +507,17 @@ static int verity_verify_io(struct dm_verity_io *io)
continue;
}
- r = verity_hash_init(v, desc);
+ r = verity_hash_init(v, req, &res);
if (unlikely(r < 0))
return r;
start = io->iter;
- r = verity_for_bv_block(v, io, &io->iter, verity_bv_hash_update);
+ r = verity_for_io_block(v, io, &io->iter, &res);
if (unlikely(r < 0))
return r;
- r = verity_hash_final(v, desc, verity_io_real_digest(v, io));
+ r = verity_hash_final(v, req, verity_io_real_digest(v, io),
+ &res);
if (unlikely(r < 0))
return r;
@@ -705,7 +786,7 @@ static void verity_dtr(struct dm_target *ti)
kfree(v->zero_digest);
if (v->tfm)
- crypto_free_shash(v->tfm);
+ crypto_free_ahash(v->tfm);
kfree(v->alg_name);
@@ -723,7 +804,7 @@ static void verity_dtr(struct dm_target *ti)
static int verity_alloc_zero_digest(struct dm_verity *v)
{
int r = -ENOMEM;
- struct shash_desc *desc;
+ struct ahash_request *req;
u8 *zero_data;
v->zero_digest = kmalloc(v->digest_size, GFP_KERNEL);
@@ -731,9 +812,9 @@ static int verity_alloc_zero_digest(struct dm_verity *v)
if (!v->zero_digest)
return r;
- desc = kmalloc(v->shash_descsize, GFP_KERNEL);
+ req = kmalloc(v->ahash_reqsize, GFP_KERNEL);
- if (!desc)
+ if (!req)
return r; /* verity_dtr will free zero_digest */
zero_data = kzalloc(1 << v->data_dev_block_bits, GFP_KERNEL);
@@ -741,11 +822,11 @@ static int verity_alloc_zero_digest(struct dm_verity *v)
if (!zero_data)
goto out;
- r = verity_hash(v, desc, zero_data, 1 << v->data_dev_block_bits,
+ r = verity_hash(v, req, zero_data, 1 << v->data_dev_block_bits,
v->zero_digest);
out:
- kfree(desc);
+ kfree(req);
kfree(zero_data);
return r;
@@ -923,21 +1004,21 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
goto bad;
}
- v->tfm = crypto_alloc_shash(v->alg_name, 0, 0);
+ v->tfm = crypto_alloc_ahash(v->alg_name, 0, 0);
if (IS_ERR(v->tfm)) {
ti->error = "Cannot initialize hash function";
r = PTR_ERR(v->tfm);
v->tfm = NULL;
goto bad;
}
- v->digest_size = crypto_shash_digestsize(v->tfm);
+ v->digest_size = crypto_ahash_digestsize(v->tfm);
if ((1 << v->hash_dev_block_bits) < v->digest_size * 2) {
ti->error = "Digest size too big";
r = -EINVAL;
goto bad;
}
- v->shash_descsize =
- sizeof(struct shash_desc) + crypto_shash_descsize(v->tfm);
+ v->ahash_reqsize = sizeof(struct ahash_request) +
+ crypto_ahash_reqsize(v->tfm);
v->root_digest = kmalloc(v->digest_size, GFP_KERNEL);
if (!v->root_digest) {
@@ -1037,7 +1118,7 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
}
ti->per_io_data_size = sizeof(struct dm_verity_io) +
- v->shash_descsize + v->digest_size * 2;
+ v->ahash_reqsize + v->digest_size * 2;
r = verity_fec_ctr(v);
if (r)
diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h
index fb419f4..a59e0ad 100644
--- a/drivers/md/dm-verity.h
+++ b/drivers/md/dm-verity.h
@@ -37,7 +37,7 @@ struct dm_verity {
struct dm_target *ti;
struct dm_bufio_client *bufio;
char *alg_name;
- struct crypto_shash *tfm;
+ struct crypto_ahash *tfm;
u8 *root_digest; /* digest of the root block */
u8 *salt; /* salt: its size is salt_size */
u8 *zero_digest; /* digest for a zero block */
@@ -52,7 +52,7 @@ struct dm_verity {
unsigned char levels; /* the number of tree levels */
unsigned char version;
unsigned digest_size; /* digest size for the current hash algorithm */
- unsigned shash_descsize;/* the size of temporary space for crypto */
+ unsigned int ahash_reqsize;/* the size of temporary space for crypto */
int hash_failed; /* set to 1 if hash of any block failed */
enum verity_mode mode; /* mode for handling verification errors */
unsigned corrupted_errs;/* Number of errors for corrupted blocks */
@@ -81,31 +81,36 @@ struct dm_verity_io {
/*
* Three variably-size fields follow this struct:
*
- * u8 hash_desc[v->shash_descsize];
+ * u8 hash_req[v->ahash_reqsize];
* u8 real_digest[v->digest_size];
* u8 want_digest[v->digest_size];
*
- * To access them use: verity_io_hash_desc(), verity_io_real_digest()
+ * To access them use: verity_io_hash_req(), verity_io_real_digest()
* and verity_io_want_digest().
*/
};
-static inline struct shash_desc *verity_io_hash_desc(struct dm_verity *v,
+struct verity_result {
+ struct completion completion;
+ int err;
+};
+
+static inline struct ahash_request *verity_io_hash_req(struct dm_verity *v,
struct dm_verity_io *io)
{
- return (struct shash_desc *)(io + 1);
+ return (struct ahash_request *)(io + 1);
}
static inline u8 *verity_io_real_digest(struct dm_verity *v,
struct dm_verity_io *io)
{
- return (u8 *)(io + 1) + v->shash_descsize;
+ return (u8 *)(io + 1) + v->ahash_reqsize;
}
static inline u8 *verity_io_want_digest(struct dm_verity *v,
struct dm_verity_io *io)
{
- return (u8 *)(io + 1) + v->shash_descsize + v->digest_size;
+ return (u8 *)(io + 1) + v->ahash_reqsize + v->digest_size;
}
static inline u8 *verity_io_digest_end(struct dm_verity *v,
@@ -120,7 +125,7 @@ extern int verity_for_bv_block(struct dm_verity *v, struct dm_verity_io *io,
struct dm_verity_io *io,
u8 *data, size_t len));
-extern int verity_hash(struct dm_verity *v, struct shash_desc *desc,
+extern int verity_hash(struct dm_verity *v, struct ahash_request *req,
const u8 *data, size_t len, u8 *digest);
extern int verity_hash_for_block(struct dm_verity *v, struct dm_verity_io *io,
--
2.1.4
^ permalink raw reply related
* Re: [PATCH -next] crypto: caam - fix error return code in caam_qi_init()
From: Horia Geantă @ 2017-04-12 8:18 UTC (permalink / raw)
To: Wei Yongjun, Dan Douglass, herbert@gondor.apana.org.au,
davem@davemloft.net
Cc: Wei Yongjun, linux-crypto@vger.kernel.org
In-Reply-To: <20170411160409.18430-1-weiyj.lk@gmail.com>
On 4/11/2017 7:04 PM, Wei Yongjun wrote:
> From: Wei Yongjun <weiyongjun1@huawei.com>
>
> Fix to return error code -ENOMEM from the kmem_cache_create() error
> handling case instead of 0(err is 0 here), as done elsewhere in this
> function.
>
> Fixes: 67c2315def06 ("crypto: caam - add Queue Interface (QI) backend support")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Acked-by: Horia Geantă <horia.geanta@nxp.com>
Thanks,
Horia
^ permalink raw reply
* Re: [PATCH 4/7] arm64: marvell: dts: add crypto engine description for 7k/8k
From: Gregory CLEMENT @ 2017-04-12 8:36 UTC (permalink / raw)
To: Antoine Tenart
Cc: herbert, davem, jason, andrew, sebastian.hesselbarth,
linux-crypto, linux-arm-kernel, thomas.petazzoni, boris.brezillon,
oferh, igall, nadavh
In-Reply-To: <20170329124432.27457-5-antoine.tenart@free-electrons.com>
Hi Antoine,
On mer., mars 29 2017, Antoine Tenart <antoine.tenart@free-electrons.com> wrote:
> Add the description of the crypto engine hardware block for the Marvell
> Armada 7k and Armada 8k processors; for both the CP110 slave and master.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Applied on mvebu/dt64, I fixed a merge conflict with the current
mvebu/dt64 and I took this opportunity to fix the lines over 80
characters for the interrupts.
Thanks,
Gregory
> ---
> arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi | 15 +++++++++++++++
> arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 15 +++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> index 9a2ce2ae49cd..7530a1b541c5 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> @@ -210,6 +210,21 @@
> clocks = <&cpm_syscon0 1 25>;
> status = "okay";
> };
> +
> + cpm_crypto: crypto@800000 {
> + compatible = "inside-secure,safexcel-eip197";
> + reg = <0x800000 0x200000>;
> + interrupts = <GIC_SPI 34 (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)>,
> + <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "mem", "ring0", "ring1",
> + "ring2", "ring3", "eip";
> + clocks = <&cpm_syscon0 1 26>;
> + status = "disabled";
> + };
> };
>
> cpm_pcie0: pcie@f2600000 {
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> index c9dfa244ebb6..6110d39087a6 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> @@ -210,6 +210,21 @@
> clocks = <&cps_syscon0 1 25>;
> status = "okay";
> };
> +
> + cps_crypto: crypto@800000 {
> + compatible = "inside-secure,safexcel-eip197";
> + reg = <0x800000 0x200000>;
> + interrupts = <GIC_SPI 34 (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)>,
> + <GIC_SPI 278 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 279 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 280 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 281 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "mem", "ring0", "ring1",
> + "ring2", "ring3", "eip";
> + clocks = <&cps_syscon0 1 26>;
> + status = "disabled";
> + };
> };
>
> cps_pcie0: pcie@f4600000 {
> --
> 2.11.0
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 5/7] arm64: marvell: dts: enable the crypto engine on the Armada 7040 DB
From: Gregory CLEMENT @ 2017-04-12 8:36 UTC (permalink / raw)
To: Antoine Tenart
Cc: herbert, davem, jason, andrew, sebastian.hesselbarth,
linux-crypto, linux-arm-kernel, thomas.petazzoni, boris.brezillon,
oferh, igall, nadavh
In-Reply-To: <20170329124432.27457-6-antoine.tenart@free-electrons.com>
Hi Antoine,
On mer., mars 29 2017, Antoine Tenart <antoine.tenart@free-electrons.com> wrote:
> Enable the cryptographic engine available in the CP110 master on the
> Armada 7040 DB.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Applied on mvebu/dt64
Thanks,
Gregory
> ---
> arch/arm64/boot/dts/marvell/armada-7040-db.dts | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-7040-db.dts b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
> index bb0e1e8e7138..7e9a8a4e89a4 100644
> --- a/arch/arm64/boot/dts/marvell/armada-7040-db.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
> @@ -171,3 +171,7 @@
> phy = <&phy1>;
> phy-mode = "rgmii-id";
> };
> +
> +&cpm_crypto {
> + status = "okay";
> +};
> --
> 2.11.0
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 6/7] arm64: marvell: dts: enable the crypto engine on the Armada 8040 DB
From: Gregory CLEMENT @ 2017-04-12 8:37 UTC (permalink / raw)
To: Antoine Tenart
Cc: herbert, davem, jason, andrew, sebastian.hesselbarth,
linux-crypto, linux-arm-kernel, thomas.petazzoni, boris.brezillon,
oferh, igall, nadavh
In-Reply-To: <20170329124432.27457-7-antoine.tenart@free-electrons.com>
Hi Antoine,
On mer., mars 29 2017, Antoine Tenart <antoine.tenart@free-electrons.com> wrote:
> Enable the cryptographic engine available in the CP110 master on the
> Armada 8040 DB. Do not enable the one in the CP110 salve for now, as we
> do not support multiple cryptographic engines yet.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Applied on mvebu/dt64
Thanks,
Gregory
> ---
> arch/arm64/boot/dts/marvell/armada-8040-db.dts | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-8040-db.dts b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
> index 80e685e4235d..f6d808bced51 100644
> --- a/arch/arm64/boot/dts/marvell/armada-8040-db.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
> @@ -140,6 +140,10 @@
> phy-mode = "rgmii-id";
> };
>
> +&cpm_crypto {
> + status = "okay";
> +};
> +
> /* CON5 on CP1 expansion */
> &cps_pcie2 {
> status = "okay";
> --
> 2.11.0
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 7/7] arm64: defconfig: enable the Safexcel crypto engine as a module
From: Gregory CLEMENT @ 2017-04-12 8:38 UTC (permalink / raw)
To: Antoine Tenart
Cc: herbert, davem, jason, andrew, sebastian.hesselbarth,
linux-crypto, linux-arm-kernel, thomas.petazzoni, boris.brezillon,
oferh, igall, nadavh
In-Reply-To: <20170329124432.27457-8-antoine.tenart@free-electrons.com>
Hi Antoine,
On mer., mars 29 2017, Antoine Tenart <antoine.tenart@free-electrons.com> wrote:
> The Safexcel EIP197 cryptographic engine is used on some Marvell SoCs,
> such as Armada 7040 and Armada 8040. Enable this driver as a module in
> the ARM64 defconfig.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Applied on mvebu/defconfig64
Thanks,
Gregory
> ---
> arch/arm64/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index fece4d3c2095..76308fa2fa7a 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -519,6 +519,7 @@ CONFIG_MEMTEST=y
> CONFIG_SECURITY=y
> CONFIG_CRYPTO_ECHAINIV=y
> CONFIG_CRYPTO_ANSI_CPRNG=y
> +CONFIG_CRYPTO_DEV_SAFEXCEL=m
> CONFIG_ARM64_CRYPTO=y
> CONFIG_CRYPTO_SHA1_ARM64_CE=y
> CONFIG_CRYPTO_SHA2_ARM64_CE=y
> --
> 2.11.0
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH] padata: get_next is never NULL
From: Jason A. Donenfeld @ 2017-04-12 8:40 UTC (permalink / raw)
To: steffen.klassert, linux-crypto, dan.carpenter, linux-kernel
Cc: Jason A. Donenfeld
In-Reply-To: <20170411091601.GA24844@mwanda>
Per Dan's static checker warning, the code that returns NULL was removed
in 2010, so this patch updates the comments and fixes the code
assumptions.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
---
kernel/padata.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/kernel/padata.c b/kernel/padata.c
index f1aef1639204..ac8f1e524836 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -154,8 +154,6 @@ EXPORT_SYMBOL(padata_do_parallel);
* A pointer to the control struct of the next object that needs
* serialization, if present in one of the percpu reorder queues.
*
- * NULL, if all percpu reorder queues are empty.
- *
* -EINPROGRESS, if the next object that needs serialization will
* be parallel processed by another cpu and is not yet present in
* the cpu's reorder queue.
@@ -182,8 +180,6 @@ static struct padata_priv *padata_get_next(struct parallel_data *pd)
cpu = padata_index_to_cpu(pd, next_index);
next_queue = per_cpu_ptr(pd->pqueue, cpu);
- padata = NULL;
-
reorder = &next_queue->reorder;
spin_lock(&reorder->lock);
@@ -235,12 +231,11 @@ static void padata_reorder(struct parallel_data *pd)
padata = padata_get_next(pd);
/*
- * All reorder queues are empty, or the next object that needs
- * serialization is parallel processed by another cpu and is
- * still on it's way to the cpu's reorder queue, nothing to
- * do for now.
+ * If the next object that needs serialization is parallel
+ * processed by another cpu and is still on it's way to the
+ * cpu's reorder queue, nothing to do for now.
*/
- if (!padata || PTR_ERR(padata) == -EINPROGRESS)
+ if (PTR_ERR(padata) == -EINPROGRESS)
break;
/*
--
2.12.2
^ permalink raw reply related
* Re: [PATCH 4/7] arm64: marvell: dts: add crypto engine description for 7k/8k
From: Thomas Petazzoni @ 2017-04-12 8:56 UTC (permalink / raw)
To: Antoine Tenart
Cc: herbert, davem, jason, andrew, gregory.clement,
sebastian.hesselbarth, linux-crypto, linux-arm-kernel,
boris.brezillon, oferh, igall, nadavh
In-Reply-To: <20170329124432.27457-5-antoine.tenart@free-electrons.com>
Hello,
On Wed, 29 Mar 2017 14:44:29 +0200, Antoine Tenart wrote:
> + cpm_crypto: crypto@800000 {
> + compatible = "inside-secure,safexcel-eip197";
> + reg = <0x800000 0x200000>;
> + interrupts = <GIC_SPI 34 (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)>,
Now that I look into this, does it makes sense for an interrupt to be
both an edge interrupt and a level interrupt at the same time? This
looks odd.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 2/7] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver
From: Robin Murphy @ 2017-04-12 13:54 UTC (permalink / raw)
To: Antoine Tenart, herbert, davem, jason, andrew, gregory.clement,
sebastian.hesselbarth
Cc: thomas.petazzoni, boris.brezillon, igall, nadavh, linux-crypto,
oferh, linux-arm-kernel
In-Reply-To: <20170329124432.27457-3-antoine.tenart@free-electrons.com>
Hi Antoine,
Bit of a drive-by, but since I have it in my head that crypto drivers
are a hotspot for dodgy DMA usage (in part due to the hardware often
being a bit esoteric with embedded RAMs and such), this caught my eye
and I thought I'd give this a quick once-over to check for anything
smelly. Unfortunately, I was not disappointed... ;)
On 29/03/17 13:44, Antoine Tenart wrote:
[...]
> diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
> new file mode 100644
> index 000000000000..00f3f2c85d05
> --- /dev/null
> +++ b/drivers/crypto/inside-secure/safexcel.c
[...]
> +int safexcel_invalidate_cache(struct crypto_async_request *async,
> + struct safexcel_context *ctx,
> + struct safexcel_crypto_priv *priv,
> + dma_addr_t ctxr_dma, int ring,
> + struct safexcel_request *request)
> +{
> + struct safexcel_command_desc *cdesc;
> + struct safexcel_result_desc *rdesc;
> + phys_addr_t ctxr_phys;
> + int ret = 0;
> +
> + ctxr_phys = dma_to_phys(priv->dev, ctxr_dma);
No no no. This is a SWIOTLB-specific (or otherwise arch-private,
depending on implementation) helper, not a DMA API function, and should
not be called directly by drivers. There is no guarantee it will give
the result you expect, or even compile, in all cases (if the kbuild
robot hasn't already revealed via the COMPILE_TEST dependency).
That said, AFAICS ctxr_phys ends up in a descriptor which is given to
the device, so trying to use a physical address is wrong and it should
still be the DMA address anyway.
> + spin_lock_bh(&priv->ring[ring].egress_lock);
> +
> + /* prepare command descriptor */
> + cdesc = safexcel_add_cdesc(priv, ring, true, true, 0, 0, 0, ctxr_phys);
> + if (IS_ERR(cdesc)) {
> + ret = PTR_ERR(cdesc);
> + goto unlock;
> + }
> +
> + cdesc->control_data.type = EIP197_TYPE_EXTENDED;
> + cdesc->control_data.options = 0;
> + cdesc->control_data.refresh = 0;
> + cdesc->control_data.control0 = CONTEXT_CONTROL_INV_TR;
> +
> + /* prepare result descriptor */
> + rdesc = safexcel_add_rdesc(priv, ring, true, true, 0, 0);
> +
> + if (IS_ERR(rdesc)) {
> + ret = PTR_ERR(rdesc);
> + goto cdesc_rollback;
> + }
> +
> + request->req = async;
> + goto unlock;
> +
> +cdesc_rollback:
> + safexcel_ring_rollback_wptr(priv, &priv->ring[ring].cdr);
> +
> +unlock:
> + spin_unlock_bh(&priv->ring[ring].egress_lock);
> + return ret;
> +}
[...]
> +static int safexcel_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + struct safexcel_crypto_priv *priv;
> + int i, ret;
> +
> + priv = devm_kzalloc(dev, sizeof(struct safexcel_crypto_priv),
> + GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->dev = dev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + priv->base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(priv->base)) {
> + dev_err(dev, "failed to get resource\n");
> + return PTR_ERR(priv->base);
> + }
> +
> + priv->clk = of_clk_get(dev->of_node, 0);
> + if (!IS_ERR(priv->clk)) {
> + ret = clk_prepare_enable(priv->clk);
> + if (ret) {
> + dev_err(dev, "unable to enable clk (%d)\n", ret);
> + return ret;
> + }
> + } else {
> + /* The clock isn't mandatory */
> + if (PTR_ERR(priv->clk) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + }
You should call dma_set_mask_and_coherent() before any DMA API calls,
both to confirm DMA really is going to work all, and also (since this IP
apparently supports >32-bit addresses) to describe the full inherent
addressing capability, not least to avoid wasting time/space with
unnecessary bounce buffering otherwise.
> + priv->context_pool = dmam_pool_create("safexcel-context", dev,
> + sizeof(struct safexcel_context_record),
> + 1, 0);
> + if (!priv->context_pool) {
> + ret = -ENOMEM;
> + goto err_clk;
> + }
> +
> + safexcel_configure(priv);
> +
> + for (i = 0; i < priv->config.rings; i++) {
> + char irq_name[6] = {0}; /* "ringX\0" */
> + char wq_name[9] = {0}; /* "wq_ringX\0" */
> + int irq;
> + struct safexcel_ring_irq_data *ring_irq;
> +
> + ret = safexcel_init_ring_descriptors(priv,
> + &priv->ring[i].cdr,
> + &priv->ring[i].rdr);
> + if (ret)
> + goto err_pool;
> +
> + ring_irq = devm_kzalloc(dev, sizeof(struct safexcel_ring_irq_data),
> + GFP_KERNEL);
> + if (!ring_irq) {
> + ret = -ENOMEM;
> + goto err_pool;
> + }
> +
> + ring_irq->priv = priv;
> + ring_irq->ring = i;
> +
> + snprintf(irq_name, 6, "ring%d", i);
> + irq = safexcel_request_ring_irq(pdev, irq_name, safexcel_irq_ring,
> + ring_irq);
> +
> + if (irq < 0)
> + goto err_pool;
> +
> + priv->ring[i].work_data.priv = priv;
> + priv->ring[i].work_data.ring = i;
> + INIT_WORK(&priv->ring[i].work_data.work, safexcel_handle_result_work);
> +
> + snprintf(wq_name, 9, "wq_ring%d", i);
> + priv->ring[i].workqueue = create_singlethread_workqueue(wq_name);
> + if (!priv->ring[i].workqueue) {
> + ret = -ENOMEM;
> + goto err_pool;
> + }
> +
> + INIT_LIST_HEAD(&priv->ring[i].list);
> + spin_lock_init(&priv->ring[i].lock);
> + spin_lock_init(&priv->ring[i].egress_lock);
> + }
> +
> + platform_set_drvdata(pdev, priv);
> + atomic_set(&priv->ring_used, 0);
> +
> + spin_lock_init(&priv->lock);
> + crypto_init_queue(&priv->queue, EIP197_DEFAULT_RING_SIZE);
> +
> + ret = safexcel_hw_init(priv);
> + if (ret) {
> + dev_err(dev, "EIP h/w init failed (%d)\n", ret);
> + goto err_pool;
> + }
> +
> + ret = safexcel_register_algorithms(priv);
> + if (ret) {
> + dev_err(dev, "Failed to register algorithms (%d)\n", ret);
> + goto err_pool;
> + }
> +
> + return 0;
> +
> +err_pool:
> + dma_pool_destroy(priv->context_pool);
You used dmam_pool_create(), so not only is an explicit destroy
unnecessary, but doing so with the non-managed version is actively
harmful, since devres would end up double-freeing the pool after you return.
> +err_clk:
> + clk_disable_unprepare(priv->clk);
> + return ret;
> +}
[...]
> diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
> new file mode 100644
> index 000000000000..ffe39aaabe1c
> --- /dev/null
> +++ b/drivers/crypto/inside-secure/safexcel.h
[...]
> +/* Result data */
> +struct result_data_desc {
> + u32 packet_length:17;
> + u32 error_code:15;
> +
> + u8 bypass_length:4;
> + u8 e15:1;
> + u16 rsvd0;
> + u8 hash_bytes:1;
> + u8 hash_length:6;
> + u8 generic_bytes:1;
> + u8 checksum:1;
> + u8 next_header:1;
> + u8 length:1;
> +
> + u16 application_id;
> + u16 rsvd1;
> +
> + u32 rsvd2;
> +} __packed;
> +
> +
> +/* Basic Result Descriptor format */
> +struct safexcel_result_desc {
> + u32 particle_size:17;
> + u8 rsvd0:3;
> + u8 descriptor_overflow:1;
> + u8 buffer_overflow:1;
> + u8 last_seg:1;
> + u8 first_seg:1;
> + u16 result_size:8;
> +
> + u32 rsvd1;
> +
> + u32 data_lo;
> + u32 data_hi;
> +
> + struct result_data_desc result_data;
> +} __packed;
> +
> +struct safexcel_token {
> + u32 packet_length:17;
> + u8 stat:2;
> + u16 instructions:9;
> + u8 opcode:4;
> +} __packed;
Using bitfields in hardware descriptors seems pretty risky, since you've
no real guarantee two compilers will lay them out the same way (or that
either would be correct WRT what the hardware expects). I'd be more
inclined to construct all these fields with explicit masking and shifting.
[...]
> diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
> new file mode 100644
> index 000000000000..ff42e45ae43e
> --- /dev/null
> +++ b/drivers/crypto/inside-secure/safexcel_cipher.c
[...]
> +static int safexcel_aes_send(struct crypto_async_request *async,
> + int ring, struct safexcel_request *request,
> + int *commands, int *results)
> +{
> + struct ablkcipher_request *req = ablkcipher_request_cast(async);
> + struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
> + struct safexcel_crypto_priv *priv = ctx->priv;
> + struct safexcel_command_desc *cdesc;
> + struct safexcel_result_desc *rdesc;
> + struct scatterlist *sg;
> + phys_addr_t ctxr_phys;
> + int nr_src, nr_dst, n_cdesc = 0, n_rdesc = 0, queued = req->nbytes;
> + int i, ret = 0;
> +
> + request->req = &req->base;
> +
> + if (req->src == req->dst) {
> + nr_src = sg_nents_for_len(req->src, req->nbytes);
> + nr_dst = nr_src;
> +
> + if (dma_map_sg(priv->dev, req->src, nr_src, DMA_BIDIRECTIONAL) <= 0)
Nit: you only need to check for zero/nonzero to determine failure
(similarly below) - dma_map_sg() cannot return negative values.
Bigger complaint: you should not ignore the successful return value and
rely on it being equal to nr_src - please see the description of
dma_map_sg() in Documenatation/DMA-API.txt
> + return -EINVAL;
> + } else {
> + nr_src = sg_nents_for_len(req->src, req->nbytes);
> + nr_dst = sg_nents_for_len(req->dst, req->nbytes);
> +
> + if (dma_map_sg(priv->dev, req->src, nr_src, DMA_TO_DEVICE) <= 0)
> + return -EINVAL;
> +
> + if (dma_map_sg(priv->dev, req->dst, nr_dst, DMA_FROM_DEVICE) <= 0) {
> + dma_unmap_sg(priv->dev, req->src, nr_src,
> + DMA_TO_DEVICE);
> + return -EINVAL;
> + }
> + }
> +
> + ctxr_phys = dma_to_phys(priv->dev, ctx->base.ctxr_dma);
Once again, wrong and probably unnecessary.
> + memcpy(ctx->base.ctxr->data, ctx->key, ctx->key_len);
> +
> + spin_lock_bh(&priv->ring[ring].egress_lock);
> +
> + /* command descriptors */
> + for_each_sg(req->src, sg, nr_src, i) {
> + phys_addr_t sg_phys = dma_to_phys(priv->dev, sg_dma_address(sg));
> + int len = sg_dma_len(sg);
If dma_map_sg() coalesced any entries into the same mapping such that
count < nents, these could well give bogus values toward the end of the
list once i >= count(if you're lucky, at least len might be 0).
> + /* Do not overflow the request */
> + if (queued - len < 0)
> + len = queued;
> +
> + cdesc = safexcel_add_cdesc(priv, ring, !n_cdesc, !(queued - len),
> + sg_phys, len, req->nbytes, ctxr_phys);
> + if (IS_ERR(cdesc)) {
> + /* No space left in the command descriptor ring */
> + ret = PTR_ERR(cdesc);
> + goto cdesc_rollback;
> + }
> + n_cdesc++;
> +
> + if (n_cdesc == 1) {
> + safexcel_context_control(ctx, cdesc);
> + safexcel_cipher_token(ctx, async, cdesc, req->nbytes);
> + }
> +
> + queued -= len;
> + if (!queued)
> + break;
> + }
> +
> + /* result descriptors */
> + for_each_sg(req->dst, sg, nr_dst, i) {
> + bool first = !i, last = (i == nr_dst - 1);
> + phys_addr_t sg_phys = dma_to_phys(priv->dev, sg_dma_address(sg));
And again, no. Also note that sg_phys() already exists as a function, so
I imagine sparse could get quite cross here.
> + u32 len = sg_dma_len(sg);
Plus the same comment again about potentially having iterated past the
point where these are valid.
> +
> + rdesc = safexcel_add_rdesc(priv, ring, first, last, sg_phys, len);
> + if (IS_ERR(rdesc)) {
> + /* No space left in the result descriptor ring */
> + ret = PTR_ERR(rdesc);
> + goto rdesc_rollback;
> + }
> + n_rdesc++;
> + }
> +
> + ctx->base.handle_result = safexcel_handle_result;
> +
> + spin_unlock_bh(&priv->ring[ring].egress_lock);
> +
> + *commands = n_cdesc;
> + *results = nr_dst;
> + return 0;
> +
> +rdesc_rollback:
> + for (i = 0; i < n_rdesc; i++)
> + safexcel_ring_rollback_wptr(priv, &priv->ring[ring].rdr);
> +cdesc_rollback:
> + for (i = 0; i < n_cdesc; i++)
> + safexcel_ring_rollback_wptr(priv, &priv->ring[ring].cdr);
> +
> + spin_unlock_bh(&priv->ring[ring].egress_lock);
> +
> + if (req->src == req->dst) {
> + dma_unmap_sg(priv->dev, req->src, nr_src, DMA_BIDIRECTIONAL);
> + } else {
> + dma_unmap_sg(priv->dev, req->src, nr_src, DMA_TO_DEVICE);
> + dma_unmap_sg(priv->dev, req->dst, nr_dst, DMA_FROM_DEVICE);
I will note that the unmap_sg calls *should* have the same nents value
as originally passed to map_sg, so these remain correct.
> + }
> +
> + return ret;
> +}
[...]
> diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
> new file mode 100644
> index 000000000000..1f44e0a2ddf1
> --- /dev/null
> +++ b/drivers/crypto/inside-secure/safexcel_hash.c
[...]
> +static int safexcel_ahash_send(struct crypto_async_request *async, int ring,
> + struct safexcel_request *request, int *commands,
> + int *results)
> +{
> + struct ahash_request *areq = ahash_request_cast(async);
> + struct crypto_ahash *ahash = crypto_ahash_reqtfm(areq);
> + struct safexcel_ahash_req *req = ahash_request_ctx(areq);
> + struct safexcel_ahash_ctx *ctx = crypto_ahash_ctx(crypto_ahash_reqtfm(areq));
> + struct safexcel_crypto_priv *priv = ctx->priv;
> + struct safexcel_command_desc *cdesc, *first_cdesc = NULL;
> + struct safexcel_result_desc *rdesc;
> + struct scatterlist *sg;
> + int i, nents, queued, len, cache_len, extra, n_cdesc = 0, ret = 0;
> + phys_addr_t ctxr_phys = 0;
> +
> + queued = len = req->len - req->processed;
> + if (queued < crypto_ahash_blocksize(ahash))
> + cache_len = queued;
> + else
> + cache_len = queued - areq->nbytes;
> +
> + /*
> + * If this is not the last request and the queued data does not fit
> + * into full blocks, cache it for the next send() call.
> + */
> + extra = queued & (crypto_ahash_blocksize(ahash) - 1);
> + if (!req->last_req && extra) {
> + sg_pcopy_to_buffer(areq->src, sg_nents(areq->src),
> + req->cache_next, extra, areq->nbytes - extra);
> +
> + queued -= extra;
> + len -= extra;
> + }
> +
> + request->req = &areq->base;
> + ctx->base.handle_result = safexcel_handle_result;
> + ctxr_phys = dma_to_phys(priv->dev, ctx->base.ctxr_dma);
No.
> + spin_lock_bh(&priv->ring[ring].egress_lock);
> +
> + /* Add a command descriptor for the cached data, if any */
> + if (cache_len) {
> + ctx->base.cache_dma = dma_map_single(priv->dev, req->cache,
This is pretty dodgy, since req->cache is inside a live data structure,
adjoining parts of which are updated whilst still mapped (the update of
req->processed below). You just about get away without data corruption
since we *probably* don't invalidate anything when unmapping
DMA_TO_DEVICE, and the coherency hardware *probably* doesn't do anything
crazy, but you've no real guarantee of that - any DMA buffer should
really be separately kmalloced. "That's a nice dirty cache line you've
got there, it'd be a shame if anything were to happen to it..."
> + cache_len, DMA_TO_DEVICE);
> + if (dma_mapping_error(priv->dev, ctx->base.cache_dma)) {
> + ret = -EINVAL;
> + goto unlock;
> + }
> +
> + ctx->base.cache_sz = cache_len;
> + first_cdesc = safexcel_add_cdesc(priv, ring, 1,
> + (cache_len == len),
> + dma_to_phys(priv->dev, ctx->base.cache_dma),
No.
> + cache_len, len, ctxr_phys);
> + if (IS_ERR(first_cdesc)) {
> + ret = PTR_ERR(first_cdesc);
> + goto free_cache;
> + }
> + n_cdesc++;
> +
> + queued -= cache_len;
> + if (!queued)
> + goto send_command;
> + }
> +
> + /* Now handle the current ahash request buffer(s) */
> + nents = sg_nents_for_len(areq->src, areq->nbytes);
> + if (dma_map_sg(priv->dev, areq->src, nents, DMA_TO_DEVICE) <= 0) {
Same comments about the return value.
> + ret = -ENOMEM;
> + goto cdesc_rollback;
> + }
> +
> + for_each_sg(areq->src, sg, nents, i) {
> + phys_addr_t sg_phys = dma_to_phys(priv->dev, sg_dma_address(sg));
No.
> + int sglen = sg_dma_len(sg);
> +
> + /* Do not overflow the request */
> + if (queued - sglen < 0)
> + sglen = queued;
> +
> + cdesc = safexcel_add_cdesc(priv, ring, !n_cdesc,
> + !(queued - sglen), sg_phys, sglen,
> + len, ctxr_phys);
> + if (IS_ERR(cdesc)) {
> + ret = PTR_ERR(cdesc);
> + goto cdesc_rollback;
> + }
> + n_cdesc++;
> +
> + if (n_cdesc == 1)
> + first_cdesc = cdesc;
> +
> + queued -= sglen;
> + if (!queued)
> + break;
> + }
> +
> +send_command:
> + /* Setup the context options */
> + safexcel_context_control(ctx, req, first_cdesc, req->state_sz,
> + crypto_ahash_blocksize(ahash));
> +
> + /* Add the token */
> + safexcel_hash_token(first_cdesc, len, req->state_sz);
> +
> + ctx->base.result_dma = dma_map_single(priv->dev, areq->result,
> + req->state_sz, DMA_FROM_DEVICE);
> + if (dma_mapping_error(priv->dev, ctx->base.result_dma)) {
> + ret = -EINVAL;
> + goto cdesc_rollback;
> + }
> +
> + /* Add a result descriptor */
> + rdesc = safexcel_add_rdesc(priv, ring, 1, 1,
> + dma_to_phys(priv->dev, ctx->base.result_dma),
> + req->state_sz);
> + if (IS_ERR(rdesc)) {
> + ret = PTR_ERR(rdesc);
> + goto cdesc_rollback;
> + }
> +
> + req->processed += len;
> + spin_unlock_bh(&priv->ring[ring].egress_lock);
> +
> + *commands = n_cdesc;
> + *results = 1;
> + return 0;
> +
> +cdesc_rollback:
> + for (i = 0; i < n_cdesc; i++)
> + safexcel_ring_rollback_wptr(priv, &priv->ring[ring].cdr);
> +free_cache:
> + if (ctx->base.cache_dma)
> + dma_unmap_single(priv->dev, ctx->base.cache_dma,
> + ctx->base.cache_sz, DMA_TO_DEVICE);
> +
> +unlock:
> + spin_unlock_bh(&priv->ring[ring].egress_lock);
> + return ret;
> +}
[...]
> diff --git a/drivers/crypto/inside-secure/safexcel_ring.c b/drivers/crypto/inside-secure/safexcel_ring.c
> new file mode 100644
> index 000000000000..5b4ba7a8cc0a
> --- /dev/null
> +++ b/drivers/crypto/inside-secure/safexcel_ring.c
[...]
> +int safexcel_init_ring_descriptors(struct safexcel_crypto_priv *priv,
> + struct safexcel_ring *cdr,
> + struct safexcel_ring *rdr)
> +{
> + cdr->offset = sizeof(u32) * priv->config.cd_offset;
> + cdr->base = dmam_alloc_coherent(priv->dev,
> + cdr->offset * EIP197_DEFAULT_RING_SIZE,
> + &cdr->base_dma, GFP_KERNEL);
> + if (!cdr->base)
> + return -ENOMEM;
> + cdr->write = cdr->base;
> + cdr->base_end = cdr->base + cdr->offset * EIP197_DEFAULT_RING_SIZE;
> + cdr->read = cdr->base;
> +
> + rdr->offset = sizeof(u32) * priv->config.rd_offset;
> + rdr->base = dmam_alloc_coherent(priv->dev,
> + rdr->offset * EIP197_DEFAULT_RING_SIZE,
> + &rdr->base_dma, GFP_KERNEL);
> + if (!rdr->base) {
> + dmam_free_coherent(priv->dev,
> + cdr->offset * EIP197_DEFAULT_RING_SIZE,
> + cdr->base, cdr->base_dma);
Returning an error here will abort your probe routine, so devres will
clean these up there and then - there's no need to do free anything
explicitly. That's rather the point of using devm_*() to begin with.
> + return -ENOMEM;
> + }
> + rdr->write = rdr->base;
> + rdr->base_end = rdr->base + rdr->offset * EIP197_DEFAULT_RING_SIZE;
> + rdr->read = rdr->base;
> +
> + return 0;
> +}
> +
> +void safexcel_free_ring_descriptors(struct safexcel_crypto_priv *priv,
> + struct safexcel_ring *cdr,
> + struct safexcel_ring *rdr)
> +{
> + dmam_free_coherent(priv->dev,
> + cdr->offset * EIP197_DEFAULT_RING_SIZE,
> + cdr->base, cdr->base_dma);
> + dmam_free_coherent(priv->dev,
> + rdr->offset * EIP197_DEFAULT_RING_SIZE,
> + rdr->base, rdr->base_dma);
> +}
Again, this is only called at device teardown, so the whole function is
redundant.
Robin.
^ permalink raw reply
* [patch 13/13] crypto: n2 - Replace racy task affinity logic
From: Thomas Gleixner @ 2017-04-12 20:07 UTC (permalink / raw)
To: LKML
Cc: Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
Benjamin Herrenschmidt, David S. Miller, Fenghua Yu, Herbert Xu,
Lai Jiangshan, Len Brown, Michael Ellerman, Rafael J. Wysocki,
Tejun Heo, Tony Luck, Viresh Kumar, linux-crypto
In-Reply-To: <20170412200726.941336635@linutronix.de>
[-- Attachment #1: crypto--n2---Use-work_on_cpu---instead-of-affinity-games.patch --]
[-- Type: text/plain, Size: 2324 bytes --]
spu_queue_register() needs to invoke setup functions on a particular
CPU. This is achieved by temporarily setting the affinity of the
calling user space thread to the requested CPU and reset it to the original
affinity afterwards.
That's racy vs. CPU hotplug and concurrent affinity settings for that
thread resulting in code executing on the wrong CPU and overwriting the
new affinity setting.
Replace it by using work_on_cpu_safe() which guarantees to run the code on
the requested CPU or to fail in case the CPU is offline.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-crypto@vger.kernel.org
---
drivers/crypto/n2_core.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
--- a/drivers/crypto/n2_core.c
+++ b/drivers/crypto/n2_core.c
@@ -65,6 +65,11 @@ struct spu_queue {
struct list_head list;
};
+struct spu_qreg {
+ struct spu_queue *queue;
+ unsigned long type;
+};
+
static struct spu_queue **cpu_to_cwq;
static struct spu_queue **cpu_to_mau;
@@ -1631,31 +1636,27 @@ static void queue_cache_destroy(void)
kmem_cache_destroy(queue_cache[HV_NCS_QTYPE_CWQ - 1]);
}
-static int spu_queue_register(struct spu_queue *p, unsigned long q_type)
+static long spu_queue_register_workfn(void *arg)
{
- cpumask_var_t old_allowed;
+ struct spu_qreg *qr = arg;
+ struct spu_queue *p = qr->queue;
+ unsigned long q_type = qr->type;
unsigned long hv_ret;
- if (cpumask_empty(&p->sharing))
- return -EINVAL;
-
- if (!alloc_cpumask_var(&old_allowed, GFP_KERNEL))
- return -ENOMEM;
-
- cpumask_copy(old_allowed, ¤t->cpus_allowed);
-
- set_cpus_allowed_ptr(current, &p->sharing);
-
hv_ret = sun4v_ncs_qconf(q_type, __pa(p->q),
CWQ_NUM_ENTRIES, &p->qhandle);
if (!hv_ret)
sun4v_ncs_sethead_marker(p->qhandle, 0);
- set_cpus_allowed_ptr(current, old_allowed);
+ return hv_ret ? -EINVAL : 0;
+}
- free_cpumask_var(old_allowed);
+static int spu_queue_register(struct spu_queue *p, unsigned long q_type)
+{
+ int cpu = cpumask_any_and(&p->sharing, cpu_online_mask);
+ struct spu_qreg qr = { .queue = p, .type = q_type };
- return (hv_ret ? -EINVAL : 0);
+ return work_on_cpu_safe(cpu, &qr);
}
static int spu_queue_setup(struct spu_queue *p)
^ permalink raw reply
* [PATCH 0/2] Interrupt management fixes
From: Gary R Hook @ 2017-04-12 22:54 UTC (permalink / raw)
To: linux-crypto; +Cc: thomas.lendacky, herbert, davem
The following series implements...
Correct the driver to attend only to interrupt bits that
are relevant, and ensure that interrupts are managed
properly at module unload.
---
Gary R Hook (2):
crypto: ccp - Use only the relevant interrupt bits
crypto: ccp - Disable interrupts early on unload
drivers/crypto/ccp/ccp-dev-v5.c | 4 ++--
drivers/crypto/ccp/ccp-dev.h | 5 ++---
2 files changed, 4 insertions(+), 5 deletions(-)
^ permalink raw reply
* [PATCH 1/2] crypto: ccp - Use only the relevant interrupt bits
From: Gary R Hook @ 2017-04-12 22:54 UTC (permalink / raw)
To: linux-crypto; +Cc: thomas.lendacky, herbert, davem
In-Reply-To: <20170412225324.17707.75176.stgit@taos>
Each CCP queue can product interrupts for 4 conditions:
operation complete, queue empty, error, and queue stopped.
This driver only works with completion and error events.
Signed-off-by: Gary R Hook <gary.hook@amd.com>
---
drivers/crypto/ccp/ccp-dev.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/ccp/ccp-dev.h b/drivers/crypto/ccp/ccp-dev.h
index 191274d..a60aac7 100644
--- a/drivers/crypto/ccp/ccp-dev.h
+++ b/drivers/crypto/ccp/ccp-dev.h
@@ -109,9 +109,8 @@
#define INT_COMPLETION 0x1
#define INT_ERROR 0x2
#define INT_QUEUE_STOPPED 0x4
-#define ALL_INTERRUPTS (INT_COMPLETION| \
- INT_ERROR| \
- INT_QUEUE_STOPPED)
+#define INT_EMPTY_QUEUE 0x8
+#define ALL_INTERRUPTS (INT_COMPLETION | INT_ERROR)
#define LSB_REGION_WIDTH 5
#define MAX_LSB_CNT 8
^ permalink raw reply related
* [PATCH 2/2] crypto: ccp - Disable interrupts early on unload
From: Gary R Hook @ 2017-04-12 22:54 UTC (permalink / raw)
To: linux-crypto; +Cc: thomas.lendacky, herbert, davem
In-Reply-To: <20170412225324.17707.75176.stgit@taos>
From: Gary R Hook <ghook@amd.com>
Ensure that we disable interrupts first when shutting down
the driver.
Signed-off-by: Gary R Hook <ghook@amd.com>
---
drivers/crypto/ccp/ccp-dev-v5.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/ccp/ccp-dev-v5.c b/drivers/crypto/ccp/ccp-dev-v5.c
index e03d06a..293c08e 100644
--- a/drivers/crypto/ccp/ccp-dev-v5.c
+++ b/drivers/crypto/ccp/ccp-dev-v5.c
@@ -942,10 +942,10 @@ static void ccp5_destroy(struct ccp_device *ccp)
iowrite32(cmd_q->qcontrol & ~CMD5_Q_RUN, cmd_q->reg_control);
/* Disable the interrupts */
- iowrite32(ALL_INTERRUPTS, cmd_q->reg_interrupt_status);
+ iowrite32(0x00, cmd_q->reg_int_enable);
/* Clear the interrupt status */
- iowrite32(0x00, cmd_q->reg_int_enable);
+ iowrite32(ALL_INTERRUPTS, cmd_q->reg_interrupt_status);
ioread32(cmd_q->reg_int_status);
ioread32(cmd_q->reg_status);
}
^ permalink raw reply related
* Re: [patch 13/13] crypto: n2 - Replace racy task affinity logic
From: Herbert Xu @ 2017-04-13 4:56 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
Benjamin Herrenschmidt, David S. Miller, Fenghua Yu,
Lai Jiangshan, Len Brown, Michael Ellerman, Rafael J. Wysocki,
Tejun Heo, Tony Luck, Viresh Kumar, linux-crypto
In-Reply-To: <20170412201043.231299672@linutronix.de>
On Wed, Apr 12, 2017 at 10:07:39PM +0200, Thomas Gleixner wrote:
> spu_queue_register() needs to invoke setup functions on a particular
> CPU. This is achieved by temporarily setting the affinity of the
> calling user space thread to the requested CPU and reset it to the original
> affinity afterwards.
>
> That's racy vs. CPU hotplug and concurrent affinity settings for that
> thread resulting in code executing on the wrong CPU and overwriting the
> new affinity setting.
>
> Replace it by using work_on_cpu_safe() which guarantees to run the code on
> the requested CPU or to fail in case the CPU is offline.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-crypto@vger.kernel.org
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH 0/2] hwrng: mtk: add support for hardware random generator on MT7623 SoC
From: sean.wang-NuS5LvNUpcJWk0Htik3J/w @ 2017-04-13 7:05 UTC (permalink / raw)
To: herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q,
mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w,
prasannatsmkumar-Re5JQEeQqe8AvxtiuMwx3w,
romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
shannon.nelson-QHcLZuEGTsvQT0dZR+AlfA
Cc: weiyongjun1-hv44wF8Li93QT0dZR+AlfA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
keyhaede-Re5JQEeQqe8AvxtiuMwx3w, Sean Wang
From: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
This patchset introduces support for Mediatek hardware random generator (RNG)
Currently, the driver is already tested successfully with rng-tools on MT7623
SoC. And it should also be workable on other similar Mediatek SoCs.
SoC that also works on other similar SoCs.
Sean Wang (2):
dt-bindings: hwrng: Add Mediatek hardware random generator bindings
hwrng: mtk: Add driver for hardware random generator on MT7623 SoC
Documentation/devicetree/bindings/rng/mtk-rng.txt | 18 +++
drivers/char/hw_random/Kconfig | 16 +-
drivers/char/hw_random/Makefile | 2 +-
drivers/char/hw_random/mtk-rng.c | 174 ++++++++++++++++++++++
4 files changed, 208 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/rng/mtk-rng.txt
create mode 100644 drivers/char/hw_random/mtk-rng.c
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/2] dt-bindings: hwrng: Add Mediatek hardware random generator bindings
From: sean.wang @ 2017-04-13 7:05 UTC (permalink / raw)
To: herbert, mpm, robh+dt, mark.rutland, clabbe.montjoie,
prasannatsmkumar, romain.perier, shannon.nelson
Cc: weiyongjun1, devicetree, linux-crypto, linux-mediatek,
linux-arm-kernel, linux-kernel, keyhaede, Sean Wang
In-Reply-To: <1492067108-14748-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
Document the devicetree bindings for Mediatek random number
generator which could be found on MT7623 SoC or other similar
Mediatek SoCs.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
Documentation/devicetree/bindings/rng/mtk-rng.txt | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rng/mtk-rng.txt
diff --git a/Documentation/devicetree/bindings/rng/mtk-rng.txt b/Documentation/devicetree/bindings/rng/mtk-rng.txt
new file mode 100644
index 0000000..a6d62a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/rng/mtk-rng.txt
@@ -0,0 +1,18 @@
+Device-Tree bindings for Mediatek random number generator
+found in Mediatek SoC family
+
+Required properties:
+- compatible : Should be "mediatek,mt7623-rng"
+- clocks : list of clock specifiers, corresponding to
+ entries in clock-names property;
+- clock-names : Should contain "rng" entries;
+- reg : Specifies base physical address and size of the registers
+
+Example:
+
+rng: rng@1020f000 {
+ compatible = "mediatek,mt7623-rng";
+ reg = <0 0x1020f000 0 0x1000>;
+ clocks = <&infracfg CLK_INFRA_TRNG>;
+ clock-names = "rng";
+};
--
1.9.1
^ permalink raw reply related
* [PATCH 2/2] hwrng: mtk: Add driver for hardware random generator on MT7623 SoC
From: sean.wang @ 2017-04-13 7:05 UTC (permalink / raw)
To: herbert, mpm, robh+dt, mark.rutland, clabbe.montjoie,
prasannatsmkumar, romain.perier, shannon.nelson
Cc: weiyongjun1, devicetree, linux-crypto, linux-mediatek,
linux-arm-kernel, linux-kernel, keyhaede, Sean Wang
In-Reply-To: <1492067108-14748-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
This patch adds support for hardware random generator on MT7623 SoC
and should also work on other similar Mediatek SoCs. Currently,
the driver is already tested successfully with rng-tools.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/char/hw_random/Kconfig | 16 +++-
drivers/char/hw_random/Makefile | 2 +-
drivers/char/hw_random/mtk-rng.c | 174 +++++++++++++++++++++++++++++++++++++++
3 files changed, 190 insertions(+), 2 deletions(-)
create mode 100644 drivers/char/hw_random/mtk-rng.c
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 0cafe08..af782ce 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -419,10 +419,24 @@ config HW_RANDOM_CAVIUM
Generator hardware found on Cavium SoCs.
To compile this driver as a module, choose M here: the
- module will be called cavium_rng.
+ module will be called mtk-rng.
If unsure, say Y.
+config HW_RANDOM_MTK
+ tristate "Mediatek Random Number Generator support"
+ depends on HW_RANDOM
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ default y
+ ---help---
+ This driver provides kernel-side support for the Random Number
+ Generator hardware found on Mediatek SoCs.
+
+ To compile this driver as a module, choose M here. the
+ module will be called mtk-rng.
+
+ If unsure, say Y.
+
endif # HW_RANDOM
config UML_RANDOM
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 5f52b1e..68be716 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -1,7 +1,6 @@
#
# Makefile for HW Random Number Generator (RNG) device drivers.
#
-
obj-$(CONFIG_HW_RANDOM) += rng-core.o
rng-core-y := core.o
obj-$(CONFIG_HW_RANDOM_TIMERIOMEM) += timeriomem-rng.o
@@ -36,3 +35,4 @@ obj-$(CONFIG_HW_RANDOM_STM32) += stm32-rng.o
obj-$(CONFIG_HW_RANDOM_PIC32) += pic32-rng.o
obj-$(CONFIG_HW_RANDOM_MESON) += meson-rng.o
obj-$(CONFIG_HW_RANDOM_CAVIUM) += cavium-rng.o cavium-rng-vf.o
+obj-$(CONFIG_HW_RANDOM_MTK) += mtk-rng.o
diff --git a/drivers/char/hw_random/mtk-rng.c b/drivers/char/hw_random/mtk-rng.c
new file mode 100644
index 0000000..6561ee0
--- /dev/null
+++ b/drivers/char/hw_random/mtk-rng.c
@@ -0,0 +1,174 @@
+/*
+ * Driver for Mediatek Hardware Random Number Generator
+ *
+ * Copyright (C) 2017 Sean Wang <sean.wang@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#define MTK_RNG_DEV KBUILD_MODNAME
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/hw_random.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#define USEC_POLL 2
+#define TIMEOUT_POLL 20
+
+#define RNG_CTRL 0x00
+#define RNG_EN BIT(0)
+#define RNG_READY BIT(31)
+
+#define RNG_DATA 0x08
+
+#define to_mtk_rng(p) container_of(p, struct mtk_rng, rng)
+
+struct mtk_rng {
+ struct device *dev;
+ void __iomem *base;
+ struct clk *clk;
+ struct hwrng rng;
+};
+
+static int mtk_rng_init(struct hwrng *rng)
+{
+ struct mtk_rng *priv = to_mtk_rng(rng);
+ u32 val;
+ int err;
+
+ err = clk_prepare_enable(priv->clk);
+ if (err)
+ return err;
+
+ val = readl(priv->base + RNG_CTRL);
+ val |= RNG_EN;
+ writel(val, priv->base + RNG_CTRL);
+
+ return 0;
+}
+
+static void mtk_rng_cleanup(struct hwrng *rng)
+{
+ struct mtk_rng *priv = to_mtk_rng(rng);
+ u32 val;
+
+ val = readl(priv->base + RNG_CTRL);
+ val &= ~RNG_EN;
+ writel(val, priv->base + RNG_CTRL);
+
+ clk_disable_unprepare(priv->clk);
+}
+
+static bool mtk_rng_wait_ready(struct hwrng *rng, bool wait)
+{
+ struct mtk_rng *priv = to_mtk_rng(rng);
+ int ready;
+
+ ready = readl(priv->base + RNG_CTRL) & RNG_READY;
+ if (!ready && wait)
+ readl_poll_timeout_atomic(priv->base + RNG_CTRL, ready,
+ ready & RNG_READY, USEC_POLL,
+ TIMEOUT_POLL);
+ return !!ready;
+}
+
+static int mtk_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
+{
+ struct mtk_rng *priv = to_mtk_rng(rng);
+ int retval = 0;
+
+ while (max >= sizeof(u32)) {
+ if (!mtk_rng_wait_ready(rng, wait))
+ break;
+
+ *(u32 *)buf = readl(priv->base + RNG_DATA);
+ retval += sizeof(u32);
+ buf += sizeof(u32);
+ max -= sizeof(u32);
+ }
+
+ if (unlikely(wait && max))
+ dev_warn(priv->dev, "timeout might be not properly set\n");
+
+ return retval || !wait ? retval : -EIO;
+}
+
+static int mtk_rng_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ int ret;
+ struct mtk_rng *priv;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "no iomem resource\n");
+ return -ENXIO;
+ }
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->dev = &pdev->dev;
+ priv->rng.name = pdev->name;
+ priv->rng.init = mtk_rng_init;
+ priv->rng.cleanup = mtk_rng_cleanup;
+ priv->rng.read = mtk_rng_read;
+
+ priv->clk = devm_clk_get(&pdev->dev, "rng");
+ if (IS_ERR(priv->clk)) {
+ ret = PTR_ERR(priv->clk);
+ dev_err(&pdev->dev, "no clock for device: %d\n", ret);
+ return ret;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ priv->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ ret = devm_hwrng_register(&pdev->dev, &priv->rng);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register rng device: %d\n",
+ ret);
+ return ret;
+ }
+
+ dev_info(&pdev->dev, "registered RNG driver\n");
+
+ return 0;
+}
+
+static const struct of_device_id mtk_rng_match[] = {
+ { .compatible = "mediatek,mt7623-rng" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mtk_rng_match);
+
+static struct platform_driver mtk_rng_driver = {
+ .probe = mtk_rng_probe,
+ .driver = {
+ .name = MTK_RNG_DEV,
+ .of_match_table = mtk_rng_match,
+ },
+};
+
+module_platform_driver(mtk_rng_driver);
+
+MODULE_DESCRIPTION("Mediatek Random Number Generator Driver");
+MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>");
+MODULE_LICENSE("GPL");
--
1.9.1
^ permalink raw reply related
* [patch V2 13/13] crypto: n2 - Replace racy task affinity logic
From: Thomas Gleixner @ 2017-04-13 8:20 UTC (permalink / raw)
To: LKML
Cc: Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
Benjamin Herrenschmidt, David S. Miller, Fenghua Yu, Herbert Xu,
Lai Jiangshan, Len Brown, Michael Ellerman, Rafael J. Wysocki,
Tejun Heo, Tony Luck, Viresh Kumar, linux-crypto
In-Reply-To: <20170412201043.231299672@linutronix.de>
spu_queue_register() needs to invoke setup functions on a particular
CPU. This is achieved by temporarily setting the affinity of the
calling user space thread to the requested CPU and reset it to the original
affinity afterwards.
That's racy vs. CPU hotplug and concurrent affinity settings for that
thread resulting in code executing on the wrong CPU and overwriting the
new affinity setting.
Replace it by using work_on_cpu_safe() which guarantees to run the code on
the requested CPU or to fail in case the CPU is offline.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-crypto@vger.kernel.org
---
V2: Fixup build-bot complaints
drivers/crypto/n2_core.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
--- a/drivers/crypto/n2_core.c
+++ b/drivers/crypto/n2_core.c
@@ -65,6 +65,11 @@ struct spu_queue {
struct list_head list;
};
+struct spu_qreg {
+ struct spu_queue *queue;
+ unsigned long type;
+};
+
static struct spu_queue **cpu_to_cwq;
static struct spu_queue **cpu_to_mau;
@@ -1631,31 +1636,27 @@ static void queue_cache_destroy(void)
kmem_cache_destroy(queue_cache[HV_NCS_QTYPE_CWQ - 1]);
}
-static int spu_queue_register(struct spu_queue *p, unsigned long q_type)
+static long spu_queue_register_workfn(void *arg)
{
- cpumask_var_t old_allowed;
+ struct spu_qreg *qr = arg;
+ struct spu_queue *p = qr->queue;
+ unsigned long q_type = qr->type;
unsigned long hv_ret;
- if (cpumask_empty(&p->sharing))
- return -EINVAL;
-
- if (!alloc_cpumask_var(&old_allowed, GFP_KERNEL))
- return -ENOMEM;
-
- cpumask_copy(old_allowed, ¤t->cpus_allowed);
-
- set_cpus_allowed_ptr(current, &p->sharing);
-
hv_ret = sun4v_ncs_qconf(q_type, __pa(p->q),
CWQ_NUM_ENTRIES, &p->qhandle);
if (!hv_ret)
sun4v_ncs_sethead_marker(p->qhandle, 0);
- set_cpus_allowed_ptr(current, old_allowed);
+ return hv_ret ? -EINVAL : 0;
+}
- free_cpumask_var(old_allowed);
+static int spu_queue_register(struct spu_queue *p, unsigned long q_type)
+{
+ int cpu = cpumask_any_and(&p->sharing, cpu_online_mask);
+ struct spu_qreg qr = { .queue = p, .type = q_type };
- return (hv_ret ? -EINVAL : 0);
+ return work_on_cpu_safe(cpu, spu_queue_register_workfn, &qr);
}
static int spu_queue_setup(struct spu_queue *p)
^ permalink raw reply
* Re: [PATCH 2/2] hwrng: mtk: Add driver for hardware random generator on MT7623 SoC
From: PrasannaKumar Muralidharan @ 2017-04-13 8:39 UTC (permalink / raw)
To: sean.wang
Cc: Herbert Xu, Matt Mackall, Rob Herring, Mark Rutland,
Corentin LABBE, Romain Perier, shannon.nelson, Wei Yongjun,
devicetree, linux-crypto, linux-mediatek, linux-arm-kernel,
linux-kernel, keyhaede
In-Reply-To: <1492067108-14748-3-git-send-email-sean.wang@mediatek.com>
Hi Sean,
Mostly looks good, have few minor comments.
On 13 April 2017 at 12:35, <sean.wang@mediatek.com> wrote:
> +static bool mtk_rng_wait_ready(struct hwrng *rng, bool wait)
> +{
> + struct mtk_rng *priv = to_mtk_rng(rng);
> + int ready;
> +
> + ready = readl(priv->base + RNG_CTRL) & RNG_READY;
> + if (!ready && wait)
> + readl_poll_timeout_atomic(priv->base + RNG_CTRL, ready,
> + ready & RNG_READY, USEC_POLL,
> + TIMEOUT_POLL);
> + return !!ready;
> +}
Use readl_poll_timeout_atomic's return value or -EIO instead of
!!ready. This will simplify mtk_rng_read.
> +static int mtk_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
> +{
> + struct mtk_rng *priv = to_mtk_rng(rng);
> + int retval = 0;
> +
> + while (max >= sizeof(u32)) {
> + if (!mtk_rng_wait_ready(rng, wait))
> + break;
> +
> + *(u32 *)buf = readl(priv->base + RNG_DATA);
> + retval += sizeof(u32);
> + buf += sizeof(u32);
> + max -= sizeof(u32);
> + }
> +
> + if (unlikely(wait && max))
> + dev_warn(priv->dev, "timeout might be not properly set\n");
Is this really necessary? Better to choose proper timeout than
providing this warning message. In rare cases if the timeout could
occur due to some reason (may be a hardware fault) print appropriate
warning message.
> + return retval || !wait ? retval : -EIO;
> +}
Set retavl to mtk_rng_wait_ready and return retval.
Regards,
Prasanna
^ permalink raw reply
* Re: [RFC PATCH v5] IV Generation algorithms for dm-crypt
From: Binoy Jayan @ 2017-04-13 8:46 UTC (permalink / raw)
To: Milan Broz
Cc: Oded, Ofir, Herbert Xu, David S. Miller, linux-crypto, Mark Brown,
Arnd Bergmann, Linux kernel mailing list, Alasdair Kergon,
Mike Snitzer, dm-devel, Shaohua Li, linux-raid, Rajendra, Gilad
In-Reply-To: <774a9713-05ef-0162-0203-461254a04f6e@gmail.com>
Hi Milan,
On 10 April 2017 at 19:30, Milan Broz <gmazyland@gmail.com> wrote:
Thank you for the reply.
> Well, it is good that there is no performance degradation but it
> would be nice to have some user of it that proves it is really
> working for your hw.
I have been able to get access to a hardware with IV generation support
a few days back. The hardware I was having before did not have IV
generation support. Will be able to come up with numbers after making
it work with the new one.
> FYI - with patch that increases dmcrypt sector size to 4k
> I can see improvement in speed usually in 5-15% with sync AES-NI
> (depends on access pattern), with dmcrypt mapped to memory
> it is even close to 20% speed up (but such a configuration is
> completely artificial).
>
> I wonder why increased dmcrypt sector size does not work for your hw,
> it should help as well (and can be combiuned later with this IV approach).
> (For native 4k drives this should be used in future anyway...)
I think it should work well too with backward incompatibility.
Thanks,
Binoy
^ permalink raw reply
* [PATCH] padata: allow caller to control queue length
From: Jason A. Donenfeld @ 2017-04-13 9:52 UTC (permalink / raw)
To: linux-crypto, linux-kernel, steffen.klassert; +Cc: Jason A. Donenfeld
Allow users of padata to determine the queue length themselves, via this
added helper function, so that we can later remove the hard-coded 1000-
job limit. We thus add a helper function, and then move the limiting
functionality to pcrypt-proper, since it was the only current consumer
relying on the 1000-job limit. We do, however, impose a limit on padata
so that the reference count does not have an integer overflow.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Documentation/padata.txt | 8 ++++++++
crypto/pcrypt.c | 5 +++++
include/linux/padata.h | 2 ++
kernel/padata.c | 20 +++++++++++++++++---
4 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/Documentation/padata.txt b/Documentation/padata.txt
index 7ddfe216a0aa..9347d145bb27 100644
--- a/Documentation/padata.txt
+++ b/Documentation/padata.txt
@@ -158,3 +158,11 @@ when a padata instance is no longer needed:
This function will busy-wait while any remaining tasks are completed, so it
might be best not to call it while there is work outstanding. Shutting
down the workqueue, if necessary, should be done separately.
+
+While you cannot have more than 2^31-1 taks submitted at the same time, this
+maximum is well above what you might actually want to be submitted. Thus,
+callers are encouraged to determine their maximum latency/memory/throughput
+constraints, and limit calls to padata_do_parallel() based on the current
+queue length, which can be determined with:
+
+ int padata_queue_len(struct padata_instance *pinst);
diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
index ee9cfb99fe25..ea321154994b 100644
--- a/crypto/pcrypt.c
+++ b/crypto/pcrypt.c
@@ -70,6 +70,8 @@ struct pcrypt_aead_ctx {
unsigned int cb_cpu;
};
+#define MAX_OBJ_NUM 1000
+
static int pcrypt_do_parallel(struct padata_priv *padata, unsigned int *cb_cpu,
struct padata_pcrypt *pcrypt)
{
@@ -78,6 +80,9 @@ static int pcrypt_do_parallel(struct padata_priv *padata, unsigned int *cb_cpu,
cpu = *cb_cpu;
+ if (padata_queue_len(pcrypt->pinst) >= MAX_OBJ_NUM)
+ return -EBUSY;
+
rcu_read_lock_bh();
cpumask = rcu_dereference_bh(pcrypt->cb_cpumask);
if (cpumask_test_cpu(cpu, cpumask->mask))
diff --git a/include/linux/padata.h b/include/linux/padata.h
index 0f9e567d5e15..2482b442f136 100644
--- a/include/linux/padata.h
+++ b/include/linux/padata.h
@@ -3,6 +3,7 @@
*
* Copyright (C) 2008, 2009 secunet Security Networks AG
* Copyright (C) 2008, 2009 Steffen Klassert <steffen.klassert@secunet.com>
+ * Copyright (C) 2016, 2017 Jason A. Donenfeld <Jason@zx2c4.com>
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -181,4 +182,5 @@ extern int padata_register_cpumask_notifier(struct padata_instance *pinst,
struct notifier_block *nblock);
extern int padata_unregister_cpumask_notifier(struct padata_instance *pinst,
struct notifier_block *nblock);
+extern int padata_queue_len(struct padata_instance *pinst);
#endif
diff --git a/kernel/padata.c b/kernel/padata.c
index ac8f1e524836..6ba2db73413f 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -5,6 +5,7 @@
*
* Copyright (C) 2008, 2009 secunet Security Networks AG
* Copyright (C) 2008, 2009 Steffen Klassert <steffen.klassert@secunet.com>
+ * Copyright (C) 2016, 2017 Jason A. Donenfeld <Jason@zx2c4.com>
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -32,8 +33,6 @@
#include <linux/rcupdate.h>
#include <linux/module.h>
-#define MAX_OBJ_NUM 1000
-
static int padata_index_to_cpu(struct parallel_data *pd, int cpu_index)
{
int cpu, target_cpu;
@@ -122,7 +121,7 @@ int padata_do_parallel(struct padata_instance *pinst,
if ((pinst->flags & PADATA_RESET))
goto out;
- if (atomic_read(&pd->refcnt) >= MAX_OBJ_NUM)
+ if (unlikely(atomic_read(&pd->refcnt) == INT_MAX))
goto out;
err = 0;
@@ -1021,6 +1020,21 @@ void padata_free(struct padata_instance *pinst)
}
EXPORT_SYMBOL(padata_free);
+/**
+ * padata_queue_len - retreive the number of in progress jobs
+ *
+ * @padata_inst: padata instance from which to read the queue size
+ */
+int padata_queue_len(struct padata_instance *pinst)
+{
+ int len;
+ rcu_read_lock_bh();
+ len = atomic_read(&rcu_dereference_bh(pinst->pd)->refcnt);
+ rcu_read_unlock_bh();
+ return len;
+}
+EXPORT_SYMBOL(padata_queue_len);
+
#ifdef CONFIG_HOTPLUG_CPU
static __init int padata_driver_init(void)
--
2.12.2
^ permalink raw reply related
* Re: [PATCH 2/2] hwrng: mtk: Add driver for hardware random generator on MT7623 SoC
From: Corentin Labbe @ 2017-04-13 11:06 UTC (permalink / raw)
To: sean.wang-NuS5LvNUpcJWk0Htik3J/w
Cc: herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q,
mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, prasannatsmkumar-Re5JQEeQqe8AvxtiuMwx3w,
romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
shannon.nelson-QHcLZuEGTsvQT0dZR+AlfA,
weiyongjun1-hv44wF8Li93QT0dZR+AlfA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
keyhaede-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1492067108-14748-3-git-send-email-sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Hello
I have some minor comment below:
On Thu, Apr 13, 2017 at 03:05:08PM +0800, sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> From: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>
> This patch adds support for hardware random generator on MT7623 SoC
> and should also work on other similar Mediatek SoCs. Currently,
> the driver is already tested successfully with rng-tools.
>
> Signed-off-by: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
> drivers/char/hw_random/Kconfig | 16 +++-
> drivers/char/hw_random/Makefile | 2 +-
> drivers/char/hw_random/mtk-rng.c | 174 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 190 insertions(+), 2 deletions(-)
> create mode 100644 drivers/char/hw_random/mtk-rng.c
>
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 0cafe08..af782ce 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -419,10 +419,24 @@ config HW_RANDOM_CAVIUM
> Generator hardware found on Cavium SoCs.
>
> To compile this driver as a module, choose M here: the
> - module will be called cavium_rng.
> + module will be called mtk-rng.
Unwanted change
>
> If unsure, say Y.
>
> +config HW_RANDOM_MTK
> + tristate "Mediatek Random Number Generator support"
> + depends on HW_RANDOM
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + default y
> + ---help---
> + This driver provides kernel-side support for the Random Number
> + Generator hardware found on Mediatek SoCs.
> +
> + To compile this driver as a module, choose M here. the
> + module will be called mtk-rng.
> +
> + If unsure, say Y.
> +
> endif # HW_RANDOM
>
> config UML_RANDOM
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index 5f52b1e..68be716 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -1,7 +1,6 @@
> #
> # Makefile for HW Random Number Generator (RNG) device drivers.
> #
> -
Another unwanted change
> obj-$(CONFIG_HW_RANDOM) += rng-core.o
> rng-core-y := core.o
> obj-$(CONFIG_HW_RANDOM_TIMERIOMEM) += timeriomem-rng.o
> @@ -36,3 +35,4 @@ obj-$(CONFIG_HW_RANDOM_STM32) += stm32-rng.o
> obj-$(CONFIG_HW_RANDOM_PIC32) += pic32-rng.o
> obj-$(CONFIG_HW_RANDOM_MESON) += meson-rng.o
> obj-$(CONFIG_HW_RANDOM_CAVIUM) += cavium-rng.o cavium-rng-vf.o
> +obj-$(CONFIG_HW_RANDOM_MTK) += mtk-rng.o
> diff --git a/drivers/char/hw_random/mtk-rng.c b/drivers/char/hw_random/mtk-rng.c
> new file mode 100644
> index 0000000..6561ee0
> --- /dev/null
> +++ b/drivers/char/hw_random/mtk-rng.c
> @@ -0,0 +1,174 @@
> +/*
> + * Driver for Mediatek Hardware Random Number Generator
> + *
> + * Copyright (C) 2017 Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +#define MTK_RNG_DEV KBUILD_MODNAME
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/hw_random.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#define USEC_POLL 2
> +#define TIMEOUT_POLL 20
> +
> +#define RNG_CTRL 0x00
> +#define RNG_EN BIT(0)
> +#define RNG_READY BIT(31)
Keep only one space between define and name
> +
> +#define RNG_DATA 0x08
> +
> +#define to_mtk_rng(p) container_of(p, struct mtk_rng, rng)
> +
> +struct mtk_rng {
> + struct device *dev;
> + void __iomem *base;
> + struct clk *clk;
> + struct hwrng rng;
> +};
> +
> +static int mtk_rng_init(struct hwrng *rng)
> +{
> + struct mtk_rng *priv = to_mtk_rng(rng);
> + u32 val;
> + int err;
> +
> + err = clk_prepare_enable(priv->clk);
> + if (err)
> + return err;
> +
> + val = readl(priv->base + RNG_CTRL);
> + val |= RNG_EN;
> + writel(val, priv->base + RNG_CTRL);
> +
> + return 0;
> +}
> +
> +static void mtk_rng_cleanup(struct hwrng *rng)
> +{
> + struct mtk_rng *priv = to_mtk_rng(rng);
> + u32 val;
> +
> + val = readl(priv->base + RNG_CTRL);
> + val &= ~RNG_EN;
> + writel(val, priv->base + RNG_CTRL);
> +
> + clk_disable_unprepare(priv->clk);
> +}
> +
> +static bool mtk_rng_wait_ready(struct hwrng *rng, bool wait)
> +{
> + struct mtk_rng *priv = to_mtk_rng(rng);
> + int ready;
> +
> + ready = readl(priv->base + RNG_CTRL) & RNG_READY;
> + if (!ready && wait)
> + readl_poll_timeout_atomic(priv->base + RNG_CTRL, ready,
> + ready & RNG_READY, USEC_POLL,
> + TIMEOUT_POLL);
> + return !!ready;
> +}
> +
> +static int mtk_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
> +{
> + struct mtk_rng *priv = to_mtk_rng(rng);
> + int retval = 0;
> +
> + while (max >= sizeof(u32)) {
> + if (!mtk_rng_wait_ready(rng, wait))
> + break;
> +
> + *(u32 *)buf = readl(priv->base + RNG_DATA);
> + retval += sizeof(u32);
> + buf += sizeof(u32);
> + max -= sizeof(u32);
> + }
> +
> + if (unlikely(wait && max))
> + dev_warn(priv->dev, "timeout might be not properly set\n");
> +
> + return retval || !wait ? retval : -EIO;
> +}
> +
> +static int mtk_rng_probe(struct platform_device *pdev)
> +{
> + struct resource *res;
> + int ret;
> + struct mtk_rng *priv;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "no iomem resource\n");
> + return -ENXIO;
> + }
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->dev = &pdev->dev;
> + priv->rng.name = pdev->name;
> + priv->rng.init = mtk_rng_init;
> + priv->rng.cleanup = mtk_rng_cleanup;
> + priv->rng.read = mtk_rng_read;
> +
> + priv->clk = devm_clk_get(&pdev->dev, "rng");
> + if (IS_ERR(priv->clk)) {
> + ret = PTR_ERR(priv->clk);
> + dev_err(&pdev->dev, "no clock for device: %d\n", ret);
> + return ret;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
You get that resource twice
Regards
Corentin Labbe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 0/2] Fix/improve some error handling related to 'chcr_alloc_shash'
From: Christophe JAILLET @ 2017-04-13 12:13 UTC (permalink / raw)
To: herbert, davem, harsh, hariprasad
Cc: linux-crypto, linux-kernel, kernel-janitors, Christophe JAILLET
This serie is divided into 2 patches. They are more or less related to the
same issue, but the first patch is not a bug in itself, just a clean-up
(IMHO).
If I'm correct, the 2nd one, is a real (unlikely) issue.
Christophe JAILLET (2):
crypto: chcr - Improve error checking
crypto: chcr - Fix error checking
drivers/crypto/chelsio/chcr_algo.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH 1/2] crypto: chcr - Improve error checking
From: Christophe JAILLET @ 2017-04-13 12:14 UTC (permalink / raw)
To: herbert, davem, harsh, hariprasad
Cc: linux-crypto, linux-kernel, kernel-janitors, Christophe JAILLET
In-Reply-To: <cover.1492085165.git.christophe.jaillet@wanadoo.fr>
'chcr_alloc_shash()' can return NULL. Here it is not possible because this
code is reached only if 'get_alg_config()' a few lines above has succeeded.
So we are garanteed that the value of 'max_authsize' is a correct
parameter.
Anyway, this is harmless to add a check for NULL.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/crypto/chelsio/chcr_algo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/chelsio/chcr_algo.c b/drivers/crypto/chelsio/chcr_algo.c
index 41bc7f4f58cd..f19590ac8775 100644
--- a/drivers/crypto/chelsio/chcr_algo.c
+++ b/drivers/crypto/chelsio/chcr_algo.c
@@ -2294,7 +2294,7 @@ static int chcr_authenc_setkey(struct crypto_aead *authenc, const u8 *key,
aeadctx->enckey_len << 3);
base_hash = chcr_alloc_shash(max_authsize);
- if (IS_ERR(base_hash)) {
+ if (IS_ERR_OR_NULL(base_hash)) {
pr_err("chcr : Base driver cannot be loaded\n");
goto out;
}
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox