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

On Fri, Aug 21, 2026 at 3:05 PM Zong Li <[email protected]> wrote:
>
> 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
>

Sounds good.

Regards,
Anup

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