[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 01/13] vfio: KABI for migration interface
From: |
Cornelia Huck |
Subject: |
Re: [Qemu-devel] [PATCH v7 01/13] vfio: KABI for migration interface |
Date: |
Tue, 23 Jul 2019 14:13:57 +0200 |
On Tue, 16 Jul 2019 14:56:32 -0600
Alex Williamson <address@hidden> wrote:
> On Tue, 9 Jul 2019 15:19:08 +0530
> Kirti Wankhede <address@hidden> wrote:
I'm still a bit unsure about the device_state bit handling as well.
> > + * 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.
Does 'device state transition fails' include 'the device state written
was invalid'?
> > + * It consists of 3 bits:
> > + * - If bit 0 set, indicates _RUNNING state. When its reset, that
> > indicates
> > + * _STOPPED state. When device is changed to _STOPPED, driver
> > should stop
> > + * device before write() returns.
So _STOPPED is always !_RUNNING, regardless of which other bits are set?
> > + * - If bit 1 set, indicates _SAVING state.
> > + * - If bit 2 set, indicates _RESUMING state.
> > + * _SAVING and _RESUMING set at the same time is invalid state.
What about _RUNNING | _RESUMING -- does that make sense?
>
> I think in the previous version there was a question of how we handle
> yet-to-be-defined bits. For instance, if we defined a
> SUBTYPE_MIGRATIONv2 with the intention of making it backwards
> compatible with this version, do we declare the undefined bits as
> preserved so that the user should do a read-modify-write operation?
Or can we state that undefined bits are ignored, and may or may not
preserved, so that we can skip the read-modify-write requirement? v1
and v2 can hopefully be distinguished in a different way.
(...)
> > +struct vfio_device_migration_info {
> > + __u32 device_state; /* VFIO device state */
> > +#define VFIO_DEVICE_STATE_RUNNING (1 << 0)
> > +#define VFIO_DEVICE_STATE_SAVING (1 << 1)
> > +#define VFIO_DEVICE_STATE_RESUMING (1 << 2)
> > +#define VFIO_DEVICE_STATE_MASK (VFIO_DEVICE_STATE_RUNNING | \
> > + VFIO_DEVICE_STATE_SAVING | \
> > + VFIO_DEVICE_STATE_RESUMING)
>
> Yes, we have the mask in here now, but no mention above how the user
> should handle undefined bits. Thanks,
>
> Alex
>
> > +#define VFIO_DEVICE_STATE_INVALID (VFIO_DEVICE_STATE_SAVING | \
> > + VFIO_DEVICE_STATE_RESUMING)
As mentioned above, does _RESUMING | _RUNNING make sense?
- [Qemu-devel] [PATCH v7 00/13] Add migration support for VFIO device, Kirti Wankhede, 2019/07/09
- [Qemu-devel] [PATCH v7 01/13] vfio: KABI for migration interface, Kirti Wankhede, 2019/07/09
- [Qemu-devel] [PATCH v7 02/13] vfio: Add function to unmap VFIO region, Kirti Wankhede, 2019/07/09
- [Qemu-devel] [PATCH v7 03/13] vfio: Add vfio_get_object callback to VFIODeviceOps, Kirti Wankhede, 2019/07/09
- [Qemu-devel] [PATCH v7 04/13] vfio: Add save and load functions for VFIO PCI devices, Kirti Wankhede, 2019/07/09
- [Qemu-devel] [PATCH v7 05/13] vfio: Add migration region initialization and finalize function, Kirti Wankhede, 2019/07/09