More MPU work #9061

kjbracey · 2018-12-11T17:02:08Z

Description

Don't call HAL MPU free code when platform.use-mpu is turned off.
Correct MPU synchronisation sequences.
Reduce MPU code size further with a loop
Use higher-level calls, fix ARMv8-M error
Assert number of regions, rather than error
ARMv7-M: correctly protect 80000000 RAM region
Fix target.mpu-rom-end setting, for ARMv8-M too
nRF52840: Set mpu-rom-end to 0x1fffffff (to test this case)

Pull request type

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

Use the MPU through the platform layer rather than through the HAL.

Synchronisation instructions were not quite right - too strict on entry, and not quite correctly synchronising the instruction stream on exit. References: * https://static.docs.arm.com/dai0321/a/DAI0321A_programming_guide_memory_barriers_for_m_profile.pdf * https://static.docs.arm.com/100699/0100/armv8m_architecture_memory_protection_unit_100699_0100_00_en.pdf

kjbracey · 2018-12-11T17:05:05Z

Needs some more local testing - would be good if @c1728p9 could have a look at ARMv8-M in particular. Also intending one more commit to change MBED_ERRORs to MBED_ASSERTs.

cmonr · 2018-12-11T17:39:14Z

@kjbracey-arm Don't forget the astyle fix :)

kjbracey · 2018-12-12T09:17:17Z

Adjusted a bit more, making ARMv7-M use higher-level API too.

ARMv7-M init code now down to 170 bytes from 215.

0xc0170 · 2018-12-12T09:48:40Z

Please review astyle travis CI

kjbracey · 2018-12-12T10:11:51Z

Full mbed_mpu_v7m.c + mbed_mpu_mgmt.c is now 246+351 = 597, down from 659 in current master.

Switch to higher-level calls and macros, and fix an error in the ARMv8-M version - "inner" attributes were not being set correctly due to a copy/paste error - "outer" was being set twice. This means RAM would have been marked WTRA rather than WBWA for the inner cache. Slightly reduces ARMv7-M init code size by feeding region number into RBAR instead of using RNR.

As we build for a specific CPU, a runtime check for number of MPU regions in release builds is not worthwhile. Make it an assert only. Saves a little space in develop images, a lot in release.

Subregion mask for this region was set fully disabled, instead of fully enabled.

kjbracey · 2018-12-12T13:15:24Z

Two more bugs found - target.mpu-rom-end JSON setting wasn't working, and 80000000 wasn't protected in ARMv7-M.

Fixed up, including making mpu-rom-end work on ARMv8-M - was simpler than figuring out what to do to not hit the "not supported" test once JSON was fixed.

Also will save a few more bytes if you do set mpu-rom-end to 0x1fffffff.

targets.json was not specifying the same macro name as the code was checking for, so setting was ineffective. Making this work tripped up not-supported checks in ARMv8-M - rather than deal with making this work, support it instead. Both ARMv7-M and ARMv8-M slightly reduce code size and runtime impact if mpu-rom-end is 0x1fffffff, using one fewer region. This means default setup for ARMv8-M now requires 5 regions, with mpu-rom-end set to default 0x0fffffff, but this can be put back to 4 by changing the setting.

So we have at least one test platform exercising the special case of mpu-rom-end being 0x1fffffff, set that for nRF52840.

cmonr · 2018-12-12T22:22:18Z

CI started

mbed-ci · 2018-12-13T01:56:21Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

kjbracey · 2018-12-13T07:52:28Z

This does really need rereview - I did some more significant stuff since @c1728p9 last looked at it.

cmonr · 2018-12-13T16:30:54Z

Hmm... I wonder.

I recall a time when GitHub would auto-dismiss reviews when a change was made.

Significant changes have occurred. Please re-review.

Move the start of RAM from 0x10000000 to 0x20000000 on the NUMAKER_PFM_M2351. This allows the target to work correctly.

c1728p9

I tested this on a NUMAKER_PFM_M2351 and got crashes on boot due to the MPU. I pushed a commit to update the ram/rom split for this target to correct the crashes. This PR looks good to me. Thanks @kjbracey-arm!

0xc0170 · 2018-12-19T09:12:24Z

CI started

mbed-ci · 2018-12-19T12:09:36Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

c1728p9 and others added 3 commits December 11, 2018 18:59

Use MPU platform deinit rather than hal free

cdc61c5

Use the MPU through the platform layer rather than through the HAL.

Reduce dynamic MPU code size with a loop

2b5518a

kjbracey changed the title ~~More mpu work~~ More MPU work Dec 11, 2018

cmonr requested a review from c1728p9 December 11, 2018 17:06

cmonr added needs: review release-version: 5.11.1 labels Dec 11, 2018

c1728p9 previously approved these changes Dec 11, 2018

View reviewed changes

kjbracey force-pushed the more_mpu_work branch 2 times, most recently from c212214 to 3477c45 Compare December 12, 2018 09:15

kjbracey force-pushed the more_mpu_work branch from 3477c45 to 7631745 Compare December 12, 2018 09:28

0xc0170 approved these changes Dec 12, 2018

View reviewed changes

kjbracey force-pushed the more_mpu_work branch from 7631745 to ced5cb9 Compare December 12, 2018 10:01

kjbracey force-pushed the more_mpu_work branch from ced5cb9 to 5635188 Compare December 12, 2018 12:18

kjbracey added 3 commits December 12, 2018 15:12

Assert MPU regions, rather than error

171e575

As we build for a specific CPU, a runtime check for number of MPU regions in release builds is not worthwhile. Make it an assert only. Saves a little space in develop images, a lot in release.

ARMv7-M MPU: correctly protect 80000000 RAM region

170b734

Subregion mask for this region was set fully disabled, instead of fully enabled.

kjbracey force-pushed the more_mpu_work branch from c3f55f3 to cc5049f Compare December 12, 2018 13:12

kjbracey force-pushed the more_mpu_work branch 2 times, most recently from d4d5fca to 8ee286e Compare December 12, 2018 13:22

kjbracey added 2 commits December 12, 2018 15:44

nRF52840: Set mpu-rom-end to 0x1fffffff

574f9fa

So we have at least one test platform exercising the special case of mpu-rom-end being 0x1fffffff, set that for nRF52840.

kjbracey force-pushed the more_mpu_work branch from a3124a5 to 574f9fa Compare December 12, 2018 13:44

cmonr added the needs: CI label Dec 12, 2018

cmonr removed the needs: review label Dec 12, 2018

cmonr added ready for merge and removed needs: CI labels Dec 13, 2018

kjbracey added needs: review and removed ready for merge labels Dec 13, 2018

Update MPU ram/rom split for Nuvoton M2351

adf253e

Move the start of RAM from 0x10000000 to 0x20000000 on the NUMAKER_PFM_M2351. This allows the target to work correctly.

c1728p9 approved these changes Dec 18, 2018

View reviewed changes

cmonr added needs: CI and removed needs: review labels Dec 19, 2018

0xc0170 added ready for merge and removed needs: CI labels Dec 19, 2018

cmonr merged commit 6f57600 into ARMmbed:master Dec 19, 2018

cmonr removed the ready for merge label Dec 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

More MPU work #9061

More MPU work #9061

Uh oh!

kjbracey commented Dec 11, 2018 •

edited

Loading

Uh oh!

kjbracey commented Dec 11, 2018

Uh oh!

cmonr commented Dec 11, 2018

Uh oh!

kjbracey commented Dec 12, 2018

Uh oh!

0xc0170 commented Dec 12, 2018

Uh oh!

kjbracey commented Dec 12, 2018

Uh oh!

kjbracey commented Dec 12, 2018 •

edited

Loading

Uh oh!

cmonr commented Dec 12, 2018

Uh oh!

mbed-ci commented Dec 13, 2018

Uh oh!

kjbracey commented Dec 13, 2018 •

edited

Loading

Uh oh!

cmonr commented Dec 13, 2018

Uh oh!

c1728p9 left a comment

Uh oh!

0xc0170 commented Dec 19, 2018

Uh oh!

mbed-ci commented Dec 19, 2018

Uh oh!

Uh oh!

More MPU work #9061

More MPU work #9061

Uh oh!

Conversation

kjbracey commented Dec 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Uh oh!

kjbracey commented Dec 11, 2018

Uh oh!

cmonr commented Dec 11, 2018

Uh oh!

kjbracey commented Dec 12, 2018

Uh oh!

0xc0170 commented Dec 12, 2018

Uh oh!

kjbracey commented Dec 12, 2018

Uh oh!

kjbracey commented Dec 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmonr commented Dec 12, 2018

Uh oh!

mbed-ci commented Dec 13, 2018

Test run: SUCCESS

Uh oh!

kjbracey commented Dec 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmonr commented Dec 13, 2018

Uh oh!

c1728p9 left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Dec 19, 2018

Uh oh!

mbed-ci commented Dec 19, 2018

Test run: SUCCESS

Uh oh!

Uh oh!

kjbracey commented Dec 11, 2018 •

edited

Loading

kjbracey commented Dec 12, 2018 •

edited

Loading

kjbracey commented Dec 13, 2018 •

edited

Loading