Hi Simon: Simon Glass <s...@chromium.org> 于2019年7月7日周日 上午1:16写道:
> Hi Andy, > > On Mon, 24 Jun 2019 at 04:37, Andy Yan <andy....@rock-chips.com> wrote: > > > > Hi Simon: > > > > Glad to see you online again. > > > > On 2019/6/23 上午3:10, Simon Glass wrote: > > > Hi Andy, > > > > > > On Tue, 28 May 2019 at 09:34, Andy Yan <andy....@rock-chips.com> > wrote: > > >> Hi Simon: > > >> > > >> On 2019/5/23 上午3:39, Simon Glass wrote: > > >> > > >> Hi Andy, > > >> > > >> On Tue, 21 May 2019 at 19:56, Andy Yan <andys...@gmail.com> wrote: > > >> > > >> Hi Simon: > > >> > > >> Simon Glass <s...@chromium.org> 于2019年5月22日周三 上午8:28写道: > > >> > > >> Hi Andy, > > >> > > >> On Tue, 21 May 2019 at 00:51, Andy Yan <andy....@rock-chips.com> > wrote: > > >> > > >> Hi Simon: > > >> > > >> On 2019/5/20 下午11:35, Simon Glass wrote: > > >> > > >> Hi Andy, > > >> > > >> On Mon, 20 May 2019 at 00:34, Andy Yan <andy....@rock-chips.com> > wrote: > > >> > > >> Hi Simon: > > >> > > >> On 2019/5/19 上午12:26, Simon Glass wrote: > > >> > > >> Hi Andy, > > >> > > >> Instead of this could you: > > >> > > >> - move ATF? > > >> > > >> All rockchip based arm64 ATF run from the start 64KB of dram as this > > >> will give convenient for kernel manage the memory. > > >> > > >> On the other hand, change the ATF load address will break the > > >> compatibility of the exiting firmware. > > >> > > >> - change the SPL load address so it is not in the way (since TPL can > > >> load to any address) > > >> > > >> The SPL is loaded by bootrom after TPL back to bootrom, so the load > > >> address if fixed by bootrom code. > > >> > > >> I think you are creating a nightmare here. If you have to do things > > >> like this for older and smaller SoCs, OK. But it should not be used > > >> for newer ones that can do things properly. > > >> > > >> Most rockchip based SOC sram is small, even in the future soc > roadmap, > > >> this situation will still exist, larger sram means more cost. > > >> > > >> I believe the RK3399 has 192KB. What is the minimum size in new chips? > > >> > > >> The sram size of RK3328 is 32KB, and now the u-boot-tpl.bin of rk3328 > without storage drive is 28KB. > > >> The available sram size for TPL on RK3326 is 10KB, our another A35 > based IOT SOC has the same limitation. > > >> > > >> OK, I see. > > >> > > >> > > >> As for the current spl for rockchip soc in mainline, we use a > workaround > > >> by reserve large space at the head of spl(see > > >> CONFIG_ROCKCHIP_SPL_RESERVE_IRAM ), this generate a very large spl > binary. > > >> > > >> Yes. > > >> > > >> As for my patch, the spl relocation is disabled default, we only > enable > > >> it on necessary platform, so it won't hurt others . > > >> > > >> Well it adds more code and complexity. Perhaps it makes sense to add > > >> this, but I want to understand the need. > > >> > > >> The bootrom has so many limitations that it just creates pain. > > >> > > >> I know we can build mmc or other storage driver into TPL so we can use > > >> tpl load spl on some platform that sram is big enough, but there are > > >> also many rockchip soc has very small sram, so we tend to only do dram > > >> initialization in tpl, and let bootrom load next stage . > > >> > > >> See above > > >> > > >> For the consideration of software development, we also want to keep > TPL > > >> clean, only do dram initialization(as it current status), this make > our > > >> internal dram team work more simple, they don't need to care about > other > > >> modules like mmc. > > >> > > >> Yes I understand this, but the boot ROM should be provided as a > > >> library to call into: > > >> > > >> int mmc_read(void *addr, int start_block, int end_block) > > >> int spi_read(void *addr, int start_block, int end_block) > > >> > > >> Then SPL or TPL can use it without all the strange limitations we > have now. > > >> > > >> Since you probably already have these functions somewhere in the boot > > >> ROM, you could implement this using a function table somewhere in the > > >> ROM with a magic number before it, so that SPL can find it. > > >> > > >> The Bootrom do much more work than directly load the spl binary. It > will do somthing like checksum, look for the backup when the current image > is invalid, also including security check when secure boot is enabled. This > is why we did much work to add back_too_bootrom mechanism in mainline in > 2017. > > >> > > >> Yes I understand that, but it is also quite inflexible, and creates > > >> enormous problems with bootloaders. > > >> > > >> I am not suggesting that you remove functionality. I am suggesting > > >> that you allow bootloaders to call into some of it, to reduce the > > >> problems caused by the inflexible bootrom. > > >> > > >> > > >> I checked with people who write bootrom code these days, as > different chip written by different people from different team, it took a > bit long time to figure out this. > > >> > > >> Yes , bootrom have storage access api like > mmc_read/read_sfc_nand/read_sfc_nor/nandc_read, but bootrom does not > provide a fixed table for these api, and the address for these api are > different on different chip, this means we have to list the api address > chip by chip in SPL code. There is another thing, as the bootrom code are > written by different person, the api interface don't keep constant: > sfc_nor_read on one chip is sfc_nor_read(void *addr, int start, int len), > but on another chip is sfc_nor_read(int start, void *addr, int len), this > make things complicated. > > >> > > >> Also as what I mentioned before, the Bootrom do much more work than > directly load the spl binary, especially in secure boot mode, but the > bootrom don't want to export secure related api for security concern . > > >> > > >> So this seems not a good choice to reuse bootrom api in spl. > > > While I understand what you are saying, I don't think it would be hard > > > to add a little interface layer for each SoC which supports reading > > > from each type of device, and knows the SoC ROM address to call, and > > > deals with changing args, etc. > > > > > > Really, it should be a very small amount of code. And the benefit is a > > > large reduction in TPL/SPL code size and a lot less of the insane > > > hacks we ahve now! > > > > > > > Yes, it's not hard to add a wrapper for every soc from technical side, > > but it make things complicated. When we get a new soc, we need to check > > with the bootrom person for the storage api type by type, the we get a > > long header file nested with ''if chipA, else if chipB, esle if chipC", > > this looks not nice . > > I suggest having one header per SoC. > > Also, if you do talk to your chip people, ask them to use a consistent > API from now on, and put the table of function pointers (and perhaps > an API version) in the ROM itself. > > > > > What's more , there is one more issue, bootrom will not export the > > secure related api, so we can't do secure boot with this method. > > Why not? Making an API hidden does not help security. > > I honestly worry that no one at Rockchip is thinking about how to > actually implement a bootloader that covers all the chips that you > ship. Consistency takes effort. > > > > > I still prefer to reuse the existing relocation mechanism. > > To keep compatibility with the old closed-source binary? > > I feel that you should show some willingness to make things better in > the future. No other SoC needs relocation in SPL as far as I know. > Actually: There are ! (1)Just do a grep about relocate_code function, you will see powerpc handle spl relocate themselves. (2) function spl_relocate_stack_gd written by yourself. What has happened with Rockchip SoCs to cause this? > > If you must add relocation please use CONFIG_SPL_RELOCATE since not > relocating should be the default. And try to put the code hidden away > in spl_reloc.c or something. > I have already reuse the existing CONFIG_SPL_SKIP_RELOCATE config in my patch. > Regards, > Simon > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot