Cortex-R Port Improvement for 2.2

[stephanosio]

This PR provides a number of improvements to the existing Cortex-R port.

Move PLATFORM_SPECIFIC_INIT declaration from Cortex-M Kconfig to the
ARM arch Kconfig in order to make it available for all ARM variants.

The rationale is that there is really no good reason why
platform-specific initialisation should be a Cortex-M-specific feature
and that Cortex-R port is expected to utilise this in a near future.

1. Allow CPU_HAS_FPU_DOUBLE_PRECISION on Cortex-R4 and Cortex-R5, as
   the optional VFPv3-D16 is, by default, a double precision unit.

2. Add GCC compiler support for Cortex-R4F and Cortex-R5F floating
   point unit. The FPU type on these cores is fixed to VFPv3-D16,
   which supports double precision by default. While later revisions
   of Cortex-R5F can have single precision VFPv3-D16, GCC does not
   have a separate type for it.

Add a new Kconfig configuration for specifying Dual-redundant Core
Lock-step (DCLS) processor topology.

1. Register initialisation is only required when Dual-redundant Core
   Lock-step (DCLS) is implemented in hardware. This initialisation is
   required on DCLS only because the architectural registers are in an
   indeterminate state after reset and therefore the initial register
   state of the two parallel executing cores are not guaranteed to be
   identical, which can lead to DCCM detecting it as a hardware fault.
   A conditional compilation check for this hardware configuration
   using the newly added CONFIG_CPU_HAS_DCLS flag has been added.

2. The existing CPU register initialisation code did not take into
   account the banked registers for every execution mode. The new
   implementation ensures that all architectural registers of every
   mode are initialised.

3. Add VFP register initialisation for when floating-point support is
   enabled and the core is configured in DCLS topology. This
   initialisation sequence is required for the same reason given in
   the first issue.

4. Add provision for platform-specific initialisation on Cortex-R
   using PLATFORM_SPECIFIC_INIT config and z_platform_init function.

5. Remove seemingly pointless and inadequately defined STACK_MARGIN.
   Not only does it violate the 8-byte stack alignment rule, it does
   not provide any form of real stack protection.

This commit reorganises the existing arch/arm/irq_manage.c file into
three separate files:

* arch/arm/irq.c: Common IRQ handlers and management routines
* arch/arm/cortex_m/irq_manage.c: Cortex-M IRQ management routines
* arch/arm/cortex_r/irq_manage.c: Cortex-R IRQ management routines

This commit fixes the following incorrect implementation of Cortex-R
interrupt management logic:

1. 'irq' parameter to interrupt management functions are manipulated
   in an interrupt controller implementation-specific way before being
   passed down to the interrupt controller driver. Since Cortex-R does
   not designate a specific interrupt controller to be used, these
   functions must remain interrupt controller-agnostic and any
   implementation-specific details involving 'irq' parameter should be
   handled by the interrupt controller driver.

2. 'z_arch_irq_is_enabled' is supposed to return the state of the
   interrupt line specified by the 'irq' parameter, but the current
   implementation returns the global interrupt controller state. This
   problem is fixed by calling the interrupt controller driver
   function `irq_line_is_enabled_next_level` instead.

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.

z_arm_exc_exit (z_arm_int_exit) requires the current execution mode to
be specified as a parameter (through r0). This is not necessary because
this value can be directly read from CPSR.

This commit modifies the exception return function to retrieve the
current execution mode from CPSR and removes all provisions for passing
the execution mode parameter.

This commit implements direct interrupt service routine support for
Cortex-R.

On Cortex-R, the direct ISR function must be declared 'naked' with manual
context preservation due to the following reasons:

1. 'z_arm_int_exit' does not return to the caller on Cortex-R (whereas,
  on Cortex-M, it can either return to the caller or directly exit the
  IRQ mode depending on the value of LR).

2. If 'z_arm_int_exit' is called (when ISR body returns true), since
  this function does not return to the caller on Cortex-R, the
  registers pushed into stack by the compiler will not get popped.

3. The caller-saved registers must be saved into the system mode stack
  because a context switch can occur.

This commit makes the following changes to allow choosing either ARM
or Thumb instruction set for C code compilation:

1. Select 'ISA_THUMB2' for 'ARMV7_R'. The semantic of 'ISA_THUMB2' is
  that the processor supports Thumb instruction set. Since ARMv7-R
  architecture supports Thumb-2 instruction set, this option should be
  selected alongside 'ISA_ARM'.

2. Add 'COMPILER_ISA_THUMB2' option to make C code compilation using
  Thumb instruction set configurable.

3. When 'ISA_ARM' and 'ISA_THUMB2' are selected simultaneously (i.e.
  on Cortex-R and Cortex-A processors), make all ASM functions use ARM
  instruction set by default.

This commit defines a new Cortex-R configuration 'VIC_IRQ_VECTOR' and
implements IRQ initialisation routine to configure SCTLR.VE.

This commit relocates the exception vector table address range
configuration routine that was previously implemented as part of
Cortex-R architecture reset function to SoC platform-specific
initialisation routine.

The existing isr_tables implementation does not allow enabling only
hardware interrupt vector table without software isr table.

This commit ensures that CONFIG_GEN_IRQ_VECTOR_TABLE can be used
without setting CONFIG_GEN_SW_ISR_TABLE.

Make hardware interrupt vector table generation default on Cortex-R
unless the SoC config specifies not to.

This is desirable because most Cortex-R SoCs implement an interrupt
controller that supports direct vectoring through the VIC port.

The current Cortex-R interrupt system relies on the multi-level
interrupt mechanism and 'irq_nextlevel' public interface.

This is a less-than-ideal implementation for the following reasons:

1. SoC main interrupt controller (e.g. GIC) is not a 2nd level
  interrupt controller; in fact, it is the root interrupt controller
  and therefore should be treated as such.

2. The only reason for using 'irq_nextlevel' here is to interface
  Cortex-R arch implementation to the interrupt controller functions.
  Since there is no nesting or multiple instances of an interrupt
  controller involved, there is really no point in adding such an
  abstraction.

3. 2nd level topology adds many unnecessary abstractions and results
  in strange coding artefacts as well as performance penalty due to
  additional branching.

The solution provided by this commit is as follows:

1. Define a set of SoC layer interrupt management functions
  (z_soc_irq_*) to be used by the Cortex-R variants that do not have
  an architecturally specified interrupt controller.

2. Map arch interrupt management functiosn to the SoC layer interrupt
  management functions (e.g. map 'z_arch_irq_enable' to
  'z_soc_irq_enable').

3. The SoC layer interrupt management functions are implemented by
  soc/arm/*. If an SoC incorporates a proprietary (not commonly used)
  interrupt controller, the interrupt management functions can be
  simply implemented within the SoC layer. If an SoC incorporates a
  standard interrupt controller such as GIC, these functions can be
  mapped to the appropriate interrupt controller driver functions
  either through the 'irq_nextlevel' interface, or a direct driver-
  specific interface exported by include/drivers/interrupt_controller.

This commit updates 'xilinx_zynqmp' to use the refactored Cortex-R
interrupt system.

For more details on the refactored Cortex-R interrupt system, see the
PR #19698.

This PR is part of the preparations for #19644.

[stephanosio]

Instead of making multiple PRs for various Cortex-R fix/refactoring/improvement tasks, I will be consolidating them all into this PR. Please do not merge this in the meantime.

[stephanosio]

Note for TEINIT and SCTLR.TE
Zephyr RTOS assumes that Cortex-R processor core is synthesised with TEINIT=0 (the default exception vector entry mode is ARM).

The rationale for this assumption is as follows:

1. All commonly available Cortex-R processors seem to use TEINIT=0.
2. There is really no good reason to configure the CPU exception vector entry mode to be Thumb, as doing so will make branching to handlers while preserving all the registers very difficult.

[zephyrbot]

Some checks failed. Please fix and resubmit.

checkpatch issues

-:363: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#363: FILE: arch/arm/core/cortex_r/irq_init.c:24:
+    z_soc_irq_init();$

-:367: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#367: FILE: arch/arm/core/cortex_r/irq_init.c:28:
+    unsigned int sctlr = __get_SCTLR();$

-:368: WARNING:LINE_SPACING: Missing a blank line after declarations
#368: FILE: arch/arm/core/cortex_r/irq_init.c:29:
+    unsigned int sctlr = __get_SCTLR();
+    sctlr |= SCTLR_VE_Msk;

-:368: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#368: FILE: arch/arm/core/cortex_r/irq_init.c:29:
+    sctlr |= SCTLR_VE_Msk;$

-:369: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#369: FILE: arch/arm/core/cortex_r/irq_init.c:30:
+    __set_SCTLR(sctlr);$

-:1363: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#1363: FILE: include/arch/arm/irq.h:63:
+    unsigned int irq, unsigned int prio, unsigned int flags);$

-:1574: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#1574: FILE: include/toolchain/gcc.h:222:
+#define FUNC_CODE() .code 32

-:1581: ERROR:MULTISTATEMENT_MACRO_USE_DO_WHILE: Macros with multiple statements should be enclosed in a do - while loop
#1581: FILE: include/toolchain/gcc.h:227:
+#define FUNC_CODE() .thumb;

-:1581: WARNING:TRAILING_SEMICOLON: macros should not use a trailing semicolon
#1581: FILE: include/toolchain/gcc.h:227:
+#define FUNC_CODE() .thumb;

-:1603: ERROR:MULTISTATEMENT_MACRO_USE_DO_WHILE: Macros with multiple statements should be enclosed in a do - while loop
#1603: FILE: include/toolchain/gcc.h:350:
+#define _ASM_FILE_PROLOGUE .text; .code 32

-:1718: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#1718: FILE: soc/arm/xilinx_zynqmp/irq.c:19:
+    arm_gic_init();$

-:1724: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#1724: FILE: soc/arm/xilinx_zynqmp/irq.c:25:
+    arm_gic_irq_enable(irq);$

-:1730: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#1730: FILE: soc/arm/xilinx_zynqmp/irq.c:31:
+    arm_gic_irq_disable(irq);$

-:1735: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#1735: FILE: soc/arm/xilinx_zynqmp/irq.c:36:
+    return arm_gic_irq_is_enabled(irq);$

-:1740: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#1740: FILE: soc/arm/xilinx_zynqmp/irq.c:41:
+    return arm_gic_irq_get_active();$

-:1745: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#1745: FILE: soc/arm/xilinx_zynqmp/irq.c:46:
+    arm_gic_irq_eoi(irq);$

-:1760: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#1760: FILE: soc/arm/xilinx_zynqmp/irq.c:61:
+    unsigned int irq, unsigned int prio, unsigned int flags)$

-:1762: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#1762: FILE: soc/arm/xilinx_zynqmp/irq.c:63:
+    arm_gic_irq_set_priority(irq, prio, flags);$

-:1786: WARNING:LINE_SPACING: Missing a blank line after declarations
#1786: FILE: soc/arm/xilinx_zynqmp/soc.c:37:
+	unsigned int sctlr = __get_SCTLR();
+	sctlr &= ~SCTLR_V_Msk;

- total: 2 errors, 17 warnings, 1574 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

[stephanosio]

@ioannisg Here is the Cortex-R interrupt system changes.

[stephanosio]

@bbolen I have made some major changes to the Cortex-R port.

It would be very helpful if you could review this.

[stephanosio]

Force-pushing to rebase onto the latest master and provide a minor inline asm styling fix (using __asm__ instead of __asm).

[stephanosio]

Force-pushing to rebase onto the latest master and provide an improved interrupt controller-agnostic implementation of Cortex-R (and to be future AArch32 Cortex-A) arch.

[stephanosio]

Here is a quick note on how to test this in qemu_cortex_r5:

1. Download and install the test SDK release based on Add Xilinx QEMU (qemu-system-xilinx-aarch64) sdk-ng#138 from the following link:
   https://github.com/stephanosio/zephyr-sdk-ng/releases/tag/stephanos-v0.11.0-alpha-arm_generic_fdt_2

2. Set QEMU_BIN_PATH to the test SDK path (this allows ninja run to use the qemu-system-xilinx-aarch64 from the test SDK release).

   export QEMU_BIN_PATH=/opt/zephyr-sdk/2.2/sysroots/x86_64-pokysdk-linux/usr/bin
   

3. Check out this PR.

   git fetch upstream pull/19698/head:test_cortex_r_improvement
   git checkout test_cortex_r_improvement
   

4. Rebase onto PR Refactor and fix xilinx_zynqmp SoC definition #20267 (fix incorrect dts xilinx_zynqmp GIC address).

   git fetch upstream pull/20267/head:test_fix_zynqmp
   git rebase test_fix_zynqmp
   

5. Rebase onto PR boards: arm: qemu_cortex_r5: Use arm-generic-fdt machine type. #20298 (use arm-generic-fdt QEMU machine type for qemu_cortex_r5 emulation).

   git fetch upstream pull/20298/head:test_qemu_cortex_r5_use_xilinx_qemu
   git rebase test_qemu_cortex_r5_use_xilinx_qemu
   

6. Build and run kernel tests (e.g. tests/kernel/threads/thread_apis).

Most kernel tests should pass! (without the changes in this PR, many kernel tests will fail due to incorrect interrupt management implementation). Any failing tests, even with this PR, are due to timing instability of QEMU as well as possibly incorrect implementation of Cadence TTC system timer driver (verification required, this should be addressed separately in the future).

On a side note, the reason that this currently works without using the correct GIC driver is that the GIC-400 GICv2 driver currently being used is mostly compatible with the PL390 GICv1 (the current driver sets a few extra registers that do not exist on GICv1, but this does not really hinder GIC operation; this is to be addressed in the context of #20217).

[andrewboie]

Please rebase and address checkpatch issues. You guys are the experts on this HW but I don't see anything that sticks out to me.

[andrewboie]

@stephanosio what is the status of this PR?

[stephanosio]

@stephanosio what is the status of this PR?

@andrewboie Most of the commits in this PR were separated into smaller PRs for easier reviewing. As for the unmerged commits in this PR, I plan to open separate smaller PRs for them as well, once all the dependencies are merged.

[stephanosio]

PR Status
(checked = has PR)

• arch: arm: Make PLATFORM_SPECIFIC_INIT available to all ARM variants. (arch: arm: Make PLATFORM_SPECIFIC_INIT available to all ARM variants. #20468)
• arch: arm: Add Cortex-R floating point configuration support. (arch: arm: Add Cortex-R floating point configuration support. #20469)
• arch: Add Dual-redundant Core Lock-step (DCLS) configuration. (arch: arm: Rewrite Cortex-R reset vector function. #20473)
• arch: arm: Rewrite Cortex-R reset vector function. (arch: arm: Rewrite Cortex-R reset vector function. #20473)
• arch: arm: Reorganise ARM interrupt management routines.
  arch: arm: Fix incorrect Cortex-R interrupt management logic.
• arch: arm: Fix incorrect Cortex-R interrupt state control logic. (arch: arm: Fix incorrect Cortex-R interrupt state control logic. #20474)
• arch: arm: Optimise Cortex-R exception return function. (arch: arm: Optimise Cortex-R exception return function. #20477)
• isr_tables: Support hardware interrupt vector table-only configuration. (isr_tables: Support hardware interrupt vector table-only configuration. #22742)
  arch: arm: Make hardware IVT generation default on Cortex-R.
• arch: arm: Add Cortex-R direct interrupt service routine support.
  (NOTE: do it after VIC support is implemented)
• arch: arm: Allow selecting instruction set for C code compilation. (arch: arm: aarch32: Allow selecting compiler instruction set #22741)
• arch: arm: Add vectored interrupt support for Cortex-R.
  (NOTE: do it after Refactor ARM interrupt system (Cortex-A & Cortex-R) #22718 and arch: arm: cortex_r: Add CMSIS support #22740 are merged)
• soc: arm: xilinx_zynqmp: Relocate platform-specific initialisation. (arch: arm: Rewrite Cortex-R reset vector function. #20473)
  arch: arm: Refactor Cortex-R interrupt system. (superseded by Refactor ARM interrupt system (Cortex-A & Cortex-R) #22718)
  soc: arm: xilinx_zynqmp: Use the refactored Cortex-R interrupt system. (superseded by Refactor ARM interrupt system (Cortex-A & Cortex-R) #22718)
• arch: arm: Update Cortex-R port to use CMSIS-Core(R). (arch: arm: cortex_r: Add CMSIS support #22740)
• soc: arm: xilinx_zynqmp: Use CMSIS-Core(R). (arch: arm: cortex_r: Add CMSIS support #22740)