[SPAM] RockPi4B: spl_load_fit_image() writing past end of buffer & MMC errors

[Xavier Drudis Ferran]

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'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.

When I didn't use that patch, my SPL hanged after "Trying to boot from MMC1", before 
loading the ATF2 image, just when loading ATF1. But maybe depening on what's in 
that area, or the particular layout resulting from a particular build, 
the effect might be different. It might be a little random.

> 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 ? Why can one only use the
first 8K from the 64K INTMEM1 ? Did you find any reason the rest
shouldn't be writable?

Could corrupted memory explain what you're seeing ?