Re: [PATCH v26 07/17] vfio: Register SaveVMHandlers for VFIO device

[Kirti Wankhede]

On 9/25/2020 5:23 PM, Cornelia Huck wrote:
> On Wed, 23 Sep 2020 04:54:09 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
>
> > Define flags to be used as delimeter in migration file stream.
> > Added .save_setup and .save_cleanup functions. Mapped & unmapped migration
> > region from these functions at source during saving or pre-copy phase.
> > Set VFIO device state depending on VM's state. During live migration, VM is
> > running when .save_setup is called, _SAVING | _RUNNING state is set for VFIO
> > device. During save-restore, VM is paused, _SAVING state is set for VFIO device.
> >
> > Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> > Reviewed-by: Neo Jia <cjia@nvidia.com>
> > ---
> >   hw/vfio/migration.c  | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   hw/vfio/trace-events |  2 ++
> >   2 files changed, 93 insertions(+)
>
> (...)
>
> > +/*
> > + * Flags used as delimiter:
> > + * 0xffffffff => MSB 32-bit all 1s
> > + * 0xef10     => emulated (virtual) function IO
>
> Where is this value coming from?

Delimiter flags should be unique and this is a magic number that 
represents (e)mulated (f)unction (10) representing IO.

> > + * 0x0000     => 16-bits reserved for flags
> > + */
> > +#define VFIO_MIG_FLAG_END_OF_STATE      (0xffffffffef100001ULL)
> > +#define VFIO_MIG_FLAG_DEV_CONFIG_STATE  (0xffffffffef100002ULL)
> > +#define VFIO_MIG_FLAG_DEV_SETUP_STATE   (0xffffffffef100003ULL)
> > +#define VFIO_MIG_FLAG_DEV_DATA_STATE    (0xffffffffef100004ULL)
>
> I think we need some more documentation what these values mean and how
> they are used. From reading ahead a bit, it seems there is always
> supposed to be a pair of DEV_*_STATE and END_OF_STATE framing some kind
> of data?

Adding comment as below, hope it helps.

/*
 * Flags used as delimiter for VFIO devices should be unique in 
migration stream
 * These flags are composed as:
 * 0xffffffff => MSB 32-bit all 1s
 * 0xef10     => Magic ID, represents emulated (virtual) function IO
 * 0x0000     => 16-bits reserved for flags
 *
 * Flags _DEV_CONFIG_STATE, _DEV_SETUP_STATE and _DEV_DATA_STATE marks 
start of
 * respective states in migration stream.
 * FLAG _END_OF_STATE indicates end of current state, state could be any
 * of above states.
 */

Thanks,
Kirti