Skip to content

fix regression due to #90305 #90867

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 31, 2025
Merged

fix regression due to #90305 #90867

merged 2 commits into from
May 31, 2025

Conversation

manuargue
Copy link
Member

Address a regression introduced in #90305. While the intention was to avoid speculative access in the entire memory region, it overlooked the fact that many devices may rely on the architectural background region. Hence for now revert the change in arm_mpu_regions.c and address it only in the device where it was reported.

@manuargue
Revert "arch: arm: mpu: fix erratum 1013783-B"
788bd78 
This reverts commit 983b1d0.

Signed-off-by: Manuel Argüelles <[email protected]>
@manuargue manuargue added the Hotfix Fix for issues blocking development, i.e. CI issues, tests failing in CI, etc. label May 30, 2025
@github-actions github-actions bot added platform: NXP S32 NXP Semiconductors, S32 area: ARM ARM (32-bit) Architecture labels May 30, 2025
wearyzen
wearyzen previously approved these changes May 30, 2025
View reviewed changes
@wearyzen
Copy link
Collaborator

Hi @manuargue, would you be able to provide more details on where the regression was seen on? I am trying to understand if we could have caught this early.

bperseghetti
bperseghetti previously approved these changes May 30, 2025
View reviewed changes
@manuargue
Copy link
Member Author

manuargue commented May 30, 2025 •
edited
Loading

Hi @manuargue, would you be able to provide more details on where the regression was seen on? I am trying to understand if we could have caught this early.

It was seen on the K3 board, because the region 0 is P_NA_U_NA_Msk hence it leaves everything else unmapped. The test was mistakenly done using P_RW_U_NA_Msk, different than the code reviewed.

Reverting the changes in arm_mpu_regions.c is more preventive than anything else. We could bring this back but we need to make sure to map all other memory regions that need access, e.g. based on the architectural background region. I do not have many devices to test this though.

Dat-NguyenDuy
Dat-NguyenDuy reviewed May 30, 2025
View reviewed changes
Copy link
Collaborator

@Dat-NguyenDuy Dat-NguyenDuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some nitpicks

@manuargue
soc: nxp: s32k3: configure missing mpu regions
5866d6e 
This fixes a regression introduced in c316402 where all regions
except Flash and RAM where left unmapped. Before introducing region
0 that prevents speculative access to the entire memory space, we
were relying on the architectural background map to access them.

Signed-off-by: Manuel Argüelles <[email protected]>
@manuargue manuargue dismissed stale reviews from bperseghetti and wearyzen via 5866d6e May 30, 2025 14:03
@manuargue manuargue force-pushed the nxp-hotfix-mpu-arm-k3 branch from b577b0b to 5866d6e Compare May 30, 2025 14:03
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@pavlohamov pavlohamov mentioned this pull request May 31, 2025
Closed
1 task
@manuargue manuargue requested a review from JarmouniA May 31, 2025 09:01
@nashif nashif merged commit 6681f8d into zephyrproject-rtos:main May 31, 2025
30 checks passed
@manuargue manuargue deleted the nxp-hotfix-mpu-arm-k3 branch May 31, 2025 10:17
@JarmouniA JarmouniA linked an issue May 31, 2025 that may be closed by this pull request
Cortex-m7 targets fails to start (stm32f746g, stm32h7b3i) #90902
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JarmouniA JarmouniA JarmouniA approved these changes

@Dat-NguyenDuy Dat-NguyenDuy Dat-NguyenDuy approved these changes

@bperseghetti bperseghetti bperseghetti left review comments

@wearyzen wearyzen wearyzen left review comments

@bbolen bbolen Awaiting requested review from bbolen

@carlocaione carlocaione Awaiting requested review from carlocaione

@congnguyenhuu congnguyenhuu Awaiting requested review from congnguyenhuu

@galak galak Awaiting requested review from galak

@ithinuel ithinuel Awaiting requested review from ithinuel

@MaureenHelm MaureenHelm Awaiting requested review from MaureenHelm

@microbuilder microbuilder Awaiting requested review from microbuilder

@stephanosio stephanosio Awaiting requested review from stephanosio

Assignees

@wearyzen wearyzen

Labels
area: ARM ARM (32-bit) Architecture Hotfix Fix for issues blocking development, i.e. CI issues, tests failing in CI, etc. platform: NXP S32 NXP Semiconductors, S32
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cortex-m7 targets fails to start (stm32f746g, stm32h7b3i)
6 participants
@manuargue @wearyzen @bperseghetti @JarmouniA @Dat-NguyenDuy @nashif