Re: [PATCH v10 Kernel 1/5] vfio: KABI for migration interface for device state

Yan Zhao
2019-12-17T07:21:09Z

On Tue, Dec 17, 2019 at 02:28:44PM +0800, Kirti Wankhede wrote:
>
>
> On 12/17/2019 4:14 AM, Alex Williamson wrote:
> > On Tue, 17 Dec 2019 01:51:36 +0530
> > Kirti Wankhede <kwankhede@nvidia.com> wrote:
> >
> >> - Defined MIGRATION region type and sub-type.
> >>
> >> - Defined vfio_device_migration_info structure which will be placed at 0th
> >> offset of migration region to get/set VFIO device related information.
> >> Defined members of structure and usage on read/write access.
> >>
> >> - Defined device states and added state transition details in the comment.
> >>
> >> - Added sequence to be followed while saving and resuming VFIO device state
> >>
> >> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> >> Reviewed-by: Neo Jia <cjia@nvidia.com>
> >> ---
> >> include/uapi/linux/vfio.h | 180 ++++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 180 insertions(+)
> >>
> >> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> >> index 9e843a147ead..a0817ba267c1 100644
> >> --- a/include/uapi/linux/vfio.h
> >> +++ b/include/uapi/linux/vfio.h
> >> @@ -305,6 +305,7 @@ struct vfio_region_info_cap_type {
> >> #define VFIO_REGION_TYPE_PCI_VENDOR_MASK (0xffff)
> >> #define VFIO_REGION_TYPE_GFX (1)
> >> #define VFIO_REGION_TYPE_CCW (2)
> >> +#define VFIO_REGION_TYPE_MIGRATION (3)
> >>
> >> /* sub-types for VFIO_REGION_TYPE_PCI_* */
> >>
> >> @@ -379,6 +380,185 @@ struct vfio_region_gfx_edid {
> >> /* sub-types for VFIO_REGION_TYPE_CCW */
> >> #define VFIO_REGION_SUBTYPE_CCW_ASYNC_CMD (1)
> >>
> >> +/* sub-types for VFIO_REGION_TYPE_MIGRATION */
> >> +#define VFIO_REGION_SUBTYPE_MIGRATION (1)
> >> +
> >> +/*
> >> + * Structure vfio_device_migration_info is placed at 0th offset of
> >> + * VFIO_REGION_SUBTYPE_MIGRATION region to get/set VFIO device related migration
> >> + * information. Field accesses from this structure are only supported at their
> >> + * native width and alignment, otherwise the result is undefined and vendor
> >> + * drivers should return an error.
> >> + *
> >> + * device_state: (read/write)
> >> + * To indicate vendor driver the state VFIO device should be transitioned
> >> + * to. If device state transition fails, write on this field return error.
> >> + * It consists of 3 bits:
> >> + * - If bit 0 set, indicates _RUNNING state. When its clear, that indicates
> >
> > s/its/it's/
> >
> >> + * _STOP state. When device is changed to _STOP, driver should stop
> >> + * device before write() returns.
> >> + * - If bit 1 set, indicates _SAVING state. When set, that indicates driver
> >> + * should start gathering device state information which will be provided
> >> + * to VFIO user space application to save device's state.
> >> + * - If bit 2 set, indicates _RESUMING state. When set, that indicates
> >> + * prepare to resume device, data provided through migration region
> >> + * should be used to resume device.
> >> + * Bits 3 - 31 are reserved for future use. User should perform
> >> + * read-modify-write operation on this field.
> >> + *
> >> + * +------- _RESUMING
> >> + * |+------ _SAVING
> >> + * ||+----- _RUNNING
> >> + * |||
> >> + * 000b => Device Stopped, not saving or resuming
> >> + * 001b => Device running state, default state
> >> + * 010b => Stop Device & save device state, stop-and-copy state
> >> + * 011b => Device running and save device state, pre-copy state
> >> + * 100b => Device stopped and device state is resuming
> >> + * 101b => Invalid state
> >
> > Eventually this would be intended for post-copy, if supported by the
> > device, right?
> >
>
> No, as per Yan mentioned in earlier version, _RESUMING + _RUNNING can't
> be used for post-copy. New flag will be required for post-copy.
>
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg658768.html

sorry, I didn't mean _RESUMING + _RUNNING can't be used for post-copy.
I just mean another POSCOPY state needs to be introduced. But I'm not
sure what _RESUMING state is for.
actually, we do nothing in response to _RESUMING state, no matter precopy
or poscopy.
If in your side_RESUMING state means it is allowed to restore device data,
then I think _RESUMING + _RUNNING is a valid state for postcopy.

Thanks
Yan