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

[Pratyush Yadav <pratyush@kernel.org>]

On Mon, Jun 01 2026, Pasha Tatashin wrote:

> 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.

Hmm, it feels odd, but I don't have any real counter arguments. So let's
keep this as-is.

>
>> 
>> >  
>> >  /**
>> >   * 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.

Yeah, that was my point. We don't support anything other than exact
agreement on formats. But let's keep it this way for now, so we can grow
the struct in a backwards compatible way if needed.

>
> 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

-- 
Regards,
Pratyush Yadav