Hi Simon,

Thanks for testing this. You're right, the FDT is written in place
inside the FIT when it has no load address, so this needs fixing in
this series rather than in a follow-up.

Please don't apply v3, I'll send v4 which copies the FDT to its own
area before recording the loadables and adds that case to the test.

I'll also reword the 4/6 commit message as E Shattow suggested.

On Thu, 2026-11-05 at 09:11 +0300, Nikita Shubin wrote:
> Hi Simon,
> 
> On Tue, 2026-11-03 at 07:49 -0800, Simon Glass wrote:
> > Hi Nikita,
> > 
> > On 2026-11-03T09:53:01+11:00, Nikita Shubin
> > <[email protected]> wrote:
> > 
> > > Tested with provided sandbox test with a grafted FIT image
> > > containing OpenSBI (firmware), U-Boot (loadable) and a DTB.
> > 
> > One thing I'd like to check.. the new test gives the FDT image a
> > load
> > address, but the in-tree RISC-V FIT in arch/riscv/dts/binman.dtsi
> > does
> > not, so the test misses the layout this series is aimed at.
> > 
> > Without a load address, fit_image_load() leaves the FDT inside the
> > FIT, or copies it to a buffer of exactly its own length if it is
> > not
> > 8-byte aligned. Patch 5 then adds 8K to the FDT's total size and
> > writes the fit-images nodes into it.
> > 
> > I tried both cases by dropping the load address from a scratch copy
> > of
> > your test, with two loadables. With an aligned FDT, the nodes
> > overwrite the FIT that follows, so fit_conf_get_node() on 'config-
> > 1'
> > fails after the load. With a misaligned FDT, the total size ends up
> > at
> > about 10K and the nodes go past the end of the small buffer. The
> > test
> > still passes as is, since neither problem shows up when the FDT has
> > its own load address.
> > 
> > Really you should copy the FDT somewhere with enough space before
> > recording the loadables, and add this case to the test.
> > spl_load_simple_fit() avoids this by loading the FDT to its own
> > area.
> > Perhap a follow-up?
> 
> Noted. I'll investigate this.
> 
> > 
> > > This series improves CONFIG_SPL_LOAD_FIT_FULL support on RISC-V,
> > > primarily to enable loading OpenSBI as firmware and recording
> > > loadable images (e.g., U-Boot) into the FDT passed to the next
> > > boot
> > > stage.
> > 
> > Just to note that this still reads as though the aim is OpenSBI and
> > loadables, which the simple path already handles. Patch 2 now gives
> > the real reason (CONFIG_SPL_FIT_FULL_CHECK) but this isn't worth a
> > new
> > version.
> > 
> > Regards,
> > Simon

Reply via email to