Re: [PATCH 3/5] bloblist: Load the bloblist from the previous loader

[Michal Simek <michal.simek@amd.com>]

On 12/19/23 22:11, Raymond Mao wrote:
> During bloblist initialization, when CONFIG_OF_BOARD is defined,
> invoke the platform custom function to load the bloblist via boot
> arguments from the previous loader.
> If the bloblist exists, copy it into the fixed bloblist memory region.
> 
> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> ---
>   common/bloblist.c  | 47 ++++++++++++++++++++++++++++------------------
>   include/bloblist.h | 16 ++++++++++++++++
>   2 files changed, 45 insertions(+), 18 deletions(-)
> 
> diff --git a/common/bloblist.c b/common/bloblist.c
> index 232ca4c6ce..c89f7a1554 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -486,31 +486,38 @@ int bloblist_init(void)
>   	bool fixed = IS_ENABLED(CONFIG_BLOBLIST_FIXED);
>   	int ret = -ENOENT;
>   	ulong addr, size;
> -	bool expected;
> -
> -	/**
> -	 * We don't expect to find an existing bloblist in the first phase of
> -	 * U-Boot that runs. Also we have no way to receive the address of an
> -	 * allocated bloblist from a previous stage, so it must be at a fixed
> +	/*
> +	 * If U-Boot is not in the first phase, an existing bloblist must be
> +	 * at a fixed address.
> +	 */
> +	bool from_addr = fixed && !u_boot_first_phase();
> +	/*
> +	 * If U-Boot is in the first phase that a board specific routine should
> +	 * install the bloblist passed from previous loader to this fixed
>   	 * address.
>   	 */
> -	expected = fixed && !u_boot_first_phase();
> +	bool from_board = fixed && IS_ENABLED(CONFIG_OF_BOARD) &&
> +			  u_boot_first_phase();
> +
>   	if (spl_prev_phase() == PHASE_TPL && !IS_ENABLED(CONFIG_TPL_BLOBLIST))
> -		expected = false;
> +		from_addr = false;
>   	if (fixed)
>   		addr = IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED,
>   				      CONFIG_BLOBLIST_ADDR);
>   	size = CONFIG_BLOBLIST_SIZE;
> -	if (expected) {
> +
> +	if (from_board)
> +		ret = board_bloblist_from_boot_arg(addr, size);
> +	else if (from_addr)
>   		ret = bloblist_check(addr, size);
> -		if (ret) {
> -			log_warning("Expected bloblist at %lx not found (err=%d)\n",
> -				    addr, ret);
> -		} else {
> -			/* Get the real size, if it is not what we expected */
> -			size = gd->bloblist->total_size;
> -		}
> -	}
> +
> +	if (ret)
> +		log_warning("Bloblist at %lx not found (err=%d)\n",
> +			    addr, ret);
> +	else
> +		/* Get the real size */
> +		size = gd->bloblist->total_size;
> +
>   	if (ret) {
>   		if (CONFIG_IS_ENABLED(BLOBLIST_ALLOC)) {
>   			void *ptr = memalign(BLOBLIST_ALIGN, size);
> @@ -519,7 +526,8 @@ int bloblist_init(void)
>   				return log_msg_ret("alloc", -ENOMEM);
>   			addr = map_to_sysmem(ptr);
>   		} else if (!fixed) {
> -			return log_msg_ret("!fixed", ret);
> +			return log_msg_ret("BLOBLIST_FIXED is not enabled",
> +					   ret);
>   		}
>   		log_debug("Creating new bloblist size %lx at %lx\n", size,
>   			  addr);
> @@ -532,6 +540,9 @@ int bloblist_init(void)
>   		return log_msg_ret("ini", ret);
>   	gd->flags |= GD_FLG_BLOBLIST_READY;
>   
> +	bloblist_show_stats();
> +	bloblist_show_list();
> +
>   	return 0;
>   }
>   
> diff --git a/include/bloblist.h b/include/bloblist.h
> index b5d0f147f6..f5c623133d 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -445,6 +445,22 @@ void bloblist_reloc(void *to, uint to_size, void *from, uint from_size);
>    */
>   int bloblist_init(void);
>   
> +#if (IS_ENABLED(CONFIG_ARCH_QEMU) && IS_ENABLED(CONFIG_ARM))

How do you think that others start to use it?
Adding that platforms here? I don't think it is scalable.

Thanks,
Michal