Re: [PATCH v1 09/13] kho: Update FDT dynamically for subtree addition/removal

[Pasha Tatashin <pasha.tatashin@soleen.com>]

On Fri, Nov 14, 2025 at 11:15 AM Mike Rapoport <rppt@kernel.org> wrote:
>
> On Fri, Nov 14, 2025 at 10:53:54AM -0500, Pasha Tatashin wrote:
> > Currently, sub-FDTs were tracked in a list (kho_out.sub_fdts) and the
> > final FDT is constructed entirely from scratch during kho_finalize().
> >
> > We can maintain the FDT dynamically:
> > 1. Initialize a valid, empty FDT in kho_init().
> > 2. Use fdt_add_subnode and fdt_setprop in kho_add_subtree to
> >    update the FDT immediately when a subsystem registers.
> > 3. Use fdt_del_node in kho_remove_subtree to remove entries.
> >
> > This removes the need for the intermediate sub_fdts list and the
> > reconstruction logic in kho_finalize(). kho_finalize() now
> > only needs to trigger memory map serialization.
> >
> > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> > ---
> >  kernel/liveupdate/kexec_handover.c | 144 ++++++++++++++---------------
> >  1 file changed, 68 insertions(+), 76 deletions(-)
> >
> > diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
> > index 8ab77cb85ca9..822da961d4c9 100644
> > --- a/kernel/liveupdate/kexec_handover.c
> > +++ b/kernel/liveupdate/kexec_handover.c
> > @@ -724,37 +713,67 @@ static void __init kho_reserve_scratch(void)
> >   */
> >  int kho_add_subtree(const char *name, void *fdt)
> >  {
> > -     struct kho_sub_fdt *sub_fdt;
> > +     phys_addr_t phys = virt_to_phys(fdt);
> > +     void *root_fdt = kho_out.fdt;
> > +     int err = -ENOMEM;
> > +     int off, fdt_err;
> >
> > -     sub_fdt = kmalloc(sizeof(*sub_fdt), GFP_KERNEL);
> > -     if (!sub_fdt)
> > -             return -ENOMEM;
> > +     guard(mutex)(&kho_out.lock);
> > +
> > +     fdt_err = fdt_open_into(root_fdt, root_fdt, PAGE_SIZE);
> > +     if (fdt_err < 0)
> > +             return err;
> > -     INIT_LIST_HEAD(&sub_fdt->l);
> > -     sub_fdt->name = name;
> > -     sub_fdt->fdt = fdt;
> > +     off = fdt_add_subnode(root_fdt, 0, name);
>
>         fdt_err = fdt_add_subnode();
>
> and then we don't need off
>
> > +     if (off < 0) {
> > +             if (off == -FDT_ERR_EXISTS)
> > +                     err = -EEXIST;
>
> Is it really -ENOMEM for other FDT_ERR values?

In practice, yes. There are some other errors like format mismatch,
magic values etc, but all of them are internal FDT problems. The only
error that really matters to users is the -ENOMEM one.

Pasha

>
> > +             goto out_pack;
> > +     }
> > +
> > +     err = fdt_setprop(root_fdt, off, PROP_SUB_FDT, &phys, sizeof(phys));
> > +     if (err < 0)
> > +             goto out_pack;
> >
> > -     guard(mutex)(&kho_out.fdts_lock);
> > -     list_add_tail(&sub_fdt->l, &kho_out.sub_fdts);
> >       WARN_ON_ONCE(kho_debugfs_fdt_add(&kho_out.dbg, name, fdt, false));
> >
> > -     return 0;
> > +out_pack:
> > +     fdt_pack(root_fdt);
> > +
> > +     return err;
> >  }
> >  EXPORT_SYMBOL_GPL(kho_add_subtree);
>
> --
> Sincerely yours,
> Mike.