Re: [RFC v2 06/16] luo: luo_subsystems: add subsystem registration

[Mike Rapoport]

On Thu, May 15, 2025 at 06:23:10PM +0000, Pasha Tatashin wrote:
> Introduce the framework for kernel subsystems (e.g., KVM, IOMMU, device
> drivers) to register with LUO and participate in the live update process
> via callbacks.

...

> diff --git a/include/linux/liveupdate.h b/include/linux/liveupdate.h
> index c2740da70958..7a130680b5f2 100644
> --- a/include/linux/liveupdate.h
> +++ b/include/linux/liveupdate.h
> @@ -86,6 +86,39 @@ enum liveupdate_state  {
>  	LIVEUPDATE_STATE_UPDATED = 3,
>  };
>  
> +/**
> + * struct liveupdate_subsystem - Represents a subsystem participating in LUO
> + * @prepare:      Optional. Called during LUO prepare phase. Should perform
> + *                preparatory actions and can store a u64 handle/state
> + *                via the 'data' pointer for use in later callbacks.
> + *                Return 0 on success, negative error code on failure.
> + * @freeze:       Optional. Called during LUO freeze event (before actual jump
> + *                to new kernel). Should perform final state saving actions and
> + *                can update the u64 handle/state via the 'data' pointer. Retur:
> + *                0 on success, negative error code on failure.
> + * @cancel:       Optional. Called if the live update process is canceled after
> + *                prepare (or freeze) was called. Receives the u64 data
> + *                set by prepare/freeze. Used for cleanup.
> + * @finish:       Optional. Called after the live update is finished in the new
> + *                kernel.
> + *                Receives the u64 data set by prepare/freeze. Used for cleanup.
> + * @name:         Mandatory. Unique name identifying the subsystem.
> + * @arg:          Add this argument to callback functions.
> + * @list:         List head used internally by LUO. Should not be modified by
> + *                caller after registration.
> + * @private_data: For LUO internal use, cached value of data field.
> + */
> +struct liveupdate_subsystem {
> +	int (*prepare)(void *arg, u64 *data);
> +	int (*freeze)(void *arg, u64 *data);
> +	void (*cancel)(void *arg, u64 data);
> +	void (*finish)(void *arg, u64 data);

What is the intended use of arg in all these?

> +	const char *name;
> +	void *arg;
> +	struct list_head list;
> +	u64 private_data;
> +};

I suggest to split callbacks into, say, liveupdate_ops so we could constify
them.
And then it seems that the data in liveupdate_subsystem can be private to
LUO.

> +
>  #ifdef CONFIG_LIVEUPDATE
>  
>  /* Return true if live update orchestrator is enabled */
> @@ -105,6 +138,10 @@ bool liveupdate_state_updated(void);
>   */
>  bool liveupdate_state_normal(void);
>  
> +int liveupdate_register_subsystem(struct liveupdate_subsystem *h);

int liveupdate_register_subsystem(name, ops, data) ?

> +int liveupdate_unregister_subsystem(struct liveupdate_subsystem *h);
> +int liveupdate_get_subsystem_data(struct liveupdate_subsystem *h, u64 *data);
> +
>  #else /* CONFIG_LIVEUPDATE */

-- 
Sincerely yours,
Mike.