> On 12/27/22 21:22, Rick Chen wrote: > > Hi Samuel, > > > > Samuel Holland <sam...@sholland.org> 於 2022年12月28日 週三 上午10:47寫道: > >> > >> On 12/22/22 01:21, Rick Chen wrote: > >>> When fit image boots from ram, the payload will > >>> be prepared in the address of SPL_LOAD_FIT_ADDRESS. > >>> In spl fit generic flow, it will malloc another > >>> memory address and copy whole fit image to this > >>> malloc address. But it is un-necessary for booting > >>> from RAM. > >> > >> This should mostly be solved by using `mkimage -E` or binman's > >> `fit,external-offset`. Then only the FIT structure, without any data, > >> will be copied. > >> > >>> This patch improves this flow by declare the > >>> board_spl_fit_buffer_addr() to replace the original one. > >>> The larger image size (eq: Kernel Image 10~20MB), it > >>> can save more booting time. > >>> > >>> Also enhance memcpy function by checking source and > >>> destination address. If they are the same address, > >>> just return and don't copy data anymore. > >>> > >>> Signed-off-by: Rick Chen <r...@andestech.com> > >>> --- > >>> arch/riscv/lib/memcpy.S | 2 ++ > >>> arch/riscv/lib/spl.c | 16 ++++++++++++++++ > >>> 2 files changed, 18 insertions(+) > >>> > >>> diff --git a/arch/riscv/lib/memcpy.S b/arch/riscv/lib/memcpy.S > >>> index 00672c19ad..9884077c93 100644 > >>> --- a/arch/riscv/lib/memcpy.S > >>> +++ b/arch/riscv/lib/memcpy.S > >>> @@ -9,6 +9,7 @@ > >>> /* void *memcpy(void *, const void *, size_t) */ > >>> ENTRY(__memcpy) > >>> WEAK(memcpy) > >>> + beq a0, a1, .copy_end > >> > >> I see one call to memcpy() in common/spl/spl_ram.c. I think it would > >> make more sense to do the check there instead of adding a branch to > >> every memcpy() call. > > > > There is not only one memcpy being called in spl_ram.c during spl > > booting progress, but also in spl_fit.c . > > Either your FIT uses external data, and the one in spl_ram.c is trivial, > or it uses internal data, and the one in spl_fit.c is unavoidable. > Either way, there is only one place where this change makes a difference. > > > Otherwise I prefer not to modify it in generic flow level but arch > > level, because the src and dest checking > > may not be necessary for other booting devices. > > I see arch/arm/lib/memcpy.S has similar logic, so other code may be > depending on this optimization, and there is precedent for this change. > > >> > >>> /* Save for return value */ > >>> mv t6, a0 > >>> > >>> @@ -121,6 +122,7 @@ WEAK(memcpy) > >>> 2: > >>> > >>> mv a0, t6 > >>> +.copy_end: > >>> ret > >>> > >>> .Lmisaligned_word_copy: > >>> diff --git a/arch/riscv/lib/spl.c b/arch/riscv/lib/spl.c > >>> index f4d3b67e5d..18f86ee207 100644 > >>> --- a/arch/riscv/lib/spl.c > >>> +++ b/arch/riscv/lib/spl.c > >>> @@ -61,3 +61,19 @@ void __noreturn jump_to_image_no_args(struct > >>> spl_image_info *spl_image) > >>> #endif > >>> image_entry(gd->arch.boot_hart, fdt_blob); > >>> } > >>> + > >>> +#ifdef CONFIG_SPL_RAM_SUPPORT > >>> +struct legacy_img_hdr *spl_get_load_buffer(ssize_t offset, size_t size) > >>> +{ > >>> + return (void *)(CONFIG_SPL_LOAD_FIT_ADDRESS + offset); > >>> +} > >>> + > >>> +void *board_spl_fit_buffer_addr(ulong fit_size, int sectors, int bl_len) > >>> +{ > >>> + void *buf; > >>> + > >>> + buf = spl_get_load_buffer(0, sectors * bl_len); > >>> + > >>> + return buf; > >>> +} > >>> +#endif > >> > >> You cannot provide strong definitions of these functions at an > >> architecture level, as this prevents any board from overriding them. > > > > Originally I wanted to put it in arch/riscv/cpu/ax25/spl.c . > > However It is a fundamental improvement for spl_ram flow, but not > > likely a private change. > > That's why I put it here, so that all boards of RISC-V can be benefited. > > This change isn't in any way specific to RISC-V. If you want to make > this generic, it would belong in spl_ram.c. But I still think it is > inappropriate to provide a strong definition of board_* functions > outside board code.
OK, I will move it to arch/riscv/cpu/ax25/spl.c. > > Regards, > Samuel >