* Re: [PATCH] crypto: zip - add a cast for printing atomic64_t values
From: Herbert Xu @ 2017-03-24 14:14 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S. Miller, Mahipal Challa, Jan Glauber, linux-crypto,
linux-kernel
In-Reply-To: <20170320123955.4061033-1-arnd@arndb.de>
On Mon, Mar 20, 2017 at 01:39:16PM +0100, Arnd Bergmann wrote:
> kernelci.org reports a build-time regression on linux-next, with a harmless
> warning in x86 allmodconfig:
>
> drivers/crypto/cavium/zip/zip_main.c:489:18: warning: format '%ld' expects argument of type 'long int', but argument 7 has type 'long long int' [-Wformat=]
> drivers/crypto/cavium/zip/zip_main.c:489:18: warning: format '%ld' expects argument of type 'long int', but argument 6 has type 'long long int' [-Wformat=]
> drivers/crypto/cavium/zip/zip_main.c:489:18: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'long long int' [-Wformat=]
>
> The return type for atomic64_read() unfortunately differs between
> architectures, with some defining it as atomic_long_read() and others
> returning a 64-bit type explicitly. Fixing this in general would be nice,
> but also require changing other users of these functions, so the simpler
> workaround is to add a cast here that avoids the warnings on the default
> build.
>
> Fixes: 09ae5d37e093 ("crypto: zip - Add Compression/Decompression statistics")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Patch applied. 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
* Re: [PATCH] crypto: zip - Memory corruption in zip_clear_stats()
From: Herbert Xu @ 2017-03-24 14:14 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mahipal Challa, David S. Miller, Jan Glauber, linux-crypto,
kernel-janitors
In-Reply-To: <20170317204621.GD16505@mwanda>
On Fri, Mar 17, 2017 at 11:46:21PM +0300, Dan Carpenter wrote:
> There is a typo here. It should be "stats" instead of "state". The
> impact is that we clear 224 bytes instead of 80 and we zero out memory
> that we shouldn't.
>
> Fixes: 09ae5d37e093 ("crypto: zip - Add Compression/Decompression statistics")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Patch applied. 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
* Re: [PATCH] crypto: testmgr - mark ctr(des3_ede) as fips_allowed
From: Herbert Xu @ 2017-03-24 14:14 UTC (permalink / raw)
To: Marcelo Henrique Cerri
Cc: linux-crypto, David S. Miller, linux-kernel, Stephan Muller
In-Reply-To: <1490041685-25947-1-git-send-email-marcelo.cerri@canonical.com>
On Mon, Mar 20, 2017 at 05:28:05PM -0300, Marcelo Henrique Cerri wrote:
> 3DES is missing the fips_allowed flag for CTR mode.
>
> Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
Patch applied. 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
* Re: [PATCH v2] dt-bindings: rng: clocks property on omap_rng not always mandatory
From: Herbert Xu @ 2017-03-24 14:13 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: devicetree, Rob Herring, Ian Campbell, Pawel Moll, Mark Rutland,
Kumar Gala, linux-crypto, romain.perier, Nadav Haklai, Hanna Hawa,
Yehuda Yitschak, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, stable
In-Reply-To: <1489755519-32664-1-git-send-email-thomas.petazzoni@free-electrons.com>
On Fri, Mar 17, 2017 at 01:58:39PM +0100, Thomas Petazzoni wrote:
> Commit 52060836f79 ("dt-bindings: omap-rng: Document SafeXcel IP-76
> device variant") update the omap_rng Device Tree binding to add support
> for the IP-76 variation of the IP. As part of this change, a "clocks"
> property was added, but is indicated as "Required", without indicated
> it's actually only required for some compatible strings.
>
> This commit fixes that, by explicitly stating that the clocks property
> is only required with the inside-secure,safexcel-eip76 compatible
> string.
>
> Fixes: 52060836f79 ("dt-bindings: omap-rng: Document SafeXcel IP-76 device variant")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Patch applied. 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
* Re: [PATCH] crypto: DRBG - initialize SGL only once
From: Herbert Xu @ 2017-03-24 14:15 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <1909030.BalyQCuxjY@positron.chronox.de>
On Wed, Mar 22, 2017 at 03:26:36PM +0100, Stephan Müller wrote:
> An SGL to be initialized only once even when its buffers are written
> to several times.
>
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
Patch applied. 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
* Re: [PATCH] crypto: xts,lrw - fix out-of-bounds write after kmalloc failure
From: Herbert Xu @ 2017-03-24 14:15 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-crypto, David S . Miller, linux-kernel, Dmitry Vyukov,
syzkaller, Eric Biggers, stable
In-Reply-To: <20170323203946.11242-1-ebiggers3@gmail.com>
On Thu, Mar 23, 2017 at 01:39:46PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> In the generic XTS and LRW algorithms, for input data > 128 bytes, a
> temporary buffer is allocated to hold the values to be XOR'ed with the
> data before and after encryption or decryption. If the allocation
> fails, the fixed-size buffer embedded in the request buffer is meant to
> be used as a fallback --- resulting in more calls to the ECB algorithm,
> but still producing the correct result. However, we weren't correctly
> limiting subreq->cryptlen in this case, resulting in pre_crypt()
> overrunning the embedded buffer. Fix this by setting subreq->cryptlen
> correctly.
>
> Fixes: f1c131b45410 ("crypto: xts - Convert to skcipher")
> Fixes: 700cb3f5fe75 ("crypto: lrw - Convert to skcipher")
> Cc: stable@vger.kernel.org # v4.10+
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Patch applied. 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
* Re: [PATCH] padata: avoid race in reordering
From: Herbert Xu @ 2017-03-24 14:16 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: steffen.klassert, linux-crypto, linux-kernel, netdev, Jason
In-Reply-To: <20170323112443.30843-1-Jason@zx2c4.com>
Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Under extremely heavy uses of padata, crashes occur, and with list
> debugging turned on, this happens instead:
>
> [87487.298728] WARNING: CPU: 1 PID: 882 at lib/list_debug.c:33
> __list_add+0xae/0x130
> [87487.301868] list_add corruption. prev->next should be next
> (ffffb17abfc043d0), but was ffff8dba70872c80. (prev=ffff8dba70872b00).
> [87487.339011] [<ffffffff9a53d075>] dump_stack+0x68/0xa3
> [87487.342198] [<ffffffff99e119a1>] ? console_unlock+0x281/0x6d0
> [87487.345364] [<ffffffff99d6b91f>] __warn+0xff/0x140
> [87487.348513] [<ffffffff99d6b9aa>] warn_slowpath_fmt+0x4a/0x50
> [87487.351659] [<ffffffff9a58b5de>] __list_add+0xae/0x130
> [87487.354772] [<ffffffff9add5094>] ? _raw_spin_lock+0x64/0x70
> [87487.357915] [<ffffffff99eefd66>] padata_reorder+0x1e6/0x420
> [87487.361084] [<ffffffff99ef0055>] padata_do_serial+0xa5/0x120
>
> padata_reorder calls list_add_tail with the list to which its adding
> locked, which seems correct:
>
> spin_lock(&squeue->serial.lock);
> list_add_tail(&padata->list, &squeue->serial.list);
> spin_unlock(&squeue->serial.lock);
>
> This therefore leaves only place where such inconsistency could occur:
> if padata->list is added at the same time on two different threads.
> This pdata pointer comes from the function call to
> padata_get_next(pd), which has in it the following block:
>
> next_queue = per_cpu_ptr(pd->pqueue, cpu);
> padata = NULL;
> reorder = &next_queue->reorder;
> if (!list_empty(&reorder->list)) {
> padata = list_entry(reorder->list.next,
> struct padata_priv, list);
> spin_lock(&reorder->lock);
> list_del_init(&padata->list);
> atomic_dec(&pd->reorder_objects);
> spin_unlock(&reorder->lock);
>
> pd->processed++;
>
> goto out;
> }
> out:
> return padata;
>
> I strongly suspect that the problem here is that two threads can race
> on reorder list. Even though the deletion is locked, call to
> list_entry is not locked, which means it's feasible that two threads
> pick up the same padata object and subsequently call list_add_tail on
> them at the same time. The fix is thus be hoist that lock outside of
> that block.
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Patch applied. 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
* Re: [PATCH] crypto: ixp4xx - Use sg_virt()
From: Herbert Xu @ 2017-03-24 14:17 UTC (permalink / raw)
To: Geliang Tang; +Cc: David S. Miller, linux-crypto, linux-kernel
In-Reply-To: <a41fae798473b657f09859ae113403655b1717f9.1490258141.git.geliangtang@gmail.com>
On Thu, Mar 23, 2017 at 09:16:30PM +0800, Geliang Tang wrote:
> Use sg_virt() instead of open-coding it.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
Patch applied. 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
* Re: [PATCH] crypto: ccp - Make some CCP DMA channels private
From: Herbert Xu @ 2017-03-24 14:18 UTC (permalink / raw)
To: Gary R Hook; +Cc: linux-crypto, thomas.lendacky, davem
In-Reply-To: <20170323175330.8742.62747.stgit@taos>
On Thu, Mar 23, 2017 at 12:53:30PM -0500, Gary R Hook wrote:
> The CCP registers its queues as channels capable of handling
> general DMA operations. The NTB driver will use DMA if
> directed, but as public channels can be reserved for use in
> asynchronous operations some channels should be held back
> as private. Since the public/private determination is
> handled at a device level, reserve the "other" (secondary)
> CCP channels as private.
>
> Add a module parameter that allows for override, to be
> applied to all channels on all devices.
>
> CC: <stable@vger.kernel.org> # 4.10.x-
> Signed-off-by: Gary R Hook <gary.hook@amd.com>
Patch applied. 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
* Re: [PATCH v2] dt-bindings: rng: clocks property on omap_rng not always mandatory
From: Rob Herring @ 2017-03-24 14:19 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Ian Campbell, Pawel Moll,
Mark Rutland, Kumar Gala, Herbert Xu,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
romain.perier-ZGY8ohtN/8qB+jHODAdFcQ, Nadav Haklai, Hanna Hawa,
Yehuda Yitschak, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, stable-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170324135200.j7m3zzgphbetqn7p@rob-hp-laptop>
On Fri, Mar 24, 2017 at 08:52:00AM -0500, Rob Herring wrote:
> On Fri, Mar 17, 2017 at 01:58:39PM +0100, Thomas Petazzoni wrote:
> > Commit 52060836f79 ("dt-bindings: omap-rng: Document SafeXcel IP-76
> > device variant") update the omap_rng Device Tree binding to add support
> > for the IP-76 variation of the IP. As part of this change, a "clocks"
> > property was added, but is indicated as "Required", without indicated
> > it's actually only required for some compatible strings.
> >
> > This commit fixes that, by explicitly stating that the clocks property
> > is only required with the inside-secure,safexcel-eip76 compatible
> > string.
> >
> > Fixes: 52060836f79 ("dt-bindings: omap-rng: Document SafeXcel IP-76 device variant")
> > Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > ---
> > Changes since v1:
> > - Instead of indicating the property as optional, indicate it as
> > mandatory for the inside-secure,safexcel-eip76 compatible string, as
> > suggested by Rob Herring.
> > ---
> > Documentation/devicetree/bindings/rng/omap_rng.txt | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
And applied.
Rob
--
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
* Re: [PATCH v2] dt-bindings: rng: clocks property on omap_rng not always mandatory
From: Rob Herring @ 2017-03-24 14:20 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: devicetree, Ian Campbell, Pawel Moll, Mark Rutland, Kumar Gala,
Herbert Xu, linux-crypto, romain.perier, Nadav Haklai, Hanna Hawa,
Yehuda Yitschak, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, stable
In-Reply-To: <20170324141905.scn7vrzrcrmzplk5@rob-hp-laptop>
On Fri, Mar 24, 2017 at 09:19:05AM -0500, Rob Herring wrote:
> On Fri, Mar 24, 2017 at 08:52:00AM -0500, Rob Herring wrote:
> > On Fri, Mar 17, 2017 at 01:58:39PM +0100, Thomas Petazzoni wrote:
> > > Commit 52060836f79 ("dt-bindings: omap-rng: Document SafeXcel IP-76
> > > device variant") update the omap_rng Device Tree binding to add support
> > > for the IP-76 variation of the IP. As part of this change, a "clocks"
> > > property was added, but is indicated as "Required", without indicated
> > > it's actually only required for some compatible strings.
> > >
> > > This commit fixes that, by explicitly stating that the clocks property
> > > is only required with the inside-secure,safexcel-eip76 compatible
> > > string.
> > >
> > > Fixes: 52060836f79 ("dt-bindings: omap-rng: Document SafeXcel IP-76 device variant")
> > > Cc: <stable@vger.kernel.org>
> > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > > ---
> > > Changes since v1:
> > > - Instead of indicating the property as optional, indicate it as
> > > mandatory for the inside-secure,safexcel-eip76 compatible string, as
> > > suggested by Rob Herring.
> > > ---
> > > Documentation/devicetree/bindings/rng/omap_rng.txt | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Acked-by: Rob Herring <robh@kernel.org>
>
> And applied.
Ah, I see Herbert applied it, so I've dropped it.
Rob
^ permalink raw reply
* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Dmitry Vyukov @ 2017-03-24 14:22 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Borislav Petkov, Alexander Potapenko, Michael Davidson,
Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke,
x86@kernel.org, open list:KERNEL BUILD + fi..., LKML,
linux-crypto, linux-raid, kbuild-all, Fengguang Wu
In-Reply-To: <20170324141053.lte3qq7mfl6krlkb@hirez.programming.kicks-ass.net>
On Fri, Mar 24, 2017 at 3:10 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Mar 24, 2017 at 02:50:24PM +0100, Dmitry Vyukov wrote:
>> OK, I guess should not have referenced the llvm-linux page.
>> So here are reasons on our side that I am ready to vouch:
>>
>> - clang make it possible to implement KMSAN (dynamic detection of
>> uses of uninit memory)
>
> How does GCC make this impossible?
Too complex and too difficult to implement correctly on all corner
cases. All other sanitizers were ported to gcc very quickly, but msan
wasn't. Nobody is brave enough to even approach it.
>> - better code coverage for fuzzing
>
> How so? Why can't the same be achieved using GCC?
Same reason.
>> - why simpler and faster development (e.g. we can port our user-space
>> hardening technologies -- CFI and SafeStack)
>
> That's just because you've already implemented this in clang, right? So
> less work for you. Not because its impossible.
I am not saying that it's impossible. It would just require
unreasonable amount of time, and then perpetual maintenance to fix
corner cases and regressions.
For background: I implemented the current fuzzing coverage (KCOV) in
gcc, and user-space tsan instrumentation in gcc.
^ permalink raw reply
* [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Krzysztof Kozlowski @ 2017-03-24 14:24 UTC (permalink / raw)
To: Kukjin Kim, Javier Martinez Canillas, Matt Mackall, Herbert Xu,
David S. Miller, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-crypto
Cc: Arnd Bergmann, Olof Johansson, Krzysztof Kozlowski
In-Reply-To: <20170324142446.31129-1-krzk@kernel.org>
Replace existing hw_ranndom/exynos-rng driver with a new, reworked one.
This is a driver for pseudo random number generator block which on
Exynos4 chipsets must be seeded with some value. On newer Exynos5420
chipsets it might seed itself from true random number generator block
but this is not implemented yet.
New driver is a complete rework to use the crypto ALGAPI instead of
hw_random API. Rationale for the change:
1. hw_random interface is for true RNG devices.
2. The old driver was seeding itself with jiffies which is not a
reliable source for randomness.
3. Device generates five random numbers in each pass but old driver was
returning only one thus its performance was reduced.
Compatibility with DeviceTree bindings is preserved.
New driver does not use runtime power management but manually enables
and disables the clock when needed. This is preferred approach because
using runtime PM just to toggle clock is huge overhead. Another
difference is using the same seed after resuming from system suspend
(previously driver was re-seeding itself again with jiffies).
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
MAINTAINERS | 8 +
drivers/char/hw_random/Kconfig | 14 --
drivers/char/hw_random/Makefile | 1 -
drivers/char/hw_random/exynos-rng.c | 231 ---------------------------
drivers/crypto/Kconfig | 15 ++
drivers/crypto/Makefile | 1 +
drivers/crypto/exynos-rng.c | 306 ++++++++++++++++++++++++++++++++++++
7 files changed, 330 insertions(+), 246 deletions(-)
delete mode 100644 drivers/char/hw_random/exynos-rng.c
create mode 100644 drivers/crypto/exynos-rng.c
diff --git a/MAINTAINERS b/MAINTAINERS
index affecc6d59f4..371fda859d43 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10977,6 +10977,14 @@ L: alsa-devel@alsa-project.org (moderated for non-subscribers)
S: Supported
F: sound/soc/samsung/
+SAMSUNG EXYNOS PSEUDO RANDOM NUMBER GENERATOR (RNG) DRIVER
+M: Krzysztof Kozlowski <krzk@kernel.org>
+L: linux-crypto@vger.kernel.org
+L: linux-samsung-soc@vger.kernel.org
+S: Maintained
+F: drivers/crypto/exynos-rng.c
+F: Documentation/devicetree/bindings/rng/samsung,exynos-rng4.txt
+
SAMSUNG FRAMEBUFFER DRIVER
M: Jingoo Han <jingoohan1@gmail.com>
L: linux-fbdev@vger.kernel.org
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 0cafe08919c9..bdae802e7154 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -294,20 +294,6 @@ config HW_RANDOM_POWERNV
If unsure, say Y.
-config HW_RANDOM_EXYNOS
- tristate "EXYNOS HW random number generator support"
- depends on ARCH_EXYNOS || COMPILE_TEST
- depends on HAS_IOMEM
- default HW_RANDOM
- ---help---
- This driver provides kernel-side support for the Random Number
- Generator hardware found on EXYNOS SOCs.
-
- To compile this driver as a module, choose M here: the
- module will be called exynos-rng.
-
- If unsure, say Y.
-
config HW_RANDOM_TPM
tristate "TPM HW Random Number Generator support"
depends on TCG_TPM
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 5f52b1e4e7be..6f1eecc2045c 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -24,7 +24,6 @@ obj-$(CONFIG_HW_RANDOM_OCTEON) += octeon-rng.o
obj-$(CONFIG_HW_RANDOM_NOMADIK) += nomadik-rng.o
obj-$(CONFIG_HW_RANDOM_PSERIES) += pseries-rng.o
obj-$(CONFIG_HW_RANDOM_POWERNV) += powernv-rng.o
-obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-rng.o
obj-$(CONFIG_HW_RANDOM_HISI) += hisi-rng.o
obj-$(CONFIG_HW_RANDOM_TPM) += tpm-rng.o
obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o
diff --git a/drivers/char/hw_random/exynos-rng.c b/drivers/char/hw_random/exynos-rng.c
deleted file mode 100644
index 23d358553b21..000000000000
--- a/drivers/char/hw_random/exynos-rng.c
+++ /dev/null
@@ -1,231 +0,0 @@
-/*
- * exynos-rng.c - Random Number Generator driver for the exynos
- *
- * Copyright (C) 2012 Samsung Electronics
- * Jonghwa Lee <jonghwa3.lee@samsung.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;
- *
- * 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.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
- */
-
-#include <linux/hw_random.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/io.h>
-#include <linux/platform_device.h>
-#include <linux/clk.h>
-#include <linux/pm_runtime.h>
-#include <linux/err.h>
-
-#define EXYNOS_PRNG_STATUS_OFFSET 0x10
-#define EXYNOS_PRNG_SEED_OFFSET 0x140
-#define EXYNOS_PRNG_OUT1_OFFSET 0x160
-#define SEED_SETTING_DONE BIT(1)
-#define PRNG_START 0x18
-#define PRNG_DONE BIT(5)
-#define EXYNOS_AUTOSUSPEND_DELAY 100
-
-struct exynos_rng {
- struct device *dev;
- struct hwrng rng;
- void __iomem *mem;
- struct clk *clk;
-};
-
-static u32 exynos_rng_readl(struct exynos_rng *rng, u32 offset)
-{
- return readl_relaxed(rng->mem + offset);
-}
-
-static void exynos_rng_writel(struct exynos_rng *rng, u32 val, u32 offset)
-{
- writel_relaxed(val, rng->mem + offset);
-}
-
-static int exynos_rng_configure(struct exynos_rng *exynos_rng)
-{
- int i;
- int ret = 0;
-
- for (i = 0 ; i < 5 ; i++)
- exynos_rng_writel(exynos_rng, jiffies,
- EXYNOS_PRNG_SEED_OFFSET + 4*i);
-
- if (!(exynos_rng_readl(exynos_rng, EXYNOS_PRNG_STATUS_OFFSET)
- & SEED_SETTING_DONE))
- ret = -EIO;
-
- return ret;
-}
-
-static int exynos_init(struct hwrng *rng)
-{
- struct exynos_rng *exynos_rng = container_of(rng,
- struct exynos_rng, rng);
- int ret = 0;
-
- pm_runtime_get_sync(exynos_rng->dev);
- ret = exynos_rng_configure(exynos_rng);
- pm_runtime_mark_last_busy(exynos_rng->dev);
- pm_runtime_put_autosuspend(exynos_rng->dev);
-
- return ret;
-}
-
-static int exynos_read(struct hwrng *rng, void *buf,
- size_t max, bool wait)
-{
- struct exynos_rng *exynos_rng = container_of(rng,
- struct exynos_rng, rng);
- u32 *data = buf;
- int retry = 100;
- int ret = 4;
-
- pm_runtime_get_sync(exynos_rng->dev);
-
- exynos_rng_writel(exynos_rng, PRNG_START, 0);
-
- while (!(exynos_rng_readl(exynos_rng,
- EXYNOS_PRNG_STATUS_OFFSET) & PRNG_DONE) && --retry)
- cpu_relax();
- if (!retry) {
- ret = -ETIMEDOUT;
- goto out;
- }
-
- exynos_rng_writel(exynos_rng, PRNG_DONE, EXYNOS_PRNG_STATUS_OFFSET);
-
- *data = exynos_rng_readl(exynos_rng, EXYNOS_PRNG_OUT1_OFFSET);
-
-out:
- pm_runtime_mark_last_busy(exynos_rng->dev);
- pm_runtime_put_sync_autosuspend(exynos_rng->dev);
-
- return ret;
-}
-
-static int exynos_rng_probe(struct platform_device *pdev)
-{
- struct exynos_rng *exynos_rng;
- struct resource *res;
- int ret;
-
- exynos_rng = devm_kzalloc(&pdev->dev, sizeof(struct exynos_rng),
- GFP_KERNEL);
- if (!exynos_rng)
- return -ENOMEM;
-
- exynos_rng->dev = &pdev->dev;
- exynos_rng->rng.name = "exynos";
- exynos_rng->rng.init = exynos_init;
- exynos_rng->rng.read = exynos_read;
- exynos_rng->clk = devm_clk_get(&pdev->dev, "secss");
- if (IS_ERR(exynos_rng->clk)) {
- dev_err(&pdev->dev, "Couldn't get clock.\n");
- return -ENOENT;
- }
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- exynos_rng->mem = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(exynos_rng->mem))
- return PTR_ERR(exynos_rng->mem);
-
- platform_set_drvdata(pdev, exynos_rng);
-
- pm_runtime_set_autosuspend_delay(&pdev->dev, EXYNOS_AUTOSUSPEND_DELAY);
- pm_runtime_use_autosuspend(&pdev->dev);
- pm_runtime_enable(&pdev->dev);
-
- ret = devm_hwrng_register(&pdev->dev, &exynos_rng->rng);
- if (ret) {
- pm_runtime_dont_use_autosuspend(&pdev->dev);
- pm_runtime_disable(&pdev->dev);
- }
-
- return ret;
-}
-
-static int exynos_rng_remove(struct platform_device *pdev)
-{
- pm_runtime_dont_use_autosuspend(&pdev->dev);
- pm_runtime_disable(&pdev->dev);
-
- return 0;
-}
-
-static int __maybe_unused exynos_rng_runtime_suspend(struct device *dev)
-{
- struct platform_device *pdev = to_platform_device(dev);
- struct exynos_rng *exynos_rng = platform_get_drvdata(pdev);
-
- clk_disable_unprepare(exynos_rng->clk);
-
- return 0;
-}
-
-static int __maybe_unused exynos_rng_runtime_resume(struct device *dev)
-{
- struct platform_device *pdev = to_platform_device(dev);
- struct exynos_rng *exynos_rng = platform_get_drvdata(pdev);
-
- return clk_prepare_enable(exynos_rng->clk);
-}
-
-static int __maybe_unused exynos_rng_suspend(struct device *dev)
-{
- return pm_runtime_force_suspend(dev);
-}
-
-static int __maybe_unused exynos_rng_resume(struct device *dev)
-{
- struct platform_device *pdev = to_platform_device(dev);
- struct exynos_rng *exynos_rng = platform_get_drvdata(pdev);
- int ret;
-
- ret = pm_runtime_force_resume(dev);
- if (ret)
- return ret;
-
- return exynos_rng_configure(exynos_rng);
-}
-
-static const struct dev_pm_ops exynos_rng_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(exynos_rng_suspend, exynos_rng_resume)
- SET_RUNTIME_PM_OPS(exynos_rng_runtime_suspend,
- exynos_rng_runtime_resume, NULL)
-};
-
-static const struct of_device_id exynos_rng_dt_match[] = {
- {
- .compatible = "samsung,exynos4-rng",
- },
- { },
-};
-MODULE_DEVICE_TABLE(of, exynos_rng_dt_match);
-
-static struct platform_driver exynos_rng_driver = {
- .driver = {
- .name = "exynos-rng",
- .pm = &exynos_rng_pm_ops,
- .of_match_table = exynos_rng_dt_match,
- },
- .probe = exynos_rng_probe,
- .remove = exynos_rng_remove,
-};
-
-module_platform_driver(exynos_rng_driver);
-
-MODULE_DESCRIPTION("EXYNOS 4 H/W Random Number Generator driver");
-MODULE_AUTHOR("Jonghwa Lee <jonghwa3.lee@samsung.com>");
-MODULE_LICENSE("GPL");
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 1a60626937e4..7d1fdf6a751c 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -388,6 +388,21 @@ config CRYPTO_DEV_MXC_SCC
This option enables support for the Security Controller (SCC)
found in Freescale i.MX25 chips.
+config CRYPTO_DEV_EXYNOS_RNG
+ tristate "EXYNOS HW pseudo random number generator support"
+ depends on ARCH_EXYNOS || COMPILE_TEST
+ depends on HAS_IOMEM
+ select CRYPTO_RNG
+ ---help---
+ This driver provides kernel-side support through the
+ cryptographic API for the pseudo random number generator hardware
+ found on Exynos SoCs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called exynos-rng.
+
+ If unsure, say Y.
+
config CRYPTO_DEV_S5P
tristate "Support for Samsung S5PV210/Exynos crypto accelerator"
depends on ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index 41ca339e89d3..9603f1862b30 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_CRYPTO_DEV_CAVIUM_ZIP) += cavium/
obj-$(CONFIG_CRYPTO_DEV_CCP) += ccp/
obj-$(CONFIG_CRYPTO_DEV_CHELSIO) += chelsio/
obj-$(CONFIG_CRYPTO_DEV_CPT) += cavium/cpt/
+obj-$(CONFIG_CRYPTO_DEV_EXYNOS_RNG) += exynos-rng.o
obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM) += caam/
obj-$(CONFIG_CRYPTO_DEV_GEODE) += geode-aes.o
obj-$(CONFIG_CRYPTO_DEV_HIFN_795X) += hifn_795x.o
diff --git a/drivers/crypto/exynos-rng.c b/drivers/crypto/exynos-rng.c
new file mode 100644
index 000000000000..9993f0eb3425
--- /dev/null
+++ b/drivers/crypto/exynos-rng.c
@@ -0,0 +1,306 @@
+/*
+ * exynos-rng.c - Random Number Generator driver for the Exynos
+ *
+ * Copyright (c) 2017 Krzysztof Kozlowski <krzk@kernel.org>
+ *
+ * Loosely based on old driver from drivers/char/hw_random/exynos-rng.c:
+ * Copyright (C) 2012 Samsung Electronics
+ * Jonghwa Lee <jonghwa3.lee@samsung.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;
+ *
+ * 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.
+ */
+
+#include <linux/clk.h>
+#include <linux/crypto.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include <crypto/internal/rng.h>
+
+#define EXYNOS_RNG_CONTROL 0x0
+#define EXYNOS_RNG_STATUS 0x10
+#define EXYNOS_RNG_SEED_BASE 0x140
+#define EXYNOS_RNG_SEED(n) (EXYNOS_RNG_SEED_BASE + (n * 0x4))
+#define EXYNOS_RNG_OUT_BASE 0x160
+#define EXYNOS_RNG_OUT(n) (EXYNOS_RNG_OUT_BASE + (n * 0x4))
+
+/* EXYNOS_RNG_CONTROL bit fields */
+#define EXYNOS_RNG_CONTROL_START 0x18
+/* EXYNOS_RNG_STATUS bit fields */
+#define EXYNOS_RNG_STATUS_SEED_SETTING_DONE BIT(1)
+#define EXYNOS_RNG_STATUS_RNG_DONE BIT(5)
+
+/* Five seed and output registers, each 4 bytes */
+#define EXYNOS_RNG_SEED_REGS 5
+#define EXYNOS_RNG_SEED_SIZE (EXYNOS_RNG_SEED_REGS * 4)
+
+/* Context for crypto */
+struct exynos_rng_ctx {
+ struct exynos_rng_dev *rng;
+};
+
+/* Device associated memory */
+struct exynos_rng_dev {
+ struct device *dev;
+ struct exynos_rng_ctx *ctx;
+ void __iomem *mem;
+ struct clk *clk;
+ u8 seed_save[EXYNOS_RNG_SEED_SIZE];
+};
+
+static struct exynos_rng_dev *exynos_rng_dev;
+
+static u32 exynos_rng_readl(struct exynos_rng_dev *rng, u32 offset)
+{
+ return readl_relaxed(rng->mem + offset);
+}
+
+static void exynos_rng_writel(struct exynos_rng_dev *rng, u32 val, u32 offset)
+{
+ writel_relaxed(val, rng->mem + offset);
+}
+
+static unsigned int exynos_rng_copy_random(struct exynos_rng_dev *rng,
+ u8 *dst, unsigned int dlen)
+{
+ unsigned int cnt = 0;
+ int i, j;
+ u32 val;
+
+ for (j = 0; j < EXYNOS_RNG_SEED_REGS; j++) {
+ val = exynos_rng_readl(rng, EXYNOS_RNG_OUT(j));
+
+ for (i = 0; i < 4; i++) {
+ dst[cnt] = val & 0xff;
+ val >>= 8;
+ if (++cnt >= dlen)
+ return cnt;
+ }
+ }
+
+ return cnt;
+}
+
+static int exynos_rng_get_random(struct exynos_rng_dev *rng,
+ u8 *dst, unsigned int dlen,
+ unsigned int *read)
+{
+ int retry = 100;
+
+ exynos_rng_writel(rng, EXYNOS_RNG_CONTROL_START,
+ EXYNOS_RNG_CONTROL);
+
+ while (!(exynos_rng_readl(rng,
+ EXYNOS_RNG_STATUS) & EXYNOS_RNG_STATUS_RNG_DONE) && --retry)
+ cpu_relax();
+
+ if (!retry)
+ return -ETIMEDOUT;
+
+ /* Clear status bit */
+ exynos_rng_writel(rng, EXYNOS_RNG_STATUS_RNG_DONE,
+ EXYNOS_RNG_STATUS);
+
+ *read = exynos_rng_copy_random(rng, dst, dlen);
+
+ return 0;
+}
+
+static int exynos_rng_generate(struct crypto_rng *tfm,
+ const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int dlen)
+{
+ struct exynos_rng_ctx *ctx = crypto_rng_ctx(tfm);
+ struct exynos_rng_dev *rng = ctx->rng;
+ unsigned int read = 0;
+ int ret;
+
+ ret = clk_prepare_enable(rng->clk);
+ if (ret)
+ return ret;
+
+ do {
+ ret = exynos_rng_get_random(rng, dst, dlen, &read);
+ if (ret)
+ break;
+
+ dlen -= read;
+ dst += read;
+ } while (dlen > 0);
+
+ clk_disable_unprepare(rng->clk);
+
+ return ret;
+}
+
+static int exynos_rng_set_seed(struct exynos_rng_dev *rng,
+ const u8 *seed, unsigned int slen)
+{
+ int ret, i;
+ u32 val;
+
+ dev_dbg(rng->dev, "Seeding with %u bytes\n", slen);
+
+ ret = clk_prepare_enable(rng->clk);
+ if (ret)
+ return ret;
+
+ if (slen < EXYNOS_RNG_SEED_SIZE) {
+ dev_warn(rng->dev, "Seed too short (only %u bytes)\n", slen);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ for (i = 0 ; i < EXYNOS_RNG_SEED_REGS ; i++) {
+ val = seed[i * 4] << 24;
+ val |= seed[i * 4 + 1] << 16;
+ val |= seed[i * 4 + 2] << 8;
+ val |= seed[i * 4 + 3] << 0;
+
+ exynos_rng_writel(rng, val, EXYNOS_RNG_SEED(i));
+ }
+
+ val = exynos_rng_readl(rng, EXYNOS_RNG_STATUS);
+ if (!(val & EXYNOS_RNG_STATUS_SEED_SETTING_DONE)) {
+ dev_warn(rng->dev, "Seed setting not finished\n");
+ ret = -EIO;
+ goto out;
+ }
+
+ ret = 0;
+ /* Save seed for suspend */
+ memcpy(rng->seed_save, seed, slen);
+
+out:
+ clk_disable_unprepare(rng->clk);
+
+ return ret;
+}
+
+static int exynos_rng_seed(struct crypto_rng *tfm, const u8 *seed,
+ unsigned int slen)
+{
+ struct exynos_rng_ctx *ctx = crypto_rng_ctx(tfm);
+
+ return exynos_rng_set_seed(ctx->rng, seed, slen);
+}
+
+static int exynos_rng_kcapi_init(struct crypto_tfm *tfm)
+{
+ struct exynos_rng_ctx *ctx = crypto_tfm_ctx(tfm);
+
+ ctx->rng = exynos_rng_dev;
+
+ return 0;
+}
+
+static struct rng_alg exynos_rng_alg = {
+ .generate = exynos_rng_generate,
+ .seed = exynos_rng_seed,
+ .seedsize = EXYNOS_RNG_SEED_SIZE,
+ .base = {
+ .cra_name = "exynos_rng",
+ .cra_driver_name = "exynos_rng",
+ .cra_priority = 100,
+ .cra_ctxsize = sizeof(struct exynos_rng_ctx),
+ .cra_module = THIS_MODULE,
+ .cra_init = exynos_rng_kcapi_init,
+ }
+};
+
+static int exynos_rng_probe(struct platform_device *pdev)
+{
+ struct exynos_rng_dev *rng;
+ struct resource *res;
+ int ret;
+
+ if (exynos_rng_dev)
+ return -EEXIST;
+
+ rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
+ if (!rng)
+ return -ENOMEM;
+
+ rng->dev = &pdev->dev;
+ rng->clk = devm_clk_get(&pdev->dev, "secss");
+ if (IS_ERR(rng->clk)) {
+ dev_err(&pdev->dev, "Couldn't get clock.\n");
+ return PTR_ERR(rng->clk);
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ rng->mem = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(rng->mem))
+ return PTR_ERR(rng->mem);
+
+ platform_set_drvdata(pdev, rng);
+
+ exynos_rng_dev = rng;
+
+ ret = crypto_register_rng(&exynos_rng_alg);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Couldn't register rng crypto alg: %d\n", ret);
+ exynos_rng_dev = NULL;
+ }
+
+ return ret;
+}
+
+static int exynos_rng_remove(struct platform_device *pdev)
+{
+ crypto_unregister_rng(&exynos_rng_alg);
+
+ exynos_rng_dev = NULL;
+
+ return 0;
+}
+
+static int __maybe_unused exynos_rng_suspend(struct device *dev)
+{
+ return 0;
+}
+
+static int __maybe_unused exynos_rng_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct exynos_rng_dev *rng = platform_get_drvdata(pdev);
+
+ return exynos_rng_set_seed(rng, rng->seed_save, sizeof(rng->seed_save));
+}
+
+static SIMPLE_DEV_PM_OPS(exynos_rng_pm_ops, exynos_rng_suspend,
+ exynos_rng_resume);
+
+static const struct of_device_id exynos_rng_dt_match[] = {
+ {
+ .compatible = "samsung,exynos4-rng",
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, exynos_rng_dt_match);
+
+static struct platform_driver exynos_rng_driver = {
+ .driver = {
+ .name = "exynos-rng",
+ .pm = &exynos_rng_pm_ops,
+ .of_match_table = exynos_rng_dt_match,
+ },
+ .probe = exynos_rng_probe,
+ .remove = exynos_rng_remove,
+};
+
+module_platform_driver(exynos_rng_driver);
+
+MODULE_DESCRIPTION("Exynos H/W Random Number Generator driver");
+MODULE_AUTHOR("Krzysztof Kozlowski <krzk@kernel.org>");
+MODULE_LICENSE("GPL");
--
2.9.3
^ permalink raw reply related
* [PATCH 2/3] ARM: exynos_defconfig: Enable Exynos RNG and user-space crypto API
From: Krzysztof Kozlowski @ 2017-03-24 14:24 UTC (permalink / raw)
To: Kukjin Kim, Javier Martinez Canillas, Matt Mackall, Herbert Xu,
David S. Miller, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-crypto
Cc: Arnd Bergmann, Olof Johansson, Krzysztof Kozlowski
In-Reply-To: <20170324142446.31129-1-krzk@kernel.org>
Enable the new Exynos pseudo random number generator driver and
user-space API to access crypto algorithms and devices (not only RNG).
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
arch/arm/configs/exynos_defconfig | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
index 6dc661c4a2c1..960d55445e05 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -265,6 +265,12 @@ CONFIG_DEBUG_RT_MUTEXES=y
CONFIG_DEBUG_SPINLOCK=y
CONFIG_DEBUG_MUTEXES=y
CONFIG_DEBUG_USER=y
+CONFIG_CRYPTO_USER=m
+CONFIG_CRYPTO_USER_API_HASH=m
+CONFIG_CRYPTO_USER_API_SKCIPHER=m
+CONFIG_CRYPTO_USER_API_RNG=m
+CONFIG_CRYPTO_USER_API_AEAD=m
+CONFIG_CRYPTO_DEV_EXYNOS_RNG=y
CONFIG_CRYPTO_DEV_S5P=y
CONFIG_ARM_CRYPTO=y
CONFIG_CRYPTO_SHA1_ARM_NEON=m
--
2.9.3
^ permalink raw reply related
* [PATCH 3/3] ARM: multi_v7_defconfig: Enable Exynos RNG and user-space crypto API
From: Krzysztof Kozlowski @ 2017-03-24 14:24 UTC (permalink / raw)
To: Kukjin Kim, Javier Martinez Canillas, Matt Mackall, Herbert Xu,
David S. Miller, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-crypto
Cc: Arnd Bergmann, Olof Johansson, Krzysztof Kozlowski
In-Reply-To: <20170324142446.31129-1-krzk@kernel.org>
Enable the new Exynos pseudo random number generator driver and
user-space API to access crypto algorithms and devices (not only RNG).
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
arch/arm/configs/multi_v7_defconfig | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 36c1b39cfb54..9bc1bd7f2afa 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -935,7 +935,13 @@ CONFIG_CPUFREQ_DT=y
CONFIG_KEYSTONE_IRQ=y
CONFIG_HW_RANDOM=y
CONFIG_HW_RANDOM_ST=y
+CONFIG_CRYPTO_USER=m
+CONFIG_CRYPTO_USER_API_HASH=m
+CONFIG_CRYPTO_USER_API_SKCIPHER=m
+CONFIG_CRYPTO_USER_API_RNG=m
+CONFIG_CRYPTO_USER_API_AEAD=m
CONFIG_CRYPTO_DEV_MARVELL_CESA=m
+CONFIG_CRYPTO_DEV_EXYNOS_RNG=m
CONFIG_CRYPTO_DEV_S5P=m
CONFIG_CRYPTO_DEV_SUN4I_SS=m
CONFIG_CRYPTO_DEV_ROCKCHIP=m
--
2.9.3
^ permalink raw reply related
* [PATCH 0/3] crypto: hw_random - Add new Exynos RNG driver
From: Krzysztof Kozlowski @ 2017-03-24 14:24 UTC (permalink / raw)
To: Kukjin Kim, Javier Martinez Canillas, Matt Mackall, Herbert Xu,
David S. Miller, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-crypto
Cc: Arnd Bergmann, Olof Johansson, Krzysztof Kozlowski
Hi,
This is a follow up of my questions around exynos-rng [1].
The existing exynos-rng has many issues. The most important one is that
it is a pseudo RNG device but uses hw_random interface which does not allow
proper seeding.
The RNG module on Exynos4 requires seeding. On newer SoCs (like Exynos5420)
it can seed itself from a true RNG. Converting the existing driver
to use TRNG would effectively drop support for Exynos4 and break
compatibility with existing users.
Instead I decided to convert it to crypto API. In the future I hope
to add support for seeding from TRNG module.
Tested with app [2].
Patches are independent. I will take the defconfig changes (2/3 and 3/3)
through samsung-soc tree.
Best regards,
Krzysztof
[1] https://www.spinics.net/lists/arm-kernel/msg569641.html
[2] https://www.spinics.net/lists/arm-kernel/msg571184.html
Krzysztof Kozlowski (3):
crypto: hw_random - Add new Exynos RNG driver
ARM: exynos_defconfig: Enable Exynos RNG and user-space crypto API
ARM: multi_v7_defconfig: Enable Exynos RNG and user-space crypto API
MAINTAINERS | 8 +
arch/arm/configs/exynos_defconfig | 6 +
arch/arm/configs/multi_v7_defconfig | 6 +
drivers/char/hw_random/Kconfig | 14 --
drivers/char/hw_random/Makefile | 1 -
drivers/char/hw_random/exynos-rng.c | 231 ---------------------------
drivers/crypto/Kconfig | 15 ++
drivers/crypto/Makefile | 1 +
drivers/crypto/exynos-rng.c | 306 ++++++++++++++++++++++++++++++++++++
9 files changed, 342 insertions(+), 246 deletions(-)
delete mode 100644 drivers/char/hw_random/exynos-rng.c
create mode 100644 drivers/crypto/exynos-rng.c
--
2.9.3
^ permalink raw reply
* Re: next build: 208 builds: 9 failed, 199 passed, 857 errors, 444 warnings (next-20170323)
From: Ralf Baechle @ 2017-03-24 14:32 UTC (permalink / raw)
To: Arnd Bergmann
Cc: kernelci.org bot, kernel-build-reports, Linux Kernel Mailing List,
linux-crypto, linux-mips, Steven J. Hill
In-Reply-To: <20170323184450.GB13254@linux-mips.org>
On Thu, Mar 23, 2017 at 07:44:50PM +0100, Ralf Baechle wrote:
> > On Thu, Mar 23, 2017 at 6:46 AM, kernelci.org bot <bot@kernelci.org> wrote:
> >
> > > acs5k_defconfig (arm) — PASS, 0 errors, 2 warnings, 0 section mismatches
> > >
> > > Warnings:
> > > :1328:2: warning: #warning syscall arch_prctl not implemented [-Wcpp]
> > > :1328:2: warning: #warning syscall arch_prctl not implemented [-Wcpp]
> >
> > patch sent today, we don't really want this syscall except on x86
> >
> > > allmodconfig (arm64) — FAIL, 6 errors, 5 warnings, 0 section mismatches
> > >
> > > Errors:
> > > ERROR (phandle_references): Reference to non-existent node or label "pwm_f_clk_pins"
> > > ERROR (phandle_references): Reference to non-existent node or label "pwm_ao_a_3_pins"
> > > ERROR: Input tree has errors, aborting (use -f to force output)
> > > ERROR (phandle_references): Reference to non-existent node or label "pwm_f_clk_pins"
> > > ERROR (phandle_references): Reference to non-existent node or label "pwm_ao_a_3_pins"
> > > ERROR: Input tree has errors, aborting (use -f to force output)
> >
> > Kevin has already backed out the commit that caused this.
> >
> > > Warnings:
> > > :1325:2: warning: #warning syscall statx not implemented [-Wcpp]
> >
> > Should get fixed in the next few days when the patch gets picked up for arm64.
> >
> > > drivers/misc/aspeed-lpc-ctrl.c:232:17: warning: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'resource_size_t {aka long long unsigned int}' [-Wformat=]
> >
> > patch sent today
> >
> > > include/uapi/linux/byteorder/big_endian.h:32:26: warning: large integer implicitly truncated to unsigned type [-Woverflow]
> > > include/uapi/linux/byteorder/big_endian.h:32:26: warning: large integer implicitly truncated to unsigned type [-Woverflow]
> >
> > I sent this one a few days ago
> >
> > > allmodconfig (x86) — PASS, 0 errors, 11 warnings, 0 section mismatches
> > >
> > > Warnings:
> > > drivers/crypto/cavium/zip/zip_main.c:489:18: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'long long int' [-Wformat=]
> > > drivers/crypto/cavium/zip/zip_main.c:489:18: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'long long int' [-Wformat=]
> >
> > This one too.
> >
> > > cavium_octeon_defconfig (mips) — FAIL, 830 errors, 3 warnings, 0 section mismatches
> > >
> > > Errors:
> > > arch/mips/include/asm/octeon/cvmx-mio-defs.h:78:3: error: expected specifier-qualifier-list before '__BITFIELD_FIELD'
> > > arch/mips/include/asm/octeon/cvmx-mio-defs.h:95:3: error: expected specifier-qualifier-list before '__BITFIELD_FIELD'
> >
> > Maybe caused by 4cd156de2ca8 ("MIPS: Octeon: Remove unused MIO types
> > and macros.")
> > I have not looked in detail
>
> Seems an #include <uapi/asm/bitfield.h.> is missing. I'm going to sort
> this one.
I fixed this in my branch for linux-next only to encounter another build
error so I dropped another two patches.
Ralf
^ permalink raw reply
* Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Stephan Müller @ 2017-03-24 14:37 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Kukjin Kim, Javier Martinez Canillas, Matt Mackall, Herbert Xu,
David S. Miller, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-crypto, Arnd Bergmann, Olof Johansson
In-Reply-To: <20170324142446.31129-2-krzk@kernel.org>
Am Freitag, 24. März 2017, 15:24:44 CET schrieb Krzysztof Kozlowski:
Hi Krzysztof,
> +
> +static int exynos_rng_set_seed(struct exynos_rng_dev *rng,
> + const u8 *seed, unsigned int slen)
> +{
> + int ret, i;
> + u32 val;
> +
> + dev_dbg(rng->dev, "Seeding with %u bytes\n", slen);
> +
> + ret = clk_prepare_enable(rng->clk);
> + if (ret)
> + return ret;
> +
> + if (slen < EXYNOS_RNG_SEED_SIZE) {
> + dev_warn(rng->dev, "Seed too short (only %u bytes)\n", slen);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + for (i = 0 ; i < EXYNOS_RNG_SEED_REGS ; i++) {
> + val = seed[i * 4] << 24;
> + val |= seed[i * 4 + 1] << 16;
> + val |= seed[i * 4 + 2] << 8;
> + val |= seed[i * 4 + 3] << 0;
> +
> + exynos_rng_writel(rng, val, EXYNOS_RNG_SEED(i));
> + }
> +
> + val = exynos_rng_readl(rng, EXYNOS_RNG_STATUS);
> + if (!(val & EXYNOS_RNG_STATUS_SEED_SETTING_DONE)) {
> + dev_warn(rng->dev, "Seed setting not finished\n");
> + ret = -EIO;
> + goto out;
> + }
> +
> + ret = 0;
> + /* Save seed for suspend */
> + memcpy(rng->seed_save, seed, slen);
Is this really necessary? If you need to save some seed, shouldn't that be an
output of the DRNG and not the real seed?
Besides, how do you know that slen is not larger than EXYNOS_RNG_SEED_SIZE?
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Krzysztof Kozlowski @ 2017-03-24 14:43 UTC (permalink / raw)
To: Stephan Müller
Cc: Kukjin Kim, Javier Martinez Canillas, Matt Mackall, Herbert Xu,
David S. Miller, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-crypto, Arnd Bergmann, Olof Johansson
In-Reply-To: <2503592.nlbQnVP8UR@positron.chronox.de>
On Fri, Mar 24, 2017 at 03:37:59PM +0100, Stephan Müller wrote:
> Am Freitag, 24. März 2017, 15:24:44 CET schrieb Krzysztof Kozlowski:
>
> Hi Krzysztof,
>
> > +
> > +static int exynos_rng_set_seed(struct exynos_rng_dev *rng,
> > + const u8 *seed, unsigned int slen)
> > +{
> > + int ret, i;
> > + u32 val;
> > +
> > + dev_dbg(rng->dev, "Seeding with %u bytes\n", slen);
> > +
> > + ret = clk_prepare_enable(rng->clk);
> > + if (ret)
> > + return ret;
> > +
> > + if (slen < EXYNOS_RNG_SEED_SIZE) {
> > + dev_warn(rng->dev, "Seed too short (only %u bytes)\n", slen);
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + for (i = 0 ; i < EXYNOS_RNG_SEED_REGS ; i++) {
> > + val = seed[i * 4] << 24;
> > + val |= seed[i * 4 + 1] << 16;
> > + val |= seed[i * 4 + 2] << 8;
> > + val |= seed[i * 4 + 3] << 0;
> > +
> > + exynos_rng_writel(rng, val, EXYNOS_RNG_SEED(i));
> > + }
> > +
> > + val = exynos_rng_readl(rng, EXYNOS_RNG_STATUS);
> > + if (!(val & EXYNOS_RNG_STATUS_SEED_SETTING_DONE)) {
> > + dev_warn(rng->dev, "Seed setting not finished\n");
> > + ret = -EIO;
> > + goto out;
> > + }
> > +
> > + ret = 0;
> > + /* Save seed for suspend */
> > + memcpy(rng->seed_save, seed, slen);
>
> Is this really necessary? If you need to save some seed, shouldn't that be an
> output of the DRNG and not the real seed?
When suspended to RAM device will loose the contents of registers
(including SEED registers) so it has to be initialized with something on
resume.
The seed registers are write-only so I cannot read them and store
contents just before suspend.
I understand that real seed should not be stored... but then if I am not
able to re-seed it with same values, I will loose the continuous and
reproducible pseudo-random generation after suspend. Aren't this
expected out of PRNG?
> Besides, how do you know that slen is not larger than EXYNOS_RNG_SEED_SIZE?
Right, there is a overflow here. It should be sizeof(rng->seed_save);
Thanks for review!
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Stephan Müller @ 2017-03-24 14:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-samsung-soc, Herbert Xu, Arnd Bergmann, linux-kernel,
Javier Martinez Canillas, Kukjin Kim, linux-crypto, Matt Mackall,
Olof Johansson, David S. Miller, linux-arm-kernel
In-Reply-To: <20170324144348.v4hegak7dffh5awh@kozik-lap>
Am Freitag, 24. März 2017, 15:43:48 CET schrieb Krzysztof Kozlowski:
Hi Krzysztof,
> On Fri, Mar 24, 2017 at 03:37:59PM +0100, Stephan Müller wrote:
> > Am Freitag, 24. März 2017, 15:24:44 CET schrieb Krzysztof Kozlowski:
> >
> > Hi Krzysztof,
> >
> > > +
> > > +static int exynos_rng_set_seed(struct exynos_rng_dev *rng,
> > > + const u8 *seed, unsigned int slen)
> > > +{
> > > + int ret, i;
> > > + u32 val;
> > > +
> > > + dev_dbg(rng->dev, "Seeding with %u bytes\n", slen);
> > > +
> > > + ret = clk_prepare_enable(rng->clk);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + if (slen < EXYNOS_RNG_SEED_SIZE) {
> > > + dev_warn(rng->dev, "Seed too short (only %u bytes)\n", slen);
> > > + ret = -EINVAL;
> > > + goto out;
> > > + }
> > > +
> > > + for (i = 0 ; i < EXYNOS_RNG_SEED_REGS ; i++) {
> > > + val = seed[i * 4] << 24;
> > > + val |= seed[i * 4 + 1] << 16;
> > > + val |= seed[i * 4 + 2] << 8;
> > > + val |= seed[i * 4 + 3] << 0;
> > > +
> > > + exynos_rng_writel(rng, val, EXYNOS_RNG_SEED(i));
> > > + }
> > > +
> > > + val = exynos_rng_readl(rng, EXYNOS_RNG_STATUS);
> > > + if (!(val & EXYNOS_RNG_STATUS_SEED_SETTING_DONE)) {
> > > + dev_warn(rng->dev, "Seed setting not finished\n");
> > > + ret = -EIO;
> > > + goto out;
> > > + }
> > > +
> > > + ret = 0;
> > > + /* Save seed for suspend */
> > > + memcpy(rng->seed_save, seed, slen);
> >
> > Is this really necessary? If you need to save some seed, shouldn't that be
> > an output of the DRNG and not the real seed?
>
> When suspended to RAM device will loose the contents of registers
> (including SEED registers) so it has to be initialized with something on
> resume.
>
> The seed registers are write-only so I cannot read them and store
> contents just before suspend.
>
> I understand that real seed should not be stored... but then if I am not
> able to re-seed it with same values, I will loose the continuous and
> reproducible pseudo-random generation after suspend. Aren't this
> expected out of PRNG?
An RNG has to be stateless from the perspective of the caller -- this is the
core implication of entropy.
Then, if you add the initial seed after the RNG lost its state implies that
the same sequence of random numbers starts again. I.e. where is the
randomness?
>
> > Besides, how do you know that slen is not larger than
> > EXYNOS_RNG_SEED_SIZE?
>
> Right, there is a overflow here. It should be sizeof(rng->seed_save);
shouldn't it be min(rng->seed_save, slen)?
>
> Thanks for review!
>
> Best regards,
> Krzysztof
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Krzysztof Kozlowski @ 2017-03-24 14:51 UTC (permalink / raw)
To: Stephan Müller
Cc: Kukjin Kim, Javier Martinez Canillas, Matt Mackall, Herbert Xu,
David S. Miller, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-crypto, Arnd Bergmann, Olof Johansson
In-Reply-To: <3628768.ze2UxnIp5I@tauon.atsec.com>
On Fri, Mar 24, 2017 at 03:46:44PM +0100, Stephan Müller wrote:
> Am Freitag, 24. März 2017, 15:43:48 CET schrieb Krzysztof Kozlowski:
>
> Hi Krzysztof,
>
> > On Fri, Mar 24, 2017 at 03:37:59PM +0100, Stephan Müller wrote:
> > > Am Freitag, 24. März 2017, 15:24:44 CET schrieb Krzysztof Kozlowski:
> > >
> > > Hi Krzysztof,
> > >
> > > > +
> > > > +static int exynos_rng_set_seed(struct exynos_rng_dev *rng,
> > > > + const u8 *seed, unsigned int slen)
> > > > +{
> > > > + int ret, i;
> > > > + u32 val;
> > > > +
> > > > + dev_dbg(rng->dev, "Seeding with %u bytes\n", slen);
> > > > +
> > > > + ret = clk_prepare_enable(rng->clk);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + if (slen < EXYNOS_RNG_SEED_SIZE) {
> > > > + dev_warn(rng->dev, "Seed too short (only %u bytes)\n", slen);
> > > > + ret = -EINVAL;
> > > > + goto out;
> > > > + }
> > > > +
> > > > + for (i = 0 ; i < EXYNOS_RNG_SEED_REGS ; i++) {
> > > > + val = seed[i * 4] << 24;
> > > > + val |= seed[i * 4 + 1] << 16;
> > > > + val |= seed[i * 4 + 2] << 8;
> > > > + val |= seed[i * 4 + 3] << 0;
> > > > +
> > > > + exynos_rng_writel(rng, val, EXYNOS_RNG_SEED(i));
> > > > + }
> > > > +
> > > > + val = exynos_rng_readl(rng, EXYNOS_RNG_STATUS);
> > > > + if (!(val & EXYNOS_RNG_STATUS_SEED_SETTING_DONE)) {
> > > > + dev_warn(rng->dev, "Seed setting not finished\n");
> > > > + ret = -EIO;
> > > > + goto out;
> > > > + }
> > > > +
> > > > + ret = 0;
> > > > + /* Save seed for suspend */
> > > > + memcpy(rng->seed_save, seed, slen);
> > >
> > > Is this really necessary? If you need to save some seed, shouldn't that be
> > > an output of the DRNG and not the real seed?
> >
> > When suspended to RAM device will loose the contents of registers
> > (including SEED registers) so it has to be initialized with something on
> > resume.
> >
> > The seed registers are write-only so I cannot read them and store
> > contents just before suspend.
> >
> > I understand that real seed should not be stored... but then if I am not
> > able to re-seed it with same values, I will loose the continuous and
> > reproducible pseudo-random generation after suspend. Aren't this
> > expected out of PRNG?
>
> An RNG has to be stateless from the perspective of the caller -- this is the
> core implication of entropy.
>
> Then, if you add the initial seed after the RNG lost its state implies that
> the same sequence of random numbers starts again. I.e. where is the
> randomness?
Ahhh, yes, you are right. The device would not continue on its random
generation because all state is lost anyway.
I'll use the generated random numbers.
> > > Besides, how do you know that slen is not larger than
> > > EXYNOS_RNG_SEED_SIZE?
> >
> > Right, there is a overflow here. It should be sizeof(rng->seed_save);
>
> shouldn't it be min(rng->seed_save, slen)?
Now it is irrelevant but anyway I am not allowing the seed to be less
then EXYNOS_RNG_SEED_SIZE (== sizeof(rng->seed_save)). The device
expects all five channels (registers) to be seeded.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Stephan Müller @ 2017-03-24 14:55 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Kukjin Kim, Javier Martinez Canillas, Matt Mackall, Herbert Xu,
David S. Miller, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-crypto, Arnd Bergmann, Olof Johansson
In-Reply-To: <20170324145156.miqmxgkhn2v7rbcu@kozik-lap>
Am Freitag, 24. März 2017, 15:51:56 CET schrieb Krzysztof Kozlowski:
Hi Krzysztof,
>
> I'll use the generated random numbers.
If you do that, may I propse that you update this seed field periodically?
E.g. with every (or every other) user request for random data, you may update
that field with new data.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Krzysztof Kozlowski @ 2017-03-24 14:58 UTC (permalink / raw)
To: Stephan Müller
Cc: Kukjin Kim, Javier Martinez Canillas, Matt Mackall, Herbert Xu,
David S. Miller, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-crypto, Arnd Bergmann, Olof Johansson
In-Reply-To: <2090327.CHzkLTTdtg@positron.chronox.de>
On Fri, Mar 24, 2017 at 03:55:22PM +0100, Stephan Müller wrote:
> Am Freitag, 24. März 2017, 15:51:56 CET schrieb Krzysztof Kozlowski:
>
> Hi Krzysztof,
> >
> > I'll use the generated random numbers.
>
> If you do that, may I propse that you update this seed field periodically?
> E.g. with every (or every other) user request for random data, you may update
> that field with new data.
Sounds reasonable. Thanks for feedback, it is much appreciated.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Bartlomiej Zolnierkiewicz @ 2017-03-24 15:26 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-arm-kernel, Kukjin Kim, Javier Martinez Canillas,
Matt Mackall, Herbert Xu, David S. Miller, linux-kernel,
linux-samsung-soc, linux-crypto, Olof Johansson, Arnd Bergmann
In-Reply-To: <20170324142446.31129-2-krzk@kernel.org>
Hi,
Firstly, thanks for working on this.
The patch looks fine overall for me, some review comments below.
On Friday, March 24, 2017 05:24:44 PM Krzysztof Kozlowski wrote:
> Replace existing hw_ranndom/exynos-rng driver with a new, reworked one.
> This is a driver for pseudo random number generator block which on
> Exynos4 chipsets must be seeded with some value. On newer Exynos5420
> chipsets it might seed itself from true random number generator block
> but this is not implemented yet.
>
> New driver is a complete rework to use the crypto ALGAPI instead of
> hw_random API. Rationale for the change:
> 1. hw_random interface is for true RNG devices.
> 2. The old driver was seeding itself with jiffies which is not a
> reliable source for randomness.
> 3. Device generates five random numbers in each pass but old driver was
> returning only one thus its performance was reduced.
>
> Compatibility with DeviceTree bindings is preserved.
>
> New driver does not use runtime power management but manually enables
> and disables the clock when needed. This is preferred approach because
> using runtime PM just to toggle clock is huge overhead. Another
I'm not entirely convinced that the new approach is better.
With the old approach exynos_rng_generate() can be called more
than once before PM autosuspend kicks in and thus clk_prepare_enable()/
clk_disable()_unprepare() operations will be done only once. This
would give better performance on the "burst" operations.
[ The above assumes that clock operations are more costly than
going through PM core to check the current device state. ]
> +static int exynos_rng_get_random(struct exynos_rng_dev *rng,
> + u8 *dst, unsigned int dlen,
> + unsigned int *read)
> +{
> + int retry = 100;
I know that this is copied verbatim from the old driver but please
use define for the maximum number of retries.
> +static int exynos_rng_probe(struct platform_device *pdev)
> +{
> + struct exynos_rng_dev *rng;
> + struct resource *res;
> + int ret;
> +
> + if (exynos_rng_dev)
> + return -EEXIST;
How this condition could ever happen?
The probe function will never be called twice.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Krzysztof Kozlowski @ 2017-03-24 15:46 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: linux-arm-kernel, Kukjin Kim, Javier Martinez Canillas,
Matt Mackall, Herbert Xu, David S. Miller, linux-kernel,
linux-samsung-soc, linux-crypto, Olof Johansson, Arnd Bergmann,
Marek Szyprowski
In-Reply-To: <3940941.n3qdgn1RbN@amdc3058>
On Fri, Mar 24, 2017 at 04:26:47PM +0100, Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
> Firstly, thanks for working on this.
>
> The patch looks fine overall for me, some review comments below.
>
> On Friday, March 24, 2017 05:24:44 PM Krzysztof Kozlowski wrote:
> > Replace existing hw_ranndom/exynos-rng driver with a new, reworked one.
> > This is a driver for pseudo random number generator block which on
> > Exynos4 chipsets must be seeded with some value. On newer Exynos5420
> > chipsets it might seed itself from true random number generator block
> > but this is not implemented yet.
> >
> > New driver is a complete rework to use the crypto ALGAPI instead of
> > hw_random API. Rationale for the change:
> > 1. hw_random interface is for true RNG devices.
> > 2. The old driver was seeding itself with jiffies which is not a
> > reliable source for randomness.
> > 3. Device generates five random numbers in each pass but old driver was
> > returning only one thus its performance was reduced.
> >
> > Compatibility with DeviceTree bindings is preserved.
> >
> > New driver does not use runtime power management but manually enables
> > and disables the clock when needed. This is preferred approach because
> > using runtime PM just to toggle clock is huge overhead. Another
>
> I'm not entirely convinced that the new approach is better.
>
> With the old approach exynos_rng_generate() can be called more
> than once before PM autosuspend kicks in and thus clk_prepare_enable()/
> clk_disable()_unprepare() operations will be done only once. This
> would give better performance on the "burst" operations.
>
> [ The above assumes that clock operations are more costly than
> going through PM core to check the current device state. ]
I agree that we loose the "burst" mode but:
1. At least on Exynso4 SSS is part of TOP power domain so it will not
help to reduce any more power consumption (on Exynos5422 it is
mentioned in G2D... which seems incorrect).
2. I think the overhead of clk operations is much smaller. These are only
two locks (prepare mutex + enable spin), simple tree traversal and
play with few SFRs.
The power domain code in comparison to that is huge and complicated
with inter-device links and dependencies. Also the actual runtime PM
suspend would anyway fall back at then end to clk prepare/enable
locks and paths.
We've been talking about this a lot with Marek Szyprowski (cc'ed) and
he was always (AFAIR) against attempts of runtime power
management of a single clock...
>
> > +static int exynos_rng_get_random(struct exynos_rng_dev *rng,
> > + u8 *dst, unsigned int dlen,
> > + unsigned int *read)
> > +{
> > + int retry = 100;
>
> I know that this is copied verbatim from the old driver but please
> use define for the maximum number of retries.
No problem. Number is chosen arbitrarily so there will not be any
additional information coming from the define.
> > +static int exynos_rng_probe(struct platform_device *pdev)
> > +{
> > + struct exynos_rng_dev *rng;
> > + struct resource *res;
> > + int ret;
> > +
> > + if (exynos_rng_dev)
> > + return -EEXIST;
>
> How this condition could ever happen?
>
> The probe function will never be called twice.
I really do not like global or file-scope variables. I do not like
drivers using them. Actually I hate them.
>From time to time I encounter a driver which was designed with that
approach - static fields and hidden assumption that there will be only
one instance. Usually that assumption is really hidden...
... and then it happens that I want to use two instances which of course
fails.
This code serves as a clear documentation for this assumption - only one
instance is allowed. You can look at it as a self-documenting
requirement.
And I think the probe might be called twice, for example in case of
mistake in DTB.
Best regards,
Krzysztof
^ permalink raw reply
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