RISCV: PMP: Use correct pmpcfg index for multi-register configurations #96192
+12
−10
Conversation
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
zephyrbot
requested review from
edersondisouza,
fkokosinski,
katsuster,
kgugala,
masz-nordic,
mgielda,
npitre,
tgorochowik,
VynDragon and
ycsin
fsammoura1980
force-pushed
the
higher_index
branch
5 times, most recently
from
8fd7348 to
c905cb2
Compare
fsammoura1980
force-pushed
the
higher_index
branch
2 times, most recently
from
7b222d1 to
e160b46
Compare
The arrays `m_mode_pmpaddr_regs` and `m_mode_pmpcfg_regs` within the `_thread_arch` struct, used for the M-mode stack guard (when CONFIG_PMP_STACK_GUARD is enabled), were previously sized using the hardcoded PMP_M_MODE_SLOTS macro (defined as 8). This commit changes the array sizing to use the Kconfig option CONFIG_PMP_SLOTS. This option reflects the total number of Physical Memory Protection (PMP) slots available and configured for the target hardware. This change aligns the M-mode stack guard implementation with the CONFIG_USERSPACE feature, which already uses CONFIG_PMP_SLOTS for sizing its PMP-related arrays. Furthermore, using CONFIG_PMP_SLOTS ensures the size of these arrays is known at compile time, allowing code in other places, such as drivers/riscv/pmp.c, to safely and accurately copy or manage these PMP entries when needed. Signed-off-by: Firas Sammoura <[email protected]>
The PMP initialization and thread seeding logic in arch/riscv/core/pmp.c
did not correctly handle scenarios where the global PMP entries span
across multiple `pmpcfg` hardware registers.
The `global_pmp_cfg` array, intended to store the hardware register
values, was hardcoded to size 1. This is only sufficient if all global
PMP entries fall within the range covered by the first `pmpcfg` register
(e.g., pmpcfg0). When more global entries are used, their configurations
reside in subsequent `pmpcfg` registers (pmpcfg1, pmpcfg2, etc.).
The code was only saving/restoring and checking `global_pmp_cfg[0]`,
leading to loss of configuration for entries mapped to higher `pmpcfg`
registers.
This patch fixes this by:
1. Resizing the `global_pmp_cfg` array to
`CONFIG_PMP_SLOTS / PMPCFG_STRIDE` to correctly accommodate values
from all potentially used `pmpcfg` CSRs.
2. In `z_riscv_pmp_init`, using `memcpy` to save the entire contents of
the local `pmp_cfg` array (derived from initial setup) into the
`global_pmp_cfg` array.
3. In `z_riscv_pmp_thread_init`, using `memcpy` to restore the entire
saved global configuration from `global_pmp_cfg` into a thread's
`pmp_cfg` array.
4. Updating the SMP consistency assertion in `z_riscv_pmp_init` to use
`memcmp` to compare the full contents of the `pmp_cfg` arrays between
CPUs.
These changes ensure that the configurations from all relevant `pmpcfg`
registers are preserved and correctly propagated to thread contexts.
Signed-off-by: Firas Sammoura <[email protected]>
fsammoura1980
force-pushed
the
higher_index
branch
from
e160b46 to
c171cfd
Compare
In `z_riscv_pmp_stackguard_enable`, the call to `write_pmp_entries` has been updated to set the `clear_trailing_entries` argument to `true`. This change ensures that any PMP entries beyond the ones being actively configured are disabled. This is critical for systems, such as firmwares transitioning between different execution stages (e.g., from a Read-Only (RO) phase to a Read-Write (RW) phase), which may have distinct PMP setups. Without clearing trailing entries, stale configurations from a previous stage could persist. These leftover entries could lead to incorrect memory access permissions, resulting in functional bugs or potential security vulnerabilities. This commit guarantees a clean PMP state after an update. Signed-off-by: Firas Sammoura <[email protected]>
fsammoura1980
force-pushed
the
higher_index
branch
from
c171cfd to
da544f9
Compare
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
arch: riscv: Enhance PMP configuration and consistency
This commit refactors the RISC-V Physical Memory Protection (PMP) implementation to improve correctness, consistency, and safety, addressing several issues:
Correct Global PMP Configuration Array Sizing:
The
global_pmp_cfgarray inpmp.cwas hardcoded to size 1. This incorrectly assumed all global PMP settings fit within the firstpmpcfgControl and Status Register (CSR). On RISC-V, configurations can span multiplepmpcfgCSRs if more thanPMPCFG_STRIDE(typically 4 or 8) entries are used.global_pmp_cfg[CONFIG_PMP_SLOTS / PMPCFG_STRIDE], ensuring it can store all necessarypmpcfgregister values.z_riscv_pmp_initandz_riscv_pmp_thread_initnow usememcpyto save/restore the entire configuration.z_riscv_pmp_initnow usesmemcmpto compare the full configuration.Unified Thread PMP Array Sizing:
The M-mode stack guard arrays in
_thread_arch(i.e.,m_mode_pmpaddr_regs,m_mode_pmpcfg_regs), used whenCONFIG_PMP_STACK_GUARDis enabled, were sized using the hardcodedPMP_M_MODE_SLOTS. These arrays are now sized using the Kconfig optionCONFIG_PMP_SLOTS. This aligns their sizing with theCONFIG_USERSPACEPMP arrays and reflects the actual number of available PMP slots.Ensure Clean PMP State on Stack Guard Enable:
The call to
write_pmp_entrieswithinz_riscv_pmp_stackguard_enablewas updated to setclear_trailing_entries = true. This ensures that any PMP entries beyond those actively configured for the thread's stack guard are disabled. This prevents stale, potentially insecure PMP settings from persisting from previous execution contexts or boot stages.These changes collectively enhance the robustness of the PMP setup, especially on systems with a larger number of PMP slots, and ensure consistent management of PMP resources.