MPU refactor and support for ARMv8 MPU - #1287
Closed
gmarull wants to merge 8 commits into
Closed
Conversation
The MPU implementation choice was previously derived from MICRO_FAMILY_SF32LB52, leaking SoC knowledge into a generic driver and preventing other ARMv8-M targets (QEMU CM33: qemu_emery, qemu_gabbro) from picking the right MPU register format. Define MPU_TYPE_ARMV7M / MPU_TYPE_ARMV8M centrally in platform/wscript based on MICRO_FAMILY, and switch the driver and memory_layout conditionals over. No runtime behavior change: SF32LB52 keeps the ARMv8 paths; NRF52/QEMU CM4 keep the ARMv7 paths; QEMU CM33 now compiles the matching ARMv8 paths (still gated off at runtime by the early-return in memory_layout_setup_mpu, which a later step will remove). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
mpu.c was a single source file with #ifdef MPU_TYPE_ARMV8M sprinkled throughout, making the two backends hard to read and to extend. Move the ARMv7-M and ARMv8-M paths into mpu/mpu_armv7m.c and mpu/mpu_armv8m.c respectively, leaving mpu.c with only the arch-independent helpers (mpu_disable, mpu_set_task_configurable_regions, mpu_memory_is_cachable, mpu_init_region_from_region). wscript_build picks the right backend based on MICRO_FAMILY. Output binaries are byte-identical (qemu_emery) or near-identical (qemu_flint, +16 bytes from differing inlining across translation units). No runtime behavior change. Verified boot to menu on both qemu_flint and qemu_emery. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
MpuRegion previously exposed a disabled_subregions field that callers had to compute via tools/mpu_calc.py at build time. That field is an ARMv7-M MPU implementation detail (rounding an arbitrary range up to a power-of-two block plus an 8-bit subregion mask) and has no analogue on ARMv8-M, which programs the limit register directly. Drop the field from the public struct. Callers now pass the real (base_address, size) pair. The ARMv7-M backend's mpu_get_register_settings computes the block + subregion mask at runtime from these values, and mpu_get_region decodes them back from the registers. ARMv8-M is unchanged. The generated kernel/mpu_regions.auto.h header and the mpu_calc.py helper that produced it both go away; memory_layout.c reads the actual __APP_RAM__/__APP_RAM_size__/__WORKER_RAM__/__WORKER_RAM_size__ linker symbols directly, removing a source of drift between the linker script and the auto-generated MPU sizes. Verified on qemu_flint via gdb register inspection: - App task running (control=0x2) sees AP=3 (priv R/W + user R/W) on App RAM and AP=1 (priv-only) on Worker RAM, with subregion masks byte-identical to what mpu_calc.py used to emit. - Stack guard region (32B at __isr_stack_start__) programmed AP=0 (no access). - Menu navigation through multiple submenus exercises the App-task context switch path without faults. qemu_emery still uses the ARMv8 backend and continues to boot to the launcher (MPU still disabled at runtime via the early-return that a later step removes). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
…t macro MpuCachePolicy and MemoryRegionAssignments previously declared four named placeholder values (MpuCachePolicy_Reserved0..3, MemoryRegion_Reserved0..3) on SF32LB52 so the firmware's own values would land above whatever the SoC vendor SDK leaves in regions 0-3 and cache-attribute indices 0-2. Anonymous "Reserved" names in a public enum are an awkward leak of an SoC quirk into headers used by many callers. Replace them with a single MPU_SOC_RESERVED_SLOTS constant (4 on SF32LB52, 0 elsewhere) and seed the enums from that value (MpuCachePolicy_NotCacheable = MPU_SOC_RESERVED_SLOTS, etc.). The numeric values are byte-identical to before; only the placeholder names are gone. Binary size unchanged on qemu_flint and qemu_emery; both still boot to the launcher. The reservation on SF32LB52 is *not* a bootloader artifact -- pblboot clears the MPU before jumping to firmware (see lib/pb/fwjump/ fwjump_arm_cm.c in coredevices/pblboot). It's the vendor SDK's SystemInit() that calls prv_mpu_config() in third_party/hal_sifli/sf32lb52/system_bf0_ap.c, which fills regions 0-3 with code/RAM/device and attribute indices 0-2 with the matching ARM_MPU_ATTR_* values. The rest of the vendor SDK runtime assumes those stay programmed, so the firmware works around them by starting its own enums at MPU_SOC_RESERVED_SLOTS. Drop the offset once the firmware owns MPU setup end-to-end on SF32LB52. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Three things had to come together for the MPU to actually run on the
QEMU Cortex-M33 boards (qemu_emery, qemu_gabbro):
1. memory_layout_setup_mpu() early-returned on
MICRO_FAMILY_QEMU_PEBBLE_ARMCM33, so the firmware never programmed
its static regions (Flash, RO BSS, ISR stack guard) and never
called mpu_enable(). Drop the guard -- the ARMv8 backend already
handles those regions correctly.
2. third_party/freertos/wscript set FREERTOS_FIRST/LAST_MPU_REGION
to 8/7 for CM33, which yields portNUM_CONFIGURABLE_REGIONS == 0
and disables FreeRTOS' per-task region setup entirely. Use 4/7
(same range the CM4F port uses): firmware programs slots 0-3
statically and FreeRTOS owns 4-7 for the App / Worker /
stack-guard regions on context switch.
3. mpu_set_task_configurable_regions() put the raw region base into
pvBaseAddress. The CM4F port re-encodes it on context switch
(OR's region_num and VALID into MPU_RBAR); the CM33 port writes
pvBaseAddress straight to MPU_RBAR with no further encoding, so
AP/SH bits never reached the hardware. Pass the fully-encoded
base_reg from mpu_get_register_settings() -- idempotent on CM4F
(the OR-in bits are already there) and correct on CM33.
SF32LB52 stays on FIRST=8/LAST=7 (0 configurable regions). The vendor
SDK's SystemInit() in third_party/hal_sifli/sf32lb52/system_bf0_ap.c
already owns regions 0-3 (see the previous commit and
MPU_SOC_RESERVED_SLOTS=4) and the firmware does not yet program its
own static or per-task regions on that board. A follow-up will plumb
SF32LB52 through end-to-end.
Verified on qemu_emery with gdb: MPU_CTRL.ENABLE=1, slots 4-7 hold
App RAM (AP=1: priv + user R/W), Worker RAM (AP=0: priv R/W only)
and the App stack guard (AP=2: priv RO -- the closest ARMv8-M's
two-bit AP field can express to "no access"). qemu_flint binary
byte-identical.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Runs as the unprivileged App task and deliberately writes 0xDEADBEEF to
Worker RAM, which the App task's MPU configuration maps as privileged
R/W only (no user access). If the MPU is enforcing protection the
write triggers a MemManage fault, the kernel kills the App task, and
the launcher reclaims the screen. If protection is NOT being enforced
the write completes and the app updates its window to "MPU FAILED" so
the regression is obvious.
The app is scoped to qemu_flint/qemu_emery/qemu_gabbro in the system
app registry (a self-test for the QEMU MPU paths) and the only its
source file is pulled into the QEMU build, so the rest of apps/demo
(currently bit-rotted on QEMU; --test_apps does not build) does not
need to be fixed to use this test. Drop the wscript hook once those
demos build cleanly and the app can move under --test_apps.
Validated via gdb on both QEMU boards:
qemu_flint (ARMv7-M MPU):
mem_manage_handler_c fired
CFSR = 0x82 (MMARVALID + DACCVIOL)
MMFAR = 0x20025000 <- __WORKER_RAM__
prv_return_to_landing_zone ran -> App task killed
screenshot: launcher reclaimed
qemu_emery (ARMv8-M MPU):
mem_manage_handler_c fired
CFSR = 0x82 (MMARVALID + DACCVIOL)
MMFAR = 0x20045000 <- __WORKER_RAM__
prv_return_to_landing_zone ran -> App task killed
screenshot: launcher reclaimed
Launch from the PULSE prompt: `app launch -191`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
The 32K reservation on obelix and getafix was a workaround for tools/mpu_calc.py's ARMv7-M subregion math: rounding the App/Worker RAM ranges up to a power-of-two block could push the block past the end of physical SRAM, so the build reserved 32K at the top to keep that algorithm solvable. mpu_calc.py is gone now (the ARMv7-M backend does the math at runtime, the ARMv8-M backend doesn't need it at all), so the workaround can go too. Bring obelix and getafix up to the real 511K usable -- the only hardware reservation is the documented 1K LCPU IPC area at the top of SRAM. Build verified on obelix_dvt. qemu_emery and qemu_gabbro stay at 480K because the corresponding QEMU machines (pebble-emery, pebble-gabbro) only expose 480K of SRAM (0x20000000-0x20077fff); the comments are updated to point that out explicitly so the size mismatch with the matching hardware boards isn't surprising. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
SF32LB52 (obelix, getafix) previously kept MPU support gated behind
#ifndef MICRO_FAMILY_SF32LB52 in memory_layout_setup_mpu() and
pebble_task_create(), and FREERTOS_FIRST/LAST_MPU_REGION was set to
8/7 (zero configurable regions) -- so neither static nor per-task
regions ever reached the hardware on real SF32LB52 watches.
The vendor SDK's SystemInit() in third_party/hal_sifli/sf32lb52/
system_bf0_ap.c still pre-programs slots 0-3 (CODE, DEVICE, RAM,
RAM2) and the cache-attribute indices it depends on (these are
already accounted for by MPU_SOC_RESERVED_SLOTS=4 introduced in an
earlier commit). The firmware now owns slots 4-7 (Flash, RO BSS, ISR
stack guard) and FreeRTOS owns slots 8-11 (App RAM, Worker RAM,
TaskStackGuard, Task4) on context switch.
Changes:
- memory_layout_setup_mpu(): drop the SF32LB52 guard; program the
static regions on every board.
- pebble_task_create(): drop the SF32LB52 guards; configure the
per-task stack-guard region and pass the populated region_ptrs
array to FreeRTOS for every task.
- third_party/freertos/wscript: SF32LB52 now uses
FREERTOS_FIRST_MPU_REGION=8 / FREERTOS_LAST_MPU_REGION=11 (4
configurable regions starting at slot 8, matching the firmware's
MemoryRegion_AppRAM..Task4 numbering).
Verified obelix_dvt and getafix_dvt build clean. qemu_emery
regression-tests still pass (boot + test_mpu_violation triggers
MemManage with CFSR=0x82, MMFAR=__WORKER_RAM__). Runtime validation
on SF32LB52 silicon to follow.
Note: ARMv8-M behavior for overlapping MPU regions is implementation
defined. The firmware's slots 4-11 overlap with the vendor SDK's
slot 2 (RAM @ 0x20000000-0x2027ffff). If Cortex-M33 STAR-MC1 picks
the lowest-numbered match, firmware regions will be ignored and
isolation won't take effect; a follow-up will clear or reprogram
slot 2 if that's the case.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
Member
Author
|
need coredevices/qemu#2 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.