Add SMPU and SPU Support for SC59x - #3339
Conversation
|
For manual testing, here are the commands that I have used/been using. 1 - On the U-Boot side, we should do: mw.l 0x31007800 0x00000504
mw.l 0x31083800 0x00000504
mw.l 0x31084800 0x00000504
mw.l 0x31085800 0x00000504
mw.l 0x31086800 0x00000504
mw.l 0x310A0800 0x00000504
mw.l 0x310A1800 0x00000504This is just to set SECURECTL to enable non secure read write. I was also curious about whether they are persistent. 2 - Then we can test a memory that is already read only. # Configure SMPU2
devmem 0x31083000 32 0x00000009
# Clear any previous status
devmem 0x31083004 32 0xFFFFFFFF
# RADDR[0]: Boot ROM base address
devmem 0x31083024 32 0x20200000
# RIDA[0]: Fake ID that cannot be matching any real hardware
devmem 0x31083028 32 0x00001FFF
# RIDMSKA[0]: Match all 13 bits (exact match, for the filter mask)
devmem 0x3108302C 32 0x00001FFF
# RCTL[0]: EN=1, SIZE=4 (64KB), RPROTEN=1, WPROTEN=1
devmem 0x31083020 32 0x00000909
# Trigger violation by writing to Boot ROM
devmem 0x20200000 32 0xADADADAD
# Check results
devmem 0x31083004 32
# Expected: 0x00000004 (BERR bit set)
devmem 0x31083010 32
# Expected: 0x20200000
devmem 0x31083014 32
# Expected: transaction ID3 - Or, we can also test with SMPU9 on ddr. # First, test that memory is accessible without SMPU
devmem 0x9E100000 32 0x12345678
devmem 0x9E100000 32
# Should read back: 0x12345678
# Configure SMPU9 instance
devmem 0x310A0000 32 0x00000009
# Clear status
devmem 0x310A0004 32 0xFFFFFFFF
# RADDR[0]: DDR test address
devmem 0x310A0024 32 0x9E100000
# RIDA[0]: Block with fake ID
devmem 0x310A0028 32 0x00001234
# RIDMSKA[0]: Exact match
devmem 0x310A002C 32 0x00001FFF
# RCTL[0]: EN=1, SIZE=6 (256KB), RPROTEN=1, WPROTEN=1
devmem 0x310A0020 32 0x00000D09
# Trigger violation
devmem 0x9E100000 32 0xBADC0DE0
# Check results
devmem 0x310A0004 32
# Expected: 0x00000004 (BERR) or 0x00000001 (IRQ)
devmem 0x310A0008 32
# Expected: 0x9E100000 (if IRQ) or check BADDR below
devmem 0x310A000C 32
# Expected: non-zero with transaction ID
devmem 0x310A0010 32
# Expected: 0x9E100000 (if BERR)
devmem 0x310A0014 32
# Expected: non-zero with transaction IDThird one did not work as expected for me. Thats the part I am investigating. |
|
Bit more on secure control registers: The driver includes an adi,secure-access device tree property that controls whether the driver attempts to write SMPU SECURECTL registers. This property is currently reserved for future use and should NOT be set in production device trees. SMPU instances have security registers at offset 0x800+ including SECURECTL. By default, SMPU instances are configured as secure slaves in the System Protection Unit (SPU). This means only secure privileged code can access these registers. In the future, if SPU is configured to make SMPU instances non-secure slaves (SPU_SECUREP.SSEC=0), the Linux driver running in non-secure EL1 could potentially write SECURECTL registers. The adi,secure-access property is reserved for this use case. IN current status, we should not set adi,secure-access property. The driver cannot write SECURECTL from Linux because kernel runs in non-secure EL1 mode and SMPU instances are secure slaves by default. According to Errata 20000003, non-secure slaves cannot access secure slave registers. This errata is not the actual reason we cannot access SECURECTL currently. The actual reason is standard security with SMPU as a secure slave. The errata only matters if we configure SMPU as non-secure slave in the future. |
nunojsa
left a comment
There was a problem hiding this comment.
Here it goes my first run. Only reviewed the main driver for now. I think this is already feedback for the first round.
We should consider the place where the driver is. Not an expert here but the idea I have is that we should avoid drivers/soc as much as we can. I see some STM like:
bus: rifsc: introduce RIFSC firewall controller driver
RIFSC is a peripheral firewall controller that filter accesses based on
Arm TrustZone secure state, Arm CPU privilege execution level and
Compartment IDentification of the STM32 SoC subsystems.
Not sure if it matches that much with what we have in here. Anyways just to bring some discussion/awareness
d4a8553 to
dcb22bd
Compare
dcb22bd to
f528a06
Compare
|
Hey all, So just as context, all this SPU and SMPU are same for all(sc57x, sc59x, sx89x) ADSP processors. Only thing that differs is write-protect list size per chip (SC59x=214, SC846=213, SC594=202) and interrupt ID, which is expected. SC57x cannot be added same way though, its SMPU has different region count per instance (not uniform like others), so it need separate handling, out of scope for now. SPU though can be added but I will just keep it seperate from this PR. Since IPs are shared, I generalized driver/module naming from sc59x to adsp and moved peripheral count into per-chip match data, so later SC846 support can reuse same driver, only need devicetree node added. For now only SC598/SC59x stays wired into real devicetree. |
LLM reviewThis series adds the ADI ADSP System Memory Protection Unit (SMPU) and run: 31330441748
|
f528a06 to
678b09e
Compare
|
Addressed the points from LLM review. |
Add the binding for the System Memory Protection Unit (SMPU), the memory firewall found on Analog Devices ADSP/SHARC-FX processors. Register layout is identical across the supported chips, so a single compatible enum covers them; the SEC interrupt ID differs per chip and is described per-node via the "interrupts" property. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
Add a driver for the SMPU. The protection policy is owned by secure firmware. This driver maps the hardware, reports violations through the shared interrupt, and exposes the current state read-only via debugfs. Region count is uniform across the chips covered by this binding, so no per-chip match data is needed yet. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
Describe the System Memory Protection Unit on the SC59x SoC. The six protection-unit instances wired up on the SC598 and the shared violation interrupt. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
Add the binding for the System Protection Unit (SPU), the peripheral firewall found on Analog Devices ADSP processors. It sits between the system crossbar and the peripheral MMR interface and can write-protect a peripheral's registers from selected bus masters. Register layout is identical across the supported chips. The SEC interrupt ID and the number of write-protectable peripherals differ per chip. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
Add a driver for the SPU. The protection policy is owned by secure firmware; this driver maps the hardware, reports protection/security violations through its interrupt, and exposes the write-protect state read-only via debugfs. The number of write-protectable peripherals differs per chip, so it is carried as per-chip of_device_id match data rather than a fixed constant. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
Describe the System Protection Unit on the SC59x. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
Enable CONFIG_ADI_ADSP_SMPU and CONFIG_ADI_ADSP_SPU on the SC598 SOM EZKIT and EZLITE defconfigs. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
678b09e to
e6473c3
Compare
Thank you for all the comments! @nunojsa and @qasim-ijaz
v2 Changes:
So let me explain what happened here and why the drivers look completely different now. I fixed also all the concerns shared about v1.
TLDR: I reworked both the SMPU and SPU drivers into report-only drivers. They dont program any protection policy from device tree anymore. They just map the hardware, take the violation interrupt, log the violations and show the programmed state read-only over debugfs. The policy (which region is protected, from which master) is now owned by the firmware that runs before Linux, not by Linux.
Now the long story, because I think the reasoning matters more than the diff.
The static-regions parsing was failing because the device tree wrote base/size as single cells (u32) but the driver was reading them with of_property_read_u64, so it returned -EINVAL and the whole probe bailed. Fixed that first.
While bisecting I also found a real driver bug that had nothing to do with the test config: the RADDR base encoding. SMPU_RADDR[n].BADDR holds the base address in bits [31:12] in place, but the driver was right-shifting by 12 and writing it into [19:0]. So the region got programmed at the wrong address. It was hidden for 1MB-aligned addresses like 0x20040000 because the shifted value happened to keep the right high bits, it only showed up when I used 0x200D0000 which read back as 0x20000000. Also found a phantom STAT bit (BIT(5), doesnt exist in the HRM) sitting in the W1C mask, and a fail-open case where a single allowed-id leaves the second ID comparator matching transaction ID 0.
Then I hit the wall that actually changed the whole design. I wanted a clean interactive test: block something, poke it from the console with devmem, see it get blocked, keep the shell alive. For a WRITE block this is impossible on this SoC. The SPU/SMPU return the block as a posted bus error, which comes back as an uncontainable asynchronous SError, and arm64 has to treat that as fatal, so the kernel panics. I even tried enabling CONFIG_ARM64_RAS_EXTN to see if the SError could be contained, but the syndrome says uncontainable (AET=UC) so RAS cant help. Thats hardware, not fixable in software.
A READ block is different, it faults synchronously (precise data abort -> SIGBUS to just the reading process), so the shell survives. So the survivable test is read-protect + devmem read. Once I fixed the RADDR bug and the ID mask and pointed the region at the right instance (SMPU2 = L2-Core Port 0 covers 0x20000000-0x200FFFFF, so the base has to be in that window), it worked: devmem read -> Bus error, shell alive, region visible in debugfs.
But that whole exercise made the real problem obvious. Blocking the A55 from its own memory is not a real use case, its just a test artifact, and it fights the hardware the whole way. The actual value of these units is isolating an untrusted master (like the SHARC cores) from Linux memory/peripherals, and DETECTING when someone violates that. And critically, the SMPU/SPU config registers live in the secure MMR block (0x800+) which non-secure Linux cant even touch (bus error, also anomaly 20000003). So Linux is the wrong place to program the policy anyway.
Thats why I moved to report-only. I looked at how mainline handles this exact class of hardware and the pattern is completely uniform:
None of them program a protection policy from device tree. The DT binding rule says it straight out as "describe what the hardware has, not what an OS/driver does with it" (https://docs.kernel.org/devicetree/bindings/writing-bindings.html). So encoding permissions/allowed-ids/regions in DT was always going to get pushed back by the maintainers.
So,drivers/bus/ is the right home (STM32 and i.MX firewalls both live there). Report-only is the right model (matches all of the above). debugfs for the status/violations is fine because its not a stable ABI and nothing depends on it functionally (https://docs.kernel.org/filesystems/debugfs.html).
On who programs the policy then, for now this SoC has no TF-A BL31, the chain is Boot ROM -> U-Boot SPL -> U-Boot -> Linux, and U-Boot hands off to Linux at EL2 (ARMV8_SWITCH_TO_EL1 not set). U-Boot is the EL3/secure software so it can reach the secure register block. I confirmed this on hardware:
So U-Boot can read AND write the secure registers that Linux cant. That confirms the split: firmware (U-Boot) programs policy, Linux reports.
For anyone who wants to test/reproduce:
In U-Boot:
Then on kernel:
What we are expecting is this:
regionsbase0x200d0000,R--devmemreadBus error, shell alivestatusBus error: yesdmesg/violationsv1
One note,
These all works:
And these not:
I am investigating that issue, first I thought it is related with the SECURECTL (only secure accesible registers) but probably its deeper than that.
PR Type
PR Checklist