Re: [PATCH 30/32] fdt: Allow the devicetree to come from a bloblist

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Simon,

On Wed, Aug 30, 2023 at 12:05:01PM -0600, Simon Glass wrote:
> Standard passage provides for a bloblist to be passed from one firmware
> phase to the next. That can be used to pass the devicetree along as well.
> Add an option to support this.
> 
> Tests for this will be added as part of the Universal Payload work.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/bloblist.c                  |  2 ++
>  doc/develop/devicetree/control.rst |  3 ++
>  dts/Kconfig                        |  8 ++++++
>  include/bloblist.h                 |  5 ++++
>  include/fdtdec.h                   |  3 +-
>  lib/fdtdec.c                       | 44 ++++++++++++++++++++++--------
>  6 files changed, 52 insertions(+), 13 deletions(-)
> 
> diff --git a/common/bloblist.c b/common/bloblist.c
> index 6f2a4577708..b07ede11cfe 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -48,9 +48,11 @@ static struct tag_name {
>  	{ BLOBLISTT_ACPI_TABLES, "ACPI tables for x86" },
>  	{ BLOBLISTT_SMBIOS_TABLES, "SMBIOS tables for x86" },
>  	{ BLOBLISTT_VBOOT_CTX, "Chrome OS vboot context" },
> +	{ BLOBLISTT_CONTROL_FDT, "Control FDT" },
>  
>  	/* BLOBLISTT_PROJECT_AREA */
>  	{ BLOBLISTT_U_BOOT_SPL_HANDOFF, "SPL hand-off" },
> +	{ BLOBLISTT_VBE, "VBE" },
>  	{ BLOBLISTT_U_BOOT_VIDEO, "SPL video handoff" },
>  
>  	/* BLOBLISTT_VENDOR_AREA */
> diff --git a/doc/develop/devicetree/control.rst b/doc/develop/devicetree/control.rst
> index cbb65c9b177..56e00090166 100644
> --- a/doc/develop/devicetree/control.rst
> +++ b/doc/develop/devicetree/control.rst
> @@ -108,6 +108,9 @@ If CONFIG_OF_BOARD is defined, a board-specific routine will provide the
>  devicetree at runtime, for example if an earlier bootloader stage creates
>  it and passes it to U-Boot.
>  
> +If CONFIG_OF_BLOBLIST is defined, the devicetree comes from a bloblist passed
> +from a previous stage.
> +

Is this bloblist in the format described in the firmware handoff [0] ?

> +config OF_BLOBLIST
> +	bool "DTB is provided by a bloblist"
> +	help
> +	  Select this to read the devicetree from the bloblist. This allows
> +	  using a bloblist to transfer the devicetree between  U-Boot phases.
> +	  The devicetree is stored in the bloblist by an early phase so that
> +	  U-Boot can read it.
> +

I dont think this is a good idea.  We used to have 4-5 different options
here, which we tried to clean up and ended up with two very discrete
options.  Why do we have to reintroduce a new one?  Doesn't that bloblist
have a header of some sort?  The bloblist literally comes from a previous
stage bootloader which is what OF_BOARD is here for. So why can't we just
read the header and figure out if the magic of the bloblist matches?

>  config OF_BOARD
>  	bool "Provided by the board (e.g a previous loader) at runtime"
>  	default y if SANDBOX || OF_HAS_PRIOR_STAGE
> diff --git a/include/bloblist.h b/include/bloblist.h
> index 080cc46a126..e16d122f4fb 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -103,6 +103,11 @@ enum bloblist_tag_t {
>  	BLOBLISTT_ACPI_TABLES = 0x104,	/* ACPI tables for x86 */
>  	BLOBLISTT_SMBIOS_TABLES = 0x105, /* SMBIOS tables for x86 */
>  	BLOBLISTT_VBOOT_CTX = 0x106,	/* Chromium OS verified boot context */
> +	/*

[...]

> 
[0] https://github.com/FirmwareHandoff/firmware_handoff

Thanks
/Ilias