Message-ID: <CA+CK2bBrH6ExCzWMpzHpX6vUFvSW6ca_edR_hzEj84xLw20UXQ@mail.gmail.com>
Date: Sun, 8 Jun 2025 11:08:07 -0400
From: Pasha Tatashin <pasha.tatashin@...een.com>
To: Mike Rapoport <rppt@...nel.org>
Cc: pratyush@...nel.org, jasonmiu@...gle.com, graf@...zon.com, 
	changyuanl@...gle.com, dmatlack@...gle.com, rientjes@...gle.com, 
	corbet@....net, rdunlap@...radead.org, ilpo.jarvinen@...ux.intel.com, 
	kanie@...ux.alibaba.com, ojeda@...nel.org, aliceryhl@...gle.com, 
	masahiroy@...nel.org, akpm@...ux-foundation.org, tj@...nel.org, 
	yoann.congal@...le.fr, mmaurer@...gle.com, roman.gushchin@...ux.dev, 
	chenridong@...wei.com, axboe@...nel.dk, mark.rutland@....com, 
	jannh@...gle.com, vincent.guittot@...aro.org, hannes@...xchg.org, 
	dan.j.williams@...el.com, david@...hat.com, joel.granados@...nel.org, 
	rostedt@...dmis.org, anna.schumaker@...cle.com, song@...nel.org, 
	zhangguopeng@...inos.cn, linux@...ssschuh.net, linux-kernel@...r.kernel.org, 
	linux-doc@...r.kernel.org, linux-mm@...ck.org, gregkh@...uxfoundation.org, 
	tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, 
	dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, rafael@...nel.org, 
	dakr@...nel.org, bartosz.golaszewski@...aro.org, cw00.choi@...sung.com, 
	myungjoo.ham@...sung.com, yesanishhere@...il.com, Jonathan.Cameron@...wei.com, 
	quic_zijuhu@...cinc.com, aleksander.lobakin@...el.com, ira.weiny@...el.com, 
	andriy.shevchenko@...ux.intel.com, leon@...nel.org, lukas@...ner.de, 
	bhelgaas@...gle.com, wagi@...nel.org, djeffery@...hat.com, 
	stuart.w.hayes@...il.com, ptyadav@...zon.de
Subject: Re: [RFC v2 10/16] luo: luo_ioctl: add ioctl interface

On Mon, May 26, 2025 at 4:43 AM Mike Rapoport <rppt@...nel.org> wrote:
>
> On Thu, May 15, 2025 at 06:23:14PM +0000, Pasha Tatashin wrote:
> > Introduce the user-space interface for the Live Update Orchestrator
> > via ioctl commands, enabling external control over the live update
> > process and management of preserved resources.
> >
> > Create a misc character device at /dev/liveupdate. Access
> > to this device requires the CAP_SYS_ADMIN capability.
> >
> > A new UAPI header, <uapi/linux/liveupdate.h>, defines the necessary
> > structures. The magic number is registered in
> > Documentation/userspace-api/ioctl/ioctl-number.rst.
> >
> > Signed-off-by: Pasha Tatashin <pasha.tatashin@...een.com>
>
> ...
>
> > -/**
> > - * enum liveupdate_state - Defines the possible states of the live update
> > - * orchestrator.
> > - * @LIVEUPDATE_STATE_NORMAL:         Default state, no live update in progress.
> > - * @LIVEUPDATE_STATE_PREPARED:       Live update is prepared for reboot; the
> > - *                                   LIVEUPDATE_PREPARE callbacks have completed
> > - *                                   successfully.
> > - *                                   Devices might operate in a limited state
> > - *                                   for example the participating devices might
> > - *                                   not be allowed to unbind, and also the
> > - *                                   setting up of new DMA mappings might be
> > - *                                   disabled in this state.
> > - * @LIVEUPDATE_STATE_FROZEN:         The final reboot event
> > - *                                   (%LIVEUPDATE_FREEZE) has been sent, and the
> > - *                                   system is performing its final state saving
> > - *                                   within the "blackout window". User
> > - *                                   workloads must be suspended. The actual
> > - *                                   reboot (kexec) into the next kernel is
> > - *                                   imminent.
> > - * @LIVEUPDATE_STATE_UPDATED:        The system has rebooted into the next
> > - *                                   kernel via live update the system is now
> > - *                                   running the next kernel, awaiting the
> > - *                                   finish event.
> > - *
> > - * These states track the progress and outcome of a live update operation.
> > - */
> > -enum liveupdate_state  {
> > -     LIVEUPDATE_STATE_NORMAL = 0,
> > -     LIVEUPDATE_STATE_PREPARED = 1,
> > -     LIVEUPDATE_STATE_FROZEN = 2,
> > -     LIVEUPDATE_STATE_UPDATED = 3,
> > -};
> > -
>
> Nit: this seems an unnecessary churn, these definitions can go to
> include/uapi from the start.

True, but we do not have a user api at that moment yet :-)

>
> > diff --git a/include/uapi/linux/liveupdate.h b/include/uapi/linux/liveupdate.h
> > +/**
> > + * struct liveupdate_fd - Holds parameters for preserving and restoring file
> > + * descriptors across live update.
> > + * @fd:    Input for %LIVEUPDATE_IOCTL_FD_PRESERVE: The user-space file
> > + *         descriptor to be preserved.
> > + *         Output for %LIVEUPDATE_IOCTL_FD_RESTORE: The new file descriptor
> > + *         representing the fully restored kernel resource.
> > + * @flags: Unused, reserved for future expansion, must be set to 0.
> > + * @token: Output for %LIVEUPDATE_IOCTL_FD_PRESERVE: An opaque, unique token
> > + *         generated by the kernel representing the successfully preserved
> > + *         resource state.
> > + *         Input for %LIVEUPDATE_IOCTL_FD_RESTORE: The token previously
> > + *         returned by the preserve ioctl for the resource to be restored.
> > + *
> > + * This structure is used as the argument for the %LIVEUPDATE_IOCTL_FD_PRESERVE
> > + * and %LIVEUPDATE_IOCTL_FD_RESTORE ioctls. These ioctls allow specific types
> > + * of file descriptors (for example memfd, kvm, iommufd, and VFIO) to have their
> > + * underlying kernel state preserved across a live update cycle.
> > + *
> > + * To preserve an FD, user space passes this struct to
> > + * %LIVEUPDATE_IOCTL_FD_PRESERVE with the @fd field set. On success, the
> > + * kernel populates the @token field.
> > + *
> > + * After the live update transition, user space passes the struct populated with
> > + * the *same* @token to %LIVEUPDATE_IOCTL_FD_RESTORE. The kernel uses the @token
> > + * to find the preserved state and, on success, populates the @fd field with a
> > + * new file descriptor referring to the fully restored resource.
> > + */
> > +struct liveupdate_fd {
> > +     int             fd;
> > +     __u32           flags;
> > +     __u64           token;
> > +};
>
> Consider using __aligned_u64 here for size-based versioning.

Good suggestion, added.

>
> > +
> > +/* The ioctl type, documented in ioctl-number.rst */
> > +#define LIVEUPDATE_IOCTL_TYPE                0xBA
>
> ...
>
> > +/**
> > + * LIVEUPDATE_IOCTL_EVENT_PREPARE - Initiate preparation phase and trigger state
> > + * saving.
>
> This (and others below) is more a command than an event IMHO. Maybe just
> LIVEUPDATE_IOCTL_PREPARE?

Renamed.

>
> > + * Argument: None.
> > + *
> > + * Initiates the live update preparation phase. This action corresponds to
> > + * the internal %LIVEUPDATE_PREPARE kernel event and can also be triggered
>
> This action is a reason for LIVEUPDATE_PREPARE event, isn't it?
> The same applies to other IOCTL_EVENTS

It is.

>
> > + * by writing '1' to ``/sys/kernel/liveupdate/prepare``. This typically

Oops, this is a leftover from LUO RFCv1, fixed.

> > + * triggers the main state saving process for items marked via the PRESERVE
> > + * ioctls. This occurs *before* the main "blackout window", while user
> > + * applications (e.g., VMs) may still be running. Kernel subsystems
> > + * receiving the %LIVEUPDATE_PREPARE event should serialize necessary state.
> > + * This command does not transfer data.
>
> I'm not sure I follow what this sentence means.

Fixed

Thanks,
Pasha

Powered by blists - more mailing lists