-
Notifications
You must be signed in to change notification settings - Fork 3k
Disable write buffer in debug builds (M3/M4) #8655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I think my main desire for this (writes to NULL) is relieved by the latest version of #8335, but could still be worthwhile for other weird bus accesses. |
CI is still problematic, but trying to test this for build/export |
Build : SUCCESSBuild number : 3663 Triggering tests/morph test |
Exporter Build : FAILUREBuild number : 3270 |
Test : FAILUREBuild number : 3440 |
Info: This PR has been re-bundled into a new rollup PR (#8800). No further work is needed here, as once that PR is merged, this PR will also be closed and marked as merged. |
This needs work. Exporters failure here are valid (latest results above) and were reproduced in the rollup |
Will investigate. You can treat this as low priority - it's a nice-to-have, but not really needed for 5.11 as long as #8335 makes it in. |
Okay, re-read the manual while investigating M3 revisions (turns out not all revisions of M3 have the bit, and not all platforms specify their M3 revision, and This bit only affects behaviour when the MPU is turned off, so it interacts weirdly with #8335, and there's no simple portable way to reproduce the behaviour with the MPU on, so I'm going to just drop it. #8335's ROM write protection should cover 90% of the cases I'm worried about - will revisit with an MPU solution if I hit others. (Maybe make some areas Strongly Ordered in debug builds?) |
Reopening, as #8335 does not in fact cover my most common case - the K64F. Even if the K64F had a MPU driver, it still wouldn't provide precise aborts like the ARM MPU. So this patch is still useful. Now incorporating a CMSIS patch to make the ACTLR register easier to deal with - should fix the previous CI failures. That has already been merged upstream: ARM-software/CMSIS_5#478 |
61491c2
to
4ea4762
Compare
The ACTLR register itself is conditional on chip revision, but its bit definitions were always defined. Make the the bit definitions also conditional, so it is possible to produce portable code that sets DISDEFWBUF if available: #ifdef SCnSCB_ACTLR_DISDEFWBUF_Msk SCnSCB->ACTLR |= SCnSCB_ACTLR_DISDEFWBUF_Msk; #endif (cherry-picked from CMSIS b2b04dbeece0a046556bfc320bef6b20bef3f16f)
As part of work to improve the debugging of exceptions, have Mbed OS make an effort to make exceptions more precise in debug builds at the cost of performance. Related pyOCD work: pyocd/pyOCD#430
4ea4762
to
bd413f1
Compare
Shouldn't need work any more - I believe the extra commit clears the previous CI failures. |
CI started |
Test run: SUCCESSSummary: 11 of 11 test jobs passed |
Description
As part of work to improve the debugging of exceptions, have
Mbed OS make an effort to make exceptions more precise in debug builds
at the cost of performance.
Related pyOCD work:
pyocd/pyOCD#430
Pull request type