Re: [PATCH v12 Kernel 1/7] vfio: KABI for migration interface for device state

[Cornelia Huck]

On Sat, 8 Feb 2020 01:12:28 +0530
Kirti Wankhede <address@hidden> wrote:

(...)

Minor wording nits:

> +/*
> + * Structure vfio_device_migration_info is placed at 0th offset of

"...at the 0th offset..."

> + * 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)
> + *      - User application writes this field to inform vendor driver about 
> the

I'd probably add a definitive article before "user application",
"vendor driver", etc. Not sure if it's too much churn.

> + *        device state to be transitioned to.
> + *      - Vendor driver should take necessary actions to change device state.
> + *        On successful transition to given state, vendor driver should 
> return
> + *        success on write(device_state, state) system call. If device state
> + *        transition fails, vendor driver should return error, -EFAULT.
> + *      - On user application side, if device state transition fails, i.e. if
> + *        write(device_state, state) returns error, read device_state again 
> to
> + *        determine the current state of the device from vendor driver.
> + *      - Vendor driver should return previous state of the device unless 
> vendor
> + *        driver has encountered an internal error, in which case vendor 
> driver
> + *        may report the device_state VFIO_DEVICE_STATE_ERROR.
> + *   - User application must use the device reset ioctl in order to recover
> + *     the device from VFIO_DEVICE_STATE_ERROR state. If the device is
> + *     indicated in a valid device state via reading device_state, the user
> + *     application may decide attempt to transition the device to any valid
> + *     state reachable from the current state or terminate itself.
> + *
> + *      device_state consists of 3 bits:
> + *      - If bit 0 set, indicates _RUNNING state. When it's clear, that
> + *     indicates _STOP state. When device is changed to _STOP, driver should
> + *     stop device before write() returns.

"If set, bit 0 indicates _RUNNING state. If unset, it indicates _STOP
state. When the device is changed to _STOP state, the driver should
stop the 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 application to save device's state.

"If set, bit 1 indicates _SAVING state. When it is set, the driver
should start to gather the device state information that will be
provided to the VFIO user application to save the 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.

"If set, bit 2 indicates _RESUMING state. When it is set, the driver
should prepare to resume the device, using the data provided via the
migration region."

?

> + *      Bits 3 - 31 are reserved for future use. In order to preserve them,
> + *   user application should perform read-modify-write operation on this
> + *   field when modifying the specified bits.

"In order to preserve them, the user application should use a
read-modify-write operation on the device_state field when modifying
the state."

?


(...)