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>
Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
---
kernel/liveupdate/kexec_handover.c | 144 ++++++++++++++---------------
1 file changed, 69 insertions(+), 75 deletions(-)
diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
index 624fd648d21f..461d96084c12 100644
--- a/kernel/liveupdate/kexec_handover.c
+++ b/kernel/liveupdate/kexec_handover.c
@@ -104,20 +104,11 @@ struct kho_mem_track {
struct khoser_mem_chunk;
-struct kho_sub_fdt {
- struct list_head l;
- const char *name;
- void *fdt;
-};
-
struct kho_out {
void *fdt;
bool finalized;
struct mutex lock; /* protects KHO FDT finalization */
- struct list_head sub_fdts;
- struct mutex fdts_lock;
-
struct kho_mem_track track;
struct kho_debugfs dbg;
};
@@ -127,8 +118,6 @@ static struct kho_out kho_out = {
.track = {
.orders = XARRAY_INIT(kho_out.track.orders, 0),
},
- .sub_fdts = LIST_HEAD_INIT(kho_out.sub_fdts),
- .fdts_lock = __MUTEX_INITIALIZER(kho_out.fdts_lock),
.finalized = false,
};
@@ -725,37 +714,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);
+ if (off < 0) {
+ if (off == -FDT_ERR_EXISTS)
+ err = -EEXIST;
+ 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);
void kho_remove_subtree(void *fdt)
{
- struct kho_sub_fdt *sub_fdt;
+ phys_addr_t target_phys = virt_to_phys(fdt);
+ void *root_fdt = kho_out.fdt;
+ int off;
+ int err;
+
+ guard(mutex)(&kho_out.lock);
+
+ err = fdt_open_into(root_fdt, root_fdt, PAGE_SIZE);
+ if (err < 0)
+ return;
+
+ for (off = fdt_first_subnode(root_fdt, 0); off >= 0;
+ off = fdt_next_subnode(root_fdt, off)) {
+ const u64 *val;
+ int len;
+
+ val = fdt_getprop(root_fdt, off, PROP_SUB_FDT, &len);
+ if (!val || len != sizeof(phys_addr_t))
+ continue;
- guard(mutex)(&kho_out.fdts_lock);
- list_for_each_entry(sub_fdt, &kho_out.sub_fdts, l) {
- if (sub_fdt->fdt == fdt) {
- list_del(&sub_fdt->l);
- kfree(sub_fdt);
+ if ((phys_addr_t)*val == target_phys) {
+ fdt_del_node(root_fdt, off);
kho_debugfs_fdt_remove(&kho_out.dbg, fdt);
break;
}
}
+
+ fdt_pack(root_fdt);
}
EXPORT_SYMBOL_GPL(kho_remove_subtree);
@@ -1232,48 +1251,6 @@ void kho_restore_free(void *mem)
}
EXPORT_SYMBOL_GPL(kho_restore_free);
-static int __kho_finalize(void)
-{
- void *root = kho_out.fdt;
- struct kho_sub_fdt *fdt;
- u64 empty_mem_map = 0;
- int err;
-
- err = fdt_create(root, PAGE_SIZE);
- err |= fdt_finish_reservemap(root);
- err |= fdt_begin_node(root, "");
- err |= fdt_property_string(root, "compatible", KHO_FDT_COMPATIBLE);
- err |= fdt_property(root, PROP_PRESERVED_MEMORY_MAP, &empty_mem_map,
- sizeof(empty_mem_map));
- if (err)
- goto err_exit;
-
- mutex_lock(&kho_out.fdts_lock);
- list_for_each_entry(fdt, &kho_out.sub_fdts, l) {
- phys_addr_t phys = virt_to_phys(fdt->fdt);
-
- err |= fdt_begin_node(root, fdt->name);
- err |= fdt_property(root, PROP_SUB_FDT, &phys, sizeof(phys));
- err |= fdt_end_node(root);
- }
- mutex_unlock(&kho_out.fdts_lock);
-
- err |= fdt_end_node(root);
- err |= fdt_finish(root);
- if (err)
- goto err_exit;
-
- err = kho_mem_serialize(&kho_out);
- if (err)
- goto err_exit;
-
- return 0;
-
-err_exit:
- pr_err("Failed to convert KHO state tree: %d\n", err);
- return err;
-}
-
int kho_finalize(void)
{
int ret;
@@ -1282,12 +1259,7 @@ int kho_finalize(void)
return -EOPNOTSUPP;
guard(mutex)(&kho_out.lock);
- if (kho_out.finalized) {
- kho_update_memory_map(NULL);
- kho_out.finalized = false;
- }
-
- ret = __kho_finalize();
+ ret = kho_mem_serialize(&kho_out);
if (ret)
return ret;
@@ -1372,6 +1344,24 @@ int kho_retrieve_subtree(const char *name, phys_addr_t *phys)
}
EXPORT_SYMBOL_GPL(kho_retrieve_subtree);
+static __init int kho_out_fdt_setup(void)
+{
+ void *root = kho_out.fdt;
+ u64 empty_mem_map = 0;
+ int err;
+
+ err = fdt_create(root, PAGE_SIZE);
+ err |= fdt_finish_reservemap(root);
+ err |= fdt_begin_node(root, "");
+ err |= fdt_property_string(root, "compatible", KHO_FDT_COMPATIBLE);
+ err |= fdt_property(root, PROP_PRESERVED_MEMORY_MAP, &empty_mem_map,
+ sizeof(empty_mem_map));
+ err |= fdt_end_node(root);
+ err |= fdt_finish(root);
+
+ return err;
+}
+
static __init int kho_init(void)
{
const void *fdt = kho_get_fdt();
@@ -1394,6 +1384,10 @@ static __init int kho_init(void)
if (err)
goto err_free_fdt;
+ err = kho_out_fdt_setup();
+ if (err)
+ goto err_free_fdt;
+
if (fdt) {
kho_in_debugfs_init(&kho_in.dbg, fdt);
return 0;
--
2.52.0.rc1.455.g30608eb744-googOn Fri, Nov 14, 2025 at 01:59:59PM -0500, Pasha Tatashin wrote:
> - 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);
Why not
fdt_err = fdt_add_subnode()
as I asked in v1 review?
> + if (off < 0) {
> + if (off == -FDT_ERR_EXISTS)
> + err = -EEXIST;
> + goto out_pack;
> + }
--
Sincerely yours,
Mike.
On Sat, Nov 15, 2025 at 4:40 AM Mike Rapoport <rppt@kernel.org> wrote:
>
> On Fri, Nov 14, 2025 at 01:59:59PM -0500, Pasha Tatashin wrote:
> > - 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);
>
> Why not
> fdt_err = fdt_add_subnode()
>
> as I asked in v1 review?
>
Oh, I missed that, there is a slight difference between the two:
'fdt_err' only contains FDT return value, i.e. error if negative. The
'off' on the other hand in the happy path contains subnode offset, and
contains error only in the unhappy path. This is why I think it is a
little cleaner to keep different name, however, if you still prefer
re-using a single local variable for both, this is fix-up patch:
diff --git a/kernel/liveupdate/kexec_handover.c
b/kernel/liveupdate/kexec_handover.c
index 224bdf5becb6..81f60ccb2dc7 100644
--- a/kernel/liveupdate/kexec_handover.c
+++ b/kernel/liveupdate/kexec_handover.c
@@ -713,7 +713,7 @@ int kho_add_subtree(const char *name, void *fdt)
phys_addr_t phys = virt_to_phys(fdt);
void *root_fdt = kho_out.fdt;
int err = -ENOMEM;
- int off, fdt_err;
+ int fdt_err;
guard(mutex)(&kho_out.lock);
@@ -721,14 +721,14 @@ int kho_add_subtree(const char *name, void *fdt)
if (fdt_err < 0)
return err;
- off = fdt_add_subnode(root_fdt, 0, name);
- if (off < 0) {
- if (off == -FDT_ERR_EXISTS)
+ fdt_err = fdt_add_subnode(root_fdt, 0, name);
+ if (fdt_err < 0) {
+ if (fdt_err == -FDT_ERR_EXISTS)
err = -EEXIST;
goto out_pack;
}
- err = fdt_setprop(root_fdt, off, PROP_SUB_FDT, &phys, sizeof(phys));
+ err = fdt_setprop(root_fdt, fdt_err, PROP_SUB_FDT, &phys, sizeof(phys));
if (err < 0)
goto out_pack;
On Sat, Nov 15, 2025 at 09:51:07AM -0500, Pasha Tatashin wrote:
> On Sat, Nov 15, 2025 at 4:40 AM Mike Rapoport <rppt@kernel.org> wrote:
> >
> > On Fri, Nov 14, 2025 at 01:59:59PM -0500, Pasha Tatashin wrote:
> > > - 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);
> >
> > Why not
> > fdt_err = fdt_add_subnode()
> >
> > as I asked in v1 review?
> >
>
> Oh, I missed that, there is a slight difference between the two:
> 'fdt_err' only contains FDT return value, i.e. error if negative. The
> 'off' on the other hand in the happy path contains subnode offset, and
> contains error only in the unhappy path. This is why I think it is a
> little cleaner to keep different name, however, if you still prefer
> re-using a single local variable for both, this is fix-up patch:
>
> diff --git a/kernel/liveupdate/kexec_handover.c
> b/kernel/liveupdate/kexec_handover.c
> index 224bdf5becb6..81f60ccb2dc7 100644
> --- a/kernel/liveupdate/kexec_handover.c
> +++ b/kernel/liveupdate/kexec_handover.c
> @@ -713,7 +713,7 @@ int kho_add_subtree(const char *name, void *fdt)
> phys_addr_t phys = virt_to_phys(fdt);
> void *root_fdt = kho_out.fdt;
> int err = -ENOMEM;
> - int off, fdt_err;
> + int fdt_err;
>
> guard(mutex)(&kho_out.lock);
>
> @@ -721,14 +721,14 @@ int kho_add_subtree(const char *name, void *fdt)
> if (fdt_err < 0)
> return err;
>
> - off = fdt_add_subnode(root_fdt, 0, name);
> - if (off < 0) {
> - if (off == -FDT_ERR_EXISTS)
> + fdt_err = fdt_add_subnode(root_fdt, 0, name);
> + if (fdt_err < 0) {
> + if (fdt_err == -FDT_ERR_EXISTS)
> err = -EEXIST;
> goto out_pack;
> }
>
> - err = fdt_setprop(root_fdt, off, PROP_SUB_FDT, &phys, sizeof(phys));
> + err = fdt_setprop(root_fdt, fdt_err, PROP_SUB_FDT, &phys, sizeof(phys));
I missed 'off' here, never mind
> if (err < 0)
> goto out_pack;
>
--
Sincerely yours,
Mike.
© 2016 - 2026 Red Hat, Inc.