Re: [PATCH v6 1/4] kho: add size parameter to kho_add_subtree()

[Breno Leitao <leitao@debian.org>]

Hello Pratyush,

Apologies for the delayed response — I was on vacation and just got back.

On Wed, Feb 04, 2026 at 02:54:35PM +0100, Pratyush Yadav wrote:
> Hi Breno,
>
> On Tue, Jan 27 2026, Breno Leitao wrote:
>
> > kho_add_subtree() assumes the fdt argument is always an FDT and calls
> > fdt_totalsize() on it in the debugfs code path. This assumption will
> > break if a caller passes arbitrary data instead of an FDT.
> >
> > When CONFIG_KEXEC_HANDOVER_DEBUGFS is enabled, kho_debugfs_fdt_add()
> > calls __kho_debugfs_fdt_add(), which executes:
> >
> >     f->wrapper.size = fdt_totalsize(fdt);
> >
> > Fix this by adding an explicit size parameter to kho_add_subtree() so
> > callers specify the blob size. This allows subtrees to contain
> > arbitrary data formats, not just FDTs. Update all callers:
> >
> >   - memblock.c: use fdt_totalsize(fdt)
> >   - luo_core.c: use fdt_totalsize(fdt_out)
> >   - test_kho.c: use fdt_totalsize()
> >   - kexec_handover.c (root fdt): use fdt_totalsize(kho_out.fdt)
> >
> > Also update kho_in_debugfs_init() to compute sizes using fdt_totalsize()
> > for the root and sub-FDTs it processes, since these are known to be
> > actual FDT blobs.
>
> No, this doesn't seem right. The "fdt" field that kho_in_debugfs_init()
> uses is the same "fdt" field where we put our non-FDT blobs. So I don't
> see how these can be known to be actual FDTs. All this happened to work
> because so far all users were FDT, but I bet it will break if you use
> your blob here. Perhaps give it a try and see if I am understanding this
> correctly?
>
> To be honest, I didn't think of this when I suggested you use the size
> parameter. We have lost the size information, and it is hard for
> kho_in_debugfs_init() to find out since it has no idea what the
> underlying format is.
>
> One option is to have it in the KHO FDT, but I am not sure that is a
> good idea. Adding to ABI for debug feature sounds odd (not that I am
> completely against it, it just feels off).
>
> Another would be to give users a hook to populate the blobs when they
> call kho_retrieve_subtree(), so they can figure out how large the blob
> needs to be. This has another benefit: once we move away from FDT, it
> makes little sense to dump the blob since userspace won't have a way to
> parse it. Even with FDT, userspace still can't parse everything. For
> example, say the FDT has a reference to a struct kho_vmalloc. You'd get
> a pointer to the head of the list, but you would have no way of knowing
> what is inside the vmalloc buffer. This has the downside of not being
> able to show anything if the subsystem never calls
> kho_retrieve_subtree().
>
> Unfortunately I don't have much time this week to dive deeper into this.
> These are only things off the top of my head and I haven't thought too
> deeply, so please don't take them as strong suggestions. It would be
> great if you can think a bit more about the problem and come up with a
> recommendation?
>
> I will try to get back to this series in the next 1-2 weeks and
> hopefully find some way to make progress.
>
> I skimmed the rest of the patches and they all LGTM at a high level.

I went with storing the size in the KHO FDT. The addition is small — a
"blob-size" u64 property per subnode — and I think it's warranted: this is
intrinsic metadata about the blob rather than a purely debug-specific artifact.

It also rounds out the API nicely. I extended kho_retrieve_subtree() with an
optional size_t *size output parameter so callers can retrieve the blob size
without needing to understand the underlying format.

I'll send a new version soon. That said, the patchset has been growing
considerably, so if necessary I can defer reworking this infrastructure to
follow-up patches and get the kexec-metadata patches later.

Thanks for the review,
--breno