Re: [PATCH 0/7] firmware: add FW_DYNAMIC_APPEND firmware type

On Wed, Aug 19, 2026 at 8:57 PM Anup Patel <[email protected]> wrote:
>
> On Tue, Jul 7, 2026 at 9:10 AM Zong Li <[email protected]> wrote:
> >
> > This series adds a fourth OpenSBI firmware type, FW_DYNAMIC_APPEND, next to
> > the existing FW_DYNAMIC, FW_JUMP and FW_PAYLOAD.
> >
> > Motivation
> > ==========
> >
> > FW_DYNAMIC requires the previous booting stage to build a struct
> > fw_dynamic_info in memory and pass its address in a2. Some booting stages
> > cannot easily allocate and pass such a structure, but they can load the
> > OpenSBI image and patch a few words at a known location before jumping to
> > it. FW_DYNAMIC_APPEND targets exactly that case: the struct fw_dynamic_info
> > is appended into the OpenSBI binary and patched in place by the previous
> > stage. For example, the previous boot stage runs on a dedicated hart.
> > Therefore, it cannot set up CSRs of another hart that runs OpenSBI.
> > This scenario will occurs in server systems with secure boot.
> >
> > Key design points
> > =================
> >
> >   - The appended struct fw_dynamic_info is placed immediately before .bss.
> >     .bss is NOBITS and is dropped by 'objcopy -O binary', so a structure
> >     after it would not be part of the flat .bin. Placing it before .bss
> >     makes it the last PROGBITS content, i.e. it sits at the tail of the
> >     .bin and can be located from the image size. The previous booting
> >     tage can therefore find the structure at:
> >
> >         load_address + binary_file_size - sizeof(struct fw_dynamic_info)
> >
> >   - struct fw_dynamic_info gains a new 'boot_dtb' field (info version 3)
> >     to carry the DTB address. Existing offsets are unchanged.
> >
> >   - The previous booting stage patches every field at runtime. The
> >     reservation is a KEEP() PROGBITS section, so the zero-initialised
> >     bytes still land at the tail of the .bin.
> >
> >   - On the cold-boot path (guarded by FW_DYNAMIC_APPEND), fw_base.S calls
> >     fw_dynamic_append_boot_args() in fw_dynamic_append.S. The helper sets
> >     a0 = mhartid, a2 = &appended struct, and a1 = boot_dtb, and clears the
> >     reserved a3/a4 that fw_platform_init consumes. Only a0-a4 are handled
> >     here, the remaining GPRs are already cleared by the existing _reset_regs
> >     later on the cold path. The boot_dtb load is gated on info version 3, so
> >     an older layout leaves a1 = 0 instead of reading past the struct.
> >
> > Backward compatibility
> > ======================
> >
> > All changes for the existing three firmware types are compiled out: the
> > fw_base.S fixup and the .fw_dynamic_info output section are under
> > #ifdef FW_DYNAMIC_APPEND, which is defined only by fw_dynamic_append.S and
> > fw_dynamic_append.elf.ldS. FW_DYNAMIC, FW_JUMP and FW_PAYLOAD binaries are
> > byte-for-byte unchanged.
> >
> > Build
> > =====
> >
> >   make PLATFORM=<platform_subdir> FW_DYNAMIC_APPEND=y
> >
> > On the generic platform FW_DYNAMIC_APPEND=y is set in
> > platform/generic/objects.mk, so it is built by default.
> >
> > Zong Li (7):
> >   firmware: fw_dynamic: add boot_dtb field to struct fw_dynamic_info
> >   firmware: fw_base.ldS: add .fw_dynamic_info output section before .bss
> >   firmware: add fw_dynamic_append firmware type
> >   firmware: fw_base.S: call fw_dynamic_append_boot_args on cold boot
> >   firmware: build and enable the FW_DYNAMIC_APPEND firmware type
> >   docs: firmware: document the FW_DYNAMIC_APPEND firmware type
> >   docs: firmware: list FW_DYNAMIC_APPEND in fw.md
>
> Instead of introducing new firmware type, same thing can be achieved by
> simply introducing a formatted header for all OpenSBI firmware.
>
> If the previous booting stage cannot pass parameters in a0, a1, and a2
> then it can simply override the values of a0, a1, and a2 by setting fields
> in OpenSBI firmware header.
>
> For example, we can have 128-byte OpenSBI firmware header defined
> in fw_base.S as follows:
>
> diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> index 0c5c65c1..0d63fe1e 100644
> --- a/firmware/fw_base.S
> +++ b/firmware/fw_base.S
> @@ -44,8 +44,64 @@
>   .section .entry, "ax", %progbits
>   .align 3
>   .globl _start
> + .globl _start_real
>   .globl _start_warm
>  _start:
> + /* OpenSBI firmware header */
> +_fw_header_jump:
> + .option push
> + .option norvc
> + j _start_real
> + .option pop
> +_fw_header_magic:
> + .word 0x4942534f /* ASCII string "OSBI" */
> +_fw_header_version:
> + .word 0x1
> +_fw_header_xlen:
> + .word __riscv_xlen
> +_fw_header_size:
> + .word (_fw_end - _fw_start)
> +_fw_header_flags:
> +#define FW_HEADER_FLAGS_OVERRIDE_A0 (1 << 0)
> +#define FW_HEADER_FLAGS_OVERRIDE_A1 (1 << 1)
> +#define FW_HEADER_FLAGS_OVERRIDE_A2 (1 << 2)
> + .word 0
> +_fw_header_override_a0:
> + .dword 0
> +_fw_header_reserved0:
> + .dword 0
> +_fw_header_override_a1:
> + .dword 0
> +_fw_header_reserved1:
> + .dword 0
> +_fw_header_override_a2:
> + .dword 0
> +_fw_header_reserved2:
> + .dword 0
> +_fw_header_reserved:
> + .fill 7, 8, 0
> +_start_real:
> + /* Check and override a0, a1, and a2 registers */
> + lla t0, _fw_header_flags
> + lw t0, (t0)
> + li t1, FW_HEADER_FLAGS_OVERRIDE_A0
> + and t2, t0, t1
> + beq t2, zero, _skip_override_a0
> + lla t2, _fw_header_override_a0
> + REG_L a0, (t2)
> +_skip_override_a0:
> + li t1, FW_HEADER_FLAGS_OVERRIDE_A1
> + and t2, t0, t1
> + beq t2, zero, _skip_override_a1
> + lla t2, _fw_header_override_a1
> + REG_L a1, (t2)
> +_skip_override_a1:
> + li t1, FW_HEADER_FLAGS_OVERRIDE_A2
> + and t2, t0, t1
> + beq t2, zero, _skip_override_a2
> + lla t2, _fw_header_override_a2
> + REG_L a2, (t2)
> +_skip_override_a2:
>   /* Find preferred boot HART id */
>   MOV_3R s0, a0, s1, a1, s2, a2
>   call fw_boot_hart
>
>
> Please note that OpenSBI firmware header defined above should
> work for both RV32 and RV64 and in the future can also be extended
> for RV128.
>

Hi Anup,
Thanks for your suggestion! I will send another patch series to implement this.
Additionally, I noticed that .option norvc might not be enough. We
also need .option norelax for the rv32 case. This is because GAS puts
R_RISCV_RELAX together with R_RISCV_JAL. As a result, the linker will
still use a compressed jump, which causes the offset to change. Let me
submit it for further review.
Thanks

> Regards,
> Anup

-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi