From 797d5e6060697ae9cb44e27871dfea5cbeaadece Mon Sep 17 00:00:00 2001 From: Stephanos Ioannidis Date: Tue, 15 Oct 2019 12:45:51 +0900 Subject: [PATCH 1/4] arch: arm: Fix incorrect Cortex-R interrupt state control logic. 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 --- arch/arm/include/aarch32/cortex_r/exc.h | 4 +++- include/arch/arm/aarch32/asm_inline_gcc.h | 20 ++++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/arch/arm/include/aarch32/cortex_r/exc.h b/arch/arm/include/aarch32/cortex_r/exc.h index 05b0235204ba4d..5d965d6bb32b37 100644 --- a/arch/arm/include/aarch32/cortex_r/exc.h +++ b/arch/arm/include/aarch32/cortex_r/exc.h @@ -42,7 +42,9 @@ static ALWAYS_INLINE bool arch_is_in_isr(void) : "=r" (status) : : "memory", "cc"); status &= MODE_MASK; - return (status == MODE_FIQ) || (status == MODE_IRQ); + return (status == MODE_FIQ) || + (status == MODE_IRQ) || + (status == MODE_SVC); } /** diff --git a/include/arch/arm/aarch32/asm_inline_gcc.h b/include/arch/arm/aarch32/asm_inline_gcc.h index f2d881d656b0f7..2ff8e45c0d331f 100644 --- a/include/arch/arm/aarch32/asm_inline_gcc.h +++ b/include/arch/arm/aarch32/asm_inline_gcc.h @@ -22,6 +22,10 @@ #include #include +#if defined(CONFIG_CPU_CORTEX_R) +#include +#endif + #ifdef __cplusplus extern "C" { #endif @@ -58,8 +62,10 @@ static ALWAYS_INLINE unsigned int arch_irq_lock(void) : "i"(_EXC_IRQ_DEFAULT_PRIO) : "memory"); #elif defined(CONFIG_ARMV7_R) - __asm__ volatile("mrs %0, cpsr;" - "cpsid i" + __asm__ volatile( + "mrs %0, cpsr;" + "and %0, #" TOSTR(I_BIT) ";" + "cpsid i;" : "=r" (key) : : "memory", "cc"); @@ -91,10 +97,12 @@ static ALWAYS_INLINE void arch_irq_unlock(unsigned int key) "isb;" : : "r"(key) : "memory"); #elif defined(CONFIG_ARMV7_R) - __asm__ volatile("msr cpsr_c, %0" - : - : "r" (key) - : "memory", "cc"); + if (key) { + return; + } + __asm__ volatile( + "cpsie i;" + : : : "memory", "cc"); #else #error Unknown ARM architecture #endif /* CONFIG_ARMV6_M_ARMV8_M_BASELINE */ From 4864817fa632c95d1cd9352112e8caba5133e8f4 Mon Sep 17 00:00:00 2001 From: Stephanos Ioannidis Date: Mon, 10 Feb 2020 12:29:44 +0900 Subject: [PATCH 2/4] tests: sleep: Add tick margin definition 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 --- tests/kernel/sleep/src/main.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/kernel/sleep/src/main.c b/tests/kernel/sleep/src/main.c index ac7ec2fe276410..02038c89045fdf 100644 --- a/tests/kernel/sleep/src/main.c +++ b/tests/kernel/sleep/src/main.c @@ -24,6 +24,8 @@ #define ONE_SECOND_ALIGNED \ (u32_t)(k_ticks_to_ms_floor64(k_ms_to_ticks_ceil32(ONE_SECOND) + _TICK_ALIGN)) +#define TICK_MARGIN 1 + static struct k_sem test_thread_sem; static struct k_sem helper_thread_sem; static struct k_sem task_sem; @@ -88,7 +90,7 @@ static int sleep_time_valid(u32_t start, u32_t end, u32_t dur) { u32_t dt = end - start; - return dt >= dur && dt <= (dur + 1); + return dt >= dur && dt <= (dur + TICK_MARGIN); } static void test_thread(int arg1, int arg2) @@ -120,7 +122,7 @@ static void test_thread(int arg1, int arg2) k_sleep(ONE_SECOND); end_tick = k_uptime_get_32(); - if (end_tick - start_tick > 1) { + if (end_tick - start_tick > TICK_MARGIN) { TC_ERROR(" *** k_wakeup() took too long (%d ticks)\n", end_tick - start_tick); return; @@ -134,7 +136,7 @@ static void test_thread(int arg1, int arg2) k_sleep(ONE_SECOND); end_tick = k_uptime_get_32(); - if (end_tick - start_tick > 1) { + if (end_tick - start_tick > TICK_MARGIN) { TC_ERROR(" *** k_wakeup() took too long (%d ticks)\n", end_tick - start_tick); return; @@ -148,7 +150,7 @@ static void test_thread(int arg1, int arg2) k_sleep(ONE_SECOND); /* Task will execute */ end_tick = k_uptime_get_32(); - if (end_tick - start_tick > 1) { + if (end_tick - start_tick > TICK_MARGIN) { TC_ERROR(" *** k_wakeup() took too long (%d ticks) at LAST\n", end_tick - start_tick); return; From 287086eb8dc1e6aac775b9e821fc40711b1fedfa Mon Sep 17 00:00:00 2001 From: Stephanos Ioannidis Date: Mon, 10 Feb 2020 12:39:10 +0900 Subject: [PATCH 3/4] tests: sleep: Increase tick margin for Xilinx QEMU 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 --- tests/kernel/sleep/src/main.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/kernel/sleep/src/main.c b/tests/kernel/sleep/src/main.c index 02038c89045fdf..7651a74f77b53a 100644 --- a/tests/kernel/sleep/src/main.c +++ b/tests/kernel/sleep/src/main.c @@ -24,7 +24,16 @@ #define ONE_SECOND_ALIGNED \ (u32_t)(k_ticks_to_ms_floor64(k_ms_to_ticks_ceil32(ONE_SECOND) + _TICK_ALIGN)) +#if defined(CONFIG_SOC_XILINX_ZYNQMP) +/* + * The Xilinx QEMU, used to emulate the Xilinx ZynqMP platform, is particularly + * unstable in terms of timing. The tick margin of at least 5 is necessary to + * allow this test to pass with a reasonable repeatability. + */ +#define TICK_MARGIN 5 +#else #define TICK_MARGIN 1 +#endif static struct k_sem test_thread_sem; static struct k_sem helper_thread_sem; From 99665a250c05204670f808a1f83e9856b944f92d Mon Sep 17 00:00:00 2001 From: Stephanos Ioannidis Date: Mon, 10 Feb 2020 13:22:43 +0900 Subject: [PATCH 4/4] boards: qemu_cortex_r5: Remove ignore tags for working tests This commit removes the ignore tags for the tests that work after the changes in the PR #22037. Signed-off-by: Stephanos Ioannidis --- boards/arm/qemu_cortex_r5/qemu_cortex_r5.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/boards/arm/qemu_cortex_r5/qemu_cortex_r5.yaml b/boards/arm/qemu_cortex_r5/qemu_cortex_r5.yaml index 10140ff2c50a52..2ae4d72dd23a99 100644 --- a/boards/arm/qemu_cortex_r5/qemu_cortex_r5.yaml +++ b/boards/arm/qemu_cortex_r5/qemu_cortex_r5.yaml @@ -13,8 +13,6 @@ testing: default: true ignore_tags: - benchmark - - cmsis_rtos - interrupt - - kernel - memory_protection - userspace