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.mada...@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 <b...@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.mada...@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 <l...@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.mada...@gmail.com <mailto:dev.mada...@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 <b...@shrike-systems.com > > <mailto:b...@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.mada...@gmail.com > > <mailto:dev.mada...@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 <ged...@rtems.org > > <mailto:ged...@rtems.org>> wrote: > > > > On Tue, Mar 6, 2018 at 9:43 AM, Udit agarwal <dev.mada...@gmail.com > > <mailto:dev.mada...@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.mada...@gmail.com > > <mailto:dev.mada...@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 <b...@shrike-systems.com <mailto: > b...@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.mada...@gmail.com <mailto: > dev.mada...@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 > > <l...@c-mauderer.de <mailto:l...@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 > > >> > + * <rt...@embedded-brains.de > > <mailto:rt...@embedded-brains.de> <mailto:rt...@embedded-brains.de > > <mailto:rt...@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@rtems.org <mailto:devel@rtems.org> > > >> > http://lists.rtems.org/mailman/listinfo/devel > > <http://lists.rtems.org/mailman/listinfo/devel> > > >> > > > > > > > > > > > > > _______________________________________________ > > > devel mailing list > > > devel@rtems.org <mailto:devel@rtems.org> > > > http://lists.rtems.org/mailman/listinfo/devel > > <http://lists.rtems.org/mailman/listinfo/devel> > > > > > > > > > > _______________________________________________ > > devel mailing list > > devel@rtems.org > > http://lists.rtems.org/mailman/listinfo/devel > > >
From f87b39708dd06482957fc7b33e78ab0ee095287b Mon Sep 17 00:00:00 2001 From: Udit agarwal <dev.mada...@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 <b...@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.mada...@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
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel