On 9/29/2021 4:50 PM, Alex Williamson wrote:
On Wed, 29 Sep 2021 16:26:55 +0300
Max Gurtovoy <mgurtovoy@xxxxxxxxxx> wrote:
On 9/29/2021 3:35 PM, Alex Williamson wrote:
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,
Why do you need this state machine for userspace that doesn't support
migration ?
For userspace that doesn't support migration, there's one state,
_RUNNING. That's what we're trying to be compatible and consistent
with. Migration is an extension, not a base requirement.
Userspace without migration doesn't care about this state.
We left with kernel now. vfio-pci today doesn't support migration, right
? state is in theory is 0 (STOP).
This state machine is controlled by the migration SW. The drivers don't
move state implicitly.
mlx5-vfio-pci support migration and will work fine with non-migration SW
(it will stay with state = 0 unless someone will move it. but nobody
will) exactly like vfio-pci does today.
So where is the problem ?
What is the definition of RUNNING state for a paused VM that is waiting
for incoming migration blob ?
A VM supporting migration of the device would move the device to
_RESUMING to load the incoming data. If the VM leaves the device in
_RUNNING, then it doesn't support migration of the device and it's out
of scope how it handles that device state. Existing devices continue
running regardless of whether the VM state is paused, it's only devices
supporting migration where userspace could optionally have the device
run state follow the VM run state. Thanks,
Alex
