Add a platform config to disable the MPU #9040

c1728p9 · 2018-12-10T22:39:15Z

Description

Add the platform config DISABLE_MPU to allow the MPU to be disabled from an application.

This PR is identical to #9030 but has an additional commit to add this config.

Pull request type

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

ciarmcom · 2018-12-11T00:00:27Z

@c1728p9, thank you for your changes.
@ARMmbed/mbed-os-hal @ARMmbed/mbed-os-core @ARMmbed/mbed-os-test @ARMmbed/mbed-os-maintainers please review.

platform/mbed_mpu_mgmt.c

cmonr · 2018-12-11T01:32:45Z

CI started (in case it's decided to bring in as-is for RC3)

kjbracey · 2018-12-11T08:06:08Z

platform/mbed_lib.json

@@ -128,6 +128,10 @@
        "fatal-error-auto-reboot-enabled": {
            "help": "Setting this to true enables auto-reboot on a fatal error.",
            "value": false
+        },
+        "disable-mpu": {


Config options should be positive unless there's a good reason. This could be enable-mpu, or maybe use-mpu is more to the point.

0xc0170 · 2018-12-11T08:45:04Z

This PR is identical to #9030 but has an additional commit to add this config.

This shall be rebased and some comments addressed above, marking it as "needs: work".

@kjbracey-arm Do you want to push an update for positive config value? This PR looks minimal and might be considered for rc3?

kjbracey · 2018-12-11T10:50:40Z

Going to rework a little more - as this is a platform option, it should only be affecting platform code. It shouldn't actually be creating dummy HAL calls (and hence requiring custom MPU HALs to follow the config option).

Add the platform config DISABLE_MPU to allow the MPU to be disabled from an application.

Make the option positively named, and as it is a platform config option make sure it only affects platform code. HAL functions still remain available even if platform is told not to use them.

kjbracey · 2018-12-11T11:05:28Z

Reworked, and rebased to make the PR clearer. Naming the option "use-mpu" made it clearer how it should be implemented. (ie don't touch the HAL, just don't use it).

mbed-ci · 2018-12-11T11:29:59Z

Test run: FAILED

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

Failed test jobs:

jenkins-ci/mbed-os-ci_exporter

0xc0170 · 2018-12-11T11:36:23Z

CI was aborted (timeout anyway) - for older commits, will restart now for the latest update

0xc0170 · 2018-12-11T11:36:52Z

CI started

mbed-ci · 2018-12-11T11:43:06Z

Test run: FAILED

Summary: 1 of 1 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_unittests

adbridge · 2018-12-11T12:00:14Z

Looks like device out of space error!

0xc0170 · 2018-12-11T12:35:27Z

CI should be fixed (other fixes ongoing)

adbridge · 2018-12-11T12:57:06Z

CI should be fixed (other fixes ongoing)

Other fixes ?

0xc0170 · 2018-12-11T13:25:19Z

CI team is checking what caused the space to run out (it was now cleaned)

CI restarted

kjbracey · 2018-12-11T14:45:26Z

platform/mbed_mpu_mgmt.h

@@ -35,6 +35,8 @@ extern "C" {

 #define mbed_mpu_manager_init() mbed_mpu_init()

+#define #define mbed_mpu_manager_deinit() mbed_mpu_free()


At least is isn't #undefined behavior. Just kidding, I pushed a fix.

kjbracey

Russ's last commit looks good to me.

mbed-ci · 2018-12-11T15:37:30Z

Test run: SUCCESS

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

mbed-ci · 2018-12-11T15:58:10Z

Test run: FAILED

Summary: 6 of 7 test jobs failed
Build number : 4
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_mbed2-build-IAR
jenkins-ci/mbed-os-ci_mbed2-build-ARM
jenkins-ci/mbed-os-ci_mbed2-build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-IAR
jenkins-ci/mbed-os-ci_build-GCC_ARM

cmonr · 2018-12-11T15:58:45Z

Ignore #9040 (comment)

I accidentally re-enqueued the PR a while back, and forgot to remove it (and force aborted the job above).

deepikabhavnani · 2018-12-11T15:59:30Z

@cmonr - I see Commits after the CI run was triggered, last force push 15 mins ago.

cmonr · 2018-12-11T16:26:24Z

@cmonr - I see Commits after the CI run was triggered, last force push 15 mins ago.

@deepikabhavnani Correct. There was a third commit in this PR, but CI was started and passed when there were only two commits. We opted to remove the third commit to make this PR green to increase the chance of having RC3 generated before COB UK time.

This was referenced Dec 10, 2018

Document how to turn MPU off at compile time ARMmbed/mbed-os-5-docs#868

Merged

Reduce ROM impact of MPU code #9030

Merged

ciarmcom requested review from a team December 11, 2018 00:00

ciarmcom added the needs: review label Dec 11, 2018

deepikabhavnani reviewed Dec 11, 2018

View reviewed changes

platform/mbed_mpu_mgmt.c Outdated Show resolved Hide resolved

cmonr added the release-version: 5.11.0-rc3 label Dec 11, 2018

kjbracey reviewed Dec 11, 2018

View reviewed changes

c1728p9 and others added 2 commits December 11, 2018 13:03

Add a platform config to disable the MPU

2af6642

Add the platform config DISABLE_MPU to allow the MPU to be disabled from an application.

Rework MPU use option

770baa2

Make the option positively named, and as it is a platform config option make sure it only affects platform code. HAL functions still remain available even if platform is told not to use them.

kjbracey force-pushed the disable_mpu_config branch from 753c8a2 to 770baa2 Compare December 11, 2018 11:04

0xc0170 requested a review from bulislaw December 11, 2018 11:09

0xc0170 approved these changes Dec 11, 2018

View reviewed changes

kjbracey reviewed Dec 11, 2018

View reviewed changes

c1728p9 force-pushed the disable_mpu_config branch from dcf8ce9 to 3e20e7e Compare December 11, 2018 14:47

kjbracey approved these changes Dec 11, 2018

View reviewed changes

c1728p9 force-pushed the disable_mpu_config branch from 3e20e7e to 770baa2 Compare December 11, 2018 15:43

cmonr added ready for merge and removed needs: review labels Dec 11, 2018

cmonr merged commit 7546186 into ARMmbed:master Dec 11, 2018

cmonr removed the ready for merge label Dec 11, 2018

		@@ -35,6 +35,8 @@ extern "C" {

		#define mbed_mpu_manager_init() mbed_mpu_init()

		#define #define mbed_mpu_manager_deinit() mbed_mpu_free()

Add a platform config to disable the MPU #9040

Add a platform config to disable the MPU #9040

Uh oh!

Conversation

c1728p9 commented Dec 10, 2018

Description

Pull request type

Uh oh!

ciarmcom commented Dec 11, 2018

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

kjbracey Dec 11, 2018

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Dec 11, 2018

Uh oh!

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

Uh oh!

Uh oh!

kjbracey commented Dec 11, 2018

Uh oh!

mbed-ci commented Dec 11, 2018

Test run: FAILED

Uh oh!

0xc0170 commented Dec 11, 2018

Uh oh!

0xc0170 commented Dec 11, 2018

Uh oh!

mbed-ci commented Dec 11, 2018

Test run: FAILED

Uh oh!

adbridge commented Dec 11, 2018

Uh oh!

0xc0170 commented Dec 11, 2018

Uh oh!

adbridge commented Dec 11, 2018

Uh oh!

0xc0170 commented Dec 11, 2018

Uh oh!

kjbracey Dec 11, 2018

Choose a reason for hiding this comment

Uh oh!

c1728p9 Dec 11, 2018

Choose a reason for hiding this comment

Uh oh!

kjbracey left a comment

Choose a reason for hiding this comment

Uh oh!

mbed-ci commented Dec 11, 2018

Test run: SUCCESS

Uh oh!

mbed-ci commented Dec 11, 2018

Test run: FAILED

Uh oh!

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

Uh oh!

Uh oh!

deepikabhavnani commented Dec 11, 2018

Uh oh!

cmonr commented Dec 11, 2018

Uh oh!

Uh oh!

cmonr commented Dec 11, 2018 •

edited

Loading

kjbracey commented Dec 11, 2018 •

edited

Loading

cmonr commented Dec 11, 2018 •

edited

Loading