Fixup ARM MPU to use opaque types / Rebase of #500

arch/arm/core/cortex_m/mpu/arm_core_mpu.c

@@ -24,7 +24,7 @@ void configure_mpu_stack_guard(struct k_thread *thread)
 {
 	arm_core_mpu_disable();
 	arm_core_mpu_configure(THREAD_STACK_GUARD_REGION,
-			       thread->stack_info.start,
+			       thread->stack_info.start - STACK_ALIGN,
 			       thread->stack_info.size);
 	arm_core_mpu_enable();
 }

arch/arm/core/thread.c

@@ -64,7 +64,6 @@ void _new_thread(struct k_thread *thread, k_thread_stack_t stack,
 
 	char *stackEnd = pStackMem + stackSize;
 	struct __esf *pInitCtx;
-
 	_new_thread_init(thread, pStackMem, stackSize, priority, options);
 
 	/* carve the thread entry struct from the "base" of the stack */

include/arch/arm/arch.h

@@ -42,8 +42,96 @@ extern "C" {
 #include <arch/arm/cortex_m/sys_io.h>
 #include <arch/arm/cortex_m/nmi.h>
 #endif
-
+#if defined(CONFIG_MPU_STACK_GUARD)
+#define STACK_ALIGN  32
+#else  /* CONFIG_MPU_STACK_GUARD */
 #define STACK_ALIGN  4
+#endif
+
+/**
+ * @brief Declare a toplevel thread stack memory region
+ *
+ * This declares a region of memory suitable for use as a thread's stack.
+ *
+ * This is the generic, historical definition. Align to STACK_ALIGN and put in
+ * 'noinit' section so that it isn't zeroed at boot
+ *
+ * The declared symbol will always be a character array which can be passed to
+ * k_thread_create, but should otherwise not be manipulated.
+ *
+ * It is legal to precede this definition with the 'static' keyword.
+ *
+ * It is NOT legal to take the sizeof(sym) and pass that to the stackSize
+ * parameter of k_thread_create(), it may not be the same as the
+ * 'size' parameter. Use K_THREAD_STACK_SIZEOF() instead.
+ *
+ * @param sym Thread stack symbol name
+ * @param size Size of the stack memory region
+ */
+#define _ARCH_THREAD_STACK_DEFINE(sym, size) \
+	struct _k_thread_stack_element __noinit __aligned(STACK_ALIGN) \
+		sym[size+STACK_ALIGN]
+
+/**
+ * @brief Declare a toplevel array of thread stack memory regions
+ *
+ * Create an array of equally sized stacks. See K_THREAD_STACK_DEFINE
+ * definition for additional details and constraints.
+ *
+ * This is the generic, historical definition. Align to STACK_ALIGN and put in
+ * 'noinit' section so that it isn't zeroed at boot
+ *
+ * @param sym Thread stack symbol name
+ * @param nmemb Number of stacks to declare
+ * @param size Size of the stack memory region
+ */
+#define _ARCH_THREAD_STACK_ARRAY_DEFINE(sym, nmemb, size) \
+	struct _k_thread_stack_element __noinit __aligned(STACK_ALIGN) \
+		sym[nmemb][size+STACK_ALIGN]
+
+/**
+ * @brief Declare an embedded stack memory region
+ *
+ * Used for stacks embedded within other data structures. Use is highly
+ * discouraged but in some cases necessary. For memory protection scenarios,
+ * it is very important that any RAM preceding this member not be writable
+ * by threads else a stack overflow will lead to silent corruption. In other
+ * words, the containing data structure should live in RAM owned by the kernel.
+ *
+ * @param sym Thread stack symbol name
+ * @param size Size of the stack memory region
+ */
+#define _ARCH_THREAD_STACK_MEMBER(sym, size) \
+	struct _k_thread_stack_element __aligned(STACK_ALIGN) \
+		sym[size+STACK_ALIGN]
+
+/**
+ * @brief Return the size in bytes of a stack memory region
+ *
+ * Convenience macro for passing the desired stack size to k_thread_create()
+ * since the underlying implementation may actually create something larger
+ * (for instance a guard area).
+ *
+ * The value returned here is guaranteed to match the 'size' parameter
+ * passed to K_THREAD_STACK_DEFINE and related macros.
+ *
+ * @param sym Stack memory symbol
+ * @return Size of the stack
+ */
+#define _ARCH_THREAD_STACK_SIZEOF(sym) (sizeof(sym) - STACK_ALIGN)
+
+/**
+ * @brief Get a pointer to the physical stack buffer
+ *
+ * Convenience macro to get at the real underlying stack buffer used by
+ * the CPU. Guaranteed to be a character pointer of size K_THREAD_STACK_SIZEOF.
+ * This is really only intended for diagnostic tools which want to examine
+ * stack memory contents.
+ *
+ * @param sym Declared stack symbol name
+ * @return The buffer itself, a char *
+ */
+#define _ARCH_THREAD_STACK_BUFFER(sym) ((char *)(sym + STACK_ALIGN))
 
 #ifdef __cplusplus
 }

samples/mpu/mpu_stack_guard_test/README.rst

@@ -35,33 +35,40 @@ To build the test with the MPU enabled and the stack guard feature present:
 Sample Output
 =============
 
-With the MPU disabled:
+With the MPU enabled but the stack guard feature disabled:
 
 .. code-block:: console
 
-   ***** BOOTING ZEPHYR OS v1.7.99 - BUILD: Mar 29 2017 11:07:09 *****
-   MPU STACK GUARD Test
-   Canary Initial Value = 0xf0cacc1a
-   Canary = 0x200003a8     Test not passed.
-   ...
-
-With the MPU enabled and the stack guard feature present:
+    ***** BOOTING ZEPHYR OS v1.8.99 - BUILD: Jun 15 2017 23:12:13 *****
+    STACK_ALIGN 4
+    MPU STACK GUARD Test
+    Canary Initial Value = 0xf0cacc1a threads 0x20000b9c
+    Canary = 0xfffffff5     Test not passed.
+    ***** MPU FAULT *****
+      Executing thread ID (thread): 0x20000b9c
+      Faulting instruction address:  0x80025b0
+      Data Access Violation
+      Address: 0x8000edb
+    Fatal fault in thread 0x20000b9c! Aborting.
+    ***** HARD FAULT *****
+      Fault escalation (see below)
+    ***** MPU FAULT *****
+      Executing thread ID (thread): 0x20000b9c
+      Faulting instruction address:  0x8001db6
+      Data Access Violation
+      Address: 0x8000edb
+    Fatal fault in ISR! Spinning...
+
+With the MPU enabled and the stack guard feature enabled:
 
 .. code-block:: console
 
-   ***** BOOTING ZEPHYR OS v1.7.99 - BUILD: Mar 29 2017 11:20:10 *****
-   MPU STACK GUARD Test
-   Canary Initial Value = 0xf0cacc1a
-   [general] [DBG] arm_core_mpu_configure: Region info: 0x200003ac 0x400
-   ***** MPU FAULT *****
-     Executing thread ID (thread): 0x200003ac
-     Faulting instruction address:  0x0
-     Stacking error
-   Fatal fault in thread 0x200003ac! Aborting.
-   ***** HARD FAULT *****
-     Fault escalation (see below)
-   ***** MPU FAULT *****
-     Executing thread ID (thread): 0x200003ac
-     Faulting instruction address:  0x8000466
-     Stacking error
-   Fatal fault in ISR! Spinning...
+    ***** BOOTING ZEPHYR OS v1.8.99 - BUILD: Jun 15 2017 23:05:56 *****
+    STACK_ALIGN 20
+    MPU STACK GUARD Test
+    Canary Initial Value = 0xf0cacc1a threads 0x20000be0
+    ***** MPU FAULT *****
+      Executing thread ID (thread): 0x20000be0
+      Faulting instruction address:  0x8001dc2
+      Stacking error
+    Fatal fault in thread 0x20000be0! Aborting.

samples/mpu/mpu_stack_guard_test/prj_stack_guard.conf

@@ -1,5 +1,3 @@
 CONFIG_STDOUT_CONSOLE=y
 CONFIG_MPU_STACK_GUARD=y
 CONFIG_SYS_LOG=y
-CONFIG_SYS_LOG_DEFAULT_LEVEL=4
-CONFIG_SYS_LOG_OVERRIDE_LEVEL=4

samples/mpu/mpu_stack_guard_test/src/main.c

@@ -10,62 +10,76 @@
 #include <misc/printk.h>
 
 /* size of stack area used by each thread */
-#define STACKSIZE 1024
+#define STACKSIZE  512
 
 /* scheduling priority used by each thread */
 #define PRIORITY 7
 
 /* delay between greetings (in ms) */
 #define SLEEPTIME 500
 
+/* delay between greetings (in ms) */
+#define SCHEDTIME 30
+
 /* Focaccia tastes better than DEEDBEEF ;-) */
 #define STACK_GUARD_CANARY 0xF0CACC1A
 
 struct stack_guard_buffer {
 	/* Make sure canary is not optimized by the compiler */
+	struct k_thread thread;
 	volatile u32_t canary;
 	K_THREAD_STACK_MEMBER(stack, STACKSIZE);
 };
 
-struct stack_guard_buffer buf;
-struct k_thread test_thread;
+#define LAST_THREAD_NUM 6
+struct stack_guard_buffer buf[LAST_THREAD_NUM+1];
 
-u32_t recursive_loop(u32_t counter)
+u32_t recursive_loop(u32_t counter, int num, void *dummy)
 {
-	if (buf.canary != STACK_GUARD_CANARY) {
-		printk("Canary = 0x%08x\tTest not passed.\n", buf.canary);
+
+	if (buf[num].canary != STACK_GUARD_CANARY) {
+		printk("Canary = 0x%08x\tTest not passed.\n", buf[num].canary);
 
 		while (1) {
 			k_sleep(SLEEPTIME);
 		}
 	}
-
 	counter++;
-
-	return recursive_loop(counter) + recursive_loop(counter);
+	if (dummy == 0)
+		return counter;
+	return recursive_loop(counter, num, dummy)
+		+recursive_loop(counter, num, dummy);
 }
 
+
 /* stack_guard_thread is a dynamic thread */
 void stack_guard_thread(void *dummy1, void *dummy2, void *dummy3)
 {
 	ARG_UNUSED(dummy1);
-	ARG_UNUSED(dummy2);
-	ARG_UNUSED(dummy3);
-
-	u32_t result = recursive_loop(0);
+	recursive_loop(0, (int)dummy1, dummy2);
 
-	/* We will never arrive here */
-	printk("Result: %d", result);
 }
 
 void main(void)
 {
-	buf.canary = STACK_GUARD_CANARY;
+	int i;
+	bool failed;
+
+	printk("STACK_ALIGN 0x%x\n", STACK_ALIGN);
+	for (i = 0; i <= LAST_THREAD_NUM; i++) {
+		buf[i].canary = STACK_GUARD_CANARY;
+		failed = (i == LAST_THREAD_NUM) ? true : false;
+		if (failed) {
+			printk("MPU STACK GUARD Test\n");
+			printk("Canary Initial Value = 0x%x threads %p\n",
+					buf[i].canary, &buf[i].thread);
+		}
 
-	printk("MPU STACK GUARD Test\n");
-	printk("Canary Initial Value = 0x%x\n", buf.canary);
+		/* create stack_guard_thread */
+		k_thread_create(&buf[i].thread, buf[i].stack,
+				K_THREAD_STACK_SIZEOF(buf[i].stack),
+				stack_guard_thread, (void *)i,
+				(void *)failed, NULL, PRIORITY, 0, K_NO_WAIT);
+	}
 
-	/* spawn stack_guard_thread */
-	k_thread_create(&test_thread, buf.stack, STACKSIZE, stack_guard_thread,
-			NULL, NULL, NULL, PRIORITY, 0, K_NO_WAIT);
 }