Skip to content

plat-rockchip: rk3588: option to leave the DDR firewall to BL31 - #8032

Open
nikicat wants to merge 1 commit into
OP-TEE:masterfrom
nikicat:rk3588-firewall-by-bl31
Open

nikicat wants to merge 1 commit into
OP-TEE:masterfrom
nikicat:rk3588-firewall-by-bl31

Conversation

@nikicat

@nikicat nikicat commented Sep 19, 2026 •

Copy link
Copy Markdown

platform_secure_ddr_region() programs the DDR/DSU firewall for TZDRAM from S-EL1, but BL31 already owns that block on rk3588. Its secure_region_init() clears regions 1-15, takes region 0 for its own memory, and pmu.c carries every FW_DSU RGN/MST/CON register through suspend in PMUSRAM — all before BL32 starts. With a BL31 that also programs region 1 for BL32, the write from S-EL1 is redundant.

On a Radxa ROCK 5B booting edk2-rk3588 (TF-A from worproject with SPD=opteed) it is worse than redundant: the register writes never return and the primary core hangs in platform_init(). Located with a boot-stage marker per initcall, read from the normal world — no UART was available on this board.

This adds CFG_RK3588_FIREWALL_BY_BL31 (default n, behaviour unchanged) to leave the firewall to BL31. The skip is logged with MSG, the way the weak platform_secure_ddr_region() in platform.c reports an unprotected region, so delegating TZDRAM protection is not silent — the existing DMSG in this function is compiled out at the default log level.

The matching TF-A side is at nikicat/arm-trusted-firmware rk3588-optee-upstream, submitted to edk2-rk3588 as patch files in edk2-porting/edk2-rk3588#290. It firewalls PLAT_RK_SEC_DRAM_BASE/SIZE, the secure DRAM window the platform reserves, rather than any particular BL32's base and size, so an OP-TEE that moves within the window or grows is not a TF-A change.

Tested on ROCK 5B with edk2-rk3588 v1.1 and that TF-A programming region 1 for TZDRAM: OP-TEE 4.10 / master boots, the Linux driver probes with dynamic shared memory, the PKCS#11 early TA imports and signs keys, storage persists across reboots.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VPqq7YaURA87cbGTbVXbrZ

https://claude.ai/code/session_01Rqe5yoJTwPu4ZZKb96vijY

@github-actions

Copy link
Copy Markdown

FYI @OOHehir

@mmind

mmind commented Sep 22, 2026

Copy link
Copy Markdown

In the matching TF-A changes you then hard-code the OP-TEE location and size forever. Which creates a hard dependency between TF-A and OP-TEE und will cause a lot of headache down the road, when things need to move and or OP-TEE gets bigger.

If the whole issue hinges on your EDK thing, why not fix that?

@nikicat

nikicat commented Sep 22, 2026

Copy link
Copy Markdown
Author

Fair point on the hardcoding — I've reworked the TF-A side so it no longer describes OP-TEE at all.

Instead of naming OP-TEE's base and size, the platform declares the secure DRAM carve-out it reserves, the same way TZRAM_BASE/TZRAM_SIZE right above it already declare BL31's:

#define PLAT_RK_SEC_DRAM_BASE	0x08400000
#define PLAT_RK_SEC_DRAM_SIZE	0x00f00000

BL31 firewalls that window; any BL32 boots as long as it fits inside it, so OP-TEE moving within it or growing is no longer a TF-A change. It's also the window the non-secure loader has to keep out of the OS memory map anyway, so it isn't a new number — it's the existing reservation, stated in one more place. And when a loader does pass a BL32 entry point, BL31 now checks it lands inside the window and panics if it doesn't, so the two drifting apart is loud instead of silent. (The handover carries no BL32 size, so only the base can be checked; a BL32 that starts inside the window and outgrows it is left for BL32 to notice.)

On "why not fix the EDK thing": the hang isn't reachable from edk2. Boot order is BL31 → BL32 → BL33, and this is OP-TEE's platform_init running at BL32 init — edk2 hasn't executed an instruction yet. What has run is BL31, and the rk3588 firewall code there is Rockchip's, not worproject's or edk2's: secure_region_init() clears DDR/DSU regions 1-15 and takes region 0 for TZRAM before BL32 starts, sgrf_init() reclassifies every firewall slave, and pmu.c carries the whole FW_DSU RGN/MST/CON set through suspend in PMUSRAM. So the block is BL31-owned, with no handshake with BL32 over it — that's what this knob is for, and why it defaults to n.

Happy to take the OP-TEE side somewhere else if you'd rather — e.g. making region 1 ownership explicit in the port instead of a build flag.

nikicat added a commit to nikicat/edk2-rk3588 that referenced this pull request Sep 22, 2026
Review feedback on the OP-TEE side (OP-TEE/optee_os#8032): naming
OP-TEE's base and size in TF-A ties the two together, so OP-TEE moving
or growing would need a TF-A change. Regenerate 0010/0011 from the
reworked series: PLAT_RK_BL32_BASE/SIZE become PLAT_RK_SEC_DRAM_BASE/
SIZE, the secure DRAM window this platform reserves, and BL31 checks a
loader-supplied BL32 entry point against it instead of assuming the two
agree. The patches also carry Signed-off-by now.

No functional change to the build: the same window is firewalled.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rqe5yoJTwPu4ZZKb96vijY
platform_secure_ddr_region() programs the DDR/DSU firewall for TZDRAM
from S-EL1, but BL31 already owns that block on this SoC: it clears
regions 1-15, takes region 0 for its own memory, and carries the region
registers through suspend. With a BL31 that also programs region 1 for
BL32, the write from S-EL1 is redundant; on a Radxa ROCK 5B booting
edk2-rk3588 (TF-A from worproject with SPD=opteed) it never returns and
the primary core hangs in platform_init().

Add an option to leave the firewall to BL31 in that setup, and say so in
the boot log the way the weak platform_secure_ddr_region() does, so a
delegated TZDRAM protection is not silent. Default unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VPqq7YaURA87cbGTbVXbrZ
Claude-Session: https://claude.ai/code/session_01Rqe5yoJTwPu4ZZKb96vijY
@nikicat
nikicat force-pushed the rk3588-firewall-by-bl31 branch from a6c64dc to 0cc16a4 Compare September 22, 2026 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants