[PATCH 1/4] common/spl: fit: Avoid directly loading images to load_addr

[Jonas Karlman]

Hi Alexey,

On 12/12/2025 10:01 AM, Alexey Charkov wrote:
> Hi Jonas,
> 
> Thanks for your review!
> 
> On Thu, Dec 11, 2025 at 3:27 PM Jonas Karlman <jonas at kwiboo.se> wrote:
>>
>> Hi Alexey,
>>
>> On 12/10/2025 9:10 PM, Alexey Charkov wrote:
>>> Load images unconditionally to a known-safe location (which is
>>> CONFIG_SYS_LOAD_ADDR) to be later relocated to its desired load address.
>>>
>>> Some storage devices may be unable to directly load each of the images
>>> listed in a FIT to their desired load addresses, as is the case with
>>> e.g. booting from UFS storage on Rockchip RK3576, where part of the
>>> ATF should end up in SRAM but the UFS controller cannot directly write
>>> there.
>>>
>>> Furthermore, the code down the line does a memmove or image decompression
>>> from the original src_ptr, anyway, so there shouldn't be any performance
>>> implications from this change.
>>
>> If I am not mistaken the memmove will try to move the src_ptr into
>> load_addr and as long as they are same this is a no-op, so I am
>> expecting this will have performance implications.
> 
> If there is no difference in alignment between src_ptr vs. load_addr
> and no image overhead, the memmove down the line should indeed return
> early. You're right that there are after all cases where this change
> will affect performance.

On Rockchip platform the FIT images are always sector (512 byte) aligned
to help ensure source addr and load addr are always aligned and not
needing use of memmove or a bounce buffer.

See commit 5ad03fc77dfa ("rockchip: Align FIT image data to SD/MMC block
length").

I am not sure how the use of a larger sector size for UFS affect this.

> 
>> I suggest we instead try to configure the rk3576 firewall to allow the
>> UFS controller to access SRAM (SYSMEM) similar to how this is allowed on
>> e.g. rk3588. Please see U-Boot rk3576.c and rk3588.c, and also TF-A
>> plat/rockchip/rk3576/drivers/secure/firewall.c.
> 
> Won't this introduce some sort of a security hole? I have to admit I
> don't know how the "trusted" part of "trusted firmware" is enforced,
> but I have a suspicion that there is a security reason why this
> particular segment gets written to a more locked-down SRAM rather than
> system RAM before TF-A code takes over the management of the firewall.
> Would be great to know otherwise, because this sounds like a much
> cleaner (=fewer code changes) solution than the alternatives. Any
> advice from the list would be appreciated!

It is the part of TF-A that uses __pmusram* attributes that will be
loaded into the PMUSRAM, e.g. resume from suspend and other code that
must work in low power mode.

DRAM to SRAM is allowed so I cannot see an issue with allowing U-Boot
SPL to use DMA mode to load directly into SRAM until TF-A takes over.

As an alternative we can also add some option to not use external data
for the FIT image that is not using a load addr in normal DRAM, see [1]
for a hack I made before DMA/PIO mode was fixed for RK3588.

[1] https://gitlab.collabora.com/hardware-enablement/rockchip-3588/u-boot/-/commit/3dde83844f43e036283947f015224d2fc740a349

Regards,
Jonas

> 
>> For other Rockchip SoCs we have also used non-DMA mode or a bounce
>> buffer to work around similar issue in the drivers.
> 
> I'll look into a potential non-DMA mode in the driver for use in SPL,
> thanks for the pointer!
> 
>> On a final note I would not fully trust the SYS_LOAD_ADDR addr on
>> Rockchip platform, to my knowledge this is not really used for anything
>> and because of this have not fully been considered when other static
>> load addresses have been picked, so this could introduce some unintended
>> overlaps.
> 
> That should be relatively easy to fix with a quick Kconfig change, but
> I agree it might lead to an unknown number of regressions on platforms
> that didn't use SYS_LOAD_ADDR before. Good point, thank you.
> 
> Best regards,
> Alexey