Adding CC310 entropy driver utilizing nrf_cc310_platform library #1268

frkv · 2019-10-09T16:17:57Z

-Adding hw_cc310 driver with RTOS mutex/abort support
-Changing entropy_cc310 driver to use nrf_cc310_platform library
-Changing entropy_cc310 driver to support non-secure application

Added description for:

cc310 entropy driver
cc310 hw initialization

Signed-off-by: Frank Audun Kvamtrø [email protected]
Signed-off-by: Torsten Rasmussen [email protected]

NordicBuilder · 2019-10-09T16:21:17Z

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

-Adding hw_cc310 driver with RTOS mutex/abort support -Changing entropy_cc310 driver to use nrf_cc310_platform library -Changing entropy_cc310 driver to support non-secure application Signed-off-by: Frank Audun Kvamtrø <[email protected]> Signed-off-by: Torsten Rasmussen <[email protected]>

The Kconfig in zephyr/subsys/bluetooth/controller/Kconfig selects ENTROPY_NRF5_RNG when building for an nRF SoC. This collides with the alternative cc310 entropy driver causing conflicts. Therefore the cc310 entropy driver will be dependent on BT_LL_SW_LEGACY not being set. Signed-off-by: Torsten Rasmussen <[email protected]>

When building multi image including mcuboot then mcuboot is compiled with hard float, however the flags CONFIG_FLOAT and CONFIG_FP_HARDABI are not set which causes wrong linking. Therefore hw cc310 will default to yes in mcuboot until this has been corrected. This is possible, as cc310 is currently not required in mcuboot. Signed-off-by: Torsten Rasmussen <[email protected]>

pdunaj · 2019-10-28T09:46:37Z

Hi @frkv , can you tell why it is disabled for desktop? Is there any reason we would like to have it enabled?

tejlmand · 2019-10-28T10:14:10Z

Hi @frkv , can you tell why it is disabled for desktop? Is there any reason we would like to have it enabled?

When enabling cc310 on nrf desktop, we face the following issue:

Found 12 asserts!
desktop52_base_test Line:223  Assert raised, [00000003] <err> os: ***** MPU FAULT *****
desktop52_base_test Line:223  Assert raised, [00000003] <err> os:   Stacking error (context area might be not valid)
desktop52_base_test Line:223  Assert raised, [00000003] <err> os:   Data Access Violation
desktop52_base_test Line:223  Assert raised, [00000003] <err> os:   MMFAR Address: 0x2000829c
desktop52_base_test Line:223  Assert raised, [00000004] <err> os: r0/a1:  0x00000000  r1/a2:  0x0002be03  r2/a3:  0x00000000
desktop52_base_test Line:223  Assert raised, [00000004] <err> os: r3/a4:  0x0002be03 r12/ip:  0x0004134a r14/lr:  0x01000000
desktop52_base_test Line:223  Assert raised, [00000004] <err> os:  xpsr:  0x20001cbc
desktop52_base_test Line:223  Assert raised, [00000004] <err> os: Faulting instruction address (r15/pc): 0x2000cd38
desktop52_base_test Line:223  Assert raised, [00000004] <err> os: >>> ZEPHYR FATAL ERROR 2: Stack overflow
desktop52_base_test Line:223  Assert raised, [00000004] <err> os: >>> ZEPHYR FATAL ERROR 2: Stack overflow
desktop52_base_test Line:223  Assert raised, [00000005] <err> os: Current thread: 0x20005734 (unknown)
desktop52_base_test Line:223  Assert raised, [00328145] <err> os: Halting system

which indicates that MPU must be configured differently.

But as first step, the cc310 is disabled in nrf desktop, as this should be identical to how nrf desktop is working without this PR.
Then it allows time to look into the MPU configuration aftwerwards.

pdunaj · 2019-10-28T10:35:25Z

Hi @tejlmand , nrf_desktop uses the default MPU configuration from Zephyr. What we use however in the debug build is stack guard (CONFIG_MPU_STACK_GUARD). Note that this may be something that others will use as well so it would be good if changes introduced are compatible with this feature. Or if the feature is not compatible it would be good to set your config to depend on !MPU_STACK_GUARD.

Pinging @ioannisg .

tejlmand · 2019-10-28T11:01:53Z

Hi @tejlmand , nrf_desktop uses the default MPU configuration from Zephyr. What we use however in the debug build is stack guard (CONFIG_MPU_STACK_GUARD). Note that this may be something that others will use as well so it would be good if changes introduced are compatible with this feature. Or if the feature is not compatible it would be good to set your config to depend on !MPU_STACK_GUARD.

Thanks for info.
Has changed to MPU_STACK_GUARD dependency.
Of course this must work with the MPU in future, but this allows for introducing the entropy driver through SPM as a first step.

pdunaj · 2019-10-28T11:13:58Z

Hi @tejlmand , nrf_desktop uses the default MPU configuration from Zephyr. What we use however in the debug build is stack guard (CONFIG_MPU_STACK_GUARD). Note that this may be something that others will use as well so it would be good if changes introduced are compatible with this feature. Or if the feature is not compatible it would be good to set your config to depend on !MPU_STACK_GUARD.

Thanks for info.
Has changed to MPU_STACK_GUARD dependency.
Of course this must work with the MPU in future, but this allows for introducing the entropy driver through SPM as a first step.

Thanks. Also is it possible that the feature would not be enabled by default? This is to make sure that currently working configurations will be fine. Apps/samples that need it can enabled it in the config. Multiple things are not enabled by default, Bluetooth included.

tejlmand · 2019-10-28T11:24:23Z

Hi @tejlmand , nrf_desktop uses the default MPU configuration from Zephyr. What we use however in the debug build is stack guard (CONFIG_MPU_STACK_GUARD). Note that this may be something that others will use as well so it would be good if changes introduced are compatible with this feature. Or if the feature is not compatible it would be good to set your config to depend on !MPU_STACK_GUARD.

Thanks for info.
Has changed to MPU_STACK_GUARD dependency.
Of course this must work with the MPU in future, but this allows for introducing the entropy driver through SPM as a first step.

Thanks. Also is it possible that the feature would not be enabled by default? This is to make sure that currently working configurations will be fine. Apps/samples that need it can enabled it in the config. Multiple things are not enabled by default, Bluetooth included.

If a sample requires entropy, then I would argue that we should aim for most secure way of obtaning that, and that would be to use the cc310 when it is available.
Just as you use the nrf5 rng today, because there were no driver for the cc310 entropy.
Note, only samples that sets ENTROPY_GENERATOR will use the cc310 entropy if available, so cc310 entropy is not enabled everyewhere.

lemrey

+1 for lwm2m_carrier changes.

rlubos · 2019-10-28T15:08:40Z

samples/nrf9160/lwm2m_carrier/prj.conf

There are two overlay-carrier.conf files that should be updated as well in mqtt and coap samples.

rlubos · 2019-10-29T07:32:50Z

@b-gent Could have a look at docs in this PR?

b-gent · 2019-10-29T09:09:24Z

If the CI is green, let us merge now and I will start reviewing the docs on my own branch today.

b-gent · 2019-10-29T09:27:22Z

nrf/rst/doc/nrf/drivers.rst:10: WARNING: toctree glob pattern '../../doc/drivers/*' didn't match any documents

@tejlmand

tejlmand · 2019-10-29T09:29:31Z

ivers.rst:10: WARNING:

strange, that worked locally
looking at it.

Added dependency to ensure a true entropy driver is available Signed-off-by: Torsten Rasmussen <[email protected]>

Disable cc310 entropy driver for MPU enabled applications. Signed-off-by: Torsten Rasmussen <[email protected]>

tejlmand · 2019-10-29T10:42:54Z

dox commit dropped, as not needed for this PR.
Will make separate PR with dox content when this is merged

frkv requested a review from anangl as a code owner October 9, 2019 16:17

frkv force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch from d257bdc to 43ac50a Compare October 14, 2019 10:47

frkv requested review from carlescufi, ioannisg and tejlmand as code owners October 14, 2019 10:47

frkv force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch from 43ac50a to 1bc448d Compare October 14, 2019 12:54

tejlmand force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch from 1bc448d to 955efe6 Compare October 24, 2019 05:59

frkv requested a review from ru-fu as a code owner October 24, 2019 05:59

tejlmand mentioned this pull request Oct 24, 2019

Nrf cc310 mbedcrypto version 0.9.0 #1322

Closed

tejlmand added this to the 1.1.0 milestone Oct 24, 2019

tejlmand force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch 3 times, most recently from d877ee9 to a1d5a53 Compare October 24, 2019 06:37

tejlmand added the DNM label Oct 24, 2019

tejlmand force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch from a1d5a53 to 8adb309 Compare October 24, 2019 06:50

ru-fu requested review from b-gent and removed request for ru-fu October 24, 2019 06:51

tejlmand force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch 5 times, most recently from bf3573b to 7fd88cc Compare October 25, 2019 14:56

frkv and others added 3 commits October 25, 2019 18:13

tejlmand force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch from 7fd88cc to 25f7eb2 Compare October 25, 2019 16:13

frkv requested review from lemrey and rlubos as code owners October 25, 2019 18:06

tejlmand force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch from 5d8ab6a to e4ecdef Compare October 25, 2019 18:58

tejlmand force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch 2 times, most recently from 73ddacc to 9dfdea6 Compare October 28, 2019 08:22

frkv requested a review from pdunaj as a code owner October 28, 2019 08:22

tejlmand force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch from 9dfdea6 to 69dd0c1 Compare October 28, 2019 10:58

tejlmand force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch from 69dd0c1 to fd6acdb Compare October 28, 2019 13:02

tejlmand removed the DNM label Oct 28, 2019

tejlmand changed the title ~~[DNM] Adding CC310 entropy driver utilizing nrf_cc310_platform library~~ Adding CC310 entropy driver utilizing nrf_cc310_platform library Oct 28, 2019

tejlmand approved these changes Oct 28, 2019

View reviewed changes

lemrey approved these changes Oct 28, 2019

View reviewed changes

rlubos reviewed Oct 28, 2019

View reviewed changes

tejlmand force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch from fd6acdb to c12f03b Compare October 28, 2019 15:15

tejlmand force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch from c12f03b to 0b1530f Compare October 29, 2019 08:55

b-gent approved these changes Oct 29, 2019

View reviewed changes

tejlmand force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch from 0b1530f to 6dd78a9 Compare October 29, 2019 10:03

tejlmand added 2 commits October 29, 2019 11:23

sample: lwm2m carrier entropy dependency

83e634f

Added dependency to ensure a true entropy driver is available Signed-off-by: Torsten Rasmussen <[email protected]>

entropy: cc310 dependency to mpu settings introduced

47b6a10

Disable cc310 entropy driver for MPU enabled applications. Signed-off-by: Torsten Rasmussen <[email protected]>

tejlmand force-pushed the entropy-for-nrf_cc310_mbedcrypto-v0.9.0 branch from 6dd78a9 to 47b6a10 Compare October 29, 2019 10:23

rlubos merged commit cdb864f into nrfconnect:master Oct 29, 2019

tejlmand mentioned this pull request Oct 29, 2019

nrf_cc310: Added description for cc310 drivers #1349

Merged

Adding CC310 entropy driver utilizing nrf_cc310_platform library #1268

Adding CC310 entropy driver utilizing nrf_cc310_platform library #1268

Uh oh!

Conversation

frkv commented Oct 9, 2019 • edited by tejlmand Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NordicBuilder commented Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pdunaj commented Oct 28, 2019

Uh oh!

tejlmand commented Oct 28, 2019

Uh oh!

pdunaj commented Oct 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tejlmand commented Oct 28, 2019

Uh oh!

pdunaj commented Oct 28, 2019

Uh oh!

tejlmand commented Oct 28, 2019

Uh oh!

lemrey left a comment

Choose a reason for hiding this comment

Uh oh!

rlubos Oct 28, 2019

Choose a reason for hiding this comment

Uh oh!

tejlmand Oct 28, 2019

Choose a reason for hiding this comment

Uh oh!

rlubos commented Oct 29, 2019

Uh oh!

b-gent commented Oct 29, 2019

Uh oh!

b-gent commented Oct 29, 2019

Uh oh!

tejlmand commented Oct 29, 2019

Uh oh!

tejlmand commented Oct 29, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

frkv commented Oct 9, 2019 •

edited by tejlmand

Loading

NordicBuilder commented Oct 9, 2019 •

edited

Loading

pdunaj commented Oct 28, 2019 •

edited

Loading