Hi Simon,
On 8/9/24 17:58, Simon Glass wrote:
Hi Michal,
On Fri, 9 Aug 2024 at 08:47, Michal Simek michal.simek@amd.com wrote:
On 8/9/24 16:44, Simon Glass wrote:
Hi Michal,
On Thu, 8 Aug 2024 at 23:39, Michal Simek michal.simek@amd.com wrote:
Hi Simon,
On 8/8/24 16:28, Simon Glass wrote:
Hi Michal,
On Wed, 7 Aug 2024 at 23:31, Michal Simek michal.simek@amd.com wrote:
On 8/7/24 16:36, Simon Glass wrote: > Hi Prasad, > > On Tue, 6 Aug 2024 at 23:05, Kummari, Prasad Prasad.Kummari@amd.com wrote: >> >> Hi Glass, >> >>> -----Original Message----- >>> From: Simon Glass sjg@chromium.org >>> Sent: Wednesday, August 7, 2024 3:21 AM >>> To: Kummari, Prasad Prasad.Kummari@amd.com >>> Cc: u-boot@lists.denx.de; git (AMD-Xilinx) git@amd.com; Simek, Michal >>> michal.simek@amd.com; Abbarapu, Venkatesh >>> venkatesh.abbarapu@amd.com; git@xilinx.com; >>> jagan@amarulasolutions.com; n-francis@ti.com; d-gole@ti.com >>> Subject: Re: [PATCH] cmd: sf: prevent overwriting the reserved memory >>> >>> Caution: This message originated from an External Source. Use proper >>> caution when opening attachments, clicking links, or responding. >>> >>> >>> Hi Prasad, >>> >>> On Tue, 6 Aug 2024 at 06:08, Prasad Kummari prasad.kummari@amd.com wrote: >>>> >>>> Added LMB API to prevent SF command from overwriting reserved memory >>>> areas. The current SPI code does not use LMB APIs for loading data >>>> into memory addresses. To resolve this, LMB APIs were added to check >>>> the load address of an SF command and ensure it does not overwrite >>>> reserved memory addresses. Similar checks are used in TFTP, serial >>>> load, and boot code to prevent overwriting reserved memory. >>> >>> The SPI flash may be used to load other things, not just an OS. What is your >>> use case or problem here? >> >> [Prasad]: We have observed that SF command can overwrite the reserved area without throwing any errors or warnings. >> This issue was noticed when the TF-A area is reserved in the Device Tree at address 0xf000000. The sf command is >> corrupting the reserved area, and U-Boot relocation address too. >> >> EX: TF-A reserved at ddr address 0xf000000 >> >> Versal NET> sf read 0x0f000000 0x0 0x100 ----> Overwriting reserved area. >> device 0 offset 0x0, size 0x100 >> SF: 256 bytes @ 0x0 Read: OK >> >> U-boot relocation address relocaddr = 0x000000007fec2000 >> >> Versal NET> sf write 0x0000000077ec2000 0x0 0x100 --> Overwriting reserved area. >> device 0 offset 0x0, size 0x100 >> SF: 256 bytes @ 0x0 Written: OK > > Yes. There are many things which can overwrite memory, e.g. the mw > command. It is a boot loader so this is normal. > > What image are you loading here?
In spi boot it can be Kernel/rootfs but at the end of day it doesn't really matter.
OK, in that case yes it should use lmb. That was the question I was trying to understand.
We have protection for srec, fs load, tftp and wget already.
c6855195e4b4 ("loads: Block writes into LMB reserved areas of U-Boot") aa3c609e2be5 ("fs: prevent overwriting reserved memory") a156c47e39ad ("tftp: prevent overwriting reserved memory") 04592adbdb99 ("net: wget: prevent overwriting reserved memory")
And this is just +1 patch to protect sf command that it doesn't touch reserved location. The same code should be used for other commands(nand, usb, etc) which loading block of data to memory because all of them shouldn't rewrite reserved memory.
In connection to mw/mtest/etc command protection can be also done but not sure if this is useful because you normally not using them for booting.
Exactly.
I am hoping that we can pull SPI flash into bootstd...has anyone looked at that? Are you using scripts or is there a special bootmeth?
We didn't find this issue in connection to boot. As I wrote in another reply we found it via spi testcases where TF-A was placed lower in DDR and test overwrite it without any other evidence. Part of the reason is that protection units are not enabled to protect secure FW.
Do you mean the sandbox test test/dm/sf.c ? Or something else? If the former, then we could mark dm_test_spi_flash() with CONFIG_SANDBOX
pytest one and I think it was this one. https://github.com/Xilinx/u-boot-xlnx/blob/master/test/py/tests/test_spi.py
Love is working on sending this test upstream as he did with others.
OK. But why is TF-A low in RAM? We really need to have a think about this TF-A thing. This is the second problem I've seen in a week (the first was rockchip resetting the timer). Is there a spec for what TF-A is supposed to do / not do?
It is user choice where they put trusted firmware. All depends on their application. Normally TF-A is in OCM but some users can have a need to use OCM for user application because for example it is much faster than DDR.
Not sure if there is any official spec but documentation is here. https://trustedfirmware-a.readthedocs.io/en/latest/
But this issue is not related to TF-A. It is just the way how we found it based on our partitioning. Because DDR can be partitioned for Secure OS, different cpus (RPUs in our case) or for processing units(MB/Risc-V/other masters) running out of programmable logic. In general when you are reserved location for whatever reason all loading commands shouldn't use them.
Thanks, Michal