[PATCH v3] platform: generic: Keep some empty space in FDT passed to next stage
Anup Patel
anup at brainfault.org
Tue Dec 16 10:07:03 PST 2025
On Tue, Dec 16, 2025 at 11:01 PM Samuel Holland
<samuel.holland at sifive.com> wrote:
>
> On 2025-12-16 11:09 AM, Anup Patel wrote:
> > On Tue, Dec 16, 2025 at 9:25 PM Samuel Holland
> > <samuel.holland at sifive.com> wrote:
> >>
> >> Anup,
> >>
> >> On 2025-12-16 8:45 AM, Anup Patel wrote:
> >>> On Tue, Dec 9, 2025 at 11:01 AM Anup Patel <apatel at ventanamicro.com> wrote:
> >>>>
> >>>> Leaving no empty space in the FDT passed to the next booting stage
> >>>> causes the following U-Boot crash on Ventana internal platforms:
> >>>>
> >>>> Unhandled exception: Load access fault
> >>>> EPC: 00000000fffa6372 RA: 00000000fffa7418 TVAL: 0001746174730068
> >>>> EPC: 0000000080245372 RA: 0000000080246418 reloc adjusted
> >>>>
> >>>> SP: 00000000fef38440 GP: 00000000fef40e60 TP: 0000000000000000
> >>>> T0: 00000000fef40a70 T1: 000000000000ff00 T2: 0000000000000000
> >>>> S0: 00000000fffc17a8 S1: 00000000fef38d40 A0: 7375746174730068
> >>>> A1: 00000000fffc17a8 A2: 0000000000000010 A3: 0000000000000010
> >>>> A4: 0000000000000000 A5: 00000000fffc17b8 A6: 0000000000ff0000
> >>>> A7: 000000000000b100 S2: 0000000000000000 S3: 0000000000000001
> >>>> S4: 00000000fef38d40 S5: 7375746174730068 S6: 0000000000000000
> >>>> S7: 00000000fef4eef0 S8: 00000000fef4ef90 S9: 0000000000000000
> >>>> S10: 0000000000000000 S11: 00000000fef4efc0 T3: 00000000fef40ea8
> >>>> T4: 0000000000ff0000 T5: 00000000fef40a60 T6: 00000000fef40a6c
> >>>>
> >>>> To address the above issue, keep some minimal empty space in the
> >>>> FDT instead of no empty space.
> >>>>
> >>>> Fixes: bbe9a23060e9 ("platform: generic: Pack the FDT after applying fixups")
> >>>> Signed-off-by: Anup Patel <apatel at ventanamicro.com>
> >>>
> >>> Applied this patch to the riscv/opensbi repo.
> >>
> >> Again, this patch reintroduces the bug that was fixed by the "Fixes:" commit.
> >> You can't just say "somebody will add the memset() somewhere later" and ignore
> >> the bug you are introducing. You need to add the memset() *now* along with the
> >> fdt_open_into() call.
> >
> > Yes, but that fix broke U-boot booting.
>
> And it is entirely possible to fix U-boot booting without breaking other users
> (cvw, etc.) in the process. Yes, the original commit exposed a bug in U-Boot.
> Bugs happen; we can fix them or work around them. But needing to fix one bug is
> not license to intentionally introduce more bugs.
You failed to understand my point. You original fix was already
incomplete since instead of packing the FDT it should have done
memset() of uninitialized space in FDT.
>
> > The original fix itself was incomplete. Adding memset() to
> > fdt_open_into() is a separate change. Feel free to send
> > patch for it.
>
> If you want adding memset() to fdt_open_into() to be a separate change, then
> that change must be merged *first*, before this patch, so it is then safe to add
> the call to fdt_open_into(). Because now you have broken bisectability for the
> affected platforms. The uninitialized memory issue was completely fixed by the
> original patch, and OpenSBI could boot on cvw as of that commit; now it is
> broken again, and that is a regression.
The original fix for packing FDT was already broken so don't
blame me for partially reverting.
Regards,
Anup
More information about the opensbi
mailing list