netdev - Re: [PATCH V8 mlx5-next 09/15] vfio: Define device migration protocol v2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 23 Feb 2022 18:06:13 +0100
From:   Cornelia Huck <cohuck@...hat.com>
To:     Yishai Hadas <yishaih@...dia.com>, alex.williamson@...hat.com,
        bhelgaas@...gle.com, jgg@...dia.com, saeedm@...dia.com
Cc:     linux-pci@...r.kernel.org, kvm@...r.kernel.org,
        netdev@...r.kernel.org, kuba@...nel.org, leonro@...dia.com,
        kwankhede@...dia.com, mgurtovoy@...dia.com, yishaih@...dia.com,
        maorg@...dia.com, ashok.raj@...el.com, kevin.tian@...el.com,
        shameerali.kolothum.thodi@...wei.com
Subject: Re: [PATCH V8 mlx5-next 09/15] vfio: Define device migration
 protocol v2

On Sun, Feb 20 2022, Yishai Hadas <yishaih@...dia.com> wrote:

> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index ca69516f869d..3bbadcdbc9c8 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -56,6 +56,14 @@ struct vfio_device {
>   *         match, -errno for abort (ex. match with insufficient or incorrect
>   *         additional args)
>   * @device_feature: Fill in the VFIO_DEVICE_FEATURE ioctl
> + * @migration_set_state: Optional callback to change the migration state for
> + *         devices that support migration. The returned FD is used for data
> + *         transfer according to the FSM definition. The driver is responsible
> + *         to ensure that FD reaches end of stream or error whenever the
> + *         migration FSM leaves a data transfer state or before close_device()
> + *         returns.
> + * @migration_get_state: Optional callback to get the migration state for
> + *         devices that support migration.

Nit: I'd add "mandatory for VFIO_DEVICE_FEATURE_MIGRATION migration
support" to both descriptions to be a bit more explicit.

(...)

> +/*
> + * Indicates the device can support the migration API through
> + * VFIO_DEVICE_FEATURE_MIG_DEVICE_STATE. If present flags must be non-zero and
> + * VFIO_DEVICE_FEATURE_MIG_DEVICE_STATE is supported. The RUNNING and

I'm having trouble parsing this. I think what it tries to say is that at
least one of the flags defined below must be set?

> + * ERROR states are always supported if this GET succeeds.

What about the following instead:

"Indicates device support for the migration API through
VFIO_DEVICE_FEATURE_MIG_DEVICE_STATE. If present, the RUNNING and ERROR
states are always supported. Support for additional states is indicated
via the flags field; at least one of the flags defined below must be
set."

> + *
> + * VFIO_MIGRATION_STOP_COPY means that STOP, STOP_COPY and
> + * RESUMING are supported.
> + */
> +struct vfio_device_feature_migration {
> +	__aligned_u64 flags;
> +#define VFIO_MIGRATION_STOP_COPY	(1 << 0)
> +};