getentropy() implementation on BBB
Udit agarwal
dev.madaari at gmail.com
Sun Mar 11 18:22:12 UTC 2018
Hi,
I have updated the patch and tested getentropy() , it did seem to work
(Logs: here
<https://gist.github.com/madaari/17bdec209eac993538b0192fba014918>).
Moreover, i've added BSD-2-clause license please verify it once. Also, i
need to shift
TRNG register structure from AM335x.h to bbb_getentropy.c . since, keeping
it in am335x.h
requires including stdint.h (for uint32_t etc) which i don't think would be
right, do let me know if this
is the right way.
Here's the patch:
>From f87b39708dd06482957fc7b33e78ab0ee095287b Mon Sep 17 00:00:00 2001
From: Udit agarwal <dev.madaari at gmail.com>
Date: Sun, 11 Mar 2018 23:34:17 +0530
Subject: [PATCH] Added getentropy support to BBB BSP
---
bsps/arm/include/libcpu/am335x.h | 22 +++-
c/src/lib/libbsp/arm/beagle/Makefile.am | 4 +-
.../libbsp/arm/beagle/getentropy/bbb_getentropy.c | 128
+++++++++++++++++++++
3 files changed, 152 insertions(+), 2 deletions(-)
create mode 100644 c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
diff --git a/bsps/arm/include/libcpu/am335x.h
b/bsps/arm/include/libcpu/am335x.h
index 367e97c..bbdc8d0 100644
--- a/bsps/arm/include/libcpu/am335x.h
+++ b/bsps/arm/include/libcpu/am335x.h
@@ -14,6 +14,9 @@
* Modified by Ben Gras <beng at shrike-systems.com> to add lots
* of beagleboard/beaglebone definitions, delete lpc32xx specific
* ones, and merge with some other header files.
+ *
+ * Modified by Udit agarwal <dev.madaari at gmail.com> to add random
+ * number generating module definitions.
*/
#if !defined(_AM335X_H_)
@@ -701,4 +704,21 @@
#define AM335X_CM_PER_OCPWP_L3_CLKSTCTRL_CLKACTIVITY_OCPWP_L4_GCLK
(0x00000020u)
#define AM335X_I2C_INT_STOP_CONDITION AM335X_I2C_IRQSTATUS_BF
-#endif
+/* TRNG Register */
+
+/* RNG base address */
+#define RNG_BASE 0x48310000
+/* RNG clock control */
+#define CM_PER_RNG_CLKCTRL (AM335X_CM_PER_ADDR | (9 << 4))
+/* rng module clock status bits */
+#define AM335X_CLK_RNG_BIT_MASK (0x30000)
+/* Offset from RNG base for output ready flag */
+#define RNG_STATUS_RDY (1u << 0)
+/* Offset from RNG base for FRO related error */
+#define RNG_STATUS_ERR (1u << 1)
+/* Offset from RNG base for clock status */
+#define RNG_STATUS_CLK (1u << 31)
+/* enable module */
+#define AM335X_RNG_ENABLE (1 << 10)
+
+#endif
\ No newline at end of file
diff --git a/c/src/lib/libbsp/arm/beagle/Makefile.am
b/c/src/lib/libbsp/arm/beagle/Makefile.am
index 8251660..5d5ade3 100644
--- a/c/src/lib/libbsp/arm/beagle/Makefile.am
+++ b/c/src/lib/libbsp/arm/beagle/Makefile.am
@@ -40,7 +40,6 @@ libbsp_a_LIBADD =
# Shared
libbsp_a_SOURCES += ../../shared/bootcard.c
-libbsp_a_SOURCES += ../../shared/getentropy-cpucounter.c
libbsp_a_SOURCES += ../../shared/src/bsp-fdt.c
libbsp_a_SOURCES += ../../shared/bspclean.c
libbsp_a_SOURCES += ../../shared/bspgetworkarea.c
@@ -88,6 +87,9 @@ libbsp_a_SOURCES += gpio/bbb-gpio.c
#pwm
libbsp_a_SOURCES += pwm/pwm.c
+#getentropy
+libbsp_a_SOURCES += getentropy/bbb_getentropy.c
+
#RTC
libbsp_a_SOURCES += rtc.c
libbsp_a_SOURCES += ../../shared/tod.c
diff --git a/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
b/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
new file mode 100644
index 0000000..117c6b8
--- /dev/null
+++ b/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
@@ -0,0 +1,128 @@
+/**
+* @file
+*
+* @ingroup arm_beagle
+*
+* @brief Getentropy implementation on BeagleBone Black BSP
+*/
+
+/*
+*Copyright 2018 Udit kumar agarwal <dev.madaari at gmail.com>
+*
+*Redistribution and use in source and binary forms, with or without
+*modification, are permitted provided that the following conditions are
met:
+*
+*1. Redistributions of source code must retain the above copyright notice,
+* this list of conditions and the following disclaimer.
+*
+*2. Redistributions in binary form must reproduce the above copyright
notice,
+* this list of conditions and the following disclaimer in the
documentation
+* and/or other materials provided with the distribution.
+*
+*THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
IS"
+*AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+*IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
PURPOSE
+*ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+*LIABLE FOR ANY DIRECT,INDIRECT, INCIDENTAL, SPECIAL,EXEMPLARY, OR
+*CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+*SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA,OR PROFITS; OR BUSINESS
+*INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,WHETHER IN
CONTRACT,
+*STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+*ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
POSSIBILITY
+*OF SUCH DAMAGE.
+*/
+
+#include <libcpu/am335x.h>
+#include <unistd.h>
+#include <string.h>
+#include <rtems/sysinit.h>
+#include <stdint.h>
+
+/* max refill 34 * 256 cycles */
+#define AM335X_RNG_MAX_REFILL (34 << 16)
+/* min refill 33 * 64 cycles */
+#define AM335X_RNG_MIN_REFILL (33 << 0)
+/* startup 33 * 256 cycles */
+#define AM335X_RNG_STARTUP_CYCLES (33 << 16)
+
+/* TRNG register structure */
+struct bbb_trng_register
+{
+ uint64_t output; /* 00 */
+ uint32_t status; /* 08 */
+ uint32_t irq_en; /* 0c */
+ uint32_t status_clr; /* 10 */
+ uint32_t control; /* 14 */
+ uint32_t config; /* 18 */
+};
+typedef struct bbb_trng_register bbb_trng_register;
+
+/* maximun and minimum refill cycle sets the number of samples to be taken
+ from FRO to generate random number */
+static void am335x_rng_enable(volatile bbb_trng_register *rng)
+{
+ rng->control = rng->config = 0;
+ rng->config |= AM335X_RNG_MIN_REFILL | AM335X_RNG_MAX_REFILL ;
+ rng->control |= AM335X_RNG_STARTUP_CYCLES | AM335X_RNG_ENABLE ;
+}
+
+static void am335x_rng_clock_enable(void)
+{
+ *(volatile uint8_t *) CM_PER_RNG_CLKCTRL = 2;
+ while(
+ *(volatile uint32_t *) CM_PER_RNG_CLKCTRL &
+ AM335X_CLK_RNG_BIT_MASK
+ ) {
+ /* wait */
+ }
+}
+
+static uint64_t trng_getranddata(volatile bbb_trng_register *rng)
+{
+ uint64_t output = rng->output;
+ return output;
+}
+
+int getentropy(void *ptr, size_t n)
+{
+ volatile bbb_trng_register *rng = (bbb_trng_register*) RNG_BASE;
+ am335x_rng_enable(rng);
+ while (n > 0)
+ {
+ uint64_t random;
+ size_t copy;
+
+ /* wait untill RNG becomes ready with next set of random data */
+ while( ( rng->status & RNG_STATUS_RDY ) == 0 )
+ {
+ /* wait */
+ }
+
+ random = trng_getranddata(rng);
+
+ /* Checking for error by masking all bits other then error bit in
+ status register */
+ if( ((rng->status & RNG_STATUS_ERR)>>1) == 1)
+ {
+ /* clear the status flag after reading to generate new random
+ value */
+ rng->status_clr = RNG_STATUS_RDY;
+ copy = sizeof(random);
+ if ( n < copy )
+ {
+ copy = n;
+ }
+ memcpy(ptr, &random, copy);
+ n -= copy;
+ ptr = (char*)ptr + copy;
+ }
+ }
+
+ return 0;
+}
+
+RTEMS_SYSINIT_ITEM(
+ am335x_rng_clock_enable,
+ RTEMS_SYSINIT_DEVICE_DRIVERS,
+ RTEMS_SYSINIT_ORDER_LAST
+);
\ No newline at end of file
--
1.9.1
Best regards,
Udit agarwal
On Wed, Mar 7, 2018 at 11:40 PM, Christian Mauderer <list at c-mauderer.de>
wrote:
> Hello Udit,
>
> let me start with a hint: It's quite normal that the first patches of a
> contributor get a lot of attention and have a lot of revisions. That can
> be quite annoying for the one who writes the patch. But please
> understand it in the way it's thought: As suggestions for improvement.
>
> Did you already test that patches? There is a (quite basic) test in the
> RTEMS testsuite: libtests/getentropy01. This test should run without
> problems. If you have a debugger, you might also want to check whether
> your getentropy implementation is reached and not the generic one.
>
> The patch looks quite good already. But like I said: A lot of revisions.
> I added some further remarks in the patch below ;-)
>
> Best regards
>
> Christian
>
>
> Am 07.03.2018 um 11:26 schrieb Udit agarwal:
> > Hi,
> > I have updated the code, please have a look.
> > From 74b8f4f5b9dd929b71ed5fb9dd0bc721a6f27a28 Mon Sep 17 00:00:00 2001
> > From: Udit agarwal <dev.madaari at gmail.com <mailto:dev.madaari at gmail.com>
> >
> > Date: Wed, 7 Mar 2018 15:52:13 +0530
> > Subject: [PATCH] Added getentropy support to beagle BSP
> >
> > ---
> > bsps/arm/include/libcpu/am335x.h | 34 ++++++++-
> > c/src/lib/libbsp/arm/beagle/Makefile.am | 4 +
> > .../libbsp/arm/beagle/getentropy/bbb_getentropy.c | 88
> > ++++++++++++++++++++++
> > 3 files changed, 125 insertions(+), 1 deletion(-)
> > create mode 100644 c/src/lib/libbsp/arm/beagle/
> getentropy/bbb_getentropy.c
> >
> > diff --git a/bsps/arm/include/libcpu/am335x.h
> > b/bsps/arm/include/libcpu/am335x.h
> > index 367e97c..92af6dc 100644
> > --- a/bsps/arm/include/libcpu/am335x.h
> > +++ b/bsps/arm/include/libcpu/am335x.h
> > @@ -14,6 +14,9 @@
> > * Modified by Ben Gras <beng at shrike-systems.com
> > <mailto:beng at shrike-systems.com>> to add lots
> > * of beagleboard/beaglebone definitions, delete lpc32xx specific
> > * ones, and merge with some other header files.
> > + *
> > + * Modified by Udit agarwal <dev.madaari at gmail.com
> > <mailto:dev.madaari at gmail.com>> to add random
> > + * number generating module definitions along with register structure.
> > */
> >
> > #if !defined(_AM335X_H_)
> > @@ -701,4 +704,33 @@
> > #define AM335X_CM_PER_OCPWP_L3_CLKSTCTRL_CLKACTIVITY_OCPWP_L4_GCLK
> > (0x00000020u)
> > #define AM335X_I2C_INT_STOP_CONDITION AM335X_I2C_IRQSTATUS_BF
> >
> > -#endif
> > +/* TRNG Register */
> > +
> > +/* RNG base address */
> > +#define RNG_BASE 0x48310000
> > +/* RNG clock control */
> > +#define CM_PER_RNG_CLKCTRL (AM335X_CM_PER_ADDR | (9 << 4))
> > +/* rng module clock status bits */
> > +#define AM335X_CLK_RNG_BIT_MASK (0x30000)
> > +/* Offset from RNG base for output ready flag */
> > +#define RNG_STATUS_RDY (1u << 0)
> > +/* Offset from RNG base for FRO related error */
> > +#define RNG_STATUS_ERR (1u << 1)
> > +/* Offset from RNG base for clock status */
> > +#define RNG_STATUS_CLK (1u << 31)
> > +/* enable module */
> > +#define AM335X_RNG_ENABLE (1 << 10)
> > +
> > +/* TRNG register structure */
> > +struct bbb_trng_register
> > +{
> > + uint64_t output; /*00*/
> > + uint32_t status; /*08*/
> > + uint32_t irq_en; /*0c*/
> > + uint32_t status_clr; /*10*/
> > + uint32_t control; /*14*/
> > + uint32_t config; /*18*/
> > +};
> > +typedef struct bbb_trng_register bbb_trng_register;
> > +
> > +#endif
> > \ No newline at end of file
> > diff --git a/c/src/lib/libbsp/arm/beagle/Makefile.am
> > b/c/src/lib/libbsp/arm/beagle/Makefile.am
> > index 8251660..bf92081 100644
> > --- a/c/src/lib/libbsp/arm/beagle/Makefile.am
> > +++ b/c/src/lib/libbsp/arm/beagle/Makefile.am
> > @@ -88,9 +88,13 @@ libbsp_a_SOURCES += gpio/bbb-gpio.c
> > #pwm
> > libbsp_a_SOURCES += pwm/pwm.c
> >
> > +#getentropy
> > +libbsp_a_SOURCES += getentropy/bbb_getentropy.c
> > +
>
> Adding that one is good. But please also remove the default
> implementation from the Makefile.am:
>
> libbsp_a_SOURCES += ../../shared/getentropy-cpucounter.c
>
> Otherwise you might get problems during linking.
>
> > #RTC
> > libbsp_a_SOURCES += rtc.c
> > libbsp_a_SOURCES += ../../shared/tod.c
> > +
>
> You added a empty line in a section where you didn't change anything
> else. Normally that's not a good idea because it slightly obfuscates the
> history. Try to generate patches that only touch the area that is
> related to your change. If you really think that this line should be
> added, you should create a second patch with a description like "fix
> spacing" or similar. But I wouldn't suggest to do that.
>
> > # Clock
> > libbsp_a_SOURCES += clock.c
> > libbsp_a_SOURCES += ../../shared/clockdrv_shell.h
> > diff --git a/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
> > b/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
> > new file mode 100644
> > index 0000000..5b7c5d0
> > --- /dev/null
> > +++ b/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
> > @@ -0,0 +1,88 @@
> > +/**
> > +* @file
> > +*
> > +* @ingroup arm_beagle
> > +*
> > +* @brief Getentropy implementation on BeagleBone Black BSP
> > +*/
> > +
> > +/*
> > +* Copyright (c) 2018 Udit kumar agarwal <dev.madaari at gmail.com
> > <http://gmail.com>>
> > +*
> > +* The license and distribution terms for this file may be
> > +* found in the file LICENSE in this distribution or at
> > +* http://www.rtems.org/license/LICENSE.
> > +*/
> > +
> > +#include <libcpu/am335x.h>
> > +#include <unistd.h>
> > +#include <string.h>
> > +#include <rtems/sysinit.h>
> > +#include <rtems/types.h>
> > +
> > +/* max refill 34 * 256 cycles */
> > +#define AM335X_RNG_MAX_REFILL (34 << 16)
> > +/* min refill 33 * 64 cycles */
> > +#define AM335X_RNG_MIN_REFILL (33 << 0)
> > +/* startup 33 * 256 cycles */
> > +#define AM335X_RNG_STARTUP_CYCLES (33 << 16)
> > +
> > +/* maximun and minimum refill cycle sets the number of samples to be
> taken
> > + from FRO to generate random number */
> > +static void am335x_rng_enable(bbb_trng_register *rng)
> > +{
> > + rng->control = rng->config = 0;
> > + rng->config |= AM335X_RNG_MIN_REFILL | AM335X_RNG_MAX_REFILL ;
> > + rng->control |= AM335X_RNG_STARTUP_CYCLES | AM335X_RNG_ENABLE ;
> > +}
> > +
> > +static void am335x_rng_clock_enable(void)
> > +{
> > + *(volatile uint8_t *) CM_PER_RNG_CLKCTRL = 2;
> > + while( *(volatile uint32_t *) CM_PER_RNG_CLKCTRL &
> > AM335X_CLK_RNG_BIT_MASK ) {}
>
> Is this line within 80 characters? The RTEMS style uses lines <= 80 chars.
>
> When I'm already at style: Although it's not strictly enforced in BSPs,
> RTEMS uses two spaces for indentation. See
> https://devel.rtems.org/wiki/Developer/Coding/Conventions
>
> Also I don't think we have a style rule for that I would suggest that
> you mark it if you have an intentionally empty loop. That makes it clear
> that you didn't just forget to write some code. E.g.
>
> while( some condition ) {
> /* wait */
> }
>
> > +}
> > +
> > +static uint64_t trng_getranddata(bbb_trng_register *rng)
> > +{
> > + uint64_t output = rng->output;
> > + return output;
> > +}
> > +
> > +int getentropy(void *ptr, size_t n)
> > +{
> > + volatile const bbb_trng_register *rng = (bbb_trng_register*)
> RNG_BASE;
> > + am335x_rng_enable(rng);
> > + while (n > 0)
> > + {
> > + uint64_t random;
> > + size_t copy;
> > +
> > + /* wait untill RNG becomes ready with next set of random data */
> > + while( ( rng->status & RNG_STATUS_RDY ) == 0 );
>
> Again: I would suggest to mark it that this loop is intentionally empty.
>
> > +
> > + random = trng_getranddata(rng);
> > +
> > + /* Checking for error by masking all bits other then error bit
> > in status register */
>
> <= 80 Chars?
>
> > + if( ((rng->status & RNG_STATUS_ERR)>>1) == 0)
> > + {
> > + /* clear the status flag after reading to generate new
> > random value*/
>
> <= 80 Chars?
>
> > + rng->status_clr = RNG_STATUS_RDY;
> > + copy = sizeof(random);
> > + if ( n < copy )
> > + {
> > + copy = n;
> > + }
> > + memcpy(ptr, &random, copy);
> > + n -= copy;
> > + ptr = (char*)ptr + copy;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +RTEMS_SYSINIT_ITEM(
> > + am335x_rng_clock_enable,
> > + RTEMS_SYSINIT_DEVICE_DRIVERS,
> > + RTEMS_SYSINIT_ORDER_LAST
> > +);
> > \ No newline at end of file
> > --
> > 1.9.1
> >
> > Regards,
> > Udit agarwal
> >
> > On Tue, Mar 6, 2018 at 11:05 PM, Gedare Bloom <gedare at rtems.org
> > <mailto:gedare at rtems.org>> wrote:
> >
> > On Tue, Mar 6, 2018 at 9:43 AM, Udit agarwal <dev.madaari at gmail.com
> > <mailto:dev.madaari at gmail.com>> wrote:
> > > Hi,
> > > Here's the updated code(I have also attached the patch PFA):
> > >
> > > From 96e6e1bfd8cffeef5d309eb0a07fe2bfd086ef0a Mon Sep 17 00:00:00
> 2001
> > > From: Udit agarwal <dev.madaari at gmail.com
> > <mailto:dev.madaari at gmail.com>>
> > > Date: Tue, 6 Mar 2018 20:07:44 +0530
> > > Subject: [PATCH] Added getentropy support to BBB BSP
> > >
> > > ---
> > > bsps/arm/beagle/headers.am <http://headers.am>
> > | 1 +
> > > bsps/arm/beagle/include/bsp/bbb_getentropy.h | 57
> > +++++++++++++++
> > > bsps/arm/include/libcpu/am335x.h | 16 +++++
> > > c/src/lib/libbsp/arm/beagle/Makefile.am | 4 ++
> > > .../libbsp/arm/beagle/getentropy/bbb_getentropy.c | 80
> > > ++++++++++++++++++++++
> > > 5 files changed, 158 insertions(+)
> > > create mode 100644 bsps/arm/beagle/include/bsp/bbb_getentropy.h
> > > create mode 100644
> > c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
> > >
> > > diff --git a/bsps/arm/beagle/headers.am <http://headers.am>
> > b/bsps/arm/beagle/headers.am <http://headers.am>
> > > index 6692d0b..ac4ff7c 100644
> > > --- a/bsps/arm/beagle/headers.am <http://headers.am>
> > > +++ b/bsps/arm/beagle/headers.am <http://headers.am>
> > > @@ -12,3 +12,4 @@ include_bsp_HEADERS +=
> > > ../../../../../../bsps/arm/beagle/include/bsp/bbb-pwm.h
> > > include_bsp_HEADERS +=
> > > ../../../../../../bsps/arm/beagle/include/bsp/beagleboneblack.h
> > > include_bsp_HEADERS +=
> > ../../../../../../bsps/arm/beagle/include/bsp/i2c.h
> > > include_bsp_HEADERS +=
> > ../../../../../../bsps/arm/beagle/include/bsp/irq.h
> > > +include_bsp_HEADERS +=
> > > ../../../../../../bsps/arm/beagle/include/bsp/bbb_getentropy.h
> > > diff --git a/bsps/arm/beagle/include/bsp/bbb_getentropy.h
> > > b/bsps/arm/beagle/include/bsp/bbb_getentropy.h
> > > new file mode 100644
> > > index 0000000..5e46f89
> > > --- /dev/null
> > > +++ b/bsps/arm/beagle/include/bsp/bbb_getentropy.h
> > > @@ -0,0 +1,57 @@
> > > +/**
> > > + * @file
> > > + *
> > > + * @ingroup arm_beagle
> > > + *
> > > + * @brief TRNG Definations
> > > + */
> > > +
> > > +/*
> > > + * Copyright (c) 2018 Udit kumar agarwal <dev.madaari at
> > gmail.com <http://gmail.com>>
> > > + *
> > > + * The license and distribution terms for this file may be
> > > + * found in the file LICENSE in this distribution or at
> > > + * http://www.rtems.org/license/LICENSE
> > <http://www.rtems.org/license/LICENSE>.
> > > + */
> > > +
> > > +
> > > +#ifndef _TRNG_
> > > +#define _TRNG_
> > > +
> > >
> > +/*---------------------------------------------------------
> ---------------------
> > Please look at similar files in RTEMS for how to add comments.
> > We do not use this style /* --------------------
> > And we don't put this kind of comment about the file sections.
> >
> > > + * Headers
> > > +
> > > *-----------------------------------------------------------
> -----------------*/
> > > +
> > > +#include <libcpu/am335x.h>
> > > +
> > > +
> > > /*----------------------------------------------------------
> --------------------
> > > + * Register structure
> > > +
> > > *-----------------------------------------------------------
> -----------------*/
> > > +
> > > +struct rng {
> > > + uint64_t output; /*00*/
> > > + uint32_t status; /*08*/
> > > + uint32_t irq_en; /*0c*/
> > > + uint32_t status_clr; /*10*/
> > > + uint32_t control; /*14*/
> > > + uint32_t config; /*18*/
> > > +};
> > for an exported header, the struct namespace needs to be user
> > friendly. See similar files at this layer for how to name things.
> > Probably, this can be defined in the am335x.h header?
> >
> > > +
> > > +typedef volatile struct rng *rng_ref_t;
> > I'm not such a big fan of using the typedef to hide things like
> > volatile or pointer notation. I don't see a need for this usually.
> >
> > > +
> > > +/*---------------------------------------------------------
> -------------------*/
> > > +/* Exported functions
> > > */
> > > +/*---------------------------------------------------------
> -------------------*/
> > > +
> > > +/* configure and enable RNG module */
> > > +static void am335x_rng_enable(rng_ref_t);
> > > +/* enable module clock for random number generator */
> > > +static void am335x_rng_clock_enable(void);
> > > +/* outputs random data */
> > > +static uint64_t trng_getranddata(rng_ref_t);
> >
> > static keyword implies these are not exported.
> >
> > > +/* Copy random data of a specified size to the pointer supplied */
> > > +int getentropy(void *ptr, size_t);
> > I guess getentropy() is already exported by a different header file
> > (unistd.h) in libc? This doesn't belong here.
> >
> > I suspect this header file is not actually necessary.
> >
> > > +
> > > +
> > > +
> > Don't use multiple blank lines in a row. Only one empty line is ever
> > needed by RTEMS style.
> >
> > > +#endif /* #ifndef _TRNG_ */
> > > \ No newline at end of file
> > > diff --git a/bsps/arm/include/libcpu/am335x.h
> > > b/bsps/arm/include/libcpu/am335x.h
> > > index 367e97c..8b58f1a 100644
> > > --- a/bsps/arm/include/libcpu/am335x.h
> > > +++ b/bsps/arm/include/libcpu/am335x.h
> > > @@ -14,6 +14,9 @@
> > > * Modified by Ben Gras <beng at shrike-systems.com <mailto:
> beng at shrike-systems.com>> to add lots
> > > * of beagleboard/beaglebone definitions, delete lpc32xx specific
> > > * ones, and merge with some other header files.
> > > + *
> > > + * Modified by Udit agarwal <dev.madaari at gmail.com <mailto:
> dev.madaari at gmail.com>> to add random
> > > + * number generating module definations
> > typo: definitions
> > add a period (full stop).
> >
> > > */
> > >
> > > #if !defined(_AM335X_H_)
> > > @@ -701,4 +704,17 @@
> > > #define AM335X_CM_PER_OCPWP_L3_CLKSTCTRL_CLKACTIVITY_OCPWP_
> L4_GCLK
> > > (0x00000020u)
> > > #define AM335X_I2C_INT_STOP_CONDITION AM335X_I2C_IRQSTATUS_BF
> > >
> > > +/* TRNG Register */
> > > +
> > > +/* RNG base address */
> > > +#define RNG_BASE 0x48310000
> > > +/* RNG clock control */
> > > +#define CM_PER_RNG_CLKCTRL AM335X_CM_PER_ADDR | 9<<4
> > I'd put parentheses around this to avoid any problems with macro
> > expansions and order of operations.
> >
> > > +/* Offset from RNG base for output ready flag */
> > > +#define RNG_STATUS_RDY (1u << 0)
> > > +/* Offset from RNG base for FRO related error*/
> > > +#define RNG_STATUS_ERR (1u << 1)
> > > +/* Offset from RNG base for clock status */
> > > +#define RNG_STATUS_CLK (1u << 31)
> > > +
> > > #endif
> > > diff --git a/c/src/lib/libbsp/arm/beagle/Makefile.am
> > > b/c/src/lib/libbsp/arm/beagle/Makefile.am
> > > index 8251660..bf92081 100644
> > > --- a/c/src/lib/libbsp/arm/beagle/Makefile.am
> > > +++ b/c/src/lib/libbsp/arm/beagle/Makefile.am
> > > @@ -88,9 +88,13 @@ libbsp_a_SOURCES += gpio/bbb-gpio.c
> > > #pwm
> > > libbsp_a_SOURCES += pwm/pwm.c
> > >
> > > +#getentropy
> > > +libbsp_a_SOURCES += getentropy/bbb_getentropy.c
> > > +
> > > #RTC
> > > libbsp_a_SOURCES += rtc.c
> > > libbsp_a_SOURCES += ../../shared/tod.c
> > > +
> > > # Clock
> > > libbsp_a_SOURCES += clock.c
> > > libbsp_a_SOURCES += ../../shared/clockdrv_shell.h
> > > diff --git a/c/src/lib/libbsp/arm/beagle/
> getentropy/bbb_getentropy.c
> > > b/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
> > > new file mode 100644
> > > index 0000000..cab2102
> > > --- /dev/null
> > > +++ b/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
> > > @@ -0,0 +1,80 @@
> > > +/**
> > > +* @file
> > > +*
> > > +* @ingroup arm_beagle
> > > +*
> > > +* @brief Getentropy implementation on BeagleBone Black BSP
> > > +*/
> > > +
> > > +/*
> > > +* Copyright (c) 2018 Udit kumar agarwal <dev.madaari at gmail.com
> > <http://gmail.com>>
> > > +*
> > > +* The license and distribution terms for this file may be
> > > +* found in the file LICENSE in this distribution or at
> > > +* http://www.rtems.org/license/LICENSE
> > <http://www.rtems.org/license/LICENSE>.
> > > +*/
> > > +
> > > +#include <unistd.h>
> > > +#include <stdint.h>
> > > +#include <string.h>
> > > +#include <rtems/sysinit.h>
> > > +
> > > +
> > > +/* maximun and minimun refill cycle sets the number of samples to
> > be taken
> > typo: minimum
> > > + from FRO to generate random number */
> > > +static void am335x_rng_enable(rng_ref_t rng){
> > put { on a new line.
> >
> > > + rng->config = 0
> > > + | 34 << 16 /* max refill 34 * 256 cycles */
> > indent broken lines, but
> > > + | 33 << 0; /* min refill 33 * 64 cycles */
> > You can fit this on one line, remove the comments and put them above
> > the line, e.g.
> > rng->config = 34<<16 | 33;
> >
> > I would prefer not to have these hard-coded values though it is
> > fragile. what means 34, 16, and 33?
> > #define AM335X_RNG_MAX_REFILL (34 << 16)
> > #define AM335X_RNG_MIN_REFILL (33)
> > #define AM335X_RNG_CONFIG_REFILL (AM335X_RNG_MAX_REFILL |
> > AM335X_RNG_MIN_REFILL)
> >
> > rng.config = rng.control = 0;
> > rng.config |= AM335X_RNG_CONFIG_REFILL_TIME;
> >
> > > + rng->control = 0
> > > + | 33 << 16 /* startup 33 * 256 cycles */
> > > + | 1 << 10; /* enable module */
> >
> > rng.control |= AM335X_RNG_CONTROL_STARTUP;
> > rng.control |= AM335X_RNG_CONTROL_ENABLE;
> >
> > for example is more readable. look around for other examples.
> >
> > > +}
> > > +
> > > +static void am335x_rng_clock_enable(void)
> > > +{
> > > + *(volatile uint8_t *) CM_PER_RNG_CLKCTRL = 2;
> > > + while( *(volatile uint32_t *) CM_PER_RNG_CLKCTRL & 0x30000 )
> {}
> > > +}
> > > +
> > > +static uint64_t trng_getranddata(rng_ref_t rng){
> > > + uint64_t output = rng->output;
> > > + return output;
> > > +}
> > > +
> > > +int getentropy(void *ptr, size_t n)
> > > +{
> > > + rng_ref_t const rng = (rng_ref_t) RNG_BASE;
> > > + am335x_rng_enable(rng);
> > > + while (n > 0) {
> > > + uint64_t random;
> > indent blocks of code.
> >
> > > + size_t copy;
> > > +
> > > + /* wait untill RNG becomes ready with next set of random data
> */
> > > + while( ( rng->status & RNG_STATUS_RDY ) == 0 );
> > > +
> > > + random = trng_getranddata(rng);
> > > +
> > > + /* Checking for error by masking all bits other then error
> bit in
> > > status register */
> > > + if( ((rng->status & RNG_STATUS_ERR)>>1) == 0){
> > > +
> > > + /* clear the status flag after reading to generate new
> random
> > > value*/
> > > + rng->status_clr = RNG_STATUS_RDY;
> > > + copy = sizeof(random);
> > > + if ( n < copy ) {
> > > + copy = n;
> > ditto.
> >
> > > + }
> > > + memcpy(ptr, &random, copy);
> > > + n -= copy;
> > > + ptr += copy;
> > pointer arithmetic on void pointer is not legal. You can portably use
> > ptr = (char*)ptr + copy;
> >
> > > + }
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +RTEMS_SYSINIT_ITEM(
> > > + am335x_rng_clock_enable,
> > > + RTEMS_SYSINIT_DEVICE_DRIVERS,
> > > + RTEMS_SYSINIT_ORDER_LAST
> > > +);
> > > \ No newline at end of file
> > > --
> > > 1.9.1
> > >
> > > Regards,
> > > Udit Agarwal
> > >
> > > On Tue, Mar 6, 2018 at 12:50 AM, Christian Mauderer
> > <list at c-mauderer.de <mailto:list at c-mauderer.de>>
> > > wrote:
> > >>
> > >> Am 05.03.2018 um 14:51 schrieb Udit agarwal:
> > >> > Hi,
> > >> > I tried implementing getentropy on BBB, below is the patch.
> > Please have
> > >> > a look.
> > >> > I followed these(1
> > >> > <https://e2e.ti.com/support/arm/sitara_arm/f/791/t/355064
> > <https://e2e.ti.com/support/arm/sitara_arm/f/791/t/355064>> & 2
> > >> >
> > >> >
> > <http://ftp.netbsd.org/pub/NetBSD/NetBSD-current/src/sys/
> arch/arm/omap/am335x_trng.c
> > <http://ftp.netbsd.org/pub/NetBSD/NetBSD-current/src/sys/
> arch/arm/omap/am335x_trng.c>>)
> > >> > links for code reference. and this
> > >> >
> > >> >
> > <https://docs.google.com/spreadsheets/d/
> 1VpghMlLtrWQIrcvCsRg3ueRZkX0eGSQkFnzjfsqrd8A/view#gid=0
> > <https://docs.google.com/spreadsheets/d/
> 1VpghMlLtrWQIrcvCsRg3ueRZkX0eGSQkFnzjfsqrd8A/view#gid=0>>
> > >> > for register reference. Moreover, what further configuration in
> > RTEMS is
> > >> > needed to execute and test this code along with getentropy()
> sample
> > >> > testcode?
> > >>
> > >> Hello Udit,
> > >>
> > >> regarding the patch: Please use 'git format-patch' for generating
> > your
> > >> patches and send them unchanged to the mailing list (for example
> with
> > >> 'git send-email'. If you follow this workflow, it's easier to
> > apply and
> > >> test the patches. The preferred workflow for RTEMS is described
> here:
> > >> https://devel.rtems.org/wiki/Developer/Git/Users#CreatingaPatch
> > <https://devel.rtems.org/wiki/Developer/Git/Users#CreatingaPatch>
> > >>
> > >> Regarding documentation: It seems that there is really only few
> > >> documentation regarding the crypto part available. The FreeBSD
> > driver is
> > >> most likely your best source.
> > >>
> > >> I added further comments down in your code.
> > >>
> > >> Best regards
> > >>
> > >> Christian
> > >>
> > >> >
> > >> >
> > >> > + /*
> > >> > + * Copyright (c) 2018 embedded brains GmbH. All rights
> reserved.
> > >> > + *
> > >> > + * embedded brains GmbH
> > >> > + * Dornierstr. 4
> > >> > + * 82178 Puchheim
> > >> > + * Germany
> > >> > + * <rtems at embedded-brains.de
> > <mailto:rtems at embedded-brains.de> <mailto:rtems at embedded-brains.de
> > <mailto:rtems at embedded-brains.de>>>
> > >> > + *
> > >>
> > >> Also it's nice if you add an embedded brains header, I don't
> > think that
> > >> I've seen you when I have been at work today. So please add your
> own
> > >> name to the copyright line instead. With that you can even point
> > to that
> > >> file and tell everyone that it's from you.
> > >>
> > >> An address isn't really necessary except if you want it for some
> > reason
> > >> (like PR in case of a company).
> > >>
> > >> > + * The license and distribution terms for this file may be
> > >> > + * found in the file LICENSE in this distribution or at
> > >> > + * http://www.rtems.org/license/LICENSE
> > <http://www.rtems.org/license/LICENSE>.
> > >> > + */
> > >> > +
> > >> > + #include <libcpu/am335x.h>
> > >> > + #include <unistd.h>
> > >> > + #include <string.h>
> > >> > + #include <rtems/sysinit.h>
> > >> > +
> > >> > + #define RNG_BASE 0x48310000
> > >> > + #define CM_PER_RNG_CLKCTRL 0x44e00090
> > >> > + #define RNG_STATUS_RDY (1u << 0) // output ready for
> reading
> > >> > + #define RNG_STATUS_ERR (1u << 1) // FRO shutdown alarm
> > >> > + #define RNG_STATUS_CLK (1u << 31) // module functional
> clock
> > >> > active (no irq)
> > >>
> > >> Please add address definitions and the bit masks to the
> > >> bsps/arm/include/libcpu/am335x.h instead of defining them here
> > locally.
> > >> Note that for the CM_PER_RNG_CLKCTRL, there is already a
> > >> AM335X_CM_PER_ADDR that should be used as a base.
> > >>
> > >> > +
> > >> > + typedef volatile struct rng *rng_ref_t;
> > >> > +
> > >> > + struct rng {
> > >> > + /*00*/ uint64_t output; //r-
> > >> > + /*08*/ uint32_t status; //r-
> > >> > + /*0c*/ uint32_t irq_en; //rw
> > >> > + /*10*/ uint32_t status_clr; //-c
> > >> > + /*14*/ uint32_t control; //rw
> > >> > + /*18*/ uint32_t config; //rw
> > >> > + };
> > >>
> > >> Same is true here: It's a structure describing the registers. So
> it
> > >> should be in the header instead.
> > >>
> > >> It seems that you copied that from the forum post. Please always
> > check
> > >> the license when you copy code. We have to avoid that we get any
> code
> > >> into RTEMS that isn't compatible with RTEMS license.
> > >>
> > >> In this case the code should be obvious enough that - if you just
> > >> rewrite it in RTEMS style, there shouldn't be a problem. But just
> > >> remember that licensing is always a difficult topic.
> > >>
> > >> > +
> > >> > + static void TRNG_Enable(rng_ref_t rng){
> > >> > + rng->config = 0
> > >> > + | 34 << 16 // max refill 34 * 256 cycles
> > >> > + | 33 << 0; // min refill 33 * 64 cycles
> > >> > + rng->control = 0
> > >> > + | 33 << 16 // startup 33 * 256 cycles
> > >> > + | 1 << 10; // enable module
> > >> > + }
> > >>
> > >> Please use C-Style comments /* */ in all pure C code (most of
> RTEMS).
> > >>
> > >> > +
> > >> > + static void beagle_trng_clock_enable(void)
> > >> > + {
> > >> > + // enable module clock
> > >> > + *(volatile uint8_t *) CM_PER_RNG_CLKCTRL = 2;
> > >> > + while( *(volatile uint32_t *) CM_PER_RNG_CLKCTRL &
> > 0x30000 ) {}
> > >> > + }
> > >> > +
> > >> > + static uint64_t TRNG_GetRandData(){
> > >> > + uint64_t output = rng->output;
> > >>
> > >> 'rng' isn't defined here.
> > >>
> > >> > + return output;
> > >> > + }
> > >> > +
> > >> > + int getentropy(void *ptr, size_t n)
> > >> > + {
> > >> > + rng_ref_t const rng = (rng_ref_t) RNG_BASE;
> > >> > + TRNG_Enable(rng);
> > >> > + while (n > 0) {
> > >> > + uint64_t random;
> > >> > + size_t copy;
> > >> > +
> > >> > + while( ( rng->status & RNG_STATUS_RDY ) == 0 ); //wait
> > >>
> > >> You should check specifically for the TRNG_STATUS_READY flag here
> > like
> > >> done in FreeBSD. I'm not sure what the other flags are but it's
> quite
> > >> possible that there is also some error flag or similar. In that
> > case it
> > >> would not be a good idea to use the random number.
> > >>
> > >> > +
> > >> > + random = TRNG_GetRandData(rng);
> > >>
> > >> Note that you don't have to copy the function names from the atsam
> > >> exactly. In the atsam BSP, the library has provided these names.
> > >> Otherwise it's quite unusual to start functions with upper case
> > in RTEMS.
> > >>
> > >> > +
> > >> > + /*
> > >> > + * Read TRNG status one more time to avoid race
> > condition.
> > >> > + * Otherwise we can read (and clear) an old ready
> > status but
> > >> > get
> > >> > + * a new value. The ready status for this value
> > wouldn't be
> > >> > + * reset.
> > >> > + */
> > >>
> > >> That comment seems like it doesn't fit here any longer. You
> > acknowledge
> > >> the status instead in your next line. Just remove the comment.
> > >>
> > >> > + rng->status_clr = RNG_STATUS_RDY;
> > >> > +
> > >> > + copy = sizeof(random);
> > >> > + if (n < copy ) {
> > >> > + copy = n;
> > >> > + }
> > >> > + memcpy(ptr, &random, copy);
> > >> > + n -= copy;
> > >> > + ptr += copy;
> > >> > + }
> > >> > +
> > >> > + return 0;
> > >> > + }
> > >> > +
> > >> > + RTEMS_SYSINIT_ITEM(
> > >> > + beagle_trng_clock_enable,
> > >> > + RTEMS_SYSINIT_DEVICE_DRIVERS,
> > >> > + RTEMS_SYSINIT_ORDER_LAST
> > >> > + );
> > >> >
> > >> > Regards,
> > >> > Udit agarwal
> > >> >
> > >> >
> > >> > _______________________________________________
> > >> > devel mailing list
> > >> > devel at rtems.org <mailto:devel at rtems.org>
> > >> > http://lists.rtems.org/mailman/listinfo/devel
> > <http://lists.rtems.org/mailman/listinfo/devel>
> > >> >
> > >
> > >
> > >
> > > _______________________________________________
> > > devel mailing list
> > > devel at rtems.org <mailto:devel at rtems.org>
> > > http://lists.rtems.org/mailman/listinfo/devel
> > <http://lists.rtems.org/mailman/listinfo/devel>
> >
> >
> >
> >
> > _______________________________________________
> > devel mailing list
> > devel at rtems.org
> > http://lists.rtems.org/mailman/listinfo/devel
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20180311/3ec89a15/attachment-0002.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Added-getentropy-support-to-BBB-BSP.patch
Type: text/x-patch
Size: 6703 bytes
Desc: not available
URL: <http://lists.rtems.org/pipermail/devel/attachments/20180311/3ec89a15/attachment-0002.bin>
More information about the devel
mailing list