Hi Ludwig, On 2026-05-07T12:06:22, Ludwig Nussel <[email protected]> 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 <[email protected]> > > board/emulation/qemu-arm/qemu-arm.c | 50 ++++++++++++++++++++++++++++++++++--- > configs/qemu_arm64_defconfig | 1 + > 2 files changed, 48 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass <[email protected]> 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
