MPU refactor and support for ARMv8 MPU

platform/wscript

@@ -141,6 +141,11 @@ def add_platform_defines(ctx, env):
     # Add umbrella MICRO_FAMILY_QEMU define for all QEMU boards
     if env.MICRO_FAMILY.startswith('QEMU_PEBBLE'):
         defines.append('MICRO_FAMILY_QEMU=1')
+    # MPU architecture: ARMv8-M for Cortex-M33-based families, ARMv7-M otherwise.
+    if env.MICRO_FAMILY in ('SF32LB52', 'QEMU_PEBBLE_ARMCM33'):
+        defines.append('MPU_TYPE_ARMV8M=1')
+    else:
+        defines.append('MPU_TYPE_ARMV7M=1')
     env.append_value('DEFINES', defines)
 
     for cap, val in ctx.capabilities_dict().items():

src/fw/apps/demo/test_mpu_violation.c

@@ -0,0 +1,85 @@
+/* SPDX-FileCopyrightText: 2026 Core Devices LLC */
+/* SPDX-License-Identifier: Apache-2.0 */
+
+#include "test_mpu_violation.h"
+
+#include "applib/app.h"
+#include "applib/app_timer.h"
+#include "applib/ui/app_window_stack.h"
+#include "applib/ui/ui.h"
+#include "font_resource_keys.auto.h"
+#include "kernel/pbl_malloc.h"
+#include "process_state/app_state/app_state.h"
+
+// This demo app deliberately tries to write to memory that the MPU should
+// deny the unprivileged App task (Worker RAM is mapped priv-only). If the
+// MPU is enforcing access protection the App task will MemManage fault and
+// be killed by the kernel; we will never see the "MPU FAILED" text on the
+// display. If we *do* see it, the MPU is not blocking the access.
+//
+// __WORKER_RAM__ is exported by the linker script (src/fw/fw_common.ld).
+extern const uint32_t __WORKER_RAM__[];
+
+typedef struct {
+  Window window;
+  TextLayer text;
+} AppData;
+
+static void prv_attempt_violation(void *data) {
+  AppData *app_data = data;
+
+  volatile uint32_t *forbidden = (volatile uint32_t *)__WORKER_RAM__;
+  *forbidden = 0xDEADBEEF;  // Expected to MemManage-fault and kill the app.
+
+  // Reaching this point means the MPU did NOT block the unprivileged write
+  // to Worker RAM. Surface the failure so the screenshot makes it obvious.
+  text_layer_set_text(&app_data->text, "MPU FAILED");
+  layer_mark_dirty(text_layer_get_layer(&app_data->text));
+}
+
+static void prv_window_load(Window *window) {
+  AppData *app_data = window_get_user_data(window);
+
+  GRect frame;
+  layer_get_frame(window_get_root_layer(window), &frame);
+  GRect text_frame = { .size.h = 64, .size.w = frame.size.w };
+  grect_align(&text_frame, &frame, GAlignCenter, false);
+
+  text_layer_init(&app_data->text, &text_frame);
+  text_layer_set_font(&app_data->text, fonts_get_system_font(FONT_KEY_GOTHIC_28));
+  text_layer_set_text_alignment(&app_data->text, GTextAlignmentCenter);
+  text_layer_set_text(&app_data->text, "POKING\nKERNEL...");
+  layer_add_child(window_get_root_layer(window), (Layer *)&app_data->text);
+
+  // Let the window paint first, then attempt the forbidden write.
+  app_timer_register(500, prv_attempt_violation, app_data);
+}
+
+static void prv_handle_init(void) {
+  AppData *app_data = app_malloc_check(sizeof(AppData));
+  app_state_set_user_data(app_data);
+
+  window_init(&app_data->window, WINDOW_NAME("test_mpu_violation"));
+  window_set_user_data(&app_data->window, app_data);
+  window_set_window_handlers(&app_data->window, &(WindowHandlers) {
+      .load = prv_window_load,
+  });
+
+  app_window_stack_push(&app_data->window, true /* animated */);
+}
+
+static void prv_main(void) {
+  prv_handle_init();
+  app_event_loop();
+}
+
+const PebbleProcessMd* test_mpu_violation_get_info(void) {
+  static const PebbleProcessMdSystem s_info = {
+    .common.main_func = prv_main,
+    // System apps default to privileged; this one must run unprivileged
+    // for the MPU to actually evaluate access against the App task regions.
+    .common.is_unprivileged = true,
+    .name = "Test MPU violation",
+  };
+  return (const PebbleProcessMd *)&s_info;
+}

src/fw/apps/demo/test_mpu_violation.h

@@ -0,0 +1,8 @@
+/* SPDX-FileCopyrightText: 2026 Core Devices LLC */
+/* SPDX-License-Identifier: Apache-2.0 */
+
+#pragma once
+
+#include "process_management/pebble_process_md.h"
+
+const PebbleProcessMd* test_mpu_violation_get_info(void);

src/fw/drivers/mpu.c

@@ -5,9 +5,6 @@
 
 #include "mcu/cache.h"
 #include "system/passert.h"
-#include "util/size.h"
-
-#include <inttypes.h>
 
 #include "FreeRTOS.h"
 #include "task.h"
@@ -26,230 +23,10 @@ extern const uint32_t __SRAM_size__[];
 #endif
 #define SRAM_END (SRAM_BASE + (uint32_t)__SRAM_size__)
 
-typedef struct PermissionMapping {
-  bool priv_read:1;
-  bool priv_write:1;
-  bool user_read:1;
-  bool user_write:1;
-#ifdef MPU_ARMV8
-  uint8_t value:2;
-#else
-  uint8_t value:3;
-#endif
-} PermissionMapping;
-
-static const PermissionMapping s_permission_mappings[] = {
-#ifdef MPU_ARMV8
-  // NOTE(1): we cannot have all accesses disabled, keep RO by privileged code only.
-  // NOTE(2): we cannot have different write access for priv/unpriv, allow R/W to any level
-  { false, false, false, false, 0x2 }, // AP=0b10: RO by privileged code only (1)
-  { true,  true,  false, false, 0x0 }, // AP=0b00: R/W by privileged code only
-  { true,  true,  true,  false, 0x1 }, // AP=0b01: R/W by any privilege level (2)
-  { true,  true,  true,  true,  0x1 }, // AP=0b01: R/W by any privilege level
-  { true,  false, false, false, 0x2 }, // AP=0b10: RO by privileged code only
-  { true,  false, true,  false, 0x3 }, // AP=0b11: RO by by any privilege level
-#else
-  { false, false, false, false, 0x0 },
-  { true,  true,  false, false, 0x1 },
-  { true,  true,  true,  false, 0x2 },
-  { true,  true,  true,  true,  0x3 },
-  { true,  false, false, false, 0x5 },
-  { true,  false, true,  false, 0x6 },
-  { true,  false, true,  false, 0x7 } // Both 0x6 and 0x7 map to the same permissions.
-#endif
-};
-
-static const uint32_t s_cache_settings[] = {
-#ifdef MPU_ARMV8
-  [MpuCachePolicy_NotCacheable] = ARM_MPU_ATTR(ARM_MPU_ATTR_NON_CACHEABLE,
-                                               ARM_MPU_ATTR_NON_CACHEABLE),
-  [MpuCachePolicy_WriteThrough] = ARM_MPU_ATTR(ARM_MPU_ATTR_MEMORY_(1, 0, 1, 0),
-                                               ARM_MPU_ATTR_MEMORY_(1, 0, 1, 0)),
-  [MpuCachePolicy_WriteBackWriteAllocate] = ARM_MPU_ATTR(ARM_MPU_ATTR_MEMORY_(1, 1, 1, 1),
-                                                         ARM_MPU_ATTR_MEMORY_(1, 1, 1, 1)),
-  [MpuCachePolicy_WriteBackNoWriteAllocate] = ARM_MPU_ATTR(ARM_MPU_ATTR_MEMORY_(1, 1, 0, 1),
-                                                           ARM_MPU_ATTR_MEMORY_(1, 1, 0, 1))
-#else
-  [MpuCachePolicy_NotCacheable] = (0x1 << MPU_RASR_TEX_Pos) | (MPU_RASR_S_Msk),
-  [MpuCachePolicy_WriteThrough] = (MPU_RASR_S_Msk | MPU_RASR_C_Msk),
-  [MpuCachePolicy_WriteBackWriteAllocate] =
-      (0x1 << MPU_RASR_TEX_Pos) | (MPU_RASR_S_Msk | MPU_RASR_C_Msk | MPU_RASR_B_Msk),
-  [MpuCachePolicy_WriteBackNoWriteAllocate] =
-      (MPU_RASR_S_Msk | MPU_RASR_C_Msk | MPU_RASR_B_Msk),
-#endif
-};
-
-static uint8_t get_permission_value(const MpuRegion* region) {
-  for (unsigned int i = 0; i < ARRAY_LENGTH(s_permission_mappings); ++i) {
-    if (s_permission_mappings[i].priv_read == region->priv_read &&
-        s_permission_mappings[i].priv_write == region->priv_write &&
-        s_permission_mappings[i].user_read == region->user_read &&
-        s_permission_mappings[i].user_write == region->user_write) {
-      return s_permission_mappings[i].value;
-    }
-  }
-  WTF;
-  return 0;
-}
-
-#ifndef MPU_ARMV8
-static uint32_t get_size_field(const MpuRegion* region) {
-  unsigned int size = 32;
-  int result = 4;
-  while (size != region->size) {
-    PBL_ASSERT(size < region->size || size == 0x400000, "Invalid region size: %"PRIu32,
-               region->size);
-
-    size *= 2;
-    ++result;
-  }
-
-  return result;
-}
-#endif
-
-void mpu_enable(void) {
-#ifdef MPU_ARMV8
-  ARM_MPU_SetMemAttr(MpuCachePolicy_NotCacheable,
-                     s_cache_settings[MpuCachePolicy_NotCacheable]);
-  ARM_MPU_SetMemAttr(MpuCachePolicy_WriteThrough,
-                     s_cache_settings[MpuCachePolicy_WriteThrough]);
-  ARM_MPU_SetMemAttr(MpuCachePolicy_WriteBackWriteAllocate,
-                     s_cache_settings[MpuCachePolicy_WriteBackWriteAllocate]);
-  ARM_MPU_SetMemAttr(MpuCachePolicy_WriteBackNoWriteAllocate,
-                     s_cache_settings[MpuCachePolicy_WriteBackNoWriteAllocate]);
-#endif
-
-  ARM_MPU_Enable(MPU_CTRL_PRIVDEFENA_Msk);
-}
-
 void mpu_disable(void) {
   ARM_MPU_Disable();
 }
 
-// Get the required region base address and region attribute register settings for the given region.
-// These are the values which should written to the RBAR and RASR registers to configure that
-// region.
-void mpu_get_register_settings(const MpuRegion* region, uint32_t *base_address_reg,
-                               uint32_t *attributes_reg) {
-  PBL_ASSERTN(region);
-  PBL_ASSERTN((region->base_address & 0x1f) == 0);
-  PBL_ASSERTN((region->region_num & ~0xf) == 0);
-  PBL_ASSERTN((region->cache_policy < ARRAY_LENGTH(s_cache_settings)));
-
-#ifdef MPU_ARMV8
-  PBL_ASSERTN((region->size & 0x1f) == 0);
-
-  *base_address_reg = ((region->base_address & MPU_RBAR_BASE_Msk) |
-                       ((ARM_MPU_SH_INNER << MPU_RBAR_SH_Pos) & MPU_RBAR_SH_Msk) |
-                       ((get_permission_value(region) << MPU_RBAR_AP_Pos) & MPU_RBAR_AP_Msk));
-  *attributes_reg = (((region->base_address + region->size - 1U) & MPU_RLAR_LIMIT_Msk) |
-                     ((region->cache_policy << MPU_RLAR_AttrIndx_Pos) & MPU_RLAR_AttrIndx_Msk) |
-                     ((region->enabled << MPU_RLAR_EN_Pos) & MPU_RLAR_EN_Msk));
-#else
-  // MPU Region Base Address Register
-  // | Addr (27 bits) | Region Valid Bit | Region Num (4 bits) |
-  // The address is unshifted, we take the top bits of the address and assume everything below
-  // is zero, since the address must be power of 2 size aligned.
-  *base_address_reg = region->base_address |
-              0x1 << 4 |
-              region->region_num;
-
-  // MPU Region Attribute and Size Register
-  // A lot of stuff here! Split into bytes...
-  // | Reserved (3 bits) | XN Bit | Reserved Bit | Permission Field (3 bits) |
-  // | Reserved (2 bits) | TEX (3 bits) | S | C | B |
-  // | Subregion Disable Byte |
-  // | Reserved (2 bits) | Size Field (5 bits) | Enable Bit |
-  *attributes_reg = (get_permission_value(region) << 24) |
-              s_cache_settings[region->cache_policy] |
-              region->disabled_subregions << 8 |      // Disabled subregions
-              (get_size_field(region) << 1) |
-              region->enabled; // Enabled
-#endif
-}
-
-
-void mpu_set_region(const MpuRegion* region) {
-  uint32_t base_reg, attr_reg;
-
-  mpu_get_register_settings(region, &base_reg, &attr_reg);
-
-#ifdef MPU_ARMV8
-  ARM_MPU_SetRegion(region->region_num, base_reg, attr_reg);
-#else
-  MPU->RBAR = base_reg;
-  MPU->RASR = attr_reg;
-#endif
-}
-
-
-MpuRegion mpu_get_region(int region_num) {
-#ifdef MPU_ARMV8
-  MpuRegion region;
-  uint32_t rbar, rlar;
-  uint8_t access_permissions;
-
-  region.region_num = region_num;
-
-  MPU->RNR = region_num;
-  rbar = MPU->RBAR;
-  rlar = MPU->RLAR;
-
-  region.base_address = rbar & MPU_RBAR_BASE_Msk;
-
-  access_permissions = (rbar & MPU_RBAR_AP_Msk) >> MPU_RBAR_AP_Pos;
-  for (size_t i = 0; i < ARRAY_LENGTH(s_permission_mappings); ++i) {
-    if (s_permission_mappings[i].value == access_permissions) {
-      region.priv_read = s_permission_mappings[i].priv_read;
-      region.priv_write = s_permission_mappings[i].priv_write;
-      region.user_read = s_permission_mappings[i].user_read;
-      region.user_write = s_permission_mappings[i].user_write;
-      break;
-    }
-  }
-
-  region.size = (rlar & MPU_RLAR_LIMIT_Msk) - region.base_address + 0x20;
-  region.enabled = (rlar & MPU_RLAR_EN_Msk) != 0;
-  region.cache_policy = (rlar & MPU_RLAR_AttrIndx_Msk) >> MPU_RLAR_AttrIndx_Pos;
-
-  return region;
-#else
-  MpuRegion region = { .region_num = region_num };
-
-  MPU->RNR = region_num;
-
-  const uint32_t attributes = MPU->RASR;
-
-  region.enabled = attributes & 0x1;
-
-  if (region.enabled) {
-    const uint8_t size_field = (attributes >> 1) & 0x1f;
-    region.size = 32 << (size_field - 4);
-
-    region.disabled_subregions = (attributes & 0x0000ff00) >> 8;
-
-    const uint32_t raw_base_address = MPU->RBAR;
-    region.base_address = raw_base_address & ~(region.size - 1);
-
-    const uint8_t access_permissions = (attributes >> 24) & 0x7;
-
-    for (unsigned int i = 0; i < ARRAY_LENGTH(s_permission_mappings); ++i) {
-      if (s_permission_mappings[i].value == access_permissions) {
-        region.priv_read = s_permission_mappings[i].priv_read;
-        region.priv_write = s_permission_mappings[i].priv_write;
-        region.user_read = s_permission_mappings[i].user_read;
-        region.user_write = s_permission_mappings[i].user_write;
-        break;
-      }
-    }
-  }
-
-  return region;
-#endif
-}
-
-
 // Fill in the task parameters for a new task with the configurable memory regions we want.
 void mpu_set_task_configurable_regions(MemoryRegion_t *memory_regions,
                                        const MpuRegion **region_ptrs) {
@@ -265,18 +42,20 @@ void mpu_set_task_configurable_regions(MemoryRegion_t *memory_regions,
     // If not region defined, use unused
     if (mpu_region == NULL) {
       mpu_region = &unused_region;
+      base_reg = 0;
       attr_reg = 0; // Has a 0 in the enable bit, so this region won't be enabled.
     } else {
       // Make sure that the region numbers passed in jive with the configurable region numbers.
       PBL_ASSERTN(mpu_region->region_num == region_num);
-      // Our FreeRTOS port makes the assumption that the ulParameters field contains exactly what
-      // should be placed into the MPU_RASR register. It will figure out the MPU_RBAR from the
-      // pvBaseAddress field.
       mpu_get_register_settings(mpu_region, &base_reg, &attr_reg);
     }
 
+    // pvBaseAddress / ulParameters carry the arch-specific RBAR / RASR-or-RLAR
+    // payload. The CM4F port OR's region_num+VALID into ulRegionBaseAddress
+    // (no-op since mpu_get_register_settings already encodes them on ARMv7-M);
+    // the CM33 port writes pvBaseAddress straight to MPU_RBAR.
     memory_regions[region_idx] = (MemoryRegion_t) {
-      .pvBaseAddress = (void *)mpu_region->base_address,
+      .pvBaseAddress = (void *)base_reg,
       .ulLengthInBytes = mpu_region->size,
       .ulParameters = attr_reg,
     };