Hi Xavier,
On 6/3/22 16:58, Xavier Drudis Ferran wrote:
El Fri, Jun 03, 2022 at 03:17:05PM +0200, Jerome Forissier deia:
First, I noticed that spl_load_fit_image() may write memory outside the range given in the image node. For example on RockPi 4B I have the following FIT (irrelevant parts omitted):
/ { images { atf_2 { /* File size is 8024 bytes */ data = /incbin/("bl31_0xff3b0000.bin"); compression = "none"; load = <0xff3b0000> } } }
I expect SPL to load atf_2 into 0xff3b0000 - 0xff3b200b but due to the alignment of the source data in the MMC, spl_load_fit_image() writes one more block and later moves the data in place with memcpy(). What are the guarantees that it is a safe thing to do?
In my case bl31_0xff3b0000.bin is 8020 bytes in size.
In my case, the image starts at offset 308 in the 512-byte MMC block (that offset is called 'overhead' in spl_load_fit_image()). As a result, (8204 + 308) / 512 + 1 = 17 blocks = 8704 bytes are read.
My overhead is 352, it's reading 17 blocks too.
For some reason I can't explain, on my board only the first 8K of the 64K SRAM range 0xff3b0000 - 0xff3c0000 (INTMEM1) can be safely written to. Any data written after this point corrupt the lower 8K. I found nothing in the rk3399 TRM about this [1].
In my Rock Pi 4B this does not happen. It happily writes the 17 blocks. Or it's just that I'm not noticing the problem ? Does it hang or give you an error, or do you just find it out by reading the SRAM?
I found out when trying to use signed FIT images. The board would boot fine otherwise.
I am setting CONFIG_SPL_FIT_SIGNATURE=y and I have modified the script arch/arm/mach-rockchip/make_fit_atf.py so that it generates the required hash and signature nodes in u-boot.its. Then I am basically following the instructions in doc/uImage.FIT/signature.txt -- generating a RSA key pair with OpenSSL and using 'mkimage -k keys -K -r $(other-args...)'.
Please refer to the patches at:
https://git.codelinaro.org/linaro/dependable-boot/meta-ts/-/merge_requests/7...
It is work in progress really but it should give you an idea of what I am doing, and possibly help reproduce the issue.
I also confirmed the issue with a small test case. Could you please try the following patch?
---8<----------------------------------------------------------------------- diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index f6ccd837aa..810d5fa6db 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -401,6 +401,12 @@ static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start, struct mmc_cmd cmd; struct mmc_data data;
+ printf("== 0x%08x\n", *(u32 *)0xff3b0100); + *(u32 *)0xff3b0100 = 0; + printf("== 0x%08x\n", *(u32 *)0xff3b0100); + *(u32 *)0xff3b2100 = 0xdeadbeef; + printf("== 0x%08x\n", *(u32 *)0xff3b0100); + if (blkcnt > 1) cmd.cmdidx = MMC_CMD_READ_MULTIPLE_BLOCK; else ---8<-----------------------------------------------------------------------
Guess what, this **does** print "0xdeadbeef" on the console on my board! :O
I'm using the patch I sent yesterday:
[PATCH 0/3] arm: rockchip: rk3399: rock-pi-4: power domain driver to boot from MMCSD https://lists.denx.de/pipermail/u-boot/2022-June/485498.html
(instead of the patch you can simply configure CONFIG_ROCKCHIP_SPL_RESERVE_IRAM from make nconfig or so)
Which reserves 0x0 - 0x14fff bytes in DDRAM for bl31. I found that otherwise, SPL memory in that area gets overwritten by bl31_0x00000000.bin. I don't remember exactly what was there, but in my tests, bl31_0x00000000 gets loaded before bl31_0xff3b0000.bin. Depending on what data or code is overwritten, SPL might be doing something funny.
I don't have bl31_0x00000000.bin. The bl31*.bin files come from bl31.elf which is split by arch/arm/mach-rockchip/make_fit_atf.py. It could be that we are using different versions of TF-A or different build parameters, but that seems unrelated I think.
Anyways, I tried using a temporary buffer allocated on the heap to handle the first and last blocks at least (the load address is properly aligned for info->read() so the middle blocks can be read in one go). It works but not reliably. And that is the second problem. mmc_read_blocks() in mmc_bread() sometimes returns an error. If I read blocks one by one in a loop THE read randomly fails after a few blocks only. The error is -110 (-ETIMEDOUT) from dwmci_send_cmd().
Am I using the MMC read incorrectly?
It's not that you're using the read incorrectly.
It's that the change shouldn't be needed ?
Ideally, yes, but it doesn't seem right to overflow the specified range. Who knows what might be immediately after the end of the buffer for a given image? Perhaps another image has been written there already, or the image might be right before a reserved address range such is some mapped I/O area?
Why can one only use the first 8K from the 64K INTMEM1 ? Did you find any reason the rest shouldn't be writable?
No, I don't understand. As I said the rest seems writable (at least up to some point) but writing to it corrupts the memory below...
Could corrupted memory explain what you're seeing ?
I found no evidence of corruption until SPL writes to 0xff3b2000 and above.
Thanks,