Re: [PATCH v4 04/13] liveupdate: register luo_ser as KHO subtree

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

On 06-01 14:39, Pratyush Yadav wrote:
> On Sat, May 30 2026, Pasha Tatashin wrote:
> 
> > Entirely remove the LUO FDT wrapper since the FDT only carries the
> > compatible string and the pointer to the centralized struct luo_ser.
> > Instead, register the struct luo_ser via the KHO raw subtree
> > API, placing the compatibility string inside the structure itself.
> >
> > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> > ---
> >  include/linux/kho/abi/luo.h  | 57 +++++++++---------------
> >  kernel/liveupdate/luo_core.c | 85 +++++++++++-------------------------
> >  2 files changed, 46 insertions(+), 96 deletions(-)
> >
> > diff --git a/include/linux/kho/abi/luo.h b/include/linux/kho/abi/luo.h
> > index 1b2f865a771a..9a4fe491812b 100644
> > --- a/include/linux/kho/abi/luo.h
> > +++ b/include/linux/kho/abi/luo.h
> > @@ -10,11 +10,11 @@
> >   *
> >   * Live Update Orchestrator uses the stable Application Binary Interface
> >   * defined below to pass state from a pre-update kernel to a post-update
> > - * kernel. The ABI is built upon the Kexec HandOver framework and uses a
> > - * Flattened Device Tree to describe the preserved data.
> > + * kernel. The ABI is built upon the Kexec HandOver framework and registers
> > + * the central `struct luo_ser` via the KHO raw subtree API.
> >   *
> > - * This interface is a contract. Any modification to the FDT structure, node
> > - * properties, compatible strings, or the layout of the `__packed` serialization
> > + * This interface is a contract. Any modification to the structure fields,
> > + * compatible strings, or the layout of the `__packed` serialization
> >   * structures defined here constitutes a breaking change. Such changes require
> >   * incrementing the version number in the relevant `_COMPATIBLE` string to
> >   * prevent a new kernel from misinterpreting data from an old kernel.
> > @@ -23,31 +23,15 @@
> >   * however, backward/forward compatibility is only guaranteed for kernels
> >   * supporting the same ABI version.
> >   *
> > - * FDT Structure Overview:
> > + * KHO Structure Overview:
> >   *   The entire LUO state is encapsulated within a single KHO entry named "LUO".
> > - *   This entry contains an FDT with the following layout:
> > - *
> > - *   .. code-block:: none
> > - *
> > - *     / {
> > - *         compatible = "luo-v2";
> > - *         luo-abi-header = <phys_addr_of_luo_ser>;
> > - *     };
> > - *
> > - * Main LUO Node (/):
> > - *
> > - *   - compatible: "luo-v2"
> > - *     Identifies the overall LUO ABI version.
> > - *   - luo-abi-header: u64
> > - *     The physical address of `struct luo_ser`.
> > + *   This entry contains the `struct luo_ser` structure.
> >   *
> >   * Serialization Structures:
> > - *   The FDT properties point to memory regions containing arrays of simple,
> > - *   `__packed` structures. These structures contain the actual preserved state.
> > - *
> >   *   - struct luo_ser:
> >   *     The central ABI structure that contains the overall state of the LUO.
> > - *     It includes the liveupdate-number and pointers to sessions and FLBs.
> > + *     It includes the compatibility string, the liveupdate-number, and pointers
> > + *     to sessions and FLBs.
> >   *
> >   *   - struct luo_session_header_ser:
> >   *     Header for the session array. Contains the total page count of the
> > @@ -78,26 +62,27 @@
> >  #ifndef _LINUX_KHO_ABI_LUO_H
> >  #define _LINUX_KHO_ABI_LUO_H
> >  
> > +#include <linux/align.h>
> >  #include <uapi/linux/liveupdate.h>
> >  
> >  /*
> > - * The LUO FDT hooks all LUO state for sessions, fds, etc.
> > + * The LUO state is registered under this KHO entry name.
> >   */
> > -#define LUO_FDT_SIZE		PAGE_SIZE
> > -#define LUO_FDT_KHO_ENTRY_NAME	"LUO"
> > -#define LUO_FDT_COMPATIBLE	"luo-v2"
> > -#define LUO_FDT_ABI_HEADER	"luo-abi-header"
> > +#define LUO_KHO_ENTRY_NAME	"LUO"
> > +#define LUO_ABI_COMPATIBLE	"luo-v3"
> > +#define LUO_ABI_COMPAT_LEN	ALIGN(sizeof(LUO_ABI_COMPATIBLE), 8)
> 
> The length of the compatible field will change depending on the length
> of the string. While that is technically fine since a new ABI version is
> allowed to change the layout, it feels odd. I think it would be better
> if we define a static size here, say 64 bytes. This way you can avoid
> all the weirdness that can happen when you move from one version to
> another.

This is what I used initially, but we have cases where one LUO/KHO 
subsystem depends on another. For example, the LUO version must change 
when the block version changes, making the static length too 
restrictive. I would prefer to use proper strncmp() everywhere and allow 
the version string to change dynamically between kernels, while still 
allowing something like this (from [PATCH v4 09/13] liveupdate: Remove 
limit on the number of sessions):

#define LUO_COMPAT_BASE		"luo-v3"
#define LUO_ABI_COMPATIBLE	LUO_COMPAT_BASE "-" 
KHO_BLOCK_ABI_COMPATIBLE

In the future, we may extend this further as we add more dependencies, 
such as your preservable xarray, vmalloc, etc. Everything that depends 
on an external version should include that in its compatibility string.

> 
> >  
> >  /**
> >   * struct luo_ser - Centralized LUO ABI header.
> > + * @compatible:     Compatibility string identifying the LUO ABI version.
> >   * @liveupdate_num: A counter tracking the number of successful live updates.
> >   * @sessions_pa:    Physical address of the first session block header.
> >   * @flbs_pa:        Physical address of the FLB header.
> >   *
> > - * This structure is the root of all preserved LUO state. It is pointed to by
> > - * the "luo-abi-header" property in the LUO FDT.
> > + * This structure is the root of all preserved LUO state.
> >   */
> >  struct luo_ser {
> > +	char compatible[LUO_ABI_COMPAT_LEN];
> >  	u64 liveupdate_num;
> >  	u64 sessions_pa;
> >  	u64 flbs_pa;
> [...]
> > @@ -94,40 +91,29 @@ static int __init luo_early_startup(void)
> >  		return 0;
> >  	}
> >  
> > -	/* Retrieve LUO subtree, and verify its format. */
> > -	err = kho_retrieve_subtree(LUO_FDT_KHO_ENTRY_NAME, &fdt_phys, NULL);
> > +	/* Retrieve LUO state from KHO. */
> > +	err = kho_retrieve_subtree(LUO_KHO_ENTRY_NAME, &luo_ser_phys, &len);
> >  	if (err) {
> >  		if (err != -ENOENT) {
> > -			pr_err("failed to retrieve FDT '%s' from KHO: %pe\n",
> > -			       LUO_FDT_KHO_ENTRY_NAME, ERR_PTR(err));
> > +			pr_err("failed to retrieve LUO state '%s' from KHO: %pe\n",
> > +			       LUO_KHO_ENTRY_NAME, ERR_PTR(err));
> >  			return err;
> >  		}
> >  
> >  		return 0;
> >  	}
> >  
> > -	luo_global.fdt_in = phys_to_virt(fdt_phys);
> > -	err = fdt_node_check_compatible(luo_global.fdt_in, 0,
> > -					LUO_FDT_COMPATIBLE);
> > -	if (err) {
> > -		pr_err("FDT '%s' is incompatible with '%s' [%d]\n",
> > -		       LUO_FDT_KHO_ENTRY_NAME, LUO_FDT_COMPATIBLE, err);
> > -
> > +	if (len < sizeof(*luo_ser)) {
> 
> len != sizeof(*luo_ser) here?

I can change this, but it is not necessary. It is common practice to 
verify that a "struct" is not smaller when compatibility is checked, 
allowing for future expansion without breaking compatibility with older 
kernels. I know we do not support forward/backward compatibility in any 
way right now, but I do not think it hurts to put the proper safeguards 
in place.

Pasha

> 
> > +		pr_err("LUO state is too small (%zu < %zu)\n", len, sizeof(*luo_ser));
> >  		return -EINVAL;
> >  	}
> >  
> > -	header_size = 0;
> > -	ptr = fdt_getprop(luo_global.fdt_in, 0, LUO_FDT_ABI_HEADER, &header_size);
> > -	if (!ptr || header_size != sizeof(u64)) {
> > -		pr_err("Unable to get ABI header '%s' [%d]\n",
> > -		       LUO_FDT_ABI_HEADER, header_size);
> > -
> > +	luo_ser = phys_to_virt(luo_ser_phys);
> > +	if (strncmp(luo_ser->compatible, LUO_ABI_COMPATIBLE, LUO_ABI_COMPAT_LEN)) {
> > +		pr_err("LUO state is incompatible with '%s'\n", LUO_ABI_COMPATIBLE);
> >  		return -EINVAL;
> >  	}
> >  
> > -	luo_ser_pa = get_unaligned((u64 *)ptr);
> > -	luo_ser = phys_to_virt(luo_ser_pa);
> > -
> >  	luo_global.liveupdate_num = luo_ser->liveupdate_num;
> >  	pr_info("Retrieved live update data, liveupdate number: %lld\n",
> >  		luo_global.liveupdate_num);
> [...]
> 
> -- 
> Regards,
> Pratyush Yadav