arch: arm: Fix Cortex-R power management interrupt handling

[stephanosio]

The system power management handling code in the '_isr_wrapper' enables
interrupts by executing the 'cpsie i' instruction, which causes a
system crash on the Cortex-R devices because the Cortex-R arch port
does not support nested interrupts at this time.

This commit restricts the interrupt state manipulations in the system
power management code to the Cortex-M arch, in order to prevent
interrupt nesting on other AArch32 family archs (only Cortex-R for
now).

Signed-off-by: Stephanos Ioannidis <[email protected]>

p.s. I am planning to implement arch-level interrupt nesting support for the Cortex-R (and AArch32 Cortex-A) in the near future.

[ioannisg]

Looks fine with me, I've got two comments

• we're mixing CPU_CORTEX_M and ARMV..... Kconfig options here; could we stick to one variant of Kconfigs?
• would it be possible to explain inline why the change-processor-state instructions are not used for cotex-R?

[stephanosio]

• we're mixing CPU_CORTEX_M and ARMV..... Kconfig options here; could we stick to one variant of Kconfigs?

The main reason I went with defined(CONFIG_CPU_CORTEX_M) instead of defined(CONFIG_ARMV6_M_ARMV8_M_BASELINE) || defined(CONFIG_ARMV7_M_ARMV8_M_MAINLINE) is that the former is much shorter and simpler to use (it would be nice if there is a symbol like CONFIG_ARM_M or CONFIG_ARMVX_M, but then that is pretty much what CONFIG_CPU_CORTEX_M is in the current configuration layout).

If you still think we should stick with the longer form, I will change it.

• would it be possible to explain inline why the change-processor-state instructions are not used for cotex-R?

The cpsid i and cpsie i instructions used there basically translate to irq_lock() and irq_unlock(), respectively, for checking/updating _kernel.idle and calling z_sys_power_save_idle_exit (many things will go wrong if an ISR is preempted by a higher priority IRQ while this happens).

This is necessary on the Cortex-M because the it supports interrupt nesting and hence interrupts are enabled by default when entering an ISR (this is automatically done by the NVIC core).

As for the Cortex-R, the current arch port does not/cannot support interrupt nesting because context preservation is implemented in a quite incorrect/non-standard way (I am planning to address this in the near future). Note that, without interrupt nesting support, interrupts are (and must be) disabled inside an ISR; hence, it is not necessary to use a lock for checking and updating _kernel.idle.

I will add FIXME comments referencing this PR, since they should be removed once interrupt nesting support is implemented in the Cortex-R arch port (which will eventually become AArch32 Cortex-A port as well).

[ioannisg]

Thanks for the update @stephanosio , I think the fixme text looks fine

The cpsid i and cpsie i instructions used there basically translate to irq_lock() and irq_unlock(), respectively, for checking/updating _kernel.idle and calling z_sys_power_save_idle_exit (many things will go wrong if an ISR is preempted by a higher priority IRQ while this happens).

As discussed offline this is not accurate for ARMv7-M where CPS does more than IRQ_lock/unlock, which just modify the BASEPRI.