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

arch: arm: Fix incorrect Cortex-R interrupt state control logic. #20474

Merged

Conversation

stephanosio
Copy link
Member

@stephanosio stephanosio commented Nov 8, 2019

This commit fixes incorrect Cortex-R interrupt lock, unlock and state
check function implementations.

The issues can be summarised as follows:

  1. The current implementation of 'z_arch_irq_lock' returns the value
    of CPSR as the IRQ key and, since CPSR contains many other state
    bits, this caused 'z_arch_irq_unlocked' to return false even when
    IRQ is unlocked. This problem is fixed by isolating only the I-bit
    of CPSR and returning this value as the IRQ key, such that it
    returns a non-zero value when interrupt is disabled.

  2. The current implementation of 'z_arch_irq_unlock' directly updates
    the value of CPSR control field with the IRQ key and this can cause
    other state bits in CPSR to be corrupted. This problem is fixed by
    conditionally enabling interrupt using CPSIE instruction when the
    value of IRQ key is a zero.

  3. The current implementation of 'z_arch_is_in_isr' checks the value
    of CPSR MODE field and returns true if its value is IRQ or FIQ.
    While this does not normally cause an issue, the function can return
    false when IRQ offloading is used because the offload function
    executes in SVC mode. This problem is fixed by adding check for SVC
    mode.

Signed-off-by: Stephanos Ioannidis root@stephanos.io

NOTE:

@zephyrbot zephyrbot added area: ARM ARM (32-bit) Architecture area: API Changes to public APIs labels Nov 8, 2019
@stephanosio stephanosio added the bug The issue is a bug, or the PR is fixing a bug label Nov 8, 2019
@stephanosio stephanosio mentioned this pull request Nov 12, 2019
25 tasks
@stephanosio stephanosio added this to the v2.2.0 milestone Dec 9, 2019
@stephanosio stephanosio force-pushed the cortex_r_fix_irq_mgmt branch from 8d2b0b3 to e1f1bd1 Compare January 2, 2020 09:37
@stephanosio
Copy link
Member Author

Rebased

@stephanosio stephanosio closed this Jan 2, 2020
@stephanosio stephanosio reopened this Jan 2, 2020
This commit fixes incorrect Cortex-R interrupt lock, unlock and state
check function implementations.

The issues can be summarised as follows:

1. The current implementation of 'z_arch_irq_lock' returns the value
  of CPSR as the IRQ key and, since CPSR contains many other state
  bits, this caused 'z_arch_irq_unlocked' to return false even when
  IRQ is unlocked. This problem is fixed by isolating only the I-bit
  of CPSR and returning this value as the IRQ key, such that it
  returns a non-zero value when interrupt is disabled.

2. The current implementation of 'z_arch_irq_unlock' directly updates
  the value of CPSR control field with the IRQ key and this can cause
  other state bits in CPSR to be corrupted. This problem is fixed by
  conditionally enabling interrupt using CPSIE instruction when the
  value of IRQ key is a zero.

3. The current implementation of 'z_arch_is_in_isr' checks the value
  of CPSR MODE field and returns true if its value is IRQ or FIQ.
  While this does not normally cause an issue, the function can return
  false when IRQ offloading is used because the offload function
  executes in SVC mode. This problem is fixed by adding check for SVC
  mode.

Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
@stephanosio stephanosio linked an issue Feb 10, 2020 that may be closed by this pull request
3 tasks
This commit introduces the common tick margin definition that can be
used to specify the maximum allowable deviation from the expected
number of ticks.

Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
The Xilinx QEMU, used to emulate the Xilinx ZynqMP platform, is
particularly unstable in terms of timing.

This commit increases the tick margin for the Xilinx ZynqMP platform
from 1 to 5 in order to allow the sleep test to pass with a reasonable
repeatability.

Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
This commit removes the ignore tags for the tests that work after the
changes in the PR zephyrproject-rtos#22037.

Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
@stephanosio stephanosio force-pushed the cortex_r_fix_irq_mgmt branch from e1f1bd1 to 99665a2 Compare February 10, 2020 04:24
@zephyrbot zephyrbot added area: Boards area: Tests Issues related to a particular existing or missing test area: Kernel labels Feb 10, 2020
@andrewboie andrewboie merged commit 89132fe into zephyrproject-rtos:master Feb 11, 2020
@stephanosio stephanosio deleted the cortex_r_fix_irq_mgmt branch April 23, 2020 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: ARM ARM (32-bit) Architecture area: Boards area: Kernel area: Tests Issues related to a particular existing or missing test bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

qemu_cortex_r5 excludes too many tests
6 participants