Disable MBEDTLS_CONFIG_HW_SUPPORT on STM targets. #8142

moranpeker · 2018-09-16T12:24:33Z

Description

Disable MBEDTLS_CONFIG_HW_SUPPORT on all STM targets.
Fix issue: #6545

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

JanneKiiskila · 2018-09-17T06:16:08Z

targets/targets.json

@@ -2373,6 +2373,7 @@
    "MBED_CONNECT_ODIN": {
        "inherits": ["MODULE_UBLOX_ODIN_W2"],
        "release_versions": ["5"],
+        "macros_add": ["MBEDTLS_CONFIG_HW_SUPPORT"],


Should it not be removed, I believe?

Is should be disabled for the all three as CONNECT_ODIN and MTB boards are using same WIFI binary driver.

Yep, but if you ADD the CONFIG_HW_SUPPORT, then you are again activating it - i.e. reversing the 1st change done to the common target fíle. In my humble opinion these macros_add changes should not be done, we should ONLY do the common target removal.

0xc0170 · 2018-09-17T12:05:23Z

As in the referenced issue, disabling it for other platforms comes in a separate PR ?

0xc0170 · 2018-09-20T09:11:54Z

As in the referenced issue, disabling it for other platforms comes in a separate PR ?

@moranpeker Please review this one, what shall happen to other platforms?

JanneKiiskila · 2018-09-25T11:08:22Z

Can we get this to next patch release, please.
@0xc0170 @adbridge

0xc0170 · 2018-09-25T11:13:57Z

We can proceed with this one, but as I understood also other platforms are affected and should be updated (I haven't yet seen PR neither an action that will take care of it).

JanneKiiskila · 2018-09-25T11:21:34Z

Ah, OK - it can't be done for this target alone, it should be done to all STM targets (I assume the driver is shared amongst all STM targets).

0xc0170 · 2018-09-25T11:27:05Z

@Patater should MBEDTLS_CONFIG_HW_SUPPORT be removed from targets (the issue referenced above states that it's broken - not specifying other platforms, just some stm32) ? This PR removes it so far only for odin target.

cmonr · 2018-09-26T03:27:55Z

@ARMmbed/team-st-mcd Thoughts on #8142 (comment) ?

Patater · 2018-09-26T07:01:59Z

Yes, hardware acceleration should be removed from all targets that fail testing. At the time this PR was made, only UBLOX_ODIN_W2 was known to fail testing.

0xc0170 · 2018-09-26T07:29:25Z

/morph build

mbed-ci · 2018-09-26T08:29:41Z

Build : FAILURE

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

asifrizwanubx · 2018-09-27T07:59:44Z

Hello,
Please do not merge !
Did anyone prove that the CRYPTO HW ACCELERATION is failing on non-ublox boards ?
Could you show me where you point the evidences of this ?

Kind regards

@adustm
#6545 (comment) reported that issue on non-ublox board. But still I agree with you, @teetak01 could you please confirm it? because you have some results on different boards.

LMESTM · 2018-09-27T09:18:59Z

@adustm
#6545 (comment) reported that issue on non-ublox board. But still I agree with you, @teetak01 could you please confirm it? because you have some results on different boards.

the TARGET_STM32F437xG based is TARGET_UBLOX_C030

asifrizwanubx · 2018-09-27T09:25:02Z

then is there any other target which support CRYPTO HW ACCELERATION working?

adustm · 2018-09-28T15:38:20Z

Hello,
I managed to reproduce the issue on NUCLEO_F439ZI (hw crypto)

Only the SHA256 hw acceleration is causing the problem. This PR is doing more than simply removing SHA256 as it removes every HW acceleration...
You may want to transform it into

--- a/features/mbedtls/targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F439xI/mbedtls_device.h
+++ b/features/mbedtls/targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F439xI/mbedtls_device.h
@@ -22,7 +22,6 @@

 #define MBEDTLS_AES_ALT

-#define MBEDTLS_SHA256_ALT

 #define MBEDTLS_SHA1_ALT

(+the same for other devices)
Thanks you for having waited until I managed to prove this.

asifrizwanubx · 2018-10-01T05:07:12Z

I managed to reproduce the issue on NUCLEO_F439ZI (hw crypto)

Could you please share how did you reproduce it?

Only the SHA256 hw acceleration is causing the problem. This PR is doing more than simply removing SHA256 as it removes every HW acceleration...

I agree with you on that.

adustm · 2018-10-01T07:35:01Z

Could you please share how did you reproduce it?

It is not plug and play... You have to know that NUCLEO_F439ZI = NUCLEO_F429ZI + hw crypto functionality. Both platforms are 100 % compatible.

mbed import https://github.com/ARMmbed/mbed-cloud-client-example
-> I have mbed-cloud-client v 2.0.0 and mbed-os v 5.10

mbed config -G CLOUD_SDK_API_KEY <API_KEY>
mbed target NUCLEO_F439ZI
mbed toolchain GCC_ARM
mbed device-management init -d arm.com --model-name example-app --force -q

Then a task to find every NUCLEO_F429ZI in any json file, any cpp file, any heady file to make sure the NUCLEO_F439ZI has the same declarations and settings than the NUCLEO_F429ZI
Another taks to find every TARGET_NUCLEO_F429ZI directory and make sure that it is copied as TARGET_NUCLEO_F439ZI directory
mbed-cloud-client-example modified files

        modified:   configs/eth_v4.json
        modified:   configs/eth_v6.json
        modified:   configs/mesh_6lowpan.json
        modified:   configs/mesh_thread.json
        modified:   configs/wifi_esp8266_v4.json
        modified:   mbed_app.json
        modified:   mbed_cloud_client_user_config
        modified:   mbed_cloud_dev_credentials.c
        modified:   mbed_lib.json
        modified:   update_default_resources.c

mbed-cloud-client-example/mbed-cloud-client modified files

        modified:   mbed-client-pal/Configs/pal_config/mbedOS/mbedOS_crypto_only.h
        modified:   mbed-client-pal/Configs/pal_config/mbedOS/mbedOS_default.h
mbed-cloud-client-example/mbed-cloud-client Untracked files:
        mbed-client-pal/Configs/pal_config/mbedOS/TARGET_NUCLEO_F439ZI/

mbed-cloud-client-example/mbed-os modified files

        modified:   components/storage/blockdevice/COMPONENT_SD/config/mbed_lib.json
        modified:   features/mbedtls/targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F439xI/mbedtls_device.h
        modified:   targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F439xI/TARGET_NUCLEO_F439ZI/system_clock.c
        modified:   targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F439xI/device/TOOLCHAIN_ARM_MICRO/startup_stm32f439xx.S
        modified:   targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F439xI/device/TOOLCHAIN_ARM_MICRO/stm32f439xx.sct
        modified:   targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F439xI/device/TOOLCHAIN_GCC_ARM/STM32F439ZI.ld
        modified:   targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F439xI/device/TOOLCHAIN_GCC_ARM/startup_stm32f439xx.S
        modified:   targets/targets.json

mbed-cloud-client-example/mbed-os Untracked files

        features/FEATURE_BOOTLOADER/targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F439xI/TARGET_NUCLEO_F439ZI/
        targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F439xI/device/system_init_pre.c

I have copied the bootloader file from the NUCLEO_F429ZI, because I don't know how to create one. This means that this binary file is compiled with sw crypto if any mbedtls functions inside.

mbed compile
drag and drop

0xc0170 · 2018-10-01T09:20:43Z

#8142 (comment)

@moranpeker @Patater Please review the proposal

adustm · 2018-10-02T08:27:38Z

Hello,
Could you remind me if disabling MBEDTLS_CONFIG_HW_SUPPORT is also disabling the hardware random generator (RNG) ?
If it is the case, you really should consider disabling hw acceleration directly from the mbedtls_device.h files, otherwise entropy HW source will no more be granted.

Kind regards

moranpeker · 2018-10-02T12:33:24Z

Hi,
I just update the PR according to @adustm proposal.
Could you please re-review the changes?

0xc0170 · 2018-10-05T05:13:07Z

/morph build

mbed-ci · 2018-10-05T07:37:49Z

Build : SUCCESS

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

Triggering tests

/morph test
/morph mbed2-build

0xc0170 · 2018-10-05T10:34:19Z

License issue, restarting

/morph mbed2-build

0xc0170 · 2018-10-05T12:16:43Z

/morph export-build

mbed-ci · 2018-10-05T13:42:08Z

Exporter Build : FAILURE

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

0xc0170 · 2018-10-05T13:43:16Z

IAR license issue, restarting

/morph export-build

mbed-ci · 2018-10-05T16:18:56Z

Exporter Build : SUCCESS

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

mbed-ci · 2018-10-05T22:38:49Z

Test : SUCCESS

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

JanneKiiskila reviewed Sep 17, 2018

View reviewed changes

moranpeker changed the title ~~Disable MBEDTLS_CONFIG_HW_SUPPORT on UBLOX_EVK_ODIN_W2 target.~~ Disable MBEDTLS_CONFIG_HW_SUPPORT on UBLOX_ODIN_W2 target. Sep 17, 2018

moranpeker force-pushed the disable-HW-acceleration branch from fd28d78 to ae906df Compare September 17, 2018 11:13

0xc0170 added the needs: review label Sep 17, 2018

0xc0170 requested a review from a team September 17, 2018 12:00

0xc0170 requested a review from Patater September 17, 2018 12:05

asifrizwanubx approved these changes Sep 17, 2018

View reviewed changes

Patater approved these changes Sep 20, 2018

View reviewed changes

0xc0170 approved these changes Sep 20, 2018

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Sep 20, 2018

cmonr added needs: review and removed needs: CI labels Sep 23, 2018

0xc0170 added needs: CI and removed needs: review labels Sep 25, 2018

0xc0170 added the release-version: 5.10.1 label Sep 25, 2018

cmonr added needs: work and removed needs: CI labels Sep 26, 2018

0xc0170 added needs: CI and removed needs: work labels Sep 26, 2018

cmonr removed the release-version: 5.10.1 label Sep 27, 2018

Disable MBEDTLS_SHA256_ALT on STM targets.

333f087

moranpeker force-pushed the disable-HW-acceleration branch from e1d0cc1 to 333f087 Compare October 2, 2018 11:45

Patater approved these changes Oct 2, 2018

View reviewed changes

adustm approved these changes Oct 3, 2018

View reviewed changes

NirSonnenschein added needs: CI and removed needs: review labels Oct 4, 2018

0xc0170 added the release-version: 5.10.1 label Oct 5, 2018

0xc0170 added ready for merge and removed needs: CI labels Oct 6, 2018

0xc0170 merged commit bc706f0 into ARMmbed:master Oct 6, 2018

0xc0170 removed the ready for merge label Oct 6, 2018

alekshex mentioned this pull request Mar 14, 2019

Temporary disable Crypto HW accelerator on UBLOX_EVK_ODIN_W2 #10088

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable MBEDTLS_CONFIG_HW_SUPPORT on STM targets. #8142

Disable MBEDTLS_CONFIG_HW_SUPPORT on STM targets. #8142

moranpeker commented Sep 16, 2018 •

edited

Loading

JanneKiiskila Sep 17, 2018

teetak01 Sep 17, 2018

JanneKiiskila Sep 17, 2018

0xc0170 commented Sep 17, 2018

0xc0170 commented Sep 20, 2018

JanneKiiskila commented Sep 25, 2018

0xc0170 commented Sep 25, 2018

JanneKiiskila commented Sep 25, 2018

0xc0170 commented Sep 25, 2018 •

edited

Loading

cmonr commented Sep 26, 2018

Patater commented Sep 26, 2018

0xc0170 commented Sep 26, 2018

mbed-ci commented Sep 26, 2018

asifrizwanubx commented Sep 27, 2018 •

edited

Loading

LMESTM commented Sep 27, 2018

asifrizwanubx commented Sep 27, 2018

adustm commented Sep 28, 2018

asifrizwanubx commented Oct 1, 2018

adustm commented Oct 1, 2018 •

edited

Loading

0xc0170 commented Oct 1, 2018

adustm commented Oct 2, 2018

moranpeker commented Oct 2, 2018

0xc0170 commented Oct 5, 2018

mbed-ci commented Oct 5, 2018

0xc0170 commented Oct 5, 2018

0xc0170 commented Oct 5, 2018

mbed-ci commented Oct 5, 2018

0xc0170 commented Oct 5, 2018

mbed-ci commented Oct 5, 2018

mbed-ci commented Oct 5, 2018

Disable MBEDTLS_CONFIG_HW_SUPPORT on STM targets. #8142

Disable MBEDTLS_CONFIG_HW_SUPPORT on STM targets. #8142

Conversation

moranpeker commented Sep 16, 2018 • edited Loading

Description

Pull request type

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xc0170 commented Sep 17, 2018

0xc0170 commented Sep 20, 2018

JanneKiiskila commented Sep 25, 2018

0xc0170 commented Sep 25, 2018

JanneKiiskila commented Sep 25, 2018

0xc0170 commented Sep 25, 2018 • edited Loading

cmonr commented Sep 26, 2018

Patater commented Sep 26, 2018

0xc0170 commented Sep 26, 2018

mbed-ci commented Sep 26, 2018

asifrizwanubx commented Sep 27, 2018 • edited Loading

LMESTM commented Sep 27, 2018

asifrizwanubx commented Sep 27, 2018

adustm commented Sep 28, 2018

asifrizwanubx commented Oct 1, 2018

adustm commented Oct 1, 2018 • edited Loading

0xc0170 commented Oct 1, 2018

adustm commented Oct 2, 2018

moranpeker commented Oct 2, 2018

0xc0170 commented Oct 5, 2018

mbed-ci commented Oct 5, 2018

Triggering tests

0xc0170 commented Oct 5, 2018

0xc0170 commented Oct 5, 2018

mbed-ci commented Oct 5, 2018

0xc0170 commented Oct 5, 2018

mbed-ci commented Oct 5, 2018

mbed-ci commented Oct 5, 2018

moranpeker commented Sep 16, 2018 •

edited

Loading

0xc0170 commented Sep 25, 2018 •

edited

Loading

asifrizwanubx commented Sep 27, 2018 •

edited

Loading

adustm commented Oct 1, 2018 •

edited

Loading