On Wed, 29 Sep 2021 13:44:10 +0300 Max Gurtovoy <mgurtovoy@xxxxxxxxxx> wrote: > On 9/28/2021 2:12 AM, Jason Gunthorpe wrote: > > On Mon, Sep 27, 2021 at 04:46:48PM -0600, Alex Williamson wrote: > >>> + enum { MAX_STATE = VFIO_DEVICE_STATE_RESUMING }; > >>> + static const u8 vfio_from_state_table[MAX_STATE + 1][MAX_STATE + 1] = { > >>> + [VFIO_DEVICE_STATE_STOP] = { > >>> + [VFIO_DEVICE_STATE_RUNNING] = 1, > >>> + [VFIO_DEVICE_STATE_RESUMING] = 1, > >>> + }, > >> Our state transition diagram is pretty weak on reachable transitions > >> out of the _STOP state, why do we select only these two as valid? > > I have no particular opinion on specific states here, however adding > > more states means more stuff for drivers to implement and more risk > > driver writers will mess up this uAPI. > > _STOP == 000b => Device Stopped, not saving or resuming (from UAPI). > > This is the default initial state and not RUNNING. > > The user application should move device from STOP => RUNNING or STOP => > RESUMING. > > Maybe we need to extend the comment in the UAPI file. include/uapi/linux/vfio.h: ... * +------- _RESUMING * |+------ _SAVING * ||+----- _RUNNING * ||| * 000b => Device Stopped, not saving or resuming * 001b => Device running, which is the default state ^^^^^^^^^^^^^^^^^^^^^^^^^^ ... * State transitions: * * _RESUMING _RUNNING Pre-copy Stop-and-copy _STOP * (100b) (001b) (011b) (010b) (000b) * 0. Running or default state * | ^^^^^^^^^^^^^ ... * 0. Default state of VFIO device is _RUNNING when the user application starts. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ The uAPI is pretty clear here. A default state of _STOP is not compatible with existing devices and userspace that does not support migration. Thanks, Alex