Re: [PATCH] riscv: bypass malloc when spl fit boots from ram

Samuel Holland
2022-12-28T02:47:54Z

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 <rick@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.

> /* 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.

Regards,
Samuel