crypto: hw_random - Add new Exynos RNG driver
Subsystems (8)
Krzysztof Kozlowski
maintainerv4 cover crypto: hw_random - Add new Exynos RNG driver
Hi,
This is a follow up of my questions around exynos-rng [1].
Changes since v3:
=================
1. New patch: 1/2 for ALIGN_DOWN macro. The change in metag architecture was not compiled. Please test it.
2. Dropped patches touching ARM defconfig as they are not changing and they pollute this submission.
3. Utilize all seed provided by kcapi (suggested by Stephan Müller).
4. Drop dev->ctx (suggested by PrasannaKumar Muralidharan).
5. Remove any printks from set_seed callback as this might be a way for unprivileged user to pollute the log (suggested by Stephan).
6. Minor cleanups: initialize 'read' variable in exynos_rng_reseed() for readability (it is not strictly required).
7. Add review tags from Stephen and PrasannaKumar.
Changes since v2:
=================
1. Do not re-use random numbers for re-seed (neither for system resume nor for periodic re-seed). Instead the driver will just generate new random numbers (suggested by Stephan Müller).
Suspend path tested with suspend-to-freeze, not real suspend. Testing on Trats2 would be welcomed.
Changes since v1:
=================
1. Re-work the code for seeding after system resume, following suggestions and review by Stephan Müller.
2. Re-seed itself from time to time (every 100 ms), suggested by Stephan Müller.
3. Use a define for retries (Bartlomiej Zolnierkiewicz).
4. Add some docs.
Description:
============
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 (2):
linux/kernel.h: Add ALIGN_DOWN macro
crypto: hw_random - Add new Exynos RNG driver
MAINTAINERS | 8 + arch/metag/kernel/stacktrace.c | 2 - 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 | 389 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/udl/udl_fb.c | 2 +- include/linux/kernel.h | 1 + include/video/udlfb.h | 2 +- 11 files changed, 416 insertions(+), 250 deletions(-) delete mode 100644 drivers/char/hw_random/exynos-rng.c create mode 100644 drivers/crypto/exynos-rng.c
Signature · 1 line
2.9.3
Krzysztof Kozlowski
maintainerPATCH v4 1/2 linux/kernel.h: Add ALIGN_DOWN macro
Few parts of kernel define their own macro for aligning down so provide a common define for this, with the same usage and assumptions as existing ALIGN.
Convert also three existing implementations to this one.
---
The metag change was not even compiled due to lack of cross compiler.
--- arch/metag/kernel/stacktrace.c | 2 -- drivers/gpu/drm/udl/udl_fb.c | 2 +- include/linux/kernel.h | 1 + include/video/udlfb.h | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-)
arch/metag/kernel/stacktrace.c +0 -2
@@ -31,8 +31,6 @@ static void tbi_boing_init(void) }#endif -#define ALIGN_DOWN (addr, size) ((addr)&(~((size)-1))) -/* * Unwind the current stack frame and store the new register values in the * structure passed as argument. Unwinding is equivalent to a function return,
drivers/gpu/drm/udl/udl_fb.c +1 -1
@@ -37,7 +37,7 @@ struct udl_fbdev { };#define DL_ALIGN_UP (x, a) ALIGN(x, a) -#define DL_ALIGN_DOWN (x, a) ALIGN(x-(a-1), a) +#define DL_ALIGN_DOWN (x, a) ALIGN_DOWN(x, a)/** Read the red component (0..255) of a 32 bpp colour. */ #define DLO_RGB_GETRED (col) (uint8_t)((col) & 0xFF)
include/linux/kernel.h +1 -0
@@ -47,6 +47,7 @@/* @a is a power of 2 value */ #define ALIGN (x, a) __ALIGN_KERNEL((x), (a)) +#define ALIGN_DOWN (x, a) __ALIGN_KERNEL((x) - ((a) - 1), (a))#define __ALIGN_MASK (x, mask) __ALIGN_KERNEL_MASK((x), (mask))#define PTR_ALIGN (p, a) ((typeof(p))ALIGN((unsigned long)(p), (a)))#define IS_ALIGNED (x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
include/video/udlfb.h +1 -1
@@ -92,6 +92,6 @@ struct dlfb_data {/* remove these once align.h patch is taken into kernel */ #define DL_ALIGN_UP (x, a) ALIGN(x, a) -#define DL_ALIGN_DOWN (x, a) ALIGN(x-(a-1), a) +#define DL_ALIGN_DOWN (x, a) ALIGN_DOWN(x, a)#endif
Signature · 1 line
2.9.3
Krzysztof Kozlowski
maintainerPATCH v4 2/2 crypto: hw_random - Add new Exynos RNG driver
In reply to Krzysztof KozlowskiReplace 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 32-bit numbers in each pass but old driver was returning only one 32-bit number 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 reseeding itself with generated random data periodically and during resuming from system suspend (previously driver was re-seeding itself again with jiffies).
--- 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 | 389 ++++++++++++++++++++++++++++++++++++ 7 files changed, 413 insertions(+), 246 deletions(-) delete mode 100644 drivers/char/hw_random/exynos-rng.c create mode 100644 drivers/crypto/exynos-rng.c
MAINTAINERS +8 -0
@@ -10977,6 +10977,14 @@ L: [email protected] (moderated for non-subscribers) S: Supported F: sound/soc/samsung/ +SAMSUNG EXYNOS PSEUDO RANDOM NUMBER GENERATOR (RNG) DRIVER +M: Krzysztof Kozlowski <[email protected]> +L: [email protected] +L: [email protected] +S: Maintained +F: drivers/crypto/exynos-rng.c +F: Documentation/devicetree/bindings/rng/samsung,exynos-rng4.txt + SAMSUNG FRAMEBUFFER DRIVER M: Jingoo Han <[email protected]> L: [email protected]
drivers/char/hw_random/Kconfig +0 -14
@@ -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
drivers/char/hw_random/Makefile +0 -1
@@ -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
drivers/char/hw_random/exynos-rng.c deleted +0 -231
231 lines not on this page · raw message
drivers/crypto/Kconfig +15 -0
@@ -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
drivers/crypto/Makefile +1 -0
@@ -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
drivers/crypto/exynos-rng.c new file +389 -0
389 lines not on this page · raw message
Signature · 1 line
2.9.3
Hi,
Quoted 21 lines · Krzysztof Kozlowski
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_randomAPI. Rationale for the change:
1.hw_randominterface 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 32-bit numbers in each pass but old driver was returning only one 32-bit number 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.
Quoted 7 lines
Another difference is reseeding itself with generated random data periodically and during resuming from system suspend (previously driver was re-seeding itself again with jiffies).
Signed-off-by Krzysztof Kozlowski <[email protected]>Reviewed-by Stephan M?ller <[email protected]>Reviewed-by PrasannaKumar Muralidharan <[email protected]>
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Herbert Xu
maintainerdrivers/crypto/exynos-rng.c line 260
Quoted 7 lines · Krzysztof Kozlowski
drivers/crypto/exynos-rng.c new file +6
-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" ,
Please use stdrng. Or is there a reason why this can't be used by the crypto layer itself?
Cheers,
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Krzysztof Kozlowski
maintainerQuoted 11 lines · Herbert Xu
drivers/crypto/exynos-rng.c line 260
Quoted 7 lines · Krzysztof Kozlowski
drivers/crypto/exynos-rng.c new file +6
-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" ,Please use stdrng. Or is there a reason why this can't be used by the crypto layer itself?
I think there is no reason against, I'll fix it.
Best regards,
Krzysztof