Skip to content

Conversation

fsammoura1980
Copy link
Contributor

The Physical Memory Protection (PMP) setup for the ROM region currently relies on the fixed linker symbols __rom_region_start and __rom_region_size.

In environments where the firmware is loaded by a bootloader (example a two-stage boot, or jumping from a RO-image to a RW-image), the actual physical address of the executable code might change. This is especially critical when the PMP must protect the entire persistent ROM area, regardless of which stage of the firmware is executing.

This commit addresses this by:

  1. Introducing CONFIG_ROM_REGION_START and CONFIG_ROM_REGION_SIZE Kconfig options, guarded by RISCV_PMP. These default to the standard linker symbols but allow manual override via hexadecimal strings.

  2. Adding a compile-time ADDRESS_RESOLVER macro in pmp.c to parse the Kconfig string, correctly handling the conversion of either the default linker symbol or a user-provided hexadecimal address into a numerical value.

  3. Updating z_riscv_pmp_init() to use the resolved Kconfig values, ensuring that the PMP always protects the correct, full ROM area as defined by the user or the linker.

The Physical Memory Protection (PMP) setup for the ROM region currently
relies on the fixed linker symbols `__rom_region_start` and
`__rom_region_size`.

In environments where the firmware is loaded by a bootloader (example
a two-stage boot, or jumping from a RO-image to a RW-image), the actual
physical address of the executable code might change. This is especially
critical when the PMP must protect the entire persistent ROM area,
regardless of which stage of the firmware is executing.

This commit addresses this by:

1. Introducing `CONFIG_ROM_REGION_START` and `CONFIG_ROM_REGION_SIZE`
   Kconfig options, guarded by `RISCV_PMP`. These default to the standard
   linker symbols but allow manual override via hexadecimal strings.

2. Adding a compile-time `ADDRESS_RESOLVER` macro in `pmp.c` to parse
   the Kconfig string, correctly handling the conversion of either the
   default linker symbol or a user-provided hexadecimal address into
   a numerical value.

3. Updating `z_riscv_pmp_init()` to use the resolved Kconfig values,
   ensuring that the PMP always protects the correct, full ROM area as
   defined by the user or the linker.

Signed-off-by: Firas Sammoura <[email protected]>
Copy link

@fkokosinski
Copy link
Member

Hi, thanks for the PR!

I’m not convinced this is the right approach, but even if it was - it’s something that I believe should be uniform across different architectures in Zephyr, i.e. it should not be introduced at RISC-V level.

Have you considered other options? We could e.g. make z_riscv_pmp_init() respect the previous configuration.

Other than that, we could maybe extend the feature you’re adding in #96241 to support setting more than one user-defined PMP entries (e.g. with devicetree) to cover this use case?

Copy link

@npitre npitre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You say:

"In environments where the firmware is loaded by a bootloader (example a
two-stage boot, or jumping from a RO-image to a RW-image), the actual physical
address of the executable code might change."

Why would that change?

Please note that __rom_region_start and __rom_region_size always
refer to the final execution address not necessarily the actual ROM
chip on the system. If you are using XIP then __rom_region_start will
be somewhere inside the ROM chip. But if the bootloader copies the Zephyr
binary image into RAM then __rom_region_start will be in RAM, denoting
the region of RAM that should never be written to.

@fsammoura1980
Copy link
Contributor Author

You say:

"In environments where the firmware is loaded by a bootloader (example a two-stage boot, or jumping from a RO-image to a RW-image), the actual physical address of the executable code might change."

Why would that change?

Please note that __rom_region_start and __rom_region_size always refer to the final execution address not necessarily the actual ROM chip on the system. If you are using XIP then __rom_region_start will be somewhere inside the ROM chip. But if the bootloader copies the Zephyr binary image into RAM then __rom_region_start will be in RAM, denoting the region of RAM that should never be written to.

In multi-stage firmware environments (RO → RW jump), the PMP setup fails to consistently protect the entire ROM region. The issue is that the linker symbols defining the ROM boundaries (__rom_region_start, __rom_region_size) vary between the RO and RW images. Since the PMP region protecting the ROM must be statically defined and locked early in the boot process, relying on these variable stage-specific symbols prevents us from enforcing continuous protection across the full physical ROM space.

@fsammoura1980
Copy link
Contributor Author

Hi, thanks for the PR!

I’m not convinced this is the right approach, but even if it was - it’s something that I believe should be uniform across different architectures in Zephyr, i.e. it should not be introduced at RISC-V level.

Have you considered other options? We could e.g. make z_riscv_pmp_init() respect the previous configuration.

Other than that, we could maybe extend the feature you’re adding in #96241 to support setting more than one user-defined PMP entries (e.g. with devicetree) to cover this use case?

What other options you propose? I believe this is a RISCV specific issue. RISCV is the architecture where you cannot unlock a locked entry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: RISCV RISCV Architecture (32-bit & 64-bit)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants