Re: [PATCH RFC] vfio: Move the saving of the config space to the right place in VFIO migration

[Shenming Lu]

On 2020/12/2 18:55, Dr. David Alan Gilbert wrote:
> * Shenming Lu (lushenming@huawei.com) wrote:
>> Hi,
>>
>> After reading everyone's opinions, we have a rough idea for this issue.
>>
>> One key point is whether it is necessary to setup the config space before
>> the device can accept further migration data. I think it is decided by
>> the vendor driver, so we can simply ask the vendor driver about it in
>> .save_setup, which could avoid a lot of unnecessary copies and settings.
>> Once we have known the need, we can iterate the config space (before)
>> along with the device migration data in .save_live_iterate and
>> .save_live_complete_precopy, and if not needed, we can only migrate the
>> config space in .save_state.
>>
>> Another key point is that the interrupt enabling should be after the
>> restoring of the interrupt controller (might not only interrupts).
>> My solution is to add a subflag at the beginning of the config data
>> (right after VFIO_MIG_FLAG_DEV_CONFIG_STATE) to indicate the triggered
>> actions on the dst (such as whether to enable interrupts).
>>
>> Below is it's workflow.
>>
>> On the save path:
>>      In vfio_save_setup():
>>      Ask the vendor driver if it needs the config space setup before it
>>      can accept further migration data.
> 
> You could argue that you could always send an initial copy of the config
> data at the start; and then send new versions at the end anyway.
> But is this just about interrupts?

In the current implementation, the effect of the config space is mainly about
interrupts (SET_IRQS ioctl).

> Is the dst going to interpret the
> received migration data differently depending on the config data?

I also have a little doubt about the dependency... But not sure.

> That
> would seem dangerous if the config data was to change during the
> migration.
> 
> Dave
> 
>>              |
>>      In vfio_save_iterate() (pre-copy):
>>      If *needed*, save the config space which would be setup on the dst
>>      before the migration data, but send with a subflag to instruct not
>>      to (such as) enable interrupts.
>>              |
>>      In vfio_save_complete_precopy() (stop-and-copy, iterable process):
>>      The same as that in vfio_save_iterate().
>>              |
>>      In .save_state (stop-and-copy, non-iterable process):
>>      If *needed*, only send a subflag to instruct to enable interrupts.
>>      If *not needed*, save the config space and setup everything on the dst.
>>
>> Besides the above idea, we might be able to choose to let the vendor driver 
>> do
>> more: qemu just sends and writes the config data (before) along with the 
>> device
>> migration data every time, and it's up to the vendor driver to filter 
>> out/buffer
>> the received data or reorder the settings...
>>
>> Thanks,
>> Shenming
>>