Skip to content
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

ISR sometimes run with the MPU disabled: breaks __nocache #12548

Closed
aurel32 opened this issue Jan 17, 2019 · 5 comments
Closed

ISR sometimes run with the MPU disabled: breaks __nocache #12548

aurel32 opened this issue Jan 17, 2019 · 5 comments
Assignees
Labels
area: ARM ARM (32-bit) Architecture area: Memory Protection bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Milestone

Comments

@aurel32
Copy link
Collaborator

aurel32 commented Jan 17, 2019

Describe the bug

On ARM platforms with and CONFIG_CPU_HAS_ARM_MPU=y and CONFIG_MPU_STACK_GUARD=y, the ISR might sometimes (but rarely) run with the MPU disabled.

This happens because the MPU is briefly disabled and then re-enabled in configure_mpu_stack_guard, which is called during context switch:

arm_core_mpu_disable();
arm_core_mpu_configure(THREAD_STACK_GUARD_REGION, guard_start,
guard_size);
arm_core_mpu_enable();

If an interrupt happens during this small amount of time, it runs with the MPU disabled. The MPU is used by the __nocache feature to disable cache on a memory region. When the MPU is disabled, the cache is enabled again. Access to this regions therefore sometimes returns stale data from the previous time the cache was enabled.

The workaround it to set CONFIG_MPU_STACK_GUARD=n and CONFIG_USERSPACE=n. This avoid the MPU to be disabled and re-enabled during context switch.

To Reproduce
This was observed when working on PR #11888. The problem happens when doing multi-Mb transfer on the ethernet interface, therefore causing a lot of interrupts. I used this debug code in the interrupt, I guess it can also be reproduced with other interrupts:

if ((MPU->CTRL & MPU_CTRL_ENABLE_Msk) == 0) {
        printk("MPU is disabled\n");
}

Expected behavior
The MPU is always enabled when running ISR code.

Impact
This breaks __nocache support, which in turns prevents to enable cache on SAM E70 when the ethernet driver is enabled (PR #11888).

Environment (please complete the following information):

  • OS: Linux
  • Toolchain: Binutils/GCC from Debian sid.
  • Commit SHA: 689ad43
@aurel32 aurel32 added bug The issue is a bug, or the PR is fixing a bug area: Memory Protection labels Jan 17, 2019
@aurel32 aurel32 added this to the v1.14.0 milestone Jan 17, 2019
@aurel32
Copy link
Collaborator Author

aurel32 commented Jan 17, 2019

@andrewboie @andyross @ioannisg @angansari You might be interested by this issue.

@andrewboie
Copy link
Contributor

We agreed to get rid of the disable/enable calls.

@ioannisg
Copy link
Member

We agreed to get rid of the disable/enable calls.

This will be handled in the ARM MPU re-work (for 1.14)

@ioannisg ioannisg added the area: ARM ARM (32-bit) Architecture label Jan 23, 2019
@aurel32
Copy link
Collaborator Author

aurel32 commented Jan 23, 2019

We agreed to get rid of the disable/enable calls.

This will be handled in the ARM MPU re-work (for 1.14)

This currently blocks PR #11888. Can we agree to also have that one merged for 1.14?

@ioannisg
Copy link
Member

We agreed to get rid of the disable/enable calls.

This will be handled in the ARM MPU re-work (for 1.14)

This currently blocks PR #11888. Can we agree to also have that one merged for 1.14?

That's ok with me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARM ARM (32-bit) Architecture area: Memory Protection bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

No branches or pull requests

4 participants