Linux cryptographic layer development
 help / color / mirror / Atom feed
* 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

* Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Bartlomiej Zolnierkiewicz @ 2017-03-24 16:11 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,
	Marek Szyprowski
In-Reply-To: <20170324154600.lavc22dbw7rtioli@kozik-lap>

On Friday, March 24, 2017 06:46:00 PM Krzysztof Kozlowski wrote:
> 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...

OK, thanks for explanation.

> > > +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.

For me it looks as needless case of defensive programming and when
I see the code like this it always raises questions about the real
intentions of the code. I find it puzzling and not helpful.

> And I think the probe might be called twice, for example in case of
> mistake in DTB.

Even if this is possible resource allocation code in the driver will
take take care of handling it just fine,

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 16:19 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: <9265537.P6AeF50kg8@amdc3058>

On Fri, Mar 24, 2017 at 05:11:25PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Friday, March 24, 2017 06:46:00 PM Krzysztof Kozlowski wrote:
> > 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.
> 
> For me it looks as needless case of defensive programming and when
> I see the code like this it always raises questions about the real
> intentions of the code. I find it puzzling and not helpful.

I do not understand what might be puzzling about check for static
file-scope value. It is of course subjective, but for me that looks
pretty self-explanatory.

> 
> > And I think the probe might be called twice, for example in case of
> > mistake in DTB.
> 
> Even if this is possible resource allocation code in the driver will
> take take care of handling it just fine,

Indeed, the devm_ioremap_resource() solves the case. I can drop the
check then.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Bartlomiej Zolnierkiewicz @ 2017-03-24 16:45 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,
	Marek Szyprowski
In-Reply-To: <20170324161934.kc6g36nazr3y32kp@kozik-lap>

On Friday, March 24, 2017 07:19:34 PM Krzysztof Kozlowski wrote:
> On Fri, Mar 24, 2017 at 05:11:25PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Friday, March 24, 2017 06:46:00 PM Krzysztof Kozlowski wrote:
> > > 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.
> > 
> > For me it looks as needless case of defensive programming and when
> > I see the code like this it always raises questions about the real
> > intentions of the code. I find it puzzling and not helpful.
> 
> I do not understand what might be puzzling about check for static
> file-scope value. It is of course subjective, but for me that looks
> pretty self-explanatory.

The check should never happen given that ->probe will not happen twice.

However it seems that this is possible now with DT platform devices and
incorrect DTB.

> > > And I think the probe might be called twice, for example in case of
> > > mistake in DTB.
> > 
> > Even if this is possible resource allocation code in the driver will
> > take take care of handling it just fine,
> 
> Indeed, the devm_ioremap_resource() solves the case. I can drop the
> check then.

Looking on this a bit more it seems that devm_ioremap_resource() will
not cover all mistakes (using compatible by mistake in some other DTB
node).

Leave the check, I take my objection back.

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 17:01 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  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,
	Marek Szyprowski
In-Reply-To: <87677624.EBdhaJY1KM@amdc3058>

On Fri, Mar 24, 2017 at 05:45:41PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > > And I think the probe might be called twice, for example in case of
> > > > mistake in DTB.
> > > 
> > > Even if this is possible resource allocation code in the driver will
> > > take take care of handling it just fine,
> > 
> > Indeed, the devm_ioremap_resource() solves the case. I can drop the
> > check then.
> 
> Looking on this a bit more it seems that devm_ioremap_resource() will
> not cover all mistakes (using compatible by mistake in some other DTB
> node).
> 
> Leave the check, I take my objection back.

Great! Thanks for feedback.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [RFC PATCH v2 15/32] x86: Add support for changing memory encryption attribute in early boot
From: Borislav Petkov @ 2017-03-24 17:12 UTC (permalink / raw)
  To: Brijesh Singh
  Cc: simon.guinot, linux-efi, kvm, rkrcmar, matt, linux-pci,
	linus.walleij, gary.hook, linux-mm, paul.gortmaker, hpa, cl,
	dan.j.williams, aarcange, sfr, andriy.shevchenko, herbert, bhe,
	xemul, joro, x86, peterz, piotr.luc, mingo, msalter, ross.zwisler,
	dyoung, thomas.lendacky, jroedel, keescook, arnd, toshi.kani,
	mathieu.desnoyers, luto, devel, bhelgaas
In-Reply-To: <148846772794.2349.1396854638510933455.stgit@brijesh-build-machine>

On Thu, Mar 02, 2017 at 10:15:28AM -0500, Brijesh Singh wrote:
> Some KVM-specific custom MSRs shares the guest physical address with
> hypervisor. When SEV is active, the shared physical address must be mapped
> with encryption attribute cleared so that both hypervsior and guest can
> access the data.
> 
> Add APIs to change memory encryption attribute in early boot code.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  arch/x86/include/asm/mem_encrypt.h |   15 +++++++++
>  arch/x86/mm/mem_encrypt.c          |   63 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 78 insertions(+)
> 
> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> index 9799835..95bbe4c 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -47,6 +47,9 @@ void __init sme_unmap_bootdata(char *real_mode_data);
>  
>  void __init sme_early_init(void);
>  
> +int __init early_set_memory_decrypted(void *addr, unsigned long size);
> +int __init early_set_memory_encrypted(void *addr, unsigned long size);
> +
>  /* Architecture __weak replacement functions */
>  void __init mem_encrypt_init(void);
>  
> @@ -110,6 +113,18 @@ static inline void __init sme_early_init(void)
>  {
>  }
>  
> +static inline int __init early_set_memory_decrypted(void *addr,
> +						    unsigned long size)
> +{
> +	return 1;
	^^^^^^^^

return 1 when !CONFIG_AMD_MEM_ENCRYPT ?

The non-early variants return 0.

> +}
> +
> +static inline int __init early_set_memory_encrypted(void *addr,
> +						    unsigned long size)
> +{
> +	return 1;
> +}
> +
>  #define __sme_pa		__pa
>  #define __sme_pa_nodebug	__pa_nodebug
>  
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index 7df5f4c..567e0d8 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -15,6 +15,7 @@
>  #include <linux/mm.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/swiotlb.h>
> +#include <linux/mem_encrypt.h>
>  
>  #include <asm/tlbflush.h>
>  #include <asm/fixmap.h>
> @@ -258,6 +259,68 @@ static void sme_free(struct device *dev, size_t size, void *vaddr,
>  	swiotlb_free_coherent(dev, size, vaddr, dma_handle);
>  }
>  
> +static unsigned long __init get_pte_flags(unsigned long address)
> +{
> +	int level;
> +	pte_t *pte;
> +	unsigned long flags = _KERNPG_TABLE_NOENC | _PAGE_ENC;
> +
> +	pte = lookup_address(address, &level);
> +	if (!pte)
> +		return flags;
> +
> +	switch (level) {
> +	case PG_LEVEL_4K:
> +		flags = pte_flags(*pte);
> +		break;
> +	case PG_LEVEL_2M:
> +		flags = pmd_flags(*(pmd_t *)pte);
> +		break;
> +	case PG_LEVEL_1G:
> +		flags = pud_flags(*(pud_t *)pte);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return flags;
> +}
> +
> +int __init early_set_memory_enc_dec(void *vaddr, unsigned long size,
> +				    unsigned long flags)
> +{
> +	unsigned long pfn, npages;
> +	unsigned long addr = (unsigned long)vaddr & PAGE_MASK;
> +
> +	/* We are going to change the physical page attribute from C=1 to C=0.
> +	 * Flush the caches to ensure that all the data with C=1 is flushed to
> +	 * memory. Any caching of the vaddr after function returns will
> +	 * use C=0.
> +	 */

Kernel comments style is:

	/*
	 * A sentence ending with a full-stop.
	 * Another sentence. ...
	 * More sentences. ...
	 */

> +	clflush_cache_range(vaddr, size);
> +
> +	npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> +	pfn = slow_virt_to_phys((void *)addr) >> PAGE_SHIFT;
> +
> +	return kernel_map_pages_in_pgd(init_mm.pgd, pfn, addr, npages,
> +					flags & ~sme_me_mask);
> +
> +}
> +
> +int __init early_set_memory_decrypted(void *vaddr, unsigned long size)
> +{
> +	unsigned long flags = get_pte_flags((unsigned long)vaddr);

So this does lookup_address()...

> +	return early_set_memory_enc_dec(vaddr, size, flags & ~sme_me_mask);

... and this does it too in slow_virt_to_phys(). So you do it twice per
vaddr.

So why don't you define a __slow_virt_to_phys() helper - notice
the "__" - which returns flags in its second parameter and which
slow_virt_to_phys() calls with a NULL second parameter in the other
cases?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply

* [cryptodev:master 42/60] drivers/crypto/ccp/ccp-dev-v5.c:436:28: note: in expansion of macro 'cpu_to_le32'
From: kbuild test robot @ 2017-03-24 17:53 UTC (permalink / raw)
  To: Gary R Hook; +Cc: kbuild-all, linux-crypto, Herbert Xu

[-- Attachment #1: Type: text/plain, Size: 3165 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
head:   796b40c6171456274b02447e1dbbea97456403fe
commit: 990672d48515ce09c76fcf1ceccee48b0dd1942b [42/60] crypto: ccp - Enable 3DES function on v5 CCPs
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 990672d48515ce09c76fcf1ceccee48b0dd1942b
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/byteorder/big_endian.h:4:0,
                    from arch/arm64/include/uapi/asm/byteorder.h:20,
                    from include/asm-generic/bitops/le.h:5,
                    from arch/arm64/include/asm/bitops.h:50,
                    from include/linux/bitops.h:36,
                    from include/linux/kernel.h:10,
                    from include/linux/list.h:8,
                    from include/linux/module.h:9,
                    from drivers/crypto/ccp/ccp-dev-v5.c:13:
   drivers/crypto/ccp/ccp-dev-v5.c: In function 'ccp5_perform_des3':
   include/uapi/linux/byteorder/big_endian.h:32:26: warning: large integer implicitly truncated to unsigned type [-Woverflow]
    #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
                             ^
   include/linux/byteorder/generic.h:87:21: note: in expansion of macro '__cpu_to_le32'
    #define cpu_to_le32 __cpu_to_le32
                        ^~~~~~~~~~~~~
>> drivers/crypto/ccp/ccp-dev-v5.c:436:28: note: in expansion of macro 'cpu_to_le32'
     CCP5_CMD_KEY_MEM(&desc) = cpu_to_le32(CCP_MEMTYPE_SB);
                               ^~~~~~~~~~~

vim +/cpu_to_le32 +436 drivers/crypto/ccp/ccp-dev-v5.c

   420		CCP_DES3_MODE(&function) = op->u.des3.mode;
   421		CCP_DES3_TYPE(&function) = op->u.des3.type;
   422		CCP5_CMD_FUNCTION(&desc) = cpu_to_le32(function.raw);
   423	
   424		CCP5_CMD_LEN(&desc) = cpu_to_le32(op->src.u.dma.length);
   425	
   426		CCP5_CMD_SRC_LO(&desc) = cpu_to_le32(ccp_addr_lo(&op->src.u.dma));
   427		CCP5_CMD_SRC_HI(&desc) = cpu_to_le32(ccp_addr_hi(&op->src.u.dma));
   428		CCP5_CMD_SRC_MEM(&desc) = cpu_to_le32(CCP_MEMTYPE_SYSTEM);
   429	
   430		CCP5_CMD_DST_LO(&desc) = cpu_to_le32(ccp_addr_lo(&op->dst.u.dma));
   431		CCP5_CMD_DST_HI(&desc) = cpu_to_le32(ccp_addr_hi(&op->dst.u.dma));
   432		CCP5_CMD_DST_MEM(&desc) = cpu_to_le32(CCP_MEMTYPE_SYSTEM);
   433	
   434		CCP5_CMD_KEY_LO(&desc) = cpu_to_le32(lower_32_bits(key_addr));
   435		CCP5_CMD_KEY_HI(&desc) = 0;
 > 436		CCP5_CMD_KEY_MEM(&desc) = cpu_to_le32(CCP_MEMTYPE_SB);
   437		CCP5_CMD_LSB_ID(&desc) = cpu_to_le32(op->sb_ctx);
   438	
   439		return ccp5_do_cmd(&desc, op->cmd_q);
   440	}
   441	
   442	static int ccp5_perform_rsa(struct ccp_op *op)
   443	{
   444		struct ccp5_desc desc;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54633 bytes --]

^ permalink raw reply

* [PATCH v2 0/3] crypto: hw_random - Add new Exynos RNG driver
From: Krzysztof Kozlowski @ 2017-03-24 18:26 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, Bartlomiej Zolnierkiewicz,
	Stephan Müller
  Cc: Arnd Bergmann, Olof Johansson, Krzysztof Kozlowski

Hi,


This is a follow up of my questions around exynos-rng [1].

Changes since v1:
=================
1. Re-work the code for seeding after system resume, following suggestions
   and review by Stephan Müller.
   The resume path was not tested.

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 (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         | 392 ++++++++++++++++++++++++++++++++++++
 9 files changed, 428 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

* [PATCH v2 2/3] ARM: exynos_defconfig: Enable Exynos RNG and user-space crypto API
From: Krzysztof Kozlowski @ 2017-03-24 18:26 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, Bartlomiej Zolnierkiewicz,
	Stephan Müller
  Cc: Arnd Bergmann, Olof Johansson, Krzysztof Kozlowski
In-Reply-To: <20170324182606.23339-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 v2 3/3] ARM: multi_v7_defconfig: Enable Exynos RNG and user-space crypto API
From: Krzysztof Kozlowski @ 2017-03-24 18:26 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, Bartlomiej Zolnierkiewicz,
	Stephan Müller
  Cc: Olof Johansson, Krzysztof Kozlowski, Arnd Bergmann
In-Reply-To: <20170324182606.23339-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 v2 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Krzysztof Kozlowski @ 2017-03-24 18:26 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, Bartlomiej Zolnierkiewicz,
	Stephan Müller
  Cc: Arnd Bergmann, Olof Johansson, Krzysztof Kozlowski
In-Reply-To: <20170324182606.23339-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 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 <krzk@kernel.org>

---

The resume path is untested as my Odroid U3 fails to go online. Testing
on Trats2 or other devices would be appreciated.
---
 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         | 392 ++++++++++++++++++++++++++++++++++++
 7 files changed, 416 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..ad7772fa6553
--- /dev/null
+++ b/drivers/crypto/exynos-rng.c
@@ -0,0 +1,392 @@
+/*
+ * 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)
+
+/*
+ * Driver re-seeds itself with generated random numbers to increase
+ * the randomness.
+ *
+ * Time for next re-seed in ms.
+ */
+#define EXYNOS_RNG_RESEED_TIME		100
+/*
+ * In polling mode, do not wait infinitely for the engine to finish the work.
+ */
+#define EXYNOS_RNG_WAIT_RETRIES		100
+
+/* 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;
+	/* Generated numbers stored for seeding during resume */
+	u32				seed_save[EXYNOS_RNG_SEED_REGS];
+	/* At least one random number sequence was generated for new seed? */
+	bool				new_seed_ready;
+	/* Time of last seeding in jiffies */
+	unsigned long			last_seeding;
+};
+
+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 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;
+	rng->last_seeding = jiffies;
+
+out:
+	clk_disable_unprepare(rng->clk);
+
+	return ret;
+}
+
+/*
+ * Read from output registers and put the data under 'dst' array,
+ * up to dlen bytes.
+ *
+ * The generated random data will be also stored for later seeding.
+ *
+ * Returns number of bytes actually stored in 'dst' (dlen
+ * or EXYNOS_RNG_SEED_SIZE).
+ */
+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;
+		}
+		rng->seed_save[j] = val;
+	}
+
+	/*
+	 * Engine filled all output registers, so read the remaining registers
+	 * for storing data as future seed.
+	 */
+	for (; j < EXYNOS_RNG_SEED_REGS; j++)
+		rng->seed_save[j] = exynos_rng_readl(rng, EXYNOS_RNG_OUT(j));
+	rng->new_seed_ready = true;
+
+	return cnt;
+}
+
+/* Re-seed itself from time to time */
+static void exynos_rng_reseed(struct exynos_rng_dev *rng)
+{
+	unsigned long now = jiffies;
+	unsigned long last_seeding = rng->last_seeding + \
+				     msecs_to_jiffies(EXYNOS_RNG_RESEED_TIME);
+
+	if (time_after(now, last_seeding))
+		exynos_rng_set_seed(rng, (u8 *)rng->seed_save,
+				    sizeof(rng->seed_save));
+}
+
+/*
+ * Start the engine and poll for finish.  Then read from output registers
+ * filling the 'dst' buffer up to 'dlen' bytes or up to size of generated
+ * random data (EXYNOS_RNG_SEED_SIZE).
+ *
+ * On success: return 0 and store number of read bytes under 'read' address.
+ * On error: return -ERRNO.
+ */
+static int exynos_rng_get_random(struct exynos_rng_dev *rng,
+				 u8 *dst, unsigned int dlen,
+				 unsigned int *read, bool reseed)
+{
+	int retry = EXYNOS_RNG_WAIT_RETRIES;
+
+	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);
+
+	if (reseed)
+		exynos_rng_reseed(rng);
+
+	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, true);
+		if (ret)
+			break;
+
+		dlen -= read;
+		dst += read;
+	} while (dlen > 0);
+
+	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)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct exynos_rng_dev *rng = platform_get_drvdata(pdev);
+	unsigned int read = 0;
+	int ret;
+
+	/* If we have a new seed, then nothing to do */
+	if (rng->new_seed_ready)
+		return 0;
+
+	/* If we were never seeded then after resume it will be the same */
+	if (!rng->last_seeding)
+		return 0;
+
+	ret = clk_prepare_enable(rng->clk);
+	if (ret)
+		return ret;
+
+	/*
+	 * Worst case, we have been seeded but random engine was never started.
+	 * Get new random numbers and store them for seeding on resume.
+	 */
+	exynos_rng_get_random(rng, (u8 *)rng->seed_save, sizeof(rng->seed_save),
+			      &read, false);
+	dev_dbg(rng->dev, "Stored %u bytes for seeding on system resume\n",
+		read);
+
+	clk_disable_unprepare(rng->clk);
+
+	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, (u8 *)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

* Re: [PATCH v2 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Stephan Müller @ 2017-03-24 20:41 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, Bartlomiej Zolnierkiewicz,
	Arnd Bergmann, Olof Johansson
In-Reply-To: <20170324182606.23339-2-krzk@kernel.org>

Am Freitag, 24. März 2017, 19:26:04 CET schrieb Krzysztof Kozlowski:

Hi Krzysztof,

> +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;
> +		}
> +		rng->seed_save[j] = val;

Just to clarify: is this call right? Shouldn't that be removed? Any RNG that 
is given to a caller is tainted and should not serve as seed.
> +	}
> +
> +	/*
> +	 * Engine filled all output registers, so read the remaining registers
> +	 * for storing data as future seed.
> +	 */
> +	for (; j < EXYNOS_RNG_SEED_REGS; j++)
> +		rng->seed_save[j] = exynos_rng_readl(rng, EXYNOS_RNG_OUT(j));

With this call, I guess the questioned line above could go away, right?


Ciao
Stephan

^ permalink raw reply

* KPP: DH/ECDH -- allocation returns ENOENT
From: Stephan Müller @ 2017-03-25  4:59 UTC (permalink / raw)
  To: salvatore benedetto; +Cc: linux-crypto

Hi Salvatore,

when I test 4.11-rc1 with the DH and ECDH options enabled, I get -ENOENT when 
trying to allocate the ciphers dh/dh_generic/ecdh.

Note, the dh/ecdh kernel modules are loaded.

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH v2 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Krzysztof Kozlowski @ 2017-03-25  7:36 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, Bartlomiej Zolnierkiewicz,
	Arnd Bergmann, Olof Johansson
In-Reply-To: <4248691.8fGIDmasva@tauon.atsec.com>

On Fri, Mar 24, 2017 at 09:41:59PM +0100, Stephan Müller wrote:
> Am Freitag, 24. März 2017, 19:26:04 CET schrieb Krzysztof Kozlowski:
> 
> Hi Krzysztof,
> 
> > +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;
> > +		}
> > +		rng->seed_save[j] = val;
> 
> Just to clarify: is this call right? Shouldn't that be removed? Any RNG that 
> is given to a caller is tainted and should not serve as seed.

In that case I could either re-use RNGs not passed to the caller (like
in the block quoted below) or generate another round of them just for
purpose of next seeding.

With the first approach the problem is that I might wait for such unused
numbers pretty long. If user is requesting large amount of data, then I
will always give him all five output numbers. I will not have unused
numbers.

The second approach seems safe, but requires additional engine run which
will slow down some of the generate() calls.

> > +	}
> > +
> > +	/*
> > +	 * Engine filled all output registers, so read the remaining registers
> > +	 * for storing data as future seed.
> > +	 */
> > +	for (; j < EXYNOS_RNG_SEED_REGS; j++)
> > +		rng->seed_save[j] = exynos_rng_readl(rng, EXYNOS_RNG_OUT(j));
> 
> With this call, I guess the questioned line above could go away, right?

This is used in combination with the previous line so I will get five
seeds (for five registers).

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH v2 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Stephan Müller @ 2017-03-25 12:48 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-samsung-soc, Herbert Xu, Arnd Bergmann,
	Bartlomiej Zolnierkiewicz, linux-kernel, Javier Martinez Canillas,
	Kukjin Kim, linux-crypto, Matt Mackall, Olof Johansson,
	David S. Miller, linux-arm-kernel
In-Reply-To: <20170325073648.cbphcouvi6trmhcj@kozik-lap>

Am Samstag, 25. März 2017, 08:36:48 CET schrieb Krzysztof Kozlowski:

Hi Krzysztof,

> On Fri, Mar 24, 2017 at 09:41:59PM +0100, Stephan Müller wrote:
> > Am Freitag, 24. März 2017, 19:26:04 CET schrieb Krzysztof Kozlowski:
> > 
> > Hi Krzysztof,
> > 
> > > +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;
> > > +		}
> > > +		rng->seed_save[j] = val;
> > 
> > Just to clarify: is this call right? Shouldn't that be removed? Any RNG
> > that is given to a caller is tainted and should not serve as seed.
> 
> In that case I could either re-use RNGs not passed to the caller (like
> in the block quoted below) or generate another round of them just for
> purpose of next seeding.
> 
> With the first approach the problem is that I might wait for such unused
> numbers pretty long. If user is requesting large amount of data, then I
> will always give him all five output numbers. I will not have unused
> numbers.
> 
> The second approach seems safe, but requires additional engine run which
> will slow down some of the generate() calls.

Random numbers should never be used twice.
> 
> > > +	}
> > > +
> > > +	/*
> > > +	 * Engine filled all output registers, so read the remaining registers
> > > +	 * for storing data as future seed.
> > > +	 */
> > > +	for (; j < EXYNOS_RNG_SEED_REGS; j++)
> > > +		rng->seed_save[j] = exynos_rng_readl(rng, EXYNOS_RNG_OUT(j));
> > 
> > With this call, I guess the questioned line above could go away, right?
> 
> This is used in combination with the previous line so I will get five
> seeds (for five registers).
> 
> Best regards,
> Krzysztof


Ciao
Stephan

^ permalink raw reply

* Re: KPP: DH/ECDH -- allocation returns ENOENT
From: Stephan Müller @ 2017-03-25 13:49 UTC (permalink / raw)
  To: Stephan Müller; +Cc: salvatore benedetto, linux-crypto
In-Reply-To: <2264613.TBqMtEgLeW@positron.chronox.de>

Am Samstag, 25. März 2017, 05:59:07 CET schrieb Stephan Müller:

Hi,

> Hi Salvatore,
> 
> when I test 4.11-rc1 with the DH and ECDH options enabled, I get -ENOENT
> when trying to allocate the ciphers dh/dh_generic/ecdh.
> 
> Note, the dh/ecdh kernel modules are loaded.

Apologies, please disregard this email. I had a patch in the kernel that 
forgot to remove :-)

Ciao
Stephan

^ permalink raw reply

* [PATCH v3 0/3] crypto: hw_random - Add new Exynos RNG driver
From: Krzysztof Kozlowski @ 2017-03-25 16:26 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, Bartlomiej Zolnierkiewicz,
	Stephan Müller
  Cc: Olof Johansson, Krzysztof Kozlowski, Arnd Bergmann

Hi,


This is a follow up of my questions around exynos-rng [1].

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 (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         | 390 ++++++++++++++++++++++++++++++++++++
 9 files changed, 426 insertions(+), 246 deletions(-)
 delete mode 100644 drivers/char/hw_random/exynos-rng.c
 create mode 100644 drivers/crypto/exynos-rng.c

-- 
2.9.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v3 2/3] ARM: exynos_defconfig: Enable Exynos RNG and user-space crypto API
From: Krzysztof Kozlowski @ 2017-03-25 16:26 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, Bartlomiej Zolnierkiewicz,
	Stephan Müller
  Cc: Arnd Bergmann, Olof Johansson, Krzysztof Kozlowski
In-Reply-To: <20170325162654.3827-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 v3 3/3] ARM: multi_v7_defconfig: Enable Exynos RNG and user-space crypto API
From: Krzysztof Kozlowski @ 2017-03-25 16:26 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, Bartlomiej Zolnierkiewicz,
	Stephan Müller
  Cc: Arnd Bergmann, Olof Johansson, Krzysztof Kozlowski
In-Reply-To: <20170325162654.3827-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 v3 1/3] crypto: hw_random - Add new Exynos RNG driver
From: Krzysztof Kozlowski @ 2017-03-25 16:26 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, Bartlomiej Zolnierkiewicz,
	Stephan Müller
  Cc: Arnd Bergmann, Olof Johansson, Krzysztof Kozlowski
In-Reply-To: <20170325162654.3827-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 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 <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         | 390 ++++++++++++++++++++++++++++++++++++
 7 files changed, 414 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..d657b6243d0a
--- /dev/null
+++ b/drivers/crypto/exynos-rng.c
@@ -0,0 +1,390 @@
+/*
+ * 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)
+
+/*
+ * Driver re-seeds itself with generated random numbers to increase
+ * the randomness.
+ *
+ * Time for next re-seed in ms.
+ */
+#define EXYNOS_RNG_RESEED_TIME		100
+/*
+ * In polling mode, do not wait infinitely for the engine to finish the work.
+ */
+#define EXYNOS_RNG_WAIT_RETRIES		100
+
+/* 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;
+	/* Generated numbers stored for seeding during resume */
+	u8				seed_save[EXYNOS_RNG_SEED_SIZE];
+	unsigned int			seed_save_len;
+	/* Time of last seeding in jiffies */
+	unsigned long			last_seeding;
+};
+
+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 int exynos_rng_set_seed(struct exynos_rng_dev *rng,
+			       const u8 *seed, unsigned int slen)
+{
+	u32 val;
+	int i;
+
+	dev_dbg(rng->dev, "Seeding with %u bytes\n", slen);
+
+	if (slen < EXYNOS_RNG_SEED_SIZE) {
+		dev_warn(rng->dev, "Seed too short (only %u bytes)\n", slen);
+		return -EINVAL;
+	}
+
+	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");
+		return -EIO;
+	}
+
+	rng->last_seeding = jiffies;
+
+	return 0;
+}
+
+/*
+ * Read from output registers and put the data under 'dst' array,
+ * up to dlen bytes.
+ *
+ * Returns number of bytes actually stored in 'dst' (dlen
+ * or EXYNOS_RNG_SEED_SIZE).
+ */
+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;
+}
+
+/*
+ * Start the engine and poll for finish.  Then read from output registers
+ * filling the 'dst' buffer up to 'dlen' bytes or up to size of generated
+ * random data (EXYNOS_RNG_SEED_SIZE).
+ *
+ * On success: return 0 and store number of read bytes under 'read' address.
+ * On error: return -ERRNO.
+ */
+static int exynos_rng_get_random(struct exynos_rng_dev *rng,
+				 u8 *dst, unsigned int dlen,
+				 unsigned int *read)
+{
+	int retry = EXYNOS_RNG_WAIT_RETRIES;
+
+	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;
+}
+
+/* Re-seed itself from time to time */
+static void exynos_rng_reseed(struct exynos_rng_dev *rng)
+{
+	unsigned long next_seeding = rng->last_seeding + \
+				     msecs_to_jiffies(EXYNOS_RNG_RESEED_TIME);
+	unsigned long now = jiffies;
+	u8 seed[EXYNOS_RNG_SEED_SIZE];
+	unsigned int read;
+
+	if (time_before(now, next_seeding))
+		return;
+
+	if (exynos_rng_get_random(rng, seed, sizeof(seed), &read))
+		return;
+
+	exynos_rng_set_seed(rng, seed, read);
+}
+
+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;
+
+		exynos_rng_reseed(rng);
+	} while (dlen > 0);
+
+	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);
+	struct exynos_rng_dev *rng = ctx->rng;
+	int ret;
+
+	ret = clk_prepare_enable(rng->clk);
+	if (ret)
+		return ret;
+
+	ret = exynos_rng_set_seed(ctx->rng, seed, slen);
+
+	clk_disable_unprepare(rng->clk);
+
+	return ret;
+}
+
+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)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct exynos_rng_dev *rng = platform_get_drvdata(pdev);
+	int ret;
+
+	/* If we were never seeded then after resume it will be the same */
+	if (!rng->last_seeding)
+		return 0;
+
+	rng->seed_save_len = 0;
+	ret = clk_prepare_enable(rng->clk);
+	if (ret)
+		return ret;
+
+	/* Get new random numbers and store them for seeding on resume. */
+	exynos_rng_get_random(rng, rng->seed_save, sizeof(rng->seed_save),
+			      &(rng->seed_save_len));
+	dev_dbg(rng->dev, "Stored %u bytes for seeding on system resume\n",
+		rng->seed_save_len);
+
+	clk_disable_unprepare(rng->clk);
+
+	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);
+	int ret;
+
+	/* Never seeded so nothing to do */
+	if (!rng->last_seeding)
+		return 0;
+
+	ret = clk_prepare_enable(rng->clk);
+	if (ret)
+		return ret;
+
+	ret = exynos_rng_set_seed(rng, rng->seed_save, rng->seed_save_len);
+
+	clk_disable_unprepare(rng->clk);
+
+	return ret;
+}
+
+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

* Re: [PATCH v3 0/3] crypto: hw_random - Add new Exynos RNG driver
From: Stephan Müller @ 2017-03-25 16:32 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-samsung-soc, Herbert Xu, Arnd Bergmann,
	Bartlomiej Zolnierkiewicz, linux-kernel, Javier Martinez Canillas,
	Kukjin Kim, linux-crypto, Matt Mackall, Olof Johansson,
	David S. Miller, linux-arm-kernel
In-Reply-To: <20170325162654.3827-1-krzk@kernel.org>

Am Samstag, 25. März 2017, 17:26:51 CET schrieb Krzysztof Kozlowski:

Hi Krzysztof,

> Hi,
> 
> 
> This is a follow up of my questions around exynos-rng [1].
> 
> 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).

Thank you. Regarding the cryptographic aspects:

Reviewed-by: Stephan Müller <smueller@chronox.de>

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH] padata: avoid race in reordering
From: David Miller @ 2017-03-26  3:01 UTC (permalink / raw)
  To: steffen.klassert; +Cc: Jason, linux-crypto, linux-kernel, netdev, herbert
In-Reply-To: <20170324094157.GE5279@secunet.com>

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Fri, 24 Mar 2017 10:41:59 +0100

> On Thu, Mar 23, 2017 at 12:24:43PM +0100, Jason A. Donenfeld wrote:
>> Under extremely heavy uses of padata, crashes occur, and with list
>> debugging turned on, this happens instead:
 ...
>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> 
> Acked-by: Steffen Klassert <steffen.klassert@secunet.com>

Herbert, this should probably go via your crypto tree.

^ permalink raw reply

* Re: [PATCH] padata: avoid race in reordering
From: Herbert Xu @ 2017-03-26  3:11 UTC (permalink / raw)
  To: David Miller; +Cc: steffen.klassert, Jason, linux-crypto, linux-kernel, netdev
In-Reply-To: <20170325.200151.245940842455044501.davem@davemloft.net>

On Sat, Mar 25, 2017 at 08:01:51PM -0700, David Miller wrote:
> From: Steffen Klassert <steffen.klassert@secunet.com>
> Date: Fri, 24 Mar 2017 10:41:59 +0100
> 
> > On Thu, Mar 23, 2017 at 12:24:43PM +0100, Jason A. Donenfeld wrote:
> >> Under extremely heavy uses of padata, crashes occur, and with list
> >> debugging turned on, this happens instead:
>  ...
> >> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > 
> > Acked-by: Steffen Klassert <steffen.klassert@secunet.com>
> 
> Herbert, this should probably go via your crypto tree.

Thanks David.  It's already in the crypto tree.
-- 
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: Question - seeding the hw pseudo random number generator
From: Stephan Müller @ 2017-03-26  4:10 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Corentin Labbe, PrasannaKumar Muralidharan, linux-arm-kernel,
	linux-crypto, Krzysztof Kozlowski, Matt Mackall
In-Reply-To: <20170323094406.GA6848@gondor.apana.org.au>

Am Donnerstag, 23. März 2017, 10:44:06 CEST schrieb Herbert Xu:

Hi Herbert,

> On Thu, Mar 23, 2017 at 09:23:07AM +0100, Corentin Labbe wrote:
> > Problem with this conversion, a huge regression for user space.
> > Using hwrng is simple as cat /dev/hwrng.
> > Using algif_rng via AF_ALG is ... unusable for the moment.
> > Perhaps creating an user space tool (prng-tool which provide a cat
> > /dev/hwrng replacement) is mandatory before any convertion.
> Stephan may have a tool to do this.  Stephan?

I added the application kcapi-rng to HEAD of [1]. Create the application with 
the --enable-kcapi-rngapp configure option.

$ kcapi-rng 

Kernel Crypto API Random Number Gatherer

Kernel Crypto API interface library version: libkcapi pre-release 0.13.1
Reported numeric version number 130080

Usage:
        -b --bytes <BYTES>      Number of bytes to generate (required option)
        -h --help       This help information
           --version    Print version
        -v --verbose    Verbose logging, multiple options increase verbosity

Data provided at stdin is used to seed the DRNG

[1] https://github.com/smuellerDD/libkcapi/

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH] padata: avoid race in reordering
From: Jason A. Donenfeld @ 2017-03-26 12:32 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David Miller, Steffen Klassert, Linux Crypto Mailing List, LKML,
	Netdev
In-Reply-To: <20170326031145.GA19335@gondor.apana.org.au>

I've got a few other races in padata, I think, that I'm working on
fixing. If these pan out, I'll submit them exclusively to -crypto
instead of -netdev, to avoid this confusion next time. Of course, if
I'm able to fix these, then I'll probably be bald from pulling my hair
out during this insane debugging frenzy of the last few days...

Jason

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox