Registration GR-LYCHEE board as a new mbed board #5857

TomoYamanaka · 2018-01-16T04:57:03Z

Description

I register GR-LYCHEE as a Renesas new Mbed board, based on existing GR-PEACH(RZ/A1H) support.
GR-LYCHEE incorporates RZ/A1LU that is RZ/A1 chip series.

Relation PRs

#5628
#5813

…nd tests.py I added GR-LYCHEE's configuration in targets.json file. Also, I added GR_LYCHEE as a Renesas new target board in build_travis.py and tests.py.

I added GR_LYCHEE as a new target board in DS-5, e2studio and IAR export.

For supporting to CMSIS5/RTX5, I added the start-up processing of 3 toolchains (ARMCC, GCC_ARM, IAR) and the register definition of RZ/A1LU specific. In addition, I added the linker script files to implement the dynamic HEAP the same as GR-PEACH(RZ/A1H).

In mbed_rtx.h file, I added the definition for GR-LYCHEE to use the "Dynamic Heap" processing when the target is GR_LYCHEE.

I supported the TRNG function when target is GR-LYCHEE. GR-LYCHEE generates TRNG by acquiring the random number of Wifi module(ESP32) incorporated in it using I2C.

TomoYamanaka · 2018-01-19T02:51:05Z

How is going? Is there anything to do for this PR?

0xc0170 · 2018-01-19T08:09:28Z

targets/TARGET_RENESAS/TARGET_RZ_A1XX/TARGET_GR_LYCHEE/PeripheralNames.h

+} CANName;
+
+
+#define STDIO_UART_TX     USBTX


Is there a reason why this is not part of PinNames? I noticed this in some platforms but not certain about having a macro instead of in PInNames

I don't know a clear reason. This have been defined in "PeripheralNames.h" for a long time in GR-PEACH, and I created this the same as GR-PEACH.
Many targets is still defined in PeripheralNames.h, should I move to PInNames.h?

It could be moved. I only wondered if there is anything else that I was missing

0xc0170 · 2018-01-19T08:10:30Z

targets/TARGET_RENESAS/TARGET_RZ_A1XX/TARGET_GR_LYCHEE/TARGET_MBED_MBRZA1LU/reserved_pins.h

@@ -0,0 +1,6 @@
+#ifndef RESERVED_PINS_H


Should this contain license header on top?

I modified this content in commit 4bc79b0.

0xc0170 · 2018-01-19T08:13:36Z

targets/TARGET_RENESAS/TARGET_RZ_A1XX/TARGET_GR_LYCHEE/device/cmsis_nvic.c

+#include "MBRZA1LU.h"
+#include "irq_ctrl.h"
+
+void NVIC_SetVector(IRQn_Type IRQn, uint32_t vector) {


{ new line for both functions

I modified this content in commit e8378ef .

0xc0170 · 2018-01-19T08:15:23Z

@hanno-arm please review trng addition (targets/TARGET_RENESAS/TARGET_RZ_A1XX/TARGET_GR_LYCHEE/trng_api_esp32.cpp file)

ciarmcom · 2018-01-19T08:50:21Z

ARM Internal Ref: IOTSSL-2030

I modified the lack of license header in the below header files. - targets/TARGET_RENESAS/TARGET_RZ_A1XX/TARGET_RZ_A1H/TARGET_MBED_MBRZA1H/reserved_pins.h - targets/TARGET_RENESAS/TARGET_RZ_A1XX/TARGET_GR_LYCHEE/TARGET_MBED_MBRZA1LU/reserved_pins.h

Regarding "{" that show the function start, I modified the the arrangement from right of function to new line.

0xc0170 · 2018-01-19T14:21:58Z

targets/TARGET_RENESAS/TARGET_RZ_A1XX/TARGET_GR_LYCHEE/device/cmsis.h

+#ifndef MBED_CMSIS_H
+#define MBED_CMSIS_H
+
+#include "MBRZA1LU.h"


should this include cmsis_nvic.h ? I just found the bug report for RZ A1H, and pointed me this direction (Isent just now the fix for it)

I modified this content in commit 1469fcd.

To get cmsis nvic definitions, I added the process that include "cmsis_nvic.h" in cmsis.h. Relation PR is ARMmbed#5890.

TomoYamanaka · 2018-01-24T03:12:07Z

@0xc0170
Thank you for your approval. Could you please trigger CI test?

0xc0170 · 2018-01-24T08:16:00Z

/morph build

0xc0170 · 2018-01-24T08:16:40Z

CI triggered while mbedtls reviews TRNG implementation

mbed-ci · 2018-01-24T08:59:59Z

Build : SUCCESS

Build number : 928
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5857/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

mbed-ci · 2018-01-24T11:16:05Z

Exporter Build : SUCCESS

Build number : 616
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/5857/

mbed-ci · 2018-01-24T20:29:00Z

Test : FAILURE

Build number : 768
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5857/768

0xc0170 · 2018-01-25T11:54:54Z

known issue with one target that is being retested currently, we will restart CI

cmonr · 2018-01-25T21:24:41Z

/morph build

mbed-ci · 2018-01-25T21:43:16Z

Build : SUCCESS

Build number : 962
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5857/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

mbed-ci · 2018-01-26T02:22:04Z

Exporter Build : SUCCESS

Build number : 647
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/5857/

mbed-ci · 2018-01-26T05:57:13Z

Test : SUCCESS

Build number : 789
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5857/789

0xc0170 · 2018-01-26T12:22:53Z

/morph uvisor-test

hanno-becker · 2018-01-26T16:53:32Z

I know that it is not optimal that reviewing sometimes takes a long time due to other obligations, but please do not merge PR's that you have requested approval for before the review has been done.

cmonr · 2018-01-26T16:56:45Z

@hanno-arm We're still going to holding off on releasing this PR until we get feedback for your review.

hanno-becker · 2018-01-26T16:59:36Z

@cmonr Could you indicate until when you must have received the review, so I can plan and see if and when I can fit it in? Apologies again for the delay.

cmonr · 2018-01-26T17:03:02Z

@hanno-arm No worries. The changes in this PR are minimal, and shouldn't take too much time. It would be wonderful if you could squeeze it in next week.

hanno-becker · 2018-01-29T14:18:48Z

@0xc0170 @cmonr Could you point me to the technical reference manual for the board?

hanno-becker · 2018-01-29T14:20:20Z

targets/TARGET_RENESAS/TARGET_RZ_A1XX/TARGET_GR_LYCHEE/trng_api_esp32.cpp

+    if (output_length != NULL) {
+        *output_length = idx;
+    }
+


The stack buffer recv_data holding the chunks of entropy should be zeroized before the function returns, even if it's only 4 bytes maximum.

hanno-becker · 2018-01-29T14:24:38Z

targets/TARGET_RENESAS/TARGET_RZ_A1XX/TARGET_GR_LYCHEE/trng_api_esp32.cpp

+        send_data[0] = 0;
+        ret = mI2c.write(ESP32_I2C_ADDR, send_data, 1);
+        if (ret == 0) {
+            mI2c.read(ESP32_I2C_ADDR, recv_data, sizeof(recv_data));


This call may fail, but the return value is not checked.

hanno-becker

I reviewed the TRNG addition and cannot approve it for the following reasons:

All entropy must be zeroized before the gathering function returns.
The return value for the I2C read call is not checked. This way, it could happen that predictable data is treated as the TRNG output.

cmonr · 2018-01-29T16:27:55Z

@hanno-arm Thank you for the review. For the short term, we'll introduce a PR to disable the TRNG on the board.

@TomoYamanaka, please address the above comments in a new PR.

TomoYamanaka · 2018-01-30T09:29:17Z

@cmonr @hanno-arm

I submitted a PR that addressed the above comments. So Please review PR #5970 .

Related to the review of ARMmbed#5857, I fixed the TRNG function for GR-LYCHEE. - I modified to zeroize "recv_data" before the function return. - I added the processing that check the return value of I2C.read function. If return value is error, "output" is zeroized before function return. - In trng_get_bytes_esp32 function, there is a time lag in the period from ESP32 reset to start working, error may occur when "Write" is called. Thus, I added a retry counter due to address this concern. There is not this counter for "Read" since it is called after "Write".

Related to the review of #5857, I fixed the TRNG function for GR-LYCHEE. - I modified to zeroize "recv_data" before the function return. - I added the processing that check the return value of I2C.read function. If return value is error, "output" is zeroized before function return. - In trng_get_bytes_esp32 function, there is a time lag in the period from ESP32 reset to start working, error may occur when "Write" is called. Thus, I added a retry counter due to address this concern. There is not this counter for "Read" since it is called after "Write".

TomoYamanaka added 5 commits January 16, 2018 13:25

Add GR_LYCHEE as a new target board in terget.json, build_travis.py a…

47acc9e

…nd tests.py I added GR-LYCHEE's configuration in targets.json file. Also, I added GR_LYCHEE as a Renesas new target board in build_travis.py and tests.py.

Enable exporter for GR_LYCHEE is a Renesas new target board

87968c2

I added GR_LYCHEE as a new target board in DS-5, e2studio and IAR export.

Add the definition for GR-LYCHEE in mbed_rtx.h

aa5da2c

In mbed_rtx.h file, I added the definition for GR-LYCHEE to use the "Dynamic Heap" processing when the target is GR_LYCHEE.

Support TRNG function for GR-LYCHEE

ae1eaa9

I supported the TRNG function when target is GR-LYCHEE. GR-LYCHEE generates TRNG by acquiring the random number of Wifi module(ESP32) incorporated in it using I2C.

0xc0170 added needs: review scope: new-target labels Jan 16, 2018

0xc0170 reviewed Jan 19, 2018

View reviewed changes

0xc0170 added tracking component: security labels Jan 19, 2018

0xc0170 requested a review from hanno-becker January 19, 2018 08:14

ciarmcom added the mirrored label Jan 19, 2018

TomoYamanaka added 2 commits January 19, 2018 18:39

Modify the arrangement of "{" that shows the function start

e8378ef

Regarding "{" that show the function start, I modified the the arrangement from right of function to new line.

0xc0170 reviewed Jan 19, 2018

View reviewed changes

RZ_A1LU: cmsis nvic include fix

1469fcd

To get cmsis nvic definitions, I added the process that include "cmsis_nvic.h" in cmsis.h. Relation PR is ARMmbed#5890.

0xc0170 approved these changes Jan 22, 2018

View reviewed changes

cmonr added needs: CI and removed needs: review labels Jan 25, 2018

cmonr merged commit da6532e into ARMmbed:master Jan 26, 2018

cmonr added needs: CI release-version: 5.7.4 release-version: 5.7.5 and removed needs: CI component: security mirrored release-version: 5.7.4 tracking labels Jan 26, 2018

hanno-becker reviewed Jan 29, 2018

View reviewed changes

cmonr mentioned this pull request Jan 30, 2018

Removed TRNG peripheral from GR_LECHEE target #5967

Closed

TomoYamanaka mentioned this pull request Jan 30, 2018

RZ_A1LU: Fix TRNG function #5970

Merged

Registration GR-LYCHEE board as a new mbed board #5857

Registration GR-LYCHEE board as a new mbed board #5857

Uh oh!

Conversation

TomoYamanaka commented Jan 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Relation PRs

Uh oh!

TomoYamanaka commented Jan 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomoYamanaka Jan 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Jan 19, 2018

Uh oh!

ciarmcom commented Jan 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomoYamanaka commented Jan 24, 2018

Uh oh!

0xc0170 commented Jan 24, 2018

Uh oh!

0xc0170 commented Jan 24, 2018

Uh oh!

mbed-ci commented Jan 24, 2018

Triggering tests

Uh oh!

mbed-ci commented Jan 24, 2018

Uh oh!

mbed-ci commented Jan 24, 2018

Uh oh!

0xc0170 commented Jan 25, 2018

Uh oh!

cmonr commented Jan 25, 2018

Uh oh!

mbed-ci commented Jan 25, 2018

Triggering tests

Uh oh!

mbed-ci commented Jan 26, 2018

Uh oh!

mbed-ci commented Jan 26, 2018

Uh oh!

0xc0170 commented Jan 26, 2018

Uh oh!

hanno-becker commented Jan 26, 2018

Uh oh!

cmonr commented Jan 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hanno-becker commented Jan 26, 2018

Uh oh!

cmonr commented Jan 26, 2018

Uh oh!

hanno-becker commented Jan 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hanno-becker left a comment

Choose a reason for hiding this comment

Uh oh!

cmonr commented Jan 29, 2018

TomoYamanaka commented Jan 16, 2018 •

edited

Loading

TomoYamanaka Jan 19, 2018 •

edited

Loading

cmonr commented Jan 26, 2018 •

edited

Loading