| Message ID | 1603704987-20977-6-git-send-email-kwankhede@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add migration support for VFIO devices | expand |
On Mon, 26 Oct 2020 15:06:15 +0530 Kirti Wankhede <kwankhede@nvidia.com> wrote: > VM state change handler is called on change in VM's state. Based on > VM state, VFIO device state should be changed. > Added read/write helper functions for migration region. > Added function to set device_state. > > Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com> > Reviewed-by: Neo Jia <cjia@nvidia.com> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > --- > hw/vfio/migration.c | 158 ++++++++++++++++++++++++++++++++++++++++++ > hw/vfio/trace-events | 2 + > include/hw/vfio/vfio-common.h | 4 ++ > 3 files changed, 164 insertions(+) > > diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c > index fd7faf423cdc..65ce735d667b 100644 > --- a/hw/vfio/migration.c > +++ b/hw/vfio/migration.c [snip] > @@ -64,6 +216,9 @@ static int vfio_migration_init(VFIODevice *vbasedev, > ret = -EINVAL; > goto err; > } > + > + migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change, > + vbasedev); > return 0; > > err: Fails to build, @migration is not defined. We could use vbasedev->migration or pull defining and setting @migration from patch 06. Thanks, Alex
On 10/26/2020 6:30 PM, Alex Williamson wrote: > On Mon, 26 Oct 2020 15:06:15 +0530 > Kirti Wankhede <kwankhede@nvidia.com> wrote: > >> VM state change handler is called on change in VM's state. Based on >> VM state, VFIO device state should be changed. >> Added read/write helper functions for migration region. >> Added function to set device_state. >> >> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com> >> Reviewed-by: Neo Jia <cjia@nvidia.com> >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >> --- >> hw/vfio/migration.c | 158 ++++++++++++++++++++++++++++++++++++++++++ >> hw/vfio/trace-events | 2 + >> include/hw/vfio/vfio-common.h | 4 ++ >> 3 files changed, 164 insertions(+) >> >> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c >> index fd7faf423cdc..65ce735d667b 100644 >> --- a/hw/vfio/migration.c >> +++ b/hw/vfio/migration.c > [snip] >> @@ -64,6 +216,9 @@ static int vfio_migration_init(VFIODevice *vbasedev, >> ret = -EINVAL; >> goto err; >> } >> + >> + migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change, >> + vbasedev); >> return 0; >> >> err: > > Fails to build, @migration is not defined. We could use > vbasedev->migration or pull defining and setting @migration from patch > 06. Thanks, > Pulling and setting migration from patch 06 seems better option. Should I resend patch 5 & 6 only? Thanks, Kirti
On Mon, 26 Oct 2020 19:18:51 +0530 Kirti Wankhede <kwankhede@nvidia.com> wrote: > On 10/26/2020 6:30 PM, Alex Williamson wrote: > > On Mon, 26 Oct 2020 15:06:15 +0530 > > Kirti Wankhede <kwankhede@nvidia.com> wrote: > > > >> VM state change handler is called on change in VM's state. Based on > >> VM state, VFIO device state should be changed. > >> Added read/write helper functions for migration region. > >> Added function to set device_state. > >> > >> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com> > >> Reviewed-by: Neo Jia <cjia@nvidia.com> > >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > >> Reviewed-by: Cornelia Huck <cohuck@redhat.com> > >> --- > >> hw/vfio/migration.c | 158 ++++++++++++++++++++++++++++++++++++++++++ > >> hw/vfio/trace-events | 2 + > >> include/hw/vfio/vfio-common.h | 4 ++ > >> 3 files changed, 164 insertions(+) > >> > >> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c > >> index fd7faf423cdc..65ce735d667b 100644 > >> --- a/hw/vfio/migration.c > >> +++ b/hw/vfio/migration.c > > [snip] > >> @@ -64,6 +216,9 @@ static int vfio_migration_init(VFIODevice *vbasedev, > >> ret = -EINVAL; > >> goto err; > >> } > >> + > >> + migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change, > >> + vbasedev); > >> return 0; > >> > >> err: > > > > Fails to build, @migration is not defined. We could use > > vbasedev->migration or pull defining and setting @migration from patch > > 06. Thanks, > > > > Pulling and setting migration from patch 06 seems better option. > Should I resend patch 5 & 6 only? I've resolved this locally as patch 05: @@ -38,6 +190,7 @@ static int vfio_migration_init(VFIODevice *vbasedev, { int ret; Object *obj; + VFIOMigration *migration; if (!vbasedev->ops->vfio_get_object) { return -EINVAL; @@ -64,6 +217,10 @@ static int vfio_migration_init(VFIODevice *vbasedev, ret = -EINVAL; goto err; } + + migration = vbasedev->migration; + migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change, + vbasedev); return 0; err: patch 06: @@ -219,8 +243,11 @@ static int vfio_migration_init(VFIODevice *vbasedev, } migration = vbasedev->migration; + migration->vbasedev = vbasedev; migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change, vbasedev); + migration->migration_state.notify = vfio_migration_state_notifier; + add_migration_state_change_notifier(&migration->migration_state); return 0; err: If you're satisfied with that, no need to resend. Thanks, Alex
On 10/26/2020 7:32 PM, Alex Williamson wrote: > On Mon, 26 Oct 2020 19:18:51 +0530 > Kirti Wankhede <kwankhede@nvidia.com> wrote: > >> On 10/26/2020 6:30 PM, Alex Williamson wrote: >>> On Mon, 26 Oct 2020 15:06:15 +0530 >>> Kirti Wankhede <kwankhede@nvidia.com> wrote: >>> >>>> VM state change handler is called on change in VM's state. Based on >>>> VM state, VFIO device state should be changed. >>>> Added read/write helper functions for migration region. >>>> Added function to set device_state. >>>> >>>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com> >>>> Reviewed-by: Neo Jia <cjia@nvidia.com> >>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >>>> --- >>>> hw/vfio/migration.c | 158 ++++++++++++++++++++++++++++++++++++++++++ >>>> hw/vfio/trace-events | 2 + >>>> include/hw/vfio/vfio-common.h | 4 ++ >>>> 3 files changed, 164 insertions(+) >>>> >>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c >>>> index fd7faf423cdc..65ce735d667b 100644 >>>> --- a/hw/vfio/migration.c >>>> +++ b/hw/vfio/migration.c >>> [snip] >>>> @@ -64,6 +216,9 @@ static int vfio_migration_init(VFIODevice *vbasedev, >>>> ret = -EINVAL; >>>> goto err; >>>> } >>>> + >>>> + migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change, >>>> + vbasedev); >>>> return 0; >>>> >>>> err: >>> >>> Fails to build, @migration is not defined. We could use >>> vbasedev->migration or pull defining and setting @migration from patch >>> 06. Thanks, >>> >> >> Pulling and setting migration from patch 06 seems better option. >> Should I resend patch 5 & 6 only? > > I've resolved this locally as patch 05: > > @@ -38,6 +190,7 @@ static int vfio_migration_init(VFIODevice *vbasedev, > { > int ret; > Object *obj; > + VFIOMigration *migration; > > if (!vbasedev->ops->vfio_get_object) { > return -EINVAL; > @@ -64,6 +217,10 @@ static int vfio_migration_init(VFIODevice *vbasedev, > ret = -EINVAL; > goto err; > } > + > + migration = vbasedev->migration; > + migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change, > + vbasedev); > return 0; > > err: > > patch 06: > > @@ -219,8 +243,11 @@ static int vfio_migration_init(VFIODevice *vbasedev, > } > > migration = vbasedev->migration; > + migration->vbasedev = vbasedev; > migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change, > vbasedev); > + migration->migration_state.notify = vfio_migration_state_notifier; > + add_migration_state_change_notifier(&migration->migration_state); > return 0; > > err: > > If you're satisfied with that, no need to resend. Thanks, > Yes, this is exactly I was going to send. Thanks for fixing it. Thanks, Kirti
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index fd7faf423cdc..65ce735d667b 100644 --- a/hw/vfio/migration.c +++ b/hw/vfio/migration.c @@ -10,6 +10,7 @@ #include "qemu/osdep.h" #include <linux/vfio.h> +#include "sysemu/runstate.h" #include "hw/vfio/vfio-common.h" #include "cpu.h" #include "migration/migration.h" @@ -22,6 +23,157 @@ #include "exec/ram_addr.h" #include "pci.h" #include "trace.h" +#include "hw/hw.h" + +static inline int vfio_mig_access(VFIODevice *vbasedev, void *val, int count, + off_t off, bool iswrite) +{ + int ret; + + ret = iswrite ? pwrite(vbasedev->fd, val, count, off) : + pread(vbasedev->fd, val, count, off); + if (ret < count) { + error_report("vfio_mig_%s %d byte %s: failed at offset 0x%lx, err: %s", + iswrite ? "write" : "read", count, + vbasedev->name, off, strerror(errno)); + return (ret < 0) ? ret : -EINVAL; + } + return 0; +} + +static int vfio_mig_rw(VFIODevice *vbasedev, __u8 *buf, size_t count, + off_t off, bool iswrite) +{ + int ret, done = 0; + __u8 *tbuf = buf; + + while (count) { + int bytes = 0; + + if (count >= 8 && !(off % 8)) { + bytes = 8; + } else if (count >= 4 && !(off % 4)) { + bytes = 4; + } else if (count >= 2 && !(off % 2)) { + bytes = 2; + } else { + bytes = 1; + } + + ret = vfio_mig_access(vbasedev, tbuf, bytes, off, iswrite); + if (ret) { + return ret; + } + + count -= bytes; + done += bytes; + off += bytes; + tbuf += bytes; + } + return done; +} + +#define vfio_mig_read(f, v, c, o) vfio_mig_rw(f, (__u8 *)v, c, o, false) +#define vfio_mig_write(f, v, c, o) vfio_mig_rw(f, (__u8 *)v, c, o, true) + +#define VFIO_MIG_STRUCT_OFFSET(f) \ + offsetof(struct vfio_device_migration_info, f) +/* + * Change the device_state register for device @vbasedev. Bits set in @mask + * are preserved, bits set in @value are set, and bits not set in either @mask + * or @value are cleared in device_state. If the register cannot be accessed, + * the resulting state would be invalid, or the device enters an error state, + * an error is returned. + */ + +static int vfio_migration_set_state(VFIODevice *vbasedev, uint32_t mask, + uint32_t value) +{ + VFIOMigration *migration = vbasedev->migration; + VFIORegion *region = &migration->region; + off_t dev_state_off = region->fd_offset + + VFIO_MIG_STRUCT_OFFSET(device_state); + uint32_t device_state; + int ret; + + ret = vfio_mig_read(vbasedev, &device_state, sizeof(device_state), + dev_state_off); + if (ret < 0) { + return ret; + } + + device_state = (device_state & mask) | value; + + if (!VFIO_DEVICE_STATE_VALID(device_state)) { + return -EINVAL; + } + + ret = vfio_mig_write(vbasedev, &device_state, sizeof(device_state), + dev_state_off); + if (ret < 0) { + int rret; + + rret = vfio_mig_read(vbasedev, &device_state, sizeof(device_state), + dev_state_off); + + if ((rret < 0) || (VFIO_DEVICE_STATE_IS_ERROR(device_state))) { + hw_error("%s: Device in error state 0x%x", vbasedev->name, + device_state); + return rret ? rret : -EIO; + } + return ret; + } + + migration->device_state = device_state; + trace_vfio_migration_set_state(vbasedev->name, device_state); + return 0; +} + +static void vfio_vmstate_change(void *opaque, int running, RunState state) +{ + VFIODevice *vbasedev = opaque; + VFIOMigration *migration = vbasedev->migration; + uint32_t value, mask; + int ret; + + if ((vbasedev->migration->vm_running == running)) { + return; + } + + if (running) { + /* + * Here device state can have one of _SAVING, _RESUMING or _STOP bit. + * Transition from _SAVING to _RUNNING can happen if there is migration + * failure, in that case clear _SAVING bit. + * Transition from _RESUMING to _RUNNING occurs during resuming + * phase, in that case clear _RESUMING bit. + * In both the above cases, set _RUNNING bit. + */ + mask = ~VFIO_DEVICE_STATE_MASK; + value = VFIO_DEVICE_STATE_RUNNING; + } else { + /* + * Here device state could be either _RUNNING or _SAVING|_RUNNING. Reset + * _RUNNING bit + */ + mask = ~VFIO_DEVICE_STATE_RUNNING; + value = 0; + } + + ret = vfio_migration_set_state(vbasedev, mask, value); + if (ret) { + /* + * Migration should be aborted in this case, but vm_state_notify() + * currently does not support reporting failures. + */ + error_report("%s: Failed to set device state 0x%x", vbasedev->name, + (migration->device_state & mask) | value); + qemu_file_set_error(migrate_get_current()->to_dst_file, ret); + } + vbasedev->migration->vm_running = running; + trace_vfio_vmstate_change(vbasedev->name, running, RunState_str(state), + (migration->device_state & mask) | value); +} static void vfio_migration_exit(VFIODevice *vbasedev) { @@ -64,6 +216,9 @@ static int vfio_migration_init(VFIODevice *vbasedev, ret = -EINVAL; goto err; } + + migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change, + vbasedev); return 0; err: @@ -111,6 +266,9 @@ add_blocker: void vfio_migration_finalize(VFIODevice *vbasedev) { if (vbasedev->migration) { + VFIOMigration *migration = vbasedev->migration; + + qemu_del_vm_change_state_handler(migration->vm_state); vfio_migration_exit(vbasedev); } diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events index 9ced5ec6277c..41de81f12f60 100644 --- a/hw/vfio/trace-events +++ b/hw/vfio/trace-events @@ -148,3 +148,5 @@ vfio_display_edid_write_error(void) "" # migration.c vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d" +vfio_migration_set_state(const char *name, uint32_t state) " (%s) state %d" +vfio_vmstate_change(const char *name, int running, const char *reason, uint32_t dev_state) " (%s) running %d reason %s device state %d" diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index 8275c4c68f45..9a571f1fb552 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -29,6 +29,7 @@ #ifdef CONFIG_LINUX #include <linux/vfio.h> #endif +#include "sysemu/sysemu.h" #define VFIO_MSG_PREFIX "vfio %s: " @@ -58,7 +59,10 @@ typedef struct VFIORegion { } VFIORegion; typedef struct VFIOMigration { + VMChangeStateEntry *vm_state; VFIORegion region; + uint32_t device_state; + int vm_running; } VFIOMigration; typedef struct VFIOAddressSpace {