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

[Kirti Wankhede <kwankhede@nvidia.com>]

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