[PATCH v3 1/4] qemu: overlay signature nodes

[Simon Glass]

Hi Ludwig,

On 2026-05-07T12:06:22, Ludwig Nussel <ludwig.nussel at siemens.com> wrote:
> qemu: overlay signature nodes
>
> The keys trusted for FIT signature verification are supposed to be
> embedded in the device tree built into u-boot. When running in Qemu it's
> convenient to use the device tree provided by the VM which doesn't know
> about signatures though. So merge both device trees at startup.
>
> Signed-off-by: Ludwig Nussel <ludwig.nussel at siemens.com>
>
> board/emulation/qemu-arm/qemu-arm.c | 50 ++++++++++++++++++++++++++++++++++---
>  configs/qemu_arm64_defconfig        |  1 +
>  2 files changed, 48 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

a few things below

> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> @@ -144,12 +144,56 @@ int dram_init_banksize(void)
> +/* QEMU loads a generated DTB for us at the start of RAM.
> + * When using signatures we may have a built-in FDT that contains our known
> + * public keys nevertheless. So merge those nodes into QEMU's FDT.
> + * We cannot merge the other way around (eg in fdtdec_board_setup()
> + * or board_fix_fdt() at this stage as U-Boot might be started from
> + * a ROM location.
> + * At the same time U-Boot needs QEMU's FDT to initialize serial
> + * devices even before relocation.
> + */

nits: The opening paren after 'eg' is never closed. Also the subject
says 'overlay signature nodes', but the code merges the whole root of
the built-in DT into QEMU's, not just /signature  - either narrow the
merge to /signature or reword the subject and commit message to
describe what really happens.

> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> @@ -144,12 +144,56 @@ int dram_init_banksize(void)
> +     void *qemu_fdt = (void *)CFG_SYS_SDRAM_BASE;
> +     int ret = -EINVAL;
> +
> +     if (fdt_check_header(qemu_fdt) != 0) {
> +             log_err("Invalid QEMU FDT at %p\n", qemu_fdt);
> +             goto out;
> +     }

On this error path you still execute '*fdtp = qemu_fdt' at out: and
return -EINVAL, so the caller is handed a pointer to garbage. Please
leave *fdtp untouched, or only assign it once the header has been
validated.

> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> @@ -144,12 +144,56 @@ int dram_init_banksize(void)
> +     ret = fdt_increase_size(qemu_fdt, 1024 + fdt_totalsize(*fdtp));
> +     if (ret) {
> +             log_err("Failed to resize FDT overlay: %s", fdt_strerror(ret));
> +             goto out;
> +     }

fdt_increase_size() resizes in place - there is no bound on the buffer
at SDRAM_BASE, so on failure this writes past whatever QEMU put there.
This is fine, from my understanding, but a short comment about that
assumption would help.

Also missing the trailing '\n', unlike the other log_err() calls.

> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> @@ -144,12 +144,56 @@ int dram_init_banksize(void)
> +              * existing setups might have injected them into
> +              * QEMUS's FDT already.

QEMU's.

> diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
> @@ -7,6 +7,7 @@ CONFIG_ENV_SIZE=0x40000
>  CONFIG_ENV_SECT_SIZE=0x40000
>  CONFIG_DEFAULT_DEVICE_TREE='qemu-arm64'
>  CONFIG_OF_LIBFDT_OVERLAY=y
> +CONFIG_OF_OMIT_DTB=n
>  CONFIG_SYS_LOAD_ADDR=0x40200000

The convention for disabling a bool in a defconfig is '#
CONFIG_OF_OMIT_DTB is not set', not '=n' - please run 'make
savedefconfig' if you want to check the result.

Regards,
Simon